r215612 - [modules] When we merge together multiple class template specialization
Richard Smith
richard-llvm at metafoo.co.uk
Wed Aug 13 19:21:01 PDT 2014
Author: rsmith
Date: Wed Aug 13 21:21:01 2014
New Revision: 215612
URL: http://llvm.org/viewvc/llvm-project?rev=215612&view=rev
Log:
[modules] When we merge together multiple class template specialization
definitions (because some other declaration declares a special member that
isn't present in the canonical definition), we need to search *all* of them; we
can't just stop when we find the requested name in any of the definitions,
because that can fail to find things (and in particular, it can fail to find
the member of the canonical declaration and return a bogus ODR failure).
Modified:
cfe/trunk/lib/Serialization/ASTReader.cpp
cfe/trunk/lib/Serialization/ASTWriter.cpp
cfe/trunk/test/Modules/Inputs/cxx-templates-a.h
cfe/trunk/test/Modules/Inputs/cxx-templates-b.h
cfe/trunk/test/Modules/Inputs/cxx-templates-common.h
cfe/trunk/test/Modules/cxx-templates.cpp
Modified: cfe/trunk/lib/Serialization/ASTReader.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReader.cpp?rev=215612&r1=215611&r2=215612&view=diff
==============================================================================
--- cfe/trunk/lib/Serialization/ASTReader.cpp (original)
+++ cfe/trunk/lib/Serialization/ASTReader.cpp Wed Aug 13 21:21:01 2014
@@ -6417,13 +6417,13 @@ namespace {
/// declaration context.
class DeclContextNameLookupVisitor {
ASTReader &Reader;
- SmallVectorImpl<const DeclContext *> &Contexts;
+ ArrayRef<const DeclContext *> Contexts;
DeclarationName Name;
SmallVectorImpl<NamedDecl *> &Decls;
public:
- DeclContextNameLookupVisitor(ASTReader &Reader,
- SmallVectorImpl<const DeclContext *> &Contexts,
+ DeclContextNameLookupVisitor(ASTReader &Reader,
+ ArrayRef<const DeclContext *> Contexts,
DeclarationName Name,
SmallVectorImpl<NamedDecl *> &Decls)
: Reader(Reader), Contexts(Contexts), Name(Name), Decls(Decls) { }
@@ -6436,9 +6436,9 @@ namespace {
// this context in this module.
ModuleFile::DeclContextInfosMap::iterator Info;
bool FoundInfo = false;
- for (unsigned I = 0, N = This->Contexts.size(); I != N; ++I) {
- Info = M.DeclContextInfos.find(This->Contexts[I]);
- if (Info != M.DeclContextInfos.end() &&
+ for (auto *DC : This->Contexts) {
+ Info = M.DeclContextInfos.find(DC);
+ if (Info != M.DeclContextInfos.end() &&
Info->second.NameLookupTableData) {
FoundInfo = true;
break;
@@ -6447,7 +6447,7 @@ namespace {
if (!FoundInfo)
return false;
-
+
// Look for this name within this module.
ASTDeclContextNameLookupTable *LookupTable =
Info->second.NameLookupTableData;
@@ -6468,9 +6468,11 @@ namespace {
// currently read before reading its name. The lookup is triggered by
// building that decl (likely indirectly), and so it is later in the
// sense of "already existing" and can be ignored here.
+ // FIXME: This should not happen; deserializing declarations should
+ // not perform lookups since that can lead to deserialization cycles.
continue;
}
-
+
// Record this declaration.
FoundAnything = true;
This->Decls.push_back(ND);
@@ -6510,15 +6512,17 @@ ASTReader::FindExternalVisibleDeclsByNam
if (!Name)
return false;
+ Deserializing LookupResults(this);
+
SmallVector<NamedDecl *, 64> Decls;
-
+
// Compute the declaration contexts we need to look into. Multiple such
// declaration contexts occur when two declaration contexts from disjoint
// modules get merged, e.g., when two namespaces with the same name are
// independently defined in separate modules.
SmallVector<const DeclContext *, 2> Contexts;
Contexts.push_back(DC);
-
+
if (DC->isNamespace()) {
auto Merged = MergedDecls.find(const_cast<Decl *>(cast<Decl>(DC)));
if (Merged != MergedDecls.end()) {
@@ -6526,24 +6530,40 @@ ASTReader::FindExternalVisibleDeclsByNam
Contexts.push_back(cast<DeclContext>(GetDecl(Merged->second[I])));
}
}
- if (isa<CXXRecordDecl>(DC)) {
- auto Merged = MergedLookups.find(DC);
- if (Merged != MergedLookups.end())
- Contexts.insert(Contexts.end(), Merged->second.begin(),
- Merged->second.end());
- }
- DeclContextNameLookupVisitor Visitor(*this, Contexts, Name, Decls);
+ auto LookUpInContexts = [&](ArrayRef<const DeclContext*> Contexts) {
+ DeclContextNameLookupVisitor Visitor(*this, Contexts, Name, Decls);
- // If we can definitively determine which module file to look into,
- // only look there. Otherwise, look in all module files.
- ModuleFile *Definitive;
- if (Contexts.size() == 1 &&
- (Definitive = getDefinitiveModuleFileFor(DC, *this))) {
- DeclContextNameLookupVisitor::visit(*Definitive, &Visitor);
- } else {
- ModuleMgr.visit(&DeclContextNameLookupVisitor::visit, &Visitor);
+ // If we can definitively determine which module file to look into,
+ // only look there. Otherwise, look in all module files.
+ ModuleFile *Definitive;
+ if (Contexts.size() == 1 &&
+ (Definitive = getDefinitiveModuleFileFor(Contexts[0], *this))) {
+ DeclContextNameLookupVisitor::visit(*Definitive, &Visitor);
+ } else {
+ ModuleMgr.visit(&DeclContextNameLookupVisitor::visit, &Visitor);
+ }
+ };
+
+ LookUpInContexts(Contexts);
+
+ // If this might be an implicit special member function, then also search
+ // all merged definitions of the surrounding class. We need to search them
+ // individually, because finding an entity in one of them doesn't imply that
+ // we can't find a different entity in another one.
+ if (isa<CXXRecordDecl>(DC)) {
+ auto Kind = Name.getNameKind();
+ if (Kind == DeclarationName::CXXConstructorName ||
+ Kind == DeclarationName::CXXDestructorName ||
+ (Kind == DeclarationName::CXXOperatorName &&
+ Name.getCXXOverloadedOperator() == OO_Equal)) {
+ auto Merged = MergedLookups.find(DC);
+ if (Merged != MergedLookups.end())
+ for (auto *MergedDC : Merged->second)
+ LookUpInContexts(MergedDC);
+ }
}
+
++NumVisibleDeclContextsRead;
SetExternalVisibleDeclsForName(DC, Name, Decls);
return !Decls.empty();
Modified: cfe/trunk/lib/Serialization/ASTWriter.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTWriter.cpp?rev=215612&r1=215611&r2=215612&view=diff
==============================================================================
--- cfe/trunk/lib/Serialization/ASTWriter.cpp (original)
+++ cfe/trunk/lib/Serialization/ASTWriter.cpp Wed Aug 13 21:21:01 2014
@@ -3485,9 +3485,22 @@ void ASTWriter::WriteIdentifierTable(Pre
/// recent local declaration instead. The chosen declaration will be the most
/// recent declaration in any module that imports this one.
static NamedDecl *getDeclForLocalLookup(NamedDecl *D) {
- for (Decl *Redecl = D; Redecl; Redecl = Redecl->getPreviousDecl())
- if (!Redecl->isFromASTFile())
- return cast<NamedDecl>(Redecl);
+ if (!D->isFromASTFile())
+ return D;
+
+ if (Decl *Redecl = D->getPreviousDecl()) {
+ // For Redeclarable decls, a prior declaration might be local.
+ for (; Redecl; Redecl = Redecl->getPreviousDecl())
+ if (!Redecl->isFromASTFile())
+ return cast<NamedDecl>(Redecl);
+ } else if (Decl *First = D->getCanonicalDecl()) {
+ // For Mergeable decls, the first decl might be local.
+ if (!First->isFromASTFile())
+ return cast<NamedDecl>(First);
+ }
+
+ // All declarations are imported. Our most recent declaration will also be
+ // the most recent one in anyone who imports us.
return D;
}
Modified: cfe/trunk/test/Modules/Inputs/cxx-templates-a.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/cxx-templates-a.h?rev=215612&r1=215611&r2=215612&view=diff
==============================================================================
--- cfe/trunk/test/Modules/Inputs/cxx-templates-a.h (original)
+++ cfe/trunk/test/Modules/Inputs/cxx-templates-a.h Wed Aug 13 21:21:01 2014
@@ -29,6 +29,8 @@ void use_some_template_a() {
SomeTemplate<char[2]> a;
SomeTemplate<char[1]> b, c;
b = c;
+
+ (void)&WithImplicitSpecialMembers<int>::n;
}
template<int> struct MergeTemplates;
Modified: cfe/trunk/test/Modules/Inputs/cxx-templates-b.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/cxx-templates-b.h?rev=215612&r1=215611&r2=215612&view=diff
==============================================================================
--- cfe/trunk/test/Modules/Inputs/cxx-templates-b.h (original)
+++ cfe/trunk/test/Modules/Inputs/cxx-templates-b.h Wed Aug 13 21:21:01 2014
@@ -46,6 +46,8 @@ void use_some_template_b() {
SomeTemplate<char[1]> a;
SomeTemplate<char[2]> b, c;
b = c;
+
+ WithImplicitSpecialMembers<int> wism1, wism2(wism1);
}
auto enum_b_from_b = CommonTemplate<int>::b;
Modified: cfe/trunk/test/Modules/Inputs/cxx-templates-common.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/cxx-templates-common.h?rev=215612&r1=215611&r2=215612&view=diff
==============================================================================
--- cfe/trunk/test/Modules/Inputs/cxx-templates-common.h (original)
+++ cfe/trunk/test/Modules/Inputs/cxx-templates-common.h Wed Aug 13 21:21:01 2014
@@ -36,3 +36,5 @@ typedef WithPartialSpecialization<int*>
template<typename T> struct WithExplicitSpecialization;
typedef WithExplicitSpecialization<int> WithExplicitSpecializationUse;
+
+template<typename T> struct WithImplicitSpecialMembers { int n; };
Modified: cfe/trunk/test/Modules/cxx-templates.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/cxx-templates.cpp?rev=215612&r1=215611&r2=215612&view=diff
==============================================================================
--- cfe/trunk/test/Modules/cxx-templates.cpp (original)
+++ cfe/trunk/test/Modules/cxx-templates.cpp Wed Aug 13 21:21:01 2014
@@ -106,6 +106,8 @@ void g() {
int &p = WithPartialSpecializationUse().f();
int &q = WithExplicitSpecializationUse().inner_template<int>();
int *r = PartiallyInstantiatePartialSpec<int*>::bar();
+
+ (void)&WithImplicitSpecialMembers<int>::n;
}
static_assert(Outer<int>::Inner<int>::f() == 1, "");
More information about the cfe-commits
mailing list