A collection of context-related refactoring changes.

A collection of context-related refactoring changes.

Introduce separate maps for function and with contexts.  Use the function
context map for testing whether a context is a function context (global
contexts are no longer function contexts).

Split the paths for allocating with and catch contexts.

Rename some functions.  Generally refactor code to make it simpler.

R=ager@chromium.org
BUG=
TEST=

Committed: http://code.google.com/p/v8/source/detail?r=8231

[Kevin Millikin (Chromium), 2011-06-08 16:35:51]

It includes some renaming so it's not as big as it looks.

http://codereview.chromium.org/7003058/diff/1/src/contexts.cc
File src/contexts.cc (left):

http://codereview.chromium.org/7003058/diff/1/src/contexts.cc#oldcode122
src/contexts.cc:122: if (context->is_function_context()) {
Global contexts used to satisfy this predicate, but they have the empty function
as closure, so there were never any locals, parameters, or a function name. 
It's safe to skip this for global contexts.

http://codereview.chromium.org/7003058/diff/1/src/contexts.cc#oldcode192
src/contexts.cc:192: } else if (context->is_function_context()) {
Global contexts couldn't reach this site.

http://codereview.chromium.org/7003058/diff/1/src/contexts.cc#oldcode219
src/contexts.cc:219: ASSERT(context->is_function_context());
Global contexts couldn't reach this site.

http://codereview.chromium.org/7003058/diff/1/src/contexts.h
File src/contexts.h (left):

http://codereview.chromium.org/7003058/diff/1/src/contexts.h#oldcode293
src/contexts.h:293: bool is_function_context() { return unchecked_previous() ==
NULL; }
I plan on next using previous() for context iteration, but then function
contexts will have to have previous() != NULL.

http://codereview.chromium.org/7003058/diff/1/src/objects-inl.h
File src/objects-inl.h (left):

http://codereview.chromium.org/7003058/diff/1/src/objects-inl.h#oldcode535
src/objects-inl.h:535: bool Object::IsCatchContext() {
We only ever ask this of Contexts, so I just moved it there (and avoided the
HeapObject check every time).

http://codereview.chromium.org/7003058/diff/1/src/runtime.cc
File src/runtime.cc (left):

http://codereview.chromium.org/7003058/diff/1/src/runtime.cc#oldcode8610
src/runtime.cc:8610: if (context->is_function_context()) {
Global contexts could not reach this site.

http://codereview.chromium.org/7003058/diff/1/src/runtime.cc#oldcode10142
src/runtime.cc:10142: ASSERT(context->is_function_context());
It's a little convoluted, but MaterializeClosure is never called with a global
context.

http://codereview.chromium.org/7003058/diff/1/src/runtime.cc#oldcode10241
src/runtime.cc:10241: } else if (context_->is_function_context()) {
Global contexts could not reach this site.

http://codereview.chromium.org/7003058/diff/1/src/runtime.cc#oldcode10275
src/runtime.cc:10275: if (context_->is_function_context()) {
Also here.

http://codereview.chromium.org/7003058/diff/1/src/runtime.cc#oldcode10298
src/runtime.cc:10298: if (context_->is_function_context()) {
Also here.

http://codereview.chromium.org/7003058/diff/1/src/runtime.cc
File src/runtime.cc (right):

http://codereview.chromium.org/7003058/diff/1/src/runtime.cc#newcode10865
src/runtime.cc:10865: // At the end of the chain. Return the bast context to
link to.
bast ==> base.  Duh.

[Mads Ager (chromium), 2011-06-09 07:31:00]

LGTM

http://codereview.chromium.org/7003058/diff/1/src/ast.h
File src/ast.h (right):

http://codereview.chromium.org/7003058/diff/1/src/ast.h#newcode63
src/ast.h:63: V(WithEnterStatement)                         \
Do we want to call this WithContextEnterStatement so it is clear that
ContextExitStatement will exit a context that can be either a with context of
some other context?

http://codereview.chromium.org/7003058/diff/1/src/contexts.cc
File src/contexts.cc (left):

http://codereview.chromium.org/7003058/diff/1/src/contexts.cc#oldcode122
src/contexts.cc:122: if (context->is_function_context()) {
On 2011/06/08 16:35:52, Kevin Millikin wrote:
> Global contexts used to satisfy this predicate, but they have the empty
function
> as closure, so there were never any locals, parameters, or a function name. 
> It's safe to skip this for global contexts.

Could you add a comment to that effect before the conditional?

http://codereview.chromium.org/7003058/diff/1/src/runtime.cc
File src/runtime.cc (right):

http://codereview.chromium.org/7003058/diff/1/src/runtime.cc#newcode10280
src/runtime.cc:10280: (context_->IsGlobalContext() ||
(context_->IsFunctionContext()))) {
Can't we remove the IsGlobalContext here as well since we would return if
context_ is global above?

[Kevin Millikin (Chromium), 2011-06-09 11:09:40]

http://codereview.chromium.org/7003058/diff/1/src/runtime.cc
File src/runtime.cc (right):

http://codereview.chromium.org/7003058/diff/1/src/runtime.cc#newcode10280
src/runtime.cc:10280: (context_->IsGlobalContext() ||
(context_->IsFunctionContext()))) {
On 2011/06/09 07:31:00, Mads Ager wrote:
> Can't we remove the IsGlobalContext here as well since we would return if
> context_ is global above?

Not here, because this context_ is already the next ("previous") one in the
chain.

[Mads Ager (chromium), 2011-06-09 11:12:53]

http://codereview.chromium.org/7003058/diff/1/src/runtime.cc
File src/runtime.cc (right):

http://codereview.chromium.org/7003058/diff/1/src/runtime.cc#newcode10280
src/runtime.cc:10280: (context_->IsGlobalContext() ||
(context_->IsFunctionContext()))) {
On 2011/06/09 11:09:40, Kevin Millikin wrote:
> On 2011/06/09 07:31:00, Mads Ager wrote:
> > Can't we remove the IsGlobalContext here as well since we would return if
> > context_ is global above?
> 
> Not here, because this context_ is already the next ("previous") one in the
> chain.

Oh, yeah. Thanks!