Fix Reflect.construct wrt proxy, generator, and non-subclass new.target

This makes sure that proxy + Function/Array works
Makes sure that new.target can be a generator
Makes sure that if new.target is not a subclass, but does not have a prototype, that we'll get that same prototype back the next time we look at new.target.prototype.

BUG=v8:1543, v8:3330, v8:3931
LOG=n

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

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

src/objects.cc

@@ -12206,6 +12206,10 @@ void JSFunction::EnsureHasInitialMap(Handle<JSFunction> function) {
   if (function->has_initial_map()) return;
   Isolate* isolate = function->GetIsolate();
 
+  // The constructor should be compiled for the optimization hints to be
+  // available.
+  Compiler::Compile(function, CLEAR_EXCEPTION);
+
   // First create a new map with the size and number of in-object properties
   // suggested by the function.
   InstanceType instance_type;
@@ -12245,39 +12249,59 @@ void JSFunction::EnsureHasInitialMap(Handle<JSFunction> function) {
 }
 
 
-Handle<Map> JSFunction::EnsureDerivedHasInitialMap(
-    Handle<JSFunction> new_target, Handle<JSFunction> constructor) {
-  DCHECK(constructor->has_initial_map());
-  Isolate* isolate = constructor->GetIsolate();
+// static
+MaybeHandle<Map> JSFunction::GetDerivedMap(Isolate* isolate,
+                                           Handle<JSFunction> constructor,
+                                           Handle<JSReceiver> new_target) {
+  EnsureHasInitialMap(constructor);
+
   Handle<Map> constructor_initial_map(constructor->initial_map(), isolate);
   if (*new_target == *constructor) return constructor_initial_map;
-  if (new_target->has_initial_map()) {
-    // Check that |new_target|'s initial map still in sync with
-    // the |constructor|, otherwise we must create a new initial map for
-    // |new_target|.
-    if (new_target->initial_map()->GetConstructor() == *constructor) {
-      return handle(new_target->initial_map(), isolate);
+
+  if (new_target->IsJSProxy()) {
+    Handle<JSProxy> new_target_proxy = Handle<JSProxy>::cast(new_target);
+    Handle<Object> prototype;
+    Handle<String> prototype_string = isolate->factory()->prototype_string();
+    ASSIGN_RETURN_ON_EXCEPTION(
+        isolate, prototype,
+        JSReceiver::GetProperty(new_target_proxy, prototype_string), Map);
+    Handle<Map> map = Map::CopyInitialMap(constructor_initial_map);
+
+    if (!prototype->IsJSReceiver()) {
+      Handle<Context> context;
+      ASSIGN_RETURN_ON_EXCEPTION(
+          isolate, context, JSProxy::GetFunctionRealm(new_target_proxy), Map);
+      DCHECK(context->IsNativeContext());
+      // TODO(verwaest): Use the intrinsicDefaultProto instead.
+      prototype = handle(context->initial_object_prototype(), isolate);
     }
+
+    if (map->prototype() != *prototype) {
+      Map::SetPrototype(map, prototype, FAST_PROTOTYPE);
     }
 
-  // First create a new map with the size and number of in-object properties
-  // suggested by the function.
-  DCHECK(!new_target->shared()->is_generator());
-  DCHECK(!constructor->shared()->is_generator());
+    map->SetConstructor(*constructor);
+    return map;
+  }
 
-  // Fetch or allocate prototype.
-  // TODO(verwaest): In case of non-instance prototype, use the
-  // intrinsicDefaultProto instead.
-  Handle<Object> prototype;
-  if (new_target->has_instance_prototype()) {
-    prototype = handle(new_target->instance_prototype(), isolate);
-  } else {
-    prototype = isolate->factory()->NewFunctionPrototype(new_target);
+  Handle<JSFunction> new_target_function = Handle<JSFunction>::cast(new_target);
+
+  // Check that |new_target_function|'s initial map still in sync with
+  // the |constructor|, otherwise we must create a new initial map for
+  // |new_target_function|.
+  if (new_target_function->has_initial_map() &&
+      new_target_function->initial_map()->GetConstructor() == *constructor) {
+    return handle(new_target_function->initial_map(), isolate);
   }
 
-  // Finally link initial map and constructor function if the original
-  // constructor is actually a subclass constructor.
-  if (IsSubclassConstructor(new_target->shared()->kind())) {
+  // Create a new map with the size and number of in-object properties suggested
+  // by the function.
+
+  // Link initial map and constructor function if the original constructor is
+  // actually a subclass constructor.
+  if (IsSubclassConstructor(new_target_function->shared()->kind())) {
+    Handle<Object> prototype(new_target_function->instance_prototype(),
+                             isolate);
     InstanceType instance_type = constructor_initial_map->instance_type();
     DCHECK(CanSubclassHaveInobjectProperties(instance_type));
     int internal_fields =
@@ -12286,7 +12310,7 @@ Handle<Map> JSFunction::EnsureDerivedHasInitialMap(
                         constructor_initial_map->unused_property_fields();
     int instance_size;
     int in_object_properties;
-    new_target->CalculateInstanceSizeForDerivedClass(
+    new_target_function->CalculateInstanceSizeForDerivedClass(
         instance_type, internal_fields, &instance_size, &in_object_properties);
 
     int unused_property_fields = in_object_properties - pre_allocated;
@@ -12294,12 +12318,26 @@ Handle<Map> JSFunction::EnsureDerivedHasInitialMap(
         Map::CopyInitialMap(constructor_initial_map, instance_size,
                             in_object_properties, unused_property_fields);
 
-    JSFunction::SetInitialMap(new_target, map, prototype);
+    JSFunction::SetInitialMap(new_target_function, map, prototype);
     map->SetConstructor(*constructor);
-    new_target->StartInobjectSlackTracking();
+    new_target_function->StartInobjectSlackTracking();
     return map;
+  }
 
+  // Fetch the prototype.
+  Handle<Object> prototype;
+  if (new_target_function->map()->has_non_instance_prototype()) {
+    // TODO(verwaest): In case of non-instance prototype, use the
+    // intrinsicDefaultProto instead.
+    prototype = handle(new_target_function->context()
+                           ->native_context()
+                           ->initial_object_prototype());
   } else {
+    // Make sure the prototype is cached on new_target_function.
+    EnsureHasInitialMap(new_target_function);
+    prototype = handle(new_target_function->instance_prototype(), isolate);
+  }
+
   Handle<Map> map = Map::CopyInitialMap(constructor_initial_map);
   DCHECK(prototype->IsJSReceiver());
   if (map->prototype() != *prototype) {
@@ -12307,7 +12345,6 @@ Handle<Map> JSFunction::EnsureDerivedHasInitialMap(
   }
   map->SetConstructor(*constructor);
   return map;
-  }
 }
 
 

src/objects.h

@@ -7298,11 +7298,13 @@ class JSFunction: public JSObject {
                             Handle<Object> prototype);
   inline bool has_initial_map();
   static void EnsureHasInitialMap(Handle<JSFunction> function);
-  // Ensures that the |new_target| has correct initial map and
-  // returns it. If the |new_target| is not a subclass constructor
-  // its initial map is left unmodified.
-  static Handle<Map> EnsureDerivedHasInitialMap(Handle<JSFunction> new_target,
-                                                Handle<JSFunction> constructor);
+
+  // Creates a map that matches the constructor's initial map, but with
+  // [[prototype]] being new.target.prototype. Because new.target can be a
+  // JSProxy, this can call back into JavaScript.
+  static MUST_USE_RESULT MaybeHandle<Map> GetDerivedMap(
+      Isolate* isolate, Handle<JSFunction> constructor,
+      Handle<JSReceiver> new_target);
 
   // Get and set the prototype property on a JSFunction. If the
   // function has an initial map the prototype is set on the initial

src/runtime/runtime-array.cc

@@ -272,16 +272,10 @@ Object* ArrayConstructorCommon(Isolate* isolate, Handle<JSFunction> constructor,
     }
   }
 
-  // TODO(verwaest): new_target could be a proxy. Read new.target.prototype in
-  // that case.
-  Handle<JSFunction> original_function = Handle<JSFunction>::cast(new_target);
-
-  JSFunction::EnsureHasInitialMap(constructor);
-
-  // TODO(verwaest): original_function could have non-instance-prototype
-  // (non-JSReceiver), requiring fallback to the intrinsicDefaultProto.
-  Handle<Map> initial_map =
-      JSFunction::EnsureDerivedHasInitialMap(original_function, constructor);
+  Handle<Map> initial_map;
+  ASSIGN_RETURN_FAILURE_ON_EXCEPTION(
+      isolate, initial_map,
+      JSFunction::GetDerivedMap(isolate, constructor, new_target));
 
   ElementsKind to_kind = can_use_type_feedback ? site->GetElementsKind()
                                                : initial_map->elements_kind();

src/runtime/runtime-function.cc

@@ -55,25 +55,26 @@ RUNTIME_FUNCTION(Runtime_CompleteFunctionConstruction) {
   CONVERT_ARG_HANDLE_CHECKED(Object, unchecked_new_target, 2);
   func->shared()->set_name_should_print_as_anonymous(true);
 
+  if (unchecked_new_target->IsUndefined()) return *func;
+
+  Handle<JSReceiver> new_target =
+      Handle<JSReceiver>::cast(unchecked_new_target);
   // If new.target is equal to |constructor| then the function |func| created
   // is already correctly setup and nothing else should be done here.
   // But if new.target is not equal to |constructor| then we are have a
   // Function builtin subclassing case and therefore the function |func|
   // has wrong initial map. To fix that we create a new function object with
   // correct initial map.
-  if (unchecked_new_target->IsUndefined() ||
-      *constructor == *unchecked_new_target) {
-    return *func;
-  }
+  if (*constructor == *new_target) return *func;
 
   // Create a new JSFunction object with correct initial map.
   HandleScope handle_scope(isolate);
-  Handle<JSFunction> new_target =
-      Handle<JSFunction>::cast(unchecked_new_target);
 
   DCHECK(constructor->has_initial_map());
-  Handle<Map> initial_map =
-      JSFunction::EnsureDerivedHasInitialMap(new_target, constructor);
+  Handle<Map> initial_map;
+  ASSIGN_RETURN_FAILURE_ON_EXCEPTION(
+      isolate, initial_map,
+      JSFunction::GetDerivedMap(isolate, constructor, new_target));
 
   Handle<SharedFunctionInfo> shared_info(func->shared(), isolate);
   Handle<Context> context(func->context(), isolate);

src/runtime/runtime-object.cc

@@ -1007,55 +1007,17 @@ RUNTIME_FUNCTION(Runtime_AllocateHeapNumber) {
 }
 
 
-static MaybeHandle<Map> GetDerivedMap(Isolate* isolate,
-                                      Handle<JSFunction> constructor,
-                                      Handle<JSReceiver> new_target) {
-  JSFunction::EnsureHasInitialMap(constructor);
-  DCHECK_NE(JS_FUNCTION_TYPE, constructor->initial_map()->instance_type());
-
-  if (new_target->IsJSProxy()) {
-    Handle<JSProxy> new_target_proxy = Handle<JSProxy>::cast(new_target);
-    Handle<Object> prototype;
-    Handle<String> prototype_string = isolate->factory()->prototype_string();
-    ASSIGN_RETURN_ON_EXCEPTION(
-        isolate, prototype,
-        JSReceiver::GetProperty(new_target_proxy, prototype_string), Map);
-    Handle<Map> constructor_initial_map(constructor->initial_map());
-    Handle<Map> map = Map::CopyInitialMap(constructor_initial_map);
-
-    if (!prototype->IsJSReceiver()) {
-      Handle<Context> context;
-      ASSIGN_RETURN_ON_EXCEPTION(
-          isolate, context, JSProxy::GetFunctionRealm(new_target_proxy), Map);
-      DCHECK(context->IsNativeContext());
-      // TODO(verwaest): Use the intrinsicDefaultProto instead.
-      prototype = handle(context->initial_object_prototype(), isolate);
-    }
-
-    if (map->prototype() != *prototype) {
-      Map::SetPrototype(map, prototype, FAST_PROTOTYPE);
-    }
-
-    map->SetConstructor(*constructor);
-    return map;
-  }
-
-  return JSFunction::EnsureDerivedHasInitialMap(
-      Handle<JSFunction>::cast(new_target), constructor);
-}
-
-
 static Object* Runtime_NewObjectHelper(Isolate* isolate,
                                        Handle<JSFunction> constructor,
                                        Handle<JSReceiver> new_target,
                                        Handle<AllocationSite> site) {
-  // The constructor should be compiled for the optimization hints to be
-  // available.
-  Compiler::Compile(constructor, CLEAR_EXCEPTION);
+  DCHECK(!constructor->has_initial_map() ||
+         constructor->initial_map()->instance_type() != JS_FUNCTION_TYPE);
 
   Handle<Map> initial_map;
   ASSIGN_RETURN_FAILURE_ON_EXCEPTION(
-      isolate, initial_map, GetDerivedMap(isolate, constructor, new_target));
+      isolate, initial_map,
+      JSFunction::GetDerivedMap(isolate, constructor, new_target));
 
   Handle<JSObject> result =
       isolate->factory()->NewJSObjectFromMap(initial_map, NOT_TENURED, site);

test/mjsunit/es6/classes-proxy.js

@@ -39,3 +39,35 @@ function CreateConstructableProxy(handler) {
   assertTrue(Object.getPrototypeOf(o) === Object.prototype);
   assertEquals(100, Number.prototype.valueOf.call(o));
 })();
+
+(function() {
+  var prototype = { x: 1 };
+  var log = [];
+
+  var proxy = CreateConstructableProxy({
+    get(k) {
+      log.push("get trap");
+      return prototype;
+    }});
+
+  var o = Reflect.construct(Function, ["return 1000"], proxy);
+  assertEquals(["get trap"], log);
+  assertTrue(Object.getPrototypeOf(o) === prototype);
+  assertEquals(1000, o());
+})();
+
+(function() {
+  var prototype = { x: 1 };
+  var log = [];
+
+  var proxy = CreateConstructableProxy({
+    get(k) {
+      log.push("get trap");
+      return prototype;
+    }});
+
+  var o = Reflect.construct(Array, [1, 2, 3], proxy);
+  assertEquals(["get trap"], log);
+  assertTrue(Object.getPrototypeOf(o) === prototype);
+  assertEquals([1, 2, 3], o);
+})();

test/mjsunit/harmony/reflect-construct.js

@@ -275,3 +275,12 @@
   assertEquals(10, Reflect.construct(sumSloppy,
                                      { 0: 1, 1: 2, 2: 3, 3: 4, length: 4 }).a);
 })();
+
+(function() {
+  function* f() { yield 1; yield 2; }
+  function* g() { yield 3; yield 4; }
+  var o = Reflect.construct(f, [], g);
+  assertEquals([1, 2], [...o]);
+  assertTrue(o.__proto__ === g.prototype);
+  assertTrue(o.__proto__ !== f.prototype);
+})();