Reviewers: arv, Diego Pino, Dmitry Lomov (chromium),
Message:
PTAL --- I realized today ArrayFrom was doing "the wrong thing" by not
supporting bound functions. In particular, it should only care about the
presence of a [[Construct]] internal slot.
Unfortunately, construct_stub doesn't offer a great way to check this, but
should_have_prototype seems to be a rough way of doing this.
It would be nice to inline this, and maybe make heuristics for checking if a
function is a constructor better.
Description:
Implement IsConstructor() abstract operation
LOG=N
[email protected], [email protected], [email protected]
BUG=
Please review this at https://codereview.chromium.org/851163007/
Base URL: https://chromium.googlesource.com/v8/v8.git@master
Affected files (+55, -7 lines):
M src/harmony-array.js
M src/runtime/runtime.h
M src/runtime/runtime-function.cc
M test/mjsunit/harmony/array-from.js
M test/mjsunit/harmony/array-of.js
Index: src/harmony-array.js
diff --git a/src/harmony-array.js b/src/harmony-array.js
index
5d1262a06d75bfd2444ade2f1fca0fa178079a90..762f746e56d923f1330ac177fa41403a331c18c8
100644
--- a/src/harmony-array.js
+++ b/src/harmony-array.js
@@ -149,7 +149,7 @@ function ArrayFrom(arrayLike, mapfn, receiver) {
var nextValue;
if (!IS_UNDEFINED(iterable)) {
- result = IS_SPEC_FUNCTION(this) && this.prototype ? new this() : [];
+ result = %IsConstructor(this) ? new this() : [];
k = 0;
for (nextValue of items) {
@@ -162,8 +162,7 @@ function ArrayFrom(arrayLike, mapfn, receiver) {
return result;
} else {
var len = ToLength(items.length);
- result = IS_SPEC_FUNCTION(this) && this.prototype ? new this(len) :
- new $Array(len);
+ result = %IsConstructor(this) ? new this(len) : new $Array(len);
for (k = 0; k < len; ++k) {
nextValue = items[k];
@@ -182,7 +181,7 @@ function ArrayOf() {
var length = %_ArgumentsLength();
var constructor = this;
// TODO: Implement IsConstructor (ES6 section 7.2.5)
- var array = IS_SPEC_FUNCTION(constructor) ? new constructor(length) : [];
+ var array = %IsConstructor(constructor) ? new constructor(length) : [];
for (var i = 0; i < length; i++) {
%AddElement(array, i, %_Arguments(i), NONE);
}
Index: src/runtime/runtime-function.cc
diff --git a/src/runtime/runtime-function.cc
b/src/runtime/runtime-function.cc
index
608cc9548423a34543b739c6a6d85e0571200579..4233c585f65f8a119be7975a752a384d22d217b6
100644
--- a/src/runtime/runtime-function.cc
+++ b/src/runtime/runtime-function.cc
@@ -326,6 +326,33 @@ RUNTIME_FUNCTION(Runtime_SetNativeFlag) {
}
+RUNTIME_FUNCTION(Runtime_IsConstructor) {
+ SealHandleScope shs(isolate);
+ RUNTIME_ASSERT(args.length() == 1);
+
+ CONVERT_ARG_CHECKED(Object, object, 0);
+
+ if (object->IsJSFunction()) {
+ JSFunction* func = JSFunction::cast(object);
+ bool should_have_prototype = func->should_have_prototype();
+ if (func->shared()->bound()) {
+ Handle<FixedArray> bound_args =
+ Handle<FixedArray>(FixedArray::cast(func->function_bindings()));
+ Handle<Object> bound_function(
+
JSReceiver::cast(bound_args->get(JSFunction::kBoundFunctionIndex)),
+ isolate);
+ if (bound_function->IsJSFunction()) {
+ JSFunction* bound = JSFunction::cast(*bound_function);
+ DCHECK(!bound->shared()->bound());
+ should_have_prototype = bound->should_have_prototype();
+ }
+ }
+ return isolate->heap()->ToBoolean(should_have_prototype);
+ }
+ return isolate->heap()->false_value();
+}
+
+
RUNTIME_FUNCTION(Runtime_SetInlineBuiltinFlag) {
SealHandleScope shs(isolate);
RUNTIME_ASSERT(args.length() == 1);
Index: src/runtime/runtime.h
diff --git a/src/runtime/runtime.h b/src/runtime/runtime.h
index
a39e320f1138ddfc378cf9a75e73af453c5bbdef..9c7c379e45b496c37b1caa081e6e93eaf62e6867
100644
--- a/src/runtime/runtime.h
+++ b/src/runtime/runtime.h
@@ -73,6 +73,7 @@ namespace internal {
F(CompileForOnStackReplacement, 1, 1) \
F(SetAllocationTimeout, -1 /* 2 || 3 */, 1) \
F(SetNativeFlag, 1, 1) \
+ F(IsConstructor, 1, 1) \
F(SetInlineBuiltinFlag, 1, 1) \
F(StoreArrayLiteralElement, 5, 1) \
F(DebugPrepareStepInIfStepping, 1, 1) \
Index: test/mjsunit/harmony/array-from.js
diff --git a/test/mjsunit/harmony/array-from.js
b/test/mjsunit/harmony/array-from.js
index
e7c9fef7d5d8a3826451e1c9d6fb133af26060cc..e8dde163fc227bb21bdeaab6d14ff944cec65b96
100644
--- a/test/mjsunit/harmony/array-from.js
+++ b/test/mjsunit/harmony/array-from.js
@@ -118,6 +118,7 @@ testArrayFrom({}, Array);
testArrayFrom(Object, Object);
testArrayFrom(Other, Other);
testArrayFrom(Math.cos, Array);
-testArrayFrom(boundFn, Array);
+testArrayFrom(Math.cos.bind(Math), Array);
+testArrayFrom(boundFn, boundFn);
})();
Index: test/mjsunit/harmony/array-of.js
diff --git a/test/mjsunit/harmony/array-of.js
b/test/mjsunit/harmony/array-of.js
index
c0a8ed183e597fd538b2b8a52ba983f62a2f0cfd..adf7cb547cdae9eebd9fdec14f01c8d562526e65
100644
--- a/test/mjsunit/harmony/array-of.js
+++ b/test/mjsunit/harmony/array-of.js
@@ -159,6 +159,26 @@ assertEquals(Array.of.length, 0);
assertThrows(function() { new Array.of() }, TypeError); // not a
constructor
// When the this-value passed in is not a constructor, the result is an
array.
-[undefined, null, false, "cow"].forEach(function(val) {
- assertEquals(Array.isArray(Array.of(val)), true);
+[
+ undefined,
+ null,
+ false,
+ "cow",
+ NaN,
+ 67,
+ Infinity,
+ -Infinity,
+ Math.cos, // builtin functions with no [[Construct]] slot
+ Math.cos.bind(Math) // bound builtin functions with no [[Construct]] slot
+].forEach(function(val) {
+ assertEquals(Array.isArray(Array.of.call(val, val)), true);
});
+
+
+(function testBoundConstructor() {
+ var boundFn = (function() {}).bind(null);
+ var instance = Array.of.call(boundFn, 1, 2, 3);
+ assertEquals(instance.length, 3);
+ assertEquals(instance instanceof boundFn, true);
+ assertEquals(Array.isArray(instance), false);
+})();
--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/d/optout.
