Closed Bug 322430 Opened 19 years ago Closed 19 years ago

Remove "deprecated with statement usage" strict warning

Categories

(Core :: JavaScript Engine, defect)

defect
Not set
normal

Tracking

()

VERIFIED FIXED

People

(Reporter: brendan, Assigned: brendan)

Details

(Keywords: verified1.8.0.1, verified1.8.1)

Attachments

(1 file)

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
Attached patch fixSplinter Review
Attachment #207683 - Flags: review?(shaver)
Assignee: general → brendan
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?
Attachment #207683 - Flags: review?(shaver)
Attachment #207683 - Flags: review+
Attachment #207683 - Flags: approval1.8.0.1?
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
Flags: blocking1.8.1?
Flags: blocking1.8.0.1?
Comment on attachment 207683 [details] [diff] [review]
fix

a=dveditz for drivers
Attachment #207683 - Flags: approval1.8.1+
Attachment #207683 - Flags: approval1.8.0.1?
Attachment #207683 - Flags: approval1.8.0.1+
Flags: blocking1.8.1?
Flags: blocking1.8.1+
Flags: blocking1.8.0.1?
Flags: blocking1.8.0.1+
Fixed everywhere.

/be
Status: NEW → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
Status: RESOLVED → VERIFIED
/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.
Flags: testcase+
(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
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.
(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
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
Bug 323314 filed. Should we worry about the root cause of the |with| problem?
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
(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
v 20060113 windows/linux/mac 1.8.0.1, 1.8, 1.9a1
Keywords: fixed1.8.1
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.
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
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: