-
Notifications
You must be signed in to change notification settings - Fork 15.2k
[clang][TypePrinter] Replace AppendScope with printNestedNameSpecifier #168534
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
base: main
Are you sure you want to change the base?
[clang][TypePrinter] Replace AppendScope with printNestedNameSpecifier #168534
Conversation
In debug-info we soon have the need to print names using the full scope of the entity (see discussion in llvm#159592). Particularly, when a structure is scoped inside a function, we'd like to emit the name as `func()::foo`. `CGDebugInfo` uses the `TypePrinter` to print type names into debug-info. However, `TypePrinter` stops (and ignores) `DeclContext`s that are functions. I.e., it would just print `foo`. Ideally it would behave the same way `printNestedNameSpecifier` does. The FIXME in https://github.com/llvm/llvm-project/blob/47c1aa4cef638c97b74f3afb7bed60e92bba1f90/clang/lib/AST/TypePrinter.cpp#L1520-L1521 motivated this patch. See llvm#168533 for how this will be used by `CGDebugInfo`. The plan is to introduce a new `PrintingPolicy` that prints anonymous entities using their full scope (including function/anonymous scopes) and the mangling number.
|
@llvm/pr-subscribers-clang @llvm/pr-subscribers-clang-modules Author: Michael Buch (Michael137) ChangesIn debug-info we soon have the need to print names using the full scope of the entity (see discussion in #159592). Particularly, when a structure is scoped inside a function, we'd like to emit the name as llvm-project/clang/lib/AST/TypePrinter.cpp Lines 1520 to 1521 in 47c1aa4
See #168533 for how this will be used by Full diff: https://github.com/llvm/llvm-project/pull/168534.diff 8 Files Affected:
diff --git a/clang/lib/AST/Decl.cpp b/clang/lib/AST/Decl.cpp
index 8579e51e45697..b771faa4581ec 100644
--- a/clang/lib/AST/Decl.cpp
+++ b/clang/lib/AST/Decl.cpp
@@ -1742,6 +1742,9 @@ void NamedDecl::printNestedNameSpecifier(raw_ostream &OS,
// Collect named contexts.
DeclarationName NameInScope = getDeclName();
for (; Ctx; Ctx = Ctx->getParent()) {
+ if (P.Callbacks && P.Callbacks->isScopeVisible(Ctx))
+ continue;
+
// Suppress anonymous namespace if requested.
if (P.SuppressUnwrittenScope && isa<NamespaceDecl>(Ctx) &&
cast<NamespaceDecl>(Ctx)->isAnonymousNamespace())
diff --git a/clang/lib/AST/TypePrinter.cpp b/clang/lib/AST/TypePrinter.cpp
index c18b2eafc722c..d2881d5ac518a 100644
--- a/clang/lib/AST/TypePrinter.cpp
+++ b/clang/lib/AST/TypePrinter.cpp
@@ -131,8 +131,6 @@ class TypePrinter {
void printBefore(QualType T, raw_ostream &OS);
void printAfter(QualType T, raw_ostream &OS);
- void AppendScope(DeclContext *DC, raw_ostream &OS,
- DeclarationName NameInScope);
void printTagType(const TagType *T, raw_ostream &OS);
void printFunctionAfter(const FunctionType::ExtInfo &Info, raw_ostream &OS);
#define ABSTRACT_TYPE(CLASS, PARENT)
@@ -1226,7 +1224,7 @@ void TypePrinter::printTypeSpec(NamedDecl *D, raw_ostream &OS) {
// In C, this will always be empty except when the type
// being printed is anonymous within other Record.
if (!Policy.SuppressScope)
- AppendScope(D->getDeclContext(), OS, D->getDeclName());
+ D->printNestedNameSpecifier(OS, Policy);
IdentifierInfo *II = D->getIdentifier();
OS << II->getName();
@@ -1240,7 +1238,7 @@ void TypePrinter::printUnresolvedUsingBefore(const UnresolvedUsingType *T,
OS << ' ';
auto *D = T->getDecl();
if (Policy.FullyQualifiedName || T->isCanonicalUnqualified()) {
- AppendScope(D->getDeclContext(), OS, D->getDeclName());
+ D->printNestedNameSpecifier(OS, Policy);
} else {
T->getQualifier().print(OS, Policy);
}
@@ -1257,7 +1255,7 @@ void TypePrinter::printUsingBefore(const UsingType *T, raw_ostream &OS) {
OS << ' ';
auto *D = T->getDecl();
if (Policy.FullyQualifiedName) {
- AppendScope(D->getDeclContext(), OS, D->getDeclName());
+ D->printNestedNameSpecifier(OS, Policy);
} else {
T->getQualifier().print(OS, Policy);
}
@@ -1273,7 +1271,7 @@ void TypePrinter::printTypedefBefore(const TypedefType *T, raw_ostream &OS) {
OS << ' ';
auto *D = T->getDecl();
if (Policy.FullyQualifiedName) {
- AppendScope(D->getDeclContext(), OS, D->getDeclName());
+ D->printNestedNameSpecifier(OS, Policy);
} else {
T->getQualifier().print(OS, Policy);
}
@@ -1511,59 +1509,6 @@ void TypePrinter::printPredefinedSugarBefore(const PredefinedSugarType *T,
void TypePrinter::printPredefinedSugarAfter(const PredefinedSugarType *T,
raw_ostream &OS) {}
-/// Appends the given scope to the end of a string.
-void TypePrinter::AppendScope(DeclContext *DC, raw_ostream &OS,
- DeclarationName NameInScope) {
- if (DC->isTranslationUnit())
- return;
-
- // FIXME: Consider replacing this with NamedDecl::printNestedNameSpecifier,
- // which can also print names for function and method scopes.
- if (DC->isFunctionOrMethod())
- return;
-
- if (Policy.Callbacks && Policy.Callbacks->isScopeVisible(DC))
- return;
-
- if (const auto *NS = dyn_cast<NamespaceDecl>(DC)) {
- if (Policy.SuppressUnwrittenScope && NS->isAnonymousNamespace())
- return AppendScope(DC->getParent(), OS, NameInScope);
-
- // Only suppress an inline namespace if the name has the same lookup
- // results in the enclosing namespace.
- if (Policy.SuppressInlineNamespace !=
- PrintingPolicy::SuppressInlineNamespaceMode::None &&
- NS->isInline() && NameInScope &&
- NS->isRedundantInlineQualifierFor(NameInScope))
- return AppendScope(DC->getParent(), OS, NameInScope);
-
- AppendScope(DC->getParent(), OS, NS->getDeclName());
- if (NS->getIdentifier())
- OS << NS->getName() << "::";
- else
- OS << "(anonymous namespace)::";
- } else if (const auto *Spec = dyn_cast<ClassTemplateSpecializationDecl>(DC)) {
- AppendScope(DC->getParent(), OS, Spec->getDeclName());
- IncludeStrongLifetimeRAII Strong(Policy);
- OS << Spec->getIdentifier()->getName();
- const TemplateArgumentList &TemplateArgs = Spec->getTemplateArgs();
- printTemplateArgumentList(
- OS, TemplateArgs.asArray(), Policy,
- Spec->getSpecializedTemplate()->getTemplateParameters());
- OS << "::";
- } else if (const auto *Tag = dyn_cast<TagDecl>(DC)) {
- AppendScope(DC->getParent(), OS, Tag->getDeclName());
- if (TypedefNameDecl *Typedef = Tag->getTypedefNameForAnonDecl())
- OS << Typedef->getIdentifier()->getName() << "::";
- else if (Tag->getIdentifier())
- OS << Tag->getIdentifier()->getName() << "::";
- else
- return;
- } else {
- AppendScope(DC->getParent(), OS, NameInScope);
- }
-}
-
void TypePrinter::printTagType(const TagType *T, raw_ostream &OS) {
TagDecl *D = T->getDecl();
@@ -1593,7 +1538,7 @@ void TypePrinter::printTagType(const TagType *T, raw_ostream &OS) {
// Compute the full nested-name-specifier for this type.
// In C, this will always be empty except when the type
// being printed is anonymous within other Record.
- AppendScope(D->getDeclContext(), OS, D->getDeclName());
+ D->printNestedNameSpecifier(OS, Policy);
}
if (const IdentifierInfo *II = D->getIdentifier())
@@ -1809,7 +1754,7 @@ void TypePrinter::printTemplateId(const TemplateSpecializationType *T,
// FIXME: Null TD never exercised in test suite.
if (FullyQualify && TD) {
if (!Policy.SuppressScope)
- AppendScope(TD->getDeclContext(), OS, TD->getDeclName());
+ TD->printNestedNameSpecifier(OS, Policy);
OS << TD->getName();
} else {
diff --git a/clang/test/ASTMerge/struct/test.c b/clang/test/ASTMerge/struct/test.c
index 10ea753b142bd..d11234472ef29 100644
--- a/clang/test/ASTMerge/struct/test.c
+++ b/clang/test/ASTMerge/struct/test.c
@@ -44,10 +44,10 @@
// CHECK: struct2.c:72:7: note: field 'i' has type 'int' here
// CHECK: struct2.c:76:5: warning: external variable 'x13' declared with incompatible types in different translation units ('S13' vs. 'S13')
// CHECK: struct1.c:79:5: note: declared here with type 'S13'
-// CHECK: struct1.c:130:7: warning: type 'struct DeepUnnamedError::(unnamed at [[PATH_TO_INPUTS:.+]]struct1.c:130:7)' has incompatible definitions in different translation units
+// CHECK: struct1.c:130:7: warning: type 'struct DeepUnnamedError::(anonymous union)::(anonymous union)::(unnamed at [[PATH_TO_INPUTS:.+]]struct1.c:130:7)' has incompatible definitions in different translation units
// CHECK: struct1.c:131:14: note: field 'i' has type 'long' here
// CHECK: struct2.c:128:15: note: field 'i' has type 'float' here
-// CHECK: struct1.c:129:5: warning: type 'union DeepUnnamedError::(unnamed at [[PATH_TO_INPUTS]]struct1.c:129:5)' has incompatible definitions in different translation units
+// CHECK: struct1.c:129:5: warning: type 'union DeepUnnamedError::(anonymous union)::(unnamed at [[PATH_TO_INPUTS]]struct1.c:129:5)' has incompatible definitions in different translation units
// CHECK: struct1.c:132:9: note: field 'S' has type 'struct (unnamed struct at [[PATH_TO_INPUTS]]struct1.c:130:7)' here
// CHECK: struct2.c:129:9: note: field 'S' has type 'struct (unnamed struct at [[PATH_TO_INPUTS]]struct2.c:127:7)' here
// CHECK: struct2.c:138:3: warning: external variable 'x16' declared with incompatible types in different translation units ('struct DeepUnnamedError' vs. 'struct DeepUnnamedError')
diff --git a/clang/test/CXX/drs/cwg6xx.cpp b/clang/test/CXX/drs/cwg6xx.cpp
index 11eb0bf3a27b2..8eac049211193 100644
--- a/clang/test/CXX/drs/cwg6xx.cpp
+++ b/clang/test/CXX/drs/cwg6xx.cpp
@@ -1241,7 +1241,7 @@ namespace cwg686 { // cwg686: 3.0
#endif
struct N {
operator struct O{}(){};
- // expected-error@-1 {{'N::O' cannot be defined in a type specifier}}
+ // expected-error@-1 {{'cwg686::f()::N::O' cannot be defined in a type specifier}}
};
try {}
catch (struct P *) {}
diff --git a/clang/test/Index/print-type.c b/clang/test/Index/print-type.c
index d30f4bed246c5..9fb85f8da4e66 100644
--- a/clang/test/Index/print-type.c
+++ b/clang/test/Index/print-type.c
@@ -71,7 +71,7 @@ _Atomic(unsigned long) aul;
// CHECK: TypeRef=struct Struct:16:8 [type=struct Struct] [typekind=Record] [isPOD=1]
// CHECK: StructDecl=struct (unnamed at {{.*}}):18:1 (Definition) [type=struct (unnamed at {{.*}}print-type.c:18:1)] [typekind=Record] [isPOD=1] [nbFields=2] [isAnon=1] [isAnonRecDecl=0]
// CHECK: StructDecl=struct (unnamed at {{.*}}):23:1 (Definition) [type=struct (unnamed at {{.*}}print-type.c:23:1)] [typekind=Record] [isPOD=1] [nbFields=1] [isAnon=1] [isAnonRecDecl=0]
-// CHECK: StructDecl=struct (anonymous at {{.*}}):24:3 (Definition) [type=struct (anonymous at {{.*}}print-type.c:24:3)] [typekind=Record] [isPOD=1] [nbFields=2] [isAnon=1] [isAnonRecDecl=1]
+// CHECK: StructDecl=struct (anonymous at {{.*}}):24:3 (Definition) [type=struct (anonymous struct)::(anonymous at {{.*}}print-type.c:24:3)] [typekind=Record] [isPOD=1] [nbFields=2] [isAnon=1] [isAnonRecDecl=1]
// CHECK: FieldDecl=x:25:17 (Definition) [type=_Atomic(int)] [typekind=Atomic] [valuetype=int] [valuetypekind=Int] [isPOD=0] [isAnonRecDecl=0]
// CHECK: FieldDecl=y:26:9 (Definition) [type=int] [typekind=Int] [isPOD=1] [isAnonRecDecl=0]
// CHECK: StructDecl=struct (unnamed at {{.*}}):30:10 (Definition) [type=struct (unnamed at {{.*}}print-type.c:30:10)] [typekind=Record] [isPOD=1] [nbFields=2] [isAnon=1] [isAnonRecDecl=0]
diff --git a/clang/test/Layout/ms-x86-alias-avoidance-padding.cpp b/clang/test/Layout/ms-x86-alias-avoidance-padding.cpp
index 678537bb514f5..bc6a56ef37538 100644
--- a/clang/test/Layout/ms-x86-alias-avoidance-padding.cpp
+++ b/clang/test/Layout/ms-x86-alias-avoidance-padding.cpp
@@ -49,7 +49,7 @@ struct AT3 : AT2, AT1 {
// CHECK-NEXT: 0 | struct AT2 (base)
// CHECK-NEXT: 0 | struct AT0 t
// CHECK-NEXT: 0 | union AT0::(unnamed at {{.*}} x
-// CHECK-NEXT: 0 | struct AT0::(unnamed at {{.*}} y
+// CHECK-NEXT: 0 | struct AT0::(anonymous union)::(unnamed at {{.*}} y
// CHECK-NEXT: 0 | int a
// CHECK-NEXT: 4 | struct AT t (empty)
// CHECK: 0 | int b
@@ -66,7 +66,7 @@ struct AT3 : AT2, AT1 {
// CHECK-X64-NEXT: 0 | struct AT2 (base)
// CHECK-X64-NEXT: 0 | struct AT0 t
// CHECK-X64-NEXT: 0 | union AT0::(unnamed at {{.*}} x
-// CHECK-X64-NEXT: 0 | struct AT0::(unnamed at {{.*}} y
+// CHECK-X64-NEXT: 0 | struct AT0::(anonymous union)::(unnamed at {{.*}} y
// CHECK-X64-NEXT: 0 | int a
// CHECK-X64-NEXT: 4 | struct AT t (empty)
// CHECK-X64: 0 | int b
diff --git a/clang/test/Modules/compare-record.c b/clang/test/Modules/compare-record.c
index ef4a3a5b0e90d..97caabd55fe33 100644
--- a/clang/test/Modules/compare-record.c
+++ b/clang/test/Modules/compare-record.c
@@ -496,6 +496,7 @@ struct CompareAnonymousNestedStruct compareAnonymousNestedStruct;
// expected-note@first-anonymous.h:* {{declaration of 'anonymousNestedStructField' does not match}}
#elif defined(CASE3)
struct CompareDeeplyNestedAnonymousUnionsAndStructs compareDeeplyNested;
-// expected-error-re@second-anonymous.h:* {{'CompareDeeplyNestedAnonymousUnionsAndStructs::(anonymous union)::(anonymous union)::(anonymous struct)::z' from module 'Second' is not present in definition of 'struct CompareDeeplyNestedAnonymousUnionsAndStructs::(anonymous at {{.*}})' in module 'First.Hidden'}}
+// expected-error-re@second-anonymous.h:* {{'CompareDeeplyNestedAnonymousUnionsAndStructs::(anonymous union)::(anonymous union)::(anonymous struct)::z' from module 'Second' is not present in definition of 'struct CompareDeeplyNestedAnonymousUnionsAndStructs::(anonymous union)::(anonymous union)::(anonymous at {{.*}})' in module 'First.Hidden'}}
+
// expected-note@first-anonymous.h:* {{declaration of 'z' does not match}}
#endif
diff --git a/clang/test/SemaObjCXX/arc-0x.mm b/clang/test/SemaObjCXX/arc-0x.mm
index bcaa5da6b9283..a7418eceb244b 100644
--- a/clang/test/SemaObjCXX/arc-0x.mm
+++ b/clang/test/SemaObjCXX/arc-0x.mm
@@ -162,7 +162,7 @@ void test() {
struct S1 {
union {
- union { // expected-note-re {{copy constructor of 'S1' is implicitly deleted because field 'test_union::S1::(anonymous union at {{.*}})' has a deleted copy constructor}} expected-note-re {{copy assignment operator of 'S1' is implicitly deleted because field 'test_union::S1::(anonymous union at {{.*}})' has a deleted copy assignment operator}} expected-note-re 4 {{'S1' is implicitly deleted because field 'test_union::S1::(anonymous union at {{.*}})' has a deleted}}
+ union { // expected-note-re {{copy constructor of 'S1' is implicitly deleted because field 'test_union::S1::(anonymous union)::(anonymous union at {{.*}})' has a deleted copy constructor}} expected-note-re {{copy assignment operator of 'S1' is implicitly deleted because field 'test_union::S1::(anonymous union)::(anonymous union at {{.*}})' has a deleted copy assignment operator}} expected-note-re 4 {{'S1' is implicitly deleted because field 'test_union::S1::(anonymous union)::(anonymous union at {{.*}})' has a deleted}}
id f0; // expected-note-re 2 {{{{.*}} of '(anonymous union at {{.*}}' is implicitly deleted because variant field 'f0' is an ObjC pointer}}
char f1;
};
@@ -173,7 +173,7 @@ void test() {
struct S2 {
union {
// FIXME: the note should say 'f0' is causing the special functions to be deleted.
- struct { // expected-note-re 6 {{'S2' is implicitly deleted because variant field 'test_union::S2::(anonymous struct at {{.*}})' has a non-trivial}}
+ struct { // expected-note-re 6 {{'S2' is implicitly deleted because variant field 'test_union::S2::(anonymous union)::(anonymous struct at {{.*}})' has a non-trivial}}
id f0;
int f1;
};
@@ -195,8 +195,8 @@ void test() {
};
static union { // expected-error {{call to implicitly-deleted default constructor of}}
- union { // expected-note-re {{default constructor of '(unnamed union at {{.*}}' is implicitly deleted because field 'test_union::(anonymous union at {{.*}})' has a deleted default constructor}}
- union { // expected-note-re {{default constructor of '(anonymous union at {{.*}}' is implicitly deleted because field 'test_union::(anonymous union at {{.*}})' has a deleted default constructor}}
+ union { // expected-note-re {{default constructor of '(unnamed union at {{.*}}' is implicitly deleted because field 'test_union::(anonymous union)::(anonymous union at {{.*}})' has a deleted default constructor}}
+ union { // expected-note-re {{default constructor of '(anonymous union at {{.*}}' is implicitly deleted because field 'test_union::(anonymous union)::(anonymous union)::(anonymous union at {{.*}})' has a deleted default constructor}}
__weak id g1; // expected-note-re {{default constructor of '(anonymous union at {{.*}}' is implicitly deleted because variant field 'g1' is an ObjC pointer}}
int g2;
};
|
|
Thought I'd open this and see what people think of the change in diagnostics. Description explains the motivation but the TL;DR is that we want to be able to print full nested scopes in the future for debug-info purposes. We could add some logic into CC @zygoloid since you suggested the FIXME originally |
| @@ -1241,7 +1241,7 @@ namespace cwg686 { // cwg686: 3.0 | |||
| #endif | |||
| struct N { | |||
| operator struct O{}(){}; | |||
| // expected-error@-1 {{'N::O' cannot be defined in a type specifier}} | |||
| // expected-error@-1 {{'cwg686::f()::N::O' cannot be defined in a type specifier}} | |||
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 cwg686::f was a function taking single parameter of type int, would I see it here? I don't see this being demonstrated one way or the other in any of the tests you're changing. (Maybe more tests are needed?)
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.
Based on local testing it would yes. But yea worth an extra test-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.
Sorry, I meant a test elsewhere, e.g. in clang/AST directory. DR test here is not a good fit to test compiler behavior per se, because its contents are derived from the Standard wording. In other words, your new f2 could easily be deleted in the future as a duplicate case, and no one will notice reduction in coverage.
🐧 Linux x64 Test Results
|
adrian-prantl
left a comment
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.
From a debug info perspective this change LGTM.
mizvekov
left a comment
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, nice cleanup.
In debug-info we soon have the need to print names using the full scope of the entity (see discussion in #159592). Particularly, when a structure is scoped inside a function, we'd like to emit the name as
func()::foo.CGDebugInfouses theTypePrinterto print type names into debug-info. However,TypePrinterstops (and ignores)DeclContexts that are functions. I.e., it would just printfoo. Ideally it would behave the same wayprintNestedNameSpecifierdoes. The FIXME inllvm-project/clang/lib/AST/TypePrinter.cpp
Lines 1520 to 1521 in 47c1aa4
See #168533 for how this will be used by
CGDebugInfo. The plan is to introduce a newPrintingPolicythat prints anonymous entities using their full scope (including function/anonymous scopes) and the mangling number.