[clang][TypePrinter] Unify printing of anonymous/unnamed tag types #169445
Conversation
This file contains hidden or 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
Member
|
@llvm/pr-subscribers-clangd @llvm/pr-subscribers-clang Author: Michael Buch (Michael137) ChangesIn #168534 we made the
This patch moves the printing of anonymous/unnamed tags into a common helper and re-uses it from I wasn't sure where to put the helper, so I just put it in Full diff: https://github.com/llvm/llvm-project/pull/169445.diff 7 Files Affected:
diff --git a/clang/include/clang/AST/TypeBase.h b/clang/include/clang/AST/TypeBase.h
index f07861f50fe8c..2f27deda2cd66 100644
--- a/clang/include/clang/AST/TypeBase.h
+++ b/clang/include/clang/AST/TypeBase.h
@@ -7374,6 +7374,10 @@ bool isSubstitutedDefaultArgument(ASTContext &Ctx, TemplateArgument Arg,
ArrayRef<TemplateArgument> Args,
unsigned Depth);
+void printAnonymousTagDecl(llvm::raw_ostream &OS, const TagDecl *D,
+ const PrintingPolicy &Policy,
+ bool PrintKindDecoration, bool PrintTagLocations);
+
/// Represents a qualified type name for which the type name is
/// dependent.
///
diff --git a/clang/lib/AST/Decl.cpp b/clang/lib/AST/Decl.cpp
index 591457b1d66b4..c9bb414a9a1de 100644
--- a/clang/lib/AST/Decl.cpp
+++ b/clang/lib/AST/Decl.cpp
@@ -35,6 +35,7 @@
#include "clang/AST/Stmt.h"
#include "clang/AST/TemplateBase.h"
#include "clang/AST/Type.h"
+#include "clang/AST/TypeBase.h"
#include "clang/AST/TypeLoc.h"
#include "clang/Basic/Builtins.h"
#include "clang/Basic/IdentifierTable.h"
@@ -1793,7 +1794,9 @@ void NamedDecl::printNestedNameSpecifier(raw_ostream &OS,
if (TypedefNameDecl *TD = RD->getTypedefNameForAnonDecl())
OS << *TD;
else if (!RD->getIdentifier())
- OS << "(anonymous " << RD->getKindName() << ')';
+ printAnonymousTagDecl(OS, llvm::cast<TagDecl>(RD), P,
+ /*PrintKindDecoration=*/true,
+ /*AllowSourceLocations=*/false);
else
OS << *RD;
} else if (const auto *FD = dyn_cast<FunctionDecl>(DC)) {
diff --git a/clang/lib/AST/TypePrinter.cpp b/clang/lib/AST/TypePrinter.cpp
index d2881d5ac518a..e9b06cc17ae30 100644
--- a/clang/lib/AST/TypePrinter.cpp
+++ b/clang/lib/AST/TypePrinter.cpp
@@ -23,6 +23,7 @@
#include "clang/AST/TemplateBase.h"
#include "clang/AST/TemplateName.h"
#include "clang/AST/Type.h"
+#include "clang/AST/TypeBase.h"
#include "clang/Basic/AddressSpaces.h"
#include "clang/Basic/AttrKinds.h"
#include "clang/Basic/ExceptionSpecificationType.h"
@@ -1518,11 +1519,11 @@ void TypePrinter::printTagType(const TagType *T, raw_ostream &OS) {
return;
}
- bool HasKindDecoration = false;
+ bool PrintKindDecoration = Policy.AnonymousTagLocations;
if (T->isCanonicalUnqualified()) {
if (!Policy.SuppressTagKeyword && !D->getTypedefNameForAnonDecl()) {
- HasKindDecoration = true;
+ PrintKindDecoration = false;
OS << D->getKindName();
OS << ' ';
}
@@ -1547,49 +1548,12 @@ void TypePrinter::printTagType(const TagType *T, raw_ostream &OS) {
assert(Typedef->getIdentifier() && "Typedef without identifier?");
OS << Typedef->getIdentifier()->getName();
} else {
- // Make an unambiguous representation for anonymous types, e.g.
- // (anonymous enum at /usr/include/string.h:120:9)
- OS << (Policy.MSVCFormatting ? '`' : '(');
-
- if (isa<CXXRecordDecl>(D) && cast<CXXRecordDecl>(D)->isLambda()) {
- OS << "lambda";
- HasKindDecoration = true;
- } else if ((isa<RecordDecl>(D) && cast<RecordDecl>(D)->isAnonymousStructOrUnion())) {
- OS << "anonymous";
- } else {
- OS << "unnamed";
- }
+ if (isa<CXXRecordDecl>(D) && cast<CXXRecordDecl>(D)->isLambda())
+ PrintKindDecoration = false;
- if (Policy.AnonymousTagLocations) {
- // Suppress the redundant tag keyword if we just printed one.
- // We don't have to worry about ElaboratedTypes here because you can't
- // refer to an anonymous type with one.
- if (!HasKindDecoration)
- OS << " " << D->getKindName();
-
- PresumedLoc PLoc = D->getASTContext().getSourceManager().getPresumedLoc(
- D->getLocation());
- if (PLoc.isValid()) {
- OS << " at ";
- StringRef File = PLoc.getFilename();
- llvm::SmallString<1024> WrittenFile(File);
- if (auto *Callbacks = Policy.Callbacks)
- WrittenFile = Callbacks->remapPath(File);
- // Fix inconsistent path separator created by
- // clang::DirectoryLookup::LookupFile when the file path is relative
- // path.
- llvm::sys::path::Style Style =
- llvm::sys::path::is_absolute(WrittenFile)
- ? llvm::sys::path::Style::native
- : (Policy.MSVCFormatting
- ? llvm::sys::path::Style::windows_backslash
- : llvm::sys::path::Style::posix);
- llvm::sys::path::native(WrittenFile, Style);
- OS << WrittenFile << ':' << PLoc.getLine() << ':' << PLoc.getColumn();
- }
- }
-
- OS << (Policy.MSVCFormatting ? '\'' : ')');
+ printAnonymousTagDecl(OS, D, Policy,
+ /*PrintKindDecoration=*/PrintKindDecoration,
+ /*PrintTagLocations=*/Policy.AnonymousTagLocations);
}
// If this is a class template specialization, print the template
@@ -2469,6 +2433,57 @@ static bool isSubstitutedTemplateArgument(ASTContext &Ctx, TemplateArgument Arg,
return false;
}
+void clang::printAnonymousTagDecl(llvm::raw_ostream &OS, const TagDecl *D,
+ const PrintingPolicy &Policy,
+ bool PrintKindDecoration,
+ bool PrintTagLocations) {
+ assert(D);
+
+ // Make an unambiguous representation for anonymous types, e.g.
+ // (anonymous enum at /usr/include/string.h:120:9)
+ OS << (Policy.MSVCFormatting ? '`' : '(');
+
+ if (isa<CXXRecordDecl>(D) && cast<CXXRecordDecl>(D)->isLambda()) {
+ OS << "lambda";
+ } else if ((isa<RecordDecl>(D) &&
+ cast<RecordDecl>(D)->isAnonymousStructOrUnion())) {
+ OS << "anonymous";
+ } else {
+ OS << "unnamed";
+ }
+
+ // Suppress the redundant tag keyword if we just printed one.
+ // We don't have to worry about ElaboratedTypes here because you can't
+ // refer to an anonymous type with one.
+ if (PrintKindDecoration)
+ OS << " " << D->getKindName();
+
+ if (PrintTagLocations) {
+ PresumedLoc PLoc =
+ D->getASTContext().getSourceManager().getPresumedLoc(D->getLocation());
+ if (PLoc.isValid()) {
+ OS << " at ";
+ StringRef File = PLoc.getFilename();
+ llvm::SmallString<1024> WrittenFile(File);
+ if (auto *Callbacks = Policy.Callbacks)
+ WrittenFile = Callbacks->remapPath(File);
+ // Fix inconsistent path separator created by
+ // clang::DirectoryLookup::LookupFile when the file path is relative
+ // path.
+ llvm::sys::path::Style Style =
+ llvm::sys::path::is_absolute(WrittenFile)
+ ? llvm::sys::path::Style::native
+ : (Policy.MSVCFormatting
+ ? llvm::sys::path::Style::windows_backslash
+ : llvm::sys::path::Style::posix);
+ llvm::sys::path::native(WrittenFile, Style);
+ OS << WrittenFile << ':' << PLoc.getLine() << ':' << PLoc.getColumn();
+ }
+ }
+
+ OS << (Policy.MSVCFormatting ? '\'' : ')');
+}
+
bool clang::isSubstitutedDefaultArgument(ASTContext &Ctx, TemplateArgument Arg,
const NamedDecl *Param,
ArrayRef<TemplateArgument> Args,
diff --git a/clang/lib/ASTMatchers/ASTMatchersInternal.cpp b/clang/lib/ASTMatchers/ASTMatchersInternal.cpp
index 0874b3d0c45f5..e83a6e97ec638 100644
--- a/clang/lib/ASTMatchers/ASTMatchersInternal.cpp
+++ b/clang/lib/ASTMatchers/ASTMatchersInternal.cpp
@@ -18,6 +18,7 @@
#include "clang/AST/ExprConcepts.h"
#include "clang/AST/ParentMapContext.h"
#include "clang/AST/PrettyPrinter.h"
+#include "clang/AST/TypeBase.h"
#include "clang/ASTMatchers/ASTMatchers.h"
#include "clang/Basic/LLVM.h"
#include "clang/Lex/Lexer.h"
@@ -514,7 +515,14 @@ static StringRef getNodeName(const RecordDecl &Node,
return Node.getName();
}
Scratch.clear();
- return ("(anonymous " + Node.getKindName() + ")").toStringRef(Scratch);
+
+ llvm::raw_svector_ostream OS(Scratch);
+
+ printAnonymousTagDecl(
+ OS, llvm::cast<TagDecl>(&Node), Node.getASTContext().getPrintingPolicy(),
+ /*PrintKindDecoration=*/true, /*PrintTagLocations=*/false);
+
+ return OS.str();
}
static StringRef getNodeName(const NamespaceDecl &Node,
diff --git a/clang/unittests/AST/NamedDeclPrinterTest.cpp b/clang/unittests/AST/NamedDeclPrinterTest.cpp
index cd833725b448d..2fdda1929b2a3 100644
--- a/clang/unittests/AST/NamedDeclPrinterTest.cpp
+++ b/clang/unittests/AST/NamedDeclPrinterTest.cpp
@@ -265,3 +265,31 @@ TEST(NamedDeclPrinter, NestedNameSpecifierTemplateArgs) {
ASSERT_TRUE(
PrintedNestedNameSpecifierMatches(Code, "method", "vector<int>::"));
}
+
+TEST(NamedDeclPrinter, NestedNameSpecifierLambda) {
+ const char *Code =
+ R"(
+ auto l = [] {
+ struct Foo {
+ void method();
+ };
+ };
+)";
+ ASSERT_TRUE(PrintedNestedNameSpecifierMatches(
+ Code, "method", "(lambda)::operator()()::Foo::"));
+}
+
+TEST(NamedDeclPrinter, NestedNameSpecifierAnonymousTags) {
+ const char *Code =
+ R"(
+ struct Foo {
+ struct {
+ struct {
+ void method();
+ } i;
+ };
+ };
+)";
+ ASSERT_TRUE(PrintedNestedNameSpecifierMatches(
+ Code, "method", "Foo::(anonymous)::(unnamed)::"));
+}
diff --git a/clang/unittests/AST/TypePrinterTest.cpp b/clang/unittests/AST/TypePrinterTest.cpp
index 3cadf9b265bd1..de4cfa4074eba 100644
--- a/clang/unittests/AST/TypePrinterTest.cpp
+++ b/clang/unittests/AST/TypePrinterTest.cpp
@@ -328,7 +328,7 @@ TEST(TypePrinter, NestedNameSpecifiers) {
// Further levels of nesting print the entire scope.
ASSERT_TRUE(PrintedTypeMatches(
Code, {}, fieldDecl(hasName("u"), hasType(qualType().bind("id"))),
- "union level1()::Inner::Inner(int)::(anonymous struct)::(unnamed)",
+ "union level1()::Inner::Inner(int)::(unnamed struct)::(unnamed)",
[](PrintingPolicy &Policy) {
Policy.FullyQualifiedName = true;
Policy.AnonymousTagLocations = false;
diff --git a/clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp b/clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
index 5d452355d0e43..697623c8a48e8 100644
--- a/clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
+++ b/clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
@@ -2709,25 +2709,48 @@ TEST_P(ASTMatchersTest, HasName_MatchesAnonymousNamespaces) {
EXPECT_TRUE(matches(code, recordDecl(hasName("::a::C"))));
}
-TEST_P(ASTMatchersTest, HasName_MatchesAnonymousOuterClasses) {
+TEST_P(ASTMatchersTest, HasName_MatchesUnnamedOuterClasses) {
if (!GetParam().isCXX()) {
return;
}
EXPECT_TRUE(matches("class A { class { class C; } x; };",
- recordDecl(hasName("A::(anonymous class)::C"))));
+ recordDecl(hasName("A::(unnamed class)::C"))));
EXPECT_TRUE(matches("class A { class { class C; } x; };",
- recordDecl(hasName("::A::(anonymous class)::C"))));
+ recordDecl(hasName("::A::(unnamed class)::C"))));
EXPECT_FALSE(matches("class A { class { class C; } x; };",
recordDecl(hasName("::A::C"))));
EXPECT_TRUE(matches("class A { struct { class C; } x; };",
- recordDecl(hasName("A::(anonymous struct)::C"))));
+ recordDecl(hasName("A::(unnamed struct)::C"))));
EXPECT_TRUE(matches("class A { struct { class C; } x; };",
- recordDecl(hasName("::A::(anonymous struct)::C"))));
+ recordDecl(hasName("::A::(unnamed struct)::C"))));
EXPECT_FALSE(matches("class A { struct { class C; } x; };",
recordDecl(hasName("::A::C"))));
}
+TEST_P(ASTMatchersTest, HasName_MatchesAnonymousOuterClasses) {
+ if (!GetParam().isCXX()) {
+ return;
+ }
+
+ EXPECT_TRUE(matches(
+ "class A { struct { struct { class C; } x; }; };",
+ recordDecl(hasName("A::(anonymous struct)::(unnamed struct)::C"))));
+ EXPECT_TRUE(matches(
+ "class A { struct { struct { class C; } x; }; };",
+ recordDecl(hasName("::A::(anonymous struct)::(unnamed struct)::C"))));
+ EXPECT_FALSE(matches("class A { struct { struct { class C; } x; }; };",
+ recordDecl(hasName("A::(unnamed struct)::C"))));
+ EXPECT_TRUE(matches(
+ "class A { class { public: struct { class C; } x; }; };",
+ recordDecl(hasName("A::(anonymous class)::(unnamed struct)::C"))));
+ EXPECT_TRUE(matches(
+ "class A { class { public: struct { class C; } x; }; };",
+ recordDecl(hasName("::A::(anonymous class)::(unnamed struct)::C"))));
+ EXPECT_FALSE(matches("class A { class { public: struct { class C; } x; }; };",
+ recordDecl(hasName("A::(unnamed struct)::C"))));
+}
+
TEST_P(ASTMatchersTest, HasName_MatchesFunctionScope) {
if (!GetParam().isCXX()) {
return;
|
…sistent In llvm#168534 we made the `TypePrinter` re-use `printNestedNameSpecifier` for printing scopes. However, the way that the names of anonymous/unnamed get printed by the two are slightly inconsistent with each other. `printNestedNameSpecifier` calls all `TagType`s without an identifer `(anonymous)`. On the other hand, `TypePrinter` prints them slightly more accurate (it differentiates anonymous vs. unnamed decls) and allows for some additional customization points. E.g., with `MSVCFormatting`, it will print `\`'` instead of `()`. `printNestedNameSpecifier` already accounts for `MSVCFormatting` for namespaces, but doesn't for `TagType`s. This inconsistency means that if an unnamed tag is printed as part of a scope it's displayed as `(anonymous struct)`, but if it's the entity being whose scope is being printed, then it shows as `(unnamed struct)`. This patch moves the printing of anonymous/unnamed tags into a common helper and re-uses it from `TypePrinter` and `printNestedNameSpecifier`. We also do this from the AST matchers because they were aligned with how `printNestedNameSpecifier` represents the name. I wasn't sure where to put the helper, so I just put it in `TypeBase.h` for now. Though I suspect there's a better home for it, possibly `DeclBase.h`?
Michael137
force-pushed
the
clang/nns-tag-kind-name
branch
from
f63ef7c to
34c5c51
Compare
🐧 Linux x64 Test Results
|
mizvekov
reviewed
Nov 25, 2025
Contributor
mizvekov
left a comment
mizvekov
left a comment
There was a problem hiding this comment.
I like the direction.
Some comments:
-
This would be better declared in Decl.h, since it's about printing declarations, and not related to types.
-
This could probably get rid of the additional bool flags, and rely on PrintingPolicy entirely.
-
Instead of the
printAnonymousTagDeclhelper, we could probably implement aprintmethod for TagDecls, which takes care of the named case, and for the unnamed case, takes care of the EnumDecl and RecordDecl specific behavior, including the typedef to unnamed class.
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
Member
Author
Agreed
Neat idea! Trying this out. Might need to make some compromises with when the kind name gets printed, but lets see |
Michael137
force-pushed
the
clang/nns-tag-kind-name
branch
from
2f82c0d to
e6d55c0
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
In #168534 we made the
TypePrinterre-useprintNestedNameSpecifierfor printing scopes. However, the way that the names of anonymous/unnamed types get printed by the two are slightly inconsistent with each other.printNestedNameSpecifiercalls allTagTypes without an identifer(anonymous). On the other hand,TypePrinterprints them slightly more accurate (it differentiates anonymous vs. unnamed decls) and allows for some additional customization points. E.g., withMSVCFormatting, it will print`unnamed struct'instead of(unnamed struct).printNestedNameSpecifieralready accounts forMSVCFormattingfor namespaces, but doesn't forTagTypes. This inconsistency means that if an unnamed tag is printed as part of a scope then it's displayed as(anonymous struct), but if it's the entity whose scope is being printed, then it shows as(unnamed struct).This patch moves the printing of anonymous/unnamed tags into a common helper and re-uses it from
TypePrinterandprintNestedNameSpecifier. We also do this from the AST matchers because they were aligned with howprintNestedNameSpecifierrepresents the name.I wasn't sure where to put the helper, so I just put it in
TypeBase.hfor now. Though I suspect there's a better home for it, possiblyDeclBase.h?