[clang] Improve ast-dumper text printing of TemplateArgument #93431
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
llvmbot
added
clang
|
@llvm/pr-subscribers-clang Author: Matheus Izvekov (mizvekov) ChangesThis improves and unifies our approach to printing all template arguments. The same approach to printing types is extended to all TemplateArguments: A sugared version is printed in quotes, followed by printing the canonical form, unless they would print the same. Special improvements are done to add more detail to template template arguments. It's planned in a future patch to use this improved TemplateName printer for other places besides TemplateArguments. Note: The sugared/desugared printing does not show up for TemplateNames in tests yet, because we do a poor job of preserving their type sugar. This will be improved in a future patch. Patch is 50.89 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/93431.diff 17 Files Affected:
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 825e91876ffce..403a107edef17 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -818,6 +818,11 @@ Miscellaneous Clang Crashes Fixed
when ``-fdump-record-layouts-complete`` is passed. Fixes #GH83684.
- Unhandled StructuralValues in the template differ (#GH93068).
+Miscellaneous Clang Improvements
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+- The text ast-dumper has improved printing of TemplateArguments.
+
OpenACC Specific Changes
------------------------
diff --git a/clang/include/clang/AST/TextNodeDumper.h b/clang/include/clang/AST/TextNodeDumper.h
index 1fede6e462e92..0d057b8011164 100644
--- a/clang/include/clang/AST/TextNodeDumper.h
+++ b/clang/include/clang/AST/TextNodeDumper.h
@@ -211,8 +211,11 @@ class TextNodeDumper
void dumpAccessSpecifier(AccessSpecifier AS);
void dumpCleanupObject(const ExprWithCleanups::CleanupObject &C);
void dumpTemplateSpecializationKind(TemplateSpecializationKind TSK);
+ void dumpBareNestedNameSpecifier(NestedNameSpecifier *NNS);
void dumpNestedNameSpecifier(const NestedNameSpecifier *NNS);
void dumpConceptReference(const ConceptReference *R);
+ void dumpTemplateArgument(const TemplateArgument &TA);
+ void dumpTemplateName(TemplateName TN);
void dumpDeclRef(const Decl *D, StringRef Label = {});
diff --git a/clang/lib/AST/TextNodeDumper.cpp b/clang/lib/AST/TextNodeDumper.cpp
index 4a1e94ffe283b..742203e3b946d 100644
--- a/clang/lib/AST/TextNodeDumper.cpp
+++ b/clang/lib/AST/TextNodeDumper.cpp
@@ -947,6 +947,26 @@ void TextNodeDumper::dumpDeclRef(const Decl *D, StringRef Label) {
});
}
+void TextNodeDumper::dumpTemplateArgument(const TemplateArgument &TA) {
+ llvm::SmallString<128> Str;
+ {
+ llvm::raw_svector_ostream SS(Str);
+ TA.print(PrintPolicy, SS, /*IncludeType=*/true);
+ }
+ OS << " '" << Str << "'";
+
+ if (TemplateArgument CanonTA = Context->getCanonicalTemplateArgument(TA);
+ !CanonTA.structurallyEquals(TA)) {
+ llvm::SmallString<128> CanonStr;
+ {
+ llvm::raw_svector_ostream SS(CanonStr);
+ CanonTA.print(PrintPolicy, SS, /*IncludeType=*/true);
+ }
+ if (CanonStr != Str)
+ OS << ":'" << CanonStr << "'";
+ }
+}
+
const char *TextNodeDumper::getCommandName(unsigned CommandID) {
if (Traits)
return Traits->getCommandInfo(CommandID)->Name;
@@ -1086,45 +1106,91 @@ 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;
+ }
+ // FIXME: Implement these.
+ case TemplateName::OverloadedTemplate:
+ OS << " overloaded";
+ return;
+ case TemplateName::AssumedTemplate:
+ OS << " assumed";
+ return;
+ case TemplateName::SubstTemplateTemplateParm:
+ OS << " subst";
+ return;
+ case TemplateName::SubstTemplateTemplateParmPack:
+ OS << " subst_pack";
+ return;
+ }
+ llvm_unreachable("Unexpected TemplateName Kind");
}
void TextNodeDumper::VisitTemplateTemplateArgument(const TemplateArgument &TA) {
- if (TA.getAsTemplate().getKind() == TemplateName::UsingTemplate)
- OS << " using";
- OS << " template ";
- TA.getAsTemplate().dump(OS);
+ OS << " template";
+ dumpTemplateArgument(TA);
+ dumpTemplateName(TA.getAsTemplate());
}
void TextNodeDumper::VisitTemplateExpansionTemplateArgument(
const TemplateArgument &TA) {
- if (TA.getAsTemplateOrTemplatePattern().getKind() ==
- TemplateName::UsingTemplate)
- OS << " using";
- OS << " template expansion ";
- TA.getAsTemplateOrTemplatePattern().dump(OS);
+ OS << " template expansion";
+ dumpTemplateArgument(TA);
+ dumpTemplateName(TA.getAsTemplateOrTemplatePattern());
}
-void TextNodeDumper::VisitExpressionTemplateArgument(const TemplateArgument &) {
+void TextNodeDumper::VisitExpressionTemplateArgument(
+ const TemplateArgument &TA) {
OS << " expr";
+ dumpTemplateArgument(TA);
}
-void TextNodeDumper::VisitPackTemplateArgument(const TemplateArgument &) {
+void TextNodeDumper::VisitPackTemplateArgument(const TemplateArgument &TA) {
OS << " pack";
+ dumpTemplateArgument(TA);
}
static void dumpBasePath(raw_ostream &OS, const CastExpr *Node) {
diff --git a/clang/test/AST/ast-dump-decl.cpp b/clang/test/AST/ast-dump-decl.cpp
index e062d4f068a40..b861ba8be15b5 100644
--- a/clang/test/AST/ast-dump-decl.cpp
+++ b/clang/test/AST/ast-dump-decl.cpp
@@ -459,21 +459,23 @@ namespace testClassTemplateDecl {
// CHECK: ClassTemplateDecl 0x{{.+}} <{{.+}}:[[@LINE-148]]:3, col:31> col:31 TestTemplateDefaultNonType{{$}}
// CHECK-NEXT: |-NonTypeTemplateParmDecl 0x{{.+}} <col:12, col:20> col:16 'int' depth 0 index 0 I{{$}}
-// CHECK-NEXT: | `-TemplateArgument <col:20> expr{{$}}
+// CHECK-NEXT: | `-TemplateArgument <col:20> expr '42'{{$}}
// CHECK-NEXT: | `-IntegerLiteral 0x{{.+}} <col:20> 'int' 42{{$}}
// CHECK-NEXT: `-CXXRecordDecl 0x{{.+}} <col:24, col:31> col:31 struct TestTemplateDefaultNonType{{$}}
// CHECK: ClassTemplateDecl 0x{{.+}} <{{.+}}:{{.*}}:3, col:68> col:68 TestTemplateTemplateDefaultType{{$}}
// CHECK-NEXT: |-TemplateTemplateParmDecl 0x{{.+}} <col:12, col:42> col:37 depth 0 index 0 TT{{$}}
// CHECK-NEXT: | |-TemplateTypeParmDecl 0x{{.+}} <col:21> col:29 typename depth 1 index 0{{$}}
-// CHECK-NEXT: | `-TemplateArgument <col:42> template TestClassTemplate{{$}}
-// CHECK-NEXT: `-CXXRecordDecl 0x{{.+}} <col:61, col:68> col:68 struct TestTemplateTemplateDefaultType{{$}}
+// CHECK-NEXT: | `-TemplateArgument <col:42> template 'testClassTemplateDecl::TestClassTemplate'{{$}}
+// CHECK-NEXT: | `-ClassTemplateDecl 0x{{.+}} <line:{{.+}}:3, line:{{.+}}:3> line:{{.+}}:30 TestClassTemplate{{$}}
+// CHECK-NEXT: `-CXXRecordDecl 0x{{.+}} <line:{{.*}}:61, col:68> col:68 struct TestTemplateTemplateDefaultType{{$}}
// CHECK: ClassTemplateDecl 0x{{.+}} prev 0x{{.+}} <{{.+}}:{{.*}}:3, col:82> col:48 TestTemplateTemplateDefaultType{{$}}
// CHECK-NEXT: |-TemplateTemplateParmDecl 0x{{.+}} <col:12, col:37> col:37 depth 0 index 0 TT{{$}}
// CHECK-NEXT: | |-TemplateTypeParmDecl 0x{{.+}} <col:21> col:29 typename depth 1 index 0{{$}}
-// CHECK-NEXT: | `-TemplateArgument <line:{{.*}}:42> template TestClassTemplate{{$}}
-// CHECK-NEXT: | `-inherited from TemplateTemplateParm 0x{{.+}} 'TT'{{$}}
+// CHECK-NEXT: | `-TemplateArgument <line:{{.*}}:42> template 'testClassTemplateDecl::TestClassTemplate'{{$}}
+// CHECK-NEXT: | |-inherited from TemplateTemplateParm 0x{{.+}} 'TT'{{$}}
+// CHECK-NEXT: | `-ClassTemplateDecl 0x{{.+}} <line:{{.+}}:3, line:{{.+}}:3> line:{{.+}}:30 TestClassTemplate
// CHECK-NEXT: `-CXXRecordDecl 0x{{.+}} prev 0x{{.+}} <line:{{.*}}:41, col:82> col:48 struct TestTemplateTemplateDefaultType definition{{$}}
// CHECK-NEXT: |-DefinitionData empty aggregate standard_layout trivially_copyable pod trivial literal has_constexpr_non_copy_move_ctor can_const_default_init{{$}}
// CHECK-NEXT: | |-DefaultConstructor exists trivial constexpr needs_implicit defaulted_is_constexpr{{$}}
@@ -683,7 +685,8 @@ namespace TestTemplateTemplateParmDecl {
// CHECK: FunctionTemplateDecl
// CHECK-NEXT: TemplateTemplateParmDecl{{.*}} T
// CHECK-NEXT: TemplateTypeParmDecl{{.*}} typename
-// CHECK-NEXT: TemplateArgument{{.*}} template A
+// CHECK-NEXT: TemplateArgument{{.*}} template 'TestTemplateTemplateParmDecl::A'
+// CHECK-NEXT: ClassTemplateDecl {{.*}} A
// CHECK-NEXT: TemplateTemplateParmDecl{{.*}} ... U
// CHECK-NEXT: TemplateTypeParmDecl{{.*}} typename
@@ -710,12 +713,12 @@ namespace TestTemplateArgument {
template<int> class testIntegral { };
template class testIntegral<1>;
// CHECK: ClassTemplateSpecializationDecl{{.*}} class testIntegral
- // CHECK: TemplateArgument{{.*}} integral 1
+ // CHECK: TemplateArgument{{.*}} integral '1'
template<template<typename> class> class testTemplate { };
template class testTemplate<A>;
// CHECK: ClassTemplateSpecializationDecl{{.*}} class testTemplate
- // CHECK: TemplateArgument{{.*}} A
+ // CHECK: TemplateArgument{{.*}} 'TestTemplateArgument::A'
template<template<typename> class ...T> class C {
B<T...> testTemplateExpansion;
@@ -731,10 +734,10 @@ namespace TestTemplateArgument {
template<int, int ...> class testPack { };
template class testPack<0, 1, 2>;
// CHECK: ClassTemplateSpecializationDecl{{.*}} class testPack
- // CHECK: TemplateArgument{{.*}} integral 0
+ // CHECK: TemplateArgument{{.*}} integral '0'
// CHECK-NEXT: TemplateArgument{{.*}} pack
- // CHECK-NEXT: TemplateArgument{{.*}} integral 1
- // CHECK-NEXT: TemplateArgument{{.*}} integral 2
+ // CHECK-NEXT: TemplateArgument{{.*}} integral '1'
+ // CHECK-NEXT: TemplateArgument{{.*}} integral '2'
}
namespace testUsingDecl {
diff --git a/clang/test/AST/ast-dump-openmp-begin-declare-variant_template_2.cpp b/clang/test/AST/ast-dump-openmp-begin-declare-variant_template_2.cpp
index da46cef7f3f1b..6fe05e33a5fb8 100644
--- a/clang/test/AST/ast-dump-openmp-begin-declare-variant_template_2.cpp
+++ b/clang/test/AST/ast-dump-openmp-begin-declare-variant_template_2.cpp
@@ -79,7 +79,7 @@ int test() {
// CHECK-NEXT: | | `-ReturnStmt [[ADDR_22:0x[a-z0-9]*]] <line:10:3, col:10>
// CHECK-NEXT: | | `-IntegerLiteral [[ADDR_23:0x[a-z0-9]*]] <col:10> 'int' 0
// CHECK-NEXT: | `-FunctionDecl [[ADDR_24:0x[a-z0-9]*]] <line:9:1, line:11:1> line:9:5 used also_before_mismatch 'int ({{.*}})'
-// CHECK-NEXT: | |-TemplateArgument integral 0
+// CHECK-NEXT: | |-TemplateArgument integral '0'
// CHECK-NEXT: | `-CompoundStmt [[ADDR_25:0x[a-z0-9]*]] <col:32, line:11:1>
// CHECK-NEXT: | `-ReturnStmt [[ADDR_26:0x[a-z0-9]*]] <line:10:3, col:10>
// CHECK-NEXT: | `-IntegerLiteral [[ADDR_23]] <col:10> 'int' 0
@@ -179,7 +179,7 @@ int test() {
// CHECK-NEXT: | | `-OMPDeclareVariantAttr [[ADDR_101:0x[a-z0-9]*]] <<invalid sloc>> Implicit implementation={extension(allow_templates)}
// CHECK-NEXT: | | `-DeclRefExpr [[ADDR_102:0x[a-z0-9]*]] <col:1> 'int ({{.*}})' {{.*}}Function [[ADDR_103:0x[a-z0-9]*]] 'only_def[implementation={extension(allow_templates)}]' 'int ({{.*}})'
// CHECK-NEXT: | `-FunctionDecl [[ADDR_104:0x[a-z0-9]*]] <col:1, col:18> col:5 used only_def 'int ({{.*}})'
-// CHECK-NEXT: | |-TemplateArgument integral 0
+// CHECK-NEXT: | |-TemplateArgument integral '0'
// CHECK-NEXT: | `-OMPDeclareVariantAttr [[ADDR_105:0x[a-z0-9]*]] <<invalid sloc>> Implicit implementation={extension(allow_templates)}
// CHECK-NEXT: | `-DeclRefExpr [[ADDR_106:0x[a-z0-9]*]] <col:1> 'int ({{.*}})' {{.*}}Function [[ADDR_107:0x[a-z0-9]*]] 'only_def[implementation={extension(allow_templates)}]' 'int ({{.*}})'
// CHECK-NEXT: |-FunctionTemplateDecl [[ADDR_108:0x[a-z0-9]*]] <line:37:1, line:40:1> line:38:1 only_def[implementation={extension(allow_templates)}]
@@ -189,7 +189,7 @@ int test() {
// CHECK-NEXT: | | `-ReturnStmt [[ADDR_110:0x[a-z0-9]*]] <line:39:3, col:10>
// CHECK-NEXT: | | `-IntegerLiteral [[ADDR_111:0x[a-z0-9]*]] <col:10> 'int' 0
// CHECK-NEXT: | `-FunctionDecl [[ADDR_107]] <line:38:1, line:40:1> line:38:1 only_def[implementation={extension(allow_templates)}] 'int ({{.*}})'
-// CHECK-NEXT: | |-TemplateArgument integral 0
+// CHECK-NEXT: | |-TemplateArgument integral '0'
// CHECK-NEXT: | `-CompoundStmt [[ADDR_112:0x[a-z0-9]*]] <col:20, line:40:1>
// CHECK-NEXT: | `-ReturnStmt [[ADDR_113:0x[a-z0-9]*]] <line:39:3, col:10>
// CHECK-NEXT: | `-IntegerLiteral [[ADDR_111]] <col:10> 'int' 0
diff --git a/clang/test/AST/ast-dump-template-name.cpp b/clang/test/AST/ast-dump-template-name.cpp
new file mode 100644
index 0000000000000..39100711b60a1
--- /dev/null
+++ b/clang/test/AST/ast-dump-template-name.cpp
@@ -0,0 +1,54 @@
+// RUN: %clang_cc1 -std=c++26 -ast-dump -ast-dump-filter=Test %s | FileCheck %s
+
+template <template <class> class TT> using N = TT<int>;
+
+namespace qualified {
+ namespace foo {
+ template <class T> struct A;
+ } // namespace foo
+ using TestQualified = N<foo::A>;
+} // namespace qualified
+
+// CHECK: Dumping qualified::TestQualified:
+// CHECK-NEXT: TypeAliasDecl
+// CHECK-NEXT: `-ElaboratedType
+// CHECK-NEXT: `-TemplateSpecializationType
+// CHECK-NEXT: |-TemplateArgument template 'qualified::foo::A' qualified{{$}}
+// CHECK-NEXT: | |-NestedNameSpecifier Namespace 0x{{.+}} 'foo'{{$}}
+// CHECK-NEXT: | `-ClassTemplateDecl {{.+}} A{{$}}
+
+namespace dependent {
+ template <class T> struct B {
+ using TestDependent = N<T::template X>;
+ };
+} // namespace dependent
+
+// CHECK: Dumping dependent::B::TestDependent:
+// CHECK-NEXT: TypeAliasDecl
+// CHECK-NEXT: `-ElaboratedType
+// CHECK-NEXT: `-TemplateSpecializationType
+// CHECK-NEXT: |-TemplateArgument template 'template X' dependent{{$}}
+// CHECK-NEXT: | `-NestedNameSpecifier TypeSpec 'T'{{$}}
+
+namespace subst {
+ template <class> struct A;
+
+ template <template <class> class TT> struct B {
+ template <template <class> class> struct C {};
+ using type = C<TT>;
+ };
+ using TestSubst = B<A>::type;
+} // namespace subst
+
+// CHECK: Dumping subst::TestSubst:
+// CHECK-NEXT: TypeAliasDecl
+// CHECK-NEXT: `-ElaboratedType
+// CHECK-NEXT: `-TypedefType
+// CHECK-NEXT: |-TypeAlias
+// CHECK-NEXT: `-ElaboratedType
+// CHECK-NEXT: `-TemplateSpecializationType
+// CHECK-NEXT: |-TemplateArgument template 'subst::A' subst index 0
+// CHECK-NEXT: | |-parameter: TemplateTemplateParmDecl {{.+}} depth 0 index 0 TT{{$}}
+// CHECK-NEXT: | |-associated ClassTemplateSpecialization {{.+}} 'B'{{$}}
+// CHECK-NEXT: | `-replacement:
+// CHECK-NEXT: | `-ClassTemplateDecl {{.+}} A{{$}}
diff --git a/clang/test/AST/ast-dump-using-template.cpp b/clang/test/AST/ast-dump-using-template.cpp
index de3ce277fd24f..69b199fd0606c 100644
--- a/clang/test/AST/ast-dump-using-template.cpp
+++ b/clang/test/AST/ast-dump-using-template.cpp
@@ -28,9 +28,11 @@ using B = X<S>;
// CHECK: TypeAliasDecl
// CHECK-NEXT: `-ElaboratedType {{.*}} 'X<ns::S>' sugar
// CHECK-NEXT: `-TemplateSpecializationType {{.*}} 'X<ns::S>' sugar X
-// CHECK-NEXT: |-TemplateArgument using template S
-// CHECK-NEXT: `-RecordType {{.*}} 'X<ns::S>'
-// CHECK-NEXT: `-ClassTemplateSpecialization {{.*}} 'X'
+// CHECK-NEXT: |-TemplateArgument template 'ns::S'
+// CHECK-NEXT: | |-UsingShadowDecl {{.*}} implicit ClassTemplate {{.*}} 'S'
+// CHECK-NEXT: | `-target: ClassTemplateDecl {{.*}} S
+// CHECK-NEXT: `-RecordType {{.*}} 'X<ns::S>'
+// CHECK-NEXT: `-ClassTemplateSpecialization {{.*}} 'X'
// TemplateName in DeducedTemplateSpecializationType.
S DeducedTemplateSpecializationT(123);
diff --git a/clang/test/AST/constraints-explicit-instantiation.cpp b/clang/test/AST/constraints-explicit-instantiation.cpp
index 10b6432f2db8c..79948ad1e8556 100644
--- a/clang/test/AST/constraints-explicit-instantiation.cpp
+++ b/clang/test/AST/constraints-explicit-instantiation.cpp
@@ -21,17 +21,17 @@ struct A {
// This checks that `canary1<1>` and `canaray2<2>` are instantiated, thus
// indirectly validating that the correct candidates of `A::f` were really
-// instantiated each time.
+// instantiated each time.
// The `static_assert`s validate we don't instantiate wrong candidates.
// CHECK:{{.*}}FunctionTemplateDecl {{.*}} canary1
// CHECK: {{.*}}TemplateArgument integral
-// CHECK-SAME: {{1$}}
+// CHECK-SAME: {{'1'$}}
template struct A<1>;
// CHECK: {{.*}}FunctionTemplateDecl {{.*}} canary2
// CHECK: {{.*}}TemplateArgument integral
-// CHECK-SAME: {{2$}}
+// CHECK-SAME: {{'2'$}}
template struct A<2>;
template struct A<3>;
diff --git a/clang/test/OpenMP/align_clause_ast_print.cpp b/clang/test/OpenMP/align_clause_ast_print.cpp
index 87000f9c41bae..c5e27a5d21d02 100644
--- a/clang/test/OpenMP/align_clause_ast_print.cpp
+++ b/clang/test/OpenMP/align_clause_ast_print.cpp
@@ -114,7 +114,7 @@ int template_test() {
// DUMP: FunctionDecl {{.*}}run 'double ()'
// DUMP: TemplateArgument type 'double'
// DUMP: BuiltinType {{.*}}'double'
-// DUMP: TemplateArgument integral 1
+// DUMP: TemplateArgument integral '1U'
// DUMP: OMPAllocateDeclAttr {{.*}}Implicit OMPNullMemAlloc
// DUMP: ConstantExpr {{.*}}'unsigned int'
// DUMP: value: Int 1
diff --git a/clang/test/OpenMP/generic_loop_ast_print.cpp b/clang/test/OpenMP/generic_loop_ast_print.cpp
index df806405571cf..b61ee79615d04 100644
--- a/clang/test/OpenMP/generic_loop_ast_print.cpp
+++ b/clang/test/OpenMP/generic_loop_ast_print.cpp
@@ -50,7 +50,7 @@
//PRINT: }
//DUMP: FunctionDecl{{.*}}templ_foo 'void (int)'
//DUMP: TemplateArgument type 'int'
-//DUMP: TemplateArgument integral 2
+//DUMP: TemplateArgument integral '2'
//DUMP: ParmVarDecl{{.*}}'int'
//DUMP: OMPSimdDirective
//DUMP: OMPCollapseClause
diff --git a/clang/test/OpenMP/interop_ast_print.cpp b/clang/test/OpenMP/interop_ast_print.cpp
index 7b9dda577c840..fed6febc63085 100644
--- a/clang/test/OpenMP/interop_ast_print.cpp
+++ b/clang/test/OpenMP/interop_ast_print.cpp
@@ -268,7 +268,7 @@ void fooTemp() {
//PRINT: #pragma omp interop init(prefer_type(3,4,"level_one"), target : interop_var)
//DUMP: FunctionDecl{{.*}}fooTemp
- //DUMP: TemplateArgument integral 3
+ //DUMP: TemplateArgument integral '3'
//DUMP: OMPInteropDirective
//DUMP: OMPInitClause
//DUMP: DeclRefExpr{{.*}}'omp_interop_t'{{.*}}'interop_var'
diff --git a/clang/test/SemaOpenACC/sub-array-ast.cpp b/clang/test/SemaOpenACC/sub-array-ast.cpp
index 094976e164275..43cc55a3f9a51 100644
--- a/clang/test/SemaOpenACC/sub-array-ast.cpp
+++ b/clang/test/SemaOpenACC/sub-array-ast.cpp
@@ -357,7 +357,7 @@ void Templ(int i){
// CHECK-NEXT: FunctionDecl{{.*}} Templ 'void (int)' implicit_instantiation
// CHECK-NEXT: TemplateArgument{{.*}} 'int'
// CHECK-NEXT: BuiltinType{{.*}} 'int'
- // CHECK-NEXT: TemplateArgument integral 3
+ // CHECK-NEXT: TemplateArgument integral '3U'
// CHECK-NEXT: TemplateArgument decl
// CHECK-NEXT: Var{{.*}} 'CEArray' 'const int[5]'
// CHECK-NEXT: ParmVarDecl{{.*}} i 'int'
diff --git a/clang/test/SemaTemplate/aggregate-deduction-candidate.cpp b/clang/test/SemaTemplate/aggregate-deduction-candidate.cpp
index 7f535651bb815..1ef651e9dfb38 100644
--- a/clang/test/SemaTemplate/aggregate-deduction-candidate.cpp
+++ b/clang/test/SemaTemplate/aggregate-deduction-candidate.cpp
@@ -58,7 +58,7 @@ namespace Basic {
D d2 = {1, 2, 3}; // cxx17-error {{no viable}}
D d3(1, 2); // expected-error {{no viable}}
- // CTAD succeed but brace elision is not allowed for parenthesized aggregate init.
+ // CTAD succeed but brace elision is not allowed for parenthesized aggregate init.
D d4(...
[truncated]
|
mizvekov
force-pushed
the
users/mizvekov/clang-improve-ast-dumper-template-arguments
branch
3 times, most recently
from
031e7c2
to
f9892eb
Compare
cor3ntin
reviewed
May 27, 2024
| llvm::raw_svector_ostream SS(Str); | ||
| TA.print(PrintPolicy, SS, /*IncludeType=*/true); | ||
| } | ||
| OS << " '" << Str << "'"; |
There was a problem hiding this comment.
I'm not sure how i feel about the quotes. It's particularly awkward in the char literal case
There was a problem hiding this comment.
Yeah, the quotes are what we already do for types, and I think the problem shows up there, though of course not as self-evident as here.
I don't think there is a formal solution besides to start escaping the string.
There was a problem hiding this comment.
Yes, lets keep the quotes. I'm not a fan but it appears consistent.
There was a problem hiding this comment.
I am going to fight over this but I feel like it makes less sense for integer literals and char literals. Expressions, type etc sure. Consistency for consistency sake while tempting does not feel convincing to me.
There was a problem hiding this comment.
I think the problem is that it makes little sense to show the char as a char literal for this integral dumper.
This is a resolved argument, it's not supposed to print as-written anyway.
This dumper is a debugging aid and the most important things are showing the value and the type it had.
The suffixed literals are going to obscure any typedefs. The char literal is going to obscure the signed-ness of the char besides that.
The most helpful representation is the cast followed by the plain literal. Ie (uintptr_t)28999
I think we should have a mode in that literal printer for that.
This would remove the plain ugly case of the char literal, but still the single quotes can appear within the printed type of the literal, which can already happen anyway.
Shafik would that also alleviate your concern?
mizvekov
force-pushed
the
users/mizvekov/clang-improve-ast-dumper-template-arguments
branch
from
f9892eb
to
86e3852
Compare
| } | ||
| OS << " '" << Str << "'"; | ||
|
|
||
| if (TemplateArgument CanonTA = Context->getCanonicalTemplateArgument(TA); |
There was a problem hiding this comment.
Suggested change
| if (TemplateArgument CanonTA = Context->getCanonicalTemplateArgument(TA); | |
| if (const TemplateArgument &CanonTA = Context->getCanonicalTemplateArgument(TA); |
There was a problem hiding this comment.
getCanonicalTemplateArgument returns a TemplateArgument by value. These are small objects we don't manually allocate, and don't unique tem (but may unique things it references internally). This is similar to how we don't take const references to QualType.
cor3ntin
approved these changes
May 28, 2024
| llvm::raw_svector_ostream SS(Str); | ||
| TA.print(PrintPolicy, SS, /*IncludeType=*/true); | ||
| } | ||
| OS << " '" << Str << "'"; |
There was a problem hiding this comment.
Yes, lets keep the quotes. I'm not a fan but it appears consistent.
We would ideally update it as well. Not necessarily in the same PR, as it's a separate change that doesn't need to be synchronized. It's just not important to me, because I don't personally use the JSON dumper for debugging. |
This improves and unifies our approach to printing all template arguments. The same approach to printing types is extended to all TemplateArguments: A sugared version is printed in quotes, followed by printing the canonical form, unless they would print the same. Special improvements are done to add more detail to template template arguments. It's planned in a future patch to use this improved TemplateName printer for other places besides TemplateArguments. Note: The sugared/desugared printing does not show up for TemplateNames in tests yet, because we do a poor job of preserving their type sugar. This will be improved in a future patch.
mizvekov
force-pushed
the
users/mizvekov/clang-improve-ast-dumper-template-arguments
branch
from
86e3852
to
fabcce0
Compare
mizvekov
deleted the
users/mizvekov/clang-improve-ast-dumper-template-arguments
branch
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This improves and unifies our approach to printing all template arguments.
The same approach to printing types is extended to all TemplateArguments: A sugared version is printed in quotes, followed by printing the canonical form, unless they would print the same.
Special improvements are done to add more detail to template template arguments.
It's planned in a future patch to use this improved TemplateName printer for other places besides TemplateArguments.
Note: The sugared/desugared printing does not show up for TemplateNames in tests yet, because we do a poor job of preserving their type sugar. This will be improved in a future patch.