Last Comment Bug 677703 - Add element-valued property methods to ObjectOps
: Add element-valued property methods to ObjectOps
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: unspecified
: All All
: -- normal (vote)
: mozilla9
Assigned To: Jeff Walden [:Waldo] (remove +bmo to email)
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-08-09 14:57 PDT by Jeff Walden [:Waldo] (remove +bmo to email)
Modified: 2011-09-08 10:23 PDT (History)
2 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Add element versions of all the ObjectOps property methods, directly forwarding to the jsid versions for now (36.00 KB, patch)
2011-08-09 14:57 PDT, Jeff Walden [:Waldo] (remove +bmo to email)
cdleary: review+
Details | Diff | Splinter Review

Description Jeff Walden [:Waldo] (remove +bmo to email) 2011-08-09 14:57:56 PDT
Created attachment 551899 [details] [diff] [review]
Add element versions of all the ObjectOps property methods, directly forwarding to the jsid versions for now

This extends the element-based APIs downward from the JSAPI.  They won't be used now, generally (except in rare cases), but they'll be present and can be improved up to par with existing code before the element/non-element split becomes a fundamental part of property storage.

This is a lot of patch, but it is *very* easy to verify, because the element methods just funnel to the jsid methods right now.  I'll change that and optimize that in future bugs and/or patches.
Comment 1 Chris Leary [:cdleary] (not checking bugmail) 2011-09-06 10:44:57 PDT
Comment on attachment 551899 [details] [diff] [review]
Add element versions of all the ObjectOps property methods, directly forwarding to the jsid versions for now

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

::: js/src/jsapi.h
@@ +1976,4 @@
>      JSTraceOp           trace;
>  
>      JSClassInternal     reserved1;
> +    void                *reserved[28];

19 + 7 new object ops = 26? Otherwise I'm confused, and could use a clarifying reply.

::: js/src/jsarray.cpp
@@ +883,4 @@
>      return js_DefineProperty(cx, obj, id, value, getter, setter, attrs);
>  }
>  
> +JSBool

Can we add the reason why this isn't static as a comment?

@@ +954,4 @@
>      return JS_TRUE;
>  }
>  
> +JSBool

Ditto.

::: js/src/jsobj.cpp
@@ +3116,5 @@
> +{
> +    jsid id;
> +    if (!IndexToId(cx, index, &id))
> +        return false;
> +    return with_SetElement(cx, obj, index, vp, strict);

Infinite recursion.
Comment 2 Jeff Walden [:Waldo] (remove +bmo to email) 2011-09-07 09:12:18 PDT
http://hg.mozilla.org/integration/mozilla-inbound/rev/b221c62a19c3

Note You need to log in before you can comment on or make changes to this bug.