zyounan created this revision. Herald added subscribers: jeroen.dobbelaere, kadircet, arphaman. Herald added a project: All. zyounan requested review of this revision. Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov. Herald added a project: clang-tools-extra.
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: c++ std::vector<int> 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. However, things are complicated if user introduces type- alias for brevity: we don't want to make the length of hints be too long! This patch introduces a heuristic method that displays the desugared type for type-aliases that depend on some template parameters or simply reduce to builtin types e.g. int, double. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D151785 Files: clang-tools-extra/clangd/InlayHints.cpp clang-tools-extra/clangd/unittests/InlayHintTests.cpp
Index: clang-tools-extra/clangd/unittests/InlayHintTests.cpp =================================================================== --- clang-tools-extra/clangd/unittests/InlayHintTests.cpp +++ clang-tools-extra/clangd/unittests/InlayHintTests.cpp @@ -1417,6 +1417,81 @@ ExpectedHint{": int", "h"}, ExpectedHint{": int", "i"}); } +TEST(TypeHints, DependentAliases) { + assertTypeHints( + R"cpp( + template <class T> struct allocator {}; + + template <class T, class A> + struct vector_base { + using pointer = T*; + }; + + template <class T, class A> + struct internal_iterator_type_template_we_dont_expect {}; + + struct my_iterator {}; + + template <class T, class A = allocator<T>> + struct vector : vector_base<T, A> { + using base = vector_base<T, A>; + 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<T, A>; + 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<int> 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 type-dependent or non-trivial, do not show desugared type. + using VeryLongLongTypeName = my_iterator; + using Short = VeryLongLongTypeName; + + auto $short_name[[my_value]] = Short(); + + // Same applies with templates. + template <typename T, typename A> + using basic_static_vector = vector<T, A>; + template <typename T> + using static_vector = basic_static_vector<T, allocator<T>>; + + auto $vector_name[[vec]] = static_vector<int>(); + )cpp", + ExpectedHint{": int", "no_modifier"}, + ExpectedHint{": int *", "ptr_modifier"}, + ExpectedHint{": int &", "ref_modifier"}, + ExpectedHint{": const int &", "at"}, ExpectedHint{": int *", "data"}, + ExpectedHint{": allocator<int>", "allocator"}, + ExpectedHint{": int", "size"}, ExpectedHint{": iterator", "begin"}, + ExpectedHint{": non_template_iterator", "end"}, + ExpectedHint{": Short", "short_name"}, + ExpectedHint{": static_vector<int>", "vector_name"}); +} + TEST(DesignatorHints, Basic) { assertDesignatorHints(R"cpp( struct S { int x, y, z; }; Index: clang-tools-extra/clangd/InlayHints.cpp =================================================================== --- clang-tools-extra/clangd/InlayHints.cpp +++ 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,52 @@ return Designators; } +// Determines if any part in desugaring steps of QualType QT is a dependent or +// builtin type. Ignore pointer or reference qualifiers. +bool isDependentOrTrivialSugaredType(QualType QT) { + static auto PeelQualifier = [](QualType QT) { + // Neither `PointerType` nor `ReferenceType` is considered as sugared + // type. Peel it. + QualType Last = QT; + for (QT = QT->getPointeeType(); !QT.isNull(); + Last = QT, QT = QT->getPointeeType()) + ; + return Last; + }; + for (QualType Desugared = + PeelQualifier(QT->getLocallyUnqualifiedSingleStepDesugaredType()); + Desugared != QT; QT = Desugared, + Desugared = PeelQualifier( + Desugared + ->getLocallyUnqualifiedSingleStepDesugaredType())) { + if (Desugared->getAs<SubstTemplateTypeParmType>()) + return true; + if (Desugared->getAs<BuiltinType>()) + return true; + } + return false; +} + +// A simple wrapper for `clang::desugarForDiagnostic` that provides optional +// semantic. +std::optional<QualType> 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. +QualType tryDesugar(ASTContext &AST, QualType QT) { + if (isDependentOrTrivialSugaredType(QT)) + return desugar(AST, QT).value_or(QT); + return QT; +} + class InlayHintVisitor : public RecursiveASTVisitor<InlayHintVisitor> { public: InlayHintVisitor(std::vector<InlayHint> &Results, ParsedAST &AST, @@ -687,9 +735,14 @@ if (!Cfg.InlayHints.DeducedTypes || T.isNull()) return; - std::string TypeName = T.getAsString(Policy); - if (Cfg.InlayHints.TypeNameLimit == 0 || - TypeName.length() < Cfg.InlayHints.TypeNameLimit) + auto Desugared = tryDesugar(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 original + // type. + TypeName = T.getAsString(Policy); + } + if (shouldPrintTypeHint(TypeName)) addInlayHint(R, HintSide::Right, InlayHintKind::Type, Prefix, TypeName, /*Suffix=*/""); } @@ -699,6 +752,11 @@ /*Prefix=*/"", Text, /*Suffix=*/"="); } + bool shouldPrintTypeHint(llvm::StringRef TypeName) const noexcept { + return Cfg.InlayHints.TypeNameLimit == 0 || + TypeName.size() < Cfg.InlayHints.TypeNameLimit; + } + std::vector<InlayHint> &Results; ASTContext &AST; const syntax::TokenBuffer &Tokens;
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits