[clang][TypePrinter][NFC] Make SuppressInlineNamespaceMode an enum class #170802
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
The enum cases for `SuppressInlineNamespaceMode` leaked into the rest of `PrintingPolicy`, meaning if we wanted to introduce another enum (which I'm planning on doing in an unrelated PR), then `All`/`None`/`Redundant` are all already taken, which are quite useful names. This patch turns `SuppressInlineNamespaceMode` into an `enum class`, freeing up the names for use by other future enums in `PrintingPolicy`. Unfortunately that means we have to cast the values when assigning to `PrintingPolicy::SuppressInlineNamespace`, because it's actual type is `unsigned`. But if we ever make the bitfields have proper enumeration types (which AFAIU is dependent on an MSVC codegen fix), then we can get rid of those casts. This caught three instances of assigning booleans to `SuppressInlineNamespace`. I added a FIXME in LLDB to check what the intended value of the enum should be.
Michael137
requested review from
JDevlieghere,
andykaylor,
bcardosolopes,
lanza and
xlauko
as code owners
llvmbot
added
lldb
clang:frontend
Member
|
@llvm/pr-subscribers-clangir @llvm/pr-subscribers-debuginfo Author: Michael Buch (Michael137) ChangesThe enum cases for This patch turns Unfortunately that means we have to cast the values when assigning to This caught three instances of assigning booleans to Full diff: https://github.com/llvm/llvm-project/pull/170802.diff 7 Files Affected:
diff --git a/clang/include/clang/AST/PrettyPrinter.h b/clang/include/clang/AST/PrettyPrinter.h
index fd995a653d167..289dff7b23010 100644
--- a/clang/include/clang/AST/PrettyPrinter.h
+++ b/clang/include/clang/AST/PrettyPrinter.h
@@ -55,14 +55,15 @@ class PrintingCallbacks {
/// This type is intended to be small and suitable for passing by value.
/// It is very frequently copied.
struct PrintingPolicy {
- enum SuppressInlineNamespaceMode : uint8_t { None, Redundant, All };
+ enum class SuppressInlineNamespaceMode : uint8_t { None, Redundant, All };
/// Create a default printing policy for the specified language.
PrintingPolicy(const LangOptions &LO)
: Indentation(2), SuppressSpecifiers(false),
SuppressTagKeyword(LO.CPlusPlus), IncludeTagDefinition(false),
SuppressScope(false), SuppressUnwrittenScope(false),
- SuppressInlineNamespace(SuppressInlineNamespaceMode::Redundant),
+ SuppressInlineNamespace(
+ llvm::to_underlying(SuppressInlineNamespaceMode::Redundant)),
SuppressInitializers(false), ConstantArraySizeAsWritten(false),
AnonymousTagLocations(true), SuppressStrongLifetime(false),
SuppressLifetimeQualifiers(false),
diff --git a/clang/lib/AST/Decl.cpp b/clang/lib/AST/Decl.cpp
index 591457b1d66b4..4444b10db8cc0 100644
--- a/clang/lib/AST/Decl.cpp
+++ b/clang/lib/AST/Decl.cpp
@@ -1753,9 +1753,11 @@ void NamedDecl::printNestedNameSpecifier(raw_ostream &OS,
// Suppress inline namespace if it doesn't make the result ambiguous.
if (Ctx->isInlineNamespace() && NameInScope) {
if (P.SuppressInlineNamespace ==
- PrintingPolicy::SuppressInlineNamespaceMode::All ||
+ llvm::to_underlying(
+ PrintingPolicy::SuppressInlineNamespaceMode::All) ||
(P.SuppressInlineNamespace ==
- PrintingPolicy::SuppressInlineNamespaceMode::Redundant &&
+ llvm::to_underlying(
+ PrintingPolicy::SuppressInlineNamespaceMode::Redundant) &&
cast<NamespaceDecl>(Ctx)->isRedundantInlineQualifierFor(
NameInScope))) {
continue;
diff --git a/clang/lib/ASTMatchers/ASTMatchersInternal.cpp b/clang/lib/ASTMatchers/ASTMatchersInternal.cpp
index fbb8b49676045..a556ffca96903 100644
--- a/clang/lib/ASTMatchers/ASTMatchersInternal.cpp
+++ b/clang/lib/ASTMatchers/ASTMatchersInternal.cpp
@@ -654,9 +654,9 @@ bool HasNameMatcher::matchesNodeFullSlow(const NamedDecl &Node) const {
PrintingPolicy Policy = Node.getASTContext().getPrintingPolicy();
Policy.SuppressUnwrittenScope = SkipUnwritten;
- Policy.SuppressInlineNamespace =
+ Policy.SuppressInlineNamespace = llvm::to_underlying(
SkipUnwritten ? PrintingPolicy::SuppressInlineNamespaceMode::All
- : PrintingPolicy::SuppressInlineNamespaceMode::None;
+ : PrintingPolicy::SuppressInlineNamespaceMode::None);
Node.printQualifiedName(OS, Policy);
const StringRef FullName = OS.str();
diff --git a/clang/lib/CIR/CodeGen/CIRGenTypes.cpp b/clang/lib/CIR/CodeGen/CIRGenTypes.cpp
index efc2c6c0ba500..24b106b4bcee7 100644
--- a/clang/lib/CIR/CodeGen/CIRGenTypes.cpp
+++ b/clang/lib/CIR/CodeGen/CIRGenTypes.cpp
@@ -97,7 +97,8 @@ std::string CIRGenTypes::getRecordTypeName(const clang::RecordDecl *recordDecl,
llvm::raw_svector_ostream outStream(typeName);
PrintingPolicy policy = recordDecl->getASTContext().getPrintingPolicy();
- policy.SuppressInlineNamespace = false;
+ policy.SuppressInlineNamespace =
+ llvm::to_underlying(PrintingPolicy::SuppressInlineNamespaceMode::None);
policy.AlwaysIncludeTypeForTemplateArgument = true;
policy.PrintAsCanonical = true;
policy.SuppressTagKeyword = true;
diff --git a/clang/lib/CodeGen/CGDebugInfo.cpp b/clang/lib/CodeGen/CGDebugInfo.cpp
index c50f372c1f331..5a83c151901f9 100644
--- a/clang/lib/CodeGen/CGDebugInfo.cpp
+++ b/clang/lib/CodeGen/CGDebugInfo.cpp
@@ -417,7 +417,7 @@ PrintingPolicy CGDebugInfo::getPrintingPolicy() const {
}
PP.SuppressInlineNamespace =
- PrintingPolicy::SuppressInlineNamespaceMode::None;
+ llvm::to_underlying(PrintingPolicy::SuppressInlineNamespaceMode::None);
PP.PrintAsCanonical = true;
PP.UsePreferredNames = false;
PP.AlwaysIncludeTypeForTemplateArgument = true;
diff --git a/clang/lib/CodeGen/CodeGenTypes.cpp b/clang/lib/CodeGen/CodeGenTypes.cpp
index ea31195b7f92e..be862cf07f177 100644
--- a/clang/lib/CodeGen/CodeGenTypes.cpp
+++ b/clang/lib/CodeGen/CodeGenTypes.cpp
@@ -62,7 +62,7 @@ void CodeGenTypes::addRecordTypeName(const RecordDecl *RD,
// FullyQualifiedNames.
PrintingPolicy Policy = RD->getASTContext().getPrintingPolicy();
Policy.SuppressInlineNamespace =
- PrintingPolicy::SuppressInlineNamespaceMode::None;
+ llvm::to_underlying(PrintingPolicy::SuppressInlineNamespaceMode::None);
// Name the codegen type after the typedef name
// if there is no tag type name available
diff --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
index aa8d309fbc730..2cb4a46130c84 100644
--- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -2149,7 +2149,8 @@ PrintingPolicy TypeSystemClang::GetTypePrintingPolicy() {
printing_policy.SuppressTagKeyword = true;
// Inline namespaces are important for some type formatters (e.g., libc++
// and libstdc++ are differentiated by their inline namespaces).
- printing_policy.SuppressInlineNamespace = false;
+ printing_policy.SuppressInlineNamespace =
+ llvm::to_underlying(PrintingPolicy::SuppressInlineNamespaceMode::None);
printing_policy.SuppressUnwrittenScope = false;
// Default arguments are also always important for type formatters. Otherwise
// we would need to always specify two type names for the setups where we do
@@ -3870,7 +3871,9 @@ TypeSystemClang::GetDisplayTypeName(lldb::opaque_compiler_type_t type) {
printing_policy.SuppressTagKeyword = true;
printing_policy.SuppressScope = false;
printing_policy.SuppressUnwrittenScope = true;
- printing_policy.SuppressInlineNamespace = true;
+ // FIXME: should we suppress "All" inline namespaces?
+ printing_policy.SuppressInlineNamespace = llvm::to_underlying(
+ PrintingPolicy::SuppressInlineNamespaceMode::Redundant);
return ConstString(qual_type.getAsString(printing_policy));
}
|
Member
Author
|
AArch64 LLDB test failure is unrelated: Happens on other PRs too |
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.
The enum cases for
SuppressInlineNamespaceModeleaked into the rest ofPrintingPolicy, meaning if we wanted to introduce another enum (which I'm planning on doing in an unrelated PR), thenAll/None/Redundantare all already taken, which are quite useful names.This patch turns
SuppressInlineNamespaceModeinto anenum class, freeing up the names for use by other future enums inPrintingPolicy.Unfortunately that means we have to cast the values when assigning to
PrintingPolicy::SuppressInlineNamespace, because its actual type isunsigned. But if we ever make the bitfields have proper enumeration types (which AFAIU is dependent on an MSVC codegen fix), then we can get rid of those casts.While doing this I found three instances of assigning booleans to
SuppressInlineNamespace. I added a FIXME in LLDB to check what the intended value of the enum should be.