[PATCH] Fix PR18307: Properly (de)serialize inherited constructors and their using declarations

[Stephan Tolksdorf]

  Thanks for the review!

  I've modified the comment, renamed the test file and added tests involving templates.

  As discussed on IRC, the current clang implementation actually depends on being able to look up UsingDecls for inherited constructors by the DeclarationName of a base, so I'll stick with the fix in WriteDeclContextVisibleBlock instead of trying to hide such UsingDecls.

Hi rsmith,

http://llvm-reviews.chandlerc.com/D3102

CHANGE SINCE LAST DIFF
  http://llvm-reviews.chandlerc.com/D3102?vs=7894&id=7961#toc

Files:
  lib/Serialization/ASTReaderDecl.cpp
  lib/Serialization/ASTWriter.cpp
  lib/Serialization/ASTWriterDecl.cpp
  test/PCH/cxx11-inheriting-ctors.cpp

Index: lib/Serialization/ASTReaderDecl.cpp
===================================================================
--- lib/Serialization/ASTReaderDecl.cpp
+++ lib/Serialization/ASTReaderDecl.cpp
@@ -1348,7 +1348,9 @@
 
 void ASTDeclReader::VisitCXXConstructorDecl(CXXConstructorDecl *D) {
   VisitCXXMethodDecl(D);
-  
+
+  if (auto *CD = ReadDeclAs<CXXConstructorDecl>(Record, Idx))
+    D->setInheritedConstructor(CD);
   D->IsExplicitSpecified = Record[Idx++];
   std::tie(D->CtorInitializers, D->NumCtorInitializers) =
       Reader.ReadCXXCtorInitializers(F, Record, Idx);
Index: lib/Serialization/ASTWriter.cpp
===================================================================
--- lib/Serialization/ASTWriter.cpp
+++ lib/Serialization/ASTWriter.cpp
@@ -3429,30 +3429,46 @@
   ASTDeclContextNameLookupTrait Trait(*this);
 
   // Create the on-disk hash table representation.
+  DeclarationName ConstructorName;
   DeclarationName ConversionName;
+  SmallVector<NamedDecl *, 8> ConstructorDecls;
   SmallVector<NamedDecl *, 4> ConversionDecls;
   for (StoredDeclsMap::iterator D = Map->begin(), DEnd = Map->end();
        D != DEnd; ++D) {
     DeclarationName Name = D->first;
     DeclContext::lookup_result Result = D->second.getLookupResult();
     if (!Result.empty()) {
-      if (Name.getNameKind() == DeclarationName::CXXConversionFunctionName) {
-        // Hash all conversion function names to the same name. The actual
-        // type information in conversion function name is not used in the
-        // key (since such type information is not stable across different
-        // modules), so the intended effect is to coalesce all of the conversion
-        // functions under a single key.
+      // Different DeclarationName values of certain kinds are mapped to
+      // identical serialized keys, because we don't want to use type
+      // identifiers in the keys (since type ids are local to the module).
+      switch (Name.getNameKind()) {
+      case DeclarationName::CXXConstructorName:
+        // There may be different CXXConstructorName DeclarationName values
+        // in a DeclContext because a UsingDecl that inherits constructors
+        // has the DeclarationName of the inherited constructors.
+        if (!ConstructorName)
+          ConstructorName = Name;
+        ConstructorDecls.append(Result.begin(), Result.end());
+        continue;
+      case DeclarationName::CXXConversionFunctionName:
         if (!ConversionName)
           ConversionName = Name;
         ConversionDecls.append(Result.begin(), Result.end());
         continue;
+      default:
+        break;
       }
-      
       Generator.insert(Name, Result, Trait);
     }
   }
-
-  // Add the conversion functions
+  // Add the constructors.
+  if (!ConstructorDecls.empty()) {
+    Generator.insert(ConstructorName,
+                     DeclContext::lookup_result(ConstructorDecls.begin(),
+                                                ConstructorDecls.end()),
+                     Trait);
+  }
+  // Add the conversion functions.
   if (!ConversionDecls.empty()) {
     Generator.insert(ConversionName, 
                      DeclContext::lookup_result(ConversionDecls.begin(),
Index: lib/Serialization/ASTWriterDecl.cpp
===================================================================
--- lib/Serialization/ASTWriterDecl.cpp
+++ lib/Serialization/ASTWriterDecl.cpp
@@ -1005,6 +1005,7 @@
 void ASTDeclWriter::VisitCXXConstructorDecl(CXXConstructorDecl *D) {
   VisitCXXMethodDecl(D);
 
+  Writer.AddDeclRef(D->getInheritedConstructor(), Record);
   Record.push_back(D->IsExplicitSpecified);
   Writer.AddCXXCtorInitializers(D->CtorInitializers, D->NumCtorInitializers,
                                 Record);
Index: test/PCH/cxx11-inheriting-ctors.cpp
===================================================================
--- /dev/null
+++ test/PCH/cxx11-inheriting-ctors.cpp
@@ -0,0 +1,39 @@
+// RUN: %clang_cc1 -std=c++11 -emit-pch -o %t %s
+// RUN: %clang_cc1 -std=c++11 -include-pch %t -verify %s
+
+// expected-no-diagnostics
+
+#ifndef HEADER_INCLUDED
+#define HEADER_INCLUDED
+
+struct Base {
+  Base(int) {}
+
+  template <typename T>
+  Base(T) {}
+};
+
+struct Test : Base {
+  using Base::Base;
+};
+
+template <typename T>
+struct Test2 : Base {
+  using Base::Base;
+};
+
+template <typename B>
+struct Test3 : B {
+  using B::B;
+};
+
+#else
+
+Test test1a(42);
+Test test1b(nullptr);
+Test2<int> test2a(42);
+Test2<int> test2b(nullptr);
+Test3<Base> test3a(42);
+Test3<Base> test3b(nullptr);
+
+#endif // HEADER_INCLUDED
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D3102.2.patch
Type: text/x-patch
Size: 4563 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140319/01fba2eb/attachment.bin>