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() ||
On 2013/06/11 17:58:07, Jakob wrote:
nit: I kinda preferred the old indentation here, but you can do as you
wish.
Done. (That actually was an unintended side-effect of using Ctrl-I in
Eclipse.)
https://codereview.chromium.org/16361015/diff/3001/src/code-stubs.cc#newcode490
src/code-stubs.cc:490: Isolate* isolate, State state, Handle<Map> map) {
On 2013/06/11 17:58:07, Jakob wrote:
nit: one line per argument please
Done, but, seriously? :)
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) ||
On 2013/06/11 17:58:07, Jakob wrote:
I think you could preserve this ASSERT:
if (type->Maybe(Type::Undetectable())) {
ASSERT(type->NumClasses() == 0);
...
No, as discussed offline, this would no longer be correct once the type
information flowing into this function is derived from anything else but
the stub state alone.
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) {
On 2013/06/11 17:58:07, Jakob wrote:
nit: for method definitions/declarations (as opposed to calls), one
line per
argument please (unless the entire signature fits on one line).
Done.
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); }
On 2013/06/11 17:58:07, Jakob wrote:
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".
Produces annoying name clashes between type and function in the class.
Chose "Allocated" instead.
https://codereview.chromium.org/16361015/diff/3001/src/types.h#newcode149
src/types.h:149: Handle<T> Get();
On 2013/06/11 17:58:07, Jakob wrote:
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.)
Done.
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.
