r213043 - [modules] Rearrange and unify the way we determine if we need to pull in
Richard Smith
richard-llvm at metafoo.co.uk
Mon Jul 14 20:37:06 PDT 2014
Author: rsmith
Date: Mon Jul 14 22:37:06 2014
New Revision: 213043
URL: http://llvm.org/viewvc/llvm-project?rev=213043&view=rev
Log:
[modules] Rearrange and unify the way we determine if we need to pull in
redeclaration chains when pulling in a declaration. We need the redecl chain
unless we know some other declaration will trigger it to be pulled in; that
happens if our originally-canonical declaration had all the knowledge that
we have (and isn't us).
Modified:
cfe/trunk/lib/Serialization/ASTReaderDecl.cpp
Modified: cfe/trunk/lib/Serialization/ASTReaderDecl.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReaderDecl.cpp?rev=213043&r1=213042&r2=213043&view=diff
==============================================================================
--- cfe/trunk/lib/Serialization/ASTReaderDecl.cpp (original)
+++ cfe/trunk/lib/Serialization/ASTReaderDecl.cpp Mon Jul 14 22:37:06 2014
@@ -110,8 +110,8 @@ namespace clang {
/// chain and to introduce it into the list of pending redeclaration chains
/// on destruction.
///
- /// The caller can choose not to introduce this ID into the redeclaration
- /// chain by calling \c suppress().
+ /// The caller can choose not to introduce this ID into the list of pending
+ /// redeclaration chains by calling \c suppress().
class RedeclarableResult {
ASTReader &Reader;
GlobalDeclID FirstID;
@@ -1134,8 +1134,6 @@ void ASTDeclReader::VisitNamespaceDecl(N
D->setInline(Record[Idx++]);
D->LocStart = ReadSourceLocation(Record, Idx);
D->RBraceLoc = ReadSourceLocation(Record, Idx);
- // FIXME: At the point of this call, D->getCanonicalDecl() returns 0.
- mergeRedeclarable(D, Redecl);
if (Redecl.getFirstID() == ThisDeclID) {
// Each module has its own anonymous namespace, which is disjoint from
@@ -1149,6 +1147,8 @@ void ASTDeclReader::VisitNamespaceDecl(N
// been deserialized.
D->AnonOrFirstNamespaceAndInline.setPointer(D->getFirstDecl());
}
+
+ mergeRedeclarable(D, Redecl);
}
void ASTDeclReader::VisitNamespaceAliasDecl(NamespaceAliasDecl *D) {
@@ -2009,14 +2009,25 @@ ASTDeclReader::VisitRedeclarable(Redecla
/// \brief Attempts to merge the given declaration (D) with another declaration
/// of the same entity.
template<typename T>
-void ASTDeclReader::mergeRedeclarable(Redeclarable<T> *D,
+void ASTDeclReader::mergeRedeclarable(Redeclarable<T> *DBase,
RedeclarableResult &Redecl,
DeclID TemplatePatternID) {
+ T *D = static_cast<T*>(DBase);
+ T *DCanon = D->getCanonicalDecl();
+ if (D != DCanon &&
+ (!Reader.getContext().getLangOpts().Modules ||
+ Reader.getOwningModuleFile(DCanon) == Reader.getOwningModuleFile(D))) {
+ // All redeclarations between this declaration and its originally-canonical
+ // declaration get pulled in when we load DCanon; we don't need to
+ // perform any more merging now.
+ Redecl.suppress();
+ }
+
// If modules are not available, there is no reason to perform this merge.
if (!Reader.getContext().getLangOpts().Modules)
return;
- if (FindExistingResult ExistingRes = findExisting(static_cast<T*>(D)))
+ if (FindExistingResult ExistingRes = findExisting(D))
if (T *Existing = ExistingRes)
mergeRedeclarable(D, Existing, Redecl, TemplatePatternID);
}
@@ -2036,7 +2047,8 @@ void ASTDeclReader::mergeTemplatePattern
DeclID DsID) {
auto *DPattern = D->getTemplatedDecl();
auto *ExistingPattern = Existing->getTemplatedDecl();
- RedeclarableResult Result(Reader, DsID, DPattern->getKind());
+ RedeclarableResult Result(Reader, DPattern->getCanonicalDecl()->getGlobalID(),
+ DPattern->getKind());
if (auto *DClass = dyn_cast<CXXRecordDecl>(DPattern)) {
// Merge with any existing definition.
// FIXME: This is duplicated in several places. Refactor.
@@ -2076,8 +2088,10 @@ void ASTDeclReader::mergeRedeclarable(Re
T *ExistingCanon = Existing->getCanonicalDecl();
T *DCanon = D->getCanonicalDecl();
if (ExistingCanon != DCanon) {
+ assert(DCanon->getGlobalID() == Redecl.getFirstID());
+
// Have our redeclaration link point back at the canonical declaration
- // of the existing declaration, so that this declaration has the
+ // of the existing declaration, so that this declaration has the
// appropriate canonical declaration.
D->RedeclLink = Redeclarable<T>::PreviousDeclLink(ExistingCanon);
@@ -2092,18 +2106,6 @@ void ASTDeclReader::mergeRedeclarable(Re
DTemplate, assert_cast<RedeclarableTemplateDecl*>(ExistingCanon),
TemplatePatternID);
- // Don't introduce DCanon into the set of pending declaration chains.
- Redecl.suppress();
-
- // Introduce ExistingCanon into the set of pending declaration chains,
- // if in fact it came from a module file.
- if (ExistingCanon->isFromASTFile()) {
- GlobalDeclID ExistingCanonID = ExistingCanon->getGlobalID();
- assert(ExistingCanonID && "Unrecorded canonical declaration ID?");
- if (Reader.PendingDeclChainsKnown.insert(ExistingCanonID))
- Reader.PendingDeclChains.push_back(ExistingCanonID);
- }
-
// If this declaration was the canonical declaration, make a note of
// that. We accept the linear algorithm here because the number of
// unique canonical declarations of an entity should always be tiny.
@@ -2112,14 +2114,6 @@ void ASTDeclReader::mergeRedeclarable(Re
if (std::find(Merged.begin(), Merged.end(), Redecl.getFirstID())
== Merged.end())
Merged.push_back(Redecl.getFirstID());
-
- // If ExistingCanon did not come from a module file, introduce the
- // first declaration that *does* come from a module file to the
- // set of pending declaration chains, so that we merge this
- // declaration.
- if (!ExistingCanon->isFromASTFile() &&
- Reader.PendingDeclChainsKnown.insert(Redecl.getFirstID()))
- Reader.PendingDeclChains.push_back(Merged[0]);
}
}
}
@@ -2422,6 +2416,8 @@ ASTDeclReader::FindExistingResult ASTDec
if (!Name) {
// Don't bother trying to find unnamed declarations.
FindExistingResult Result(Reader, D, /*Existing=*/nullptr);
+ // FIXME: We may still need to pull in the redeclaration chain; there can
+ // be redeclarations via 'decltype'.
Result.suppress();
return Result;
}
@@ -3001,6 +2997,8 @@ namespace {
// Visit each of the declarations.
for (unsigned I = 0, N = SearchDecls.size(); I != N; ++I)
searchForID(M, SearchDecls[I]);
+ // FIXME: If none of the SearchDecls had local IDs in this module, can
+ // we avoid searching any ancestor module files?
return false;
}
@@ -3308,6 +3306,10 @@ void ASTDeclReader::UpdateDecl(Decl *D,
// FIXME: This doesn't send the right notifications if there are
// ASTMutationListeners other than an ASTWriter.
+ // FIXME: We can't both pull in declarations (and thus create new pending
+ // redeclaration chains) *and* walk redeclaration chains in this function.
+ // We should defer the updates that require walking redecl chains.
+
// Maintain AST consistency: any later redeclarations are used too.
for (auto *Redecl = D->getMostRecentDecl(); /**/;
Redecl = Redecl->getPreviousDecl()) {
More information about the cfe-commits
mailing list