LGTM with nits.
https://codereview.chromium.org/16361015/diff/3001/src/code-stubs.cc
File src/code-stubs.cc (right):
https://codereview.chromium.org/16361015/diff/3001/src/code-stubs.cc#newcode440
src/code-stubs.cc:440: object->IsOddball() ||
nit: I kinda preferred the old indentation here, but you can do as you
wish.
https://codereview.chromium.org/16361015/diff/3001/src/code-stubs.cc#newcode490
src/code-stubs.cc:490: Isolate* isolate, State state, Handle<Map> map) {
nit: one line per argument please
https://codereview.chromium.org/16361015/diff/3001/src/hydrogen.cc
File src/hydrogen.cc (left):
https://codereview.chromium.org/16361015/diff/3001/src/hydrogen.cc#oldcode1719
src/hydrogen.cc:1719:
ASSERT(!types.Contains(CompareNilICStub::UNDETECTABLE) ||
I think you could preserve this ASSERT:
if (type->Maybe(Type::Undetectable())) {
ASSERT(type->NumClasses() == 0);
...
https://codereview.chromium.org/16361015/diff/3001/src/ic.cc
File src/ic.cc (right):
https://codereview.chromium.org/16361015/diff/3001/src/ic.cc#newcode2776
src/ic.cc:2776: Isolate* isolate, CompareIC::State state, Handle<Map>
map) {
nit: for method definitions/declarations (as opposed to calls), one line
per argument please (unless the entire signature fits on one line).
https://codereview.chromium.org/16361015/diff/3001/src/types.h
File src/types.h (right):
https://codereview.chromium.org/16361015/diff/3001/src/types.h#newcode95
src/types.h:95: static Type* Boxed() { return from_bitset(kBoxed); }
Why not "HeapObject"? I don't care much either way, but HeapObject is
the term we've been using so far for "anything that has a heap object
tag".
https://codereview.chromium.org/16361015/diff/3001/src/types.h#newcode149
src/types.h:149: Handle<T> Get();
nit: most of our iterators use the names "Current()" and "Advance()". (I
don't feel very strongly about it, but consistency is nice and
"Advance()" is arguably a better name than "Next()" -- I associate
"next()" with iterators that actually return the next element from that
function.)
https://codereview.chromium.org/16361015/
--
--
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/groups/opt_out.
