728423 - 235 gcc build warnings for jsobj.h like: "warning: inline function ‘js::StringObject& JSObject::asString()’ used but never defined" (& other JSObject methods)

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Daniel Holbert [:dholbert]]

m-c hits around 235 build warnings that look like the following (all for jsobj.h):

{
js/src/jsobj.h:1454:30: warning: inline function ‘js::StringObject& JSObject::asString()’ used but never defined [enabled by default]
js/src/jsobj.h:1442:31: warning: inline function ‘js::BooleanObject& JSObject::asBoolean()’ used but never defined [enabled by default]
js/src/jsobj.h:1449:30: warning: inline function ‘js::NumberObject& JSObject::asNumber()’ used but never defined [enabled by default]
js/src/jsobj.h:1454:30: warning: inline function ‘js::StringObject& JSObject::asString()’ used but never defined [enabled by default]
}

Sample build logs:
https://tbpl.mozilla.org/php/getParsedLog.php?id=9422633&tree=Mozilla-Inbound
https://tbpl.mozilla.org/php/getParsedLog.php?id=9421622&tree=Mozilla-Inbound

Comment 1

[Jeff Walden [:Waldo]]

Attachment: Move this-handling and method guard code into vm/MethodGuard* files, breaking forward-dependencies on inline definitions

The problem is the various Behavior specializations call the unbox() methods in the relevant boxed-primitive object classes.  But those methods haven't been defined yet, and can't be because those -inl headers depend on jsobjinlines.h.  Thus the right fix is to move this stuff into a header that can depend on those inlines.

It happens that there are several methods in this general area of functionality, all dealing with handling initial behavior in functions -- extracting the desired primitive type, forwarding across compartments if necessary, and so on.  It's small, but it seems enough to justify vm/MethodGuard.* -- name subject to debate, of course, but it seems a reasonable name given the functions already describe themselves in "guard" and "method" terms.  And anything sensible that slims down the excessiveness of jsobj* seems good.  So let's do that.

I will be exceedingly glad when I can start using the MPL2 boilerplate and know MXR will properly pick up the right comment as a per-file description -- bug 717196 (and why the changes here are seemingly so much line-bloat -- three new boilerplates added :-\ ).

Comment 2

[Andrew McCreight [:mccr8]]

(In reply to Jeff Walden (:Waldo) (remove +bmo to email) from comment #1)
> I will be exceedingly glad when I can start using the MPL2 boilerplate and
> know MXR will properly pick up the right comment as a per-file description
> -- bug 717196 (and why the changes here are seemingly so much line-bloat --
> three new boilerplates added :-\ ).

IIRC, some people have started landing new files with MPL2, but a quick MXR might be in order to confirm that.

Comment 3

[Igor Bukanov]

I had to add the patch here in front of my local MQ to fix unbearable warning noise.

Comment 4

[Jeff Walden [:Waldo]]

For what it's worth, the reason I didn't notice this warning noise locally is that clang doesn't emit the same warnings.  I'm not sure what they're doing differently than gcc to not trigger the warnings; if I had to guess, it's because clang instantiates the PrimitiveBehavior template later than gcc does.

Comment 5

[Luke Wagner [:luke]]

Comment on attachment 598506 [details] [diff] [review]
Move this-handling and method guard code into vm/MethodGuard* files, breaking forward-dependencies on inline definitions

Stealing: r+

Comment 6

[Luke Wagner [:luke]]

Courtesy push:
https://hg.mozilla.org/integration/mozilla-inbound/rev/f07a6060cc5a

Comment 7

[Ed Morley [:emorley]]

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

Comment 8

[Jeff Walden [:Waldo]]

Thanks muchly for stealing and landing!

Comment 9

[Luke Wagner [:luke]]

Thanks for the clean-making fix.  The patch I had whipped up locally (before I did a bugzilla search) was far cruder.

Comment 10

[Jeff Walden [:Waldo]]

https://twitter.com/#!/sayrer/status/19304989209