From e6ec7c8f74d1be778f4ddf794d0e2fb63b0dc3be Mon Sep 17 00:00:00 2001 From: Krystian Stasiowski Date: Mon, 15 Jul 2024 13:57:56 -0400 Subject: [Clang][AST] Move NamespaceDecl bits to DeclContext (#98567) Currently, `NamespaceDecl` has a member `AnonOrFirstNamespaceAndFlags` which stores a few pieces of data: - a bit indicating whether the namespace was declared `inline`, and - a bit indicating whether the namespace was declared as a _nested-namespace-definition_, and - a pointer a `NamespaceDecl` that either stores: - a pointer to the first declaration of that namespace if the declaration is no the first declaration, or - a pointer to the unnamed namespace that inhabits the namespace otherwise. `Redeclarable` already stores a pointer to the first declaration of an entity, so it's unnecessary to store this in `NamespaceDecl`. `DeclContext` has 8 bytes in which various bitfields can be stored for a declaration, so it's not necessary to store these in `NamespaceDecl` either. 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 in `NamespaceDecl` to `DeclContext`, and only stores a pointer to the unnamed namespace that inhabits a namespace in the first declaration of that namespace. Since `getOriginalNamespace` always returns the same `NamespaceDecl` as `getFirstDecl`, this function is removed to avoid confusion. --- .../bugprone/ForwardDeclarationNamespaceCheck.cpp | 2 +- clang/include/clang/AST/Decl.h | 67 +++++----------------- clang/include/clang/AST/DeclBase.h | 24 ++++++++ clang/lib/AST/ASTContext.cpp | 8 +-- clang/lib/AST/DeclBase.cpp | 3 +- clang/lib/AST/DeclCXX.cpp | 31 ++-------- clang/lib/AST/ItaniumMangle.cpp | 2 +- clang/lib/AST/JSONNodeDumper.cpp | 5 +- clang/lib/AST/TextNodeDumper.cpp | 4 +- clang/lib/Sema/SemaCodeComplete.cpp | 2 +- clang/lib/Sema/SemaLookup.cpp | 2 +- clang/lib/Serialization/ASTReaderDecl.cpp | 16 +----- clang/lib/Serialization/ASTWriterDecl.cpp | 2 +- clang/tools/libclang/CXIndexDataConsumer.cpp | 2 +- 14 files changed, 59 insertions(+), 111 deletions(-) diff --git a/clang-tools-extra/clang-tidy/bugprone/ForwardDeclarationNamespaceCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/ForwardDeclarationNamespaceCheck.cpp index f578f7e..0b38b18 100644 --- a/clang-tools-extra/clang-tidy/bugprone/ForwardDeclarationNamespaceCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/ForwardDeclarationNamespaceCheck.cpp @@ -95,7 +95,7 @@ static bool haveSameNamespaceOrTranslationUnit(const CXXRecordDecl *Decl1, "ParentDecl2 declaration must be a namespace"); auto *Ns1 = NamespaceDecl::castFromDeclContext(ParentDecl1); auto *Ns2 = NamespaceDecl::castFromDeclContext(ParentDecl2); - return Ns1->getOriginalNamespace() == Ns2->getOriginalNamespace(); + return Ns1->getFirstDecl() == Ns2->getFirstDecl(); } static std::string getNameOfNamespace(const CXXRecordDecl *Decl) { diff --git a/clang/include/clang/AST/Decl.h b/clang/include/clang/AST/Decl.h index 5957f14..561a9d8 100644 --- a/clang/include/clang/AST/Decl.h +++ b/clang/include/clang/AST/Decl.h @@ -542,12 +542,9 @@ public: }; /// Represent a C++ namespace. -class NamespaceDecl : public NamedDecl, public DeclContext, - public Redeclarable -{ - - enum Flags : unsigned { F_Inline = 1 << 0, F_Nested = 1 << 1 }; - +class NamespaceDecl : public NamedDecl, + public DeclContext, + public Redeclarable { /// 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 - 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 @@ public: } /// 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 @@ public: 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 6c711cf..40f01ab 100644 --- a/clang/include/clang/AST/DeclBase.h +++ b/clang/include/clang/AST/DeclBase.h @@ -1487,6 +1487,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. @@ -1985,6 +2006,7 @@ protected: /// 8 bytes with static_asserts in the ctor of DeclContext. union { DeclContextBitfields DeclContextBits; + NamespaceDeclBitfields NamespaceDeclBits; TagDeclBitfields TagDeclBits; EnumDeclBitfields EnumDeclBits; RecordDeclBitfields RecordDeclBits; @@ -1998,6 +2020,8 @@ protected: 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 497579d..6c89e38 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 ef2c57e..bc5a920 100644 --- a/clang/lib/AST/DeclBase.cpp +++ b/clang/lib/AST/DeclBase.cpp @@ -1422,8 +1422,7 @@ DeclContext *DeclContext::getPrimaryContext() { case Decl::TranslationUnit: return static_cast(this)->getFirstDecl(); case Decl::Namespace: - // The original namespace is our primary context. - return static_cast(this)->getOriginalNamespace(); + return static_cast(this)->getFirstDecl(); case Decl::ObjCMethod: return this; diff --git a/clang/lib/AST/DeclCXX.cpp b/clang/lib/AST/DeclCXX.cpp index d5c140f..72d68f3 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(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(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 5444dcf..40ef827 100644 --- a/clang/lib/AST/ItaniumMangle.cpp +++ b/clang/lib/AST/ItaniumMangle.cpp @@ -961,7 +961,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 339477d..eeb314b 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 a26f50f..5ba9523 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 8fea7b0..88d4732 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 7851c5d..7a6a645 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 cbaf1b0..76032aa 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 *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(D)) - Namespace->AnonOrFirstNamespaceAndFlags.setPointer( - assert_cast(ExistingCanon)); - // When we merge a template, merge its pattern. if (auto *DTemplate = dyn_cast(D)) mergeTemplatePattern( @@ -3293,7 +3281,7 @@ ASTDeclReader::getOrFakePrimaryClassDefinition(ASTReader &Reader, DeclContext *ASTDeclReader::getPrimaryContextForMerging(ASTReader &Reader, DeclContext *DC) { if (auto *ND = dyn_cast(DC)) - return ND->getOriginalNamespace(); + return ND->getFirstDecl(); if (auto *RD = dyn_cast(DC)) return getOrFakePrimaryClassDefinition(Reader, RD); diff --git a/clang/lib/Serialization/ASTWriterDecl.cpp b/clang/lib/Serialization/ASTWriterDecl.cpp index b6583c5..5dff0ce 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; diff --git a/clang/tools/libclang/CXIndexDataConsumer.cpp b/clang/tools/libclang/CXIndexDataConsumer.cpp index c102226..8d364ed 100644 --- a/clang/tools/libclang/CXIndexDataConsumer.cpp +++ b/clang/tools/libclang/CXIndexDataConsumer.cpp @@ -861,7 +861,7 @@ bool CXIndexDataConsumer::handleObjCProperty(const ObjCPropertyDecl *D) { } bool CXIndexDataConsumer::handleNamespace(const NamespaceDecl *D) { - DeclInfo DInfo(/*isRedeclaration=*/!D->isOriginalNamespace(), + DeclInfo DInfo(/*isRedeclaration=*/!D->isFirstDecl(), /*isDefinition=*/true, /*isContainer=*/true); return handleDecl(D, D->getLocation(), getCursor(D), DInfo); -- cgit v1.1