[Modules] Fix a sneaky bug in r233249 where we would look for implicit

constructors in the current lexical context even though name lookup
found them via some other context merged into the redecl chain.

This can only happen for implicit constructors which can only have the
name of the type of the current context, so we can fix this by simply
*always* merging those names first. This also has the advantage of
removing the walk of the current lexical context from the common case
when this is the only constructor name we need to deal with (implicit or
otherwise).

I've enhanced the tests to cover this case (and uncovered an unrelated
bug which I fixed in r233325).

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

diff --git a/lib/Serialization/ASTWriter.cpp b/lib/Serialization/ASTWriter.cpp
index 679419c9c5aec319ccd76e987d78cd36ba5547a4..24ba783c4eff372ed535369b5ffa78aae59913f8 100644 (file)
--- a/lib/Serialization/ASTWriter.cpp
+++ b/lib/Serialization/ASTWriter.cpp
@@ -3786,35 +3786,49 @@ ASTWriter::GenerateNameLookupTable(const DeclContext *ConstDC,
   // Sort the names into a stable order.
   std::sort(Names.begin(), Names.end());
 
-  if (isa<CXXRecordDecl>(DC) &&
-      (!ConstructorNameSet.empty() || !ConversionNameSet.empty())) {
+  if (auto *D = dyn_cast<CXXRecordDecl>(DC)) {
     // We need to establish an ordering of constructor and conversion function
-    // names, and they don't have an intrinsic ordering. So when we have these,
-    // we walk all the names in the decl and add the constructors and
-    // conversion functions which are visible in the order they lexically occur
-    // within the context.
-    for (Decl *ChildD : DC->decls())
-      if (auto *ChildND = dyn_cast<NamedDecl>(ChildD)) {
-        auto Name = ChildND->getDeclName();
-        switch (Name.getNameKind()) {
-        default:
-          continue;
-
-        case DeclarationName::CXXConstructorName:
-          if (ConstructorNameSet.erase(Name))
-            Names.push_back(Name);
-          break;
+    // names, and they don't have an intrinsic ordering.
+
+    // First we try the easy case by forming the current context's constructor
+    // name and adding that name first. This is a very useful optimization to
+    // avoid walking the lexical declarations in many cases, and it also
+    // handles the only case where a constructor name can come from some other
+    // lexical context -- when that name is an implicit constructor merged from
+    // another declaration in the redecl chain. Any non-implicit constructor or
+    // conversion function which doesn't occur in all the lexical contexts
+    // would be an ODR violation.
+    auto ImplicitCtorName = Context->DeclarationNames.getCXXConstructorName(
+        Context->getCanonicalType(Context->getRecordType(D)));
+    if (ConstructorNameSet.erase(ImplicitCtorName))
+      Names.push_back(ImplicitCtorName);
+
+    // If we still have constructors or conversion functions, we walk all the
+    // names in the decl and add the constructors and conversion functions
+    // which are visible in the order they lexically occur within the context.
+    if (!ConstructorNameSet.empty() || !ConversionNameSet.empty())
+      for (Decl *ChildD : cast<CXXRecordDecl>(DC)->decls())
+        if (auto *ChildND = dyn_cast<NamedDecl>(ChildD)) {
+          auto Name = ChildND->getDeclName();
+          switch (Name.getNameKind()) {
+          default:
+            continue;
+
+          case DeclarationName::CXXConstructorName:
+            if (ConstructorNameSet.erase(Name))
+              Names.push_back(Name);
+            break;
+
+          case DeclarationName::CXXConversionFunctionName:
+            if (ConversionNameSet.erase(Name))
+              Names.push_back(Name);
+            break;
+          }
 
-        case DeclarationName::CXXConversionFunctionName:
-          if (ConversionNameSet.erase(Name))
-            Names.push_back(Name);
-          break;
+          if (ConstructorNameSet.empty() && ConversionNameSet.empty())
+            break;
         }
 
-        if (ConstructorNameSet.empty() && ConversionNameSet.empty())
-          break;
-      }
-
     assert(ConstructorNameSet.empty() && "Failed to find all of the visible "
                                          "constructors by walking all the "
                                          "lexical members of the context.");

diff --git a/test/Modules/Inputs/stress1/m01.h b/test/Modules/Inputs/stress1/m01.h
index d0b150a7382c0e3ea813de5b65e392b3beed082f..23a3d4b289f6f50f55b9550eeb5bb13cba6d29d7 100644 (file)
--- a/test/Modules/Inputs/stress1/m01.h
+++ b/test/Modules/Inputs/stress1/m01.h
@@ -3,4 +3,8 @@
 
 #include "common.h"
 
+// Trigger the use of special members for a class this is also defined in other
+// modules.
+inline N00::S01 m01_special_members() { return N00::S01(); }
+
 #endif

diff --git a/test/Modules/Inputs/stress1/merge00.h b/test/Modules/Inputs/stress1/merge00.h
index fc1b4f8118804dfebc56016cdfcefbda261ada9b..46d5e4138279d76a33fc0717db6c7cdfdea65437 100644 (file)
--- a/test/Modules/Inputs/stress1/merge00.h
+++ b/test/Modules/Inputs/stress1/merge00.h
@@ -9,6 +9,7 @@
 //#pragma weak pragma_weak01 // expected-warning {{weak identifier 'pragma_weak01' never declared}}
 //#pragma weak pragma_weak04 // expected-warning {{weak identifier 'pragma_waek04' never declared}}
 
+#include "common.h"
 #include "m00.h"
 #include "m01.h"
 #include "m02.h"
@@ -16,6 +17,10 @@
 
 inline int g() { return N00::S00('a').method00('b') + (int)S00(42) + function00(42); }
 
+// Use implicit special memebers again for S01 to ensure that we merge them in
+// successfully from m01.
+inline N00::S01 h() { return N00::S01(); }
+
 #pragma weak pragma_weak02
 #pragma weak pragma_weak05