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)
Tracking
()
RESOLVED
FIXED
mozilla26
People
(Reporter: dholbert, Unassigned)
References
Details
Attachments
(1 file, 1 obsolete file)
2.82 KB,
patch
|
n.nethercote
:
review+
|
Details | Diff | Splinter Review |
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?
Reporter | ||
Comment 1•11 years ago
|
||
[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]
Reporter | ||
Comment 2•11 years ago
|
||
(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.
Comment 3•11 years ago
|
||
script->global() is defined in jsscriptinlines.h, but script->compartment() is defined in jsscript.h.
Attachment #787260 -
Flags: review?(jimb)
Comment 4•11 years ago
|
||
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-
Comment 5•11 years ago
|
||
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)
Updated•11 years ago
|
Flags: needinfo?(bobbyholley+bmo)
Comment 6•11 years ago
|
||
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+
Comment 8•11 years ago
|
||
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.
Description
•