I did not get through all the files yet. Here are my first comments.
Also I find the issue description not very informative. A commit message of a
non-trivial change should usually include something along these lines: 1. What is the problem this CL solves? (problem description) 2. How is the problem solved in this CL? (outline of the approach) 3. What is the benefit? (e.g. better performance on x, features y supported, etc.) 4. BUG= and TEST= lines if applicable. http://codereview.chromium.org/8508052/diff/6020/src/scopes.cc File src/scopes.cc (right): http://codereview.chromium.org/8508052/diff/6020/src/scopes.cc#newcode132 src/scopes.cc:132: ASSERT((type == GLOBAL_SCOPE) == (outer_scope == NULL)); Use ASSERT_EQ instead, so that you get a better error message if the assertion fails: ASSERT_EQ(type == GLOBAL_SCOPE, outer_scope == NULL); http://codereview.chromium.org/8508052/diff/6020/src/scopes.cc#newcode269 src/scopes.cc:269: !top->outer_scope()->already_resolved()) { This loop condition seems inconsistent with the comment. http://codereview.chromium.org/8508052/ -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
