From b60fae50d38a0291e1c5731b2fb22849d26ca342 Mon Sep 17 00:00:00 2001 From: Richard Smith Date: Mon, 9 Sep 2013 16:55:27 +0000 Subject: [PATCH] C++ modules: if a class is defined in multiple modules (for instance, because it is an implicit instantiation of a class template specialization), pick the first-loaded definition to be the canonical definition, and merge all other definitions into it. This is still rather incomplete -- we need to extend every form of declaration that can appear within a CXXRecordDecl to be redeclarable if it came from an AST file (this includes fields, enumerators, ...). git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@190315 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/clang/AST/DeclTemplate.h | 2 +- include/clang/Serialization/ASTReader.h | 5 + lib/Sema/SemaLookup.cpp | 2 +- lib/Serialization/ASTReaderDecl.cpp | 240 +++++++++++++++--------- lib/Serialization/ASTWriterDecl.cpp | 14 +- test/Modules/Inputs/cxx-templates-a.h | 6 + test/Modules/Inputs/cxx-templates-b.h | 6 + test/Modules/Inputs/templates-left.h | 2 + test/Modules/Inputs/templates-right.h | 2 + test/Modules/Inputs/templates-top.h | 4 + test/Modules/cxx-templates.cpp | 8 + test/Modules/templates.mm | 5 + 12 files changed, 196 insertions(+), 100 deletions(-) diff --git a/include/clang/AST/DeclTemplate.h b/include/clang/AST/DeclTemplate.h index 55821e1ac6..0a4c89d0a3 100644 --- a/include/clang/AST/DeclTemplate.h +++ b/include/clang/AST/DeclTemplate.h @@ -1450,7 +1450,7 @@ public: ClassTemplateSpecializationDecl *getMostRecentDecl() { CXXRecordDecl *Recent = cast(CXXRecordDecl::getMostRecentDecl()); - if (!isa(Recent)) { + while (!isa(Recent)) { // FIXME: Does injected class name need to be in the redeclarations chain? assert(Recent->isInjectedClassName() && Recent->getPreviousDecl()); Recent = Recent->getPreviousDecl(); diff --git a/include/clang/Serialization/ASTReader.h b/include/clang/Serialization/ASTReader.h index 0354e824dd..66fff8f4b8 100644 --- a/include/clang/Serialization/ASTReader.h +++ b/include/clang/Serialization/ASTReader.h @@ -912,6 +912,11 @@ private: MergedDeclsMap::iterator combineStoredMergedDecls(Decl *Canon, serialization::GlobalDeclID CanonID); + /// \brief A mapping from DeclContexts to the semantic DeclContext that we + /// are treating as the definition of the entity. This is used, for instance, + /// when merging implicit instantiations of class templates across modules. + llvm::DenseMap MergedDeclContexts; + /// \brief When reading a Stmt tree, Stmt operands are placed in this stack. SmallVector StmtStack; diff --git a/lib/Sema/SemaLookup.cpp b/lib/Sema/SemaLookup.cpp index d3fdecbe40..5b641d8059 100644 --- a/lib/Sema/SemaLookup.cpp +++ b/lib/Sema/SemaLookup.cpp @@ -2506,7 +2506,7 @@ Sema::SpecialMemberOverloadResult *Sema::LookupSpecialMember(CXXRecordDecl *RD, SmallVector Candidates(R.begin(), R.end()); for (SmallVectorImpl::iterator I = Candidates.begin(), - E = Candidates.end(); + E = Candidates.end(); I != E; ++I) { NamedDecl *Cand = *I; diff --git a/lib/Serialization/ASTReaderDecl.cpp b/lib/Serialization/ASTReaderDecl.cpp index a74f8c1023..791f3c9f8c 100644 --- a/lib/Serialization/ASTReaderDecl.cpp +++ b/lib/Serialization/ASTReaderDecl.cpp @@ -291,7 +291,11 @@ namespace clang { template void mergeRedeclarable(Redeclarable *D, RedeclarableResult &Redecl); - + + template + void mergeRedeclarable(Redeclarable *D, T *Existing, + RedeclarableResult &Redecl); + // FIXME: Reorder according to DeclNodes.td? void VisitObjCMethodDecl(ObjCMethodDecl *D); void VisitObjCContainerDecl(ObjCContainerDecl *D); @@ -363,9 +367,11 @@ void ASTDeclReader::VisitDecl(Decl *D) { } else { DeclContext *SemaDC = ReadDeclAs(Record, Idx); DeclContext *LexicalDC = ReadDeclAs(Record, Idx); + DeclContext *MergedSemaDC = Reader.MergedDeclContexts.lookup(SemaDC); // Avoid calling setLexicalDeclContext() directly because it uses // Decl::getASTContext() internally which is unsafe during derialization. - D->setDeclContextsImpl(SemaDC, LexicalDC, Reader.getContext()); + D->setDeclContextsImpl(MergedSemaDC ? MergedSemaDC : SemaDC, LexicalDC, + Reader.getContext()); } D->setLocation(Reader.ReadSourceLocation(F, RawLocation)); D->setInvalidDecl(Record[Idx++]); @@ -1230,7 +1236,8 @@ ASTDeclReader::VisitCXXRecordDeclImpl(CXXRecordDecl *D) { RedeclarableResult Redecl = VisitRecordDeclImpl(D); ASTContext &C = Reader.getContext(); - if (Record[Idx++]) { + bool WasDefinition = Record[Idx++]; + if (WasDefinition) { // Determine whether this is a lambda closure type, so that we can // allocate the appropriate DefinitionData structure. bool IsLambda = Record[Idx++]; @@ -1239,21 +1246,34 @@ ASTDeclReader::VisitCXXRecordDeclImpl(CXXRecordDecl *D) { false); else D->DefinitionData = new (C) struct CXXRecordDecl::DefinitionData(D); - + + ReadCXXDefinitionData(*D->DefinitionData, Record, Idx); + // Propagate the DefinitionData pointer to the canonical declaration, so // that all other deserialized declarations will see it. - // FIXME: Complain if there already is a DefinitionData! - D->getCanonicalDecl()->DefinitionData = D->DefinitionData; - - ReadCXXDefinitionData(*D->DefinitionData, Record, Idx); - - // Note that we have deserialized a definition. Any declarations - // deserialized before this one will be be given the DefinitionData pointer - // at the end. - Reader.PendingDefinitions.insert(D); + CXXRecordDecl *Canon = D->getCanonicalDecl(); + if (Canon == D) { + // Nothing to do. + } else if (!Canon->DefinitionData) { + Canon->DefinitionData = D->DefinitionData; + + // Note that we have deserialized a definition. Any declarations + // deserialized before this one will be be given the DefinitionData + // pointer at the end. + Reader.PendingDefinitions.insert(D); + } else { + // We have already deserialized a definition of this record. This + // definition is no longer really a definition. Note that the pre-existing + // definition is the *real* definition. + // FIXME: Check DefinitionData for consistency with prior definition. + Reader.MergedDeclContexts.insert( + std::make_pair(D, D->getCanonicalDecl()->DefinitionData->Definition)); + D->IsCompleteDefinition = false; + D->DefinitionData = D->getCanonicalDecl()->DefinitionData; + } } else { // Propagate DefinitionData pointer from the canonical declaration. - D->DefinitionData = D->getCanonicalDecl()->DefinitionData; + D->DefinitionData = D->getCanonicalDecl()->DefinitionData; } enum CXXRecKind { @@ -1278,8 +1298,9 @@ ASTDeclReader::VisitCXXRecordDeclImpl(CXXRecordDecl *D) { // Lazily load the key function to avoid deserializing every method so we can // compute it. - if (D->IsCompleteDefinition) { - if (DeclID KeyFn = ReadDeclID(Record, Idx)) + if (WasDefinition) { + DeclID KeyFn = ReadDeclID(Record, Idx); + if (KeyFn && D->IsCompleteDefinition) C.KeyFunctions[D] = KeyFn; } @@ -1502,16 +1523,6 @@ ASTDeclReader::VisitClassTemplateSpecializationDeclImpl( } } - // Explicit info. - if (TypeSourceInfo *TyInfo = GetTypeSourceInfo(Record, Idx)) { - ClassTemplateSpecializationDecl::ExplicitSpecializationInfo *ExplicitInfo - = new (C) ClassTemplateSpecializationDecl::ExplicitSpecializationInfo; - ExplicitInfo->TypeAsWritten = TyInfo; - ExplicitInfo->ExternLoc = ReadSourceLocation(Record, Idx); - ExplicitInfo->TemplateKeywordLoc = ReadSourceLocation(Record, Idx); - D->ExplicitInfo = ExplicitInfo; - } - SmallVector TemplArgs; Reader.ReadTemplateArgumentList(TemplArgs, F, Record, Idx); D->TemplateArgs = TemplateArgumentList::CreateCopy(C, TemplArgs.data(), @@ -1523,16 +1534,48 @@ ASTDeclReader::VisitClassTemplateSpecializationDeclImpl( if (writtenAsCanonicalDecl) { ClassTemplateDecl *CanonPattern = ReadDeclAs(Record,Idx); if (D->isCanonicalDecl()) { // It's kept in the folding set. + // Set this as, or find, the canonical declaration for this specialization + ClassTemplateSpecializationDecl *CanonSpec; if (ClassTemplatePartialSpecializationDecl *Partial = dyn_cast(D)) { - CanonPattern->getCommonPtr()->PartialSpecializations + CanonSpec = CanonPattern->getCommonPtr()->PartialSpecializations .GetOrInsertNode(Partial); } else { - CanonPattern->getCommonPtr()->Specializations.GetOrInsertNode(D); + CanonSpec = + CanonPattern->getCommonPtr()->Specializations.GetOrInsertNode(D); + } + // If there was already a canonical specialization, merge into it. + if (CanonSpec != D) { + mergeRedeclarable(D, CanonSpec, Redecl); + + // This declaration might be a definition. Merge with any existing + // definition. + if (D->DefinitionData) { + if (!CanonSpec->DefinitionData) { + CanonSpec->DefinitionData = D->DefinitionData; + } else { + // FIXME: Check DefinitionData for consistency with prior definition + Reader.PendingDefinitions.erase(D); + Reader.MergedDeclContexts.insert( + std::make_pair(D, CanonSpec->DefinitionData->Definition)); + D->IsCompleteDefinition = false; + D->DefinitionData = CanonSpec->DefinitionData; + } + } } } } + // Explicit info. + if (TypeSourceInfo *TyInfo = GetTypeSourceInfo(Record, Idx)) { + ClassTemplateSpecializationDecl::ExplicitSpecializationInfo *ExplicitInfo + = new (C) ClassTemplateSpecializationDecl::ExplicitSpecializationInfo; + ExplicitInfo->TypeAsWritten = TyInfo; + ExplicitInfo->ExternLoc = ReadSourceLocation(Record, Idx); + ExplicitInfo->TemplateKeywordLoc = ReadSourceLocation(Record, Idx); + D->ExplicitInfo = ExplicitInfo; + } + return Redecl; } @@ -1765,59 +1808,65 @@ ASTDeclReader::VisitRedeclarable(Redeclarable *D) { /// \brief Attempts to merge the given declaration (D) with another declaration /// of the same entity. template -void ASTDeclReader::mergeRedeclarable(Redeclarable *D, +void ASTDeclReader::mergeRedeclarable(Redeclarable *D, RedeclarableResult &Redecl) { // If modules are not available, there is no reason to perform this merge. if (!Reader.getContext().getLangOpts().Modules) return; - - if (FindExistingResult ExistingRes = findExisting(static_cast(D))) { - if (T *Existing = ExistingRes) { - T *ExistingCanon = Existing->getCanonicalDecl(); - T *DCanon = static_cast(D)->getCanonicalDecl(); - if (ExistingCanon != DCanon) { - // Have our redeclaration link point back at the canonical declaration - // of the existing declaration, so that this declaration has the - // appropriate canonical declaration. - D->RedeclLink = Redeclarable::PreviousDeclLink(ExistingCanon); - - // When we merge a namespace, update its pointer to the first namespace. - if (NamespaceDecl *Namespace - = dyn_cast(static_cast(D))) { - Namespace->AnonOrFirstNamespaceAndInline.setPointer( - static_cast(static_cast(ExistingCanon))); - } - - // Don't introduce DCanon into the set of pending declaration chains. - Redecl.suppress(); - - // Introduce ExistingCanon into the set of pending declaration chains, - // if in fact it came from a module file. - if (ExistingCanon->isFromASTFile()) { - GlobalDeclID ExistingCanonID = ExistingCanon->getGlobalID(); - assert(ExistingCanonID && "Unrecorded canonical declaration ID?"); - if (Reader.PendingDeclChainsKnown.insert(ExistingCanonID)) - Reader.PendingDeclChains.push_back(ExistingCanonID); - } - - // If this declaration was the canonical declaration, make a note of - // that. We accept the linear algorithm here because the number of - // unique canonical declarations of an entity should always be tiny. - if (DCanon == static_cast(D)) { - SmallVectorImpl &Merged = Reader.MergedDecls[ExistingCanon]; - if (std::find(Merged.begin(), Merged.end(), Redecl.getFirstID()) - == Merged.end()) - Merged.push_back(Redecl.getFirstID()); - - // If ExistingCanon did not come from a module file, introduce the - // first declaration that *does* come from a module file to the - // set of pending declaration chains, so that we merge this - // declaration. - if (!ExistingCanon->isFromASTFile() && - Reader.PendingDeclChainsKnown.insert(Redecl.getFirstID())) - Reader.PendingDeclChains.push_back(Merged[0]); - } - } + + if (FindExistingResult ExistingRes = findExisting(static_cast(D))) + if (T *Existing = ExistingRes) + mergeRedeclarable(D, Existing, Redecl); +} + +/// \brief Attempts to merge the given declaration (D) with another declaration +/// of the same entity. +template +void ASTDeclReader::mergeRedeclarable(Redeclarable *D, T *Existing, + RedeclarableResult &Redecl) { + T *ExistingCanon = Existing->getCanonicalDecl(); + T *DCanon = static_cast(D)->getCanonicalDecl(); + if (ExistingCanon != DCanon) { + // Have our redeclaration link point back at the canonical declaration + // of the existing declaration, so that this declaration has the + // appropriate canonical declaration. + D->RedeclLink = Redeclarable::PreviousDeclLink(ExistingCanon); + + // When we merge a namespace, update its pointer to the first namespace. + if (NamespaceDecl *Namespace + = dyn_cast(static_cast(D))) { + Namespace->AnonOrFirstNamespaceAndInline.setPointer( + static_cast(static_cast(ExistingCanon))); + } + + // Don't introduce DCanon into the set of pending declaration chains. + Redecl.suppress(); + + // Introduce ExistingCanon into the set of pending declaration chains, + // if in fact it came from a module file. + if (ExistingCanon->isFromASTFile()) { + GlobalDeclID ExistingCanonID = ExistingCanon->getGlobalID(); + assert(ExistingCanonID && "Unrecorded canonical declaration ID?"); + if (Reader.PendingDeclChainsKnown.insert(ExistingCanonID)) + Reader.PendingDeclChains.push_back(ExistingCanonID); + } + + // If this declaration was the canonical declaration, make a note of + // that. We accept the linear algorithm here because the number of + // unique canonical declarations of an entity should always be tiny. + if (DCanon == static_cast(D)) { + SmallVectorImpl &Merged = Reader.MergedDecls[ExistingCanon]; + if (std::find(Merged.begin(), Merged.end(), Redecl.getFirstID()) + == Merged.end()) + Merged.push_back(Redecl.getFirstID()); + + // If ExistingCanon did not come from a module file, introduce the + // first declaration that *does* come from a module file to the + // set of pending declaration chains, so that we merge this + // declaration. + if (!ExistingCanon->isFromASTFile() && + Reader.PendingDeclChainsKnown.insert(Redecl.getFirstID())) + Reader.PendingDeclChains.push_back(Merged[0]); } } } @@ -1992,12 +2041,11 @@ static bool isSameEntity(NamedDecl *X, NamedDecl *Y) { return true; if (isa(X)) { - // FIXME: Deal with merging of template specializations. - // For now, don't merge these; we need to check more than just the name to - // determine if they refer to the same entity. + // No need to handle these here: we merge them when adding them to the + // template. return false; } - + // Compatible tags match. if (TagDecl *TagX = dyn_cast(X)) { TagDecl *TagY = cast(Y); @@ -2044,6 +2092,18 @@ static bool isSameEntity(NamedDecl *X, NamedDecl *Y) { return false; } +/// Find the context in which we should search for previous declarations when +/// looking for declarations to merge. +static DeclContext *getPrimaryContextForMerging(DeclContext *DC) { + if (NamespaceDecl *ND = dyn_cast(DC)) + return ND->getOriginalNamespace(); + + if (CXXRecordDecl *RD = dyn_cast(DC)) + return RD->getDefinition(); + + return 0; +} + ASTDeclReader::FindExistingResult::~FindExistingResult() { if (!AddResult || Existing) return; @@ -2051,11 +2111,10 @@ ASTDeclReader::FindExistingResult::~FindExistingResult() { DeclContext *DC = New->getDeclContext()->getRedeclContext(); if (DC->isTranslationUnit() && Reader.SemaObj) { Reader.SemaObj->IdResolver.tryAddTopLevelDecl(New, New->getDeclName()); - } else if (NamespaceDecl *NS = dyn_cast(DC)) { + } else if (DeclContext *MergeDC = getPrimaryContextForMerging(DC)) { // Add the declaration to its redeclaration context so later merging // lookups will find it. - NS->getFirstDeclaration()->makeDeclVisibleInContextImpl(New, - /*Internal*/true); + MergeDC->makeDeclVisibleInContextImpl(New, /*Internal*/true); } } @@ -2069,9 +2128,6 @@ ASTDeclReader::FindExistingResult ASTDeclReader::findExisting(NamedDecl *D) { } DeclContext *DC = D->getDeclContext()->getRedeclContext(); - if (!DC->isFileContext()) - return FindExistingResult(Reader); - if (DC->isTranslationUnit() && Reader.SemaObj) { IdentifierResolver &IdResolver = Reader.SemaObj->IdResolver; @@ -2104,15 +2160,17 @@ ASTDeclReader::FindExistingResult ASTDeclReader::findExisting(NamedDecl *D) { if (isSameEntity(*I, D)) return FindExistingResult(Reader, D, *I); } - } else if (NamespaceDecl *NS = dyn_cast(DC)) { - DeclContext::lookup_result R = NS->getFirstDeclaration()->noload_lookup(Name); + return FindExistingResult(Reader, D, /*Existing=*/0); + } else if (DeclContext *MergeDC = getPrimaryContextForMerging(DC)) { + DeclContext::lookup_result R = MergeDC->noload_lookup(Name); for (DeclContext::lookup_iterator I = R.begin(), E = R.end(); I != E; ++I) { if (isSameEntity(*I, D)) return FindExistingResult(Reader, D, *I); } + return FindExistingResult(Reader, D, /*Existing=*/0); } - - return FindExistingResult(Reader, D, /*Existing=*/0); + + return FindExistingResult(Reader); } void ASTDeclReader::attachPreviousDecl(Decl *D, Decl *previous) { diff --git a/lib/Serialization/ASTWriterDecl.cpp b/lib/Serialization/ASTWriterDecl.cpp index 8072630941..dc2ed46575 100644 --- a/lib/Serialization/ASTWriterDecl.cpp +++ b/lib/Serialization/ASTWriterDecl.cpp @@ -1158,13 +1158,6 @@ void ASTDeclWriter::VisitClassTemplateSpecializationDecl( Writer.AddTemplateArgumentList(&D->getTemplateInstantiationArgs(), Record); } - // Explicit info. - Writer.AddTypeSourceInfo(D->getTypeAsWritten(), Record); - if (D->getTypeAsWritten()) { - Writer.AddSourceLocation(D->getExternLoc(), Record); - Writer.AddSourceLocation(D->getTemplateKeywordLoc(), Record); - } - Writer.AddTemplateArgumentList(&D->getTemplateArgs(), Record); Writer.AddSourceLocation(D->getPointOfInstantiation(), Record); Record.push_back(D->getSpecializationKind()); @@ -1175,6 +1168,13 @@ void ASTDeclWriter::VisitClassTemplateSpecializationDecl( Writer.AddDeclRef(D->getSpecializedTemplate()->getCanonicalDecl(), Record); } + // Explicit info. + Writer.AddTypeSourceInfo(D->getTypeAsWritten(), Record); + if (D->getTypeAsWritten()) { + Writer.AddSourceLocation(D->getExternLoc(), Record); + Writer.AddSourceLocation(D->getTemplateKeywordLoc(), Record); + } + Code = serialization::DECL_CLASS_TEMPLATE_SPECIALIZATION; } diff --git a/test/Modules/Inputs/cxx-templates-a.h b/test/Modules/Inputs/cxx-templates-a.h index dbf12dacba..37e3426fc0 100644 --- a/test/Modules/Inputs/cxx-templates-a.h +++ b/test/Modules/Inputs/cxx-templates-a.h @@ -24,3 +24,9 @@ template void PerformDelayedLookup(T &t) { template void PerformDelayedLookupInDefaultArgument(T &t, int a = (FoundByADL(T()), 0)) {} template struct RedeclaredAsFriend {}; + +void use_some_template_a() { + SomeTemplate a; + SomeTemplate b, c; + b = c; +} diff --git a/test/Modules/Inputs/cxx-templates-b.h b/test/Modules/Inputs/cxx-templates-b.h index 9bc76d5bba..c495074a93 100644 --- a/test/Modules/Inputs/cxx-templates-b.h +++ b/test/Modules/Inputs/cxx-templates-b.h @@ -35,6 +35,12 @@ struct RedeclareTemplateAsFriend { friend struct RedeclaredAsFriend; }; +void use_some_template_b() { + SomeTemplate a; + SomeTemplate b, c; + b = c; +} + @import cxx_templates_a; template void UseDefinedInBImplIndirectly(T &v) { PerformDelayedLookup(v); diff --git a/test/Modules/Inputs/templates-left.h b/test/Modules/Inputs/templates-left.h index 7451420c74..7c9be8c651 100644 --- a/test/Modules/Inputs/templates-left.h +++ b/test/Modules/Inputs/templates-left.h @@ -27,3 +27,5 @@ void triggerPendingInstantiation() { } void redeclDefinitionEmit(){} + +typedef Outer::Inner OuterIntInner_left; diff --git a/test/Modules/Inputs/templates-right.h b/test/Modules/Inputs/templates-right.h index d3524d3476..bacaa49b60 100644 --- a/test/Modules/Inputs/templates-right.h +++ b/test/Modules/Inputs/templates-right.h @@ -25,3 +25,5 @@ void triggerPendingInstantiationToo() { } void redeclDefinitionEmit(){} + +typedef Outer::Inner OuterIntInner_right; diff --git a/test/Modules/Inputs/templates-top.h b/test/Modules/Inputs/templates-top.h index 5985ee8820..b5d0b6bda1 100644 --- a/test/Modules/Inputs/templates-top.h +++ b/test/Modules/Inputs/templates-top.h @@ -15,3 +15,7 @@ template class A::WhereAmI { public: static void func() {} }; + +template struct Outer { + struct Inner {}; +}; diff --git a/test/Modules/cxx-templates.cpp b/test/Modules/cxx-templates.cpp index 9e6cd17828..f34a2bdaa7 100644 --- a/test/Modules/cxx-templates.cpp +++ b/test/Modules/cxx-templates.cpp @@ -82,6 +82,14 @@ typedef SomeTemplate SomeTemplateIntRef; SomeTemplate some_template_char_ptr; SomeTemplate some_template_char_ref; +void testImplicitSpecialMembers(SomeTemplate &a, + const SomeTemplate &b, + SomeTemplate &c, + const SomeTemplate &d) { + a = b; + c = d; +} + // CHECK-GLOBAL: DeclarationName 'f' // CHECK-GLOBAL-NEXT: |-FunctionTemplate {{.*}} 'f' // CHECK-GLOBAL-NEXT: `-FunctionTemplate {{.*}} 'f' diff --git a/test/Modules/templates.mm b/test/Modules/templates.mm index 1fef967e40..08b216646c 100644 --- a/test/Modules/templates.mm +++ b/test/Modules/templates.mm @@ -32,5 +32,10 @@ void testRedeclDefinition() { redeclDefinitionEmit(); } +// These three are all the same type. +typedef OuterIntInner_left OuterIntInner; +typedef OuterIntInner_right OuterIntInner; +typedef Outer::Inner OuterIntInner; + // CHECK: call {{.*pendingInstantiation}} // CHECK: call {{.*redeclDefinitionEmit}} -- 2.40.0