[strong] Check super constructor calls

R=marja@chromium.org
BUG=v8:3956
LOG=N

Review URL: https://codereview.chromium.org/1002253002

Cr-Commit-Position: refs/heads/master@{#27188}

src/messages.js

@@ -169,6 +169,9 @@ var kMessages = {
   strong_for_in:                 ["Please don't use 'for'-'in' loops in strong mode, use 'for'-'of' instead"],
   strong_empty:                  ["Please don't use empty sub-statements in strong mode, make them explicit with '{}' instead"],
   strong_use_before_declaration: ["Please declare variable '", "%0", "' before use in strong mode"],
+  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"],
   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."],

src/parser.cc

@@ -1192,8 +1192,27 @@ void* Parser::ParseStatementList(ZoneList<Statement*>* body, int end_token,
       directive_prologue = false;
     }
 
+    Token::Value token = peek();
     Scanner::Location token_loc = scanner()->peek_location();
+    Scanner::Location old_super_loc = function_state_->super_call_location();
     Statement* stat = ParseStatementListItem(CHECK_OK);
+    Scanner::Location super_loc = function_state_->super_call_location();
+
+    if (is_strong(language_mode()) &&
+        i::IsConstructor(function_state_->kind()) &&
+        !old_super_loc.IsValid() && super_loc.IsValid() &&
+        token != Token::SUPER) {
+      // TODO(rossberg): This is more permissive than spec'ed, it allows e.g.
+      //   super(), 1;
+      //   super() + "";
+      //   super() = 0;
+      // That should still be safe, though, thanks to left-to-right evaluation.
+      // The proper check would be difficult to implement in the preparser.
+      ReportMessageAt(super_loc, "strong_super_call_nested");
+      *ok = false;
+      return NULL;
+    }
+
     if (stat == NULL || stat->IsEmpty()) {
       directive_prologue = false;   // End of directive prologue.
       continue;
@@ -3916,6 +3935,14 @@ FunctionLiteral* Parser::ParseFunctionLiteral(
     if (is_strict(language_mode())) {
       CheckConflictingVarDeclarations(scope, CHECK_OK);
     }
+    if (is_strong(language_mode()) && IsSubclassConstructor(kind)) {
+      if (!function_state.super_call_location().IsValid()) {
+        ReportMessageAt(function_name_location, "strong_super_call_missing",
+                        kReferenceError);
+        *ok = false;
+        return nullptr;
+      }
+    }
   }
 
   FunctionLiteral* function_literal = factory()->NewFunctionLiteral(

src/preparser.cc

@@ -193,8 +193,19 @@ void PreParser::ParseStatementList(int end_token, bool* ok) {
     if (directive_prologue && peek() != Token::STRING) {
       directive_prologue = false;
     }
+    Token::Value token = peek();
+    Scanner::Location old_super_loc = function_state_->super_call_location();
     Statement statement = ParseStatementListItem(ok);
     if (!*ok) return;
+    Scanner::Location super_loc = function_state_->super_call_location();
+    if (is_strong(language_mode()) &&
+        i::IsConstructor(function_state_->kind()) &&
+        !old_super_loc.IsValid() && super_loc.IsValid() &&
+        token != Token::SUPER) {
+      ReportMessageAt(super_loc, "strong_super_call_nested");
+      *ok = false;
+      return;
+    }
     if (directive_prologue) {
       if (statement.IsUseStrictLiteral()) {
         scope_->SetLanguageMode(
@@ -938,6 +949,15 @@ PreParser::Expression PreParser::ParseFunctionLiteral(
     CheckStrictOctalLiteral(start_position, end_position, CHECK_OK);
   }
 
+  if (is_strong(language_mode()) && IsSubclassConstructor(kind)) {
+    if (!function_state.super_call_location().IsValid()) {
+      ReportMessageAt(function_name_location, "strong_super_call_missing",
+                      kReferenceError);
+      *ok = false;
+      return Expression::Default();
+    }
+  }
+
   return Expression::Default();
 }
 

src/preparser.h

@@ -217,6 +217,13 @@ class ParserBase : public Traits {
     void AddProperty() { expected_property_count_++; }
     int expected_property_count() { return expected_property_count_; }
 
+    Scanner::Location super_call_location() const {
+      return super_call_location_;
+    }
+    void set_super_call_location(Scanner::Location location) {
+      super_call_location_ = location;
+    }
+
     bool is_generator() const { return IsGeneratorFunction(kind_); }
 
     FunctionKind kind() const { return kind_; }
@@ -247,6 +254,9 @@ class ParserBase : public Traits {
     // Properties count estimation.
     int expected_property_count_;
 
+    // Location of call to the "super" constructor (invalid if none).
+    Scanner::Location super_call_location_;
+
     FunctionKind kind_;
     // For generators, this variable may hold the generator object. It variable
     // is used by yield expressions and return statements. It is not necessary
@@ -1657,6 +1667,7 @@ ParserBase<Traits>::FunctionState::FunctionState(
     : next_materialized_literal_index_(0),
       next_handler_index_(0),
       expected_property_count_(0),
+      super_call_location_(Scanner::Location::invalid()),
       kind_(kind),
       generator_object_variable_(NULL),
       function_state_stack_(function_state_stack),
@@ -2781,6 +2792,13 @@ 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();
+      }
+      function_state->set_super_call_location(scanner()->location());
       return this->SuperReference(scope_, factory());
     }
   }
@@ -2866,6 +2884,7 @@ ParserBase<Traits>::ParseArrowFunctionLiteral(int start_pos,
   int materialized_literal_count = -1;
   int expected_property_count = -1;
   int handler_count = 0;
+  Scanner::Location super_loc;
 
   {
     typename Traits::Type::Factory function_factory(ast_value_factory());
@@ -2921,6 +2940,7 @@ ParserBase<Traits>::ParseArrowFunctionLiteral(int start_pos,
       expected_property_count = function_state.expected_property_count();
       handler_count = function_state.handler_count();
     }
+    super_loc = function_state.super_call_location();
 
     scope->set_start_position(start_pos);
     scope->set_end_position(scanner()->location().end_pos);
@@ -2951,6 +2971,7 @@ ParserBase<Traits>::ParseArrowFunctionLiteral(int start_pos,
       start_pos);
 
   function_literal->set_function_token_position(start_pos);
+  if (super_loc.IsValid()) function_state_->set_super_call_location(super_loc);
 
   if (fni_ != NULL) this->InferFunctionName(fni_, function_literal);
 

test/cctest/test-parsing.cc

@@ -5548,6 +5548,36 @@ TEST(StrongForIn) {
 }
 
 
+TEST(StrongSuperCalls) {
+  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() {} }",
+      "class C extends Object { constructor() { (super()); } }",
+      "class C extends Object { constructor() { (() => super())(); } }",
+      "class C extends Object { constructor() { { super(); } } }",
+      "class C extends Object { constructor() { if (1) super(); } }",
+      "class C extends Object { constructor() { super(), super(); } }",
+      "class C extends Object { constructor() { super(); super(); } }",
+      "class C extends Object { constructor() { super(); (super()); } }",
+      "class C extends Object { constructor() { super(); { super() } } }",
+      NULL};
+
+  static const ParserFlag always_flags[] = {
+      kAllowStrongMode, kAllowHarmonyClasses, kAllowHarmonyObjectLiterals,
+      kAllowHarmonyArrowFunctions
+  };
+  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}};

test/mjsunit/strong/classes.js

@@ -3,15 +3,44 @@
 // found in the LICENSE file.
 
 // Flags: --strong-mode
+// Flags: --harmony-classes --harmony-arrow-functions
 
 'use strong';
 
 class C {}
 
+function assertTypeError(script) { assertThrows(script, TypeError) }
+function assertSyntaxError(script) { assertThrows(script, SyntaxError) }
+function assertReferenceError(script) { assertThrows(script, ReferenceError) }
+
 (function ImmutableClassBindings() {
   class D {}
-  assertThrows(function(){ eval("C = 0") }, TypeError);
-  assertThrows(function(){ eval("D = 0") }, TypeError);
+  assertTypeError(function(){ eval("C = 0") });
+  assertTypeError(function(){ eval("D = 0") });
   assertEquals('function', typeof C);
   assertEquals('function', typeof D);
 })();
+
+function constructor(body) {
+  return "'use strong'; " +
+      "(class extends Object { constructor() { " + body + " } })";
+}
+
+(function NoMissingSuper() {
+  assertReferenceError(constructor(""));
+  assertReferenceError(constructor("1"));
+})();
+
+(function NoNestedSuper() {
+  assertSyntaxError(constructor("(super())"));
+  assertSyntaxError(constructor("(() => super())(); } })"));
+  assertSyntaxError(constructor("{ super();"));
+  assertSyntaxError(constructor("if (1) super();"));
+})();
+
+(function NoDuplicateSuper() {
+  assertSyntaxError(constructor("super(), super();"));
+  assertSyntaxError(constructor("super(); super();"));
+  assertSyntaxError(constructor("super(); (super());"));
+  assertSyntaxError(constructor("super(); { super() }"));
+})();