Skip to content

[clang] Don't print extra space when dumping template names #95213

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

Merged
merged 1 commit into from
Jun 13, 2024

Conversation

hokein
Copy link
Collaborator

@hokein hokein commented Jun 12, 2024

No description provided.

@hokein hokein requested a review from mizvekov June 12, 2024 09:30
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Jun 12, 2024
@llvmbot
Copy link
Member

llvmbot commented Jun 12, 2024

@llvm/pr-subscribers-clang

Author: Haojian Wu (hokein)

Changes

Full diff: https://github.com/llvm/llvm-project/pull/95213.diff

4 Files Affected:

  • (modified) clang/lib/AST/TextNodeDumper.cpp (+1-1)
  • (modified) clang/test/AST/ast-dump-ctad-alias.cpp (+2-2)
  • (modified) clang/test/AST/ast-dump-template-decls.cpp (+3-3)
  • (modified) clang/test/AST/ast-dump-using-template.cpp (+4-4)
diff --git a/clang/lib/AST/TextNodeDumper.cpp b/clang/lib/AST/TextNodeDumper.cpp
index e1a2709507eff..a26f50f0719c1 100644
--- a/clang/lib/AST/TextNodeDumper.cpp
+++ b/clang/lib/AST/TextNodeDumper.cpp
@@ -1140,7 +1140,7 @@ void TextNodeDumper::dumpTemplateName(TemplateName TN, StringRef Label) {
         llvm::raw_svector_ostream SS(Str);
         TN.print(SS, PrintPolicy);
       }
-      OS << " '" << Str << "'";
+      OS << "'" << Str << "'";
 
       if (Context) {
         if (TemplateName CanonTN = Context->getCanonicalTemplateName(TN);
diff --git a/clang/test/AST/ast-dump-ctad-alias.cpp b/clang/test/AST/ast-dump-ctad-alias.cpp
index cd3b8c6821344..a4b6f06547443 100644
--- a/clang/test/AST/ast-dump-ctad-alias.cpp
+++ b/clang/test/AST/ast-dump-ctad-alias.cpp
@@ -36,11 +36,11 @@ Out2<double>::AInner t(1.0);
 // CHECK-NEXT: |     | |   `-TemplateTypeParmType {{.*}} 'type-parameter-1-0' dependent depth 1 index 0
 // CHECK-NEXT: |     | `-TypeTraitExpr {{.*}} 'bool' __is_deducible
 // CHECK-NEXT: |     |   |-DeducedTemplateSpecializationType {{.*}} 'Out2<double>::AInner' dependent
-// CHECK-NEXT: |     |   | `-name:  'Out2<double>::AInner'
+// CHECK-NEXT: |     |   | `-name: 'Out2<double>::AInner'
 // CHECK-NEXT: |     |   |   `-TypeAliasTemplateDecl {{.+}} AInner{{$}}
 // CHECK-NEXT: |     |   `-ElaboratedType {{.*}} 'Inner<type-parameter-1-0>' sugar dependent
 // CHECK-NEXT: |     |     `-TemplateSpecializationType {{.*}} 'Inner<type-parameter-1-0>' dependent
-// CHECK-NEXT: |     |       |-name:  'Inner':'Out<int>::Inner' qualified
+// CHECK-NEXT: |     |       |-name: 'Inner':'Out<int>::Inner' qualified
 // CHECK-NEXT: |     |       | `-ClassTemplateDecl {{.+}} Inner{{$}}
 // CHECK-NEXT: |     |       `-TemplateArgument type 'type-parameter-1-0'
 // CHECK-NEXT: |     |         `-SubstTemplateTypeParmType {{.*}} 'type-parameter-1-0'
diff --git a/clang/test/AST/ast-dump-template-decls.cpp b/clang/test/AST/ast-dump-template-decls.cpp
index fea14abb3b2f4..f0a6204ce3cfa 100644
--- a/clang/test/AST/ast-dump-template-decls.cpp
+++ b/clang/test/AST/ast-dump-template-decls.cpp
@@ -117,7 +117,7 @@ using type2 = typename C<int>::type1<void>;
 // CHECK:      TypeAliasDecl 0x{{[^ ]*}} <line:[[@LINE-1]]:1, col:42> col:7 type2 'typename C<int>::type1<void>':'void (int)'
 // CHECK-NEXT: ElaboratedType 0x{{[^ ]*}} 'typename C<int>::type1<void>' sugar
 // CHECK-NEXT: TemplateSpecializationType 0x{{[^ ]*}} 'type1<void>' sugar alias
-// CHECK-NEXT: name:  'C<int>::type1':'PR55886::C<int>::type1' qualified
+// CHECK-NEXT: name: 'C<int>::type1':'PR55886::C<int>::type1' qualified
 // CHECK-NEXT: NestedNameSpecifier TypeSpec 'C<int>':'PR55886::C<int>'
 // CHECK-NEXT: TypeAliasTemplateDecl {{.+}} type1
 // CHECK-NEXT: TemplateArgument type 'void'
@@ -153,7 +153,7 @@ template <typename... T> struct D {
 };
 using t2 = D<float, char>::B<int, short>;
 // CHECK:      TemplateSpecializationType 0x{{[^ ]*}} 'B<int, short>' sugar alias{{$}}
-// CHECK-NEXT: name:  'D<float, char>::B':'PR56099::D<float, char>::B' qualified
+// CHECK-NEXT: name: 'D<float, char>::B':'PR56099::D<float, char>::B' qualified
 // CHECK:      FunctionProtoType 0x{{[^ ]*}} 'int (int (*)(float, int), int (*)(char, short))' cdecl
 // CHECK:      FunctionProtoType 0x{{[^ ]*}} 'int (float, int)' cdecl
 // CHECK:      SubstTemplateTypeParmType 0x{{[^ ]*}} 'float' sugar typename depth 0 index 0 ... T pack_index 1
@@ -175,7 +175,7 @@ template<class E1, class E2> class E {};
 using test1 = D<E, int>;
 // CHECK:      TypeAliasDecl 0x{{[^ ]*}} <line:{{[1-9]+}}:1, col:23> col:7 test1 'D<E, int>':'subst_default_argument::E<int, subst_default_argument::A<int>>'
 // CHECK:      TemplateSpecializationType 0x{{[^ ]*}} 'A<int>' sugar
-// CHECK-NEXT: |-name:  'A':'subst_default_argument::A' qualified
+// CHECK-NEXT: |-name: 'A':'subst_default_argument::A' qualified
 // CHECK-NEXT: | `-ClassTemplateDecl {{.+}} A
 // CHECK-NEXT: |-TemplateArgument type 'int'
 // CHECK-NEXT: | `-SubstTemplateTypeParmType 0x{{[^ ]*}} 'int' sugar class depth 0 index 1 D2
diff --git a/clang/test/AST/ast-dump-using-template.cpp b/clang/test/AST/ast-dump-using-template.cpp
index 22b9b76612add..75db5eb5a9d1c 100644
--- a/clang/test/AST/ast-dump-using-template.cpp
+++ b/clang/test/AST/ast-dump-using-template.cpp
@@ -21,7 +21,7 @@ using A = S<T>;
 // CHECK:      TypeAliasDecl
 // CHECK-NEXT: `-ElaboratedType {{.*}} 'S<T>' sugar dependent
 // CHECK-NEXT:   `-TemplateSpecializationType {{.*}} 'S<T>' dependent
-// CHECK-NEXT:     |-name:  'S':'ns::S' qualified
+// CHECK-NEXT:     |-name: 'S':'ns::S' qualified
 // CHECk-NEXT:     | |-UsingShadowDecl {{.+}} ClassTemplate {{.+}} 'S'
 
 // TemplateName in TemplateArgument.
@@ -30,7 +30,7 @@ using B = X<S>;
 // CHECK:      TypeAliasDecl
 // CHECK-NEXT: `-ElaboratedType {{.*}} 'X<S>' sugar
 // CHECK-NEXT:   `-TemplateSpecializationType {{.*}} 'X<S>' sugar
-// CHECK-NEXT:     |-name:  'X' qualified
+// CHECK-NEXT:     |-name: 'X' qualified
 // CHECK-NEXT:     | `-ClassTemplateDecl {{.+}} X
 // CHECK-NEXT:     |-TemplateArgument template 'S':'ns::S' qualified
 // CHECK-NEXT:     | |-UsingShadowDecl {{.*}} implicit ClassTemplate {{.*}} 'S'
@@ -45,7 +45,7 @@ using C = decltype(DeducedTemplateSpecializationT);
 // CHECK-NEXT:  |-DeclRefExpr {{.*}}
 // CHECK-NEXT:  `-ElaboratedType {{.*}} 'S<int>' sugar
 // CHECK-NEXT:    `-DeducedTemplateSpecializationType {{.*}} 'ns::S<int>' sugar
-// CHECK-NEXT:      |-name:  'S':'ns::S' qualified
+// CHECK-NEXT:      |-name: 'S':'ns::S' qualified
 // CHECK-NEXT:      | |-UsingShadowDecl {{.+}} 'S'
 
 S2 DeducedTemplateSpecializationT2(123);
@@ -54,5 +54,5 @@ using D = decltype(DeducedTemplateSpecializationT2);
 // CHECK-NEXT:  |-DeclRefExpr {{.*}}
 // CHECK-NEXT:  `-ElaboratedType {{.*}} 'S2<int>' sugar
 // CHECK-NEXT:    `-DeducedTemplateSpecializationType {{.*}} 'S2<int>' sugar
-// CHECK-NEXT:      |-name:  'S2':'ns::S2' qualified
+// CHECK-NEXT:      |-name: 'S2':'ns::S2' qualified
 //CHECk-NEXT:       | |-UsingShadowDecl {{.+}} ClassTemplate {{.+}} 'S2'

@hokein hokein force-pushed the fix-extra-blank branch from c5f5d78 to 3c96bf1 Compare June 12, 2024 12:43
Copy link
Contributor

@mizvekov mizvekov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think what I tried to do here is generally consistent: The convention on the Text node dumper is to add a space at the beginning of the field you want to append.

I think the true issue here is the label: It breaks convention by adding a space after the colon.

I think it would be better to fix that, but I don't know the impact.

Since right now this field is only added after the label, this makes little practical difference, so I leave it up to you, either way is fine.

Thanks for the fix!

@hokein
Copy link
Collaborator Author

hokein commented Jun 13, 2024

Thanks for the review, I'd go with the current fix.

@hokein hokein merged commit 1dbd7be into llvm:main Jun 13, 2024
5 of 7 checks passed
@hokein hokein deleted the fix-extra-blank branch June 13, 2024 06:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants