AuroraMiddleware/v8
1
0
Fork 0
Code Issues 2 Pull Requests Releases Wiki Activity

Revert of change most cases of variable redeclaration from TypeError to SyntaxError (patchset #8 id:140001 of https://codereview.chromium.org/2048703002/ )

Browse Source 
Reason for revert:
This is going to break the LayoutTest inspector-protocol/console/console-let-const-with-api.html as seen in https://build.chromium.org/p/tryserver.v8/builders/v8_linux_blink_rel/builds/2247 . Please run this test manually, using instructions at https://www.chromium.org/developers/testing/webkit-layout-tests , and fix on the Chrome side if needed before resubmitting this patch.

Original issue's description:
> change most cases of variable redeclaration from TypeError to SyntaxError.
>
> Code like `let a; eval("var a;");` should throw a SyntaxError, not a TypeError
> (this caused a test262 failure.). However, the code `eval("function NaN() {}");`
> should actually throw a TypeError. This patch changes most cases of
> redeclaration errors from TypeError to SyntaxError. See the test
> mjsunit/regress/redeclaration-error-types for a thorough analysis with spec
> references.
>
> The relevant sections of the spec are ES#sec-globaldeclarationinstantiation and
> ES#sec-evaldeclarationinstantiation
>
> BUG=v8:4955
> LOG=y
>
> Committed: https://crrev.com/2b787561763d0f7e8dab698652715a742cf78291
> Cr-Commit-Position: refs/heads/master@{#36940}

TBR=adamk@chromium.org,jwolfe@igalia.com
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=v8:4955

Review-Url: https://codereview.chromium.org/2064793002
Cr-Commit-Position: refs/heads/master@{#36941}
This commit is contained in:
littledan 2016-06-13 11:21:49 -07:00 committed by Commit bot
parent 2b78756176
commit 85c2c8d847
4 changed files with 27 additions and 206 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

71
src/runtime/runtime-scopes.cc
View File

@ -16,18 +16,10 @@
namespace v8 {
namespace internal {
enum class RedeclarationType { kSyntaxError = 0, kTypeError = 1 };
static Object* ThrowRedeclarationError(Isolate* isolate, Handle<String> name,
RedeclarationType redeclaration_type) {
static Object* ThrowRedeclarationError(Isolate* isolate, Handle<String> name) {
HandleScope scope(isolate);
if (redeclaration_type == RedeclarationType::kSyntaxError) {
THROW_NEW_ERROR_RETURN_FAILURE(
isolate, NewSyntaxError(MessageTemplate::kVarRedeclaration, name));
} else {
THROW_NEW_ERROR_RETURN_FAILURE(
isolate, NewTypeError(MessageTemplate::kVarRedeclaration, name));
}
THROW_NEW_ERROR_RETURN_FAILURE(
isolate, NewTypeError(MessageTemplate::kVarRedeclaration, name));
}
@ -42,18 +34,13 @@ RUNTIME_FUNCTION(Runtime_ThrowConstAssignError) {
static Object* DeclareGlobals(Isolate* isolate, Handle<JSGlobalObject> global,
Handle<String> name, Handle<Object> value,
PropertyAttributes attr, bool is_var,
bool is_function,
RedeclarationType redeclaration_type) {
bool is_function) {
Handle<ScriptContextTable> script_contexts(
global->native_context()->script_context_table());
ScriptContextTable::LookupResult lookup;
if (ScriptContextTable::Lookup(script_contexts, name, &lookup) &&
IsLexicalVariableMode(lookup.mode)) {
// ES#sec-globaldeclarationinstantiation 6.a:
// If envRec.HasLexicalDeclaration(name) is true, throw a SyntaxError
// exception.
return ThrowRedeclarationError(isolate, name,
RedeclarationType::kSyntaxError);
return ThrowRedeclarationError(isolate, name);
}
// Do the lookup own properties only, see ES5 erratum.
@ -80,11 +67,7 @@ static Object* DeclareGlobals(Isolate* isolate, Handle<JSGlobalObject> global,
if (old_details.IsReadOnly() || old_details.IsDontEnum() ||
(it.state() == LookupIterator::ACCESSOR &&
it.GetAccessors()->IsAccessorPair())) {
// ES#sec-globaldeclarationinstantiation 5.d:
// If hasRestrictedGlobal is true, throw a SyntaxError exception.
// ES#sec-evaldeclarationinstantiation 8.a.iv.1.b:
// If fnDefinable is false, throw a TypeError exception.
return ThrowRedeclarationError(isolate, name, redeclaration_type);
return ThrowRedeclarationError(isolate, name);
}
// If the existing property is not configurable, keep its attributes. Do
attr = old_attributes;
@ -147,11 +130,9 @@ RUNTIME_FUNCTION(Runtime_DeclareGlobals) {
if (is_function && is_native) attr |= READ_ONLY;
if (!is_eval) attr |= DONT_DELETE;
// ES#sec-globaldeclarationinstantiation 5.d:
// If hasRestrictedGlobal is true, throw a SyntaxError exception.
Object* result = DeclareGlobals(
isolate, global, name, value, static_cast<PropertyAttributes>(attr),
is_var, is_function, RedeclarationType::kSyntaxError);
Object* result = DeclareGlobals(isolate, global, name, value,
static_cast<PropertyAttributes>(attr),
is_var, is_function);
if (isolate->has_pending_exception()) return result;
});
@ -234,13 +215,7 @@ Object* DeclareLookupSlot(Isolate* isolate, Handle<String> name,
context_arg->Lookup(name, LEXICAL_TEST, &index, &attributes,
&binding_flags);
if (attributes != ABSENT && binding_flags == BINDING_CHECK_INITIALIZED) {
// ES#sec-evaldeclarationinstantiation 5.a.i.1:
// If varEnvRec.HasLexicalDeclaration(name) is true, throw a SyntaxError
// exception.
// ES#sec-evaldeclarationinstantiation 5.d.ii.2.a.i:
// Throw a SyntaxError exception.
return ThrowRedeclarationError(isolate, name,
RedeclarationType::kSyntaxError);
return ThrowRedeclarationError(isolate, name);
}
attr = static_cast<PropertyAttributes>(attr & ~EVAL_DECLARED);
}
@ -260,30 +235,26 @@ Object* DeclareLookupSlot(Isolate* isolate, Handle<String> name,
// TODO(verwaest): This case should probably not be covered by this function,
// but by DeclareGlobals instead.
if (attributes != ABSENT && holder->IsJSGlobalObject()) {
// ES#sec-evaldeclarationinstantiation 8.a.iv.1.b:
// If fnDefinable is false, throw a TypeError exception.
return DeclareGlobals(isolate, Handle<JSGlobalObject>::cast(holder), name,
value, attr, is_var, is_function,
RedeclarationType::kTypeError);
value, attr, is_var, is_function);
}
if (context_arg->extension()->IsJSGlobalObject()) {
Handle<JSGlobalObject> global(
JSGlobalObject::cast(context_arg->extension()), isolate);
return DeclareGlobals(isolate, global, name, value, attr, is_var,
is_function, RedeclarationType::kTypeError);
is_function);
} else if (context->IsScriptContext()) {
DCHECK(context->global_object()->IsJSGlobalObject());
Handle<JSGlobalObject> global(
JSGlobalObject::cast(context->global_object()), isolate);
return DeclareGlobals(isolate, global, name, value, attr, is_var,
is_function, RedeclarationType::kTypeError);
is_function);
}
if (attributes != ABSENT) {
// The name was declared before; check for conflicting re-declarations.
if ((attributes & READ_ONLY) != 0) {
return ThrowRedeclarationError(isolate, name,
RedeclarationType::kSyntaxError);
return ThrowRedeclarationError(isolate, name);
}
// Skip var re-declarations.
@ -622,11 +593,7 @@ static Object* FindNameClash(Handle<ScopeInfo> scope_info,
ScriptContextTable::LookupResult lookup;
if (ScriptContextTable::Lookup(script_context, name, &lookup)) {
if (IsLexicalVariableMode(mode) || IsLexicalVariableMode(lookup.mode)) {
// ES#sec-globaldeclarationinstantiation 5.b:
// If envRec.HasLexicalDeclaration(name) is true, throw a SyntaxError
// exception.
return ThrowRedeclarationError(isolate, name,
RedeclarationType::kSyntaxError);
return ThrowRedeclarationError(isolate, name);
}
}
@ -636,13 +603,7 @@ static Object* FindNameClash(Handle<ScopeInfo> scope_info,
Maybe<PropertyAttributes> maybe = JSReceiver::GetPropertyAttributes(&it);
if (!maybe.IsJust()) return isolate->heap()->exception();
if ((maybe.FromJust() & DONT_DELETE) != 0) {
// ES#sec-globaldeclarationinstantiation 5.a:
// If envRec.HasVarDeclaration(name) is true, throw a SyntaxError
// exception.
// ES#sec-globaldeclarationinstantiation 5.d:
// If hasRestrictedGlobal is true, throw a SyntaxError exception.
return ThrowRedeclarationError(isolate, name,
RedeclarationType::kSyntaxError);
return ThrowRedeclarationError(isolate, name);
}
JSGlobalObject::InvalidatePropertyCell(global_object, name);

12
test/mjsunit/es6/block-eval-var-over-let.js
View File

@ -8,13 +8,13 @@
assertThrows(function() {
let x = 1;
eval('var x');
}, SyntaxError);
}, TypeError);
// If the eval is in its own block scope, throws
assertThrows(function() {
let y = 1;
{ eval('var y'); }
}, SyntaxError);
}, TypeError);
// If the let is in its own block scope, with the eval, throws
assertThrows(function() {
@ -22,7 +22,7 @@ assertThrows(function() {
let x = 1;
eval('var x');
}
}, SyntaxError);
}, TypeError);
// Legal if the let is no longer visible
assertDoesNotThrow(function() {
@ -37,13 +37,13 @@ assertDoesNotThrow(function() {
assertThrows(function() {
const x = 1;
eval('var x');
}, SyntaxError);
}, TypeError);
// If the eval is in its own block scope, throws
assertThrows(function() {
const y = 1;
{ eval('var y'); }
}, SyntaxError);
}, TypeError);
// If the const is in its own block scope, with the eval, throws
assertThrows(function() {
@ -51,7 +51,7 @@ assertThrows(function() {
const x = 1;
eval('var x');
}
}, SyntaxError);
}, TypeError);
// Legal if the const is no longer visible
assertDoesNotThrow(function() {

145
test/mjsunit/regress/redeclaration-error-types.js
View File

@ -1,145 +0,0 @@
// Copyright 2016 the V8 project authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
function doTest(scripts, expectedError) {
var realm = Realm.create();
for (var i = 0; i < scripts.length - 1; i++) {
Realm.eval(realm, scripts[i]);
}
assertThrows(function() {
Realm.eval(realm, scripts[scripts.length - 1]);
}, Realm.eval(realm, expectedError));
Realm.dispose(realm);
}
var tests = [
{
// ES#sec-globaldeclarationinstantiation 5.a:
// If envRec.HasVarDeclaration(name) is true, throw a SyntaxError
// exception.
scripts: [
"var a;",
"let a;",
],
expectedError: "SyntaxError",
},
{
// ES#sec-globaldeclarationinstantiation 6.a:
// If envRec.HasLexicalDeclaration(name) is true, throw a SyntaxError
// exception.
scripts: [
"let a;",
"var a;",
],
expectedError: "SyntaxError",
},
{
// ES#sec-globaldeclarationinstantiation 5.b:
// If envRec.HasLexicalDeclaration(name) is true, throw a SyntaxError
// exception.
scripts: [
"let a;",
"let a;",
],
expectedError: "SyntaxError",
},
{
// ES#sec-evaldeclarationinstantiation 5.a.i.1:
// If varEnvRec.HasLexicalDeclaration(name) is true, throw a SyntaxError
// exception.
scripts: [
'let a; eval("var a;");',
],
expectedError: "SyntaxError",
},
{
// ES#sec-evaldeclarationinstantiation 5.a.i.1:
// If varEnvRec.HasLexicalDeclaration(name) is true, throw a SyntaxError
// exception.
scripts: [
'let a; eval("function a() {}");',
],
expectedError: "SyntaxError",
},
{
// ES#sec-evaldeclarationinstantiation 5.d.ii.2.a.i:
// Throw a SyntaxError exception.
scripts: [
'(function() { let a; eval("var a;"); })();',
],
expectedError: "SyntaxError",
},
{
// ES#sec-evaldeclarationinstantiation 5.d.ii.2.a.i:
// Throw a SyntaxError exception.
scripts: [
'(function() { let a; eval("function a() {}"); })();',
],
expectedError: "SyntaxError",
},
{
// ES#sec-globaldeclarationinstantiation 5.d:
// If hasRestrictedGlobal is true, throw a SyntaxError exception.
scripts: [
'let NaN;',
],
expectedError: "SyntaxError",
},
{
// ES#sec-globaldeclarationinstantiation 5.d:
// If hasRestrictedGlobal is true, throw a SyntaxError exception.
scripts: [
'function NaN() {}',
],
expectedError: "SyntaxError",
},
{
// ES#sec-evaldeclarationinstantiation 8.a.iv.1.b:
// If fnDefinable is false, throw a TypeError exception.
scripts: [
'eval("function NaN() {}");',
],
expectedError: "TypeError",
},
{
// ES#sec-evaldeclarationinstantiation 8.a.iv.1.b:
// If fnDefinable is false, throw a TypeError exception.
scripts: [
`
let a;
try {
eval("function a() {}");
} catch (e) {}
eval("function NaN() {}");
`,
],
expectedError: "TypeError",
},
{
// ES#sec-evaldeclarationinstantiation 8.a.iv.1.b:
// If fnDefinable is false, throw a TypeError exception.
scripts: [
`
eval("
function f() {
function b() {
(0, eval)('function NaN() {}');
}
b();
}
f();
");
`.replace(/"/g, '`'),
],
expectedError: "TypeError",
},
];
tests.forEach(function(test) {
doTest(test.scripts, test.expectedError);
});

5
test/test262/test262.status
View File

@ -263,6 +263,11 @@
'language/eval-code/indirect/non-definable-function-with-function': [FAIL],
'language/eval-code/indirect/non-definable-function-with-variable': [FAIL],
# https://bugs.chromium.org/p/v8/issues/detail?id=4955
'language/eval-code/direct/var-env-global-lex-non-strict': [FAIL],
'language/eval-code/direct/var-env-lower-lex-non-strict': [FAIL],
'language/eval-code/indirect/var-env-global-lex-non-strict': [FAIL],
# https://bugs.chromium.org/p/v8/issues/detail?id=4124
'built-ins/Simd/*': [SKIP],