Last Comment Bug 709622 - tests with empty body should cause a JS strict warning
: tests with empty body should cause a JS strict warning
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: Trunk
: All All
: -- enhancement (vote)
: mozilla11
Assigned To: Chris Leary [:cdleary] (not checking bugmail)
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-12-11 14:44 PST by Florian Quèze [:florian] [:flo]
Modified: 2011-12-16 05:46 PST (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Corresponding warning. (4.91 KB, patch)
2011-12-12 11:18 PST, Chris Leary [:cdleary] (not checking bugmail)
no flags Details | Diff | Splinter Review
Corresponding warning. (5.89 KB, patch)
2011-12-12 11:34 PST, Chris Leary [:cdleary] (not checking bugmail)
jwalden+bmo: review+
Details | Diff | Splinter Review

Description Florian Quèze [:florian] [:flo] 2011-12-11 14:44:32 PST
I just wasted time today for the second time debugging a code that mysteriously didn't work because of a trailing semi-colon after a test:

  if (<test>);
    <code>;

<code> is unconditionally executed. I think this code should cause a JS strict warning to appear in my error console and in my terminal (I use a debug build).
Comment 1 Chris Leary [:cdleary] (not checking bugmail) 2011-12-12 11:18:50 PST
Created attachment 580978 [details] [diff] [review]
Corresponding warning.

Waldo, have you noticed that we do work to detect warning situations even when warnings are disabled? /me wonders how much we can win by not doing that.
Comment 2 Chris Leary [:cdleary] (not checking bugmail) 2011-12-12 11:34:50 PST
Created attachment 580984 [details] [diff] [review]
Corresponding warning.

Update: I hooked a test into an existing strict warning test.
Comment 3 Jeff Walden [:Waldo] (remove +bmo to email) 2011-12-13 12:56:31 PST
Comment on attachment 580984 [details] [diff] [review]
Corresponding warning.

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

Nice.  And yeah, I kind of do wonder about warnings, although I doubt it matters much usually.
Comment 4 Chris Leary [:cdleary] (not checking bugmail) 2011-12-15 14:01:14 PST
https://hg.mozilla.org/integration/mozilla-inbound/rev/ac2e9f57426d

Florian, thanks for the good idea!
Comment 5 Florian Quèze [:florian] [:flo] 2011-12-15 14:15:09 PST
(In reply to Chris Leary [:cdleary] from comment #4)

> Florian, thanks for the good idea!

Thanks for acting on it so quickly! I didn't expect such a fast turnaround on a small detail like this. Now I just wish I filed this bug the first time I wasted time on such a bug instead of adding on my todo list over a year ago that I should do it someday ;).
Comment 6 Chris Leary [:cdleary] (not checking bugmail) 2011-12-15 14:30:22 PST
It's definitely a crapshoot -- some ideas are easier than others and some days the most relevant developers are more highly caffeinated than others. :-)
Comment 7 :Ms2ger (⌚ UTC+1/+2) 2011-12-16 05:46:28 PST
https://hg.mozilla.org/mozilla-central/rev/ac2e9f57426d

Note You need to log in before you can comment on or make changes to this bug.