Closed Bug 877444 Opened 7 years ago Closed 7 years ago

New "used but not defined" warning for isScriptSource

Categories

(Core :: JavaScript Engine, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla24

People

(Reporter: terrence, Assigned: jimb)

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 1 obsolete file)

My tip is 132687:6b890e8b931d. This was probably introduced with the landing of ScriptSourceObject.
Yeah, hg bisect says this was introduced by
{
changeset:   132703:b048b14ab010
user:        Eddy Bruel <ejpbruel@mozilla.com>
date:        Wed May 22 16:06:54 2013 -0700
summary:     Bug 637572 - Implement ScriptSourceObject; r=jimb
}
https://hg.mozilla.org/mozilla-central/rev/b048b14ab010

That cset has this output, when compiling jsalloc.cpp:
{
jsalloc.cpp
In file included from /mozilla/js/src/vm/Shape.h:13:0,
                 from /mozilla/js/src/jsscript.h:20,
                 from /mozilla/js/src/vm/SPSProfiler.h:17,
                 from /mozilla/js/src/jscntxt.h:39,
                 from /mozilla/js/src/jsalloc.cpp:9:
/mozilla/js/src/jsobj.h:429:29: warning: inline function 'const JS::Value& JSObject::getReservedSlot(uint32_t) const' used but never defined [enabled by default]
     inline const js::Value &getReservedSlot(uint32_t index) const;
                             ^
/mozilla/js/src/jsobj.h:432:17: warning: inline function 'void JSObject::setReservedSlot(uint32_t, const JS::Value&)' used but never defined [enabled by default]
     inline void setReservedSlot(uint32_t index, const js::Value &v);
                 ^
In file included from /mozilla/js/src/vm/Shape.h:13:0,
                 from /mozilla/js/src/jsscript.h:20,
                 from /mozilla/js/src/vm/SPSProfiler.h:17,
                 from /mozilla/js/src/jscntxt.h:39,
                 from /mozilla/js/src/jsalloc.cpp:9:
/mozilla/js/src/jsobj.h:983:17: warning: inline function 'bool JSObject::isScriptSource() const' used but never defined [enabled by default]
     inline bool isScriptSource() const;
                 ^
}

...whereas its parent (b5812c4bef74) has no warnings when building jsalloc.cpp
Blocks: 637572
This makes sense, because the blamed cset ( b048b14ab010 ) added isScriptSource() -- it didn't exist before -- and also added invocations of getReservedSlot(), setReservedSlot(), and isScriptSource() to jsscript.h.

So I thihnk everything that (directly or indirectly) includes jsscript.h now also needs to include jsobjinlines.h, or else it'll spam this warning.
Attached patch fix v1 (obsolete) — Splinter Review
Seems like we need to do one of two things:
Either we need to #include jsobjinlines.h all over the place (so that the various .cpp files will get the definition for these inline functions), OR we need to make these functions non-inline.

This fix does the former (adding the #inline to every .cpp file that triggers this warning), and it leaves my mozilla-inbound build with no more instances of this warning.
Attachment #757723 - Flags: review?(jimb)
Instead of scattering jsobjinlines.h all over the place, I think it would be better to move those inline function definitions to jsscriptlines.h.

This patch seems to remove the warnings, too, and is more localized.
Assignee: general → jimb
Attachment #757723 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #757723 - Flags: review?(jimb)
Attachment #758212 - Flags: review?(terrence)
I like that idea better.
Comment on attachment 758212 [details] [diff] [review]
Remove new 'inline function used but not defined' warnings for ScriptSourceObject-related functions.

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

r=me
Attachment #758212 - Flags: review?(terrence) → review+
Revised, because it conflicted with bug 879138.

https://tbpl.mozilla.org/?tree=Try&rev=023076301ecb
https://hg.mozilla.org/integration/mozilla-inbound/rev/072142c68939
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Flags: in-testsuite-
Resolution: --- → FIXED
Target Milestone: --- → mozilla24
(Sorry, R/F is not appropriate for inbound...)
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
https://hg.mozilla.org/mozilla-central/rev/072142c68939
Status: REOPENED → RESOLVED
Closed: 7 years ago7 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.