[PATCH] D152520: [clangd] Unify printing policy for type hints

[Younan Zhang via Phabricator via cfe-commits]

zyounan created this revision.
zyounan added a reviewer: nridge.
Herald added subscribers: kadircet, arphaman.
Herald added a project: All.
zyounan published this revision for review.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang-tools-extra.

(This patch addresses the comment from https://reviews.llvm.org/D151785#4402460.)

Previously, we used a special printing policy that enabled `PrintCanonicalTypes`
to print type hints for structure bindings. This was intended to
eliminate type aliases like `tuple_element::type`. However, this also
caused TypePrinter to print default template arguments, which could
result in losing the ability to see types like `std::basic_string<char>`
if the fully expanded template-id exceeded the default inlay hint threshold.

Simply getting the canonical type at the call site could help us get rid of
the side effect.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D152520

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
@@ -1347,8 +1347,11 @@
     struct A {};
     A<float> foo();
     auto $var[[var]] = foo();
+    A<float> bar[1];
+    auto [$binding[[value]]] = bar;
   )cpp",
-                  ExpectedHint{": A<float>", "var"});
+                  ExpectedHint{": A<float>", "var"},
+                  ExpectedHint{": A<float>", "binding"});
 }
 
 TEST(TypeHints, Deduplication) {
Index: clang-tools-extra/clangd/InlayHints.cpp
===================================================================
--- clang-tools-extra/clangd/InlayHints.cpp
+++ clang-tools-extra/clangd/InlayHints.cpp
@@ -198,8 +198,7 @@
         Cfg(Cfg), RestrictRange(std::move(RestrictRange)),
         MainFileID(AST.getSourceManager().getMainFileID()),
         Resolver(AST.getHeuristicResolver()),
-        TypeHintPolicy(this->AST.getPrintingPolicy()),
-        StructuredBindingPolicy(this->AST.getPrintingPolicy()) {
+        TypeHintPolicy(this->AST.getPrintingPolicy()) {
     bool Invalid = false;
     llvm::StringRef Buf =
         AST.getSourceManager().getBufferData(MainFileID, &Invalid);
@@ -209,14 +208,8 @@
     TypeHintPolicy.AnonymousTagLocations =
         false; // do not print lambda locations
 
-    // 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.
     // Not setting PrintCanonicalTypes for "auto" allows
     // SuppressDefaultTemplateArgs (set by default) to have an effect.
-    StructuredBindingPolicy = TypeHintPolicy;
-    StructuredBindingPolicy.PrintCanonicalTypes = true;
   }
 
   bool VisitTypeLoc(TypeLoc TL) {
@@ -298,8 +291,12 @@
     // but show hints for the individual bindings.
     if (auto *DD = dyn_cast<DecompositionDecl>(D)) {
       for (auto *Binding : DD->bindings()) {
-        addTypeHint(Binding->getLocation(), Binding->getType(), /*Prefix=*/": ",
-                    StructuredBindingPolicy);
+        // 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".
+        if (auto Type = Binding->getType(); !Type.isNull())
+          addTypeHint(Binding->getLocation(), Type.getCanonicalType(),
+                      /*Prefix=*/": ", TypeHintPolicy);
       }
       return true;
     }
@@ -707,14 +704,7 @@
   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;
 };
 
 } // namespace


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D152520.529907.patch
Type: text/x-patch
Size: 3350 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20230609/50447a79/attachment.bin>