LGTM with comments.
https://codereview.chromium.org/23615012/diff/1/src/ast.h
File src/ast.h (right):
https://codereview.chromium.org/23615012/diff/1/src/ast.h#newcode291
src/ast.h:291: void FilterForPossibleTransitions(Map* root_map, Zone*
zone) {
The "zone" parameter is obsolete, as we only remove elements with this
method and don't add new ones.
https://codereview.chromium.org/23615012/diff/1/src/hydrogen.cc
File src/hydrogen.cc (right):
https://codereview.chromium.org/23615012/diff/1/src/hydrogen.cc#newcode5706
src/hydrogen.cc:5706: if (types != NULL &&
obj->HasMonomorphicJSObjectType()) {
suggestion: Does it make sense to factor this out into a static helper
method that computes the receiver types for us? I was thinking along the
lines of a method with either of the following signatures (I prefer the
first one)...
// Return true if receiver is monomorphic.
static bool ComputeReceiverTypes(Expression* expr,
Object* receiver,
SmallMapList** types);
static SmallMapList* ComputeReceiverTypes(Expression* expr,
Object* receiver,
bool* monomorphic);
https://codereview.chromium.org/23615012/diff/1/src/hydrogen.cc#newcode6991
src/hydrogen.cc:6991:
nit: Drop one of the two empty newlines.
https://codereview.chromium.org/23615012/diff/1/src/hydrogen.cc#newcode7005
src/hydrogen.cc:7005: receiver_map = (types == NULL ||
types->is_empty())
I know it's not part of this change, but can "types" really be NULL or
empty of expr->IsMonomorphic() reports true? The lines 7003 until 7008
look needlessly verbose.
https://codereview.chromium.org/23615012/
--
--
v8-dev mailing list
v8-dev@googlegroups.com
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 v8-dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.
