The default bug view has changed. See this FAQ.

Remove "deprecated with statement usage" strict warning

VERIFIED FIXED

Status

()

Core
JavaScript Engine
VERIFIED FIXED
11 years ago
8 years ago

People

(Reporter: brendan, Assigned: brendan)

Tracking

({verified1.8.0.1, verified1.8.1})

Trunk
verified1.8.0.1, verified1.8.1
Points:
---
Bug Flags:
blocking1.8.1 +
blocking1.8.0.1 +
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

11 years ago
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
(Assignee)

Comment 1

11 years ago
Created attachment 207683 [details] [diff] [review]
fix
Attachment #207683 - Flags: review?(shaver)
(Assignee)

Updated

11 years ago
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?
(Assignee)

Comment 3

11 years ago
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+
(Assignee)

Comment 5

11 years ago
Fixed everywhere.

/be
Status: NEW → RESOLVED
Last Resolved: 11 years ago
Keywords: fixed1.8.0.1, fixed1.8.1
Resolution: --- → FIXED

Updated

11 years ago
Status: RESOLVED → VERIFIED
Keywords: fixed1.8.0.1, fixed1.8.1 → verified1.8.0.1, verified1.8.1

Updated

11 years ago
Keywords: verified1.8.0.1, verified1.8.1 → fixed1.8.0.1, fixed1.8.1

Comment 6

11 years ago
/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+
(Assignee)

Comment 7

11 years ago
(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

11 years ago
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.
(Assignee)

Comment 9

11 years ago
(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
(Assignee)

Comment 10

11 years ago
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

11 years ago
Bug 323314 filed. Should we worry about the root cause of the |with| problem?
(Assignee)

Comment 12

11 years ago
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
(Assignee)

Comment 13

11 years ago
(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

11 years ago
v 20060113 windows/linux/mac 1.8.0.1, 1.8, 1.9a1
Keywords: fixed1.8.0.1, fixed1.8.1 → verified1.8.0.1, verified1.8.1
(Assignee)

Updated

11 years ago
Keywords: fixed1.8.1

Updated

11 years ago
Keywords: fixed1.8.1

Comment 15

8 years ago
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.
(Assignee)

Comment 16

8 years ago
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.