Hi Rafael, I have a reduced repro for the regression caused by this CL (as mentioned in IRC). I attached it to http://llvm.org/PR12835
Nico On Tue, May 15, 2012 at 7:09 AM, Rafael Espindola < [email protected]> wrote: > Author: rafael > Date: Tue May 15 09:09:55 2012 > New Revision: 156821 > > URL: http://llvm.org/viewvc/llvm-project?rev=156821&view=rev > Log: > Fix our handling of visibility in explicit template instantiations. > > * Don't copy the visibility attribute during instantiations. We have to be > able > to distinguish > > struct HIDDEN foo {}; > template<class T> > DEFAULT void bar() {} > template DEFAULT void bar<foo>(); > > from > > struct HIDDEN foo {}; > template<class T> > DEFAULT void bar() {} > template void bar<foo>(); > > * If an instantiation has an attribute, it takes precedence over an > attribute > in the template. > > * With instantiation attributes handled with the above logic, we can now > select the minimum visibility when looking at template arguments. > > Modified: > cfe/trunk/include/clang/Basic/Attr.td > cfe/trunk/lib/AST/Decl.cpp > cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp > cfe/trunk/test/CodeGenCXX/visibility.cpp > cfe/trunk/utils/TableGen/ClangAttrEmitter.cpp > > Modified: cfe/trunk/include/clang/Basic/Attr.td > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Attr.td?rev=156821&r1=156820&r2=156821&view=diff > > ============================================================================== > --- cfe/trunk/include/clang/Basic/Attr.td (original) > +++ cfe/trunk/include/clang/Basic/Attr.td Tue May 15 09:09:55 2012 > @@ -93,6 +93,9 @@ > list<string> Namespaces = []; > // Set to true for attributes with arguments which require delayed > parsing. > bit LateParsed = 0; > + // Set to false to prevent an attribute from being propagated from a > template > + // to the instantiation. > + bit Clone = 1; > // Set to true for attributes which must be instantiated within templates > bit TemplateDependent = 0; > // Set to true for attributes that have a corresponding AST node. > @@ -660,6 +663,7 @@ > } > > def Visibility : InheritableAttr { > + let Clone = 0; > let Spellings = ["visibility"]; > let Args = [EnumArgument<"Visibility", "VisibilityType", > ["default", "hidden", "internal", "protected"], > > Modified: cfe/trunk/lib/AST/Decl.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/Decl.cpp?rev=156821&r1=156820&r2=156821&view=diff > > ============================================================================== > --- cfe/trunk/lib/AST/Decl.cpp (original) > +++ cfe/trunk/lib/AST/Decl.cpp Tue May 15 09:09:55 2012 > @@ -158,14 +158,12 @@ > return getLVForTemplateArgumentList(TArgs.data(), TArgs.size(), > OnlyTemplate); > } > > -static bool shouldConsiderTemplateLV(const FunctionDecl *fn, > - const FunctionTemplateSpecializationInfo > *spec) { > - return !(spec->isExplicitSpecialization() && > - fn->hasAttr<VisibilityAttr>()); > +static bool shouldConsiderTemplateLV(const FunctionDecl *fn) { > + return !fn->hasAttr<VisibilityAttr>(); > } > > static bool shouldConsiderTemplateLV(const > ClassTemplateSpecializationDecl *d) { > - return !(d->isExplicitSpecialization() && d->hasAttr<VisibilityAttr>()); > + return !d->hasAttr<VisibilityAttr>(); > } > > static LinkageInfo getLVForNamespaceScopeDecl(const NamedDecl *D, > @@ -376,7 +374,7 @@ > // this is an explicit specialization with a visibility attribute. > if (FunctionTemplateSpecializationInfo *specInfo > = > Function->getTemplateSpecializationInfo()) { > - if (shouldConsiderTemplateLV(Function, specInfo)) { > + if (shouldConsiderTemplateLV(Function)) { > LV.merge(getLVForDecl(specInfo->getTemplate(), > true)); > const TemplateArgumentList &templateArgs = > *specInfo->TemplateArguments; > @@ -407,8 +405,8 @@ > > // The arguments at which the template was instantiated. > const TemplateArgumentList &TemplateArgs = spec->getTemplateArgs(); > - LV.merge(getLVForTemplateArgumentList(TemplateArgs, > - OnlyTemplate)); > + LV.mergeWithMin(getLVForTemplateArgumentList(TemplateArgs, > + OnlyTemplate)); > } > } > > @@ -527,7 +525,7 @@ > // the template parameters and arguments. > if (FunctionTemplateSpecializationInfo *spec > = MD->getTemplateSpecializationInfo()) { > - if (shouldConsiderTemplateLV(MD, spec)) { > + if (shouldConsiderTemplateLV(MD)) { > > LV.mergeWithMin(getLVForTemplateArgumentList(*spec->TemplateArguments, > OnlyTemplate)); > if (!OnlyTemplate) > > Modified: cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp?rev=156821&r1=156820&r2=156821&view=diff > > ============================================================================== > --- cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp (original) > +++ cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp Tue May 15 09:09:55 > 2012 > @@ -102,7 +102,8 @@ > } else { > Attr *NewAttr = sema::instantiateTemplateAttribute(TmplAttr, Context, > *this, > TemplateArgs); > - New->addAttr(NewAttr); > + if (NewAttr) > + New->addAttr(NewAttr); > } > } > } > > Modified: cfe/trunk/test/CodeGenCXX/visibility.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/visibility.cpp?rev=156821&r1=156820&r2=156821&view=diff > > ============================================================================== > --- cfe/trunk/test/CodeGenCXX/visibility.cpp (original) > +++ cfe/trunk/test/CodeGenCXX/visibility.cpp Tue May 15 09:09:55 2012 > @@ -712,3 +712,55 @@ > // CHECK: define weak_odr hidden void > @_ZN6test363fooINS_2S1ENS_2S2EE3barEv > // CHECK-HIDDEN: define weak_odr hidden void > @_ZN6test363fooINS_2S1ENS_2S2EE3barEv > } > + > +namespace test37 { > + struct HIDDEN foo { > + }; > + template<class T> > + DEFAULT void bar() {} > + template DEFAULT void bar<foo>(); > + // CHECK: define weak_odr void @_ZN6test373barINS_3fooEEEvv > + // CHECK-HIDDEN: define weak_odr void @_ZN6test373barINS_3fooEEEvv > +} > + > +namespace test38 { > + template<typename T> > + class DEFAULT foo { > + void bar() {} > + }; > + struct HIDDEN zed { > + }; > + template class foo<zed>; > + // CHECK: define weak_odr hidden void @_ZN6test383fooINS_3zedEE3barEv > + // CHECK-HIDDEN: define weak_odr hidden void > @_ZN6test383fooINS_3zedEE3barEv > +} > + > +namespace test39 { > + class DEFAULT default_t; > + class HIDDEN hidden_t; > + template <class T> class A { > + template <class U> class B { > + HIDDEN void hidden() {} > + void noattr() {} > + template <class V> void temp() {} > + }; > + }; > + template class DEFAULT A<hidden_t>; > + template class DEFAULT A<hidden_t>::B<hidden_t>; > + template void A<hidden_t>::B<hidden_t>::temp<default_t>(); > + template void A<hidden_t>::B<hidden_t>::temp<hidden_t>(); > + > + // CHECK: define weak_odr hidden void > @_ZN6test391AINS_8hidden_tEE1BIS1_E6hiddenEv > + // CHECK: define weak_odr void > @_ZN6test391AINS_8hidden_tEE1BIS1_E6noattrEv > + // CHECK: define weak_odr void > @_ZN6test391AINS_8hidden_tEE1BIS1_E4tempINS_9default_tEEEvv > + > + // GCC produces a default for this one. Why? > + // CHECK: define weak_odr hidden void > @_ZN6test391AINS_8hidden_tEE1BIS1_E4tempIS1_EEvv > + > + // CHECK-HIDDEN: define weak_odr hidden void > @_ZN6test391AINS_8hidden_tEE1BIS1_E6hiddenEv > + // CHECK-HIDDEN: define weak_odr void > @_ZN6test391AINS_8hidden_tEE1BIS1_E6noattrEv > + // CHECK-HIDDEN: define weak_odr void > @_ZN6test391AINS_8hidden_tEE1BIS1_E4tempINS_9default_tEEEvv > + > + // GCC produces a default for this one. Why? > + // CHECK-HIDDEN: define weak_odr hidden void > @_ZN6test391AINS_8hidden_tEE1BIS1_E4tempIS1_EEvv > +} > > Modified: cfe/trunk/utils/TableGen/ClangAttrEmitter.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/utils/TableGen/ClangAttrEmitter.cpp?rev=156821&r1=156820&r2=156821&view=diff > > ============================================================================== > --- cfe/trunk/utils/TableGen/ClangAttrEmitter.cpp (original) > +++ cfe/trunk/utils/TableGen/ClangAttrEmitter.cpp Tue May 15 09:09:55 2012 > @@ -1004,6 +1004,14 @@ > continue; > > OS << " case attr::" << R.getName() << ": {\n"; > + bool ShouldClone = R.getValueAsBit("Clone"); > + > + if (!ShouldClone) { > + OS << " return NULL;\n"; > + OS << " }\n"; > + continue; > + } > + > OS << " const " << R.getName() << "Attr *A = cast<" > << R.getName() << "Attr>(At);\n"; > bool TDependent = R.getValueAsBit("TemplateDependent"); > > > _______________________________________________ > cfe-commits mailing list > [email protected] > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
