r314955 - Remove PendingBody mechanism for function and ObjC method deserialization.
Richard Smith via cfe-commits
cfe-commits at lists.llvm.org
Wed Oct 4 17:43:38 PDT 2017
Author: rsmith
Date: Wed Oct 4 17:43:38 2017
New Revision: 314955
URL: http://llvm.org/viewvc/llvm-project?rev=314955&view=rev
Log:
Remove PendingBody mechanism for function and ObjC method deserialization.
In its place, track on the canonical function declaration whether there is a
declaration with a body (and if so, which one). This brings function definition
handling in line with what we do in all other contexts, and is necessary to
allow us to merge declarations within multiple definitions of the same function
(eg, PR33924).
No functionality change intended.
Modified:
cfe/trunk/include/clang/Serialization/ASTReader.h
cfe/trunk/lib/Serialization/ASTReader.cpp
cfe/trunk/lib/Serialization/ASTReaderDecl.cpp
Modified: cfe/trunk/include/clang/Serialization/ASTReader.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Serialization/ASTReader.h?rev=314955&r1=314954&r2=314955&view=diff
==============================================================================
--- cfe/trunk/include/clang/Serialization/ASTReader.h (original)
+++ cfe/trunk/include/clang/Serialization/ASTReader.h Wed Oct 4 17:43:38 2017
@@ -559,13 +559,9 @@ private:
/// declarations that have not yet been linked to their definitions.
llvm::SmallPtrSet<Decl *, 4> PendingDefinitions;
- typedef llvm::MapVector<Decl *, uint64_t,
- llvm::SmallDenseMap<Decl *, unsigned, 4>,
- SmallVector<std::pair<Decl *, uint64_t>, 4> >
- PendingBodiesMap;
-
- /// \brief Functions or methods that have bodies that will be attached.
- PendingBodiesMap PendingBodies;
+ /// \brief Functions or methods that are known to already have a definition
+ /// (that might not yet be merged into the redeclaration chain).
+ llvm::SmallDenseMap<FunctionDecl *, FunctionDecl*, 4> FunctionDefinitions;
/// \brief Definitions for which we have added merged definitions but not yet
/// performed deduplication.
@@ -991,25 +987,13 @@ private:
/// the last time we loaded information about this identifier.
llvm::DenseMap<IdentifierInfo *, unsigned> IdentifierGeneration;
- class InterestingDecl {
- Decl *D;
- bool DeclHasPendingBody;
-
- public:
- InterestingDecl(Decl *D, bool HasBody)
- : D(D), DeclHasPendingBody(HasBody) {}
- Decl *getDecl() { return D; }
- /// Whether the declaration has a pending body.
- bool hasPendingBody() { return DeclHasPendingBody; }
- };
-
/// \brief Contains declarations and definitions that could be
/// "interesting" to the ASTConsumer, when we get that AST consumer.
///
/// "Interesting" declarations are those that have data that may
/// need to be emitted, such as inline function definitions or
/// Objective-C protocols.
- std::deque<InterestingDecl> PotentiallyInterestingDecls;
+ std::deque<Decl*> PotentiallyInterestingDecls;
/// \brief The list of redeclaration chains that still need to be
/// reconstructed, and the local offset to the corresponding list
Modified: cfe/trunk/lib/Serialization/ASTReader.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReader.cpp?rev=314955&r1=314954&r2=314955&view=diff
==============================================================================
--- cfe/trunk/lib/Serialization/ASTReader.cpp (original)
+++ cfe/trunk/lib/Serialization/ASTReader.cpp Wed Oct 4 17:43:38 2017
@@ -9168,30 +9168,6 @@ void ASTReader::finishPendingActions() {
}
PendingDefinitions.clear();
- // Load the bodies of any functions or methods we've encountered. We do
- // this now (delayed) so that we can be sure that the declaration chains
- // have been fully wired up (hasBody relies on this).
- // FIXME: We shouldn't require complete redeclaration chains here.
- for (PendingBodiesMap::iterator PB = PendingBodies.begin(),
- PBEnd = PendingBodies.end();
- PB != PBEnd; ++PB) {
- if (FunctionDecl *FD = dyn_cast<FunctionDecl>(PB->first)) {
- // FIXME: Check for =delete/=default?
- // FIXME: Complain about ODR violations here?
- const FunctionDecl *Defn = nullptr;
- if (!getContext().getLangOpts().Modules || !FD->hasBody(Defn)) {
- FD->setLazyBody(PB->second);
- } else
- mergeDefinitionVisibility(const_cast<FunctionDecl*>(Defn), FD);
- continue;
- }
-
- ObjCMethodDecl *MD = cast<ObjCMethodDecl>(PB->first);
- if (!getContext().getLangOpts().Modules || !MD->hasBody())
- MD->setLazyBody(PB->second);
- }
- PendingBodies.clear();
-
// Do some cleanup.
for (auto *ND : PendingMergedDefinitionsToDeduplicate)
getContext().deduplicateMergedDefinitonsFor(ND);
Modified: cfe/trunk/lib/Serialization/ASTReaderDecl.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReaderDecl.cpp?rev=314955&r1=314954&r2=314955&view=diff
==============================================================================
--- cfe/trunk/lib/Serialization/ASTReaderDecl.cpp (original)
+++ cfe/trunk/lib/Serialization/ASTReaderDecl.cpp Wed Oct 4 17:43:38 2017
@@ -45,8 +45,6 @@ namespace clang {
GlobalDeclID NamedDeclForTagDecl;
IdentifierInfo *TypedefNameForLinkage;
- bool HasPendingBody;
-
///\brief A flag to carry the information for a decl from the entity is
/// used. We use it to delay the marking of the canonical decl as used until
/// the entire declaration is deserialized and merged.
@@ -216,8 +214,7 @@ namespace clang {
: Reader(Reader), Record(Record), Loc(Loc),
ThisDeclID(thisDeclID), ThisDeclLoc(ThisDeclLoc),
TypeIDForTypeDecl(0), NamedDeclForTagDecl(0),
- TypedefNameForLinkage(nullptr), HasPendingBody(false),
- IsDeclMarkedUsed(false) {}
+ TypedefNameForLinkage(nullptr), IsDeclMarkedUsed(false) {}
template <typename T> static
void AddLazySpecializations(T *D,
@@ -265,9 +262,7 @@ namespace clang {
static void markIncompleteDeclChainImpl(Redeclarable<DeclT> *D);
static void markIncompleteDeclChainImpl(...);
- /// \brief Determine whether this declaration has a pending body.
- bool hasPendingBody() const { return HasPendingBody; }
-
+ FunctionDecl *TryRegisterAsFunctionDefinition(FunctionDecl *FD);
void ReadFunctionDefinition(FunctionDecl *FD);
void Visit(Decl *D);
@@ -420,7 +415,8 @@ public:
MergedRedeclIterator &operator++() {
if (Current->isFirstDecl()) {
Canonical = Current;
- Current = Current->getMostRecentDecl();
+ Decl *MostRecent = ASTDeclReader::getMostRecentDecl(Canonical);
+ Current = MostRecent ? cast<DeclT>(MostRecent) : nullptr;
} else
Current = Current->getPreviousDecl();
@@ -451,17 +447,69 @@ uint64_t ASTDeclReader::GetCurrentCursor
return Loc.F->DeclsCursor.GetCurrentBitNo() + Loc.F->GlobalBitOffset;
}
+FunctionDecl *ASTDeclReader::TryRegisterAsFunctionDefinition(FunctionDecl *D) {
+ FunctionDecl *&Definition = Reader.FunctionDefinitions[D->getCanonicalDecl()];
+
+ if (!Definition) {
+ // No imported definition, but we might have a local definition.
+ for (auto *Redecl : merged_redecls(D)) {
+ // FIXME: If an existing declaration is a definition with no body
+ // (via =delete etc), we shouldn't permit another definition here.
+ if (Redecl->Body.isValid()) {
+ Definition = Redecl;
+ break;
+ }
+ }
+ }
+
+ if (Definition) {
+ // We might have multiple update records adding definitions to the same
+ // declaration.
+ if (Definition != D) {
+ // Already have a different definition, merge this one into it.
+ Reader.mergeDefinitionVisibility(Definition, D);
+ }
+ return Definition;
+ }
+
+ Definition = D;
+ return nullptr;
+}
+
void ASTDeclReader::ReadFunctionDefinition(FunctionDecl *FD) {
- if (Record.readInt())
+ // Is this definition scheduled for modular codegen? (That is, emitted to a
+ // separate object file for the module itself, rather than with module users.)
+ bool ModularCodegen = Record.readInt();
+
+ // Don't load this definition if we already have one.
+ if (auto *Definition = TryRegisterAsFunctionDefinition(FD)) {
+ if (ModularCodegen) {
+ // Request a strong definition be emitted if any merged definition does.
+ // FIXME: Do we need to ensure that Definition is handed to the AST
+ // consumer in this case?
+ Reader.DefinitionSource[Definition] |=
+ Loc.F->Kind == ModuleKind::MK_MainFile;
+ }
+ // Skip the ctor-initializers, if any.
+ if (isa<CXXConstructorDecl>(FD))
+ if (Record.readInt())
+ ReadGlobalOffset();
+ // FIXME: Optionally register the duplicate definitions somewhere so we can
+ // check for ODR violations.
+ return;
+ }
+
+ if (ModularCodegen)
Reader.DefinitionSource[FD] = Loc.F->Kind == ModuleKind::MK_MainFile;
+
if (auto *CD = dyn_cast<CXXConstructorDecl>(FD)) {
CD->NumCtorInitializers = Record.readInt();
if (CD->NumCtorInitializers)
CD->CtorInitializers = ReadGlobalOffset();
}
+
// Store the offset of the body so we can lazily load it later.
- Reader.PendingBodies[FD] = GetCurrentCursorOffset();
- HasPendingBody = true;
+ FD->setLazyBody(GetCurrentCursorOffset());
}
void ASTDeclReader::Visit(Decl *D) {
@@ -913,10 +961,10 @@ void ASTDeclReader::VisitFunctionDecl(Fu
void ASTDeclReader::VisitObjCMethodDecl(ObjCMethodDecl *MD) {
VisitNamedDecl(MD);
if (Record.readInt()) {
- // Load the body on-demand. Most clients won't care, because method
- // definitions rarely show up in headers.
- Reader.PendingBodies[MD] = GetCurrentCursorOffset();
- HasPendingBody = true;
+ // Load the body on-demand (if we don't already have a definition). Most
+ // clients won't care, because method definitions rarely show up in
+ // headers.
+ MD->setLazyBody(GetCurrentCursorOffset());
MD->setSelfDecl(ReadDeclAs<ImplicitParamDecl>());
MD->setCmdDecl(ReadDeclAs<ImplicitParamDecl>());
}
@@ -2567,7 +2615,7 @@ inline void ASTReader::LoadedDecl(unsign
/// This routine should return true for anything that might affect
/// code generation, e.g., inline function definitions, Objective-C
/// declarations with metadata, etc.
-static bool isConsumerInterestedIn(ASTContext &Ctx, Decl *D, bool HasBody) {
+static bool isConsumerInterestedIn(ASTContext &Ctx, Decl *D) {
// An ObjCMethodDecl is never considered as "interesting" because its
// implementation container always is.
@@ -2593,7 +2641,7 @@ static bool isConsumerInterestedIn(ASTCo
return Var->isFileVarDecl() &&
Var->isThisDeclarationADefinition() == VarDecl::Definition;
if (FunctionDecl *Func = dyn_cast<FunctionDecl>(D))
- return Func->doesThisDeclarationHaveABody() || HasBody;
+ return Func->doesThisDeclarationHaveABody();
if (auto *ES = D->getASTContext().getExternalSource())
if (ES->hasExternalDefinitions(D) == ExternalASTSource::EK_Never)
@@ -3658,8 +3706,7 @@ Decl *ASTReader::ReadDeclRecord(DeclID I
// AST consumer might need to know about, queue it.
// We don't pass it to the consumer immediately because we may be in recursive
// loading, and some declarations may still be initializing.
- PotentiallyInterestingDecls.push_back(
- InterestingDecl(D, Reader.hasPendingBody()));
+ PotentiallyInterestingDecls.push_back(D);
return D;
}
@@ -3682,10 +3729,10 @@ void ASTReader::PassInterestingDeclsToCo
EagerlyDeserializedDecls.clear();
while (!PotentiallyInterestingDecls.empty()) {
- InterestingDecl D = PotentiallyInterestingDecls.front();
+ Decl *D = PotentiallyInterestingDecls.front();
PotentiallyInterestingDecls.pop_front();
- if (isConsumerInterestedIn(getContext(), D.getDecl(), D.hasPendingBody()))
- PassInterestingDeclToConsumer(D.getDecl());
+ if (isConsumerInterestedIn(getContext(), D))
+ PassInterestingDeclToConsumer(D);
}
}
@@ -3709,7 +3756,7 @@ void ASTReader::loadDeclUpdateRecords(Pe
// to isConsumerInterestedIn because it is unsafe to call in the
// current ASTReader state.
bool WasInteresting =
- Record.JustLoaded || isConsumerInterestedIn(getContext(), D, false);
+ Record.JustLoaded || isConsumerInterestedIn(getContext(), D);
for (auto &FileAndOffset : UpdateOffsets) {
ModuleFile *F = FileAndOffset.first;
uint64_t Offset = FileAndOffset.second;
@@ -3728,10 +3775,8 @@ void ASTReader::loadDeclUpdateRecords(Pe
// We might have made this declaration interesting. If so, remember that
// we need to hand it off to the consumer.
- if (!WasInteresting &&
- isConsumerInterestedIn(getContext(), D, Reader.hasPendingBody())) {
- PotentiallyInterestingDecls.push_back(
- InterestingDecl(D, Reader.hasPendingBody()));
+ if (!WasInteresting && isConsumerInterestedIn(getContext(), D)) {
+ PotentiallyInterestingDecls.push_back(D);
WasInteresting = true;
}
}
@@ -4029,12 +4074,6 @@ void ASTDeclReader::UpdateDecl(Decl *D,
case UPD_CXX_ADDED_FUNCTION_DEFINITION: {
FunctionDecl *FD = cast<FunctionDecl>(D);
- if (Reader.PendingBodies[FD]) {
- // FIXME: Maybe check for ODR violations.
- // It's safe to stop now because this update record is always last.
- return;
- }
-
if (Record.readInt()) {
// Maintain AST consistency: any later redeclarations of this function
// are inline if this one is. (We might have merged another declaration
More information about the cfe-commits
mailing list