[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