From f09705ab9b82af99735cf7b6346f2091fae4a2dd Mon Sep 17 00:00:00 2001 From: "lrn@chromium.org" Date: Mon, 17 Jan 2011 09:36:10 +0000 Subject: [PATCH] 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= Review URL: http://codereview.chromium.org/6355006 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@6341 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/parser.cc | 50 ++++++++++++++++++++++++--------- src/parser.h | 3 ++ test/mjsunit/delay-syntax-error.js | 17 +++++------ test/mjsunit/regress/regress-1036894.js | 10 +++---- test/mjsunit/regress/regress-990205.js | 6 +++- 5 files changed, 59 insertions(+), 27 deletions(-) diff --git a/src/parser.cc b/src/parser.cc index 3730a45..6ad9ab3 100644 --- a/src/parser.cc +++ b/src/parser.cc @@ -778,7 +778,8 @@ void Parser::ReportMessageAt(Scanner::Location source_location, const char* type, Vector args) { MessageLocation location(script_, - source_location.beg_pos, source_location.end_pos); + source_location.beg_pos, + source_location.end_pos); Handle array = Factory::NewJSArray(args.length()); for (int i = 0; i < args.length(); i++) { SetElement(array, i, Factory::NewStringFromUtf8(CStrVector(args[i]))); @@ -788,6 +789,21 @@ void Parser::ReportMessageAt(Scanner::Location source_location, } +void Parser::ReportMessageAt(Scanner::Location source_location, + const char* type, + Vector > args) { + MessageLocation location(script_, + source_location.beg_pos, + source_location.end_pos); + Handle array = Factory::NewJSArray(args.length()); + for (int i = 0; i < args.length(); i++) { + SetElement(array, i, args[i]); + } + Handle 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 +1709,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 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 > args; + if (!label.is_null()) { + message = "unknown_label"; + args = Vector >(&label, 1); + } + ReportMessageAt(scanner().location(), message, args); + *ok = false; + return NULL; } ExpectSemicolon(CHECK_OK); return new ContinueStatement(target); @@ -1724,12 +1744,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 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 > args; + if (!label.is_null()) { + message = "unknown_label"; + args = Vector >(&label, 1); + } + ReportMessageAt(scanner().location(), message, args); + *ok = false; + return NULL; } ExpectSemicolon(CHECK_OK); return new BreakStatement(target); diff --git a/src/parser.h b/src/parser.h index 460f664..0613a8d 100644 --- a/src/parser.h +++ b/src/parser.h @@ -430,6 +430,9 @@ class Parser { void ReportMessageAt(Scanner::Location loc, const char* message, Vector args); + void ReportMessageAt(Scanner::Location loc, + const char* message, + Vector > args); protected: FunctionLiteral* ParseLazy(Handle info, diff --git a/test/mjsunit/delay-syntax-error.js b/test/mjsunit/delay-syntax-error.js index 4fcb143..64cc142 100644 --- a/test/mjsunit/delay-syntax-error.js +++ b/test/mjsunit/delay-syntax-error.js @@ -25,17 +25,18 @@ // (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 KJS syntax errors for illegal return, break -// and continue should be delayed to runtime. +// 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, 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;"); -// Throw syntax errors for illegal return, break and continue at -// compile time. +// 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 runtime. assertThrows("return;"); assertThrows("break;"); assertThrows("continue;"); diff --git a/test/mjsunit/regress/regress-1036894.js b/test/mjsunit/regress/regress-1036894.js index d89ceda..03ed8f9 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;}};'); diff --git a/test/mjsunit/regress/regress-990205.js b/test/mjsunit/regress/regress-990205.js index 1ab5bf8..b3024c2 100644 --- a/test/mjsunit/regress/regress-990205.js +++ b/test/mjsunit/regress/regress-990205.js @@ -25,11 +25,15 @@ // (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 throw syntax errors early for invalid break and continue statements. +// (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()"); -- 2.7.4