Properly create variables to access outer arguments and function names.

src/scopes.cc

 // Copyright 2010 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 136 matching lines @@
     params_(4),
     unresolved_(16),
     decls_(4) {
   ASSERT(scope_info != NULL);
   SetDefaults(FUNCTION_SCOPE, inner_scope->outer_scope(), scope_info);
   ASSERT(resolved());
   InsertAfterScope(inner_scope);
   if (scope_info->HasHeapAllocatedLocals()) {
     num_heap_slots_ = scope_info_->NumberOfContextSlots();
   }
+
+  // arguments shadow would be saved to the list of context slots
+  // only if inner scopes access some parameters.  Prepare to
+  // this situation by allocating arguments_shadow_ object
+  // and thread access to this scope's parameters via arguments shadow.

[Kevin Millikin (Chromium), 2011/01/19 07:39:57]

Suggested rewording, something like:

This scope's arguments shadow (if present) is context-allocated if an inner
scope accesses this one's parameters.  Allocate the arguments_shadow_ variable
if necessary.

[antonm, 2011/01/19 08:18:19]

Done.

+  Variable::Mode mode;
+  int arguments_shadow_index =
+      scope_info_->ContextSlotIndex(Heap::arguments_shadow_symbol(), &mode);
+  if (arguments_shadow_index >= 0) {
+    ASSERT(mode == Variable::INTERNAL);
+    arguments_shadow_ = new Variable(this,
+                                     Factory::arguments_shadow_symbol(),
+                                     Variable::INTERNAL,
+                                     true,
+                                     Variable::ARGUMENTS);
+    arguments_shadow_->set_rewrite(
+        new Slot(arguments_shadow_, Slot::CONTEXT, arguments_shadow_index));
+    arguments_shadow_->set_is_used(true);
+  }
 }
 
 
 
 bool Scope::Analyze(CompilationInfo* info) {
   ASSERT(info->function() != NULL);
   Scope* top = info->function()->scope();
 
   // If we have a serialized scope info, reuse it.
   if (!info->closure().is_null()) {
@@ skipping 65 matching lines @@
 
 
 Variable* Scope::LocalLookup(Handle<String> name) {
   Variable* result = variables_.Lookup(name);
   if (result != NULL || !resolved()) {
     return result;
   }
   // If the scope is resolved, we can find a variable in serialized scope info.
 
   // We should never lookup 'arguments' in this scope
   // as it is impllicitly present in any scope.

[Kevin Millikin (Chromium), 2011/01/19 07:39:57]

Could you fix the spelling of "implicitly"?

[antonm, 2011/01/19 08:18:19]

Done.

   ASSERT(*name != *Factory::arguments_symbol());
 
   // Check context slot lookup.
   Variable::Mode mode;
   int index = scope_info_->ContextSlotIndex(*name, &mode);
-  if (index < 0) {
-    return NULL;
+  if (index >= 0) {
+    // Check that there is no local slot with the given name.
+    ASSERT(scope_info_->StackSlotIndex(*name) < 0);

[Kevin Millikin (Chromium), 2011/01/19 07:39:57]

This assert could be lifted higher (up to the previous assert), couldn't it? 
The comment should say "assert" not "check".

[antonm, 2011/01/19 08:18:19]

Done.

+    Variable* var =
+        variables_.Declare(this, name, mode, true, Variable::NORMAL);
+    var->set_rewrite(new Slot(var, Slot::CONTEXT, index));
+    return var;
   }
 
-  // Check that there is no local slot with the given name.
-  ASSERT(scope_info_->StackSlotIndex(*name) < 0);
-  Variable* var = variables_.Declare(this, name, mode, true, Variable::NORMAL);
-  var->set_rewrite(new Slot(var, Slot::CONTEXT, index));
-  return var;
+  index = scope_info_->ParameterIndex(*name);
+  if (index >= 0) {
+    // ".arguments" must be present in context slots.
+    ASSERT(arguments_shadow_ != NULL);
+    // Check that there is no local slot with the given name.
+    ASSERT(scope_info_->StackSlotIndex(*name) < 0);
+    Variable* var =
+        variables_.Declare(this, name, Variable::VAR, true, Variable::NORMAL);
+    Property* rewrite =
+        new Property(new VariableProxy(arguments_shadow_),
+                      new Literal(Handle<Object>(Smi::FromInt(index))),

[Kevin Millikin (Chromium), 2011/01/19 07:39:57]

Indentation is a bit off here.

[antonm, 2011/01/19 08:18:19]

Done.

+                      RelocInfo::kNoPosition,
+                      Property::SYNTHETIC);
+    rewrite->set_is_arguments_access(true);
+    var->set_rewrite(rewrite);
+    return var;
+  }
+
+  index = scope_info_->FunctionContextSlotIndex(*name);
+  if (index >= 0) {
+    // Check that there is no local slot with the given name.
+    ASSERT(scope_info_->StackSlotIndex(*name) < 0);
+    Variable* var =
+        variables_.Declare(this, name, mode, true, Variable::NORMAL);

[Kevin Millikin (Chromium), 2011/01/19 07:39:57]

Will this be the right mode?

[antonm, 2011/01/19 08:18:19]

Oops, thanks a lot for spotting this.  Now passing explicit constant.

+    var->set_rewrite(new Slot(var, Slot::CONTEXT, index));
+    return var;
+  }
+
+  return NULL;
 }
 
 
 Variable* Scope::Lookup(Handle<String> name) {
   for (Scope* scope = this;
        scope != NULL;
        scope = scope->outer_scope()) {
     Variable* var = scope->LocalLookup(name);
     if (var != NULL) return var;
   }
@@ skipping 742 matching lines @@
   if (num_heap_slots_ == Context::MIN_CONTEXT_SLOTS &&
       !must_have_local_context) {
     num_heap_slots_ = 0;
   }
 
   // Allocation done.
   ASSERT(num_heap_slots_ == 0 || num_heap_slots_ >= Context::MIN_CONTEXT_SLOTS);
 }
 
 } }  // namespace v8::internal