Revision: 20434
Author: [email protected]
Date: Wed Apr 2 12:38:01 2014 UTC
Log: Make stray 'return' an early error
As required by the spec, and implemented by other browsers.
(Plus minor clean-up for redeclaration TypeErrors.)
[email protected]
BUG=
LOG=Y
Review URL: https://codereview.chromium.org/220473014
http://code.google.com/p/v8/source/detail?r=20434
Modified:
/branches/bleeding_edge/src/messages.js
/branches/bleeding_edge/src/parser.cc
/branches/bleeding_edge/src/parser.h
/branches/bleeding_edge/src/preparser.cc
/branches/bleeding_edge/src/preparser.h
/branches/bleeding_edge/src/runtime.cc
/branches/bleeding_edge/test/cctest/test-parsing.cc
/branches/bleeding_edge/test/mjsunit/debug-stepout-scope-part2.js
/branches/bleeding_edge/test/mjsunit/debug-stepout-scope-part3.js
/branches/bleeding_edge/test/mjsunit/debug-stepout-scope-part4.js
/branches/bleeding_edge/test/mjsunit/debug-stepout-scope-part5.js
/branches/bleeding_edge/test/mjsunit/debug-stepout-scope-part6.js
/branches/bleeding_edge/test/mjsunit/delay-syntax-error.js
/branches/bleeding_edge/test/webkit/fast/js/arguments-expected.txt
=======================================
--- /branches/bleeding_edge/src/messages.js Wed Apr 2 08:21:37 2014 UTC
+++ /branches/bleeding_edge/src/messages.js Wed Apr 2 12:38:01 2014 UTC
@@ -47,7 +47,8 @@
incompatible_method_receiver: ["Method ", "%0", " called on
incompatible receiver ", "%1"],
multiple_defaults_in_switch: ["More than one default clause in switch
statement"],
newline_after_throw: ["Illegal newline after throw"],
- redeclaration: ["%0", " '", "%1", "' has already been
declared"],
+ label_redeclaration: ["Label '", "%0", "' has already been
declared"],
+ var_redeclaration: ["Identifier '", "%0", "' has already
been declared"],
no_catch_or_finally: ["Missing catch or finally after try"],
unknown_label: ["Undefined label '", "%0", "'"],
uncaught_exception: ["Uncaught ", "%0"],
=======================================
--- /branches/bleeding_edge/src/parser.cc Wed Apr 2 11:03:05 2014 UTC
+++ /branches/bleeding_edge/src/parser.cc Wed Apr 2 12:38:01 2014 UTC
@@ -590,8 +590,7 @@
}
-Expression* ParserTraits::NewThrowReferenceError(
- const char* message, int pos) {
+Expression* ParserTraits::NewThrowReferenceError(const char* message, int
pos) {
return NewThrowError(
parser_->isolate()->factory()->MakeReferenceError_string(),
message, HandleVector<Object>(NULL, 0), pos);
@@ -609,11 +608,9 @@
Expression* ParserTraits::NewThrowTypeError(
- const char* message, Handle<Object> arg1, Handle<Object> arg2, int
pos) {
- ASSERT(!arg1.is_null() && !arg2.is_null());
- Handle<Object> elements[] = { arg1, arg2 };
- Vector< Handle<Object> > arguments =
- HandleVector<Object>(elements, ARRAY_SIZE(elements));
+ const char* message, Handle<Object> arg, int pos) {
+ int argc = arg.is_null() ? 0 : 1;
+ Vector< Handle<Object> > arguments = HandleVector<Object>(&arg, argc);
return NewThrowError(
parser_->isolate()->factory()->MakeTypeError_string(),
message, arguments, pos);
@@ -1754,18 +1751,14 @@
// In harmony we treat re-declarations as early errors. See
// ES5 16 for a definition of early errors.
SmartArrayPointer<char> c_string = name->ToCString(DISALLOW_NULLS);
- const char* elms[2] = { "Variable", c_string.get() };
- Vector<const char*> args(elms, 2);
- ReportMessage("redeclaration", args);
+ const char* elms[1] = { c_string.get() };
+ Vector<const char*> args(elms, 1);
+ ReportMessage("var_redeclaration", args);
*ok = false;
return;
}
- Handle<String> message_string =
- isolate()->factory()->InternalizeOneByteString(
- STATIC_ASCII_VECTOR("Variable"));
- Expression* expression =
- NewThrowTypeError("redeclaration",
- message_string, name, declaration->position());
+ Expression* expression = NewThrowTypeError(
+ "var_redeclaration", name, declaration->position());
declaration_scope->SetIllegalRedeclaration(expression);
}
}
@@ -2374,9 +2367,9 @@
// make later anyway so we should go back and fix this then.
if (ContainsLabel(labels, label) || TargetStackContainsLabel(label)) {
SmartArrayPointer<char> c_string = label->ToCString(DISALLOW_NULLS);
- const char* elms[2] = { "Label", c_string.get() };
- Vector<const char*> args(elms, 2);
- ReportMessage("redeclaration", args);
+ const char* elms[1] = { c_string.get() };
+ Vector<const char*> args(elms, 1);
+ ReportMessage("label_redeclaration", args);
*ok = false;
return NULL;
}
@@ -2521,7 +2514,7 @@
// reporting any errors on it, because of the way errors are
// reported (underlining).
Expect(Token::RETURN, CHECK_OK);
- int pos = position();
+ Scanner::Location loc = scanner()->location();
Token::Value tok = peek();
Statement* result;
@@ -2539,23 +2532,17 @@
Expression* generator = factory()->NewVariableProxy(
function_state_->generator_object_variable());
Expression* yield = factory()->NewYield(
- generator, return_value, Yield::FINAL, pos);
- result = factory()->NewExpressionStatement(yield, pos);
+ generator, return_value, Yield::FINAL, loc.beg_pos);
+ result = factory()->NewExpressionStatement(yield, loc.beg_pos);
} else {
- result = factory()->NewReturnStatement(return_value, pos);
+ result = factory()->NewReturnStatement(return_value, loc.beg_pos);
}
- // An ECMAScript program is considered syntactically incorrect if it
- // contains a return statement that is not within the body of a
- // function. See ECMA-262, section 12.9, page 67.
- //
- // To be consistent with KJS we report the syntax error at runtime.
- Scope* declaration_scope = scope_->DeclarationScope();
- if (declaration_scope->is_global_scope() ||
- declaration_scope->is_eval_scope()) {
- Expression* throw_error =
- NewThrowSyntaxError("illegal_return", Handle<Object>::null(), pos);
- return factory()->NewExpressionStatement(throw_error, pos);
+ Scope* decl_scope = scope_->DeclarationScope();
+ if (decl_scope->is_global_scope() || decl_scope->is_eval_scope()) {
+ ReportMessageAt(loc, "illegal_return");
+ *ok = false;
+ return NULL;
}
return result;
}
@@ -3669,13 +3656,13 @@
// errors. See ES5 16 for a definition of early errors.
Handle<String> name = decl->proxy()->name();
SmartArrayPointer<char> c_string = name->ToCString(DISALLOW_NULLS);
- const char* elms[2] = { "Variable", c_string.get() };
- Vector<const char*> args(elms, 2);
+ const char* elms[1] = { c_string.get() };
+ Vector<const char*> args(elms, 1);
int position = decl->proxy()->position();
Scanner::Location location = position == RelocInfo::kNoPosition
? Scanner::Location::invalid()
: Scanner::Location(position, position + 1);
- ParserTraits::ReportMessageAt(location, "redeclaration", args);
+ ParserTraits::ReportMessageAt(location, "var_redeclaration", args);
*ok = false;
}
}
=======================================
--- /branches/bleeding_edge/src/parser.h Wed Apr 2 11:03:05 2014 UTC
+++ /branches/bleeding_edge/src/parser.h Wed Apr 2 12:38:01 2014 UTC
@@ -538,8 +538,7 @@
// Generate AST node that throws a TypeError with the given
// type. Both arguments must be non-null (in the handle sense).
- Expression* NewThrowTypeError(
- const char* type, Handle<Object> arg1, Handle<Object> arg2, int pos);
+ Expression* NewThrowTypeError(const char* type, Handle<Object> arg, int
pos);
// Generic AST generator for throwing errors from compiled code.
Expression* NewThrowError(
=======================================
--- /branches/bleeding_edge/src/preparser.cc Wed Apr 2 11:03:05 2014 UTC
+++ /branches/bleeding_edge/src/preparser.cc Wed Apr 2 12:38:01 2014 UTC
@@ -582,7 +582,7 @@
// ReturnStatement ::
// 'return' [no line terminator] Expression? ';'
- // Consume the return token. It is necessary to do the before
+ // Consume the return token. It is necessary to do before
// reporting any errors on it, because of the way errors are
// reported (underlining).
Expect(Token::RETURN, CHECK_OK);
=======================================
--- /branches/bleeding_edge/src/preparser.h Wed Apr 2 11:03:05 2014 UTC
+++ /branches/bleeding_edge/src/preparser.h Wed Apr 2 12:38:01 2014 UTC
@@ -922,7 +922,7 @@
return PreParserExpression::Default();
}
PreParserExpression NewThrowTypeError(
- const char* type, Handle<Object> arg1, Handle<Object> arg2, int pos)
{
+ const char* type, Handle<Object> arg, int pos) {
return PreParserExpression::Default();
}
=======================================
--- /branches/bleeding_edge/src/runtime.cc Tue Apr 1 17:43:20 2014 UTC
+++ /branches/bleeding_edge/src/runtime.cc Wed Apr 2 12:38:01 2014 UTC
@@ -2117,15 +2117,11 @@
}
-static Failure* ThrowRedeclarationError(Isolate* isolate,
- const char* type,
- Handle<String> name) {
+static Failure* ThrowRedeclarationError(Isolate* isolate, Handle<String>
name) {
HandleScope scope(isolate);
- Handle<Object> type_handle =
- isolate->factory()->NewStringFromAscii(CStrVector(type));
- Handle<Object> args[2] = { type_handle, name };
- Handle<Object> error =
- isolate->factory()->NewTypeError("redeclaration", HandleVector(args,
2));
+ Handle<Object> args[1] = { name };
+ Handle<Object> error = isolate->factory()->NewTypeError(
+ "var_redeclaration", HandleVector(args, 1));
return isolate->Throw(*error);
}
@@ -2202,7 +2198,7 @@
if (lookup.IsFound() && lookup.IsDontDelete()) {
if (lookup.IsReadOnly() || lookup.IsDontEnum() ||
lookup.IsPropertyCallbacks()) {
- return ThrowRedeclarationError(isolate, "function", name);
+ return ThrowRedeclarationError(isolate, name);
}
// If the existing property is not configurable, keep its
attributes.
attr = lookup.GetAttributes();
@@ -2254,8 +2250,7 @@
if (((attributes & READ_ONLY) != 0) || (mode == READ_ONLY)) {
// Functions are not read-only.
ASSERT(mode != READ_ONLY || initial_value->IsTheHole());
- const char* type = ((attributes & READ_ONLY) != 0) ? "const" : "var";
- return ThrowRedeclarationError(isolate, type, name);
+ return ThrowRedeclarationError(isolate, name);
}
// Initialize it if necessary.
@@ -2309,7 +2304,7 @@
LookupResult lookup(isolate);
object->Lookup(*name, &lookup);
if (lookup.IsPropertyCallbacks()) {
- return ThrowRedeclarationError(isolate, "const", name);
+ return ThrowRedeclarationError(isolate, name);
}
}
if (object->IsJSGlobalObject()) {
=======================================
--- /branches/bleeding_edge/test/cctest/test-parsing.cc Wed Apr 2 11:03:05
2014 UTC
+++ /branches/bleeding_edge/test/cctest/test-parsing.cc Wed Apr 2 12:38:01
2014 UTC
@@ -1480,9 +1480,10 @@
"break",
"break label",
"break\nlabel",
- "return",
- "return 12",
- "return\n12",
+ // TODO(marja): activate once parsing 'return' is merged into
ParserBase.
+ // "return",
+ // "return 12",
+ // "return\n12",
"with ({}) ;",
"with ({}) {}",
"with ({}) 12",
=======================================
--- /branches/bleeding_edge/test/mjsunit/debug-stepout-scope-part2.js Mon
Sep 24 11:18:29 2012 UTC
+++ /branches/bleeding_edge/test/mjsunit/debug-stepout-scope-part2.js Wed
Apr 2 12:38:01 2014 UTC
@@ -54,7 +54,7 @@
var q = 42;
var prefixes = [ "debugger; ",
- "if (false) { try { throw 0; } catch(x) { return x; } };
debugger; " ];
+ "if (false) { try { throw 0; } catch(x) { this.x = x; }
}; debugger; " ];
var bodies = [ "1",
"1 ",
"1;",
=======================================
--- /branches/bleeding_edge/test/mjsunit/debug-stepout-scope-part3.js Mon
Sep 24 11:18:29 2012 UTC
+++ /branches/bleeding_edge/test/mjsunit/debug-stepout-scope-part3.js Wed
Apr 2 12:38:01 2014 UTC
@@ -55,7 +55,7 @@
var q = 42;
var prefixes = [
"debugger; ",
- "if (false) { try { throw 0; } catch(x) { return x; } }; debugger; " ];
+ "if (false) { try { throw 0; } catch(x) { this.x = x; } }; debugger; "
];
var with_bodies = [ "with ({}) {}",
"with ({x:1}) x",
"with ({x:1}) x = 1",
=======================================
--- /branches/bleeding_edge/test/mjsunit/debug-stepout-scope-part4.js Mon
Sep 24 11:18:29 2012 UTC
+++ /branches/bleeding_edge/test/mjsunit/debug-stepout-scope-part4.js Wed
Apr 2 12:38:01 2014 UTC
@@ -55,7 +55,7 @@
var q = 42;
var prefixes = [
"debugger; ",
- "if (false) { try { throw 0; } catch(x) { return x; } }; debugger; " ];
+ "if (false) { try { throw 0; } catch(x) { this.x = x; } }; debugger; " ];
var bodies = [ "1",
"1 ",
"1;",
=======================================
--- /branches/bleeding_edge/test/mjsunit/debug-stepout-scope-part5.js Mon
Sep 24 11:18:29 2012 UTC
+++ /branches/bleeding_edge/test/mjsunit/debug-stepout-scope-part5.js Wed
Apr 2 12:38:01 2014 UTC
@@ -54,7 +54,7 @@
var q = 42;
var prefixes = [ "debugger; ",
- "if (false) { try { throw 0; } catch(x) { return x; } };
debugger; " ];
+ "if (false) { try { throw 0; } catch(x) { this.x = x; }
}; debugger; " ];
var with_bodies = [ "with ({}) {}",
"with ({x:1}) x",
"with ({x:1}) x = 1",
=======================================
--- /branches/bleeding_edge/test/mjsunit/debug-stepout-scope-part6.js Mon
Sep 24 11:18:29 2012 UTC
+++ /branches/bleeding_edge/test/mjsunit/debug-stepout-scope-part6.js Wed
Apr 2 12:38:01 2014 UTC
@@ -54,7 +54,7 @@
var q = 42;
var prefixes = [ "debugger; ",
- "if (false) { try { throw 0; } catch(x) { return x; } };
debugger; " ];
+ "if (false) { try { throw 0; } catch(x) { this.x = x; }
}; debugger; " ];
var bodies = [ "1",
"1 ",
"1;",
=======================================
--- /branches/bleeding_edge/test/mjsunit/delay-syntax-error.js Mon Jan 17
09:36:10 2011 UTC
+++ /branches/bleeding_edge/test/mjsunit/delay-syntax-error.js Wed Apr 2
12:38:01 2014 UTC
@@ -25,18 +25,11 @@
// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
-// To be compatible with JSC syntax errors for illegal returns should be
delayed
-// to runtime.
-// Invalid continue and break statements are caught at compile time.
-
-// Do not throw syntax errors for illegal return at compile time.
-assertDoesNotThrow("if (false) return;");
-
-// Throw syntax errors for illegal break and continue at compile time.
+// Throw syntax errors for illegal return, break and continue at compile
time.
+assertThrows("if (false) return;");
assertThrows("if (false) break;");
assertThrows("if (false) continue;");
-// Throw syntax errors for illegal return, break and continue at runtime.
assertThrows("return;");
assertThrows("break;");
assertThrows("continue;");
=======================================
--- /branches/bleeding_edge/test/webkit/fast/js/arguments-expected.txt Thu
Jul 25 19:54:24 2013 UTC
+++ /branches/bleeding_edge/test/webkit/fast/js/arguments-expected.txt Wed
Apr 2 12:38:01 2014 UTC
@@ -157,7 +157,7 @@
PASS argumentsParam(true) is true
PASS argumentsFunctionConstructorParam(true) is true
PASS argumentsVarUndefined() is '[object Arguments]'
-FAIL argumentsConstUndefined() should be [object Arguments]. Threw
exception TypeError: Variable 'arguments' has already been declared
+FAIL argumentsConstUndefined() should be [object Arguments]. Threw
exception TypeError: Identifier 'arguments' has already been declared
PASS argumentCalleeInException() is argumentCalleeInException
PASS shadowedArgumentsApply([true]) is true
PASS shadowedArgumentsLength([]) is 0
--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/d/optout.
