[clang] [clang] Improve ast-dumper text printing of TemplateArgument (PR #93431)

[Matheus Izvekov via cfe-commits]

================
@@ -1086,45 +1106,100 @@ void TextNodeDumper::VisitNullTemplateArgument(const 
TemplateArgument &) {
 
 void TextNodeDumper::VisitTypeTemplateArgument(const TemplateArgument &TA) {
   OS << " type";
-  dumpType(TA.getAsType());
+  dumpTemplateArgument(TA);
 }
 
 void TextNodeDumper::VisitDeclarationTemplateArgument(
     const TemplateArgument &TA) {
   OS << " decl";
+  dumpTemplateArgument(TA);
   dumpDeclRef(TA.getAsDecl());
 }
 
-void TextNodeDumper::VisitNullPtrTemplateArgument(const TemplateArgument &) {
+void TextNodeDumper::VisitNullPtrTemplateArgument(const TemplateArgument &TA) {
   OS << " nullptr";
+  dumpTemplateArgument(TA);
 }
 
 void TextNodeDumper::VisitIntegralTemplateArgument(const TemplateArgument &TA) 
{
-  OS << " integral " << TA.getAsIntegral();
+  OS << " integral";
+  dumpTemplateArgument(TA);
+}
+
+void TextNodeDumper::dumpTemplateName(TemplateName TN) {
+  switch (TN.getKind()) {
+  case TemplateName::Template:
+    AddChild([=] { Visit(TN.getAsTemplateDecl()); });
+    return;
+  case TemplateName::UsingTemplate: {
+    const UsingShadowDecl *USD = TN.getAsUsingShadowDecl();
+    AddChild([=] { Visit(USD); });
+    AddChild("target", [=] { Visit(USD->getTargetDecl()); });
+    return;
+  }
+  case TemplateName::QualifiedTemplate: {
+    OS << " qualified";
+    const QualifiedTemplateName *QTN = TN.getAsQualifiedTemplateName();
+    if (QTN->hasTemplateKeyword())
+      OS << " keyword";
+    dumpNestedNameSpecifier(QTN->getQualifier());
+    dumpTemplateName(QTN->getUnderlyingTemplate());
+    return;
+  }
+  case TemplateName::DependentTemplate: {
+    OS << " dependent";
+    const DependentTemplateName *DTN = TN.getAsDependentTemplateName();
+    dumpNestedNameSpecifier(DTN->getQualifier());
+    return;
+  }
+  case TemplateName::SubstTemplateTemplateParm: {
+    const SubstTemplateTemplateParmStorage *STS =
+        TN.getAsSubstTemplateTemplateParm();
+    OS << " subst index " << STS->getIndex();
+    if (std::optional<unsigned int> PackIndex = STS->getPackIndex())
+      OS << " pack_index " << *PackIndex;
----------------
mizvekov wrote:

Oh, okay, I see the confusion.

`subst index` in this case is not one thing, they are two separate things. It 
only looks that way because we are avoiding writing to the stream two separate 
times. Ie see the other TemplateArgument kinds above.

`subst` refers to the kind of TemplateArgument, ie: type, expr, template, 
subst, subst_pack, etc.
`index 0` refers to the position in the parameter list this substitution 
represents.
`pack_index 0` refers to the position in the pack, in case `index` above points 
to a pack parameter. This 'pack_index` is the term we already use in the dumper 
for other Subst nodes.

I agree this is not great, but the way we format this text ast-dumper leads to 
this sort of confusion.

Any other suggestions?

https://github.com/llvm/llvm-project/pull/93431
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits