| Message ID | 20170422204640.47837-1-abbeyj@gmail.com |
|---|---|
| State | New |
| Headers | show |
On Sat, Apr 22, 2017 at 4:46 PM, James Abbatiello <abbeyj@gmail.com> wrote: > This is my first time attempting a contribution here so please point > out any mistakes. I've tested this on x86_64-pc-linux-gnu in a VM. Hello, It has been a few weeks. Can anybody give me some feedback here or tell me if I've done something wrong? https://gcc.gnu.org/ml/gcc-patches/2017-04/msg00975.html
On Sat, Apr 22, 2017 at 4:46 PM, James Abbatiello <abbeyj@gmail.com> wrote: > PR c++/77306 > * attribs.c (decl_attributes): Allow visibility attributes on explicit > template instantiations. > * gcc-family/c-attribs.c (handle_visibility_attribute): Likewise. This doesn't seem sufficient; any members that have already been instantiated will still have the default visibility. If you want to go this way, you'll need to call reset_type_linkage when changing the visibility of a type that's already defined. Is there a reason you can't declare the visibility before the first use of A<double>, e.g. extern template struct __attribute ((visibility("hidden"))) A<double>; followed later by template class A<double>; ? Jason
On 05/21/2017 11:47 PM, James Abbatiello wrote: > On Sat, Apr 22, 2017 at 4:46 PM, James Abbatiello <abbeyj@gmail.com> wrote: >> This is my first time attempting a contribution here so please point >> out any mistakes. I've tested this on x86_64-pc-linux-gnu in a VM. > > Hello, > > It has been a few weeks. Can anybody give me some feedback here or > tell me if I've done something wrong? > https://gcc.gnu.org/ml/gcc-patches/2017-04/msg00975.html You're changing the behaviour for all structures -- including the C FE. That seems wrong. Also, AFAICT, youre not changing the visibility for members of the struct. Which also seems incorrect. nathan
diff --git a/gcc/attribs.c b/gcc/attribs.c index 55b21271b39..ed1cfa1765b 100644 --- a/gcc/attribs.c +++ b/gcc/attribs.c @@ -534,7 +534,8 @@ decl_attributes (tree *node, tree attributes, int flags) if (TYPE_P (*anode) && (flags & (int) ATTR_FLAG_TYPE_IN_PLACE) - && TYPE_SIZE (*anode) != NULL_TREE) + && TYPE_SIZE (*anode) != NULL_TREE + && !is_attribute_p ("visibility", name)) { warning (OPT_Wattributes, "type attributes ignored after type is already defined"); continue; diff --git a/gcc/c-family/c-attribs.c b/gcc/c-family/c-attribs.c index f2a88e147ba..e3fb82aef21 100644 --- a/gcc/c-family/c-attribs.c +++ b/gcc/c-family/c-attribs.c @@ -1895,12 +1895,6 @@ handle_visibility_attribute (tree *node, tree name, tree args, name); return NULL_TREE; } - else if (TYPE_FIELDS (*node)) - { - error ("%qE attribute ignored because %qT is already defined", - name, *node); - return NULL_TREE; - } } else if (decl_function_context (decl) != 0 || !TREE_PUBLIC (decl)) { diff --git a/gcc/testsuite/g++.dg/ext/visibility/warn4.C b/gcc/testsuite/g++.dg/ext/visibility/warn4.C index 33e6f678592..a55f9682a12 100644 --- a/gcc/testsuite/g++.dg/ext/visibility/warn4.C +++ b/gcc/testsuite/g++.dg/ext/visibility/warn4.C @@ -1,10 +1,11 @@ -// Warn if we try to give an instantiation visibility after it's already +// Don't warn if we give an instantiation visibility after it's already // been instantiated. // { dg-require-visibility "" } +// { dg-final { scan-hidden "_ZN1AIdE1fEd" } } template <class T> struct A { void f (T); }; template <class T> void A<T>::f (T) { } A<double> ad; -template struct __attribute ((visibility ("hidden"))) A<double>; // { dg-warning "already defined" } +template struct __attribute ((visibility ("hidden"))) A<double>;