Closed Bug 730977 Opened 12 years ago Closed 12 years ago

IonMonkey: Implement JSOP_DELPROP

Categories

(Core :: JavaScript Engine, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: djvj, Assigned: djvj)

Details

Attachments

(1 file, 1 obsolete file)

      No description provided.
Attached patch Patch v1 (obsolete) — Splinter Review
Attachment #602424 - Flags: review?(dvander)
Comment on attachment 602424 [details] [diff] [review]
Patch v1

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

Looks great, thanks for the tests! r=me with some minor nits:

::: js/src/ion/MIR.h
@@ +3380,5 @@
> +    bool strict_;
> +    bool needsBarrier_;
> +
> +  protected:
> +    MDeleteProperty(MDefinition *val, JSAtom *atom, bool strict)

Instead of taking in a strict parameter here...

@@ +3403,5 @@
> +    virtual TypePolicy *typePolicy() {
> +        return this;
> +    }
> +    bool strict() const {
> +        return strict_;

Here, you can return block()->info().script()->strictModeCode.

@@ +3410,5 @@
> +        return needsBarrier_;
> +    }
> +    void setNeedsBarrier() {
> +        needsBarrier_ = true;
> +    }

needsBarrier functions aren't needed here (they're for property store ops)

::: js/src/jsinterp.cpp
@@ +4572,5 @@
> +    for(; (i < 99) && (jss[i] != 0); i++) {
> +        c[i] = (char) jss[i];
> +    }
> +    c[i] = '\0';
> +    */

^ You can remove this commented code.

@@ +4581,5 @@
> +    // convert value to JSObject pointer
> +    JSObject *obj = ValueToObject(ctx, val);
> +    if (!obj) {
> +        return false;
> +    }

Style nit: for single-line |if|, remove the braces.

@@ +4588,5 @@
> +    Value result = NullValue();
> +    bool delprop_ok = obj->deleteProperty(ctx, name, &result, strict);
> +    if(!delprop_ok) {
> +        return false;
> +    }

^ Here as well.
Attachment #602424 - Flags: review?(dvander) → review+
Attached patch Patch v2Splinter Review
Attachment #602424 - Attachment is obsolete: true
Attachment #603419 - Flags: review?(dvander)
Attachment #603419 - Flags: review?(dvander) → review+
http://hg.mozilla.org/projects/ionmonkey/rev/5c7806169494
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.