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

RESOLVED FIXED

Status

()

Core
JavaScript Engine
RESOLVED FIXED
10 years ago
6 years ago

People

(Reporter: Jesse Ruderman, Assigned: brendan)

Tracking

(Blocks: 1 bug, {testcase})

Trunk
testcase
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: fixed-in-tracemonkey)

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

10 years ago
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).
(Reporter)

Comment 1

10 years ago
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.
(Reporter)

Comment 2

10 years ago
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.
(Reporter)

Comment 3

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

Comment 4

10 years ago
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.
Status: NEW → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → WORKSFORME
(Reporter)

Comment 5

10 years ago
That's the wrong warning, and it's completely incorrect.  Have you updated since the patch for bug 383674 went in?
Status: RESOLVED → REOPENED
Resolution: WORKSFORME → ---
(Assignee)

Comment 6

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

/be
(Assignee)

Updated

10 years ago
Depends on: 229756
(Reporter)

Comment 7

10 years ago
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; })()

Updated

10 years ago
Blocks: 296661

Updated

7 years ago
Depends on: 611388
No longer depends on: 229756
(Assignee)

Comment 8

6 years ago
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
Assignee: general → brendan
OS: Mac OS X → All
Hardware: x86 → All
(Assignee)

Updated

6 years ago
Status: REOPENED → ASSIGNED
Summary: No "... is read-only" strict warning for assignment to local const → No "... is read-only" strict warning for assignment to declared const
(Assignee)

Comment 9

6 years ago
Created attachment 536794 [details] [diff] [review]
fix
Attachment #536794 - Flags: review?(jwalden+bmo)
(Assignee)

Updated

6 years ago
Blocks: 611388
No longer depends on: 611388
(Assignee)

Comment 10

6 years ago
Created attachment 537466 [details] [diff] [review]
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
Attachment #536794 - Attachment is obsolete: true
Attachment #537466 - Flags: review?(jwalden+bmo)
Attachment #536794 - Flags: review?(jwalden+bmo)
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]:
-----------------------------------------------------------------
Attachment #537466 - Flags: review?(jwalden+bmo) → review+
(Assignee)

Comment 12

6 years ago
http://hg.mozilla.org/tracemonkey/rev/8d21b9711d6b

/be
Whiteboard: fixed-in-tracemonkey
cdleary-bot mozilla-central merge info:
http://hg.mozilla.org/mozilla-central/rev/8d21b9711d6b
Status: ASSIGNED → RESOLVED
Last Resolved: 10 years ago6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.