Michael137 created this revision.
Michael137 added reviewers: aprantl, dblaikie.
Herald added a project: All.
Michael137 added a comment.
Michael137 updated this revision to Diff 484341.
Michael137 added a reviewer: labath.
Michael137 edited the summary of this revision.
Michael137 added a reviewer: aaron.ballman.
Michael137 updated this revision to Diff 484434.
Michael137 updated this revision to Diff 484439.
Michael137 published this revision for review.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

An example of how this would be used from LLDB is: 
https://reviews.llvm.org/D140145


Michael137 added a comment.

- Remove debug print in test


aprantl added a comment.

Overall this seems to have a pretty small surface area, so I'd be fine with 
including this.


Michael137 added a comment.

@aaron.ballman we're trying to fix printing of C++ types with default template 
arguments in LLDB. Currently DWARF doesn't have the necessary info to construct 
a `ClassTemplateDecl` with generic parameters, which means the logic for 
suppressing them in the `TypePrinter` doesn't quite work.

This patch outlines of one approach we considered. We add a hook into the 
TypePrinter to allow external AST sources to answer the question "is template 
argument X defaulted?".

Another alternative would be to have a `TemplateArgument::IsDefaulted` which 
gets set in LLDB and read from the TypePrinter.

Do you think either of these approaches would be fine for inclusion?


Michael137 added a comment.

- Fix test. Move variable into loop


Michael137 added a comment.

- Rebase


Michael137 added a comment.

putting into review queue to hear people's opinion on something along these 
lines



================
Comment at: clang/include/clang/AST/PrettyPrinter.h:52
+
+  enum class TriState : int { kYes, kNo, kUnknown };
+
----------------
The TrisState is needed because LLDB has two AST sources:
1. DWARF
2. clang modules

When we import a type from a clang module we would never have consulted DWARF 
about whether a parameter was defaulted. So instead of more complex bookkeeping 
it seemed simpler to say "if we've never seen this decl when parsing DWARF, I 
can't say anything about the default-ness of the arguments"


================
Comment at: clang/lib/AST/TypePrinter.cpp:2095
+
+  while (!Args.empty()) {
+    if (Callbacks != nullptr)
----------------
maybe this body becomes slightly less oddly indented when converting it to a 
range-based for with a counter?


This patch adds a hook into the `TypePrinter` to allow clients
decide whether a template argument corresponds to the default
parameter.

**Motivation**

DWARF encodes information about template instantiations, but
not about the generic definition of a template. If an argument
in a template instantiation corresponds to the default parameter
of the generic template then DWARF will attach a `DW_AT_default_value`
to that argument. LLDB uses the Clang TypePrinter for
displaying/formatting C++ types but currently can't give Clang the 
`ClassTemplateDecl`
that it expects. Since LLDB does know whether a particular instantiation has
default arguments, this patch allows LLDB (and other clients with external AST 
sources)
to help Clang in identifying those default arguments.

**Alternatives**

1. Encode information about generic template definitions in DWARF. It's unclear 
how long it would take to get this proposed and implemented and whether 
something like this would work in the first place. If we could construct 
`ClassTemplateDecl`s with the correct generic parameters this patch wouldn't be 
required.

2. Add something like a `bool IsDefaulted` somewhere in Clang, e.g., in 
`TemplateArgument` and consult it from the `TypePrinter`. This would be much 
simpler but requires adding a field on one of the Clang types


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D140423

Files:
  clang/include/clang/AST/PrettyPrinter.h
  clang/include/clang/AST/Type.h
  clang/lib/AST/TypePrinter.cpp
  clang/unittests/AST/TypePrinterTest.cpp

Index: clang/unittests/AST/TypePrinterTest.cpp
===================================================================
--- clang/unittests/AST/TypePrinterTest.cpp
+++ clang/unittests/AST/TypePrinterTest.cpp
@@ -127,3 +127,39 @@
         Policy.EntireContentsOfLargeArray = true;
       }));
 }
+
+TEST(TypePrinter, TemplateParameterListWithCallback) {
+  constexpr char Code[] = R"cpp(
+    template <typename T1,         
+              typename T2 = double,
+              typename T3 = int,   
+              int      T4 = 42>    
+    struct Foo {                   
+    };                             
+                                   
+    Foo<char> X;                   
+  )cpp";
+  auto Matcher = classTemplateSpecializationDecl(
+      hasName("Foo"), has(cxxConstructorDecl(
+                          has(parmVarDecl(hasType(qualType().bind("id")))))));
+
+  struct DefaulterCallback final : public PrintingCallbacks {
+    virtual TriState
+    IsTemplateArgumentDefaulted(clang::ClassTemplateSpecializationDecl const *,
+                                size_t ArgIndex) const {
+      if (ArgIndex > 1)
+        return TriState::kYes;
+
+      return TriState::kNo;
+    }
+  };
+
+  DefaulterCallback Callback;
+
+  ASSERT_TRUE(PrintedTypeMatches(Code, {""}, Matcher,
+                                 R"(const Foo<char, double> &)",
+                                 [&Callback](PrintingPolicy &Policy) {
+                                   Policy.SuppressDefaultTemplateArgs = true;
+                                   Policy.Callbacks = &Callback;
+                                 }));
+}
Index: clang/lib/AST/TypePrinter.cpp
===================================================================
--- clang/lib/AST/TypePrinter.cpp
+++ clang/lib/AST/TypePrinter.cpp
@@ -1285,7 +1285,7 @@
     const TemplateArgumentList &TemplateArgs = Spec->getTemplateArgs();
     printTemplateArgumentList(
         OS, TemplateArgs.asArray(), Policy,
-        Spec->getSpecializedTemplate()->getTemplateParameters());
+        Spec->getSpecializedTemplate()->getTemplateParameters(), Spec);
     OS << "::";
   } else if (const auto *Tag = dyn_cast<TagDecl>(DC)) {
     AppendScope(DC->getParent(), OS, Tag->getDeclName());
@@ -1383,7 +1383,7 @@
     IncludeStrongLifetimeRAII Strong(Policy);
     printTemplateArgumentList(
         OS, Args, Policy,
-        Spec->getSpecializedTemplate()->getTemplateParameters());
+        Spec->getSpecializedTemplate()->getTemplateParameters(), Spec);
   }
 
   spaceBeforePlaceHolder(OS);
@@ -2079,18 +2079,37 @@
 }
 
 template <typename TA>
-static ArrayRef<TA> dropDefaultedArguments(ArrayRef<TA> Args,
-                                           const PrintingPolicy &Policy,
-                                           const TemplateParameterList *TPL) {
+static ArrayRef<TA>
+dropDefaultedArguments(ArrayRef<TA> Args, const PrintingPolicy &Policy,
+                       const TemplateParameterList *TPL,
+                       const ClassTemplateSpecializationDecl *CTSD) {
+  using TriState = PrintingCallbacks::TriState;
+
   ASTContext &Ctx = TPL->getParam(0)->getASTContext();
   llvm::SmallVector<TemplateArgument, 8> OrigArgs;
   for (const TA &A : Args)
     OrigArgs.push_back(getArgument(A));
-  while (!Args.empty() &&
-         isSubstitutedDefaultArgument(Ctx, getArgument(Args.back()),
-                                      TPL->getParam(Args.size() - 1), OrigArgs,
-                                      TPL->getDepth()))
+
+  auto const *Callbacks = Policy.Callbacks;
+
+  while (!Args.empty()) {
+    auto IsDefaulted = PrintingCallbacks::TriState::kUnknown;
+    if (Callbacks != nullptr)
+      IsDefaulted =
+          Callbacks->IsTemplateArgumentDefaulted(CTSD, Args.size() - 1);
+
+    if (IsDefaulted == TriState::kUnknown)
+      IsDefaulted = isSubstitutedDefaultArgument(Ctx, getArgument(Args.back()),
+                                                 TPL->getParam(Args.size() - 1),
+                                                 OrigArgs, TPL->getDepth())
+                        ? TriState::kYes
+                        : TriState::kNo;
+
+    if (IsDefaulted != TriState::kYes)
+      break;
+
     Args = Args.drop_back();
+  }
 
   return Args;
 }
@@ -2098,12 +2117,13 @@
 template <typename TA>
 static void
 printTo(raw_ostream &OS, ArrayRef<TA> Args, const PrintingPolicy &Policy,
-        const TemplateParameterList *TPL, bool IsPack, unsigned ParmIndex) {
+        const TemplateParameterList *TPL, bool IsPack, unsigned ParmIndex,
+        const ClassTemplateSpecializationDecl *CTSD) {
   // Drop trailing template arguments that match default arguments.
   if (TPL && Policy.SuppressDefaultTemplateArgs &&
       !Policy.PrintCanonicalTypes && !Args.empty() && !IsPack &&
       Args.size() <= TPL->size()) {
-    Args = dropDefaultedArguments(Args, Policy, TPL);
+    Args = dropDefaultedArguments(Args, Policy, TPL, CTSD);
   }
 
   const char *Comma = Policy.MSVCFormatting ? "," : ", ";
@@ -2121,7 +2141,7 @@
       if (Argument.pack_size() && !FirstArg)
         OS << Comma;
       printTo(ArgOS, Argument.getPackAsArray(), Policy, TPL,
-              /*IsPack*/ true, ParmIndex);
+              /*IsPack*/ true, ParmIndex, CTSD);
     } else {
       if (!FirstArg)
         OS << Comma;
@@ -2170,14 +2190,21 @@
                                       ArrayRef<TemplateArgument> Args,
                                       const PrintingPolicy &Policy,
                                       const TemplateParameterList *TPL) {
-  printTo(OS, Args, Policy, TPL, /*isPack*/ false, /*parmIndex*/ 0);
+  printTo(OS, Args, Policy, TPL, /*isPack*/ false, /*parmIndex*/ 0, nullptr);
 }
 
 void clang::printTemplateArgumentList(raw_ostream &OS,
                                       ArrayRef<TemplateArgumentLoc> Args,
                                       const PrintingPolicy &Policy,
                                       const TemplateParameterList *TPL) {
-  printTo(OS, Args, Policy, TPL, /*isPack*/ false, /*parmIndex*/ 0);
+  printTo(OS, Args, Policy, TPL, /*isPack*/ false, /*parmIndex*/ 0, nullptr);
+}
+
+void clang::printTemplateArgumentList(
+    raw_ostream &OS, ArrayRef<TemplateArgument> Args,
+    const PrintingPolicy &Policy, const TemplateParameterList *TPL,
+    const clang::ClassTemplateSpecializationDecl *CTSD) {
+  printTo(OS, Args, Policy, TPL, /*isPack*/ false, /*parmIndex*/ 0, CTSD);
 }
 
 std::string Qualifiers::getAsString() const {
Index: clang/include/clang/AST/Type.h
===================================================================
--- clang/include/clang/AST/Type.h
+++ clang/include/clang/AST/Type.h
@@ -63,6 +63,7 @@
 class TagDecl;
 class TemplateParameterList;
 class Type;
+class ClassTemplateSpecializationDecl;
 
 enum {
   TypeAlignmentInBits = 4,
@@ -5463,6 +5464,11 @@
                                const PrintingPolicy &Policy,
                                const TemplateParameterList *TPL = nullptr);
 
+void printTemplateArgumentList(
+    raw_ostream &OS, ArrayRef<TemplateArgument> Args,
+    const PrintingPolicy &Policy, const TemplateParameterList *TPL,
+    const clang::ClassTemplateSpecializationDecl *CTSD);
+
 /// Make a best-effort determination of whether the type T can be produced by
 /// substituting Args into the default argument of Param.
 bool isSubstitutedDefaultArgument(ASTContext &Ctx, TemplateArgument Arg,
Index: clang/include/clang/AST/PrettyPrinter.h
===================================================================
--- clang/include/clang/AST/PrettyPrinter.h
+++ clang/include/clang/AST/PrettyPrinter.h
@@ -21,6 +21,7 @@
 class DeclContext;
 class LangOptions;
 class Stmt;
+class ClassTemplateSpecializationDecl;
 
 class PrinterHelper {
 public:
@@ -47,6 +48,14 @@
   /// The printing stops at the first isScopeVisible() == true, so there will
   /// be no calls with outer scopes.
   virtual bool isScopeVisible(const DeclContext *DC) const { return false; }
+
+  enum class TriState : int { kYes, kNo, kUnknown };
+
+  virtual TriState
+  IsTemplateArgumentDefaulted(clang::ClassTemplateSpecializationDecl const *D,
+                              size_t ArgIndex) const {
+    return TriState::kNo;
+  }
 };
 
 /// Describes how types, statements, expressions, and declarations should be
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to