[PATCH] D49729: [AST][1/4] Move the bit-fields from TagDecl, EnumDecl and RecordDecl into DeclContext
This revision was automatically updated to reflect the committed changes. Closed by commit rC338630: [AST][1/4] Move the bit-fields from TagDecl, EnumDecl and RecordDecl into… (authored by erichkeane, committed by ). Changed prior to commit: https://reviews.llvm.org/D49729?vs=158498=158624#toc Repository: rC Clang https://reviews.llvm.org/D49729 Files: include/clang/AST/Decl.h include/clang/AST/DeclBase.h lib/AST/Decl.cpp lib/AST/DeclBase.cpp lib/AST/DeclCXX.cpp lib/AST/DeclTemplate.cpp lib/Serialization/ASTReaderDecl.cpp lib/Serialization/ASTWriter.cpp lib/Serialization/ASTWriterDecl.cpp Index: include/clang/AST/DeclBase.h === --- include/clang/AST/DeclBase.h +++ include/clang/AST/DeclBase.h @@ -1260,37 +1260,407 @@ /// BlockDecl /// OMPDeclareReductionDecl class DeclContext { - /// DeclKind - This indicates which class this is. - unsigned DeclKind : 8; + // We use uint64_t in the bit-fields below since some bit-fields + // cross the unsigned boundary and this breaks the packing. - /// Whether this declaration context also has some external - /// storage that contains additional declarations that are lexically - /// part of this context. - mutable bool ExternalLexicalStorage : 1; - - /// Whether this declaration context also has some external - /// storage that contains additional declarations that are visible - /// in this context. - mutable bool ExternalVisibleStorage : 1; + /// Stores the bits used by DeclContext. + /// If modified NumDeclContextBit, the ctor of DeclContext and the accessor + /// methods in DeclContext should be updated appropriately. + class DeclContextBitfields { +friend class DeclContext; +/// DeclKind - This indicates which class this is. +uint64_t DeclKind : 7; + +/// Whether this declaration context also has some external +/// storage that contains additional declarations that are lexically +/// part of this context. +mutable uint64_t ExternalLexicalStorage : 1; + +/// Whether this declaration context also has some external +/// storage that contains additional declarations that are visible +/// in this context. +mutable uint64_t ExternalVisibleStorage : 1; + +/// Whether this declaration context has had externally visible +/// storage added since the last lookup. In this case, \c LookupPtr's +/// invariant may not hold and needs to be fixed before we perform +/// another lookup. +mutable uint64_t NeedToReconcileExternalVisibleStorage : 1; + +/// If \c true, this context may have local lexical declarations +/// that are missing from the lookup table. +mutable uint64_t HasLazyLocalLexicalLookups : 1; + +/// If \c true, the external source may have lexical declarations +/// that are missing from the lookup table. +mutable uint64_t HasLazyExternalLexicalLookups : 1; + +/// If \c true, lookups should only return identifier from +/// DeclContext scope (for example TranslationUnit). Used in +/// LookupQualifiedName() +mutable uint64_t UseQualifiedLookup : 1; + }; - /// Whether this declaration context has had external visible - /// storage added since the last lookup. In this case, \c LookupPtr's - /// invariant may not hold and needs to be fixed before we perform - /// another lookup. - mutable bool NeedToReconcileExternalVisibleStorage : 1; + /// Number of bits in DeclContextBitfields. + enum { NumDeclContextBits = 13 }; - /// If \c true, this context may have local lexical declarations - /// that are missing from the lookup table. - mutable bool HasLazyLocalLexicalLookups : 1; + /// Stores the bits used by TagDecl. + /// If modified NumTagDeclBits and the accessor + /// methods in TagDecl should be updated appropriately. + class TagDeclBitfields { +friend class TagDecl; +/// For the bits in DeclContextBitfields +uint64_t : NumDeclContextBits; + +/// The TagKind enum. +uint64_t TagDeclKind : 3; + +/// True if this is a definition ("struct foo {};"), false if it is a +/// declaration ("struct foo;"). It is not considered a definition +/// until the definition has been fully processed. +uint64_t IsCompleteDefinition : 1; + +/// True if this is currently being defined. +uint64_t IsBeingDefined : 1; + +/// True if this tag declaration is "embedded" (i.e., defined or declared +/// for the very first time) in the syntax of a declarator. +uint64_t IsEmbeddedInDeclarator : 1; + +/// True if this tag is free standing, e.g. "struct foo;". +uint64_t IsFreeStanding : 1; + +/// Indicates whether it is possible for declarations of this kind +/// to have an out-of-date definition. +/// +/// This option is only enabled when modules are enabled. +uint64_t MayHaveOutOfDateDef : 1; + +/// Has the full definition of this type been required by a use somewhere in +/// the TU. +
[PATCH] D49729: [AST][1/4] Move the bit-fields from TagDecl, EnumDecl and RecordDecl into DeclContext
erichkeane accepted this revision. erichkeane added a comment. LGTM! Repository: rC Clang https://reviews.llvm.org/D49729 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D49729: [AST][1/4] Move the bit-fields from TagDecl, EnumDecl and RecordDecl into DeclContext
bricci added a comment. @erichkeane do you have any additional comments regarding this set of patches ? I retested them on top of trunk and did not find any problem. Repository: rC Clang https://reviews.llvm.org/D49729 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D49729: [AST][1/4] Move the bit-fields from TagDecl, EnumDecl and RecordDecl into DeclContext
bricci updated this revision to Diff 158498. bricci added a comment. rebased after the great whitespace trimming Repository: rC Clang https://reviews.llvm.org/D49729 Files: include/clang/AST/Decl.h include/clang/AST/DeclBase.h lib/AST/Decl.cpp lib/AST/DeclBase.cpp lib/AST/DeclCXX.cpp lib/AST/DeclTemplate.cpp lib/Serialization/ASTReaderDecl.cpp lib/Serialization/ASTWriter.cpp lib/Serialization/ASTWriterDecl.cpp Index: lib/Serialization/ASTWriterDecl.cpp === --- lib/Serialization/ASTWriterDecl.cpp +++ lib/Serialization/ASTWriterDecl.cpp @@ -1275,7 +1275,7 @@ // Store (what we currently believe to be) the key function to avoid // deserializing every method so we can compute it. - if (D->IsCompleteDefinition) + if (D->isCompleteDefinition()) Record.AddDeclRef(Context.getCurrentKeyFunction(D)); Code = serialization::DECL_CXX_RECORD; Index: lib/Serialization/ASTWriter.cpp === --- lib/Serialization/ASTWriter.cpp +++ lib/Serialization/ASTWriter.cpp @@ -3960,7 +3960,8 @@ bool ASTWriter::isLookupResultExternal(StoredDeclsList , DeclContext *DC) { - return Result.hasExternalDecls() && DC->NeedToReconcileExternalVisibleStorage; + return Result.hasExternalDecls() && + DC->hasNeedToReconcileExternalVisibleStorage(); } bool ASTWriter::isLookupResultEntirelyExternal(StoredDeclsList , @@ -3975,8 +3976,8 @@ void ASTWriter::GenerateNameLookupTable(const DeclContext *ConstDC, llvm::SmallVectorImpl ) { - assert(!ConstDC->HasLazyLocalLexicalLookups && - !ConstDC->HasLazyExternalLexicalLookups && + assert(!ConstDC->hasLazyLocalLexicalLookups() && + !ConstDC->hasLazyExternalLexicalLookups() && "must call buildLookups first"); // FIXME: We need to build the lookups table, which is logically const. Index: lib/Serialization/ASTReaderDecl.cpp === --- lib/Serialization/ASTReaderDecl.cpp +++ lib/Serialization/ASTReaderDecl.cpp @@ -742,16 +742,16 @@ ED->setPromotionType(Record.readType()); ED->setNumPositiveBits(Record.readInt()); ED->setNumNegativeBits(Record.readInt()); - ED->IsScoped = Record.readInt(); - ED->IsScopedUsingClassTag = Record.readInt(); - ED->IsFixed = Record.readInt(); + ED->setScoped(Record.readInt()); + ED->setScopedUsingClassTag(Record.readInt()); + ED->setFixed(Record.readInt()); - ED->HasODRHash = true; + ED->setHasODRHash(true); ED->ODRHash = Record.readInt(); // If this is a definition subject to the ODR, and we already have a // definition, merge this one into it. - if (ED->IsCompleteDefinition && + if (ED->isCompleteDefinition() && Reader.getContext().getLangOpts().Modules && Reader.getContext().getLangOpts().CPlusPlus) { EnumDecl * = Reader.EnumDefinitions[ED->getCanonicalDecl()]; @@ -767,7 +767,7 @@ } if (OldDef) { Reader.MergedDeclContexts.insert(std::make_pair(ED, OldDef)); - ED->IsCompleteDefinition = false; + ED->setCompleteDefinition(false); Reader.mergeDefinitionVisibility(OldDef, ED); if (OldDef->getODRHash() != ED->getODRHash()) Reader.PendingEnumOdrMergeFailures[OldDef].push_back(ED); @@ -1744,7 +1744,7 @@ Reader.MergedDeclContexts.insert(std::make_pair(MergeDD.Definition, DD.Definition)); Reader.PendingDefinitions.erase(MergeDD.Definition); -MergeDD.Definition->IsCompleteDefinition = false; +MergeDD.Definition->setCompleteDefinition(false); Reader.mergeDefinitionVisibility(DD.Definition, MergeDD.Definition); assert(Reader.Lookups.find(MergeDD.Definition) == Reader.Lookups.end() && "already loaded pending lookups for merged definition"); @@ -1884,7 +1884,7 @@ } // Mark this declaration as being a definition. - D->IsCompleteDefinition = true; + D->setCompleteDefinition(true); // If this is not the first declaration or is an update record, we can have // other redeclarations already. Make a note that we need to propagate the @@ -1946,7 +1946,7 @@ // compute it. if (WasDefinition) { DeclID KeyFn = ReadDeclID(); -if (KeyFn && D->IsCompleteDefinition) +if (KeyFn && D->isCompleteDefinition()) // FIXME: This is wrong for the ARM ABI, where some other module may have // made this function no longer be a key function. We need an update // record or similar for that case. @@ -3076,7 +3076,7 @@ // we load the update record. if (!DD) { DD = new (Reader.getContext()) struct CXXRecordDecl::DefinitionData(RD); - RD->IsCompleteDefinition = true; + RD->setCompleteDefinition(true); RD->DefinitionData = DD; RD->getCanonicalDecl()->DefinitionData = DD; Index:
[PATCH] D49729: [AST][1/4] Move the bit-fields from TagDecl, EnumDecl and RecordDecl into DeclContext
bricci updated this revision to Diff 157522. bricci marked 2 inline comments as done. bricci added a comment. - Re-based: r337978 [ODRHash] Support hashing enums added an ODRHash to EnumDecl which conflicts with these changes. The corresponding flag HasODRHash has been put into EnumDeclBitfields. - The ugly IntegerType = (const Type *)nullptr; has been cleaned to IntegerType = nullptr; I guess the intent was to disambiguate between the two possible operator= but there is a special case for nullptr so this is not needed since const Type * is the first element of the pair. - Made hasNeedToReconcileExternalVisibleStorage, hasLazyLocalLexicalLookups and hasLazyExternalLexicalLookups private. This match what was originally the case with the private bit-fields. This also requires making ASTWriter a friend of DeclContext (as was the case before). - Fixed some comments in EnumDecl. Repository: rC Clang https://reviews.llvm.org/D49729 Files: include/clang/AST/Decl.h include/clang/AST/DeclBase.h lib/AST/Decl.cpp lib/AST/DeclBase.cpp lib/AST/DeclCXX.cpp lib/AST/DeclTemplate.cpp lib/Serialization/ASTReaderDecl.cpp lib/Serialization/ASTWriter.cpp lib/Serialization/ASTWriterDecl.cpp Index: lib/Serialization/ASTWriterDecl.cpp === --- lib/Serialization/ASTWriterDecl.cpp +++ lib/Serialization/ASTWriterDecl.cpp @@ -1275,7 +1275,7 @@ // Store (what we currently believe to be) the key function to avoid // deserializing every method so we can compute it. - if (D->IsCompleteDefinition) + if (D->isCompleteDefinition()) Record.AddDeclRef(Context.getCurrentKeyFunction(D)); Code = serialization::DECL_CXX_RECORD; Index: lib/Serialization/ASTWriter.cpp === --- lib/Serialization/ASTWriter.cpp +++ lib/Serialization/ASTWriter.cpp @@ -3960,7 +3960,8 @@ bool ASTWriter::isLookupResultExternal(StoredDeclsList , DeclContext *DC) { - return Result.hasExternalDecls() && DC->NeedToReconcileExternalVisibleStorage; + return Result.hasExternalDecls() && + DC->hasNeedToReconcileExternalVisibleStorage(); } bool ASTWriter::isLookupResultEntirelyExternal(StoredDeclsList , @@ -3975,8 +3976,8 @@ void ASTWriter::GenerateNameLookupTable(const DeclContext *ConstDC, llvm::SmallVectorImpl ) { - assert(!ConstDC->HasLazyLocalLexicalLookups && - !ConstDC->HasLazyExternalLexicalLookups && + assert(!ConstDC->hasLazyLocalLexicalLookups() && + !ConstDC->hasLazyExternalLexicalLookups() && "must call buildLookups first"); // FIXME: We need to build the lookups table, which is logically const. Index: lib/Serialization/ASTReaderDecl.cpp === --- lib/Serialization/ASTReaderDecl.cpp +++ lib/Serialization/ASTReaderDecl.cpp @@ -742,16 +742,16 @@ ED->setPromotionType(Record.readType()); ED->setNumPositiveBits(Record.readInt()); ED->setNumNegativeBits(Record.readInt()); - ED->IsScoped = Record.readInt(); - ED->IsScopedUsingClassTag = Record.readInt(); - ED->IsFixed = Record.readInt(); + ED->setScoped(Record.readInt()); + ED->setScopedUsingClassTag(Record.readInt()); + ED->setFixed(Record.readInt()); - ED->HasODRHash = true; + ED->setHasODRHash(true); ED->ODRHash = Record.readInt(); // If this is a definition subject to the ODR, and we already have a // definition, merge this one into it. - if (ED->IsCompleteDefinition && + if (ED->isCompleteDefinition() && Reader.getContext().getLangOpts().Modules && Reader.getContext().getLangOpts().CPlusPlus) { EnumDecl * = Reader.EnumDefinitions[ED->getCanonicalDecl()]; @@ -767,7 +767,7 @@ } if (OldDef) { Reader.MergedDeclContexts.insert(std::make_pair(ED, OldDef)); - ED->IsCompleteDefinition = false; + ED->setCompleteDefinition(false); Reader.mergeDefinitionVisibility(OldDef, ED); if (OldDef->getODRHash() != ED->getODRHash()) Reader.PendingEnumOdrMergeFailures[OldDef].push_back(ED); @@ -1744,7 +1744,7 @@ Reader.MergedDeclContexts.insert(std::make_pair(MergeDD.Definition, DD.Definition)); Reader.PendingDefinitions.erase(MergeDD.Definition); -MergeDD.Definition->IsCompleteDefinition = false; +MergeDD.Definition->setCompleteDefinition(false); Reader.mergeDefinitionVisibility(DD.Definition, MergeDD.Definition); assert(Reader.Lookups.find(MergeDD.Definition) == Reader.Lookups.end() && "already loaded pending lookups for merged definition"); @@ -1884,7 +1884,7 @@ } // Mark this declaration as being a definition. - D->IsCompleteDefinition = true; + D->setCompleteDefinition(true); // If this is not the first declaration or is an update record, we can have // other
[PATCH] D49729: [AST][1/4] Move the bit-fields from TagDecl, EnumDecl and RecordDecl into DeclContext
erichkeane added inline comments. Comment at: include/clang/AST/Decl.h: + + /// True if this is a C++11 scoped enumeration. + void setScopedUsingClassTag(bool ScopedUCT = true) { This is the same comment as 3330, perhaps a copy/paste error? Comment at: lib/AST/Decl.cpp:3896 + assert(Scoped || !ScopedUsingClassTag); + IntegerType = (const Type *)nullptr; + setNumPositiveBits(0); This cast is jarring. C-Style casts shouldn't be used (i realize it is in the source), and I'm not sure it is necessary here. Can you try removing the cast entirely? Repository: rC Clang https://reviews.llvm.org/D49729 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D49729: [AST][1/4] Move the bit-fields from TagDecl, EnumDecl and RecordDecl into DeclContext
rsmith added a comment. I've not done a full review, but the approach here looks good, thanks! Repository: rC Clang https://reviews.llvm.org/D49729 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D49729: [AST][1/4] Move the bit-fields from TagDecl, EnumDecl and RecordDecl into DeclContext
bricci updated this revision to Diff 157288. bricci added a comment. - clang-format on the changes in the .cpp files Repository: rC Clang https://reviews.llvm.org/D49729 Files: include/clang/AST/Decl.h include/clang/AST/DeclBase.h lib/AST/Decl.cpp lib/AST/DeclBase.cpp lib/AST/DeclCXX.cpp lib/AST/DeclTemplate.cpp lib/Serialization/ASTReaderDecl.cpp lib/Serialization/ASTWriter.cpp lib/Serialization/ASTWriterDecl.cpp Index: lib/Serialization/ASTWriterDecl.cpp === --- lib/Serialization/ASTWriterDecl.cpp +++ lib/Serialization/ASTWriterDecl.cpp @@ -1273,7 +1273,7 @@ // Store (what we currently believe to be) the key function to avoid // deserializing every method so we can compute it. - if (D->IsCompleteDefinition) + if (D->isCompleteDefinition()) Record.AddDeclRef(Context.getCurrentKeyFunction(D)); Code = serialization::DECL_CXX_RECORD; Index: lib/Serialization/ASTWriter.cpp === --- lib/Serialization/ASTWriter.cpp +++ lib/Serialization/ASTWriter.cpp @@ -3960,7 +3960,8 @@ bool ASTWriter::isLookupResultExternal(StoredDeclsList , DeclContext *DC) { - return Result.hasExternalDecls() && DC->NeedToReconcileExternalVisibleStorage; + return Result.hasExternalDecls() && + DC->hasNeedToReconcileExternalVisibleStorage(); } bool ASTWriter::isLookupResultEntirelyExternal(StoredDeclsList , @@ -3975,8 +3976,8 @@ void ASTWriter::GenerateNameLookupTable(const DeclContext *ConstDC, llvm::SmallVectorImpl ) { - assert(!ConstDC->HasLazyLocalLexicalLookups && - !ConstDC->HasLazyExternalLexicalLookups && + assert(!ConstDC->hasLazyLocalLexicalLookups() && + !ConstDC->hasLazyExternalLexicalLookups() && "must call buildLookups first"); // FIXME: We need to build the lookups table, which is logically const. Index: lib/Serialization/ASTReaderDecl.cpp === --- lib/Serialization/ASTReaderDecl.cpp +++ lib/Serialization/ASTReaderDecl.cpp @@ -742,13 +742,13 @@ ED->setPromotionType(Record.readType()); ED->setNumPositiveBits(Record.readInt()); ED->setNumNegativeBits(Record.readInt()); - ED->IsScoped = Record.readInt(); - ED->IsScopedUsingClassTag = Record.readInt(); - ED->IsFixed = Record.readInt(); + ED->setScoped(Record.readInt()); + ED->setScopedUsingClassTag(Record.readInt()); + ED->setFixed(Record.readInt()); // If this is a definition subject to the ODR, and we already have a // definition, merge this one into it. - if (ED->IsCompleteDefinition && + if (ED->isCompleteDefinition() && Reader.getContext().getLangOpts().Modules && Reader.getContext().getLangOpts().CPlusPlus) { EnumDecl * = Reader.EnumDefinitions[ED->getCanonicalDecl()]; @@ -764,7 +764,7 @@ } if (OldDef) { Reader.MergedDeclContexts.insert(std::make_pair(ED, OldDef)); - ED->IsCompleteDefinition = false; + ED->setCompleteDefinition(false); Reader.mergeDefinitionVisibility(OldDef, ED); } else { OldDef = ED; @@ -1739,7 +1739,7 @@ Reader.MergedDeclContexts.insert(std::make_pair(MergeDD.Definition, DD.Definition)); Reader.PendingDefinitions.erase(MergeDD.Definition); -MergeDD.Definition->IsCompleteDefinition = false; +MergeDD.Definition->setCompleteDefinition(false); Reader.mergeDefinitionVisibility(DD.Definition, MergeDD.Definition); assert(Reader.Lookups.find(MergeDD.Definition) == Reader.Lookups.end() && "already loaded pending lookups for merged definition"); @@ -1879,7 +1879,7 @@ } // Mark this declaration as being a definition. - D->IsCompleteDefinition = true; + D->setCompleteDefinition(true); // If this is not the first declaration or is an update record, we can have // other redeclarations already. Make a note that we need to propagate the @@ -1941,7 +1941,7 @@ // compute it. if (WasDefinition) { DeclID KeyFn = ReadDeclID(); -if (KeyFn && D->IsCompleteDefinition) +if (KeyFn && D->isCompleteDefinition()) // FIXME: This is wrong for the ARM ABI, where some other module may have // made this function no longer be a key function. We need an update // record or similar for that case. @@ -3071,7 +3071,7 @@ // we load the update record. if (!DD) { DD = new (Reader.getContext()) struct CXXRecordDecl::DefinitionData(RD); - RD->IsCompleteDefinition = true; + RD->setCompleteDefinition(true); RD->DefinitionData = DD; RD->getCanonicalDecl()->DefinitionData = DD; Index: lib/AST/DeclTemplate.cpp === --- lib/AST/DeclTemplate.cpp +++ lib/AST/DeclTemplate.cpp @@ -712,7 +712,7 @@ new
[PATCH] D49729: [AST][1/4] Move the bit-fields from TagDecl, EnumDecl and RecordDecl into DeclContext
bricci added a comment. It might be better to wait just after the upcoming release branch in any case since even though it is all NFC this causes a bit of churn. Repository: rC Clang https://reviews.llvm.org/D49729 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D49729: [AST][1/4] Move the bit-fields from TagDecl, EnumDecl and RecordDecl into DeclContext
erichkeane added a comment. This looks acceptable to me. I'd like to get @rsmith or @rnk to approve the approach though. Repository: rC Clang https://reviews.llvm.org/D49729 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D49729: [AST][1/4] Move the bit-fields from TagDecl, EnumDecl and RecordDecl into DeclContext
bricci updated this revision to Diff 157231. bricci marked 10 inline comments as done. bricci added a comment. address @erichkeane comments: - ran clang-format on changed parts - made the setters setNumPositiveBits, setNumPositiveBits, setScoped, setScopedUsingClassTag and setFixed in EnumDecl private. - made the setters setBeingDefined and setBeingDefined in TagDecl protected - moved the static_asserts to the anonymous union - factored out the doc update of DeclBase. This is now https://reviews.llvm.org/D49790 - various typos Repository: rC Clang https://reviews.llvm.org/D49729 Files: include/clang/AST/Decl.h include/clang/AST/DeclBase.h lib/AST/Decl.cpp lib/AST/DeclBase.cpp lib/AST/DeclCXX.cpp lib/AST/DeclTemplate.cpp lib/Serialization/ASTReaderDecl.cpp lib/Serialization/ASTWriter.cpp lib/Serialization/ASTWriterDecl.cpp Index: lib/Serialization/ASTWriterDecl.cpp === --- lib/Serialization/ASTWriterDecl.cpp +++ lib/Serialization/ASTWriterDecl.cpp @@ -1273,7 +1273,7 @@ // Store (what we currently believe to be) the key function to avoid // deserializing every method so we can compute it. - if (D->IsCompleteDefinition) + if (D->isCompleteDefinition()) Record.AddDeclRef(Context.getCurrentKeyFunction(D)); Code = serialization::DECL_CXX_RECORD; Index: lib/Serialization/ASTWriter.cpp === --- lib/Serialization/ASTWriter.cpp +++ lib/Serialization/ASTWriter.cpp @@ -3960,7 +3960,8 @@ bool ASTWriter::isLookupResultExternal(StoredDeclsList , DeclContext *DC) { - return Result.hasExternalDecls() && DC->NeedToReconcileExternalVisibleStorage; + return Result.hasExternalDecls() && + DC->hasNeedToReconcileExternalVisibleStorage(); } bool ASTWriter::isLookupResultEntirelyExternal(StoredDeclsList , @@ -3975,8 +3976,8 @@ void ASTWriter::GenerateNameLookupTable(const DeclContext *ConstDC, llvm::SmallVectorImpl ) { - assert(!ConstDC->HasLazyLocalLexicalLookups && - !ConstDC->HasLazyExternalLexicalLookups && + assert(!ConstDC->hasLazyLocalLexicalLookups() && + !ConstDC->hasLazyExternalLexicalLookups() && "must call buildLookups first"); // FIXME: We need to build the lookups table, which is logically const. Index: lib/Serialization/ASTReaderDecl.cpp === --- lib/Serialization/ASTReaderDecl.cpp +++ lib/Serialization/ASTReaderDecl.cpp @@ -742,13 +742,13 @@ ED->setPromotionType(Record.readType()); ED->setNumPositiveBits(Record.readInt()); ED->setNumNegativeBits(Record.readInt()); - ED->IsScoped = Record.readInt(); - ED->IsScopedUsingClassTag = Record.readInt(); - ED->IsFixed = Record.readInt(); + ED->setScoped(Record.readInt()); + ED->setScopedUsingClassTag(Record.readInt()); + ED->setFixed(Record.readInt()); // If this is a definition subject to the ODR, and we already have a // definition, merge this one into it. - if (ED->IsCompleteDefinition && + if (ED->isCompleteDefinition() && Reader.getContext().getLangOpts().Modules && Reader.getContext().getLangOpts().CPlusPlus) { EnumDecl * = Reader.EnumDefinitions[ED->getCanonicalDecl()]; @@ -764,7 +764,7 @@ } if (OldDef) { Reader.MergedDeclContexts.insert(std::make_pair(ED, OldDef)); - ED->IsCompleteDefinition = false; + ED->setCompleteDefinition(false); Reader.mergeDefinitionVisibility(OldDef, ED); } else { OldDef = ED; @@ -1739,7 +1739,7 @@ Reader.MergedDeclContexts.insert(std::make_pair(MergeDD.Definition, DD.Definition)); Reader.PendingDefinitions.erase(MergeDD.Definition); -MergeDD.Definition->IsCompleteDefinition = false; +MergeDD.Definition->setCompleteDefinition(false); Reader.mergeDefinitionVisibility(DD.Definition, MergeDD.Definition); assert(Reader.Lookups.find(MergeDD.Definition) == Reader.Lookups.end() && "already loaded pending lookups for merged definition"); @@ -1879,7 +1879,7 @@ } // Mark this declaration as being a definition. - D->IsCompleteDefinition = true; + D->setCompleteDefinition(true); // If this is not the first declaration or is an update record, we can have // other redeclarations already. Make a note that we need to propagate the @@ -1941,7 +1941,7 @@ // compute it. if (WasDefinition) { DeclID KeyFn = ReadDeclID(); -if (KeyFn && D->IsCompleteDefinition) +if (KeyFn && D->isCompleteDefinition()) // FIXME: This is wrong for the ARM ABI, where some other module may have // made this function no longer be a key function. We need an update // record or similar for that case. @@ -3071,7 +3071,7 @@ // we load the update record. if (!DD) { DD = new
[PATCH] D49729: [AST][1/4] Move the bit-fields from TagDecl, EnumDecl and RecordDecl into DeclContext
bricci updated this revision to Diff 157063. bricci added a comment. - removed some unrelated change - add a comment about why uint64_t instead of unsigned Repository: rC Clang https://reviews.llvm.org/D49729 Files: include/clang/AST/Decl.h include/clang/AST/DeclBase.h lib/AST/Decl.cpp lib/AST/DeclBase.cpp lib/AST/DeclCXX.cpp lib/AST/DeclTemplate.cpp lib/Serialization/ASTReaderDecl.cpp lib/Serialization/ASTWriter.cpp lib/Serialization/ASTWriterDecl.cpp Index: lib/Serialization/ASTWriterDecl.cpp === --- lib/Serialization/ASTWriterDecl.cpp +++ lib/Serialization/ASTWriterDecl.cpp @@ -1273,7 +1273,7 @@ // Store (what we currently believe to be) the key function to avoid // deserializing every method so we can compute it. - if (D->IsCompleteDefinition) + if (D->isCompleteDefinition()) Record.AddDeclRef(Context.getCurrentKeyFunction(D)); Code = serialization::DECL_CXX_RECORD; Index: lib/Serialization/ASTWriter.cpp === --- lib/Serialization/ASTWriter.cpp +++ lib/Serialization/ASTWriter.cpp @@ -3960,7 +3960,8 @@ bool ASTWriter::isLookupResultExternal(StoredDeclsList , DeclContext *DC) { - return Result.hasExternalDecls() && DC->NeedToReconcileExternalVisibleStorage; + return Result.hasExternalDecls() && + DC->hasNeedToReconcileExternalVisibleStorage(); } bool ASTWriter::isLookupResultEntirelyExternal(StoredDeclsList , @@ -3975,8 +3976,8 @@ void ASTWriter::GenerateNameLookupTable(const DeclContext *ConstDC, llvm::SmallVectorImpl ) { - assert(!ConstDC->HasLazyLocalLexicalLookups && - !ConstDC->HasLazyExternalLexicalLookups && + assert(!ConstDC->hasLazyLocalLexicalLookups() && + !ConstDC->hasLazyExternalLexicalLookups() && "must call buildLookups first"); // FIXME: We need to build the lookups table, which is logically const. Index: lib/Serialization/ASTReaderDecl.cpp === --- lib/Serialization/ASTReaderDecl.cpp +++ lib/Serialization/ASTReaderDecl.cpp @@ -742,13 +742,13 @@ ED->setPromotionType(Record.readType()); ED->setNumPositiveBits(Record.readInt()); ED->setNumNegativeBits(Record.readInt()); - ED->IsScoped = Record.readInt(); - ED->IsScopedUsingClassTag = Record.readInt(); - ED->IsFixed = Record.readInt(); + ED->setScoped(Record.readInt()); + ED->setScopedUsingClassTag(Record.readInt()); + ED->setFixed(Record.readInt()); // If this is a definition subject to the ODR, and we already have a // definition, merge this one into it. - if (ED->IsCompleteDefinition && + if (ED->isCompleteDefinition() && Reader.getContext().getLangOpts().Modules && Reader.getContext().getLangOpts().CPlusPlus) { EnumDecl * = Reader.EnumDefinitions[ED->getCanonicalDecl()]; @@ -764,7 +764,7 @@ } if (OldDef) { Reader.MergedDeclContexts.insert(std::make_pair(ED, OldDef)); - ED->IsCompleteDefinition = false; + ED->setCompleteDefinition(false); Reader.mergeDefinitionVisibility(OldDef, ED); } else { OldDef = ED; @@ -1739,7 +1739,7 @@ Reader.MergedDeclContexts.insert(std::make_pair(MergeDD.Definition, DD.Definition)); Reader.PendingDefinitions.erase(MergeDD.Definition); -MergeDD.Definition->IsCompleteDefinition = false; +MergeDD.Definition->setCompleteDefinition(false); Reader.mergeDefinitionVisibility(DD.Definition, MergeDD.Definition); assert(Reader.Lookups.find(MergeDD.Definition) == Reader.Lookups.end() && "already loaded pending lookups for merged definition"); @@ -1879,7 +1879,7 @@ } // Mark this declaration as being a definition. - D->IsCompleteDefinition = true; + D->setCompleteDefinition(true); // If this is not the first declaration or is an update record, we can have // other redeclarations already. Make a note that we need to propagate the @@ -1941,7 +1941,7 @@ // compute it. if (WasDefinition) { DeclID KeyFn = ReadDeclID(); -if (KeyFn && D->IsCompleteDefinition) +if (KeyFn && D->isCompleteDefinition()) // FIXME: This is wrong for the ARM ABI, where some other module may have // made this function no longer be a key function. We need an update // record or similar for that case. @@ -3071,7 +3071,7 @@ // we load the update record. if (!DD) { DD = new (Reader.getContext()) struct CXXRecordDecl::DefinitionData(RD); - RD->IsCompleteDefinition = true; + RD->setCompleteDefinition(true); RD->DefinitionData = DD; RD->getCanonicalDecl()->DefinitionData = DD; Index: lib/AST/DeclTemplate.cpp === --- lib/AST/DeclTemplate.cpp +++
[PATCH] D49729: [AST][1/4] Move the bit-fields from TagDecl, EnumDecl and RecordDecl into DeclContext
erichkeane added a comment. This seems like a patch with a good direction, I generally think that this change doesn't change the readability of the code (and generally matches the structure of other code in clang). It is generally a mechanical change, though I'd suggest running clang-format (https://clang.llvm.org/docs/ClangFormat.html#script-for-patch-reformatting) along this patch. Comment at: include/clang/AST/Decl.h:3115 + /// True if this decl has its body fully specified. + void setCompleteDefinition(bool V = true) { +TagDeclBits.IsCompleteDefinition = V; The setters that you've moved change the accessibility of these variables. I'd much prefer we maintain the protected/privateness of these. In a few of these cases, they shouldn't be modified outside of the Decl itself Comment at: include/clang/AST/Decl.h:3155 + + bool mayHaveOutOfDateDef() const { +return TagDeclBits.MayHaveOutOfDateDef; These two should also be documented. Comment at: include/clang/AST/Decl.h:3586 +return RecordDeclBits.HasFlexibleArrayMember; + } + void setHasFlexibleArrayMember(bool V) { While you're reflowing this, put a new line between teh definitions throughout the patch (of ones where you're modifying them). Comment at: include/clang/AST/DeclBase.h:1253 /// TranslationUnitDecl +/// ExternCContext /// NamespaceDecl While this documentation change is appreciated, I believe it would fit better in a separate patch. Comment at: include/clang/AST/DeclBase.h:1312 + + /// Stores the bits used by TagDecl. + /// If modified NumTagDeclBits and the accessor The newlines in this comment feel strange, did you run clang-format on these? Comment at: include/clang/AST/DeclBase.h:1603 + +// Not a bitfield but this save space. +// Note that ObjCContainerDeclBitfields is full. s/save/saves Comment at: include/clang/AST/DeclBase.h:2299 + + /// Whether this declaration context has had external visible + /// storage added since the last lookup. In this case, \c LookupPtr's 'externally visible'? Comment at: include/clang/AST/DeclBase.h:2345 private: - friend class DependentDiagnostic; + /// State that this declaration context has had external visible + /// storage added since the last lookup. In this case, \c LookupPtr's Again, externally visible? Comment at: lib/AST/DeclBase.cpp:991 + setUseQualifiedLookup(false); + static_assert(sizeof(DeclContextBitfields) <= 8, + "DeclContextBitfields is larger than 8 bytes!"); Is there value to having these static-asserts in the constructor here? It seems that it would go best under the 'union' that includes all of these. Repository: rC Clang https://reviews.llvm.org/D49729 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D49729: [AST][1/4] Move the bit-fields from TagDecl, EnumDecl and RecordDecl into DeclContext
bricci marked an inline comment as done. bricci added a comment. remove the inline comment about uint64_t. Repository: rC Clang https://reviews.llvm.org/D49729 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D49729: [AST][1/4] Move the bit-fields from TagDecl, EnumDecl and RecordDecl into DeclContext
bricci updated this revision to Diff 157052. bricci added a comment. updated some some comments in DeclContext Repository: rC Clang https://reviews.llvm.org/D49729 Files: include/clang/AST/Decl.h include/clang/AST/DeclBase.h lib/AST/Decl.cpp lib/AST/DeclBase.cpp lib/AST/DeclCXX.cpp lib/AST/DeclTemplate.cpp lib/Serialization/ASTReaderDecl.cpp lib/Serialization/ASTWriter.cpp lib/Serialization/ASTWriterDecl.cpp Index: lib/Serialization/ASTWriterDecl.cpp === --- lib/Serialization/ASTWriterDecl.cpp +++ lib/Serialization/ASTWriterDecl.cpp @@ -1273,7 +1273,7 @@ // Store (what we currently believe to be) the key function to avoid // deserializing every method so we can compute it. - if (D->IsCompleteDefinition) + if (D->isCompleteDefinition()) Record.AddDeclRef(Context.getCurrentKeyFunction(D)); Code = serialization::DECL_CXX_RECORD; Index: lib/Serialization/ASTWriter.cpp === --- lib/Serialization/ASTWriter.cpp +++ lib/Serialization/ASTWriter.cpp @@ -3960,7 +3960,8 @@ bool ASTWriter::isLookupResultExternal(StoredDeclsList , DeclContext *DC) { - return Result.hasExternalDecls() && DC->NeedToReconcileExternalVisibleStorage; + return Result.hasExternalDecls() && + DC->hasNeedToReconcileExternalVisibleStorage(); } bool ASTWriter::isLookupResultEntirelyExternal(StoredDeclsList , @@ -3975,8 +3976,8 @@ void ASTWriter::GenerateNameLookupTable(const DeclContext *ConstDC, llvm::SmallVectorImpl ) { - assert(!ConstDC->HasLazyLocalLexicalLookups && - !ConstDC->HasLazyExternalLexicalLookups && + assert(!ConstDC->hasLazyLocalLexicalLookups() && + !ConstDC->hasLazyExternalLexicalLookups() && "must call buildLookups first"); // FIXME: We need to build the lookups table, which is logically const. Index: lib/Serialization/ASTReaderDecl.cpp === --- lib/Serialization/ASTReaderDecl.cpp +++ lib/Serialization/ASTReaderDecl.cpp @@ -742,13 +742,13 @@ ED->setPromotionType(Record.readType()); ED->setNumPositiveBits(Record.readInt()); ED->setNumNegativeBits(Record.readInt()); - ED->IsScoped = Record.readInt(); - ED->IsScopedUsingClassTag = Record.readInt(); - ED->IsFixed = Record.readInt(); + ED->setScoped(Record.readInt()); + ED->setScopedUsingClassTag(Record.readInt()); + ED->setFixed(Record.readInt()); // If this is a definition subject to the ODR, and we already have a // definition, merge this one into it. - if (ED->IsCompleteDefinition && + if (ED->isCompleteDefinition() && Reader.getContext().getLangOpts().Modules && Reader.getContext().getLangOpts().CPlusPlus) { EnumDecl * = Reader.EnumDefinitions[ED->getCanonicalDecl()]; @@ -764,7 +764,7 @@ } if (OldDef) { Reader.MergedDeclContexts.insert(std::make_pair(ED, OldDef)); - ED->IsCompleteDefinition = false; + ED->setCompleteDefinition(false); Reader.mergeDefinitionVisibility(OldDef, ED); } else { OldDef = ED; @@ -1739,7 +1739,7 @@ Reader.MergedDeclContexts.insert(std::make_pair(MergeDD.Definition, DD.Definition)); Reader.PendingDefinitions.erase(MergeDD.Definition); -MergeDD.Definition->IsCompleteDefinition = false; +MergeDD.Definition->setCompleteDefinition(false); Reader.mergeDefinitionVisibility(DD.Definition, MergeDD.Definition); assert(Reader.Lookups.find(MergeDD.Definition) == Reader.Lookups.end() && "already loaded pending lookups for merged definition"); @@ -1879,7 +1879,7 @@ } // Mark this declaration as being a definition. - D->IsCompleteDefinition = true; + D->setCompleteDefinition(true); // If this is not the first declaration or is an update record, we can have // other redeclarations already. Make a note that we need to propagate the @@ -1941,7 +1941,7 @@ // compute it. if (WasDefinition) { DeclID KeyFn = ReadDeclID(); -if (KeyFn && D->IsCompleteDefinition) +if (KeyFn && D->isCompleteDefinition()) // FIXME: This is wrong for the ARM ABI, where some other module may have // made this function no longer be a key function. We need an update // record or similar for that case. @@ -3071,7 +3071,7 @@ // we load the update record. if (!DD) { DD = new (Reader.getContext()) struct CXXRecordDecl::DefinitionData(RD); - RD->IsCompleteDefinition = true; + RD->setCompleteDefinition(true); RD->DefinitionData = DD; RD->getCanonicalDecl()->DefinitionData = DD; Index: lib/AST/DeclTemplate.cpp === --- lib/AST/DeclTemplate.cpp +++ lib/AST/DeclTemplate.cpp @@ -712,7 +712,7 @@ new
[PATCH] D49729: [AST][1/4] Move the bit-fields from TagDecl, EnumDecl and RecordDecl into DeclContext
bricci added a comment. added some inline comments Comment at: include/clang/AST/DeclBase.h:1662 + enum { NumBlockDeclBits = 5 }; /// Pointer to the data structure used to lookup declarations uint64_t is used here because we will always store more than 32 bits but less than 64 bits in these SomethingDeclBitfields. Comment at: include/clang/AST/DeclBase.h:1959 - void makeDeclVisibleInContextInternal(NamedDecl *D); - StoredDeclsMap *CreateStoredDeclsMap(ASTContext ) const; This declaration has no corresponding definition and is unused. Therefore I removed it but if this is more appropriate for another patch I will do the necessary changes. Repository: rC Clang https://reviews.llvm.org/D49729 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D49729: [AST][1/4] Move the bit-fields from TagDecl, EnumDecl and RecordDecl into DeclContext
bricci created this revision. bricci added a project: clang. Herald added a subscriber: cfe-commits. DeclContext has a little less than 8 bytes free due to the alignment requirements on 64 bits archs. This set of patches moves the bit-fields from classes deriving from DeclContext into DeclContext. On 32 bits archs this increases the size of DeclContext by 4 bytes but this is balanced by an equal or larger reduction in the size of the classes deriving from it. On 64 bits archs the size of DeclContext stays the same but all the classes deriving from it shrink by 8/16 bytes. (-print-stats diff here https://reviews.llvm.org/D49728) When doing an -fsyntax-only on all of Boost this result in a 3.6% reduction in the size of all Decls and a 1% reduction in the run time due to the lower cache miss rate. This first patch introduces the anonymous union in DeclContext and all the *DeclBitfields classes holding the bit-fields, and moves the bits from TagDecl, EnumDecl and RecordDecl into DeclContext. Repository: rC Clang https://reviews.llvm.org/D49729 Files: include/clang/AST/Decl.h include/clang/AST/DeclBase.h lib/AST/Decl.cpp lib/AST/DeclBase.cpp lib/AST/DeclCXX.cpp lib/AST/DeclTemplate.cpp lib/Serialization/ASTReaderDecl.cpp lib/Serialization/ASTWriter.cpp lib/Serialization/ASTWriterDecl.cpp Index: lib/Serialization/ASTWriterDecl.cpp === --- lib/Serialization/ASTWriterDecl.cpp +++ lib/Serialization/ASTWriterDecl.cpp @@ -1273,7 +1273,7 @@ // Store (what we currently believe to be) the key function to avoid // deserializing every method so we can compute it. - if (D->IsCompleteDefinition) + if (D->isCompleteDefinition()) Record.AddDeclRef(Context.getCurrentKeyFunction(D)); Code = serialization::DECL_CXX_RECORD; Index: lib/Serialization/ASTWriter.cpp === --- lib/Serialization/ASTWriter.cpp +++ lib/Serialization/ASTWriter.cpp @@ -3960,7 +3960,8 @@ bool ASTWriter::isLookupResultExternal(StoredDeclsList , DeclContext *DC) { - return Result.hasExternalDecls() && DC->NeedToReconcileExternalVisibleStorage; + return Result.hasExternalDecls() && + DC->hasNeedToReconcileExternalVisibleStorage(); } bool ASTWriter::isLookupResultEntirelyExternal(StoredDeclsList , @@ -3975,8 +3976,8 @@ void ASTWriter::GenerateNameLookupTable(const DeclContext *ConstDC, llvm::SmallVectorImpl ) { - assert(!ConstDC->HasLazyLocalLexicalLookups && - !ConstDC->HasLazyExternalLexicalLookups && + assert(!ConstDC->hasLazyLocalLexicalLookups() && + !ConstDC->hasLazyExternalLexicalLookups() && "must call buildLookups first"); // FIXME: We need to build the lookups table, which is logically const. Index: lib/Serialization/ASTReaderDecl.cpp === --- lib/Serialization/ASTReaderDecl.cpp +++ lib/Serialization/ASTReaderDecl.cpp @@ -742,13 +742,13 @@ ED->setPromotionType(Record.readType()); ED->setNumPositiveBits(Record.readInt()); ED->setNumNegativeBits(Record.readInt()); - ED->IsScoped = Record.readInt(); - ED->IsScopedUsingClassTag = Record.readInt(); - ED->IsFixed = Record.readInt(); + ED->setScoped(Record.readInt()); + ED->setScopedUsingClassTag(Record.readInt()); + ED->setFixed(Record.readInt()); // If this is a definition subject to the ODR, and we already have a // definition, merge this one into it. - if (ED->IsCompleteDefinition && + if (ED->isCompleteDefinition() && Reader.getContext().getLangOpts().Modules && Reader.getContext().getLangOpts().CPlusPlus) { EnumDecl * = Reader.EnumDefinitions[ED->getCanonicalDecl()]; @@ -764,7 +764,7 @@ } if (OldDef) { Reader.MergedDeclContexts.insert(std::make_pair(ED, OldDef)); - ED->IsCompleteDefinition = false; + ED->setCompleteDefinition(false); Reader.mergeDefinitionVisibility(OldDef, ED); } else { OldDef = ED; @@ -1739,7 +1739,7 @@ Reader.MergedDeclContexts.insert(std::make_pair(MergeDD.Definition, DD.Definition)); Reader.PendingDefinitions.erase(MergeDD.Definition); -MergeDD.Definition->IsCompleteDefinition = false; +MergeDD.Definition->setCompleteDefinition(false); Reader.mergeDefinitionVisibility(DD.Definition, MergeDD.Definition); assert(Reader.Lookups.find(MergeDD.Definition) == Reader.Lookups.end() && "already loaded pending lookups for merged definition"); @@ -1879,7 +1879,7 @@ } // Mark this declaration as being a definition. - D->IsCompleteDefinition = true; + D->setCompleteDefinition(true); // If this is not the first declaration or is an update record, we can have // other redeclarations already. Make a note that we need to propagate the @@