[Bug c++/17542] New: Visibility attribute ignored when it precedes class head

[Bug c++/17542] New: Visibility attribute ignored when it precedes class head

[austern at apple dot com]

Consider the following file:
__attribute__((visibility("default"))) struct foo { virtual ~foo(); };
struct bar { virtual ~bar(); } __attribute__((visibility("default")));
foo::~foo() { }
bar::~bar() { }

My expectation would be that classes foo and bar would both get default visibility.  That's not what 
happens:
[isolde:tmp]$ /work/root/bin/g++ -fvisibility=hidden -c v5.cc
[isolde:tmp]$ nm -m v5.o
00000208 (__TEXT,__text) external __ZN3barD0Ev
00000000 (absolute) external __ZN3barD0Ev.eh
000001a0 (__TEXT,__text) external __ZN3barD1Ev
00000000 (absolute) external __ZN3barD1Ev.eh
00000138 (__TEXT,__text) external __ZN3barD2Ev
00000000 (absolute) external __ZN3barD2Ev.eh
000000d0 (__TEXT,__text) private external __ZN3fooD0Ev
00000000 (absolute) private external __ZN3fooD0Ev.eh
00000068 (__TEXT,__text) private external __ZN3fooD1Ev
00000000 (absolute) private external __ZN3fooD1Ev.eh
00000000 (__TEXT,__text) private external __ZN3fooD2Ev
00000000 (absolute) private external __ZN3fooD2Ev.eh
00000270 (__DATA,__datacoal_nt) weak external __ZTI3bar
00000280 (__DATA,__datacoal_nt) weak private external __ZTI3foo
00000278 (__DATA,__datacoal_nt) weak external __ZTS3bar
00000288 (__DATA,__datacoal_nt) weak private external __ZTS3foo
000002a0 (__DATA,__const) external __ZTV3bar
00000290 (__DATA,__const) private external __ZTV3foo
         (undefined) external __ZTVN10__cxxabiv117__class_type_infoE
         (undefined [lazy bound]) external __ZdlPv
         (undefined) external dyld_stub_binding_helper
[isolde:tmp]$ 

This is certainly wrong.  If putting the visibility attribute at the beginning of the class definition is 
syntactly valid, then that visibility should be respected.  If this is a syntax error then it should be 
diagnosed as such.  Silently ignoring the attribute is wrong.

-- 
           Summary: Visibility attribute ignored when it precedes class head
           Product: gcc
           Version: 4.0.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P2
         Component: c++
        AssignedTo: unassigned at gcc dot gnu dot org
        ReportedBy: austern at apple dot com
                CC: gcc-bugs at gcc dot gnu dot org
 GCC build triplet: powerpc-apple-darwin7.5.0
  GCC host triplet: powerpc-apple-darwin7.5.0
GCC target triplet: powerpc-apple-darwin7.5.0


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=17542

[Bug c++/17542] Visibility attribute ignored when it precedes class head

[pinskia at gcc dot gnu dot org]

------- Additional Comments From pinskia at gcc dot gnu dot org  2004-09-18 04:36 -------
Confirmed.

-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
     Ever Confirmed|                            |1
   Last reconfirmed|0000-00-00 00:00:00         |2004-09-18 04:36:53
               date|                            |


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=17542

[Bug c++/17542] Visibility attribute ignored when it precedes class head

[austern at apple dot com]

------- Additional Comments From austern at apple dot com  2004-10-01 23:22 -------
Actually, this is almost straightforward.  It has nothing to do with the visibility attribute: it has to do 
with attributes and C++ classes in general.  Looking at cp_parser_class, and especially at 
cp_parser_class_head, attributes can appear in one of two places.  The parser will recognize either
 struct __attribute__((visibility("hidden"))) foo { virtual ~foo(); };
or
  struct foo { virtual ~foo(); } __attribute__((visibility("hidden")));

But, as the code and the comments both make quite clear, the syntax we're recognizing does not 
include an attribute list before the class-key.  

So then how come the __atrtribute__ is being swallowed and ignored?  Answer: what we've got here is a 
simple-declaration with two decl-specifiers, an attribute list and a class definition, and no declarators.  
The attribute list applies to a declarator, which in this case is missing.  We could instead have written:
  __attribute__((visibility("hidden"))) struct foo { virtual ~foo() { } } x;
In this case we can see that the attribute isn't being ignored; it just applies to x, not to foo.

I hesitate to call this "behaves correctly", since this behavior is unexpected, hard to understand, and 
leads to the user silently not getting what they expected.  I'm afraid that with visibility, in particular, it'll 
lead to problems because users will want to hide this attribute list behind macros that expand to 
different things on different platforms.  But I'm also not completely sure what the best thing to do is.  
Here are my two two choices:
 1. Special-case this construct.  If a simple-declaration consists of a class definition with no declarator, 
then any attributes preceding the class head get applied to the class.
 2. If cp_parser_simple_declaration collects attributes in  cp_parser_decl_specifier_seq and it's throwing 
them away because there's no declarator to apply them to, then warn the user and suggest a better 
place to put the attribute list.

Option 1 is admittedly a hack, but that doesn't necessarily mean it's a bad idea.

-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=17542

[Bug c++/17542] Visibility attribute ignored when it precedes class head

[jsm at polyomino dot org dot uk]

------- Additional Comments From jsm at polyomino dot org dot uk  2004-10-01 23:29 -------
Subject: Re:  Visibility attribute ignored when it precedes
 class head

On Fri, 1 Oct 2004, austern at apple dot com wrote:

> I hesitate to call this "behaves correctly", since this behavior is 
> unexpected, hard to understand, and leads to the user silently not 
> getting what they expected.  I'm afraid that with visibility, in 
> particular, it'll lead to problems because users will want to hide this 
> attribute list behind macros that expand to different things on 
> different platforms.  But I'm also not completely sure what the best 
> thing to do is.

It is at least documented to some extent ("Attribute Syntax"), although 
with a warning that C++ may vary from C.

>  2. If cp_parser_simple_declaration collects attributes in 
> cp_parser_decl_specifier_seq and it's throwing them away because there's 
> no declarator to apply them to, then warn the user and suggest a better 
> place to put the attribute list.

FWIW, I've been considering such a warning for C, to go along with the 
warnings for useless type qualifiers and storage class specifiers on empty 
declarations.



-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=17542

[Bug c++/17542] Visibility attribute ignored when it precedes class head

[cvs-commit at gcc dot gnu dot org]

------- Additional Comments From cvs-commit at gcc dot gnu dot org  2004-10-30 21:17 -------
Subject: Bug 17542

CVSROOT:	/cvs/gcc
Module name:	gcc
Changes by:	austern@gcc.gnu.org	2004-10-30 21:17:32

Modified files:
	gcc/cp         : ChangeLog cp-tree.h decl.c error.c 
	gcc/testsuite  : ChangeLog 
Added files:
	gcc/testsuite/g++.dg/ext: attrib18.C 

Log message:
	PR c++/17542
	* cp-tree.h (class_key_or_enum_as_string): Declare.
	* error.c (class_key_or_enum): Rename to class_key_or_enum_as_string
	and remove static qualifier.
	* decl.c (shadow_tag): Warn about ignored attributes in class/struct/
	union/enum declaration.
	* g++.dg/ext/attrib18.C: New test.

Patches:
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/cp/ChangeLog.diff?cvsroot=gcc&r1=1.4465&r2=1.4466
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/cp/cp-tree.h.diff?cvsroot=gcc&r1=1.1067&r2=1.1068
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/cp/decl.c.diff?cvsroot=gcc&r1=1.1322&r2=1.1323
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/cp/error.c.diff?cvsroot=gcc&r1=1.268&r2=1.269
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/ChangeLog.diff?cvsroot=gcc&r1=1.4524&r2=1.4525
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/g++.dg/ext/attrib18.C.diff?cvsroot=gcc&r1=NONE&r2=1.1



-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=17542

[Bug c++/17542] Visibility attribute ignored when it precedes class head

[giovannibajo at libero dot it]

------- Additional Comments From giovannibajo at libero dot it  2004-10-30 21:19 -------
Fixed.

-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|                            |FIXED
   Target Milestone|---                         |4.0.0


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=17542