Make it possible to set a getter and a setter at the same time

Make it possible to set a getter and a setter at the same time

BUG=chromium:588893, chromium:325923
LOG=n
R=verwaest@chromium.org

Committed: https://crrev.com/e5baec22d30dd46fb08ee38545f0d81b1f840084
Cr-Commit-Position: refs/heads/master@{#36025}

[jochen (gone - plz use gerrit), 2016-05-03 12:51:36]

ptal

[haraken, 2016-05-03 13:28:35]

Thanks for moving the ball forward :)

[Toon Verwaest, 2016-05-04 09:32:42]

https://codereview.chromium.org/1943303002/diff/1/src/lookup.cc
File src/lookup.cc (right):

https://codereview.chromium.org/1943303002/diff/1/src/lookup.cc#newcode422
src/lookup.cc:422: pair->SetComponents(*getter, *setter);
There's no point in copying the pair if you're then going to overwrite both
entries.

https://codereview.chromium.org/1943303002/diff/1/src/objects.cc
File src/objects.cc (right):

https://codereview.chromium.org/1943303002/diff/1/src/objects.cc#newcode9725
src/objects.cc:9725: if (!getter->IsNull() &&
!current_pair->get(ACCESSOR_GETTER)->IsTheHole() &&
What about just unifying the accessor-missing sentinel across all paths? Given
that it's null coming in here, we can probably just use null in the accessor
pair? null, unlike undefined, seems to be an invalid accessor to specify. At
least right now I'm unsure what the issue would be.

https://codereview.chromium.org/1943303002/diff/1/src/objects.cc#newcode9745
src/objects.cc:9745: pair->SetComponents(*getter, *setter);
Unless you do the above, this probably will already replace the hole with null
in some cases.

[jochen (gone - plz use gerrit), 2016-05-04 09:36:37]

https://codereview.chromium.org/1943303002/diff/1/src/lookup.cc
File src/lookup.cc (right):

https://codereview.chromium.org/1943303002/diff/1/src/lookup.cc#newcode422
src/lookup.cc:422: pair->SetComponents(*getter, *setter);
On 2016/05/04 at 09:32:42, Toon Verwaest wrote:
> There's no point in copying the pair if you're then going to overwrite both
entries.

SetComponents only overwrites something if the component is not Null()

[Toon Verwaest, 2016-05-04 09:50:32]

Ok fine, but still, can't we unify to avoid all these mixed checks?

[Toon Verwaest, 2016-05-04 11:29:46]

lgtm, thanks!

[jochen (gone - plz use gerrit), 2016-05-04 13:44:49]

The CQ bit was checked by jochen@chromium.org

[jochen (gone - plz use gerrit), 2016-05-04 13:44:49]

The patchset sent to the CQ was uploaded after l-g-t-m from
verwaest@chromium.org
Link to the patchset: https://codereview.chromium.org/1943303002/#ps20001
(title: "rebase")

[commit-bot: I haz the power, 2016-05-04 13:44:51]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1943303002/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1943303002/20001

[commit-bot: I haz the power, 2016-05-04 14:14:22]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-05-04 14:15:45]

Description was changed from

==========
Make it possible to set a getter and a setter at the same time

BUG=chromium:588893, chromium:325923
LOG=n
R=verwaest@chromium.org
==========

to

==========
Make it possible to set a getter and a setter at the same time

BUG=chromium:588893, chromium:325923
LOG=n
R=verwaest@chromium.org

Committed: https://crrev.com/e5baec22d30dd46fb08ee38545f0d81b1f840084
Cr-Commit-Position: refs/heads/master@{#36025}
==========

[commit-bot: I haz the power, 2016-05-04 14:15:46]

Patchset 2 (id:??) landed as
https://crrev.com/e5baec22d30dd46fb08ee38545f0d81b1f840084
Cr-Commit-Position: refs/heads/master@{#36025}