harmony-scoping: make assignment to 'const' a late error.

Per TC39 Nov 2014 decision.

This patch also changes behavior for "legacy const": assignments to sloppy const in strict mode is now also a type error. This fixes v8:2243 and also brings us in compliance with other engines re assignment to function names (see updated webkit test), but might have bigger implications.
That change can easily be reverted by changing Variable::IsSignallingAssignmentToConst.

BUG=v8:3713,v8:2243
LOG=N

Review URL: https://codereview.chromium.org/749633002

Cr-Commit-Position: refs/heads/master@{#25516}
This commit is contained in:
dslomov 2014-11-26 03:21:09 -08:00 committed by Commit bot
parent 560b0c4534
commit 6ac4de87a8
17 changed files with 96 additions and 54 deletions

View File

@ -2685,8 +2685,9 @@ void FullCodeGenerator::EmitVariableAssignment(Variable* var, Token::Value op) {
}
EmitStoreToStackLocalOrContextSlot(var, location);
}
} else if (IsSignallingAssignmentToConst(var, op, strict_mode())) {
__ CallRuntime(Runtime::kThrowConstAssignError, 0);
}
// Non-initializing assignments to consts are ignored.
}

View File

@ -2373,8 +2373,9 @@ void FullCodeGenerator::EmitVariableAssignment(Variable* var,
}
EmitStoreToStackLocalOrContextSlot(var, location);
}
} else if (IsSignallingAssignmentToConst(var, op, strict_mode())) {
__ CallRuntime(Runtime::kThrowConstAssignError, 0);
}
// Non-initializing assignments to consts are ignored.
}

View File

@ -2020,7 +2020,12 @@ Node* AstGraphBuilder::BuildVariableAssignment(
value = BuildHoleCheckSilent(current, value, current);
}
} else if (mode == CONST_LEGACY && op != Token::INIT_CONST_LEGACY) {
// Non-initializing assignments to legacy const is ignored.
// Non-initializing assignments to legacy const is
// - exception in strict mode.
// - ignored in sloppy mode.
if (strict_mode() == STRICT) {
return BuildThrowConstAssignError(bailout_id);
}
return value;
} else if (mode == LET && op != Token::INIT_LET) {
// Perform an initialization check for let declared variables.
@ -2034,8 +2039,8 @@ Node* AstGraphBuilder::BuildVariableAssignment(
value = BuildHoleCheckThrow(current, variable, value, bailout_id);
}
} else if (mode == CONST && op != Token::INIT_CONST) {
// All assignments to const variables are early errors.
UNREACHABLE();
// Non-initializing assignments to const is exception in all modes.
return BuildThrowConstAssignError(bailout_id);
}
environment()->Bind(variable, value);
return value;
@ -2049,7 +2054,12 @@ Node* AstGraphBuilder::BuildVariableAssignment(
Node* current = NewNode(op, current_context());
value = BuildHoleCheckSilent(current, value, current);
} else if (mode == CONST_LEGACY && op != Token::INIT_CONST_LEGACY) {
// Non-initializing assignments to legacy const is ignored.
// Non-initializing assignments to legacy const is
// - exception in strict mode.
// - ignored in sloppy mode.
if (strict_mode() == STRICT) {
return BuildThrowConstAssignError(bailout_id);
}
return value;
} else if (mode == LET && op != Token::INIT_LET) {
// Perform an initialization check for let declared variables.
@ -2058,8 +2068,8 @@ Node* AstGraphBuilder::BuildVariableAssignment(
Node* current = NewNode(op, current_context());
value = BuildHoleCheckThrow(current, variable, value, bailout_id);
} else if (mode == CONST && op != Token::INIT_CONST) {
// All assignments to const variables are early errors.
UNREACHABLE();
// Non-initializing assignments to const is exception in all modes.
return BuildThrowConstAssignError(bailout_id);
}
const Operator* op = javascript()->StoreContext(depth, variable->index());
return NewNode(op, current_context(), value);
@ -2153,6 +2163,16 @@ Node* AstGraphBuilder::BuildThrowReferenceError(Variable* variable,
}
Node* AstGraphBuilder::BuildThrowConstAssignError(BailoutId bailout_id) {
// TODO(mstarzinger): Should be unified with the VisitThrow implementation.
const Operator* op =
javascript()->CallRuntime(Runtime::kThrowConstAssignError, 0);
Node* call = NewNode(op);
PrepareFrameState(call, bailout_id);
return call;
}
Node* AstGraphBuilder::BuildBinaryOp(Node* left, Node* right, Token::Value op) {
const Operator* js_op;
switch (op) {

View File

@ -100,6 +100,7 @@ class AstGraphBuilder : public StructuredGraphBuilder, public AstVisitor {
// Builders for error reporting at runtime.
Node* BuildThrowReferenceError(Variable* var, BailoutId bailout_id);
Node* BuildThrowConstAssignError(BailoutId bailout_id);
// Builders for dynamic hole-checks at runtime.
Node* BuildHoleCheckSilent(Node* value, Node* for_hole, Node* not_hole);

View File

@ -586,6 +586,19 @@ class FullCodeGenerator: public AstVisitor {
// is expected in the accumulator.
void EmitAssignment(Expression* expr);
// Shall an error be thrown if assignment with 'op' operation is perfomed
// on this variable in given language mode?
static bool IsSignallingAssignmentToConst(Variable* var, Token::Value op,
StrictMode strict_mode) {
if (var->mode() == CONST) return op != Token::INIT_CONST;
if (var->mode() == CONST_LEGACY) {
return strict_mode == STRICT && op != Token::INIT_CONST_LEGACY;
}
return false;
}
// Complete a variable assignment. The right-hand-side value is expected
// in the accumulator.
void EmitVariableAssignment(Variable* var,

View File

@ -6647,6 +6647,9 @@ void HOptimizedGraphBuilder::HandleCompoundAssignment(Assignment* expr) {
if (var->mode() == CONST_LEGACY) {
return Bailout(kUnsupportedConstCompoundAssignment);
}
if (var->mode() == CONST) {
return Bailout(kNonInitializerAssignmentToConst);
}
BindIfLive(var, Top());
break;
@ -6673,9 +6676,7 @@ void HOptimizedGraphBuilder::HandleCompoundAssignment(Assignment* expr) {
mode = HStoreContextSlot::kCheckDeoptimize;
break;
case CONST:
// This case is checked statically so no need to
// perform checks here
UNREACHABLE();
return Bailout(kNonInitializerAssignmentToConst);
case CONST_LEGACY:
return ast_context()->ReturnValue(Pop());
default:
@ -10215,6 +10216,9 @@ void HOptimizedGraphBuilder::VisitCountOperation(CountOperation* expr) {
if (var->mode() == CONST_LEGACY) {
return Bailout(kUnsupportedCountOperationWithConst);
}
if (var->mode() == CONST) {
return Bailout(kNonInitializerAssignmentToConst);
}
// Argument of the count operation is a variable, not a property.
DCHECK(prop == NULL);
CHECK_ALIVE(VisitForValue(target));

View File

@ -2575,7 +2575,6 @@ void FullCodeGenerator::EmitVariableAssignment(Variable* var,
__ CallRuntime(Runtime::kThrowReferenceError, 1);
__ bind(&assign);
EmitStoreToStackLocalOrContextSlot(var, location);
} else if (!var->is_const_mode() || op == Token::INIT_CONST) {
if (var->IsLookupSlot()) {
// Assignment to var.
@ -2597,8 +2596,9 @@ void FullCodeGenerator::EmitVariableAssignment(Variable* var,
}
EmitStoreToStackLocalOrContextSlot(var, location);
}
} else if (IsSignallingAssignmentToConst(var, op, strict_mode())) {
__ CallRuntime(Runtime::kThrowConstAssignError, 0);
}
// Non-initializing assignments to consts are ignored.
}

View File

@ -22,6 +22,14 @@ static Object* ThrowRedeclarationError(Isolate* isolate, Handle<String> name) {
}
RUNTIME_FUNCTION(Runtime_ThrowConstAssignError) {
HandleScope scope(isolate);
THROW_NEW_ERROR_RETURN_FAILURE(
isolate,
NewTypeError("harmony_const_assign", HandleVector<Object>(NULL, 0)));
}
// May throw a RedeclarationError.
static Object* DeclareGlobals(Isolate* isolate, Handle<GlobalObject> global,
Handle<String> name, Handle<Object> value,

View File

@ -500,6 +500,7 @@ namespace internal {
F(ReThrow, 1, 1) \
F(ThrowReferenceError, 1, 1) \
F(ThrowNotDateError, 0, 1) \
F(ThrowConstAssignError, 0, 1) \
F(StackGuard, 0, 1) \
F(Interrupt, 0, 1) \
F(PromoteScheduledException, 0, 1) \

View File

@ -1093,21 +1093,6 @@ bool Scope::ResolveVariable(CompilationInfo* info, VariableProxy* proxy,
DCHECK(var != NULL);
if (proxy->is_assigned()) var->set_maybe_assigned();
if (FLAG_harmony_scoping && strict_mode() == STRICT &&
var->is_const_mode() && proxy->is_assigned()) {
// Assignment to const. Throw a syntax error.
MessageLocation location(
info->script(), proxy->position(), proxy->position());
Isolate* isolate = info->isolate();
Factory* factory = isolate->factory();
Handle<JSArray> array = factory->NewJSArray(0);
Handle<Object> error;
MaybeHandle<Object> maybe_error =
factory->NewSyntaxError("harmony_const_assign", array);
if (maybe_error.ToHandle(&error)) isolate->Throw(*error, &location);
return false;
}
if (FLAG_harmony_modules) {
bool ok;
#ifdef DEBUG

View File

@ -2596,8 +2596,9 @@ void FullCodeGenerator::EmitVariableAssignment(Variable* var,
}
EmitStoreToStackLocalOrContextSlot(var, location);
}
} else if (IsSignallingAssignmentToConst(var, op, strict_mode())) {
__ CallRuntime(Runtime::kThrowConstAssignError, 0);
}
// Non-initializing assignments to consts are ignored.
}

View File

@ -35,61 +35,61 @@
// Function local const.
function constDecl0(use) {
return "(function() { const constvar = 1; " + use + "; });";
return "(function() { const constvar = 1; " + use + "; })();";
}
function constDecl1(use) {
return "(function() { " + use + "; const constvar = 1; });";
return "(function() { " + use + "; const constvar = 1; })();";
}
// Function local const, assign from eval.
function constDecl2(use) {
use = "eval('(function() { " + use + " })')";
use = "eval('(function() { " + use + " })')()";
return "(function() { const constvar = 1; " + use + "; })();";
}
function constDecl3(use) {
use = "eval('(function() { " + use + " })')";
use = "eval('(function() { " + use + " })')()";
return "(function() { " + use + "; const constvar = 1; })();";
}
// Block local const.
function constDecl4(use) {
return "(function() { { const constvar = 1; " + use + "; } });";
return "(function() { { const constvar = 1; " + use + "; } })();";
}
function constDecl5(use) {
return "(function() { { " + use + "; const constvar = 1; } });";
return "(function() { { " + use + "; const constvar = 1; } })();";
}
// Block local const, assign from eval.
function constDecl6(use) {
use = "eval('(function() {" + use + "})')";
use = "eval('(function() {" + use + "})')()";
return "(function() { { const constvar = 1; " + use + "; } })();";
}
function constDecl7(use) {
use = "eval('(function() {" + use + "})')";
use = "eval('(function() {" + use + "})')()";
return "(function() { { " + use + "; const constvar = 1; } })();";
}
// Function expression name.
function constDecl8(use) {
return "(function constvar() { " + use + "; });";
return "(function constvar() { " + use + "; })();";
}
// Function expression name, assign from eval.
function constDecl9(use) {
use = "eval('(function(){" + use + "})')";
use = "eval('(function(){" + use + "})')()";
return "(function constvar() { " + use + "; })();";
}
@ -104,6 +104,7 @@ let decls = [ constDecl0,
constDecl8,
constDecl9
];
let declsForTDZ = new Set([constDecl1, constDecl3, constDecl5, constDecl7]);
let uses = [ 'constvar = 1;',
'constvar += 1;',
'++constvar;',
@ -116,7 +117,13 @@ function Test(d,u) {
print(d(u));
eval(d(u));
} catch (e) {
assertInstanceof(e, SyntaxError);
if (declsForTDZ.has(d) && u !== uses[0]) {
// In these cases, read of a const variable occurs
// before a write to it, so TDZ kicks in before const check.
assertInstanceof(e, ReferenceError);
return;
}
assertInstanceof(e, TypeError);
assertTrue(e.toString().indexOf("Assignment to constant variable") >= 0);
return;
}

View File

@ -683,14 +683,14 @@ function assertAccessorDescriptor(object, name) {
(function TestNameBindingConst() {
assertThrows('class C { constructor() { C = 42; } }', SyntaxError);
assertThrows('(class C { constructor() { C = 42; } })', SyntaxError);
assertThrows('class C { m() { C = 42; } }', SyntaxError);
assertThrows('(class C { m() { C = 42; } })', SyntaxError);
assertThrows('class C { get x() { C = 42; } }', SyntaxError);
assertThrows('(class C { get x() { C = 42; } })', SyntaxError);
assertThrows('class C { set x(_) { C = 42; } }', SyntaxError);
assertThrows('(class C { set x(_) { C = 42; } })', SyntaxError);
assertThrows('class C { constructor() { C = 42; } }; new C();', TypeError);
assertThrows('new (class C { constructor() { C = 42; } })', TypeError);
assertThrows('class C { m() { C = 42; } }; new C().m()', TypeError);
assertThrows('new (class C { m() { C = 42; } }).m()', TypeError);
assertThrows('class C { get x() { C = 42; } }; new C().x', TypeError);
assertThrows('(new (class C { get x() { C = 42; } })).x', TypeError);
assertThrows('class C { set x(_) { C = 42; } }; new C().x = 15;', TypeError);
assertThrows('(new (class C { set x(_) { C = 42; } })).x = 15;', TypeError);
})();

View File

@ -109,7 +109,7 @@ module R {
assertThrows(function() { l }, ReferenceError)
assertThrows(function() { R.l }, ReferenceError)
assertThrows(function() { eval("c = -1") }, SyntaxError)
assertThrows(function() { eval("c = -1") }, TypeError)
assertThrows(function() { R.c = -2 }, TypeError)
// Initialize first bunch of variables.
@ -129,7 +129,7 @@ module R {
assertEquals(-4, R.v = -4)
assertEquals(-3, l = -3)
assertEquals(-4, R.l = -4)
assertThrows(function() { eval("c = -3") }, SyntaxError)
assertThrows(function() { eval("c = -3") }, TypeError)
assertThrows(function() { R.c = -4 }, TypeError)
assertEquals(-4, v)

View File

@ -27,5 +27,5 @@
// Flags: --harmony-scoping
assertThrows("'use strict'; (function f() { f = 123; })", SyntaxError);
assertThrows("(function f() { 'use strict'; f = 123; })", SyntaxError);
assertThrows("'use strict'; (function f() { f = 123; })()", TypeError);
assertThrows("(function f() { 'use strict'; f = 123; })()", TypeError);

View File

@ -46,7 +46,7 @@ for (const x in [1,2,3]) {
}
assertEquals("012", s);
assertThrows("'use strict'; for (const x in [1,2,3]) { x++ }", SyntaxError);
assertThrows("'use strict'; for (const x in [1,2,3]) { x++ }", TypeError);
// Function scope
(function() {

View File

@ -129,7 +129,7 @@ PASS (function (){ 'use strict'; delete someDeclaredGlobal;}) threw exception Sy
PASS (function(){(function (){ 'use strict'; delete someDeclaredGlobal;})}) threw exception SyntaxError: Delete of an unqualified identifier in strict mode..
PASS 'use strict'; if (0) { someGlobal = 'Shouldn\'t be able to assign this.'; }; true; is true
PASS 'use strict'; someGlobal = 'Shouldn\'t be able to assign this.'; threw exception ReferenceError: someGlobal is not defined.
FAIL 'use strict'; (function f(){ f = 'shouldn\'t be able to assign to function expression name'; })() should throw an exception. Was undefined.
PASS 'use strict'; (function f(){ f = 'shouldn\'t be able to assign to function expression name'; })() threw exception TypeError: Assignment to constant variable..
PASS 'use strict'; eval('var introducedVariable = "FAIL: variable introduced into containing scope";'); introducedVariable threw exception ReferenceError: introducedVariable is not defined.
PASS 'use strict'; objectWithReadonlyProperty.prop = 'fail' threw exception TypeError: Cannot assign to read only property 'prop' of #<Object>.
PASS 'use strict'; delete objectWithReadonlyProperty.prop threw exception TypeError: Cannot delete property 'prop' of #<Object>.