From 7d68f2ef411ea2188666c2f67a8ee8b923adb12d Mon Sep 17 00:00:00 2001 From: Younan Zhang Date: Wed, 31 May 2023 14:49:32 +0800 Subject: [PATCH] [clangd] Desugar template parameter aliases in type hints This patch alleviates https://github.com/clangd/clangd/issues/1298. Containers in C++ such as `std::vector` or `llvm::SmallVector`, introduce a series of type aliases to adapt to generic algorithms. Currently, If we write an declarator involving expressions with these containers and `auto` placeholder, we probably obtain opaque type alias like following: ``` std::vector v = {1, 2, 3}; auto value = v[1]; // hint for `value`: value_type auto *ptr = &v[0]; // hint for `ptr`: value_type * ``` These hints are useless for most of the time. It would be nice if we desugar the type of `value_type` and print `int`, `int *` respectively in this situation. But note we can't always prefer desugared type since user might introduce type-aliases for brevity, where printing sugared types makes more sense. This patch introduces a heuristic method that displays the desugared type that is an alias of template parameter. It merges analogous method `shouldPrintCanonicalType` into `maybeDesugar` as well. Previous commit for shouldPrintCanonicalType: dde8a0fe91cc Reviewed By: nridge Differential Revision: https://reviews.llvm.org/D151785 --- clang-tools-extra/clangd/InlayHints.cpp | 93 +++++++++++++++++----- .../clangd/unittests/InlayHintTests.cpp | 76 ++++++++++++++++++ 2 files changed, 151 insertions(+), 18 deletions(-) diff --git a/clang-tools-extra/clangd/InlayHints.cpp b/clang-tools-extra/clangd/InlayHints.cpp index 5c78b4c..f81b515 100644 --- a/clang-tools-extra/clangd/InlayHints.cpp +++ b/clang-tools-extra/clangd/InlayHints.cpp @@ -11,10 +11,12 @@ #include "HeuristicResolver.h" #include "ParsedAST.h" #include "SourceCode.h" +#include "clang/AST/ASTDiagnostic.h" #include "clang/AST/Decl.h" #include "clang/AST/DeclarationName.h" #include "clang/AST/ExprCXX.h" #include "clang/AST/RecursiveASTVisitor.h" +#include "clang/AST/Type.h" #include "clang/Basic/Builtins.h" #include "clang/Basic/SourceManager.h" #include "llvm/ADT/ScopeExit.h" @@ -190,6 +192,64 @@ getDesignators(const InitListExpr *Syn) { return Designators; } +// Determines if any intermediate type in desugaring QualType QT is of +// substituted template parameter type. Ignore pointer or reference wrappers. +bool isSugaredTemplateParameter(QualType QT) { + static auto PeelWrappers = [](QualType QT) { + // Neither `PointerType` nor `ReferenceType` is considered as sugared + // type. Peel it. + QualType Next; + while (!(Next = QT->getPointeeType()).isNull()) + QT = Next; + return QT; + }; + while (true) { + QualType Desugared = + PeelWrappers(QT->getLocallyUnqualifiedSingleStepDesugaredType()); + if (Desugared == QT) + break; + if (Desugared->getAs()) + return true; + QT = Desugared; + } + return false; +} + +// A simple wrapper for `clang::desugarForDiagnostic` that provides optional +// semantic. +std::optional desugar(ASTContext &AST, QualType QT) { + bool ShouldAKA; + auto Desugared = clang::desugarForDiagnostic(AST, QT, ShouldAKA); + if (!ShouldAKA) + return std::nullopt; + return Desugared; +} + +// Apply a series of heuristic methods to determine whether or not a QualType QT +// is suitable for desugaring (e.g. getting the real name behind the using-alias +// name). If so, return the desugared type. Otherwise, return the unchanged +// parameter QT. +// +// This could be refined further. See +// https://github.com/clangd/clangd/issues/1298. +QualType maybeDesugar(ASTContext &AST, QualType QT) { + // Prefer desugared type for name that aliases the template parameters. + // This can prevent things like printing opaque `: type` when accessing std + // containers. + if (isSugaredTemplateParameter(QT)) + return desugar(AST, QT).value_or(QT); + + // Prefer desugared type for `decltype(expr)` specifiers. + if (QT->isDecltypeType()) + return QT.getCanonicalType(); + if (const AutoType *AT = QT->getContainedAutoType()) + if (!AT->getDeducedType().isNull() && + AT->getDeducedType()->isDecltypeType()) + return QT.getCanonicalType(); + + return QT; +} + class InlayHintVisitor : public RecursiveASTVisitor { public: InlayHintVisitor(std::vector &Results, ParsedAST &AST, @@ -663,22 +723,7 @@ private: sourceLocToPosition(SM, Spelled->back().endLocation())}; } - static bool shouldPrintCanonicalType(QualType QT) { - // The sugared type is more useful in some cases, and the canonical - // type in other cases. For now, prefer the sugared type unless - // we are printing `decltype(expr)`. This could be refined further - // (see https://github.com/clangd/clangd/issues/1298). - if (QT->isDecltypeType()) - return true; - if (const AutoType *AT = QT->getContainedAutoType()) - if (!AT->getDeducedType().isNull() && - AT->getDeducedType()->isDecltypeType()) - return true; - return false; - } - void addTypeHint(SourceRange R, QualType T, llvm::StringRef Prefix) { - TypeHintPolicy.PrintCanonicalTypes = shouldPrintCanonicalType(T); addTypeHint(R, T, Prefix, TypeHintPolicy); } @@ -687,9 +732,16 @@ private: if (!Cfg.InlayHints.DeducedTypes || T.isNull()) return; - std::string TypeName = T.getAsString(Policy); - if (Cfg.InlayHints.TypeNameLimit == 0 || - TypeName.length() < Cfg.InlayHints.TypeNameLimit) + // The sugared type is more useful in some cases, and the canonical + // type in other cases. + auto Desugared = maybeDesugar(AST, T); + std::string TypeName = Desugared.getAsString(Policy); + if (T != Desugared && !shouldPrintTypeHint(TypeName)) { + // If the desugared type is too long to display, fallback to the sugared + // type. + TypeName = T.getAsString(Policy); + } + if (shouldPrintTypeHint(TypeName)) addInlayHint(R, HintSide::Right, InlayHintKind::Type, Prefix, TypeName, /*Suffix=*/""); } @@ -699,6 +751,11 @@ private: /*Prefix=*/"", Text, /*Suffix=*/"="); } + bool shouldPrintTypeHint(llvm::StringRef TypeName) const noexcept { + return Cfg.InlayHints.TypeNameLimit == 0 || + TypeName.size() < Cfg.InlayHints.TypeNameLimit; + } + std::vector &Results; ASTContext &AST; const syntax::TokenBuffer &Tokens; diff --git a/clang-tools-extra/clangd/unittests/InlayHintTests.cpp b/clang-tools-extra/clangd/unittests/InlayHintTests.cpp index f400148..2a4e3ca 100644 --- a/clang-tools-extra/clangd/unittests/InlayHintTests.cpp +++ b/clang-tools-extra/clangd/unittests/InlayHintTests.cpp @@ -1417,6 +1417,82 @@ TEST(TypeHints, Decltype) { ExpectedHint{": int", "h"}, ExpectedHint{": int", "i"}); } +TEST(TypeHints, SubstTemplateParameterAliases) { + assertTypeHints( + R"cpp( + template struct allocator {}; + + template + struct vector_base { + using pointer = T*; + }; + + template + struct internal_iterator_type_template_we_dont_expect {}; + + struct my_iterator {}; + + template > + struct vector : vector_base { + using base = vector_base; + typedef T value_type; + typedef base::pointer pointer; + using allocator_type = A; + using size_type = int; + using iterator = internal_iterator_type_template_we_dont_expect; + using non_template_iterator = my_iterator; + + value_type& operator[](int index) { return elements[index]; } + const value_type& at(int index) const { return elements[index]; } + pointer data() { return &elements[0]; } + allocator_type get_allocator() { return A(); } + size_type size() const { return 10; } + iterator begin() { return iterator(); } + non_template_iterator end() { return non_template_iterator(); } + + T elements[10]; + }; + + vector array; + + auto $no_modifier[[by_value]] = array[3]; + auto* $ptr_modifier[[ptr]] = &array[3]; + auto& $ref_modifier[[ref]] = array[3]; + auto& $at[[immutable]] = array.at(3); + + auto $data[[data]] = array.data(); + auto $allocator[[alloc]] = array.get_allocator(); + auto $size[[size]] = array.size(); + auto $begin[[begin]] = array.begin(); + auto $end[[end]] = array.end(); + + + // If the type alias is not of substituted template parameter type, + // do not show desugared type. + using VeryLongLongTypeName = my_iterator; + using Short = VeryLongLongTypeName; + + auto $short_name[[my_value]] = Short(); + + // Same applies with templates. + template + using basic_static_vector = vector; + template + using static_vector = basic_static_vector>; + + auto $vector_name[[vec]] = static_vector(); + )cpp", + ExpectedHint{": int", "no_modifier"}, + ExpectedHint{": int *", "ptr_modifier"}, + ExpectedHint{": int &", "ref_modifier"}, + ExpectedHint{": const int &", "at"}, ExpectedHint{": int *", "data"}, + ExpectedHint{": allocator", "allocator"}, + ExpectedHint{": size_type", "size"}, ExpectedHint{": iterator", "begin"}, + ExpectedHint{": non_template_iterator", "end"}, + ExpectedHint{": Short", "short_name"}, + ExpectedHint{": static_vector", "vector_name"}); +} + TEST(DesignatorHints, Basic) { assertDesignatorHints(R"cpp( struct S { int x, y, z; }; -- 2.7.4