Closed
Bug 514581
Opened 15 years ago
Closed 14 years ago
ES5 strict mode: function instances have no magic 'caller' or 'arguments' properties
Categories
(Core :: JavaScript Engine, defect)
Core
JavaScript Engine
Tracking
()
RESOLVED
FIXED
People
(Reporter: jimb, Assigned: Waldo)
References
(Blocks 1 open bug)
Details
(Whiteboard: fixed-in-tracemonkey)
Attachments
(1 file)
(deleted),
patch
|
jimb
:
review+
|
Details | Diff | Splinter Review |
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•15 years ago
|
||
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•15 years ago
|
||
Comment 3•15 years ago
|
||
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
Assignee | ||
Updated•14 years ago
|
Assignee: general → jwalden+bmo
Status: NEW → ASSIGNED
OS: Linux → All
Hardware: x86 → All
Assignee | ||
Comment 4•14 years ago
|
||
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
Assignee | ||
Comment 5•14 years ago
|
||
It's tempting to fold in arguments.callee/caller, but better more and narrower than fewer and more intermingled.
Attachment #462436 -
Flags: review?(jim)
Reporter | ||
Comment 6•14 years ago
|
||
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);
Attachment #462436 -
Flags: review?(jim) → review+
Assignee | ||
Comment 7•14 years ago
|
||
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. :-) )
Whiteboard: fixed-in-tracemonkey
Comment 8•14 years ago
|
||
(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•14 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Depends on: 593963
You need to log in
before you can comment on or make changes to this bug.
Description
•