[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}

diff --git a/src/messages.js b/src/messages.js
index 8da7bb1..a3a2dd9 100644 (file)
--- 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 (file)
--- 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 (file)
--- 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 (file)
--- 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<Traits>::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<Traits>::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 (file)
--- 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 (file)
--- 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();"));
 })();