769132 - Add a receiver argument to the property-setting algorithms

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Jeff Walden [:Waldo]]

Attachment: Add the argument

This is needed so that various things pass along the correct |this| to accessors and so on, and it's in the way of a minor bit of patchwork I'm working on now in an unrelated matter.  This patch triggers no regressions in jit-tests or jstests.  (It certainly could use a try run, tho, which I'll definitely do before this lands.)

This patch certainly raises the possibility of having a shorthand that would omit the receiver bit, since most callers don't pass an unusual receiver.  However, this presents issues in terms of then having to root |this| to pass a handle for it as receiver, which seems poor.  Plus EIBTI.  And I think this will play better with some of the new property API work I'm landing to prep for the object representation switch.  So unless there's some strongly vocal argument otherwise, I'd rather make everyone pass an explicit receiver.

Note that one place where you'd expect to pass a different object and receiver, for ArrayBuffers' __proto__ property, you actually can't, because the current code there *expects* the receiver not to be the array buffer object.  (That was the only tweak I had to make to a mostly mechanical patch to get it to pass tests.)  Hopefully we can make this read better soon with other patchwork in the area.

I didn't add the argument to the nonNativeSet* methods somewhat out of caution of passing an unexpected object to C++-implemented methods.  Given that that non-native speciality will go away in the course of new-representation hacking, this seems the simplest path forward for now.

Comment 1

[Luke Wagner [:luke]]

Comment on attachment 637357 [details] [diff] [review]
Add the argument

Could you please you xArg instead of x_ when parameter names conflict?  (Trailing _, when used, means "member".)

Comment 2

[Jeff Walden [:Waldo]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/03549c72043d

This wasn't quite green on try, initially -- I was crashing in a crashtest concerning __proto__ manipulation.  A little closer examination pointed out that the posted patch changed the object passed to JSPropertyOp and JSStrictPropertyOp when they're invoked through a proxy -- bad mojo.  So I made a few tweaks to pass along both that object and the receiver into those methods (which needed both so that function-based accessors would receive the receiver, not the obj).  With those changes this passed Linux64 debug try, and I pushed.

Comment 3

[Ryan VanderMeulen [:RyanVM]]

Sorry, this got caught up in an Android XUL browser-chrome orange investigation. It was backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/8cc01c494f6a.

It was re-landed in:
https://hg.mozilla.org/integration/mozilla-inbound/rev/4c2ddc60f360

Apologies for the churn.

Comment 4

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/4c2ddc60f360