Thanks for the review; handing over.
https://codereview.chromium.org/494153002/diff/1/src/ic.cc
File src/ic.cc (right):
https://codereview.chromium.org/494153002/diff/1/src/ic.cc#newcode1288
src/ic.cc:1288: }
On 2014/08/21 15:07:05, Toon Verwaest wrote:
You already found a property, so you need to make a decision. I
presume you want
to return true?
So basically:
return it->property_encoding() == LookupIterator::DESCRIPTOR?
Done.
https://codereview.chromium.org/494153002/diff/1/src/ic.cc#newcode1348
src/ic.cc:1348: // LookupResult lookup(isolate());
On 2014/08/21 15:07:05, Toon Verwaest wrote:
Remove leftover
Done.
https://codereview.chromium.org/494153002/diff/1/src/ic.cc#newcode1351
src/ic.cc:1351: if (!can_store && strict_mode() == STRICT &&
object->IsGlobalObject() &&
On 2014/08/21 15:07:05, Toon Verwaest wrote:
This shouldn't depend on "can_store". Before the reference error you
could
DCHECK(!can_store) though.
It does, actually, because we're interested in nonexistent properties.
Removing the condition would mean that we erroneously throw when trying
to overwrite writable global properties.
https://codereview.chromium.org/494153002/diff/1/src/ic.cc#newcode1352
src/ic.cc:1352: !(lookup.state() == LookupIterator::PROPERTY &&
lookup.has_property() &&
On 2014/08/21 15:07:06, Toon Verwaest wrote:
You shouldn't need to expose has_property(). Any iteration that
doesn't
has_property after visiting PROPERTY should have called Next();
continuing to
NOT_FOUND.
Done.
https://codereview.chromium.org/494153002/diff/1/src/ic.cc#newcode1426
src/ic.cc:1426: DCHECK(lookup->state() != LookupIterator::JSPROXY);
On 2014/08/21 15:07:06, Toon Verwaest wrote:
DCHECK_NE
Done.
https://codereview.chromium.org/494153002/diff/1/src/lookup.h
File src/lookup.h (right):
https://codereview.chromium.org/494153002/diff/1/src/lookup.h#newcode118
src/lookup.h:118: Handle<JSObject> GetPropertyTarget() const;
On 2014/08/21 15:07:06, Toon Verwaest wrote:
GetStoreTarget() ?
Done.
https://codereview.chromium.org/494153002/diff/1/src/lookup.h#newcode142
src/lookup.h:142: bool has_property() const { return has_property_; }
On 2014/08/21 15:07:06, Toon Verwaest wrote:
This shouldn't be necessary
Done.
https://codereview.chromium.org/494153002/
--
--
v8-dev mailing list
[email protected]
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 [email protected].
For more options, visit https://groups.google.com/d/optout.
