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

[Chandler Carruth]

Author: chandlerc
Date: Thu Mar 26 17:27:09 2015
New Revision: 233327

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

Modified:
    cfe/trunk/lib/Serialization/ASTWriter.cpp
    cfe/trunk/test/Modules/Inputs/stress1/m01.h
    cfe/trunk/test/Modules/Inputs/stress1/merge00.h

Modified: cfe/trunk/lib/Serialization/ASTWriter.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTWriter.cpp?rev=233327&r1=233326&r2=233327&view=diff
==============================================================================
--- cfe/trunk/lib/Serialization/ASTWriter.cpp (original)
+++ cfe/trunk/lib/Serialization/ASTWriter.cpp Thu Mar 26 17:27:09 2015
@@ -3786,34 +3786,48 @@ ASTWriter::GenerateNameLookupTable(const
   // 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;
-
-        case DeclarationName::CXXConversionFunctionName:
-          if (ConversionNameSet.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;
 
-        if (ConstructorNameSet.empty() && ConversionNameSet.empty())
-          break;
-      }
+          case DeclarationName::CXXConstructorName:
+            if (ConstructorNameSet.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;
+        }
 
     assert(ConstructorNameSet.empty() && "Failed to find all of the visible "
                                          "constructors by walking all the "

Modified: cfe/trunk/test/Modules/Inputs/stress1/m01.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/stress1/m01.h?rev=233327&r1=233326&r2=233327&view=diff
==============================================================================
--- cfe/trunk/test/Modules/Inputs/stress1/m01.h (original)
+++ cfe/trunk/test/Modules/Inputs/stress1/m01.h Thu Mar 26 17:27:09 2015
@@ -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

Modified: cfe/trunk/test/Modules/Inputs/stress1/merge00.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/stress1/merge00.h?rev=233327&r1=233326&r2=233327&view=diff
==============================================================================
--- cfe/trunk/test/Modules/Inputs/stress1/merge00.h (original)
+++ cfe/trunk/test/Modules/Inputs/stress1/merge00.h Thu Mar 26 17:27:09 2015
@@ -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