[cfe-commits] r156821 - in /cfe/trunk: include/clang/Basic/Attr.td lib/AST/Decl.cpp lib/Sema/SemaTemplateInstantiateDecl.cpp test/CodeGenCXX/visibility.cpp utils/TableGen/ClangAttrEmitter.cpp

Nico Weber thakis at chromium.org
Tue May 15 13:09:40 PDT 2012


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 <
rafael.espindola at gmail.com> 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
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120515/59ea14d9/attachment.html>


More information about the cfe-commits mailing list