Closed Bug 476447 Opened 15 years ago Closed 15 years ago

Array.prototype getter/setter does not work as usual

Categories

(Core :: JavaScript Engine, defect, P2)

defect

Tracking

()

VERIFIED FIXED
mozilla1.9.1

People

(Reporter: simonzack, Assigned: brendan)

References

Details

(Keywords: regression, testcase, verified1.9.1, Whiteboard: fixed-in-tracemonkey)

Attachments

(2 files, 1 obsolete file)

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.0.5) Gecko/2008120122 Firefox/3.0.5 (.NET CLR 3.5.30729)
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.0.5) Gecko/2008120122 Firefox/3.0.5 (.NET CLR 3.5.30729)

Array.protoype getters and setters not working

Reproducible: Always

Steps to Reproduce:
code:
Array.prototype.__defineGetter__('lastElement',function() this[this.length-1]);
Array.prototype.__defineSetter__('lastElement',function(num){this[this.length-1]=num});
var testArr=[1,2,3,4];
alert(testArr.lastElement);

Actual Results:  
Produces undefined

Expected Results:  
Produces 4
I see this only with latest trunk and Shiretoko (on Windows XP) but not with GranParadiso/3.0.7pre, so I assume this is a regression.
Assignee: nobody → general
Component: General → JavaScript Engine
Keywords: regression
Product: Firefox → Core
QA Contact: general → general
Version: unspecified → Trunk
Status: UNCONFIRMED → NEW
Ever confirmed: true
Keywords: testcase
OS: Windows XP → All
test needed {} for the getter, but the regression is real.
Attachment #360100 - Attachment is obsolete: true
bug 322889 regressed this.
The bug isn't what I thought.  The getter actually gets called, but with the wrong `this` parameter due, once again, to the goofy special case in JSOP_GETPROP, where we do:

    aobj = OBJ_IS_DENSE_ARRAY(cx, obj) ? OBJ_GET_PROTO(cx, obj) : obj;

Later js_GetPropertyHelper is being called with aobj rather than obj.  This too is intentional, as the function fills the property cache--but the behavior is wrong in this case.
(In reply to comment #6)
> The bug isn't what I thought.  The getter actually gets called, but with the
> wrong `this` parameter due, once again, to the goofy special case in
> JSOP_GETPROP, where we do:
> 
>     aobj = OBJ_IS_DENSE_ARRAY(cx, obj) ? OBJ_GET_PROTO(cx, obj) : obj;

Goofy or not, calling methods on dense arrays requires it to avoid going slow (as in Firefox 2 slow). By contrast, the well-predicted/branch-target-in-pipe-if-mispredicted mighty ternary operator (aobj = OBJ_IS_DENSE_ARRAY(cx, obj) ? OBJ_GET_PROTO(cx, obj) : obj) wins.

> Later js_GetPropertyHelper is being called with aobj rather than obj.  This too
> is intentional, as the function fills the property cache--but the behavior is
> wrong in this case.

Just need a bit more mighty-ternary-operator usage (I will probably make an inline helper to abstract this and take all the fun out) in js_GetPropertyHelper. Patch shortly.

/be
Pretty bad regression from Firefox 3.

/be
Assignee: general → brendan
Status: NEW → ASSIGNED
Flags: wanted1.9.1?
Priority: -- → P2
Hardware: x86 → All
Target Milestone: --- → mozilla1.9.1
Attached patch fixSplinter Review
Attachment #366053 - Flags: review?(jorendorff)
Comment on attachment 366053 [details] [diff] [review]
fix

Yes, looks good.
Attachment #366053 - Flags: review?(jorendorff) → review+
Fixed in tm:

http://hg.mozilla.org/tracemonkey/rev/fb9d906c42c1

/be
Whiteboard: fixed-in-tracemonkey
Flags: wanted1.9.1? → wanted1.9.1+
http://hg.mozilla.org/mozilla-central/rev/fb9d906c42c1
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Blocks: 474402
v 1.9.1, 1.9.2
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: