[Clang][AST] Move NamespaceDecl bits to DeclContext #98567
Merged
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
|
@llvm/pr-subscribers-clang-tools-extra @llvm/pr-subscribers-clang Author: Krystian Stasiowski (sdkrystian) ChangesCurrently,
Full diff: https://github.com/llvm/llvm-project/pull/98567.diff 12 Files Affected:
diff --git a/clang/include/clang/AST/Decl.h b/clang/include/clang/AST/Decl.h
index 5957f14098363..561a9d872acfb 100644
--- a/clang/include/clang/AST/Decl.h
+++ b/clang/include/clang/AST/Decl.h
@@ -542,12 +542,9 @@ class LabelDecl : public NamedDecl {
};
/// Represent a C++ namespace.
-class NamespaceDecl : public NamedDecl, public DeclContext,
- public Redeclarable<NamespaceDecl>
-{
-
- enum Flags : unsigned { F_Inline = 1 << 0, F_Nested = 1 << 1 };
-
+class NamespaceDecl : public NamedDecl,
+ public DeclContext,
+ public Redeclarable<NamespaceDecl> {
/// The starting location of the source range, pointing
/// to either the namespace or the inline keyword.
SourceLocation LocStart;
@@ -555,12 +552,8 @@ class NamespaceDecl : public NamedDecl, public DeclContext,
/// The ending location of the source range.
SourceLocation RBraceLoc;
- /// A pointer to either the anonymous namespace that lives just inside
- /// this namespace or to the first namespace in the chain (the latter case
- /// only when this is not the first in the chain), along with a
- /// boolean value indicating whether this is an inline namespace.
- llvm::PointerIntPair<NamespaceDecl *, 2, unsigned>
- AnonOrFirstNamespaceAndFlags;
+ /// The unnamed namespace that inhabits this namespace, if any.
+ NamespaceDecl *AnonymousNamespace = nullptr;
NamespaceDecl(ASTContext &C, DeclContext *DC, bool Inline,
SourceLocation StartLoc, SourceLocation IdLoc,
@@ -607,35 +600,19 @@ class NamespaceDecl : public NamedDecl, public DeclContext,
}
/// Returns true if this is an inline namespace declaration.
- bool isInline() const {
- return AnonOrFirstNamespaceAndFlags.getInt() & F_Inline;
- }
+ bool isInline() const { return NamespaceDeclBits.IsInline; }
/// Set whether this is an inline namespace declaration.
- void setInline(bool Inline) {
- unsigned F = AnonOrFirstNamespaceAndFlags.getInt();
- if (Inline)
- AnonOrFirstNamespaceAndFlags.setInt(F | F_Inline);
- else
- AnonOrFirstNamespaceAndFlags.setInt(F & ~F_Inline);
- }
+ void setInline(bool Inline) { NamespaceDeclBits.IsInline = Inline; }
/// Returns true if this is a nested namespace declaration.
/// \code
/// namespace outer::nested { }
/// \endcode
- bool isNested() const {
- return AnonOrFirstNamespaceAndFlags.getInt() & F_Nested;
- }
+ bool isNested() const { return NamespaceDeclBits.IsNested; }
/// Set whether this is a nested namespace declaration.
- void setNested(bool Nested) {
- unsigned F = AnonOrFirstNamespaceAndFlags.getInt();
- if (Nested)
- AnonOrFirstNamespaceAndFlags.setInt(F | F_Nested);
- else
- AnonOrFirstNamespaceAndFlags.setInt(F & ~F_Nested);
- }
+ void setNested(bool Nested) { NamespaceDeclBits.IsNested = Nested; }
/// Returns true if the inline qualifier for \c Name is redundant.
bool isRedundantInlineQualifierFor(DeclarationName Name) const {
@@ -649,34 +626,18 @@ class NamespaceDecl : public NamedDecl, public DeclContext,
std::distance(Y.begin(), Y.end());
}
- /// Get the original (first) namespace declaration.
- NamespaceDecl *getOriginalNamespace();
-
- /// Get the original (first) namespace declaration.
- const NamespaceDecl *getOriginalNamespace() const;
-
- /// Return true if this declaration is an original (first) declaration
- /// of the namespace. This is false for non-original (subsequent) namespace
- /// declarations and anonymous namespaces.
- bool isOriginalNamespace() const;
-
- /// Retrieve the anonymous namespace nested inside this namespace,
- /// if any.
+ /// Retrieve the anonymous namespace that inhabits this namespace, if any.
NamespaceDecl *getAnonymousNamespace() const {
- return getOriginalNamespace()->AnonOrFirstNamespaceAndFlags.getPointer();
+ return getFirstDecl()->AnonymousNamespace;
}
void setAnonymousNamespace(NamespaceDecl *D) {
- getOriginalNamespace()->AnonOrFirstNamespaceAndFlags.setPointer(D);
+ getFirstDecl()->AnonymousNamespace = D;
}
/// Retrieves the canonical declaration of this namespace.
- NamespaceDecl *getCanonicalDecl() override {
- return getOriginalNamespace();
- }
- const NamespaceDecl *getCanonicalDecl() const {
- return getOriginalNamespace();
- }
+ NamespaceDecl *getCanonicalDecl() override { return getFirstDecl(); }
+ const NamespaceDecl *getCanonicalDecl() const { return getFirstDecl(); }
SourceRange getSourceRange() const override LLVM_READONLY {
return SourceRange(LocStart, RBraceLoc);
diff --git a/clang/include/clang/AST/DeclBase.h b/clang/include/clang/AST/DeclBase.h
index 06ffc2ce09b89..16a9b65b754b9 100644
--- a/clang/include/clang/AST/DeclBase.h
+++ b/clang/include/clang/AST/DeclBase.h
@@ -1484,6 +1484,27 @@ class DeclContext {
/// Number of bits in DeclContextBitfields.
enum { NumDeclContextBits = 13 };
+ /// Stores the bits used by NamespaceDecl.
+ /// If modified NumNamespaceDeclBits and the accessor
+ /// methods in NamespaceDecl should be updated appropriately.
+ class NamespaceDeclBitfields {
+ friend class NamespaceDecl;
+ /// For the bits in DeclContextBitfields
+ LLVM_PREFERRED_TYPE(DeclContextBitfields)
+ uint64_t : NumDeclContextBits;
+
+ /// True if this is an inline namespace.
+ LLVM_PREFERRED_TYPE(bool)
+ uint64_t IsInline : 1;
+
+ /// True if this is a nested-namespace-definition.
+ LLVM_PREFERRED_TYPE(bool)
+ uint64_t IsNested : 1;
+ };
+
+ /// Number of inherited and non-inherited bits in NamespaceDeclBitfields.
+ enum { NumNamespaceDeclBits = NumDeclContextBits + 2 };
+
/// Stores the bits used by TagDecl.
/// If modified NumTagDeclBits and the accessor
/// methods in TagDecl should be updated appropriately.
@@ -1982,6 +2003,7 @@ class DeclContext {
/// 8 bytes with static_asserts in the ctor of DeclContext.
union {
DeclContextBitfields DeclContextBits;
+ NamespaceDeclBitfields NamespaceDeclBits;
TagDeclBitfields TagDeclBits;
EnumDeclBitfields EnumDeclBits;
RecordDeclBitfields RecordDeclBits;
@@ -1995,6 +2017,8 @@ class DeclContext {
static_assert(sizeof(DeclContextBitfields) <= 8,
"DeclContextBitfields is larger than 8 bytes!");
+ static_assert(sizeof(NamespaceDeclBitfields) <= 8,
+ "NamespaceDeclBitfields is larger than 8 bytes!");
static_assert(sizeof(TagDeclBitfields) <= 8,
"TagDeclBitfields is larger than 8 bytes!");
static_assert(sizeof(EnumDeclBitfields) <= 8,
diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index 497579dcc56b6..6c89e3890ae3e 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -7250,14 +7250,14 @@ ASTContext::getCanonicalNestedNameSpecifier(NestedNameSpecifier *NNS) const {
// A namespace is canonical; build a nested-name-specifier with
// this namespace and no prefix.
return NestedNameSpecifier::Create(*this, nullptr,
- NNS->getAsNamespace()->getOriginalNamespace());
+ NNS->getAsNamespace()->getFirstDecl());
case NestedNameSpecifier::NamespaceAlias:
// A namespace is canonical; build a nested-name-specifier with
// this namespace and no prefix.
- return NestedNameSpecifier::Create(*this, nullptr,
- NNS->getAsNamespaceAlias()->getNamespace()
- ->getOriginalNamespace());
+ return NestedNameSpecifier::Create(
+ *this, nullptr,
+ NNS->getAsNamespaceAlias()->getNamespace()->getFirstDecl());
// The difference between TypeSpec and TypeSpecWithTemplate is that the
// latter will have the 'template' keyword when printed.
diff --git a/clang/lib/AST/DeclBase.cpp b/clang/lib/AST/DeclBase.cpp
index eef946e3aea2e..81c85c7cf7c5b 100644
--- a/clang/lib/AST/DeclBase.cpp
+++ b/clang/lib/AST/DeclBase.cpp
@@ -1418,8 +1418,7 @@ DeclContext *DeclContext::getPrimaryContext() {
case Decl::TranslationUnit:
return static_cast<TranslationUnitDecl *>(this)->getFirstDecl();
case Decl::Namespace:
- // The original namespace is our primary context.
- return static_cast<NamespaceDecl *>(this)->getOriginalNamespace();
+ return static_cast<NamespaceDecl *>(this)->getFirstDecl();
case Decl::ObjCMethod:
return this;
diff --git a/clang/lib/AST/DeclCXX.cpp b/clang/lib/AST/DeclCXX.cpp
index d5c140fd34389..72d68f39a97a5 100644
--- a/clang/lib/AST/DeclCXX.cpp
+++ b/clang/lib/AST/DeclCXX.cpp
@@ -2941,7 +2941,7 @@ UsingDirectiveDecl *UsingDirectiveDecl::Create(ASTContext &C, DeclContext *DC,
NamedDecl *Used,
DeclContext *CommonAncestor) {
if (auto *NS = dyn_cast_or_null<NamespaceDecl>(Used))
- Used = NS->getOriginalNamespace();
+ Used = NS->getFirstDecl();
return new (C, DC) UsingDirectiveDecl(DC, L, NamespaceLoc, QualifierLoc,
IdentLoc, Used, CommonAncestor);
}
@@ -2966,16 +2966,9 @@ NamespaceDecl::NamespaceDecl(ASTContext &C, DeclContext *DC, bool Inline,
bool Nested)
: NamedDecl(Namespace, DC, IdLoc, Id), DeclContext(Namespace),
redeclarable_base(C), LocStart(StartLoc) {
- unsigned Flags = 0;
- if (Inline)
- Flags |= F_Inline;
- if (Nested)
- Flags |= F_Nested;
- AnonOrFirstNamespaceAndFlags = {nullptr, Flags};
+ setInline(Inline);
+ setNested(Nested);
setPreviousDecl(PrevDecl);
-
- if (PrevDecl)
- AnonOrFirstNamespaceAndFlags.setPointer(PrevDecl->getOriginalNamespace());
}
NamespaceDecl *NamespaceDecl::Create(ASTContext &C, DeclContext *DC,
@@ -2992,22 +2985,6 @@ NamespaceDecl *NamespaceDecl::CreateDeserialized(ASTContext &C,
SourceLocation(), nullptr, nullptr, false);
}
-NamespaceDecl *NamespaceDecl::getOriginalNamespace() {
- if (isFirstDecl())
- return this;
-
- return AnonOrFirstNamespaceAndFlags.getPointer();
-}
-
-const NamespaceDecl *NamespaceDecl::getOriginalNamespace() const {
- if (isFirstDecl())
- return this;
-
- return AnonOrFirstNamespaceAndFlags.getPointer();
-}
-
-bool NamespaceDecl::isOriginalNamespace() const { return isFirstDecl(); }
-
NamespaceDecl *NamespaceDecl::getNextRedeclarationImpl() {
return getNextRedeclaration();
}
@@ -3043,7 +3020,7 @@ NamespaceAliasDecl *NamespaceAliasDecl::Create(ASTContext &C, DeclContext *DC,
NamedDecl *Namespace) {
// FIXME: Preserve the aliased namespace as written.
if (auto *NS = dyn_cast_or_null<NamespaceDecl>(Namespace))
- Namespace = NS->getOriginalNamespace();
+ Namespace = NS->getFirstDecl();
return new (C, DC) NamespaceAliasDecl(C, DC, UsingLoc, AliasLoc, Alias,
QualifierLoc, IdentLoc, Namespace);
}
diff --git a/clang/lib/AST/ItaniumMangle.cpp b/clang/lib/AST/ItaniumMangle.cpp
index f14c4762bee6f..ae64d16266144 100644
--- a/clang/lib/AST/ItaniumMangle.cpp
+++ b/clang/lib/AST/ItaniumMangle.cpp
@@ -960,7 +960,7 @@ bool CXXNameMangler::isStd(const NamespaceDecl *NS) {
if (!Context.getEffectiveParentContext(NS)->isTranslationUnit())
return false;
- const IdentifierInfo *II = NS->getOriginalNamespace()->getIdentifier();
+ const IdentifierInfo *II = NS->getFirstDecl()->getIdentifier();
return II && II->isStr("std");
}
diff --git a/clang/lib/AST/JSONNodeDumper.cpp b/clang/lib/AST/JSONNodeDumper.cpp
index 339477dc65f0f..eeb314b8d32b0 100644
--- a/clang/lib/AST/JSONNodeDumper.cpp
+++ b/clang/lib/AST/JSONNodeDumper.cpp
@@ -883,9 +883,8 @@ void JSONNodeDumper::VisitNamespaceDecl(const NamespaceDecl *ND) {
VisitNamedDecl(ND);
attributeOnlyIfTrue("isInline", ND->isInline());
attributeOnlyIfTrue("isNested", ND->isNested());
- if (!ND->isOriginalNamespace())
- JOS.attribute("originalNamespace",
- createBareDeclRef(ND->getOriginalNamespace()));
+ if (!ND->isFirstDecl())
+ JOS.attribute("originalNamespace", createBareDeclRef(ND->getFirstDecl()));
}
void JSONNodeDumper::VisitUsingDirectiveDecl(const UsingDirectiveDecl *UDD) {
diff --git a/clang/lib/AST/TextNodeDumper.cpp b/clang/lib/AST/TextNodeDumper.cpp
index a26f50f0719c1..5ba9523504258 100644
--- a/clang/lib/AST/TextNodeDumper.cpp
+++ b/clang/lib/AST/TextNodeDumper.cpp
@@ -2386,8 +2386,8 @@ void TextNodeDumper::VisitNamespaceDecl(const NamespaceDecl *D) {
OS << " inline";
if (D->isNested())
OS << " nested";
- if (!D->isOriginalNamespace())
- dumpDeclRef(D->getOriginalNamespace(), "original");
+ if (!D->isFirstDecl())
+ dumpDeclRef(D->getFirstDecl(), "original");
}
void TextNodeDumper::VisitUsingDirectiveDecl(const UsingDirectiveDecl *D) {
diff --git a/clang/lib/Sema/SemaCodeComplete.cpp b/clang/lib/Sema/SemaCodeComplete.cpp
index 8fea7b0cf0d47..88d4732c7d5c6 100644
--- a/clang/lib/Sema/SemaCodeComplete.cpp
+++ b/clang/lib/Sema/SemaCodeComplete.cpp
@@ -6864,7 +6864,7 @@ void SemaCodeCompletion::CodeCompleteNamespaceDecl(Scope *S) {
NS(Ctx->decls_begin()),
NSEnd(Ctx->decls_end());
NS != NSEnd; ++NS)
- OrigToLatest[NS->getOriginalNamespace()] = *NS;
+ OrigToLatest[NS->getFirstDecl()] = *NS;
// Add the most recent definition (or extended definition) of each
// namespace to the list of results.
diff --git a/clang/lib/Sema/SemaLookup.cpp b/clang/lib/Sema/SemaLookup.cpp
index 7851c5d080cf3..7a6a64529f52e 100644
--- a/clang/lib/Sema/SemaLookup.cpp
+++ b/clang/lib/Sema/SemaLookup.cpp
@@ -2325,7 +2325,7 @@ static bool LookupQualifiedNameInUsingDirectives(Sema &S, LookupResult &R,
// We have already looked into the initial namespace; seed the queue
// with its using-children.
for (auto *I : StartDC->using_directives()) {
- NamespaceDecl *ND = I->getNominatedNamespace()->getOriginalNamespace();
+ NamespaceDecl *ND = I->getNominatedNamespace()->getFirstDecl();
if (S.isVisible(I) && Visited.insert(ND).second)
Queue.push_back(ND);
}
diff --git a/clang/lib/Serialization/ASTReaderDecl.cpp b/clang/lib/Serialization/ASTReaderDecl.cpp
index cbaf1b0a98c61..76032aa836b50 100644
--- a/clang/lib/Serialization/ASTReaderDecl.cpp
+++ b/clang/lib/Serialization/ASTReaderDecl.cpp
@@ -1847,13 +1847,8 @@ void ASTDeclReader::VisitNamespaceDecl(NamespaceDecl *D) {
// same namespace, and we have an invariant that older declarations
// get merged before newer ones try to merge.
GlobalDeclID AnonNamespace;
- if (Redecl.getFirstID() == ThisDeclID) {
+ if (Redecl.getFirstID() == ThisDeclID)
AnonNamespace = readDeclID();
- } else {
- // Link this namespace back to the first declaration, which has already
- // been deserialized.
- D->AnonOrFirstNamespaceAndFlags.setPointer(D->getFirstDecl());
- }
mergeRedeclarable(D, Redecl);
@@ -2974,13 +2969,6 @@ void ASTDeclReader::mergeRedeclarable(Redeclarable<T> *DBase, T *Existing,
ExistingCanon->Used |= D->Used;
D->Used = false;
- // When we merge a namespace, update its pointer to the first namespace.
- // We cannot have loaded any redeclarations of this declaration yet, so
- // there's nothing else that needs to be updated.
- if (auto *Namespace = dyn_cast<NamespaceDecl>(D))
- Namespace->AnonOrFirstNamespaceAndFlags.setPointer(
- assert_cast<NamespaceDecl *>(ExistingCanon));
-
// When we merge a template, merge its pattern.
if (auto *DTemplate = dyn_cast<RedeclarableTemplateDecl>(D))
mergeTemplatePattern(
@@ -3293,7 +3281,7 @@ ASTDeclReader::getOrFakePrimaryClassDefinition(ASTReader &Reader,
DeclContext *ASTDeclReader::getPrimaryContextForMerging(ASTReader &Reader,
DeclContext *DC) {
if (auto *ND = dyn_cast<NamespaceDecl>(DC))
- return ND->getOriginalNamespace();
+ return ND->getFirstDecl();
if (auto *RD = dyn_cast<CXXRecordDecl>(DC))
return getOrFakePrimaryClassDefinition(Reader, RD);
diff --git a/clang/lib/Serialization/ASTWriterDecl.cpp b/clang/lib/Serialization/ASTWriterDecl.cpp
index b6583c54c9ba1..5dff0cec5c0ea 100644
--- a/clang/lib/Serialization/ASTWriterDecl.cpp
+++ b/clang/lib/Serialization/ASTWriterDecl.cpp
@@ -1383,7 +1383,7 @@ void ASTDeclWriter::VisitNamespaceDecl(NamespaceDecl *D) {
Record.AddSourceLocation(D->getBeginLoc());
Record.AddSourceLocation(D->getRBraceLoc());
- if (D->isOriginalNamespace())
+ if (D->isFirstDecl())
Record.AddDeclRef(D->getAnonymousNamespace());
Code = serialization::DECL_NAMESPACE;
|
|
Worth noting that I've been investigating in the background whether we can do the opposite transformation, i.e. move members of |
|
@Endilll Should I go ahead with this change anyways? |
I would prefer if you can reduce the scope of the PR by avoiding the introduction of |
mizvekov
approved these changes
Jul 12, 2024
@mizvekov Yes, but it's not directly related. I'm going to submit a patch in the future which stores the |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/66/builds/1531 Here is the relevant piece of the build log for the reference: |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Currently,
NamespaceDeclhas a memberAnonOrFirstNamespaceAndFlagswhich stores a few pieces of data:inline, andNamespaceDeclthat either stores:Redeclarablealready stores a pointer to the first declaration of an entity, so it's unnecessary to store this inNamespaceDecl.DeclContexthas 8 bytes in which various bitfields can be stored for a declaration, so it's not necessary to store these inNamespaceDecleither. We only need to store a pointer to the unnamed namespace that inhabits the first declaration of a namespace. This patch moves the two bits currently stored inNamespaceDecltoDeclContext, and only stores a pointer to the unnamed namespace that inhabits a namespace in the first declaration of that namespace. SincegetOriginalNamespacealways returns the sameNamespaceDeclasgetFirstDecl, this function is removed to avoid confusion.