[es6] Fix return checking in derived constructors
authorarv <arv@chromium.org>
Tue, 28 Apr 2015 16:09:21 +0000 (09:09 -0700)
committerCommit bot <commit-bot@chromium.org>
Tue, 28 Apr 2015 16:09:30 +0000 (16:09 +0000)
In a derived class constructor in case undefined is returned, we
should return the receiver. If the return is any other value type
we should throw a TypeError.

BUG=v8:4061
LOG=N
R=dslomov@chromium.org, adamk@chromium.org

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

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

src/messages.js
src/parser.cc
test/mjsunit/harmony/classes-derived-return-type.js [new file with mode: 0644]

index f7469ee..b81215e 100644 (file)
@@ -203,6 +203,7 @@ var kMessages = {
   duplicate_proto:               ["Duplicate __proto__ fields are not allowed in object literals"],
   param_after_rest:              ["Rest parameter must be last formal parameter"],
   constructor_noncallable:       ["Class constructors cannot be invoked without 'new'"],
+  derived_constructor_return:    ["Derived constructors may only return object or undefined"],
   array_not_subclassable:        ["Subclassing Arrays is not currently supported."],
   for_in_loop_initializer:       ["for-in loop variable declaration may not have an initializer."],
   for_of_loop_initializer:       ["for-of loop variable declaration may not have an initializer."],
index 6124f58..feed1e4 100644 (file)
@@ -2873,7 +2873,53 @@ Statement* Parser::ParseReturnStatement(bool* ok) {
       *ok = false;
       return NULL;
     }
+
+    int pos = peek_position();
     return_value = ParseExpression(true, CHECK_OK);
+
+    if (IsSubclassConstructor(function_state_->kind())) {
+      // For subclass constructors we need to return this in case of undefined
+      // and throw an exception in case of a non object.
+      //
+      //   return expr;
+      //
+      // Is rewritten as:
+      //
+      //   return (temp = expr) === undefined ? this :
+      //       %_IsSpecObject(temp) ? temp : throw new TypeError(...);
+      Variable* temp = scope_->DeclarationScope()->NewTemporary(
+          ast_value_factory()->empty_string());
+      Assignment* assign = factory()->NewAssignment(
+          Token::ASSIGN, factory()->NewVariableProxy(temp), return_value, pos);
+
+      Expression* throw_expression =
+          NewThrowTypeError("derived_constructor_return",
+                            ast_value_factory()->empty_string(), pos);
+
+      // %_IsSpecObject(temp)
+      ZoneList<Expression*>* is_spec_object_args =
+          new (zone()) ZoneList<Expression*>(1, zone());
+      is_spec_object_args->Add(factory()->NewVariableProxy(temp), zone());
+      Expression* is_spec_object_call = factory()->NewCallRuntime(
+          ast_value_factory()->is_spec_object_string(),
+          Runtime::FunctionForId(Runtime::kInlineIsSpecObject),
+          is_spec_object_args, pos);
+
+      // %_IsSpecObject(temp) ? temp : throw_expression
+      Expression* is_object_conditional = factory()->NewConditional(
+          is_spec_object_call, factory()->NewVariableProxy(temp),
+          throw_expression, pos);
+
+      // temp === undefined
+      Expression* is_undefined = factory()->NewCompareOperation(
+          Token::EQ_STRICT, assign,
+          factory()->NewUndefinedLiteral(RelocInfo::kNoPosition), pos);
+
+      // is_undefined ? this : is_object_conditional
+      return_value = factory()->NewConditional(
+          is_undefined, ThisExpression(scope_, factory(), pos),
+          is_object_conditional, pos);
+    }
   }
   ExpectSemicolon(CHECK_OK);
 
diff --git a/test/mjsunit/harmony/classes-derived-return-type.js b/test/mjsunit/harmony/classes-derived-return-type.js
new file mode 100644 (file)
index 0000000..8283bcb
--- /dev/null
@@ -0,0 +1,90 @@
+// Copyright 2015 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+// Flags: --harmony-sloppy
+
+
+class Base {}
+
+class DerivedWithReturn extends Base {
+  constructor(x) {
+    super();
+    return x;
+  }
+}
+
+assertThrows(function() {
+  new DerivedWithReturn(null);
+}, TypeError);
+assertThrows(function() {
+  new DerivedWithReturn(42);
+}, TypeError);
+assertThrows(function() {
+  new DerivedWithReturn(true);
+}, TypeError);
+assertThrows(function() {
+  new DerivedWithReturn('hi');
+}, TypeError);
+assertThrows(function() {
+  new DerivedWithReturn(Symbol());
+}, TypeError);
+
+
+assertInstanceof(new DerivedWithReturn(undefined), DerivedWithReturn);
+function f() {}
+assertInstanceof(new DerivedWithReturn(new f()), f);
+assertInstanceof(new DerivedWithReturn(/re/), RegExp);
+
+
+class DerivedWithReturnNoSuper extends Base {
+  constructor(x) {
+    return x;
+  }
+}
+
+
+assertThrows(function() {
+  new DerivedWithReturnNoSuper(null);
+}, TypeError);
+assertThrows(function() {
+  new DerivedWithReturnNoSuper(42);
+}, TypeError);
+assertThrows(function() {
+  new DerivedWithReturnNoSuper(true);
+}, TypeError);
+assertThrows(function() {
+  new DerivedWithReturnNoSuper('hi');
+}, TypeError);
+assertThrows(function() {
+  new DerivedWithReturnNoSuper(Symbol());
+}, TypeError);
+assertThrows(function() {
+  new DerivedWithReturnNoSuper(undefined);
+}, ReferenceError);
+
+
+function f2() {}
+assertInstanceof(new DerivedWithReturnNoSuper(new f2()), f2);
+assertInstanceof(new DerivedWithReturnNoSuper(/re/), RegExp);
+
+
+class DerivedReturn extends Base {
+  constructor() {
+    super();
+    return;
+  }
+}
+
+assertInstanceof(new DerivedReturn(), DerivedReturn);
+
+
+
+class DerivedReturnThis extends Base {
+  constructor() {
+    super();
+    return this;
+  }
+}
+
+assertInstanceof(new DerivedReturnThis(), DerivedReturnThis);