Reviewers: Mads Ager,
Description:
Make invalid break/continue statements an early syntax error.
Previously we delayed the throwing of syntax errors until runtime, so
unreachable errors didn't get reported.
To match a change in JSC, we now stop parsing and report the error
immediately.
BUG=69736
TEST=
Please review this at http://codereview.chromium.org/6355006/
SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge/build-ia32
Affected files:
M src/parser.h
M src/parser.cc
M test/mjsunit/delay-syntax-error.js
M test/mjsunit/regress/regress-1036894.js
M test/mjsunit/regress/regress-990205.js
Index: src/parser.cc
diff --git a/src/parser.cc b/src/parser.cc
index
3730a4577cf6f1e70440637b575f8547d96892a0..bf25e7311eda35a0692579c86b754ab804b3ac2c
100644
--- a/src/parser.cc
+++ b/src/parser.cc
@@ -788,6 +788,20 @@ void Parser::ReportMessageAt(Scanner::Location
source_location,
}
+void Parser::ReportMessageAt(Scanner::Location source_location,
+ const char* type,
+ Vector<Handle<String> > args) {
+ MessageLocation location(script_,
+ source_location.beg_pos,
source_location.end_pos);
+ Handle<JSArray> array = Factory::NewJSArray(args.length());
+ for (int i = 0; i < args.length(); i++) {
+ SetElement(array, i, args[i]);
+ }
+ Handle<Object> result = Factory::NewSyntaxError(type, array);
+ Top::Throw(*result, &location);
+}
+
+
// Base class containing common code for the different finder classes used
by
// the parser.
class ParserFinder {
@@ -1693,12 +1707,16 @@ Statement* Parser::ParseContinueStatement(bool* ok)
{
IterationStatement* target = NULL;
target = LookupContinueTarget(label, CHECK_OK);
if (target == NULL) {
- // Illegal continue statement. To be consistent with KJS we delay
- // reporting of the syntax error until runtime.
- Handle<String> error_type = Factory::illegal_continue_symbol();
- if (!label.is_null()) error_type = Factory::unknown_label_symbol();
- Expression* throw_error = NewThrowSyntaxError(error_type, label);
- return new ExpressionStatement(throw_error);
+ // Illegal continue statement.
+ const char* message = "illegal_continue";
+ Vector<Handle<String> > args;
+ if (!label.is_null()) {
+ message = "unknown_label";
+ args = Vector<Handle<String> >(&label, 1);
+ }
+ ReportMessageAt(scanner().location(), message, args);
+ *ok = false;
+ return NULL;
}
ExpectSemicolon(CHECK_OK);
return new ContinueStatement(target);
@@ -1724,12 +1742,16 @@ Statement*
Parser::ParseBreakStatement(ZoneStringList* labels, bool* ok) {
BreakableStatement* target = NULL;
target = LookupBreakTarget(label, CHECK_OK);
if (target == NULL) {
- // Illegal break statement. To be consistent with KJS we delay
- // reporting of the syntax error until runtime.
- Handle<String> error_type = Factory::illegal_break_symbol();
- if (!label.is_null()) error_type = Factory::unknown_label_symbol();
- Expression* throw_error = NewThrowSyntaxError(error_type, label);
- return new ExpressionStatement(throw_error);
+ // Illegal break statement.
+ const char* message = "illegal_break";
+ Vector<Handle<String> > args;
+ if (!label.is_null()) {
+ message = "unknown_label";
+ args = Vector<Handle<String> >(&label, 1);
+ }
+ ReportMessageAt(scanner().location(), message, args);
+ *ok = false;
+ return NULL;
}
ExpectSemicolon(CHECK_OK);
return new BreakStatement(target);
Index: src/parser.h
diff --git a/src/parser.h b/src/parser.h
index
460f6647facb6bc7680f3dcf28c8ca583b081527..0613a8de99c4aad758ebc825201515aa0f56ca43
100644
--- a/src/parser.h
+++ b/src/parser.h
@@ -430,6 +430,9 @@ class Parser {
void ReportMessageAt(Scanner::Location loc,
const char* message,
Vector<const char*> args);
+ void ReportMessageAt(Scanner::Location loc,
+ const char* message,
+ Vector<Handle<String> > args);
protected:
FunctionLiteral* ParseLazy(Handle<SharedFunctionInfo> info,
Index: test/mjsunit/delay-syntax-error.js
diff --git a/test/mjsunit/delay-syntax-error.js
b/test/mjsunit/delay-syntax-error.js
index
4fcb1435c574c8d609a102b0d9ceaa955e6dae17..25ccd1ba61c94136c574a24c186f652256735fa5
100644
--- a/test/mjsunit/delay-syntax-error.js
+++ b/test/mjsunit/delay-syntax-error.js
@@ -27,15 +27,18 @@
// To be compatible with KJS syntax errors for illegal return, break
// and continue should be delayed to runtime.
+// CHANGED in Jan. 2011. JSC switched to reporting these syntax errors
+// early and we have followed.
-// Do not throw syntax errors for illegal return, break and continue
-// at compile time.
+// Do not throw syntax errors for illegal return at compile time.
assertDoesNotThrow("if (false) return;");
-assertDoesNotThrow("if (false) break;");
-assertDoesNotThrow("if (false) continue;");
+
+// Do throw syntax errors for illegal break and continue at compile time.
+assertThrows("if (false) break;");
+assertThrows("if (false) continue;");
// Throw syntax errors for illegal return, break and continue at
-// compile time.
+// runtime.
assertThrows("return;");
assertThrows("break;");
assertThrows("continue;");
Index: test/mjsunit/regress/regress-1036894.js
diff --git a/test/mjsunit/regress/regress-1036894.js
b/test/mjsunit/regress/regress-1036894.js
index
d89ceda11c0a291ca8a56f0cb0ab8e54e5d28af9..03ed8f9d61087f9fd5fdfb39fd9f037f88349c2b
100644
--- a/test/mjsunit/regress/regress-1036894.js
+++ b/test/mjsunit/regress/regress-1036894.js
@@ -25,14 +25,14 @@
// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
-xeval = function(s) { eval(s); }
-xeval("$=function anonymous() { /*noex*/do {} while(({ get x(x) { break ;
}, set x() { (undefined);} })); }");
+assertThrows("$=function anonymous() { /*noex*/do {} while(({ get x(x) {
break ; }, set x() { (undefined);} })); }");
-foo = function() { eval("$=function anonymous() { /*noex*/do {} while(({
get x(x) { break ; }, set x() { (undefined);} })); }"); }
+function foo() {
+ assertThrows("$=function anonymous() { /*noex*/do {} while(({ get x(x) {
break ; }, set x() { (undefined);} })); }");
+}
foo();
-xeval = function(s) { eval(s); }
-eval("$=function anonymous() { /*noex*/do {} while(({ get x(x) { break ;
}, set x() { (undefined);} })); }");
+assertThrows("$=function anonymous() { /*noex*/do {} while(({ get x(x) {
break ; }, set x() { (undefined);} })); }");
xeval = function(s) { eval(s); }
xeval('$=function(){L: {break L;break L;}};');
Index: test/mjsunit/regress/regress-990205.js
diff --git a/test/mjsunit/regress/regress-990205.js
b/test/mjsunit/regress/regress-990205.js
index
1ab5bf88b4252d475408e946808a557cfadbc90b..981e63f1b7e65c5c70875c696e739095ebd02d7c
100644
--- a/test/mjsunit/regress/regress-990205.js
+++ b/test/mjsunit/regress/regress-990205.js
@@ -25,11 +25,16 @@
// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+// We not throw syntax errors early for invalid break and continue
+// statements. The behavior has changed to make this a syntax error.
+// (Notice that the example isn't valid ECMAScript due to the
+// function declaration that is not at top level.)
+
function f() {
// Force eager compilation of x through the use of eval. The break
// in function x should not try to break out of the enclosing while.
return eval("while(0) function x() { break; }; 42");
};
-assertEquals(42, f());
+assertThrows("f()");
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
