Closed Bug 444787 Opened 11 years ago Closed 11 years ago

Object.getPrototypeOf

Categories

(Core :: JavaScript Engine, defect)

x86
macOS
defect
Not set

Tracking

()

RESOLVED FIXED

People

(Reporter: sayrer, Assigned: mrbkap)

References

Details

(Keywords: dev-doc-complete)

Attachments

(2 files, 3 obsolete files)

Added by ES3.1
Summary: Object.getPrototype → Object.getPrototypeOf
Version: unspecified → Trunk
Attached patch Easy fix (obsolete) — Splinter Review
We don't need to worry about call or block objects here, because only the parent can expose those.
Assignee: general → mrbkap
Status: NEW → ASSIGNED
Attachment #329100 - Flags: review?(brendan)
Flags: wanted1.9.1+
Comment on attachment 329100 [details] [diff] [review]
Easy fix

>+    obj = js_ValueToNonNullObject(cx, vp[2]);
>+    if (!obj)
>+        return JS_FALSE;

15-July-08 ES3.1 spec says

15.2.3.2	Object.getPrototypeOf ( O )
When the getPrototypeOf method is called with argument O, the following steps are taken:
1.	If Type(O) is not Object throw a TypeError exception.
2.	Return the [[Prototype]] property of O.

So you do not wan to convert vp[2] to Object type -- rather throw if JSVAL_IS_PRIMITIVE(vp[2]).

>+    vp[2] = OBJECT_TO_JSVAL(obj);

So no need to update the rooted object argument.

/be
Yeah, the version of the spec that I was writing against hadn't quite defined the semantics yet, so I guessed. Updated patch coming.
Attached patch Updated to spec (obsolete) — Splinter Review
I'm not sure if the way I've done the error reporting here is the cleanest (should I be able to pass an index to the DVG?). I could convert js_ReportIsNullOrUndefined to be more general as well, but I didn't think it was necessary.
Attachment #329100 - Attachment is obsolete: true
Attachment #329798 - Flags: review?(brendan)
Attachment #329100 - Flags: review?(brendan)
Blocks: es5
Comment on attachment 329798 [details] [diff] [review]
Updated to spec

>+    if (JSVAL_IS_PRIMITIVE(vp[2])) {
>+        char *bytes;
>+
>+        bytes = js_DecompileValueGenerator(cx, JSDVG_SEARCH_STACK,
>+                                           vp[2], NULL);

Of course you should pass the spindex when you know which stack item is being blamed. In this case, we're in a fast native that has no stack frame, so the first argument (vp[2]) is top of stack. But someone could pass extra args, so you want cx->fp->sp - cx->fp->argc, I think. Get me on IRC if this doesn't work.

/be
Attached patch With updated error handling (obsolete) — Splinter Review
-argc provides access to the first argument. argc = 0 is a weird case, since that maps to JSDVG_IGNORE_STACK.
Attachment #329798 - Attachment is obsolete: true
Attachment #330988 - Flags: review?(brendan)
Attachment #329798 - Flags: review?(brendan)
Hot from the TC39 meeting: 3.1 folks agree to relax the type check to throw only on null and undefined, for primitive string/number/boolean get the prototype of the wrapper (String.prototype, Number.prototype, Boolean.prototype).

/be
Brendan: is that going to apply to Object.defineProperty as well?
Attachment #330988 - Attachment is obsolete: true
Attachment #332235 - Flags: review?(brendan)
Attachment #330988 - Flags: review?(brendan)
Comment on attachment 332235 [details] [diff] [review]
With even more updated error handling

Looks good.

Object.defineProperty can't do useful work on what we hope will be sealed/final string, double, and boolean "value type" objects. But the fact that these are sealed may be the reason, not lack of trying to wrap the ES1-3/JS1.x "primitive type" with a corresponding capitalized-name object wrapper.

Anyway, this is for the other bug, which awaits a better spec.

/be
Attachment #332235 - Flags: review?(brendan) → review+
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Attachment #332896 - Flags: review?(mrbkap)
Attachment #332896 - Flags: review?(mrbkap) → review+
test added in Bug 450386 
Flags: in-testsuite+
Flags: in-litmus-
ES5 draft still says to throw if O is not of type Object. I've pinged Allen Wirfs-Brock about this.

/be
Depends on: 509510
You need to log in before you can comment on or make changes to this bug.