Closed Bug 901658 Opened 11 years ago Closed 11 years ago

jsscript.h:811:30: warning: inline function 'js::GlobalObject& JSScript::global() const' used but never defined [enabled by default]

Categories

(Core :: JavaScript Engine, defect)

x86_64
Linux
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla26

People

(Reporter: dholbert, Unassigned)

References

Details

Attachments

(1 file, 1 obsolete file)

New build warning:
{
 In file included from /mozilla-central/js/src/jsgc.h:20:0,
                 from /mozilla-central/js/src/vm/Runtime.h:24,
                 from /mozilla-central/js/src/jscntxt.h:23,
                 from /mozilla-central/js/src/jscompartment.h:13,
                 from /mozilla-central/js/src/vm/ObjectImpl-inl.h:14,
                 from /mozilla-central/js/src/vm/ObjectImpl.cpp:7:
/mozilla-central/js/src/jsscript.h:811:30: warning: inline function 'js::GlobalObject& JSScript::global() const' used but never defined [enabled by default]
     inline js::GlobalObject &global() const;
}

We get this same warning for a bunch of different .cpp files; ObjectImpl.cpp is just the first one I hit it for.

hg bisect says this was introduced by bug 897322, which added an invocation of "global()" (an inline function, whose definition is separate from the decl) in Debugger.h without providing the definition of global() to all of the various .cpp files that #include Debugger.h:
 http://hg.mozilla.org/mozilla-central/rev/ecd30f33574b#l2.12

bholley, can you change that patch so that the "global()" invocation happens in a .cpp file (which includes the right *-inlines.h header) instead of having the invocation in Debugger.h?
[oops, forgot to set the summary & CC bholley]

needinfo=bholley for end of comment 0.
Flags: needinfo?(bobbyholley+bmo)
Summary: global() → jsscript.h:811:30: warning: inline function 'js::GlobalObject& JSScript::global() const' used but never defined [enabled by default]
(In reply to Daniel Holbert [:dholbert] (lowered responsiveness through 8/6) from comment #0)
> We get this same warning for a bunch of different .cpp files; ObjectImpl.cpp
> is just the first one I hit it for.

Specifically, we get a block of build-warning-spew for ObjectImpl.cpp, RootMarking.cpp, and Zone.cpp.
script->global() is defined in jsscriptinlines.h, but script->compartment() is
defined in jsscript.h.
Attachment #787260 - Flags: review?(jimb)
Comment on attachment 787260 [details] [diff] [review]
Dance around include dependency issues in Debugger.h. v1

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

Stealing review -- I've been doing lots of work relating to headers lately.

This doesn't work, because JSCompartment::maybeGlobal() is defined in jscompartmentinlines.h, so you're just shifting the warning from jsscriptinlines.h to jscompartmentinlines.h.

Since this call is in an assertion and isn't perf-critical, I suggest creating a JSScript::uninlinedGlobal() function, a la JSObject::uninlinedGetType().
Attachment #787260 - Flags: review?(jimb) → review-
Good call njn. Quite indicative of the fact that the warning doesn't appear
locally for me. ;-)
Attachment #787260 - Attachment is obsolete: true
Attachment #787304 - Flags: review?(n.nethercote)
Flags: needinfo?(bobbyholley+bmo)
Comment on attachment 787304 [details] [diff] [review]
Introduce an uninlined version of JSScript::global() to use in Debugger.h assertions. v1

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

I don't know what platform you develop on, but on Linux at least, GCC issues these warnings far more frequently than clang does.
Attachment #787304 - Flags: review?(n.nethercote) → review+
https://hg.mozilla.org/mozilla-central/rev/e1fefe377b9a
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla26
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: