[clangd] Desugar template parameter aliases in type hints

This patch alleviates clangd/clangd#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<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. 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: dde8a0f

Reviewed By: nridge

Differential Revision: https://reviews.llvm.org/D151785

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<SubstTemplateTypeParmType>())
+      return true;
+    QT = Desugared;
+  }
+  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.
+//
+// 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<InlayHintVisitor> {
 public:
   InlayHintVisitor(std::vector<InlayHint> &Results, ParsedAST &AST,
@@ -663,22 +723,7 @@ class InlayHintVisitor : public RecursiveASTVisitor<InlayHintVisitor> {
                  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 @@ class InlayHintVisitor : public RecursiveASTVisitor<InlayHintVisitor> {
     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 @@ class InlayHintVisitor : public RecursiveASTVisitor<InlayHintVisitor> {
                  /*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;

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 <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 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 <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{": size_type", "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; };