https://codereview.chromium.org/934463003/diff/100001/src/objects.cc
File src/objects.cc (right):
https://codereview.chromium.org/934463003/diff/100001/src/objects.cc#newcode3149
src/objects.cc:3149: if (data_store_mode == SUPER_PROPERTY) {
Given that this flag just switches half of the implementation of this
method, what about removing the flag and instead extracting the code
above into a separate helper function used by both SetProperty and
SetSuperProperty?
The code below is a bit scary. Can you please replace it with switch
(it->state()) { ... } without "default:" to ensure all possible
LookupIterator states are handled? That would make it obvious what e.g.,
has to happen in the ACCESS_CHECK case.
https://codereview.chromium.org/934463003/diff/100001/src/objects.cc#newcode12880
src/objects.cc:12880: details.IsReadOnly() && details.kind() == kData)
||
details.kind() == kData isn't necessary since DEFINE + !Configurable +
Accessor is already handled above.
https://codereview.chromium.org/934463003/diff/100001/src/objects.cc#newcode12880
src/objects.cc:12880: details.IsReadOnly() && details.kind() == kData)
||
On 2015/02/18 01:26:05, adamk wrote:
I take it we don't need this !IsTheHole() check in the DEFINE_PROPERTY
case
simply because we don't exercise that code path?
We could add a DCHECK(element->IsTheHole()) inside the branch...
https://codereview.chromium.org/934463003/
--
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to v8-dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
