encapsulate jsobj.cpp dslots accesses

RESOLVED FIXED

Status

()

RESOLVED FIXED
9 years ago
9 years ago

People

(Reporter: njn, Assigned: njn)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: fixed-in-tracemonkey)

Attachments

(1 attachment)

(Assignee)

Description

9 years ago
Created attachment 443275 [details] [diff] [review]
patch

his patch removes all but one (which I'll do later) of the non-encapsulated uses of dslots from jsobj.cpp.  Notable things:

- Adds addressOfDenseArrayElement().

- SLOTS_TO_DYNAMIC_WORDS and DYNAMIC_WORDS_TO_SLOTS were changed to JSObject
  inline methods.
  
- js_AllocSlots(), js_GrowSlots() and js_ShrinkSlots() were pulled in as 
  JSObject methods.
  
- I replaced this idiom:
  
    *slots++ = nslots;
    
  with this:
  
    slots++;
    slots[-1] = nslots;
    
  which is clearer and will make later changes to the meaning of slots[-1] 
  easier.
Attachment #443275 - Flags: review?(brendan)
(Assignee)

Updated

9 years ago
Blocks: 555429
(Assignee)

Updated

9 years ago
Blocks: 558262
No longer blocks: 555429
Comment on attachment 443275 [details] [diff] [review]
patch

>+    jsval *tmpdslots = dslots;
>+    tmpdslots = (jsval*) cx->realloc(dslots - 1, nwords * sizeof(jsval));

First set of tmpdslots is useless -- lose it.

>+    if (!tmpdslots)
>+        return false;
>+
>+    dslots = tmpdslots;

The tmpdslots name here is a bit off. Not sure what to suggest -- "dslotsvec"?

/be
Attachment #443275 - Flags: review?(brendan) → review+
(Assignee)

Comment 2

9 years ago
(In reply to comment #1)
> (From update of attachment 443275 [details] [diff] [review])
> >+    jsval *tmpdslots = dslots;
> >+    tmpdslots = (jsval*) cx->realloc(dslots - 1, nwords * sizeof(jsval));
> 
> First set of tmpdslots is useless -- lose it.
> 
> >+    if (!tmpdslots)
> >+        return false;
> >+
> >+    dslots = tmpdslots;
> 
> The tmpdslots name here is a bit off. Not sure what to suggest -- "dslotsvec"?

It feels spot on to me -- you can't use dslots directly because if realloc() fails you will have trashed the old dslots value.  So you need to use a temporary.  Hence tmpdslots.
(Assignee)

Comment 3

9 years ago
http://hg.mozilla.org/tracemonkey/rev/187831aa97ac
Whiteboard: fixed-in-tracemonkey
tmpdslots is ok -- I've used names like that and will again -- what I was getting at was that it never is advanced such that tmpdslots[-1] is the length. So could it have a different name to connote the lack of the +1 bias? No big.

/be

Comment 5

9 years ago
http://hg.mozilla.org/mozilla-central/rev/187831aa97ac
Status: ASSIGNED → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.