Merge pull request #2482 from svaarala/function-caller-argument-props

Update Function .arguments and .caller behavior

releases/releases.yaml

@@ -1395,3 +1395,4 @@ duktape_releases:
     - "Improve DUK_USE_OS_STRING for macOS, iOS, watchOS, and tvOS (GH-2288)"
     - "Fix JSON.stringify() handling of Array 'replacer' duplicates (e.g. JSON.stringify({foo: 123}, [\"foo\", \"foo\"])); previously incorrectly serialized multiple times, now only once (GH-2379)"
     - "Add support for DJGPP (MSDOS) platform (GH-2472, GH-2473)"
+    - "Remove .caller and .arguments own properties from both strict and non-strict function objects, and add .caller and .arguments throwers to Function.prototype to match ES2015+ (GH-2482)"

src-input/builtins.yaml

@@ -877,7 +877,33 @@ objects:
           varargs: true
         present_if: DUK_USE_FUNCTION_BUILTIN
 
-      # ES2015
+        # ES2015+: AddRestrictedFunctionProperties for Function.prototype.
+        # Getters/setters must be the same %ThrowTypeError% instance.
+        # For now handled specially in built-ins init.
+      - key: "caller"
+        value:
+          type: accessor
+          getter: duk_bi_type_error_thrower
+          setter: duk_bi_type_error_thrower
+          getter_nargs: 0
+          setter_nargs: 1
+          getter_magic: 0
+          setter_magic: 0
+        attributes: "c"
+        es6: true
+        present_if: DUK_USE_FUNCTION_BUILTIN
+      - key: "arguments"
+        value:
+          type: accessor
+          getter: duk_bi_type_error_thrower
+          setter: duk_bi_type_error_thrower
+          getter_nargs: 0
+          setter_nargs: 1
+          getter_magic: 0
+          setter_magic: 0
+        attributes: "c"
+        es6: true
+        present_if: DUK_USE_FUNCTION_BUILTIN
       - key:  # @@hasInstance
           type: symbol
           variant: wellknown
@@ -2905,7 +2931,7 @@ objects:
       # Custom name, matches V8; ES2016 describes %ThrowTypeError% as being
       # anonymous.
       - key: "name"
-        value: "ThrowTypeError"
+        value: ""
         attributes: "c"
         duktape: true
 

src-input/duk_bi_function.c

@@ -350,10 +350,11 @@ DUK_INTERNAL duk_ret_t duk_bi_function_prototype_bind(duk_hthread *thr) {
 	DUK_TVAL_SET_U32(tv_tmp, (duk_uint32_t) bound_len); /* in-place update, fastint */
 	duk_xdef_prop_stridx_short(thr, -2, DUK_STRIDX_LENGTH, DUK_PROPDESC_FLAGS_C); /* attrs in E6 Section 9.2.4 */
 
-	/* XXX: could these be virtual? */
+#if 0 /* Now inherited from Function.prototype, for both strict and non-strict functions. */
 	/* Caller and arguments must use the same thrower, [[ThrowTypeError]]. */
 	duk_xdef_prop_stridx_thrower(thr, -1, DUK_STRIDX_CALLER);
 	duk_xdef_prop_stridx_thrower(thr, -1, DUK_STRIDX_LC_ARGUMENTS);
+#endif
 
 	/* Function name and fileName (non-standard). */
 	duk_push_literal(thr, "bound "); /* ES2015 19.2.3.2. */

src-input/duk_hthread_builtins.c

@@ -510,14 +510,28 @@ DUK_INTERNAL void duk_hthread_create_builtin_objects(duk_hthread *thr) {
 
 				c_func_getter = duk_bi_native_functions[natidx_getter];
 				if (c_func_getter != NULL) {
-					duk_push_c_function_builtin_noconstruct(thr, c_func_getter, 0); /* always 0 args */
+					/* Deal with Function.prototype .caller and .arguments, ensure
-					duk_set_magic(thr, -1, (duk_int_t) accessor_magic);
+					 * all share the same thrower function instance.
+					 */
+					if (c_func_getter == duk_bi_type_error_thrower) {
+						duk_dup(thr, DUK_BIDX_TYPE_ERROR_THROWER);
+					} else {
+						duk_push_c_function_builtin_noconstruct(thr, c_func_getter, 0); /* always 0 args */
+						duk_set_magic(thr, -1, (duk_int_t) accessor_magic);
+					}
 					defprop_flags |= DUK_DEFPROP_HAVE_GETTER;
 				}
 				c_func_setter = duk_bi_native_functions[natidx_setter];
 				if (c_func_setter != NULL) {
-					duk_push_c_function_builtin_noconstruct(thr, c_func_setter, 1); /* always 1 arg */
+					/* Deal with Function.prototype .caller and .arguments, ensure
-					duk_set_magic(thr, -1, (duk_int_t) accessor_magic);
+					 * all share the same thrower function instance.
+					 */
+					if (c_func_setter == duk_bi_type_error_thrower) {
+						duk_dup(thr, DUK_BIDX_TYPE_ERROR_THROWER);
+					} else {
+						duk_push_c_function_builtin_noconstruct(thr, c_func_setter, 1); /* always 1 arg */
+						duk_set_magic(thr, -1, (duk_int_t) accessor_magic);
+					}
 					defprop_flags |= DUK_DEFPROP_HAVE_SETTER;
 				}
 

src-input/duk_js_var.c

@@ -410,26 +410,36 @@ void duk_js_push_closure(duk_hthread *thr,
 
 	/*
 	 *  "arguments" and "caller" must be mapped to throwers for strict
-	 *  mode and bound functions (E5 Section 15.3.5).
+	 *  mode and bound functions in ES5.1 (E5 Section 15.3.5).  This is
+	 *  no longer required in ES2015+ for any functions; instead .arguments
+	 *  and .caller are throwing accessors in Function.prototype for all
+	 *  function types.
 	 *
-	 *  XXX: This is expensive to have for every strict function instance.
+	 *  V8 provides .caller and .arguments own properties (null) for
-	 *  Try to implement as virtual properties or on-demand created properties.
+	 *  non-strict functions.  We omit these and rely on the throwing
+	 *  accessors to reduce function size.  Both behaviors are compliant.
 	 */
 
 	/* [ ... closure template ] */
 
-	if (DUK_HOBJECT_HAS_STRICT(&fun_clos->obj)) {
-		duk_xdef_prop_stridx_thrower(thr, -2, DUK_STRIDX_CALLER);
-		duk_xdef_prop_stridx_thrower(thr, -2, DUK_STRIDX_LC_ARGUMENTS);
-	} else {
 #if defined(DUK_USE_NONSTD_FUNC_CALLER_PROPERTY)
+	if (!DUK_HOBJECT_HAS_STRICT(&fun_clos->obj)) {
 		DUK_DDD(DUK_DDDPRINT("function is non-strict and non-standard 'caller' property in use, add initial 'null' value"));
 		duk_push_null(thr);
 		duk_xdef_prop_stridx_short(thr, -3, DUK_STRIDX_CALLER, DUK_PROPDESC_FLAGS_NONE);
-#else
+	}
-		DUK_DDD(DUK_DDDPRINT("function is non-strict and non-standard 'caller' property not used"));
 #endif
+
+#if 0 /* V8 provides immutable null value .caller and .arguments for non-strict functions. */
+	if (DUK_HOBJECT_HAS_STRICT(&fun_clos->obj)) {
+		;
+	} else {
+		duk_push_null(thr);
+		duk_xdef_prop_stridx_short(thr, -3, DUK_STRIDX_CALLER, DUK_PROPDESC_FLAGS_NONE);
+		duk_push_null(thr);
+		duk_xdef_prop_stridx_short(thr, -3, DUK_STRIDX_LC_ARGUMENTS, DUK_PROPDESC_FLAGS_NONE);
 	}
+#endif
 
 	/*
 	 *  "name" used to be non-standard but is now defined by ES2015.
@@ -469,7 +479,7 @@ void duk_js_push_closure(duk_hthread *thr,
 	duk_compact(thr, -2);
 
 	/*
-	 *  Some assertions (E5 Section 13.2).
+	 *  Some assertions.
 	 */
 
 	DUK_ASSERT(DUK_HOBJECT_GET_CLASS_NUMBER(&fun_clos->obj) == DUK_HOBJECT_CLASS_FUNCTION);
@@ -478,8 +488,8 @@ void duk_js_push_closure(duk_hthread *thr,
 	DUK_ASSERT(duk_has_prop_stridx(thr, -2, DUK_STRIDX_LENGTH) != 0);
 	DUK_ASSERT(add_auto_proto == 0 || duk_has_prop_stridx(thr, -2, DUK_STRIDX_PROTOTYPE) != 0);
 	/* May be missing .name */
-	DUK_ASSERT(!DUK_HOBJECT_HAS_STRICT(&fun_clos->obj) || duk_has_prop_stridx(thr, -2, DUK_STRIDX_CALLER) != 0);
+	DUK_ASSERT(duk_has_prop_stridx(thr, -2, DUK_STRIDX_CALLER) != 0);
-	DUK_ASSERT(!DUK_HOBJECT_HAS_STRICT(&fun_clos->obj) || duk_has_prop_stridx(thr, -2, DUK_STRIDX_LC_ARGUMENTS) != 0);
+	DUK_ASSERT(duk_has_prop_stridx(thr, -2, DUK_STRIDX_LC_ARGUMENTS) != 0);
 
 	/*
 	 *  Finish

src-input/strings.yaml

@@ -737,10 +737,6 @@ strings:
     duktape: true
     class_name: true
 
-  # non-standard built-in object names
-  - str: "ThrowTypeError"  # implementation specific, matches V8
-    duktape: true
-
   # non-standard error object (or Error.prototype) properties
   - str: "stack"
     duktape: true

tests/api/test-dev-lightfunc.c

@@ -792,6 +792,8 @@ enum nonenumerable
 key: length
 key: name
 key: constructor
+key: caller
+key: arguments
 key: toString
 key: apply
 key: call

tests/ecmascript/test-bi-function-proto-caller-arguments-getset-1.js

@@ -0,0 +1,207 @@
+/*===
+- Function.prototype.arguments get
+TypeError
+TypeError
+TypeError
+TypeError
+TypeError
+TypeError
+- Function.prototype.arguments set
+TypeError
+TypeError
+TypeError
+TypeError
+TypeError
+TypeError
+- Function.prototype.caller get
+TypeError
+TypeError
+TypeError
+TypeError
+TypeError
+TypeError
+- Function.prototype.caller set
+TypeError
+TypeError
+TypeError
+TypeError
+TypeError
+TypeError
+===*/
+
+function strictFunc() {
+    'use strict';
+}
+
+function nonStrictFunc() {
+}
+
+function test() {
+    var protoArguments = Object.getOwnPropertyDescriptor(Function.prototype, 'arguments');
+    var protoCaller = Object.getOwnPropertyDescriptor(Function.prototype, 'caller');
+
+    print('- Function.prototype.arguments get');
+    try {
+        print(protoArguments.get.call(null));
+    } catch (e) {
+        print(e.name);
+    }
+
+    try {
+        print(protoArguments.get.call(123));
+    } catch (e) {
+        print(e.name);
+    }
+
+    try {
+        print(protoArguments.get.call(Math.cos));
+    } catch (e) {
+        print(e.name);
+    }
+
+    try {
+        print(protoArguments.get.call(strictFunc));
+    } catch (e) {
+        print(e.name);
+    }
+
+    try {
+        print(protoArguments.get.call(nonStrictFunc));
+    } catch (e) {
+        print(e.name);
+    }
+
+    try {
+        // Difference to V8, no own .arguments property with null value
+        // to minimize function instance size.  Trigger thrower instead.
+        // Both behaviors are compliant.
+        print(nonStrictFunc.arguments);
+    } catch (e) {
+        print(e.name);
+    }
+
+    print('- Function.prototype.arguments set');
+    try {
+        print(protoArguments.set.call(null, 123));
+    } catch (e) {
+        print(e.name);
+    }
+
+    try {
+        print(protoArguments.set.call(123, 123));
+    } catch (e) {
+        print(e.name);
+    }
+
+    try {
+        print(protoArguments.set.call(Math.cos, 123));
+    } catch (e) {
+        print(e.name);
+    }
+
+    try {
+        print(protoArguments.set.call(strictFunc, 123));
+    } catch (e) {
+        print(e.name);
+    }
+
+    try {
+        print(protoArguments.set.call(nonStrictFunc, 123));
+    } catch (e) {
+        print(e.name);
+    }
+
+    try {
+        // Difference to V8 here, V8 provides a non-configurable and non-writable
+        // own property (with null value).  To minimize function instance size we
+        // avoid the own properties and trigger the thrower instead, just like for
+        // strict functions.  Both behaviors are compliant.
+        nonStrictFunc.arguments = 123;
+        print(nonStrictFunc.arguments);
+    } catch (e) {
+        print(e.name);
+    }
+
+    print('- Function.prototype.caller get');
+    try {
+        print(protoCaller.get.call(null));
+    } catch (e) {
+        print(e.name);
+    }
+
+    try {
+        print(protoCaller.get.call(123));
+    } catch (e) {
+        print(e.name);
+    }
+
+    try {
+        print(protoCaller.get.call(Math.cos));
+    } catch (e) {
+        print(e.name);
+    }
+
+    try {
+        print(protoCaller.get.call(strictFunc));
+    } catch (e) {
+        print(e.name);
+    }
+
+    try {
+        // Difference to V8 here, see comments for .arguments above.
+        print(protoCaller.get.call(nonStrictFunc));
+    } catch (e) {
+        print(e.name);
+    }
+
+    try {
+        print(nonStrictFunc.caller);
+    } catch (e) {
+        print(e.name);
+    }
+
+    print('- Function.prototype.caller set');
+    try {
+        print(protoCaller.set.call(null, 123));
+    } catch (e) {
+        print(e.name);
+    }
+
+    try {
+        print(protoCaller.set.call(123, 123));
+    } catch (e) {
+        print(e.name);
+    }
+
+    try {
+        print(protoCaller.set.call(Math.cos, 123));
+    } catch (e) {
+        print(e.name);
+    }
+
+    try {
+        print(protoCaller.set.call(strictFunc, 123));
+    } catch (e) {
+        print(e.name);
+    }
+
+    try {
+        // Difference to V8 here, see comments for .arguments above.
+        print(protoCaller.set.call(nonStrictFunc, 123));
+    } catch (e) {
+        print(e.name);
+    }
+
+    try {
+        nonStrictFunc.caller = 123;
+        print(nonStrictFunc.caller);
+    } catch (e) {
+        print(e.name);
+    }
+}
+
+try {
+    test();
+} catch (e) {
+    print(e.stack || e);
+}

tests/ecmascript/test-bi-function-proto-restricted-props.js

@@ -0,0 +1,63 @@
+/*
+ *  ES2015+: CreateRealm() initializes Function.prototype with
+ *  AddRestrictedFunctionProperties(), "caller" and "arguments".
+ *  These properties are no longer in function instances.
+ */
+
+/*===
+true
+true
+true
+true
+object
+function
+function
+object
+function
+function
+true
+true
+true
+true
+true
+true
+true
+true
+===*/
+
+try {
+    var f1 = function () {};
+    var f2 = function () { 'use strict'; }
+    var pd1, pd2;
+
+    // In ES2015+ non-strict objects don't have these properties but
+    // e.g. V8 provides them as nulls, probably for legacy reasons.
+    pd1 = Object.getOwnPropertyDescriptor(f1, 'caller');
+    print(pd1 === void 0);
+    pd1 = Object.getOwnPropertyDescriptor(f1, 'arguments');
+    print(pd1 === void 0);
+
+    pd1 = Object.getOwnPropertyDescriptor(f2, 'caller');
+    print(pd1 === void 0);
+    pd1 = Object.getOwnPropertyDescriptor(f2, 'arguments');
+    print(pd1 === void 0);
+
+    pd1 = Object.getOwnPropertyDescriptor(Function.prototype, 'caller');
+    print(typeof pd1);
+    print(typeof pd1.get);
+    print(typeof pd1.set);
+    pd2 = Object.getOwnPropertyDescriptor(Function.prototype, 'arguments');
+    print(typeof pd2);
+    print(typeof pd2.get);
+    print(typeof pd2.set);
+    print(pd1.get !== void 0);
+    print(pd1.set !== void 0);
+    print(pd1.get === pd1.set);
+    print(pd2.get !== void 0);
+    print(pd2.set !== void 0);
+    print(pd2.get === pd1.set);
+    print(pd1.get === pd2.get);
+    print(pd1.set === pd2.set);
+} catch (e) {
+    print(e.stack || e);
+}

tests/ecmascript/test-dev-bound-func-caller.js

@@ -1,18 +1,5 @@
-/*
- *  The 'caller' property for functions is rather interesting: it is a
- *  thrower for strict functions and all bound functions (strict AND
- *  non-strict).  It is undefined for non-strict non-bound functions.
- *
- *  V8 behavior mostly agrees but 'caller' will be set to 'null' for
- *  non-strict functions (instead of being undefined).  Rhino behavior
- *  does not agree; bound non-strict functions don't have a 'caller'
- *  property in Rhino.
- *
- *  E5.1 Sections 15.3.5, 15.3.4.5.
- */
-
 /*===
-undefined
+TypeError
 TypeError
 TypeError
 TypeError

tests/ecmascript/test-dev-builtin-name-props.js

@@ -103,20 +103,16 @@ print(URIError.prototype.name);
 print(JSON.name);
 
 /*===
-ThrowTypeError
+
 ===*/
 
 /* Name of the "type error thrower" of E5 Section 13.2.3.  This is not
- * defined in the specification; V8 uses "ThrowTypeError" which we also
+ * defined in the specification; older V8 uses "ThrowTypeError", newer
- * use.
+ * uses empty string which we also use.
  */
 
-function func() {
-    'use strict';
-}
-
 try {
-    var desc = Object.getOwnPropertyDescriptor(func, 'caller');
+    var desc = Object.getOwnPropertyDescriptor(Function.prototype, 'caller');
     print(desc.get.name);
 } catch (e) {
     print(e.name, e);

tests/ecmascript/test-dev-func-strict-throwers.js

@@ -5,28 +5,12 @@
 ---*/
 
 /*===
-undefined
-undefined
 TypeError
 TypeError
-function function
+TypeError
-function function
+TypeError
-true
-true
-true
-true
 ===*/
 
-/* The 'caller' and 'arguments' properties of a strict function instance
- * are set to the thrower function of E5 Section 13.2.3.  See E5 Section
- * 13.2.1, step 19.
- *
- * A non-strict function instance should not have a 'caller' nor an
- * 'arguments' property at all.
- *
- * This testcase breaks with DUK_USE_NONSTD_FUNC_CALLER_PROPERTY.
- */
-
 function f() { }
 
 function g() { 'use strict' }
@@ -56,17 +40,3 @@ try {
 } catch (e) {
     print(e.name);
 }
-
-// the thrower is required to be the same object
-try {
-    pd1 = Object.getOwnPropertyDescriptor(g, 'caller');
-    pd2 = Object.getOwnPropertyDescriptor(g, 'arguments');
-    print(typeof pd1.get, typeof pd1.set);
-    print(typeof pd2.get, typeof pd2.set);
-    print(pd1.get === pd1.set);
-    print(pd2.get === pd2.set);
-    print(pd1.get === pd2.get);
-    print(pd1.set === pd2.set);
-} catch (e) {
-    print(e.name);
-}

tests/ecmascript/test-dev-function-props.js

@@ -12,9 +12,9 @@
 
 /*===
 name: true, fileName: true, length: true, caller: false, arguments: false, callee: false, prototype: true
-name: true, fileName: true, length: true, caller: true, arguments: true, callee: false, prototype: true
+name: true, fileName: true, length: true, caller: false, arguments: false, callee: false, prototype: true
-name: true, fileName: true, length: true, caller: true, arguments: true, callee: false, prototype: false
+name: true, fileName: true, length: true, caller: false, arguments: false, callee: false, prototype: false
-name: true, fileName: true, length: true, caller: true, arguments: true, callee: false, prototype: false
+name: true, fileName: true, length: true, caller: false, arguments: false, callee: false, prototype: false
 ===*/
 
 function test() {

tests/ecmascript/test-dev-strict-func-as-caller-prop-value.js

@@ -1,51 +1,18 @@
 /*
- *  If the 'caller' property of a non-bound function (strict or not) or an
+ *  Strict function as an arguments 'caller' property.
- *  Arguments object of a non-strict function with at least one formal parameter
- *  has a -value- which is a strict function, throw a TypeError in [[Get]].
- *  See E5.1 Sections 15.3.5.4 and 10.6.
- *
- *  The situation is a bit ambiguous for bound functions: Section 15.3.4.5
- *  (which describes bind()) states that [[Get]] is as for ordinary functions,
- *  specified in Section 15.3.5.4.  However, Section 15.3.5.4 states:
- *
- *      NOTE: Function objects created using Function.prototype.bind
- *      use the default [[Get]] internal method.
- *
- *  This seems to imply that bound functions do not have this special
- *  behavior.
- *
- *  These special [[Get]] behaviors are quite complicated to understand
- *  because both the base value and the property value affect the final
- *  behavior.  Also, the nature of the Arguments object depends on whether
- *  the function has formal arguments or not, which is quite nonintuitive.
- *
- *  Rhino and V8 also don't seem to match the behavior mandated by the
- *  specification.  For V8 there seem to be the following deviations:
- *
- *    - V8 doesn't apply the special [[Get]] semantics to non-strict
- *      functions while E5.1 Section 15.3.5.4 doesn't limit the behavior
- *      to strict functions only (the behavior is limited to 'caller'
- *      having a strict function as a -value-, but the base can be a
- *      non-strict function too).
- *
- *    - V8 promotes a string written to 'caller' into an object which
- *      seems non-compliant.
- *
- *    - V8 doesn't apply the special [[Get]] semantics for non-strict
- *      Arguments object even when the callee has formal arguments
- *      (which is supposed to enable the special behaviors for the
- *      [[ParameterMap]] handling and "caller").
- *
- *  ES2017 removes the arguments.caller thrower for strict argument objects,
- *  test case has been updated to reflect this.
  */
 
 /*===
 functions
-0 0 string
+0 0 fail to set "caller": TypeError
-0 1 function
+0 0 TypeError
+0 1 fail to set "caller": TypeError
+0 1 TypeError
+0 2 fail to set "caller": TypeError
 0 2 TypeError
-0 3 function
+0 3 fail to set "caller": TypeError
+0 3 TypeError
+0 4 fail to set "caller": TypeError
 0 4 TypeError
 1 0 fail to set "caller": TypeError
 1 0 TypeError
@@ -144,9 +111,8 @@ function testFunctions() {
         for (j = 0; j < callerPropertyValues.length; j++) {
             base = baseFunctionCreators[i]();
 
-            // Note: this step fails (as expected) for bound functions and
+            // Now fails for all functions because of an inherited
-            // strict functions because they have a non-writable 'caller'
+            // Function.prototype.caller which throws unconditionally.
-            // property.
             try {
                 base.caller = callerPropertyValues[j];
             } catch (e) {