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

Richard Smith richard at metafoo.co.uk
Mon Mar 17 12:56:52 PDT 2014



================
Comment at: lib/Serialization/ASTWriter.cpp:3442
@@ +3441,3 @@
+      // Different DeclarationName values of certain kinds are mapped to
+      // identical serialized keys, because we don't to use type identifiers
+      // in the keys.
----------------
Typo: "don't to" (missing "want").

(The old comment is correct about stability for conversion function names.)

================
Comment at: lib/Serialization/ASTWriter.cpp:3445-3452
@@ +3444,10 @@
+      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:
----------------
There's no reason for the `UsingDecl`s created to represent inheriting constructors to be found by name lookup. A simpler approach might be to hide them from the `DeclContext` name lookup tables entirely.

================
Comment at: test/PCH/pr18307.cpp:1
@@ +1,2 @@
+// RUN: %clang_cc1 -std=c++11 -emit-pch -o %t %s
+// RUN: %clang_cc1 -std=c++11 -include-pch %t -verify %s
----------------
A better name for this test would be `test/PCH/cxx11-inheriting-ctors.cpp`.

================
Comment at: test/PCH/pr18307.cpp:9-11
@@ +8,5 @@
+
+struct Base {
+  Base(int) {}
+};
+
----------------
Maybe also add a test for inheriting a constructor template?


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



More information about the cfe-commits mailing list