709622 - tests with empty body should cause a JS strict warning

Status: RESOLVED FIXED

(Core :: JavaScript Engine, enhancement)

Description

[Florian Quèze [:florian]]

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)]

Attachment: 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)]

Attachment: Corresponding warning.

Update: I hooked a test into an existing strict warning test.

Comment 3

[Jeff Walden [:Waldo]]

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)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/ac2e9f57426d

Florian, thanks for the good idea!

Comment 5

[Florian Quèze [:florian]]

(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)]

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 (he/him; ⌚ UTC+1/+2)]

https://hg.mozilla.org/mozilla-central/rev/ac2e9f57426d