Bug 39159 - unhelpful attribute warning on matching declaration after definition
Summary: unhelpful attribute warning on matching declaration after definition
Status: NEW
Alias: None
Product: gcc
Classification: Unclassified
Component: c++ (show other bugs)
Version: 4.3.1
: P3 normal
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords: diagnostic
Depends on:
Blocks:
 
Reported: 2009-02-11 23:50 UTC by Martin Sebor
Modified: 2021-09-13 00:08 UTC (History)
3 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2014-08-16 00:00:00


Attachments
Proposed patch to silence the warning if the new attribute is same (483 bytes, patch)
2016-04-06 12:43 UTC, Gert
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Sebor 2009-02-11 23:50:02 UTC
gcc 4.3 issues a warning for the declaration of struct A below despite the attribute on the declaration being consistent with that on the definition and thus benign. While the warning is valuable in cases where the attributes between the declaration and the definition do not match, it is not useful in benign cases such as the one below. In these cases the warning makes applying the attribute difficult in existing C++ libraries that rely heavily but not completely on forward declarations and that also already make use of the Visual C++ __declspec(dllexport) feature or the Sun C++ __global or __protected specifiers, neither of which warns about such benign cases.

$ cat t.C && g++ -c t.C
struct __attribute__ ((visibility ("default"))) A { };
struct __attribute__ ((visibility ("default"))) A;
t.C:2: warning: type attributes ignored after type is already defined
Comment 1 Martin Sebor 2009-02-12 17:02:24 UTC
In addition, as the test case below shows, the warning is issued inconsistently
between classes and functions, suggesting that the instance of the warning on
the class declaration on line 2 might be a bug rather than a feature:

$ cat -n t.C && g++ -dumpversion && g++ -c t.C
     1  struct __attribute__ ((visibility ("default"))) A { };
     2  struct __attribute__ ((visibility ("default"))) A;
     3
     4  void __attribute__ ((visibility ("default"))) foo () { }
     5  void __attribute__ ((visibility ("default"))) foo ();
4.3.1
t.C:2: warning: type attributes ignored after type is already defined
Comment 2 Jesper Juhl 2014-08-15 22:55:51 UTC
I've run into this as well with GCC 4.8.2 - it's rather annoying.
Comment 3 Manuel López-Ibáñez 2014-08-16 03:23:03 UTC
Confirmed.

Perhaps it is not hard to fix. Just look how it is handled for functions. Probably it is at the same place as this:

  void __attribute__ ((visibility ("default"))) foo () { }
  void __attribute__ ((visibility ("hidden"))) foo ();

pr39159.C:4:49: warning: ‘void foo()’: visibility attribute ignored because it conflicts with previous declaration [-Wattributes]
   void __attribute__ ((visibility ("default"))) foo () { }
                                                 ^
pr39159.C:4:49: note: previous declaration of ‘void foo()’

and see if you can implement the same thing for types, probably at the point where the " type attributes ignored" warning is given.
Comment 4 wipedout 2016-02-05 07:39:58 UTC
Tons of warnings like this arise when porting code from Visual C++ to gcc. Visual C++ supports "interface" keyword and you can have "interface Interface;" many times - both above and before the actual definition. "interface" has to be mapped to exactly "struct __attribute__ ((visibility ("default")))" and so the code which originally compiled no problem causes hundreds of warnings with gcc.
Comment 5 Gert 2016-04-06 12:43:33 UTC
Created attachment 38202 [details]
Proposed patch to silence the warning if the new attribute is same

Hello, 

I propose the attached patch for this. I'm flying blind here and the patch is not yet tested. 

The idea is to search whether the attribute to be applied is already set and if this is the case, to not emit a warning, and otherwise to rephrase the warning pointing out that a different attribute was specified. 

The code was inspired by the lines starting from 589 in the same file gcc/attribs.c. 

Now since I don't know whether simple_cst_equal really compares the value of the attribute, or just some attribute type, it may be that the code does not do what I hope to achieve. 

Any comments how to improve the patch are very welcome. 

Best, 
Gert
Comment 6 Gert 2016-04-06 12:51:28 UTC
I forgot to mention: the patch is against 5.3.0.
Comment 7 Martin Sebor 2016-04-08 16:26:58 UTC
I'm not an expert in this area but the approach seems reasonable to me.  I didn't test it in 5.x but with 6.0, it doesn't make a difference because simple_cst_equal() cannot compare TREE_LISTs.  Using attribute_value_equal() instead does work.

From a user's point of view, though, it would be helpful if the diagnostic included the names of the "new" attributes that are being ignored, and pointed to the location of the previous "old" declaration that the current one conflicts with.  The former should be a matter of concatenating the names of the new attributes into a string and making that part of the warning.  The latter can be done by calling:

  inform (DECL_SOURCE_LOCATION (TYPE_FIELDS (*anode)), "previous declaration here");

To get the patch reviewed and ultimately approved, you should rebase it to the current trunk, add tests and ChangeLog entries, and post it to gcc-patches.