Do not ignore an empty context with extension when creating a scope object.

src/scopes.cc

 // Copyright 2011 the V8 project authors. All rights reserved.
 // Redistribution and use in source and binary forms, with or without
 // modification, are permitted provided that the following conditions are
 // met:
 //
 //     * Redistributions of source code must retain the above copyright
 //       notice, this list of conditions and the following disclaimer.
 //     * Redistributions in binary form must reproduce the above
 //       copyright notice, this list of conditions and the following
 //       disclaimer in the documentation and/or other materials provided
@@ skipping 119 matching lines @@
   // At some point we might want to provide outer scopes to
   // eval scopes (by walking the stack and reading the scope info).
   // In that case, the ASSERT below needs to be adjusted.
   ASSERT_EQ(type == GLOBAL_SCOPE, outer_scope == NULL);
   ASSERT(!HasIllegalRedeclaration());
 }
 
 
 Scope::Scope(Scope* inner_scope,
              ScopeType type,
-             Handle<ScopeInfo> scope_info)
+             Handle<ScopeInfo> scope_info,
+             bool has_context_with_extension)
     : isolate_(Isolate::Current()),
       inner_scopes_(4),
       variables_(),
       temps_(4),
       params_(4),
       unresolved_(16),
       decls_(4),
       already_resolved_(true) {
   SetDefaults(type, NULL, scope_info);
-  if (!scope_info.is_null()) {
+  if (!scope_info.is_null() && *scope_info != ScopeInfo::Empty()) {

[Kevin Millikin (Chromium), 2012/02/02 08:36:13]

As far as I can tell, there is no reason *at all* that this function should ever
produce a scope indicating there's no context---because there is definitely a
materialized context.  I think it's much more straightforward to write this
function as:

SetDefaults(type, NULL, scope_info);
if (!scope_info.is_null()) {
  num_heap_slots_ = scope_info->ContextLength();
}
// Ensure at least MIN_CONTEXT_SLOTS to indicate a materialized context.
num_heap_slots_ = Max(num_heap_slots_, Context::MIN_CONTEXT_SLOTS);
AddInnerScope(inner_scope);

And no need for the default-valued parameter.  Does that implementation make
sense?

[ulan, 2012/02/02 09:16:19]

Indeed, this constructor is called only from DeserializeScopeChain.

     num_heap_slots_ = scope_info_->ContextLength();
-    if (*scope_info != ScopeInfo::Empty()) {

[Kevin Millikin (Chromium), 2012/02/02 08:36:13]

As a simple cleanup, you can get rid of this completely (SetDefaults already set
language_mode for all scopes, no need to overwrite it---unless I'm reading the
code wrong).

-      language_mode_ = scope_info->language_mode();
-    }
-  } else if (is_with_scope()) {
+    language_mode_ = scope_info->language_mode();
+  } else if (is_with_scope() || has_context_with_extension) {
     num_heap_slots_ = Context::MIN_CONTEXT_SLOTS;
   }
   AddInnerScope(inner_scope);
 }
 
 
 Scope::Scope(Scope* inner_scope, Handle<String> catch_variable_name)
     : isolate_(Isolate::Current()),
       inner_scopes_(1),
       variables_(),
@@ skipping 62 matching lines @@
       current_scope = with_scope;
       // All the inner scopes are inside a with.
       contains_with = true;
       for (Scope* s = innermost_scope; s != NULL; s = s->outer_scope()) {
         s->scope_inside_with_ = true;
       }
     } else if (context->IsFunctionContext()) {
       ScopeInfo* scope_info = context->closure()->shared()->scope_info();
       current_scope = new Scope(current_scope,
                                 FUNCTION_SCOPE,
-                                Handle<ScopeInfo>(scope_info));
+                                Handle<ScopeInfo>(scope_info),
+                                context->extension() != NULL);
     } else if (context->IsBlockContext()) {
       ScopeInfo* scope_info = ScopeInfo::cast(context->extension());
       current_scope = new Scope(current_scope,
                                 BLOCK_SCOPE,
                                 Handle<ScopeInfo>(scope_info));
     } else {
       ASSERT(context->IsCatchContext());
       String* name = String::cast(context->extension());
       current_scope = new Scope(current_scope, Handle<String>(name));
     }
@@ skipping 981 matching lines @@
 }
 
 
 int Scope::ContextLocalCount() const {
   if (num_heap_slots() == 0) return 0;
   return num_heap_slots() - Context::MIN_CONTEXT_SLOTS -
       (function_ != NULL && function_->var()->IsContextSlot() ? 1 : 0);
 }
 
 } }  // namespace v8::internal