From 74cf8e0122559680fd94dffa9a322edf4b34260d Mon Sep 17 00:00:00 2001 From: rossberg Date: Thu, 19 Mar 2015 12:39:53 -0700 Subject: [PATCH] [strong] Check constructor returns R=dslomov@chromium.org BUG=v8:3956 LOG=N Review URL: https://codereview.chromium.org/1019283002 Cr-Commit-Position: refs/heads/master@{#27320} --- src/messages.js | 2 ++ src/parser.cc | 9 +++++++++ src/preparser.cc | 9 +++++++++ src/preparser.h | 24 +++++++++++++++++++----- test/cctest/test-parsing.cc | 26 ++++++++++++++++++++++++++ test/mjsunit/strong/classes.js | 20 +++++++++++++++++--- 6 files changed, 82 insertions(+), 8 deletions(-) diff --git a/src/messages.js b/src/messages.js index 8da7bb1..a3a2dd9 100644 --- a/src/messages.js +++ b/src/messages.js @@ -175,6 +175,8 @@ var kMessages = { strong_super_call_missing: ["Please always invoke the super constructor in subclasses in strong mode"], strong_super_call_duplicate: ["Please don't invoke the super constructor multiple times in strong mode"], strong_super_call_nested: ["Please don't invoke the super constructor nested inside another statement or expression in strong mode"], + strong_constructor_return_value: ["Please do not return a value from a constructor in strong mode"], + strong_constructor_return_misplaced: ["Please do not return from a constructor before its super constructor invocation in strong mode"], sloppy_lexical: ["Block-scoped declarations (let, const, function, class) not yet supported outside strict mode"], malformed_arrow_function_parameter_list: ["Malformed arrow function parameter list"], generator_poison_pill: ["'caller' and 'arguments' properties may not be accessed on generator functions."], diff --git a/src/parser.cc b/src/parser.cc index 4bcdfdb..99e6600 100644 --- a/src/parser.cc +++ b/src/parser.cc @@ -2707,6 +2707,7 @@ Statement* Parser::ParseReturnStatement(bool* ok) { // reported (underlining). Expect(Token::RETURN, CHECK_OK); Scanner::Location loc = scanner()->location(); + function_state_->set_return_location(loc); Token::Value tok = peek(); Statement* result; @@ -2721,6 +2722,14 @@ Statement* Parser::ParseReturnStatement(bool* ok) { return_value = GetLiteralUndefined(position()); } } else { + if (is_strong(language_mode()) && + i::IsConstructor(function_state_->kind())) { + int pos = peek_position(); + ReportMessageAt(Scanner::Location(pos, pos + 1), + "strong_constructor_return_value"); + *ok = false; + return NULL; + } return_value = ParseExpression(true, CHECK_OK); } ExpectSemicolon(CHECK_OK); diff --git a/src/preparser.cc b/src/preparser.cc index 914f119..5a6a094 100644 --- a/src/preparser.cc +++ b/src/preparser.cc @@ -613,6 +613,7 @@ PreParser::Statement PreParser::ParseReturnStatement(bool* ok) { // reporting any errors on it, because of the way errors are // reported (underlining). Expect(Token::RETURN, CHECK_OK); + function_state_->set_return_location(scanner()->location()); // An ECMAScript program is considered syntactically incorrect if it // contains a return statement that is not within the body of a @@ -624,6 +625,14 @@ PreParser::Statement PreParser::ParseReturnStatement(bool* ok) { tok != Token::SEMICOLON && tok != Token::RBRACE && tok != Token::EOS) { + if (is_strong(language_mode()) && + i::IsConstructor(function_state_->kind())) { + int pos = peek_position(); + ReportMessageAt(Scanner::Location(pos, pos + 1), + "strong_constructor_return_value"); + *ok = false; + return Statement::Default(); + } ParseExpression(true, CHECK_OK); } ExpectSemicolon(CHECK_OK); diff --git a/src/preparser.h b/src/preparser.h index 17247e8..d53447c 100644 --- a/src/preparser.h +++ b/src/preparser.h @@ -217,9 +217,13 @@ class ParserBase : public Traits { void AddProperty() { expected_property_count_++; } int expected_property_count() { return expected_property_count_; } + Scanner::Location return_location() const { return return_location_; } Scanner::Location super_call_location() const { return super_call_location_; } + void set_return_location(Scanner::Location location) { + return_location_ = location; + } void set_super_call_location(Scanner::Location location) { super_call_location_ = location; } @@ -254,6 +258,9 @@ class ParserBase : public Traits { // Properties count estimation. int expected_property_count_; + // Location of most recent 'return' statement (invalid if none). + Scanner::Location return_location_; + // Location of call to the "super" constructor (invalid if none). Scanner::Location super_call_location_; @@ -1667,6 +1674,7 @@ ParserBase::FunctionState::FunctionState( : next_materialized_literal_index_(0), next_handler_index_(0), expected_property_count_(0), + return_location_(Scanner::Location::invalid()), super_call_location_(Scanner::Location::invalid()), kind_(kind), generator_object_variable_(NULL), @@ -2796,11 +2804,17 @@ ParserBase::ParseSuperExpression(bool is_new, bool* ok) { // new super() is never allowed. // super() is only allowed in derived constructor if (!is_new && peek() == Token::LPAREN && IsSubclassConstructor(kind)) { - if (is_strong(language_mode()) && - function_state->super_call_location().IsValid()) { - ReportMessageAt(scanner()->location(), "strong_super_call_duplicate"); - *ok = false; - return this->EmptyExpression(); + if (is_strong(language_mode())) { + if (function_state->super_call_location().IsValid()) { + ReportMessageAt(scanner()->location(), "strong_super_call_duplicate"); + *ok = false; + return this->EmptyExpression(); + } else if (function_state->return_location().IsValid()) { + ReportMessageAt(function_state->return_location(), + "strong_constructor_return_misplaced"); + *ok = false; + return this->EmptyExpression(); + } } function_state->set_super_call_location(scanner()->location()); return this->SuperReference(scope_, factory()); diff --git a/test/cctest/test-parsing.cc b/test/cctest/test-parsing.cc index dd32835..2e5019f 100644 --- a/test/cctest/test-parsing.cc +++ b/test/cctest/test-parsing.cc @@ -5608,6 +5608,32 @@ TEST(StrongSuperCalls) { } +TEST(StrongConstructorReturns) { + const char* sloppy_context_data[][2] = {{"", ""}, {NULL}}; + const char* strict_context_data[][2] = {{"'use strict';", ""}, {NULL}}; + const char* strong_context_data[][2] = {{"'use strong';", ""}, {NULL}}; + + const char* data[] = { + "class C extends Object { constructor() { super(); return {}; } }", + "class C extends Object { constructor() { super(); { return {}; } } }", + "class C extends Object { constructor() { super(); if (1) return {}; } }", + "class C extends Object { constructor() { return; super(); } }", + "class C extends Object { constructor() { { return; } super(); } }", + "class C extends Object { constructor() { if (0) return; super(); } }", + NULL}; + + static const ParserFlag always_flags[] = { + kAllowStrongMode, kAllowHarmonyClasses, kAllowHarmonyObjectLiterals + }; + RunParserSyncTest(sloppy_context_data, data, kError, NULL, 0, always_flags, + arraysize(always_flags)); + RunParserSyncTest(strict_context_data, data, kSuccess, NULL, 0, always_flags, + arraysize(always_flags)); + RunParserSyncTest(strong_context_data, data, kError, NULL, 0, always_flags, + arraysize(always_flags)); +} + + TEST(ArrowFunctionASIErrors) { const char* context_data[][2] = {{"'use strict';", ""}, {"", ""}, {NULL, NULL}}; diff --git a/test/mjsunit/strong/classes.js b/test/mjsunit/strong/classes.js index d5c4560..e33742a 100644 --- a/test/mjsunit/strong/classes.js +++ b/test/mjsunit/strong/classes.js @@ -32,9 +32,9 @@ function constructor(body) { })(); (function NoNestedSuper() { - assertSyntaxError(constructor("(super())")); - assertSyntaxError(constructor("(() => super())(); } })")); - assertSyntaxError(constructor("{ super();")); + assertSyntaxError(constructor("(super());")); + assertSyntaxError(constructor("(() => super())();")); + assertSyntaxError(constructor("{ super(); }")); assertSyntaxError(constructor("if (1) super();")); })(); @@ -43,4 +43,18 @@ function constructor(body) { assertSyntaxError(constructor("super(); super();")); assertSyntaxError(constructor("super(); (super());")); assertSyntaxError(constructor("super(); { super() }")); + assertSyntaxError(constructor("super(); (() => super())();")); +})(); + +(function NoReturnValue() { + assertSyntaxError(constructor("return {};")); + assertSyntaxError(constructor("return undefined;")); + assertSyntaxError(constructor("{ return {}; }")); + assertSyntaxError(constructor("if (1) return {};")); +})(); + +(function NoReturnBeforeSuper() { + assertSyntaxError(constructor("return; super();")); + assertSyntaxError(constructor("if (0) return; super();")); + assertSyntaxError(constructor("{ return; } super();")); })(); -- 2.7.4