LGTM with a few nits.
https://codereview.chromium.org/16154027/diff/9/src/types.cc
File src/types.cc (right):
https://codereview.chromium.org/16154027/diff/9/src/types.cc#newcode95
src/types.cc:95: default:
Are there really unhandled cases? If yes, how many? Default cases are a
road to maintenance hell...
https://codereview.chromium.org/16154027/diff/9/src/types.cc#newcode246
src/types.cc:246: if (isolate == NULL) isolate =
HeapObject::cast(*type2)->GetIsolate();
The test is a nano-optimization, leaving it out makes things more
symmetrical.
https://codereview.chromium.org/16154027/diff/9/src/types.cc#newcode272
src/types.cc:272: if (type->is_bitset())
I would have expected a ternary ?: from you... ;-)
https://codereview.chromium.org/16154027/diff/9/src/types.h
File src/types.h (right):
https://codereview.chromium.org/16154027/diff/9/src/types.h#newcode38
src/types.h:38: class Type : public Object {
Although I hate comments in general, a comment describing the cunning
Type representation might be appropriate here.
https://codereview.chromium.org/16154027/diff/9/src/types.h#newcode117
src/types.h:117: Handle<HeapObject> as_constant() { return
Handle<HeapObject>::cast(handle());}
Cheater! ;-)
https://codereview.chromium.org/16154027/diff/9/src/types.h#newcode122
src/types.h:122: ASSERT(type->IsHeapObject());
No need for this ASSERT, HeapObject::cast already does the same.
https://codereview.chromium.org/16154027/
--
--
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.
