Change [[Delete]] to follow ES6

RESOLVED FIXED in Firefox 38

Status

()

Core
JavaScript Engine
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: evilpie, Assigned: evilpie)

Tracking

(Blocks: 1 bug)

Trunk
mozilla38
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox38 fixed)

Details

Attachments

(1 attachment)

Comment hidden (empty)
(Assignee)

Updated

3 years ago
Assignee: nobody → evilpies
(Assignee)

Comment 1

3 years ago
Created attachment 8558805 [details] [diff] [review]
v1 - Implement ES6 [[Delete]]

This was already pretty much okay, we were just waling the proto chain with LookupProperty.
Attachment #8558805 - Flags: review?(jwalden+bmo)
Comment on attachment 8558805 [details] [diff] [review]
v1 - Implement ES6 [[Delete]]

Review of attachment 8558805 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/src/jit-test/tests/proxy/delete-not-invoked-on-proto.js
@@ +1,2 @@
> +// Create Proxy that throws for everything.
> +var {proxy, revoke} = Proxy.revocable({}, {});

Instead of {} for handler, triggering default-er behavior (right?), seems better to use

var angryHandler = new Proxy({}, {
    get(t, id) { throw new Error("angryHandler should not be queried (" + id + ")"); }
});

as handler.  (Props to someone for putting that in a test I remember reviewing, tho I'm not sure if it was the one I just cribbed that from.)

::: js/src/vm/NativeObject.cpp
@@ +2232,5 @@
> +
> +    // Step 4.
> +    if (!shape) {
> +        // If no property call the class's delProperty hook, passing succeeded
> +        // as the result parameter. This always succeedes when there is no hook.

succeeds

@@ +2236,5 @@
> +        // as the result parameter. This always succeedes when there is no hook.
> +        return CallJSDeletePropertyOp(cx, obj->getClass()->delProperty, obj, id, succeeded);
> +    }
> +
> +    cx->runtime()->gc.poke();

wheeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee
Attachment #8558805 - Flags: review?(jwalden+bmo) → review+
(Assignee)

Comment 3

3 years ago
Comment on attachment 8558805 [details] [diff] [review]
v1 - Implement ES6 [[Delete]]

Review of attachment 8558805 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/src/jit-test/tests/proxy/delete-not-invoked-on-proto.js
@@ +1,2 @@
> +// Create Proxy that throws for everything.
> +var {proxy, revoke} = Proxy.revocable({}, {});

Well the idea that a revoked proxy throws for every hook, so I don't need to do it manually.
(Assignee)

Comment 4

3 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/ed5c9eded5a1
https://hg.mozilla.org/mozilla-central/rev/ed5c9eded5a1
Status: NEW → RESOLVED
Last Resolved: 3 years ago
status-firefox38: --- → fixed
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla38
You need to log in before you can comment on or make changes to this bug.