383902 - No "... is read-only" strict warning for assignment to declared const

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Jesse Ruderman]

js> const c;
js> c = 6;
typein:2: strict warning: c is read-only

But:

js> function f(a) { const c; c = a + a; }
js> f(3);

I'm guessing this is because the assignment is optimized away in the latter case.  The code that optimizes it away should emit a compile-time warning similar to the existing run-time warning (using the same warning text would be fine).

Comment 1

[Jesse Ruderman]

Note that if the RHS of the assignment is simple, you can currently get a "useless expression" warning:

js> (function() { const c; c = 3 })()
typein:1: strict warning: useless expression

Ideally, this would trigger a "read-only" warning and *not* trigger a "useless expression" warning, because a "read-only" warning is easier to understand.

Comment 2

[Jesse Ruderman]

Crowder points out that my examples are wrong, because ES4 allows one-time assignment to a const if the const is not initialized.  See bug 229756.  "const c;" should be replaced with "const c = 1;" in each example.

Comment 3

[Jesse Ruderman]

I wonder if the "eliminate assignment to const" optimization will survive the fixing of bug 229756 at all.

Comment 4

[Brian Crowder]

Works for me on recent trunk:

js> function (a) { const c; c = a + a }
typein:5: strict warning: useless expression
function (a) {
    const c;
}

Not sure what changed.

Comment 5

[Jesse Ruderman]

That's the wrong warning, and it's completely incorrect.  Have you updated since the patch for bug 383674 went in?

Comment 6

[Brendan Eich [:brendan]]

This bug should really just depend on bug 229756, which is the real bug to fix.

/be

Comment 7

[Jesse Ruderman]

Fixed examples:

(From comment 0) This should trigger a "read-only" warning, either at compile time or at run time:

function f(a) { const c = 1; c = a + a; }
f(3);

(From comment 1) This should trigger a "read-only" warning and not trigger a "useless expression" warning:

(function() { const c = 1; c = 3; })()

Comment 8

[Brendan Eich [:brendan]]

This bug is a stop-gap and tree-shaker in advance of the full fix for bug 611388, which depends on the ES.next/Harmony const semantics being pinned down better (but they are likely to be unsurprising: no non-initializer assignment allowed, an initializer is required, early use in "temporal dead zone" is an error).

The patch for this bug is easy. Attaching next.

/be

Comment 9

[Brendan Eich [:brendan]]

Attachment: fix

Comment 10

[Brendan Eich [:brendan]]

Attachment: rebase, hoist var c so it's a declared global, fix test indentation

Waldo, this is still tiny -- any chance of review by tomorrow?

/be

Comment 11

[Jeff Walden [:Waldo]]

Comment on attachment 537466 [details] [diff] [review]
rebase, hoist var c so it's a declared global, fix test indentation

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

Comment 12

[Brendan Eich [:brendan]]

http://hg.mozilla.org/tracemonkey/rev/8d21b9711d6b

/be

Comment 13

[Chris Leary [:cdleary] (not checking bugmail)]

cdleary-bot mozilla-central merge info:
http://hg.mozilla.org/mozilla-central/rev/8d21b9711d6b