On 2009/02/17 13:39:02, Kasper Lund wrote: > LGTM. Do we have good test coverage of this stuff already?
Not really. I have added a couple of tests. > http://codereview.chromium.org/20419/diff/17/25 > File src/codegen-ia32.cc (right): > > http://codereview.chromium.org/20419/diff/17/25#newcode465 > Line 465: __ cmp(tmp2, 0); > More compact encoding would be: test(tmp2, tmp2) followed by j(not_zero, ...). > Alternatively you could get rid of the tmp2 altogether by doing a __ > cmp(ContextOperand(...), Immediate(0))... Done. > http://codereview.chromium.org/20419/diff/17/25#newcode2418 > Line 2418: // Check that all extension objects from the current context to > Why can't you use a variant of the ContextSlotOperandCheckExtensions code that > only checks extension objects for intermediate scopes that call eval? It would > be nice to be able to unroll the loop (I guess the nesting isn't very deep > usually) and only traverse the chain to the outermost scope that calls eval > instead of going all the way to the global context. Great idea! Done. This also revealed a bug where we would treat unresolved variables in local scopes inside an eval scope as globals. We cannot do that since we do not have information about the surrounding scopes in eval scopes. Therefore, there could be with statements in outer scopes that we do no know about. Fixed this issue by propagating information about eval scopes to inner scopes. Could you have a look again? http://codereview.chromium.org/20419 --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
