[modules] When we merge together multiple class template specialization

definitions (because some other declaration declares a special member that
isn't present in the canonical definition), we need to search *all* of them; we
can't just stop when we find the requested name in any of the definitions,
because that can fail to find things (and in particular, it can fail to find
the member of the canonical declaration and return a bogus ODR failure).

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

diff --git a/lib/Serialization/ASTReader.cpp b/lib/Serialization/ASTReader.cpp
index d3d1e48ccbb1d374bd3e22e47c909082f58f55c1..161971b35c8cacb6bed5b9625b4e6cc274d42743 100644 (file)
--- a/lib/Serialization/ASTReader.cpp
+++ b/lib/Serialization/ASTReader.cpp
@@ -6417,13 +6417,13 @@ namespace {
   /// declaration context.
   class DeclContextNameLookupVisitor {
     ASTReader &Reader;
-    SmallVectorImpl<const DeclContext *> &Contexts;
+    ArrayRef<const DeclContext *> Contexts;
     DeclarationName Name;
     SmallVectorImpl<NamedDecl *> &Decls;
 
   public:
-    DeclContextNameLookupVisitor(ASTReader &Reader, 
-                                 SmallVectorImpl<const DeclContext *> &Contexts, 
+    DeclContextNameLookupVisitor(ASTReader &Reader,
+                                 ArrayRef<const DeclContext *> Contexts,
                                  DeclarationName Name,
                                  SmallVectorImpl<NamedDecl *> &Decls)
       : Reader(Reader), Contexts(Contexts), Name(Name), Decls(Decls) { }
@@ -6436,9 +6436,9 @@ namespace {
       // this context in this module.
       ModuleFile::DeclContextInfosMap::iterator Info;
       bool FoundInfo = false;
-      for (unsigned I = 0, N = This->Contexts.size(); I != N; ++I) {
-        Info = M.DeclContextInfos.find(This->Contexts[I]);
-        if (Info != M.DeclContextInfos.end() && 
+      for (auto *DC : This->Contexts) {
+        Info = M.DeclContextInfos.find(DC);
+        if (Info != M.DeclContextInfos.end() &&
             Info->second.NameLookupTableData) {
           FoundInfo = true;
           break;
@@ -6447,7 +6447,7 @@ namespace {
 
       if (!FoundInfo)
         return false;
-      
+
       // Look for this name within this module.
       ASTDeclContextNameLookupTable *LookupTable =
         Info->second.NameLookupTableData;
@@ -6468,9 +6468,11 @@ namespace {
           // currently read before reading its name. The lookup is triggered by
           // building that decl (likely indirectly), and so it is later in the
           // sense of "already existing" and can be ignored here.
+          // FIXME: This should not happen; deserializing declarations should
+          // not perform lookups since that can lead to deserialization cycles.
           continue;
         }
-      
+
         // Record this declaration.
         FoundAnything = true;
         This->Decls.push_back(ND);
@@ -6510,15 +6512,17 @@ ASTReader::FindExternalVisibleDeclsByName(const DeclContext *DC,
   if (!Name)
     return false;
 
+  Deserializing LookupResults(this);
+
   SmallVector<NamedDecl *, 64> Decls;
-  
+
   // Compute the declaration contexts we need to look into. Multiple such
   // declaration contexts occur when two declaration contexts from disjoint
   // modules get merged, e.g., when two namespaces with the same name are 
   // independently defined in separate modules.
   SmallVector<const DeclContext *, 2> Contexts;
   Contexts.push_back(DC);
-  
+
   if (DC->isNamespace()) {
     auto Merged = MergedDecls.find(const_cast<Decl *>(cast<Decl>(DC)));
     if (Merged != MergedDecls.end()) {
@@ -6526,24 +6530,40 @@ ASTReader::FindExternalVisibleDeclsByName(const DeclContext *DC,
         Contexts.push_back(cast<DeclContext>(GetDecl(Merged->second[I])));
     }
   }
-  if (isa<CXXRecordDecl>(DC)) {
-    auto Merged = MergedLookups.find(DC);
-    if (Merged != MergedLookups.end())
-      Contexts.insert(Contexts.end(), Merged->second.begin(),
-                      Merged->second.end());
-  }
 
-  DeclContextNameLookupVisitor Visitor(*this, Contexts, Name, Decls);
+  auto LookUpInContexts = [&](ArrayRef<const DeclContext*> Contexts) {
+    DeclContextNameLookupVisitor Visitor(*this, Contexts, Name, Decls);
 
-  // If we can definitively determine which module file to look into,
-  // only look there. Otherwise, look in all module files.
-  ModuleFile *Definitive;
-  if (Contexts.size() == 1 &&
-      (Definitive = getDefinitiveModuleFileFor(DC, *this))) {
-    DeclContextNameLookupVisitor::visit(*Definitive, &Visitor);
-  } else {
-    ModuleMgr.visit(&DeclContextNameLookupVisitor::visit, &Visitor);
+    // If we can definitively determine which module file to look into,
+    // only look there. Otherwise, look in all module files.
+    ModuleFile *Definitive;
+    if (Contexts.size() == 1 &&
+        (Definitive = getDefinitiveModuleFileFor(Contexts[0], *this))) {
+      DeclContextNameLookupVisitor::visit(*Definitive, &Visitor);
+    } else {
+      ModuleMgr.visit(&DeclContextNameLookupVisitor::visit, &Visitor);
+    }
+  };
+
+  LookUpInContexts(Contexts);
+
+  // If this might be an implicit special member function, then also search
+  // all merged definitions of the surrounding class. We need to search them
+  // individually, because finding an entity in one of them doesn't imply that
+  // we can't find a different entity in another one.
+  if (isa<CXXRecordDecl>(DC)) {
+    auto Kind = Name.getNameKind();
+    if (Kind == DeclarationName::CXXConstructorName ||
+        Kind == DeclarationName::CXXDestructorName ||
+        (Kind == DeclarationName::CXXOperatorName &&
+         Name.getCXXOverloadedOperator() == OO_Equal)) {
+      auto Merged = MergedLookups.find(DC);
+      if (Merged != MergedLookups.end())
+        for (auto *MergedDC : Merged->second)
+          LookUpInContexts(MergedDC);
+    }
   }
+
   ++NumVisibleDeclContextsRead;
   SetExternalVisibleDeclsForName(DC, Name, Decls);
   return !Decls.empty();

diff --git a/lib/Serialization/ASTWriter.cpp b/lib/Serialization/ASTWriter.cpp
index e8c3177cb86745bb15d5c3a41c59c05a748c7a22..55b557c047136b8db17d01c07abbca14d0f0c012 100644 (file)
--- a/lib/Serialization/ASTWriter.cpp
+++ b/lib/Serialization/ASTWriter.cpp
@@ -3485,9 +3485,22 @@ void ASTWriter::WriteIdentifierTable(Preprocessor &PP,
 /// 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);
+  if (!D->isFromASTFile())
+    return D;
+
+  if (Decl *Redecl = D->getPreviousDecl()) {
+    // For Redeclarable decls, a prior declaration might be local.
+    for (; Redecl; Redecl = Redecl->getPreviousDecl())
+      if (!Redecl->isFromASTFile())
+        return cast<NamedDecl>(Redecl);
+  } else if (Decl *First = D->getCanonicalDecl()) {
+    // For Mergeable decls, the first decl might be local.
+    if (!First->isFromASTFile())
+      return cast<NamedDecl>(First);
+  }
+
+  // All declarations are imported. Our most recent declaration will also be
+  // the most recent one in anyone who imports us.
   return D;
 }
 

diff --git a/test/Modules/Inputs/cxx-templates-a.h b/test/Modules/Inputs/cxx-templates-a.h
index 0289c8ada82357d47a5da0a95ff449eebbf2d0de..1ade811d71d5e7f30e601c057f718bb6d0ec33be 100644 (file)
--- a/test/Modules/Inputs/cxx-templates-a.h
+++ b/test/Modules/Inputs/cxx-templates-a.h
@@ -29,6 +29,8 @@ void use_some_template_a() {
   SomeTemplate<char[2]> a;
   SomeTemplate<char[1]> b, c;
   b = c;
+
+  (void)&WithImplicitSpecialMembers<int>::n;
 }
 
 template<int> struct MergeTemplates;

diff --git a/test/Modules/Inputs/cxx-templates-b.h b/test/Modules/Inputs/cxx-templates-b.h
index 5dbf1a11216f1396501ec5acab79cd24b28acf40..77a2c2333ac9bb42175b5ddfa91682b504d0639e 100644 (file)
--- a/test/Modules/Inputs/cxx-templates-b.h
+++ b/test/Modules/Inputs/cxx-templates-b.h
@@ -46,6 +46,8 @@ void use_some_template_b() {
   SomeTemplate<char[1]> a;
   SomeTemplate<char[2]> b, c;
   b = c;
+
+  WithImplicitSpecialMembers<int> wism1, wism2(wism1);
 }
 
 auto enum_b_from_b = CommonTemplate<int>::b;

diff --git a/test/Modules/Inputs/cxx-templates-common.h b/test/Modules/Inputs/cxx-templates-common.h
index 682ef939cec0fcc577cadf8fd939a75d70b426f9..074cc8f9007f5fa08e0475d8eea54b03df62cca3 100644 (file)
--- a/test/Modules/Inputs/cxx-templates-common.h
+++ b/test/Modules/Inputs/cxx-templates-common.h
@@ -36,3 +36,5 @@ typedef WithPartialSpecialization<int*> WithPartialSpecializationUse;
 
 template<typename T> struct WithExplicitSpecialization;
 typedef WithExplicitSpecialization<int> WithExplicitSpecializationUse;
+
+template<typename T> struct WithImplicitSpecialMembers { int n; };

diff --git a/test/Modules/cxx-templates.cpp b/test/Modules/cxx-templates.cpp
index 6f5d319a8e52e09c5db2efb293fae239a55ddf87..c173d670802f6a2128457fca73d65a52677eb7b1 100644 (file)
--- a/test/Modules/cxx-templates.cpp
+++ b/test/Modules/cxx-templates.cpp
@@ -106,6 +106,8 @@ void g() {
   int &p = WithPartialSpecializationUse().f();
   int &q = WithExplicitSpecializationUse().inner_template<int>();
   int *r = PartiallyInstantiatePartialSpec<int*>::bar();
+
+  (void)&WithImplicitSpecialMembers<int>::n;
 }
 
 static_assert(Outer<int>::Inner<int>::f() == 1, "");