From 63469422c403a866157a612f2f7f97c72dcd6de3 Mon Sep 17 00:00:00 2001 From: Axel Naumann Date: Tue, 2 Oct 2012 09:09:43 +0000 Subject: [PATCH] Merge pending instantiations instead of overwriting existing ones. Check whether a pending instantiation needs to be instantiated (or whether an instantiation already exists). Verify the size of the PendingInstantiations record (was only checking size of existing PendingInstantiations). Migrate Obj-C++ part of redecl-merge into separate test, now that this is growing. templates.mm: test that CodeGen has seen exactly one definition of template instantiations. redecl-merge.m: use "@" specifier for expected-diagnostics. llvm-svn: 164993 --- clang/include/clang/Serialization/ASTReader.h | 6 ++- clang/lib/Serialization/ASTReader.cpp | 16 ++++--- clang/lib/Serialization/ASTReaderDecl.cpp | 59 ++++++++++++++++++++++++- clang/test/Modules/Inputs/module.map | 12 +++++ clang/test/Modules/Inputs/redecl-merge-bottom.h | 8 ---- clang/test/Modules/Inputs/redecl-merge-left.h | 21 --------- clang/test/Modules/Inputs/redecl-merge-right.h | 20 --------- clang/test/Modules/Inputs/redecl-merge-top.h | 9 ---- clang/test/Modules/Inputs/templates-left.h | 27 +++++++++++ clang/test/Modules/Inputs/templates-right.h | 25 +++++++++++ clang/test/Modules/Inputs/templates-top.h | 6 +++ clang/test/Modules/redecl-merge.m | 26 +++-------- clang/test/Modules/templates.mm | 28 ++++++++++++ 13 files changed, 178 insertions(+), 85 deletions(-) create mode 100644 clang/test/Modules/Inputs/templates-left.h create mode 100644 clang/test/Modules/Inputs/templates-right.h create mode 100644 clang/test/Modules/Inputs/templates-top.h create mode 100644 clang/test/Modules/templates.mm diff --git a/clang/include/clang/Serialization/ASTReader.h b/clang/include/clang/Serialization/ASTReader.h index 51db35e..e87c93f 100644 --- a/clang/include/clang/Serialization/ASTReader.h +++ b/clang/include/clang/Serialization/ASTReader.h @@ -687,7 +687,7 @@ private: /// Objective-C protocols. std::deque InterestingDecls; - /// \brief The set of redeclarable declaraations that have been deserialized + /// \brief The set of redeclarable declarations that have been deserialized /// since the last time the declaration chains were linked. llvm::SmallPtrSet RedeclsDeserialized; @@ -854,6 +854,10 @@ private: void finishPendingActions(); + /// \brief Whether D needs to be instantiated, i.e. whether an instantiation + /// for D does not exist yet. + bool needPendingInstantiation(ValueDecl* D) const; + /// \brief Produce an error diagnostic and return true. /// /// This routine should only be used for fatal errors that have to diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp index 639ef93..a897d86 100644 --- a/clang/lib/Serialization/ASTReader.cpp +++ b/clang/lib/Serialization/ASTReader.cpp @@ -2223,13 +2223,15 @@ ASTReader::ReadASTBlock(ModuleFile &F) { case PENDING_IMPLICIT_INSTANTIATIONS: if (PendingInstantiations.size() % 2 != 0) { + Error("Invalid existing PendingInstantiations"); + return Failure; + } + + if (Record.size() % 2 != 0) { Error("Invalid PENDING_IMPLICIT_INSTANTIATIONS block"); return Failure; } - - // Later lists of pending instantiations overwrite earlier ones. - // FIXME: This is most certainly wrong for modules. - PendingInstantiations.clear(); + for (unsigned I = 0, N = Record.size(); I != N; /* in loop */) { PendingInstantiations.push_back(getGlobalDeclID(F, Record[I++])); PendingInstantiations.push_back( @@ -5592,7 +5594,11 @@ void ASTReader::ReadPendingInstantiations( ValueDecl *D = cast(GetDecl(PendingInstantiations[Idx++])); SourceLocation Loc = SourceLocation::getFromRawEncoding(PendingInstantiations[Idx++]); - Pending.push_back(std::make_pair(D, Loc)); + + // For modules, find out whether an instantiation already exists + if (!getContext().getLangOpts().Modules + || needPendingInstantiation(D)) + Pending.push_back(std::make_pair(D, Loc)); } PendingInstantiations.clear(); } diff --git a/clang/lib/Serialization/ASTReaderDecl.cpp b/clang/lib/Serialization/ASTReaderDecl.cpp index fb4192f..85740de 100644 --- a/clang/lib/Serialization/ASTReaderDecl.cpp +++ b/clang/lib/Serialization/ASTReaderDecl.cpp @@ -2156,7 +2156,7 @@ Decl *ASTReader::ReadDeclRecord(DeclID ID) { // loading, and some declarations may still be initializing. if (isConsumerInterestedIn(D)) InterestingDecls.push_back(D); - + return D; } @@ -2504,3 +2504,60 @@ void ASTDeclReader::UpdateDecl(Decl *D, ModuleFile &ModuleFile, } } } + +/// \brief Return a template specialization of ND (should be a TemplateDecl) +/// that matches FD or TD. +static NamedDecl* findMatchingSpecialization(FunctionDecl* FD, + ClassTemplateSpecializationDecl*TD, + NamedDecl* ND) { + TemplateDecl* Templt = dyn_cast(ND); + if (!Templt) return 0; + if (FD) { + FunctionTemplateDecl* FTD = dyn_cast(Templt); + if (!FTD) return 0; + const TemplateArgumentList* TmpltArgs = FD->getTemplateSpecializationArgs(); + assert(TmpltArgs || "Template without arguments"); + void* InsertionPoint; + return FTD->findSpecialization(TmpltArgs->data(), TmpltArgs->size(), + InsertionPoint); + } else { + ClassTemplateDecl* CTD = dyn_cast(Templt); + if (!CTD) return 0; + const TemplateArgumentList& TmpltArgs = TD->getTemplateArgs(); + void* InsertionPoint; + return CTD->findSpecialization(TmpltArgs.data(), TmpltArgs.size(), + InsertionPoint); + } + return 0; +} + +/// \brief Find out whether an instantiation (outside the module) already exists +bool ASTReader::needPendingInstantiation(ValueDecl* D) const { + DeclContext *DC = D->getDeclContext()->getRedeclContext(); + DeclarationName Name = D->getDeclName(); + assert(Name && "unnamed template"); + + FunctionDecl* FD = dyn_cast(D); + ClassTemplateSpecializationDecl* CD + = FD ? 0 : dyn_cast(D); + + NamedDecl* FoundSpecialization = 0; + if (DC->isTranslationUnit() && SemaObj) { + IdentifierResolver &IdResolver = SemaObj->IdResolver; + for (IdentifierResolver::iterator I = IdResolver.begin(Name), + IEnd = IdResolver.end(); + I != IEnd && !FoundSpecialization; ++I) + FoundSpecialization = findMatchingSpecialization(FD, CD, *I); + } else { + // templates are redeclarables, i.e. they must have been merged into + // the primary context. Use localUncachedLookup to not pick up template + // decls from modules again. + llvm::SmallVector Results; + DC->getPrimaryContext()->localUncachedLookup(Name, Results); + for (llvm::SmallVector::const_iterator + I = Results.begin(), E = Results.end(); + I != E && FoundSpecialization; ++I) + FoundSpecialization = findMatchingSpecialization(FD, CD, *I); + } + return FoundSpecialization && isSameEntity(FoundSpecialization, D); +} diff --git a/clang/test/Modules/Inputs/module.map b/clang/test/Modules/Inputs/module.map index 79056cb..9044922 100644 --- a/clang/test/Modules/Inputs/module.map +++ b/clang/test/Modules/Inputs/module.map @@ -78,6 +78,18 @@ module namespaces_right { header "namespaces-right.h" export * } +module templates_top { + header "templates-top.h" + export * +} +module templates_left { + header "templates-left.h" + export * +} +module templates_right { + header "templates-right.h" + export * +} module MethodPoolA { header "MethodPoolA.h" } diff --git a/clang/test/Modules/Inputs/redecl-merge-bottom.h b/clang/test/Modules/Inputs/redecl-merge-bottom.h index 40a9404..cfea7dc 100644 --- a/clang/test/Modules/Inputs/redecl-merge-bottom.h +++ b/clang/test/Modules/Inputs/redecl-merge-bottom.h @@ -18,11 +18,3 @@ struct S3; void refers_to_C4(C4*); -#ifdef __cplusplus -template class Vector; - -template class Vector; - -template class Vector; -#endif - diff --git a/clang/test/Modules/Inputs/redecl-merge-left.h b/clang/test/Modules/Inputs/redecl-merge-left.h index 1f5da4f..ee794ff 100644 --- a/clang/test/Modules/Inputs/redecl-merge-left.h +++ b/clang/test/Modules/Inputs/redecl-merge-left.h @@ -78,27 +78,6 @@ extern float var2; extern double var3; -#ifdef __cplusplus -template class Vector; - -template class Vector; - -template class List; -template<> class List { -public: - void push_back(int); -}; -namespace N { - template class Set; -} -namespace N { - template class Set { - public: - void insert(T); - }; -} -#endif - // Make sure this doesn't introduce an ambiguity-creating 'id' at the // top level. typedef void funcptr_with_id(int id); diff --git a/clang/test/Modules/Inputs/redecl-merge-right.h b/clang/test/Modules/Inputs/redecl-merge-right.h index 2e05c03..f62020f 100644 --- a/clang/test/Modules/Inputs/redecl-merge-right.h +++ b/clang/test/Modules/Inputs/redecl-merge-right.h @@ -78,26 +78,6 @@ extern int var2; static double var3; -#ifdef __cplusplus -template class Vector { -public: - void push_back(const T&); -}; - -template class List; -template<> class List { -public: - void push_back(int); -}; - -namespace N { - template class Set { - public: - void insert(T); - }; -} -#endif - int ONE; @__experimental_modules_import redecl_merge_top.Explicit; const int one = ONE; diff --git a/clang/test/Modules/Inputs/redecl-merge-top.h b/clang/test/Modules/Inputs/redecl-merge-top.h index 7053936b..25456de 100644 --- a/clang/test/Modules/Inputs/redecl-merge-top.h +++ b/clang/test/Modules/Inputs/redecl-merge-top.h @@ -14,12 +14,3 @@ struct S1; struct S2; struct S2; - -#ifdef __cplusplus -template class Vector; - -template class List { -public: - void push_back(T); -}; -#endif diff --git a/clang/test/Modules/Inputs/templates-left.h b/clang/test/Modules/Inputs/templates-left.h new file mode 100644 index 0000000..7f50cd4 --- /dev/null +++ b/clang/test/Modules/Inputs/templates-left.h @@ -0,0 +1,27 @@ +@__experimental_modules_import templates_top; + +template class Vector; + +template class Vector; + +template class List; +template<> class List { +public: + void push_back(int); +}; +namespace N { + template class Set; +} +namespace N { + template class Set { + public: + void insert(T); + }; +} + +template +void pendingInstantiation(T) {} +void triggerPendingInstantiation() { + pendingInstantiation(12); + pendingInstantiation(42.); +} diff --git a/clang/test/Modules/Inputs/templates-right.h b/clang/test/Modules/Inputs/templates-right.h new file mode 100644 index 0000000..f5487fb --- /dev/null +++ b/clang/test/Modules/Inputs/templates-right.h @@ -0,0 +1,25 @@ +@__experimental_modules_import templates_top; + +template class Vector { +public: + void push_back(const T&); +}; + +template class List; +template<> class List { +public: + void push_back(int); +}; + +namespace N { + template class Set { + public: + void insert(T); + }; +} + +template +void pendingInstantiation(T) {} +void triggerPendingInstantiationToo() { + pendingInstantiation(12); +} diff --git a/clang/test/Modules/Inputs/templates-top.h b/clang/test/Modules/Inputs/templates-top.h new file mode 100644 index 0000000..80ecf23 --- /dev/null +++ b/clang/test/Modules/Inputs/templates-top.h @@ -0,0 +1,6 @@ +template class Vector; + +template class List { +public: + void push_back(T); +}; diff --git a/clang/test/Modules/redecl-merge.m b/clang/test/Modules/redecl-merge.m index 96b0b47..d722414 100644 --- a/clang/test/Modules/redecl-merge.m +++ b/clang/test/Modules/redecl-merge.m @@ -1,6 +1,5 @@ // RUN: rm -rf %t // RUN: %clang_cc1 -fmodules -fmodule-cache-path %t -I %S/Inputs %s -verify -Wno-objc-root-class -// RUN: %clang_cc1 -x objective-c++ -fmodules -fmodule-cache-path %t -I %S/Inputs %s -verify -Wno-objc-root-class @class C2; @class C3; @class C3; @@ -57,26 +56,26 @@ void testTagMerge() { void testTypedefMerge(int i, double d) { T1 *ip = &i; - // in other file: expected-note{{candidate found by name lookup is 'T2'}} // FIXME: Typedefs aren't actually merged in the sense of other merges, because // we should only merge them when the types are identical. - // in other file: expected-note{{candidate found by name lookup is 'T2'}} - // in other file: expected-note{{candidate function}} + // in other file: expected-note@60{{candidate found by name lookup is 'T2'}} + // in other file: expected-note@63{{candidate found by name lookup is 'T2'}} T2 *dp = &d; // expected-error{{reference to 'T2' is ambiguous}} } void testFuncMerge(int i) { func0(i); - // in other file: expected-note{{candidate function}} func1(i); + // in other file: expected-note@64{{candidate function}} + // in other file: expected-note@70{{candidate function}} func2(i); // expected-error{{call to 'func2' is ambiguous}} } void testVarMerge(int i) { var1 = i; - // in other files: expected-note 2{{candidate found by name lookup is 'var2'}} + // in other files: expected-note@77 2{{candidate found by name lookup is 'var2'}} var2 = i; // expected-error{{reference to 'var2' is ambiguous}} - // in other files: expected-note 2{{candidate found by name lookup is 'var3'}} + // in other files: expected-note@79 2{{candidate found by name lookup is 'var3'}} var3 = i; // expected-error{{reference to 'var3' is ambiguous}} } @@ -146,19 +145,6 @@ void g(A *a) { id p4; id p3; -#ifdef __cplusplus -void testVector() { - Vector vec_int; - vec_int.push_back(0); - - List list_bool; - list_bool.push_back(false); - - N::Set set_char; - set_char.insert('A'); -} -#endif - // Make sure we don't get conflicts with 'id'. funcptr_with_id fid; id id_global; diff --git a/clang/test/Modules/templates.mm b/clang/test/Modules/templates.mm new file mode 100644 index 0000000..a9b4591 --- /dev/null +++ b/clang/test/Modules/templates.mm @@ -0,0 +1,28 @@ +// RUN: rm -rf %t +// RUN: %clang_cc1 -x objective-c++ -fmodules -fmodule-cache-path %t -I %S/Inputs -verify %s -Wno-objc-root-class +// RUN: %clang_cc1 -x objective-c++ -fmodules -fmodule-cache-path %t -I %S/Inputs -emit-llvm %s -o - -Wno-objc-root-class | grep pendingInstantiation | FileCheck %s + +@__experimental_modules_import templates_left; +@__experimental_modules_import templates_right; + + +void testTemplateClasses() { + Vector vec_int; + vec_int.push_back(0); + + List list_bool; + list_bool.push_back(false); + + N::Set set_char; + set_char.insert('A'); +} + +void testPendingInstantiations() { + // CHECK: call + // CHECK: call + // CHECK: {{define .*pendingInstantiation.*[(]i}} + // CHECK: {{define .*pendingInstantiation.*[(]double}} + // CHECK: call + triggerPendingInstantiation(); + triggerPendingInstantiationToo(); +} -- 2.7.4