First round of comments...
https://codereview.chromium.org/14862009/diff/1/src/code-stubs.cc
File src/code-stubs.cc (right):
https://codereview.chromium.org/14862009/diff/1/src/code-stubs.cc#newcode459
src/code-stubs.cc:459: void CompareNilICStub::Types::Print(StringStream*
stream) const {
This doesn't produce very readable output when Types contains more than
a single element.
https://codereview.chromium.org/14862009/diff/1/src/code-stubs.h
File src/code-stubs.h (right):
https://codereview.chromium.org/14862009/diff/1/src/code-stubs.h#newcode1045
src/code-stubs.h:1045: class Types {
Instead of inventing tons of ad-hoc helper functions, publicly derive
from EnumSet<Type,byte> and add just Print(StringStream*), perhaps
FullCompare(). Nothing else is really needed.
https://codereview.chromium.org/14862009/diff/1/src/code-stubs.h#newcode1135
src/code-stubs.h:1135: virtual Code::ExtraICState GetExtraICState() {
Use the BitField template instead of doing the bit fiddling by hand.
https://codereview.chromium.org/14862009/diff/1/src/code-stubs.h#newcode1146
src/code-stubs.h:1146:
Do we really need all those tiny helper functions below? One reason
could be avoiding "train wrecks" like foo()->bar()->baz(), but I am not
sure if this is the case here.
https://codereview.chromium.org/14862009/diff/1/src/code-stubs.h#newcode1152
src/code-stubs.h:1152: const Types GetTypes() const { return types_; }
Why 2 consts?
https://codereview.chromium.org/14862009/diff/1/src/hydrogen.cc
File src/hydrogen.cc (right):
https://codereview.chromium.org/14862009/diff/1/src/hydrogen.cc#newcode10687
src/hydrogen.cc:10687: if (nil == kNullValue) {
Using a ternary ?: is clearer here.
https://codereview.chromium.org/14862009/
--
--
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.
