Closed
Bug 603825
Opened 14 years ago
Closed 14 years ago
Fix a bunch of warnings
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
RESOLVED
FIXED
People
(Reporter: n.nethercote, Assigned: n.nethercote)
Details
(Whiteboard: fixed-in-tracemonkey)
Attachments
(1 file)
3.53 KB,
patch
|
Waldo
:
review+
|
Details | Diff | Splinter Review |
GCC on Linux: ../jsbool.cpp: In function ‘JSBool bool_toString(JSContext*, uintN, js::Value*)’: ../jsbool.cpp:98: warning: ‘b’ may be used uninitialized in this function ../jsbool.cpp: In function ‘JSBool bool_toSource(JSContext*, uintN, js::Value*)’: ../jsbool.cpp:81: warning: ‘b’ may be used uninitialized in this function ../jsbool.cpp: In function ‘JSBool bool_valueOf(JSContext*, uintN, js::Value*)’:../jsbool.cpp:113: w arning: ‘b’ may be used uninitialized in this function ../jsstr.cpp: In function ‘JSBool js_str_toString(JSContext*, uintN, js::Value*)’: ../jsstr.cpp:951: warning: ‘str’ may be used uninitialized in this function ../jsstr.cpp: In function ‘JSBool str_toSource(JSContext*, uintN, js::Value*)’: ../jsstr.cpp:908: warning: ‘str’ may be used uninitialized in this function ../methodjit/Compiler.cpp: In member function ‘bool js::mjit::Compiler::jsop_callprop_str(JSAtom*)’: ../methodjit/Compiler.cpp:2821: warning: unused variable ‘funFe’ All of these except the last two were introduced by bug 514570. Here's the jsbool.cpp code: bool b; if (!GetPrimitiveThis(cx, vp, &b)) return false; JSAtom *atom = cx->runtime->atomState.booleanAtoms[b ? 1 : 0]; It seems that GCC is reasonably smart about detecting the uninitialized variables - if GetPrimitiveThis() returns 'false' when it doesn't initialize 'b', then GCC doesn't give the warning. However, GetPrimitiveThis() instead returns 'ReportIncompatibleMethod(cx, vp, Behavior::getClass());' which is always false. It's not smart enough to detect that. So I changed ReportIncompatibleMethod() to have a return void type and make the 'return false' explicit in GetPrimitiveThis. This removed all the jsbool.cpp and jsstr.cpp warnings and seemed preferable to initializing several variables unnecessarily. (I've always found error-reporting functions that always return false a bit creepy, and now I finally have some concrete evidence why they shouldn't be used :) The methodjit/Compiler.cpp fix was trivial.
Attachment #482728 -
Flags: review?(jwalden+bmo)
Comment 1•14 years ago
|
||
Comment on attachment 482728 [details] [diff] [review] patch (against TM 55344:db5b6e24f421) That's unfortunate. I've always been a fan of report-and-return-false functions for the slightly briefer code needed to use them; further, they can unify errors returned in multiple places into a single call. (See js_SetPropertyHelper, possibly in past revisions, for such a case.) But if the compiler's tripping over them here, and there's no attribute to add to tell it not to trip (online gcc docs indicate nothing), I guess this is the right thing to do.
Attachment #482728 -
Flags: review?(jwalden+bmo) → review+
Assignee | ||
Comment 2•14 years ago
|
||
http://hg.mozilla.org/tracemonkey/rev/8212d565d2a4
Whiteboard: fixed-in-tracemonkey
Comment 3•14 years ago
|
||
http://hg.mozilla.org/mozilla-central/rev/8212d565d2a4
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•