First round of comments.
https://chromiumcodereview.appspot.com/10801013/diff/1/src/hydrogen.cc File src/hydrogen.cc (right): https://chromiumcodereview.appspot.com/10801013/diff/1/src/hydrogen.cc#newcode4778 src/hydrogen.cc:4778: CHECK_ALIVE(store = BuildCallSetter(literal, For object literals we should never get an accessor because we always use the internal Object() constructor and duplicate identifiers are not allowed in literals. So better turn this into an assertion here. https://chromiumcodereview.appspot.com/10801013/diff/1/src/hydrogen.cc#newcode5411 src/hydrogen.cc:5411: } This part cannot yet be inlined with out current machinery, because it requires follow-up work after the inlined getter returns. Unfortunately the getter will be inlined into the outer AST-context, which might be a test-context. In this case the is no single basic block that can contain the follow-up work. What we actually want to do is inline the getter into an artificial value-context (similar to what VisitForValue does), so that we have a single return block. https://chromiumcodereview.appspot.com/10801013/diff/1/src/hydrogen.cc#newcode5688 src/hydrogen.cc:5688: bool HGraphBuilder::IsAccessorCall(Handle<Map> map, The name of this method is misleading, because it actually performs a lookup. Can we call it "LookupAccessorCall" or "LookupAccessorPair" instead? https://chromiumcodereview.appspot.com/10801013/diff/1/src/hydrogen.cc#newcode7849 src/hydrogen.cc:7849: } Same comment as for HandleCompoundAssignment() applies here. https://chromiumcodereview.appspot.com/10801013/ -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
