-
Notifications
You must be signed in to change notification settings - Fork 11.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[clang] Improve ast-dumper text printing of TemplateArgument #93431
[clang] Improve ast-dumper text printing of TemplateArgument #93431
Conversation
@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]
|
031e7c2
to
f9892eb
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
I think this is an improvement but we probably want to find a better syntax than quotes
llvm::raw_svector_ostream SS(Str); | ||
TA.print(PrintPolicy, SS, /*IncludeType=*/true); | ||
} | ||
OS << " '" << Str << "'"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, lets keep the quotes. I'm not a fan but it appears consistent.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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?
f9892eb
to
86e3852
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we also be updating JSONNodeDumper.cpp at the same time?
} | ||
OS << " '" << Str << "'"; | ||
|
||
if (TemplateArgument CanonTA = Context->getCanonicalTemplateArgument(TA); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if (TemplateArgument CanonTA = Context->getCanonicalTemplateArgument(TA); | |
if (const TemplateArgument &CanonTA = Context->getCanonicalTemplateArgument(TA); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
(we should find better delimiters but it is out of scope for this change)
llvm::raw_svector_ostream SS(Str); | ||
TA.print(PrintPolicy, SS, /*IncludeType=*/true); | ||
} | ||
OS << " '" << Str << "'"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
86e3852
to
fabcce0
Compare
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.