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

Richard Smith richard-llvm at metafoo.co.uk
Tue Aug 12 18:23:34 PDT 2014


Author: rsmith
Date: Tue Aug 12 20:23:33 2014
New Revision: 215518

URL: http://llvm.org/viewvc/llvm-project?rev=215518&view=rev
Log:
[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.

Added:
    cfe/trunk/test/Modules/Inputs/cxx-lookup/
    cfe/trunk/test/Modules/Inputs/cxx-lookup/a.h
    cfe/trunk/test/Modules/Inputs/cxx-lookup/b.h
    cfe/trunk/test/Modules/Inputs/cxx-lookup/c1.h
    cfe/trunk/test/Modules/Inputs/cxx-lookup/c2.h
    cfe/trunk/test/Modules/Inputs/cxx-lookup/module.modulemap
    cfe/trunk/test/Modules/Inputs/cxx-lookup/x.h
    cfe/trunk/test/Modules/Inputs/cxx-lookup/y.h
    cfe/trunk/test/Modules/cxx-lookup.cpp
Modified:
    cfe/trunk/include/clang/AST/DeclBase.h
    cfe/trunk/lib/AST/ASTDumper.cpp
    cfe/trunk/lib/AST/DeclBase.cpp
    cfe/trunk/lib/Serialization/ASTReader.cpp
    cfe/trunk/lib/Serialization/ASTReaderDecl.cpp
    cfe/trunk/lib/Serialization/ASTWriter.cpp
    cfe/trunk/test/Modules/cxx-templates.cpp

Modified: cfe/trunk/include/clang/AST/DeclBase.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclBase.h?rev=215518&r1=215517&r2=215518&view=diff
==============================================================================
--- cfe/trunk/include/clang/AST/DeclBase.h (original)
+++ cfe/trunk/include/clang/AST/DeclBase.h Tue Aug 12 20:23:33 2014
@@ -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,

Modified: cfe/trunk/lib/AST/ASTDumper.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTDumper.cpp?rev=215518&r1=215517&r2=215518&view=diff
==============================================================================
--- cfe/trunk/lib/AST/ASTDumper.cpp (original)
+++ cfe/trunk/lib/AST/ASTDumper.cpp Tue Aug 12 20:23:33 2014
@@ -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";

Modified: cfe/trunk/lib/AST/DeclBase.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/DeclBase.cpp?rev=215518&r1=215517&r2=215518&view=diff
==============================================================================
--- cfe/trunk/lib/AST/DeclBase.cpp (original)
+++ cfe/trunk/lib/AST/DeclBase.cpp Tue Aug 12 20:23:33 2014
@@ -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();

Modified: cfe/trunk/lib/Serialization/ASTReader.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReader.cpp?rev=215518&r1=215517&r2=215518&view=diff
==============================================================================
--- cfe/trunk/lib/Serialization/ASTReader.cpp (original)
+++ cfe/trunk/lib/Serialization/ASTReader.cpp Tue Aug 12 20:23:33 2014
@@ -1898,16 +1898,20 @@ void ASTReader::removeOverriddenMacros(I
     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, u
         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(cons
 
   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() {

Modified: cfe/trunk/lib/Serialization/ASTReaderDecl.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReaderDecl.cpp?rev=215518&r1=215517&r2=215518&view=diff
==============================================================================
--- cfe/trunk/lib/Serialization/ASTReaderDecl.cpp (original)
+++ cfe/trunk/lib/Serialization/ASTReaderDecl.cpp Tue Aug 12 20:23:33 2014
@@ -2619,11 +2619,11 @@ ASTReader::combineStoredMergedDecls(Decl
   // 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()),

Modified: cfe/trunk/lib/Serialization/ASTWriter.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTWriter.cpp?rev=215518&r1=215517&r2=215518&view=diff
==============================================================================
--- cfe/trunk/lib/Serialization/ASTWriter.cpp (original)
+++ cfe/trunk/lib/Serialization/ASTWriter.cpp Tue Aug 12 20:23:33 2014
@@ -3479,6 +3479,18 @@ void ASTWriter::WriteIdentifierTable(Pre
 // 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(co
                             [&](DeclarationName Name,
                                 DeclContext::lookup_const_result Result) {
       for (auto *Decl : Result)
-        GetDeclRef(Decl);
+        GetDeclRef(getDeclForLocalLookup(Decl));
     });
   }
 }

Added: cfe/trunk/test/Modules/Inputs/cxx-lookup/a.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/cxx-lookup/a.h?rev=215518&view=auto
==============================================================================
--- cfe/trunk/test/Modules/Inputs/cxx-lookup/a.h (added)
+++ cfe/trunk/test/Modules/Inputs/cxx-lookup/a.h Tue Aug 12 20:23:33 2014
@@ -0,0 +1,2 @@
+// a
+namespace llvm { class GlobalValue; }

Added: cfe/trunk/test/Modules/Inputs/cxx-lookup/b.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/cxx-lookup/b.h?rev=215518&view=auto
==============================================================================
--- cfe/trunk/test/Modules/Inputs/cxx-lookup/b.h (added)
+++ cfe/trunk/test/Modules/Inputs/cxx-lookup/b.h Tue Aug 12 20:23:33 2014
@@ -0,0 +1,3 @@
+// b
+namespace llvm { class GlobalValue; }
+#include "y.h"

Added: cfe/trunk/test/Modules/Inputs/cxx-lookup/c1.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/cxx-lookup/c1.h?rev=215518&view=auto
==============================================================================
--- cfe/trunk/test/Modules/Inputs/cxx-lookup/c1.h (added)
+++ cfe/trunk/test/Modules/Inputs/cxx-lookup/c1.h Tue Aug 12 20:23:33 2014
@@ -0,0 +1,3 @@
+// c1
+#include "a.h"
+#include "b.h"

Added: cfe/trunk/test/Modules/Inputs/cxx-lookup/c2.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/cxx-lookup/c2.h?rev=215518&view=auto
==============================================================================
--- cfe/trunk/test/Modules/Inputs/cxx-lookup/c2.h (added)
+++ cfe/trunk/test/Modules/Inputs/cxx-lookup/c2.h Tue Aug 12 20:23:33 2014
@@ -0,0 +1,2 @@
+// c2
+namespace llvm { class GlobalValue; }

Added: cfe/trunk/test/Modules/Inputs/cxx-lookup/module.modulemap
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/cxx-lookup/module.modulemap?rev=215518&view=auto
==============================================================================
--- cfe/trunk/test/Modules/Inputs/cxx-lookup/module.modulemap (added)
+++ cfe/trunk/test/Modules/Inputs/cxx-lookup/module.modulemap Tue Aug 12 20:23:33 2014
@@ -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 * }

Added: cfe/trunk/test/Modules/Inputs/cxx-lookup/x.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/cxx-lookup/x.h?rev=215518&view=auto
==============================================================================
--- cfe/trunk/test/Modules/Inputs/cxx-lookup/x.h (added)
+++ cfe/trunk/test/Modules/Inputs/cxx-lookup/x.h Tue Aug 12 20:23:33 2014
@@ -0,0 +1,2 @@
+template <class T> class allocator;
+struct X { virtual allocator<char> f(); };

Added: cfe/trunk/test/Modules/Inputs/cxx-lookup/y.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/cxx-lookup/y.h?rev=215518&view=auto
==============================================================================
--- cfe/trunk/test/Modules/Inputs/cxx-lookup/y.h (added)
+++ cfe/trunk/test/Modules/Inputs/cxx-lookup/y.h Tue Aug 12 20:23:33 2014
@@ -0,0 +1,5 @@
+#include "x.h"
+namespace llvm {
+  struct ulittle32_t;
+  extern allocator<ulittle32_t> *x;
+}

Added: cfe/trunk/test/Modules/cxx-lookup.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/cxx-lookup.cpp?rev=215518&view=auto
==============================================================================
--- cfe/trunk/test/Modules/cxx-lookup.cpp (added)
+++ cfe/trunk/test/Modules/cxx-lookup.cpp Tue Aug 12 20:23:33 2014
@@ -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;

Modified: cfe/trunk/test/Modules/cxx-templates.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/cxx-templates.cpp?rev=215518&r1=215517&r2=215518&view=diff
==============================================================================
--- cfe/trunk/test/Modules/cxx-templates.cpp (original)
+++ cfe/trunk/test/Modules/cxx-templates.cpp Tue Aug 12 20:23:33 2014
@@ -29,8 +29,8 @@ void g() {
   N::f<double>(1.0);
   N::f<int>();
   N::f(); // expected-error {{no matching function}}
-  // expected-note at Inputs/cxx-templates-a.h:6 {{couldn't infer template argument}}
-  // expected-note at Inputs/cxx-templates-a.h:7 {{requires 1 argument}}
+  // expected-note at Inputs/cxx-templates-b.h:6 {{couldn't infer template argument}}
+  // expected-note at 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





More information about the cfe-commits mailing list