r315439 - Revert r314955: "Remove PendingBody mechanism for function and ObjC method deserialization."
Daniel Jasper via cfe-commits
cfe-commits at lists.llvm.org
Wed Oct 11 00:47:54 PDT 2017
Author: djasper
Date: Wed Oct 11 00:47:54 2017
New Revision: 315439
URL: http://llvm.org/viewvc/llvm-project?rev=315439&view=rev
Log:
Revert r314955: "Remove PendingBody mechanism for function and ObjC method deserialization."
This is breaking a build of https://github.com/abseil/abseil-cpp and so
likely not really NFC. Also reverted subsequent r314956/7.
I'll forward reproduction instructions to Richard.
Removed:
cfe/trunk/test/Modules/merge-lambdas.cpp
Modified:
cfe/trunk/include/clang/Serialization/ASTReader.h
cfe/trunk/lib/Serialization/ASTCommon.cpp
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=315439&r1=315438&r2=315439&view=diff
==============================================================================
--- cfe/trunk/include/clang/Serialization/ASTReader.h (original)
+++ cfe/trunk/include/clang/Serialization/ASTReader.h Wed Oct 11 00:47:54 2017
@@ -559,9 +559,13 @@ private:
/// declarations that have not yet been linked to their definitions.
llvm::SmallPtrSet<Decl *, 4> PendingDefinitions;
- /// \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;
+ 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 Definitions for which we have added merged definitions but not yet
/// performed deduplication.
@@ -987,13 +991,25 @@ 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<Decl*> PotentiallyInterestingDecls;
+ std::deque<InterestingDecl> 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/ASTCommon.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTCommon.cpp?rev=315439&r1=315438&r2=315439&view=diff
==============================================================================
--- cfe/trunk/lib/Serialization/ASTCommon.cpp (original)
+++ cfe/trunk/lib/Serialization/ASTCommon.cpp Wed Oct 11 00:47:54 2017
@@ -344,8 +344,8 @@ bool serialization::needsAnonymousDeclar
return true;
}
- // Otherwise, we only care about anonymous class members / block-scope decls.
- if (D->getDeclName() || D->getLexicalDeclContext()->isFileContext())
+ // Otherwise, we only care about anonymous class members.
+ if (D->getDeclName() || !isa<CXXRecordDecl>(D->getLexicalDeclContext()))
return false;
return isa<TagDecl>(D) || isa<FieldDecl>(D);
}
Modified: cfe/trunk/lib/Serialization/ASTReader.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReader.cpp?rev=315439&r1=315438&r2=315439&view=diff
==============================================================================
--- cfe/trunk/lib/Serialization/ASTReader.cpp (original)
+++ cfe/trunk/lib/Serialization/ASTReader.cpp Wed Oct 11 00:47:54 2017
@@ -9169,6 +9169,30 @@ 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=315439&r1=315438&r2=315439&view=diff
==============================================================================
--- cfe/trunk/lib/Serialization/ASTReaderDecl.cpp (original)
+++ cfe/trunk/lib/Serialization/ASTReaderDecl.cpp Wed Oct 11 00:47:54 2017
@@ -45,6 +45,8 @@ 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.
@@ -214,7 +216,8 @@ namespace clang {
: Reader(Reader), Record(Record), Loc(Loc),
ThisDeclID(thisDeclID), ThisDeclLoc(ThisDeclLoc),
TypeIDForTypeDecl(0), NamedDeclForTagDecl(0),
- TypedefNameForLinkage(nullptr), IsDeclMarkedUsed(false) {}
+ TypedefNameForLinkage(nullptr), HasPendingBody(false),
+ IsDeclMarkedUsed(false) {}
template <typename T> static
void AddLazySpecializations(T *D,
@@ -262,7 +265,9 @@ namespace clang {
static void markIncompleteDeclChainImpl(Redeclarable<DeclT> *D);
static void markIncompleteDeclChainImpl(...);
- FunctionDecl *TryRegisterAsFunctionDefinition(FunctionDecl *FD);
+ /// \brief Determine whether this declaration has a pending body.
+ bool hasPendingBody() const { return HasPendingBody; }
+
void ReadFunctionDefinition(FunctionDecl *FD);
void Visit(Decl *D);
@@ -415,8 +420,7 @@ public:
MergedRedeclIterator &operator++() {
if (Current->isFirstDecl()) {
Canonical = Current;
- Decl *MostRecent = ASTDeclReader::getMostRecentDecl(Canonical);
- Current = MostRecent ? cast<DeclT>(MostRecent) : nullptr;
+ Current = Current->getMostRecentDecl();
} else
Current = Current->getPreviousDecl();
@@ -447,70 +451,17 @@ 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.MergedDeclContexts.insert(std::make_pair(D, Definition));
- Reader.mergeDefinitionVisibility(Definition, D);
- }
- return Definition;
- }
-
- Definition = D;
- return nullptr;
-}
-
void ASTDeclReader::ReadFunctionDefinition(FunctionDecl *FD) {
- // 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)
+ if (Record.readInt())
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.
- FD->setLazyBody(GetCurrentCursorOffset());
+ Reader.PendingBodies[FD] = GetCurrentCursorOffset();
+ HasPendingBody = true;
}
void ASTDeclReader::Visit(Decl *D) {
@@ -962,10 +913,10 @@ void ASTDeclReader::VisitFunctionDecl(Fu
void ASTDeclReader::VisitObjCMethodDecl(ObjCMethodDecl *MD) {
VisitNamedDecl(MD);
if (Record.readInt()) {
- // 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());
+ // Load the body on-demand. Most clients won't care, because method
+ // definitions rarely show up in headers.
+ Reader.PendingBodies[MD] = GetCurrentCursorOffset();
+ HasPendingBody = true;
MD->setSelfDecl(ReadDeclAs<ImplicitParamDecl>());
MD->setCmdDecl(ReadDeclAs<ImplicitParamDecl>());
}
@@ -2616,7 +2567,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) {
+static bool isConsumerInterestedIn(ASTContext &Ctx, Decl *D, bool HasBody) {
// An ObjCMethodDecl is never considered as "interesting" because its
// implementation container always is.
@@ -2642,7 +2593,7 @@ static bool isConsumerInterestedIn(ASTCo
return Var->isFileVarDecl() &&
Var->isThisDeclarationADefinition() == VarDecl::Definition;
if (FunctionDecl *Func = dyn_cast<FunctionDecl>(D))
- return Func->doesThisDeclarationHaveABody();
+ return Func->doesThisDeclarationHaveABody() || HasBody;
if (auto *ES = D->getASTContext().getExternalSource())
if (ES->hasExternalDefinitions(D) == ExternalASTSource::EK_Never)
@@ -3707,7 +3658,8 @@ 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(D);
+ PotentiallyInterestingDecls.push_back(
+ InterestingDecl(D, Reader.hasPendingBody()));
return D;
}
@@ -3730,10 +3682,10 @@ void ASTReader::PassInterestingDeclsToCo
EagerlyDeserializedDecls.clear();
while (!PotentiallyInterestingDecls.empty()) {
- Decl *D = PotentiallyInterestingDecls.front();
+ InterestingDecl D = PotentiallyInterestingDecls.front();
PotentiallyInterestingDecls.pop_front();
- if (isConsumerInterestedIn(getContext(), D))
- PassInterestingDeclToConsumer(D);
+ if (isConsumerInterestedIn(getContext(), D.getDecl(), D.hasPendingBody()))
+ PassInterestingDeclToConsumer(D.getDecl());
}
}
@@ -3757,7 +3709,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);
+ Record.JustLoaded || isConsumerInterestedIn(getContext(), D, false);
for (auto &FileAndOffset : UpdateOffsets) {
ModuleFile *F = FileAndOffset.first;
uint64_t Offset = FileAndOffset.second;
@@ -3776,8 +3728,10 @@ 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)) {
- PotentiallyInterestingDecls.push_back(D);
+ if (!WasInteresting &&
+ isConsumerInterestedIn(getContext(), D, Reader.hasPendingBody())) {
+ PotentiallyInterestingDecls.push_back(
+ InterestingDecl(D, Reader.hasPendingBody()));
WasInteresting = true;
}
}
@@ -4075,6 +4029,12 @@ 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
Removed: cfe/trunk/test/Modules/merge-lambdas.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/merge-lambdas.cpp?rev=315438&view=auto
==============================================================================
--- cfe/trunk/test/Modules/merge-lambdas.cpp (original)
+++ cfe/trunk/test/Modules/merge-lambdas.cpp (removed)
@@ -1,43 +0,0 @@
-// RUN: %clang_cc1 -std=c++11 -emit-llvm-only -fmodules %s
-
-// PR33924: ensure that we merge together local lambas in multiple definitions
-// of the same function.
-
-#pragma clang module build format
-module format {}
-#pragma clang module contents
-#pragma clang module begin format
-struct A { template<typename T> void doFormat(T &&out) {} };
-template<typename T> void format(T t) { A().doFormat([]{}); }
-#pragma clang module end
-#pragma clang module endbuild
-
-#pragma clang module build foo1
-module foo1 { export * }
-#pragma clang module contents
-#pragma clang module begin foo1
-#pragma clang module import format
-inline void foo1() {
- format(0);
-}
-#pragma clang module end
-#pragma clang module endbuild
-
-#pragma clang module build foo2
-module foo2 { export * }
-#pragma clang module contents
-#pragma clang module begin foo2
-#pragma clang module import format
-inline void foo2() {
- format(0);
-}
-#pragma clang module end
-#pragma clang module endbuild
-
-#pragma clang module import foo1
-#pragma clang module import foo2
-
-int main() {
- foo1();
- foo2();
-}
More information about the cfe-commits
mailing list