514581 - ES5 strict mode: function instances have no magic 'caller' or 'arguments' properties

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Jim Blandy :jimb]

From ES5 Annex C:

An implementation may not associate special meanings within strict mode functions to properties named caller or arguments of function instances. ECMAScript code may not create or modify properties with these names on function objects that correspond to strict mode functions (13.2).

Comment 1

[Mark S. Miller]

Actually, 13.2 step 19 shows that es5-strict functions must have poison pills for 'caller' and 'arguments'. Perhaps this is a bug in Annex C?

Comment 2

[Mark S. Miller]

https://mail.mozilla.org/pipermail/es5-discuss/2010-January/003480.html

Comment 3

[Brendan Eich [:brendan]]

See bug 514563, filed before this one. The Annex C language needs to be fixed for sure, but the es5strict bug is blocked by bug 514563, so if that bug is fixed in order to implement ES5 strict mode, the right thing should happen (knock on wood).

One confusion: bug 514563 comment 0 talks about Annex C as well, but says it informs readers that there *are* poison pills. Did Annex C's language regress?

/be

Comment 4

[Jeff Walden [:Waldo]]

A vaguely amusing pitfall in the naive implementation:

[jwalden@find-waldo-now src]$ dbg/js -j
js> function foo() { }
js> foo.caller
null
js> function foo() { "use strict"; }
js> foo.caller // !
null
js> function foo() { "use strict"; return 17; }
js> foo.caller
typein:6: TypeError: can't access arguments property on a strict-mode function

Comment 5

[Jeff Walden [:Waldo]]

Attachment: Patch and tests

It's tempting to fold in arguments.callee/caller, but better more and narrower than fewer and more intermingled.

Comment 6

[Jim Blandy :jimb]

Comment on attachment 462436 [details] [diff] [review]
Patch and tests

r=me with the comments addressed.

+MSG_DEF(JSMSG_THROW_TYPE_ERROR,       252, 0, JSEXN_TYPEERR, "[[ThrowTypeError]] function called")

I don't think error messages should cite this sort of ES5 jargon. How about "'caller', 'callee', and 'arguments' properties may not be accessed on strict mode functions and their arguments objects"?


diff --git a/js/src/jsfun.cpp b/js/src/jsfun.cpp
+/* NB: no sentinels at ends -- use JS_ARRAY_LENGTH to bound loops. */
+
+static LazyFunctionDataProp lazyFunctionDataProps[] = {

Nit: Could we make this const while we're here?

Nit: Is there a reason this switches from under_scores to StuDlYCaPs?

There are comments that refer to the old name.

     {ATOM_OFFSET(arity),     FUN_ARITY,      JSPROP_PERMANENT},
-    {ATOM_OFFSET(caller),    FUN_CALLER,     JSPROP_PERMANENT},
     {ATOM_OFFSET(name),      FUN_NAME,       JSPROP_PERMANENT},
 };
 
+/* Properties censored into [[ThrowTypeError]] in strict mode. */
+struct PoisonPillProp poisonPillProps[] = {

Shouldn't this be const?

 JSObject *
 js_InitFunctionClass(JSContext *cx, JSObject *obj)
 {
-    JSObject *proto;
-    JSFunction *fun;
-
-    proto = js_InitClass(cx, obj, NULL, &js_FunctionClass, Function, 1,
-                         NULL, function_methods, NULL, NULL);
+    JSObject *proto = js_InitClass(cx, obj, NULL, &js_FunctionClass, Function, 1,
+                                   NULL, function_methods, NULL, NULL);
     if (!proto)
         return NULL;
-    fun = js_NewFunction(cx, proto, NULL, 0, JSFUN_INTERPRETED, obj, NULL);
+
+    JSFunction *fun = js_NewFunction(cx, proto, NULL, 0, JSFUN_INTERPRETED, obj, NULL);
     if (!fun)
         return NULL;
     fun->u.i.script = JSScript::emptyScript();
+
+    /* ES5 13.2.3: Construct the unique [[ThrowTypeError]] function object. */
+    JSObject *throwTypeError =
+        NewFunction(cx, ThrowTypeError, cx->runtime->atomState.emptyAtom, 0, obj);

Is there a reason we construct this before we check JSCLASS_IS_GLOBAL?

+inline JSFunction *
+NewFunction(JSContext *cx, FastNative fn, JSAtom *name, uintN length, JSObject *parent)
+{
+    return js_NewFunction(cx, NULL, reinterpret_cast<Native>(fn), length,
+                          JSFUN_FAST_NATIVE, parent, name);
+}

* I think I'd rather just see js_NewFunction called directly than define an overloaded name, with one definition in jsobjinlines.h and one here.

+inline JSObject *
+GetThrowTypeError(JSObject *obj)

* Could this be a JSObject method, defined in jsobjinlines.h? That would be parallel to JSObject::getCompartment.

diff --git a/js/src/jsscript.cpp b/js/src/jsscript.cpp
--- a/js/src/jsscript.cpp
+++ b/js/src/jsscript.cpp
@@ -1000,13 +1000,25 @@ js_NewScriptFromCG(JSContext *cx, JSCode
+                    /*
+                     * We can't use a script singleton for empty strict mode
+                     * functions because they have poison-pill caller and
+                     * arguments properties:

* Wow, so none of the existing strict tests noticed this? I guess they're all thinking about the behavior of the function.

diff --git a/js/src/tests/ecma_5/Function/function-caller.js b/js/src/tests/ecma_5/Function/function-caller.js
new file mode 100644
--- /dev/null
+++ b/js/src/tests/ecma_5/Function/function-caller.js
+var barCaller = Object.getOwnPropertyDescriptor(bar, "caller");
+assertEq("get" in barCaller, true);
+assertEq("set" in barCaller, true);
+tte = barCaller.get;
+assertEq(tte, canonicalTTE);
+assertEq(tte, barCaller.set);

* Nit: Wouldn't it be clearer just to say the below?

assertEq(barCaller.get, canonicalTTE);
assertEq(barCaller.set, canonicalTTE);

Comment 7

[Jeff Walden [:Waldo]]

http://hg.mozilla.org/tracemonkey/rev/8acc48c670d5


(In reply to comment #6)
> I don't think error messages should cite this sort of ES5 jargon. How about
> "'caller', 'callee', and 'arguments' properties may not be accessed on strict
> mode functions and their arguments objects"?

This seems like it might become the ever-extending message, if ES6 adds any more uses of this (I expect it will).  :-\


> diff --git a/js/src/jsfun.cpp b/js/src/jsfun.cpp
> Nit: Could we make this const while we're here?
> There are comments that refer to the old name.

Fixed both, in all relevant places.


> Nit: Is there a reason this switches from under_scores to StuDlYCaPs?

under_score_names are not common these days outside of builtinclass_methodname.  uint8_clamped is the only one I can find in js/src proper (excluding xpconnect and other not-really-related things) for file-scoped structs; this seemed an odd man.


> Is there a reason we construct this before we check JSCLASS_IS_GLOBAL?

Absent-mindedness.  :-)


> * Could this be a JSObject method, defined in jsobjinlines.h? That would be
> parallel to JSObject::getCompartment.

Declared in jsobj.h and defined in jsfun.cpp (cf. JSObject::isCallable), sure.


> diff --git a/js/src/tests/ecma_5/Function/function-caller.js

> * Nit: Wouldn't it be clearer just to say the below?

Probably; changed.  (I don't give too much consideration to test simplicity except when I think the architecture might be reused.  :-) )

Comment 8

[Brendan Eich [:brendan]]

(In reply to comment #7)
> http://hg.mozilla.org/tracemonkey/rev/8acc48c670d5
> (In reply to comment #6)
> > I don't think error messages should cite this sort of ES5 jargon. How about
> > "'caller', 'callee', and 'arguments' properties may not be accessed on strict
> > mode functions and their arguments objects"?
> 
> This seems like it might become the ever-extending message, if ES6 adds any
> more uses of this (I expect it will).  :-\

Based on TC39 meetings and proposals, I can safely say that more such additions are Not Likely.

Jim's right, no [[EcmaSpeak]] in user-facing messages.

/be

Comment 9

[Robert Sayre]

http://hg.mozilla.org/mozilla-central/rev/8acc48c670d5