Last Comment Bug 322430 - Remove "deprecated with statement usage" strict warning
: Remove "deprecated with statement usage" strict warning
Status: VERIFIED FIXED
: verified1.8.0.1, verified1.8.1
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: Trunk
: All All
: -- normal (vote)
: ---
Assigned To: Brendan Eich [:brendan]
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2006-01-04 18:44 PST by Brendan Eich [:brendan]
Modified: 2009-09-24 11:44 PDT (History)
9 users (show)
dveditz: blocking1.8.1+
dveditz: blocking1.8.0.1+
bob: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
fix (1.17 KB, patch)
2006-01-05 18:02 PST, Brendan Eich [:brendan]
shaver: review+
dveditz: approval1.8.0.1+
dveditz: approval1.8.1+
Details | Diff | Review

Description Brendan Eich [:brendan] 2006-01-04 18:44:44 PST
It is futile, and wrong because blanket (insensitive to good use of with).

If with statements are considered harmful because of efficiency concerns, that's an implementation bug.  Or it's a complaint about unqualified identifier ambiguity in with statement bodies, and we should add some syntax to disambiguate, e.g.:

    with (obj)
        return a*.x*.x + b*.x + c;

where leading . means in scope chain head.  This would benefit e4x filtering predicates too.

/be
Comment 1 Brendan Eich [:brendan] 2006-01-05 18:02:39 PST
Created attachment 207683 [details] [diff] [review]
fix
Comment 2 Mike Shaver (:shaver -- probably not reading bugmail closely) 2006-01-08 19:47:03 PST
Comment on attachment 207683 [details] [diff] [review]
fix

r=shaver, I think we should take this for the branch to reduce the angst of web developers who use warnings for sanity-checking.

Who's with me?
Comment 3 Brendan Eich [:brendan] 2006-01-08 20:56:04 PST
Yeah, this is a vacuously safe removal of a strict warning.  It can't break anything, and it's considered a botch by many smarties out there.  Why not get it over with, and make strict warnings somewhat to much more useful?

/be
Comment 4 Daniel Veditz [:dveditz] 2006-01-09 12:16:27 PST
Comment on attachment 207683 [details] [diff] [review]
fix

a=dveditz for drivers
Comment 5 Brendan Eich [:brendan] 2006-01-09 12:54:54 PST
Fixed everywhere.

/be
Comment 6 Bob Clary [:bc:] 2006-01-13 08:27:12 PST
/cvsroot/mozilla/js/tests/js1_5/Regress/regress-322430.js,v  <--  regress-322430.js
initial revision: 1.1

Strangely enough, in older builds where this was still a warning, if the with statement was inside of a try catch block, the warning would not be issues. I had to eval the statement to get the warning to appear.
Comment 7 Brendan Eich [:brendan] 2006-01-13 08:38:57 PST
(In reply to comment #6)
> Strangely enough, in older builds where this was still a warning, if the with
> statement was inside of a try catch block, the warning would not be issues. I
> had to eval the statement to get the warning to appear.

Sounds like a bug -- warnings should not be dropped in try blocks.  But I couldn't reproduce it with trivial alternate tests:

javascript: try { if (xyzzy=1) print(xyzzy); } catch (e) { alert(e); }

With strict warnings enabled, this warns (two warnings, of course).  It does not catch and alert.

/be
Comment 8 Bob Clary [:bc:] 2006-01-13 08:54:46 PST
set strict and werror to true, then
try{var o = {foo: 'bar'}; with(o){ alert(foo); } } catch (e) { alert(e); }

only happens with the |with| statement. Remember to test in 1.7 or old 1.8/trunk builds now.
Comment 9 Brendan Eich [:brendan] 2006-01-13 09:11:12 PST
(In reply to comment #8)
> set strict and werror to true, then
> try{var o = {foo: 'bar'}; with(o){ alert(foo); } } catch (e) { alert(e); }
> 
> only happens with the |with| statement. Remember to test in 1.7 or old
> 1.8/trunk builds now.

werror -- that's different! ;-)  That makes the warning an error, which is an exception.  Ah, but there is a bug:

$ Linux_All_OPT.OBJ/js -s
js> options()
strict
js> options('werror')
strict
js> options()
strict,werror
js> javascript: try { if (xyzzy=1) print(xyzzy); } catch (e) { alert(e); }
typein:4: test for equality (==) mistyped as assignment (=)?:
typein:4: javascript: try { if (xyzzy=1) print(xyzzy); } catch (e) { alert(e); }typein:4: .............................^

This should have thrown, and the catch should have crapped out on the alert (we should make an alert function in the JS shell, BTW -- just alias to print would be a good start).

The bug in this case is that JSMSG_EQUAL_AS_ASSIGN in js.msg is JSEXN_NONE, when it should be JSEXN_SYNTAXERR.  Or something like that -- mrbkap, your thoughts welcome.

I'm not sure what was going on with that with strict|werror case, Bob.  The error there was JSMSG_DEPRECATED_USAGE, and it has exception type JSEXN_REFERENCEERR (an odd choice too, but not NONE, so the error should not have been quashed when on its way to becoming an exception).

/be
Comment 10 Brendan Eich [:brendan] 2006-01-13 09:22:49 PST
Bob, could you please file a new bug on the JSEXN_NONE in js.msg problems, unless mrbkap knows of an existing one?  I have a patch.

/be
Comment 11 Bob Clary [:bc:] 2006-01-13 09:29:12 PST
Bug 323314 filed. Should we worry about the root cause of the |with| problem?
Comment 12 Brendan Eich [:brendan] 2006-01-13 09:58:48 PST
Oh, and that catch clause with its doomed alert call will never run in the js shell, because the shell compiles via the JS_CompileUCScriptForPrincipals API, and that API reports uncaught exceptions.  The werror turns the warning into an error, and with my js.msg fix, the JSEXN_SYNTAXERR is not quashed, but becomes a bona fide exception.  Trouble is, the script never runs, so there's no chance for the try to catch.

So eval in try is the only way to test syntax errors (even if strict warnings optionally promoted to error-as-exception status via werror).

/be
Comment 13 Brendan Eich [:brendan] 2006-01-13 09:59:43 PST
(In reply to comment #11)
> Bug 323314 filed.

Thanks.

> Should we worry about the root cause of the |with| problem?

No, see comment 12 second paragraph.  Obvious, in hindsight.

/be
Comment 14 Bob Clary [:bc:] 2006-01-14 11:46:39 PST
v 20060113 windows/linux/mac 1.8.0.1, 1.8, 1.9a1
Comment 15 Jim Blandy :jimb 2009-09-24 10:40:29 PDT
Note that this bug is resurrected, in a sense, in bug 514576.  ECMAScript 5 strict mode forbids 'with' statements; we'd like to avoid having two separate concepts of 'strictness' (ES5; JSOPTION_STRICT classic); hilarity ensues.

I'd love to get some thoughts from the greybeards.
Comment 16 Brendan Eich [:brendan] 2009-09-24 11:44:30 PDT
No beards on MochiBob or me.

See bug 514576 comment 2. Let's continue over there, and let this old resolved (verified) bug stay asleep.

/be

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