C++ modules: fix a bug where loading a declaration with some name would prevent
name lookup from lazily deserializing the other declarations with the same
name, by tracking a bit to indicate whether a name in a DeclContext might have
additional external results. This also allows lazier reconciling of the lookup
table if a module import adds decls to a pre-existing DC.
However, this exposes a pre-existing bug, which causes a regression in
test/Modules/decldef.mm: if we have a reference to a declaration, and a
later-imported module adds a redeclaration, nothing causes us to load that
redeclaration when we use or emit the reference (which can manifest as a
reference to an undefined inline function, a use of an incomplete type, and so
on). decldef.mm has been extended with an additional testcase which fails with
or without this change.
llvm-svn: 190293
diff --git a/clang/lib/AST/DeclBase.cpp b/clang/lib/AST/DeclBase.cpp
index e4d4e77..be1992a 100644
--- a/clang/lib/AST/DeclBase.cpp
+++ b/clang/lib/AST/DeclBase.cpp
@@ -926,11 +926,8 @@
NeedToReconcileExternalVisibleStorage = false;
StoredDeclsMap &Map = *LookupPtr.getPointer();
- ExternalASTSource *Source = getParentASTContext().getExternalSource();
- for (StoredDeclsMap::iterator I = Map.begin(); I != Map.end(); ++I) {
- I->second.removeExternalDecls();
- Source->FindExternalVisibleDeclsByName(this, I->first);
- }
+ for (StoredDeclsMap::iterator I = Map.begin(); I != Map.end(); ++I)
+ I->second.setHasExternalDecls();
}
/// \brief Load the declarations within this lexical storage from an
@@ -983,8 +980,7 @@
if (!(Map = DC->LookupPtr.getPointer()))
Map = DC->CreateStoredDeclsMap(Context);
- // Add an entry to the map for this name, if it's not already present.
- (*Map)[Name];
+ (*Map)[Name].removeExternalDecls();
return DeclContext::lookup_result();
}
@@ -1246,16 +1242,14 @@
if (!Map)
Map = CreateStoredDeclsMap(getParentASTContext());
- // If a PCH/module has a result for this name, and we have a local
- // declaration, we will have imported the PCH/module result when adding the
- // local declaration or when reconciling the module.
+ // If we have a lookup result with no external decls, we are done.
std::pair<StoredDeclsMap::iterator, bool> R =
Map->insert(std::make_pair(Name, StoredDeclsList()));
- if (!R.second)
+ if (!R.second && !R.first->second.hasExternalDecls())
return R.first->second.getLookupResult();
ExternalASTSource *Source = getParentASTContext().getExternalSource();
- if (Source->FindExternalVisibleDeclsByName(this, Name)) {
+ if (Source->FindExternalVisibleDeclsByName(this, Name) || R.second) {
if (StoredDeclsMap *Map = LookupPtr.getPointer()) {
StoredDeclsMap::iterator I = Map->find(Name);
if (I != Map->end())
@@ -1304,7 +1298,8 @@
LookupPtr.setInt(false);
// There may now be names for which we have local decls but are
- // missing the external decls.
+ // missing the external decls. FIXME: Just set the hasExternalDecls
+ // flag on those names that have external decls.
NeedToReconcileExternalVisibleStorage = true;
Map = LookupPtr.getPointer();
@@ -1461,7 +1456,18 @@
// Insert this declaration into the map.
StoredDeclsList &DeclNameEntries = (*Map)[D->getDeclName()];
- if (DeclNameEntries.isNull()) {
+
+ if (Internal) {
+ // If this is being added as part of loading an external declaration,
+ // this may not be the only external declaration with this name.
+ // In this case, we never try to replace an existing declaration; we'll
+ // handle that when we finalize the list of declarations for this name.
+ DeclNameEntries.setHasExternalDecls();
+ DeclNameEntries.AddSubsequentDecl(D);
+ return;
+ }
+
+ else if (DeclNameEntries.isNull()) {
DeclNameEntries.setOnlyValue(D);
return;
}