845868 - jscntxt.h:572:7: warning: type attributes ignored after type is already defined [-Wattributes]

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Daniel Holbert [:dholbert]]

Unfortunately, while the patch for bug 845277 fixed a clang build error, it introduced a GCC build warning:
{
In file included from js/src/jsarray.h:12:0,
                 from js/src/shell/js.cpp:37:
js/src/jscntxt.h:572:7: warning: type attributes ignored after type is already defined [-Wattributes]
}

I can trigger more instances of the warning by adding more copies of the JS_FRIEND_API(AutoEnterPolicy) forward-declaration to js.cpp after its include of jswrapper.h (which is what provides the AutoEnterPolicy definition, via its own include of jsproxy.h).

So basically, it looks like GCC doesn't like forward-declarations-that-come-after-the-actual-definition to use JS_FRIEND_API, whereas clang mandates it (?)

This kind of sucks, but fortunately we can silence the warning by moving jswrapper.h down in the #include list, so that the forward-declaration in jsarray.h ends up being an actual forward-declaration rather than a declaration-after-the-compiler-has-already-seen-the-class-definition.

Comment 1

[Daniel Holbert [:dholbert]]

Attachment: fix v1: reorder #include list

As described at the end of prev. comment, we can fix this by making the forward decl (via jsarray.h) a "true" forward decl, by moving the header that gets us the class definition to the end.

Conveniently, that gets us marginally-better alphabetical order, so this isn't *just* a hack -- it's cosmetically better, too :)

Comment 2

[Jeff Walden [:Waldo]]

Comment on attachment 719047 [details] [diff] [review]
fix v1: reorder #include list

Review of attachment 719047 [details] [diff] [review]:
-----------------------------------------------------------------

Ugh, so wrong.  I guess we should figure out some way to put a single definition in a header and avoid forward-declaring this, in the long run, but if this works for now, and plays well with the #include ordering we aspire to (and sometimes hit), I guess r=me on this for now.

Comment 3

[Daniel Holbert [:dholbert]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/f44860760c1f

Comment 4

[Gregory Szorc [:gps]]

https://hg.mozilla.org/mozilla-central/rev/f44860760c1f