[clang-tools-extra] 3e03d92 - [clangd] Omit default template arguments from type hints

[Nathan Ridge via cfe-commits]

Author: Nathan Ridge
Date: 2021-09-07T02:38:27-04:00
New Revision: 3e03d92e2f4ad150469646c1b140ae6abb256c82

URL: https://github.com/llvm/llvm-project/commit/3e03d92e2f4ad150469646c1b140ae6abb256c82
DIFF: https://github.com/llvm/llvm-project/commit/3e03d92e2f4ad150469646c1b140ae6abb256c82.diff

LOG: [clangd] Omit default template arguments from type hints

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

Added: 
    

Modified: 
    clang-tools-extra/clangd/InlayHints.cpp
    clang-tools-extra/clangd/unittests/InlayHintTests.cpp

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clangd/InlayHints.cpp b/clang-tools-extra/clangd/InlayHints.cpp
index bea5306daeb7..167f8001d6e4 100644
--- a/clang-tools-extra/clangd/InlayHints.cpp
+++ b/clang-tools-extra/clangd/InlayHints.cpp
@@ -24,7 +24,8 @@ class InlayHintVisitor : public RecursiveASTVisitor<InlayHintVisitor> {
       : Results(Results), AST(AST.getASTContext()),
         MainFileID(AST.getSourceManager().getMainFileID()),
         Resolver(AST.getHeuristicResolver()),
-        TypeHintPolicy(this->AST.getPrintingPolicy()) {
+        TypeHintPolicy(this->AST.getPrintingPolicy()),
+        StructuredBindingPolicy(this->AST.getPrintingPolicy()) {
     bool Invalid = false;
     llvm::StringRef Buf =
         AST.getSourceManager().getBufferData(MainFileID, &Invalid);
@@ -33,14 +34,16 @@ class InlayHintVisitor : public RecursiveASTVisitor<InlayHintVisitor> {
     TypeHintPolicy.SuppressScope = true; // keep type names short
     TypeHintPolicy.AnonymousTagLocations =
         false; // do not print lambda locations
-    // Print canonical types. Otherwise, SuppressScope would result in
-    // things like "metafunction<args>::type" being shorted to just "type",
-    // which is useless. This is particularly important for structured
-    // bindings that use the tuple_element protocol, where the non-canonical
-    // types would be "tuple_element<I, A>::type".
-    // Note, for "auto", we would often prefer sugared types, but the AST
-    // doesn't currently retain them in DeducedType anyways.
-    TypeHintPolicy.PrintCanonicalTypes = true;
+
+    // For structured bindings, print canonical types. This is important because
+    // for bindings that use the tuple_element protocol, the non-canonical types
+    // would be "tuple_element<I, A>::type".
+    // For "auto", we often prefer sugared types, but the AST doesn't currently
+    // retain them in DeducedType. However, not setting PrintCanonicalTypes for
+    // "auto" at least allows SuppressDefaultTemplateArgs (set by default) to
+    // have an effect.
+    StructuredBindingPolicy = TypeHintPolicy;
+    StructuredBindingPolicy.PrintCanonicalTypes = true;
   }
 
   bool VisitCXXConstructExpr(CXXConstructExpr *E) {
@@ -98,7 +101,8 @@ class InlayHintVisitor : public RecursiveASTVisitor<InlayHintVisitor> {
     // but show hints for the individual bindings.
     if (auto *DD = dyn_cast<DecompositionDecl>(D)) {
       for (auto *Binding : DD->bindings()) {
-        addTypeHint(Binding->getLocation(), Binding->getType(), ": ");
+        addTypeHint(Binding->getLocation(), Binding->getType(), ": ",
+                    StructuredBindingPolicy);
       }
       return true;
     }
@@ -327,11 +331,16 @@ class InlayHintVisitor : public RecursiveASTVisitor<InlayHintVisitor> {
   }
 
   void addTypeHint(SourceRange R, QualType T, llvm::StringRef Prefix) {
+    addTypeHint(R, T, Prefix, TypeHintPolicy);
+  }
+
+  void addTypeHint(SourceRange R, QualType T, llvm::StringRef Prefix,
+                   const PrintingPolicy &Policy) {
     // Do not print useless "NULL TYPE" hint.
     if (!T.getTypePtrOrNull())
       return;
 
-    std::string TypeName = T.getAsString(TypeHintPolicy);
+    std::string TypeName = T.getAsString(Policy);
     if (TypeName.length() < TypeNameLimit)
       addInlayHint(R, InlayHintKind::TypeHint, std::string(Prefix) + TypeName);
   }
@@ -341,7 +350,14 @@ class InlayHintVisitor : public RecursiveASTVisitor<InlayHintVisitor> {
   FileID MainFileID;
   StringRef MainFileBuf;
   const HeuristicResolver *Resolver;
+  // We want to suppress default template arguments, but otherwise print
+  // canonical types. Unfortunately, they're conflicting policies so we can't
+  // have both. For regular types, suppressing template arguments is more
+  // important, whereas printing canonical types is crucial for structured
+  // bindings, so we use two separate policies. (See the constructor where
+  // the policies are initialized for more details.)
   PrintingPolicy TypeHintPolicy;
+  PrintingPolicy StructuredBindingPolicy;
 
   static const size_t TypeNameLimit = 32;
 };

diff  --git a/clang-tools-extra/clangd/unittests/InlayHintTests.cpp b/clang-tools-extra/clangd/unittests/InlayHintTests.cpp
index 62930d1f42b1..cda2ba41e264 100644
--- a/clang-tools-extra/clangd/unittests/InlayHintTests.cpp
+++ b/clang-tools-extra/clangd/unittests/InlayHintTests.cpp
@@ -602,6 +602,16 @@ TEST(TypeHints, LongTypeName) {
   )cpp");
 }
 
+TEST(TypeHints, DefaultTemplateArgs) {
+  assertTypeHints(R"cpp(
+    template <typename, typename = int>
+    struct A {};
+    A<float> foo();
+    auto $var[[var]] = foo();
+  )cpp",
+                  ExpectedHint{": A<float>", "var"});
+}
+
 // FIXME: Low-hanging fruit where we could omit a type hint:
 //  - auto x = TypeName(...);
 //  - auto x = (TypeName) (...);