[modules] When performing a lookup into a namespace, ensure that any later

redefinitions of that namespace have already been loaded. When writing out the
names in a namespace, if we see a name that is locally declared and had
imported declarations merged on top of it, export the local declaration as the
lookup result, because it will be the most recent declaration of that entity in
the redeclaration chain of an importer of the module.

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@215518 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/include/clang/AST/DeclBase.h b/include/clang/AST/DeclBase.h
index 3fb68a47c042929b7f2f2b2446127b8758b91748..551a883a8fa92fe8bcdcdd7b68fb25b578e30b35 100644 (file)
--- a/include/clang/AST/DeclBase.h
+++ b/include/clang/AST/DeclBase.h
@@ -515,9 +515,13 @@ public:
   /// indicating the declaration is used.
   void markUsed(ASTContext &C);
 
-  /// \brief Whether this declaration was referenced.
+  /// \brief Whether any declaration of this entity was referenced.
   bool isReferenced() const;
 
+  /// \brief Whether this declaration was referenced. This should not be relied
+  /// upon for anything other than debugging.
+  bool isThisDeclarationReferenced() const { return Referenced; }
+
   void setReferenced(bool R = true) { Referenced = R; }
 
   /// \brief Whether this declaration is a top-level declaration (function,

diff --git a/lib/AST/ASTDumper.cpp b/lib/AST/ASTDumper.cpp
index 5d6e18a4506f7fd1c7e11ec97df72759086325bf..d9dc8b57dfb166e1c9bbc7177699c13a5ec652fc 100644 (file)
--- a/lib/AST/ASTDumper.cpp
+++ b/lib/AST/ASTDumper.cpp
@@ -826,7 +826,7 @@ void ASTDumper::dumpDecl(const Decl *D) {
     OS << " implicit";
   if (D->isUsed())
     OS << " used";
-  else if (D->isReferenced())
+  else if (D->isThisDeclarationReferenced())
     OS << " referenced";
   if (D->isInvalidDecl())
     OS << " invalid";

diff --git a/lib/AST/DeclBase.cpp b/lib/AST/DeclBase.cpp
index 2b1506d191d126e2193e804e98bc5ffff59560fa..49d05d0d407c29633ea67cab1587f1dc0a79aca2 100644 (file)
--- a/lib/AST/DeclBase.cpp
+++ b/lib/AST/DeclBase.cpp
@@ -1296,6 +1296,11 @@ DeclContext::lookup(DeclarationName Name) {
   if (PrimaryContext != this)
     return PrimaryContext->lookup(Name);
 
+  // If this is a namespace, ensure that any later redeclarations of it have
+  // been loaded, since they may add names to the result of this lookup.
+  if (auto *ND = dyn_cast<NamespaceDecl>(this))
+    (void)ND->getMostRecentDecl();
+
   if (hasExternalVisibleStorage()) {
     if (NeedToReconcileExternalVisibleStorage)
       reconcileExternalVisibleStorage();

diff --git a/lib/Serialization/ASTReader.cpp b/lib/Serialization/ASTReader.cpp
index 1cce383f2d005d30ec369111974aaaa79aa45482..d3d1e48ccbb1d374bd3e22e47c909082f58f55c1 100644 (file)
--- a/lib/Serialization/ASTReader.cpp
+++ b/lib/Serialization/ASTReader.cpp
@@ -1898,16 +1898,20 @@ void ASTReader::removeOverriddenMacros(IdentifierInfo *II,
     SubmoduleID OwnerID = Overrides[OI];
 
     // If this macro is not yet visible, remove it from the hidden names list.
+    // It won't be there if we're in the middle of making the owner visible.
     Module *Owner = getSubmodule(OwnerID);
-    HiddenNames &Hidden = HiddenNamesMap[Owner];
-    HiddenMacrosMap::iterator HI = Hidden.HiddenMacros.find(II);
-    if (HI != Hidden.HiddenMacros.end()) {
-      // Register the macro now so we don't lose it when we re-export.
-      PP.appendMacroDirective(II, HI->second->import(PP, ImportLoc));
-
-      auto SubOverrides = HI->second->getOverriddenSubmodules();
-      Hidden.HiddenMacros.erase(HI);
-      removeOverriddenMacros(II, ImportLoc, Ambig, SubOverrides);
+    auto HiddenIt = HiddenNamesMap.find(Owner);
+    if (HiddenIt != HiddenNamesMap.end()) {
+      HiddenNames &Hidden = HiddenIt->second;
+      HiddenMacrosMap::iterator HI = Hidden.HiddenMacros.find(II);
+      if (HI != Hidden.HiddenMacros.end()) {
+        // Register the macro now so we don't lose it when we re-export.
+        PP.appendMacroDirective(II, HI->second->import(PP, ImportLoc));
+
+        auto SubOverrides = HI->second->getOverriddenSubmodules();
+        Hidden.HiddenMacros.erase(HI);
+        removeOverriddenMacros(II, ImportLoc, Ambig, SubOverrides);
+      }
     }
 
     // If this macro is already in our list of conflicts, remove it from there.
@@ -2675,7 +2679,6 @@ ASTReader::ReadASTBlock(ModuleFile &F, unsigned ClientLoadCapabilities) {
         auto *DC = cast<DeclContext>(D);
         DC->getPrimaryContext()->setHasExternalVisibleStorage(true);
         auto *&LookupTable = F.DeclContextInfos[DC].NameLookupTableData;
-        // FIXME: There should never be an existing lookup table.
         delete LookupTable;
         LookupTable = Table;
       } else
@@ -6051,12 +6054,6 @@ void ASTReader::CompleteRedeclChain(const Decl *D) {
 
   const DeclContext *DC = D->getDeclContext()->getRedeclContext();
 
-  // Recursively ensure that the decl context itself is complete
-  // (in particular, this matters if the decl context is a namespace).
-  //
-  // FIXME: This should be performed by lookup instead of here.
-  cast<Decl>(DC)->getMostRecentDecl();
-
   // If this is a named declaration, complete it by looking it up
   // within its context.
   //
@@ -8246,9 +8243,16 @@ void ASTReader::finishPendingActions() {
 }
 
 void ASTReader::diagnoseOdrViolations() {
+  if (PendingOdrMergeFailures.empty() && PendingOdrMergeChecks.empty())
+    return;
+
   // Trigger the import of the full definition of each class that had any
   // odr-merging problems, so we can produce better diagnostics for them.
-  for (auto &Merge : PendingOdrMergeFailures) {
+  // These updates may in turn find and diagnose some ODR failures, so take
+  // ownership of the set first.
+  auto OdrMergeFailures = std::move(PendingOdrMergeFailures);
+  PendingOdrMergeFailures.clear();
+  for (auto &Merge : OdrMergeFailures) {
     Merge.first->buildLookup();
     Merge.first->decls_begin();
     Merge.first->bases_begin();
@@ -8322,7 +8326,7 @@ void ASTReader::diagnoseOdrViolations() {
   }
 
   // Issue any pending ODR-failure diagnostics.
-  for (auto &Merge : PendingOdrMergeFailures) {
+  for (auto &Merge : OdrMergeFailures) {
     // If we've already pointed out a specific problem with this class, don't
     // bother issuing a general "something's different" diagnostic.
     if (!DiagnosedOdrMergeFailures.insert(Merge.first))
@@ -8361,7 +8365,6 @@ void ASTReader::diagnoseOdrViolations() {
         << Merge.first;
     }
   }
-  PendingOdrMergeFailures.clear();
 }
 
 void ASTReader::FinishedDeserializing() {

diff --git a/lib/Serialization/ASTReaderDecl.cpp b/lib/Serialization/ASTReaderDecl.cpp
index b153043082f92a95f65bc8e83e1f1818d43f15f3..46f6106c13a4049d8d06e145f1dc0d936094fee1 100644 (file)
--- a/lib/Serialization/ASTReaderDecl.cpp
+++ b/lib/Serialization/ASTReaderDecl.cpp
@@ -2619,11 +2619,11 @@ ASTReader::combineStoredMergedDecls(Decl *Canon, GlobalDeclID CanonID) {
   // Append the stored merged declarations to the merged declarations set.
   MergedDeclsMap::iterator Pos = MergedDecls.find(Canon);
   if (Pos == MergedDecls.end())
-    Pos = MergedDecls.insert(std::make_pair(Canon, 
+    Pos = MergedDecls.insert(std::make_pair(Canon,
                                             SmallVector<DeclID, 2>())).first;
   Pos->second.append(StoredPos->second.begin(), StoredPos->second.end());
   StoredMergedDecls.erase(StoredPos);
-  
+
   // Sort and uniquify the set of merged declarations.
   llvm::array_pod_sort(Pos->second.begin(), Pos->second.end());
   Pos->second.erase(std::unique(Pos->second.begin(), Pos->second.end()),

diff --git a/lib/Serialization/ASTWriter.cpp b/lib/Serialization/ASTWriter.cpp
index ccd57086c38b26a14057e92627574dc12d2a2f14..e8c3177cb86745bb15d5c3a41c59c05a748c7a22 100644 (file)
--- a/lib/Serialization/ASTWriter.cpp
+++ b/lib/Serialization/ASTWriter.cpp
@@ -3479,6 +3479,18 @@ void ASTWriter::WriteIdentifierTable(Preprocessor &PP,
 // DeclContext's Name Lookup Table Serialization
 //===----------------------------------------------------------------------===//
 
+/// Determine the declaration that should be put into the name lookup table to
+/// represent the given declaration in this module. This is usually D itself,
+/// but if D was imported and merged into a local declaration, we want the most
+/// recent local declaration instead. The chosen declaration will be the most
+/// recent declaration in any module that imports this one.
+static NamedDecl *getDeclForLocalLookup(NamedDecl *D) {
+  for (Decl *Redecl = D; Redecl; Redecl = Redecl->getPreviousDecl())
+    if (!Redecl->isFromASTFile())
+      return cast<NamedDecl>(Redecl);
+  return D;
+}
+
 namespace {
 // Trait used for the on-disk hash table used in the method pool.
 class ASTDeclContextNameLookupTrait {
@@ -3596,7 +3608,7 @@ public:
     LE.write<uint16_t>(Lookup.size());
     for (DeclContext::lookup_iterator I = Lookup.begin(), E = Lookup.end();
          I != E; ++I)
-      LE.write<uint32_t>(Writer.GetDeclRef(*I));
+      LE.write<uint32_t>(Writer.GetDeclRef(getDeclForLocalLookup(*I)));
 
     assert(Out.tell() - Start == DataLen && "Data length is wrong");
   }
@@ -3642,7 +3654,7 @@ void ASTWriter::AddUpdatedDeclContext(const DeclContext *DC) {
                             [&](DeclarationName Name,
                                 DeclContext::lookup_const_result Result) {
       for (auto *Decl : Result)
-        GetDeclRef(Decl);
+        GetDeclRef(getDeclForLocalLookup(Decl));
     });
   }
 }

diff --git a/test/Modules/Inputs/cxx-lookup/a.h b/test/Modules/Inputs/cxx-lookup/a.h
new file mode 100644 (file)
index 0000000..25f614f
--- /dev/null
+++ b/test/Modules/Inputs/cxx-lookup/a.h
@@ -0,0 +1,2 @@
+// a
+namespace llvm { class GlobalValue; }

diff --git a/test/Modules/Inputs/cxx-lookup/b.h b/test/Modules/Inputs/cxx-lookup/b.h
new file mode 100644 (file)
index 0000000..c2ede9f
--- /dev/null
+++ b/test/Modules/Inputs/cxx-lookup/b.h
@@ -0,0 +1,3 @@
+// b
+namespace llvm { class GlobalValue; }
+#include "y.h"

diff --git a/test/Modules/Inputs/cxx-lookup/c1.h b/test/Modules/Inputs/cxx-lookup/c1.h
new file mode 100644 (file)
index 0000000..dba4a4c
--- /dev/null
+++ b/test/Modules/Inputs/cxx-lookup/c1.h
@@ -0,0 +1,3 @@
+// c1
+#include "a.h"
+#include "b.h"

diff --git a/test/Modules/Inputs/cxx-lookup/c2.h b/test/Modules/Inputs/cxx-lookup/c2.h
new file mode 100644 (file)
index 0000000..463e270
--- /dev/null
+++ b/test/Modules/Inputs/cxx-lookup/c2.h
@@ -0,0 +1,2 @@
+// c2
+namespace llvm { class GlobalValue; }

diff --git a/test/Modules/Inputs/cxx-lookup/module.modulemap b/test/Modules/Inputs/cxx-lookup/module.modulemap
new file mode 100644 (file)
index 0000000..6d397af
--- /dev/null
+++ b/test/Modules/Inputs/cxx-lookup/module.modulemap
@@ -0,0 +1,8 @@
+module A { header "a.h" export * }
+module B { header "b.h" export * }
+module C {
+  module C2 { header "c2.h" export * }
+  module C1 { header "c1.h" export * }
+}
+module X { header "x.h" export * }
+module Y { header "y.h" export * }

diff --git a/test/Modules/Inputs/cxx-lookup/x.h b/test/Modules/Inputs/cxx-lookup/x.h
new file mode 100644 (file)
index 0000000..a8826e0
--- /dev/null
+++ b/test/Modules/Inputs/cxx-lookup/x.h
@@ -0,0 +1,2 @@
+template <class T> class allocator;
+struct X { virtual allocator<char> f(); };

diff --git a/test/Modules/Inputs/cxx-lookup/y.h b/test/Modules/Inputs/cxx-lookup/y.h
new file mode 100644 (file)
index 0000000..8867e8a
--- /dev/null
+++ b/test/Modules/Inputs/cxx-lookup/y.h
@@ -0,0 +1,5 @@
+#include "x.h"
+namespace llvm {
+  struct ulittle32_t;
+  extern allocator<ulittle32_t> *x;
+}

diff --git a/test/Modules/cxx-lookup.cpp b/test/Modules/cxx-lookup.cpp
new file mode 100644 (file)
index 0000000..47c879d
--- /dev/null
+++ b/test/Modules/cxx-lookup.cpp
@@ -0,0 +1,6 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -fmodules -fmodules-cache-path=%t %s -I%S/Inputs/cxx-lookup -verify
+// expected-no-diagnostics
+namespace llvm {}
+#include "c2.h"
+llvm::GlobalValue *p;

diff --git a/test/Modules/cxx-templates.cpp b/test/Modules/cxx-templates.cpp
index 1b7c045fae997dc9f35322e9c8d1576032746887..6f5d319a8e52e09c5db2efb293fae239a55ddf87 100644 (file)
--- a/test/Modules/cxx-templates.cpp
+++ b/test/Modules/cxx-templates.cpp
@@ -29,8 +29,8 @@ void g() {
   N::f<double>(1.0);
   N::f<int>();
   N::f(); // expected-error {{no matching function}}
-  // expected-note@Inputs/cxx-templates-a.h:6 {{couldn't infer template argument}}
-  // expected-note@Inputs/cxx-templates-a.h:7 {{requires 1 argument}}
+  // expected-note@Inputs/cxx-templates-b.h:6 {{couldn't infer template argument}}
+  // expected-note@Inputs/cxx-templates-b.h:7 {{requires single argument}}
 
   template_param_kinds_1<0>(); // ok, from cxx-templates-a.h
   template_param_kinds_1<int>(); // ok, from cxx-templates-b.h