Merge arguments branch to bleeding edge (second try).

src/scopes.cc

Index: src/scopes.cc
diff --git a/src/scopes.cc b/src/scopes.cc
index e159257164086840e690f66f4006a202fb00df54..74d0c2a2e258f4140fa7f0b344b4eadfa8172854 100644
--- a/src/scopes.cc
+++ b/src/scopes.cc
@@ -157,27 +157,6 @@ Scope::Scope(Scope* inner_scope, Handle<SerializedScopeInfo> scope_info)
   }
 
   AddInnerScope(inner_scope);
-
-  // 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.
-  Isolate* isolate = Isolate::Current();
-  Variable::Mode mode;
-  int arguments_shadow_index =
-      scope_info_->ContextSlotIndex(
-          isolate->heap()->arguments_shadow_symbol(), &mode);
-  if (arguments_shadow_index >= 0) {
-    ASSERT(mode == Variable::INTERNAL);
-    arguments_shadow_ = new Variable(
-        this,
-        isolate->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);
-  }
 }
 
 
@@ -191,7 +170,6 @@ void Scope::SetDefaults(Type type,
   receiver_ = NULL;
   function_ = NULL;
   arguments_ = NULL;
-  arguments_shadow_ = NULL;
   illegal_redecl_ = NULL;
   scope_inside_with_ = false;
   scope_contains_with_ = false;
@@ -299,52 +277,33 @@ Variable* Scope::LocalLookup(Handle<String> 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 implicitly present in any scope.
+  // 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 implicitly
+  // present in every scope.
   ASSERT(*name != *FACTORY->arguments_symbol());
-
-  // Assert that there is no local slot with the given name.
+  // There should be no local slot with the given name.
   ASSERT(scope_info_->StackSlotIndex(*name) < 0);
 
   // Check context slot lookup.
   Variable::Mode mode;
   int index = scope_info_->ContextSlotIndex(*name, &mode);
-  if (index >= 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);
-    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))),
-                     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, Variable::VAR, true, Variable::NORMAL);
-    var->set_rewrite(new Slot(var, Slot::CONTEXT, index));
-    return var;
+  if (index < 0) {
+    // Check parameters.
+    mode = Variable::VAR;
+    index = scope_info_->ParameterIndex(*name);
+    if (index < 0) {
+      // Check the function name.
+      index = scope_info_->FunctionContextSlotIndex(*name);
+      if (index < 0) return NULL;
+    }
   }
 
-  return NULL;
+  Variable* var =
+      variables_.Declare(this, name, mode, true, Variable::NORMAL);
+  var->set_rewrite(new Slot(var, Slot::CONTEXT, index));
+  return var;
 }
 
 
@@ -952,36 +911,17 @@ void Scope::AllocateParameterLocals() {
   Variable* arguments = LocalLookup(FACTORY->arguments_symbol());
   ASSERT(arguments != NULL);  // functions have 'arguments' declared implicitly
 
-  // Parameters are rewritten to arguments[i] if 'arguments' is used in
-  // a non-strict mode function. Strict mode code doesn't alias arguments.
-  bool rewrite_parameters = false;
+  bool uses_nonstrict_arguments = false;
 
   if (MustAllocate(arguments) && !HasArgumentsParameter()) {
     // 'arguments' is used. Unless there is also a parameter called
-    // 'arguments', we must be conservative and access all parameters via
-    // the arguments object: The i'th parameter is rewritten into
-    // '.arguments[i]' (*). If we have a parameter named 'arguments', a
-    // (new) value is always assigned to it via the function
-    // invocation. Then 'arguments' denotes that specific parameter value
-    // and cannot be used to access the parameters, which is why we don't
-    // need to rewrite in that case.
-    //
-    // (*) Instead of having a parameter called 'arguments', we may have an
-    // assignment to 'arguments' in the function body, at some arbitrary
-    // point in time (possibly through an 'eval()' call!). After that
-    // assignment any re-write of parameters would be invalid (was bug
-    // 881452). Thus, we introduce a shadow '.arguments'
-    // variable which also points to the arguments object. For rewrites we
-    // use '.arguments' which remains valid even if we assign to
-    // 'arguments'. To summarize: If we need to rewrite, we allocate an
-    // 'arguments' object dynamically upon function invocation. The compiler
-    // introduces 2 local variables 'arguments' and '.arguments', both of
-    // which originally point to the arguments object that was
-    // allocated. All parameters are rewritten into property accesses via
-    // the '.arguments' variable. Thus, any changes to properties of
-    // 'arguments' are reflected in the variables and vice versa. If the
-    // 'arguments' variable is changed, '.arguments' still points to the
-    // correct arguments object and the rewrites still work.
+    // 'arguments', we must be conservative and allocate all parameters to
+    // the context assuming they will be captured by the arguments object.
+    // If we have a parameter named 'arguments', a (new) value is always
+    // assigned to it via the function invocation. Then 'arguments' denotes
+    // that specific parameter value and cannot be used to access the
+    // parameters, which is why we don't need to allocate an arguments
+    // object in that case.
 
     // We are using 'arguments'. Tell the code generator that is needs to
     // allocate the arguments object by setting 'arguments_'.
@@ -990,75 +930,31 @@ void Scope::AllocateParameterLocals() {
     // In strict mode 'arguments' does not alias formal parameters.
     // Therefore in strict mode we allocate parameters as if 'arguments'
     // were not used.
-    rewrite_parameters = !is_strict_mode();
+    uses_nonstrict_arguments = !is_strict_mode();
   }
 
-  if (rewrite_parameters) {
-    // We also need the '.arguments' shadow variable. Declare it and create
-    // and bind the corresponding proxy. It's ok to declare it only now
-    // because it's a local variable that is allocated after the parameters
-    // have been allocated.
-    //
-    // Note: This is "almost" at temporary variable but we cannot use
-    // NewTemporary() because the mode needs to be INTERNAL since this
-    // variable may be allocated in the heap-allocated context (temporaries
-    // are never allocated in the context).
-    arguments_shadow_ = new Variable(this,
-                                     FACTORY->arguments_shadow_symbol(),
-                                     Variable::INTERNAL,
-                                     true,
-                                     Variable::ARGUMENTS);
-    arguments_shadow_->set_is_used(true);
-    temps_.Add(arguments_shadow_);
-
-    // Allocate the parameters by rewriting them into '.arguments[i]' accesses.
-    for (int i = 0; i < params_.length(); i++) {
-      Variable* var = params_[i];
-      ASSERT(var->scope() == this);
-      if (MustAllocate(var)) {
-        if (MustAllocateInContext(var)) {
-          // It is ok to set this only now, because arguments is a local
-          // variable that is allocated after the parameters have been
-          // allocated.
-          arguments_shadow_->MarkAsAccessedFromInnerScope();
-        }
-        Property* rewrite =
-            new Property(new VariableProxy(arguments_shadow_),
-                         new Literal(Handle<Object>(Smi::FromInt(i))),
-                         RelocInfo::kNoPosition,
-                         Property::SYNTHETIC);
-        rewrite->set_is_arguments_access(true);
-        var->set_rewrite(rewrite);
-      }
+  // The same parameter may occur multiple times in the parameters_ list.
+  // If it does, and if it is not copied into the context object, it must
+  // receive the highest parameter index for that parameter; thus iteration
+  // order is relevant!
+  for (int i = params_.length() - 1; i >= 0; --i) {
+    Variable* var = params_[i];
+    ASSERT(var->scope() == this);
+    if (uses_nonstrict_arguments) {
+      // Give the parameter a use from an inner scope, to force allocation
+      // to the context.
+      var->MarkAsAccessedFromInnerScope();
     }
 
-  } else {
-    // The arguments object is not used, so we can access parameters directly.
-    // The same parameter may occur multiple times in the parameters_ list.
-    // If it does, and if it is not copied into the context object, it must
-    // receive the highest parameter index for that parameter; thus iteration
-    // order is relevant!
-    for (int i = 0; i < params_.length(); i++) {
-      Variable* var = params_[i];
-      ASSERT(var->scope() == this);
-      if (MustAllocate(var)) {
-        if (MustAllocateInContext(var)) {
-          ASSERT(var->rewrite() == NULL ||
-                 (var->AsSlot() != NULL &&
-                  var->AsSlot()->type() == Slot::CONTEXT));
-          if (var->rewrite() == NULL) {
-            // Only set the heap allocation if the parameter has not
-            // been allocated yet.
-            AllocateHeapSlot(var);
-          }
-        } else {
-          ASSERT(var->rewrite() == NULL ||
-                 (var->AsSlot() != NULL &&
-                  var->AsSlot()->type() == Slot::PARAMETER));
-          // Set the parameter index always, even if the parameter
-          // was seen before! (We need to access the actual parameter
-          // supplied for the last occurrence of a multiply declared
-          // parameter.)
+    if (MustAllocate(var)) {
+      if (MustAllocateInContext(var)) {
+        ASSERT(var->rewrite() == NULL || var->IsContextSlot());
+        if (var->rewrite() == NULL) {
+          AllocateHeapSlot(var);
+        }
+      } else {
+        ASSERT(var->rewrite() == NULL || var->IsParameter());
+        if (var->rewrite() == NULL) {
           var->set_rewrite(new Slot(var, Slot::PARAMETER, i));
         }
       }
@@ -1070,8 +966,9 @@ void Scope::AllocateParameterLocals() {
 void Scope::AllocateNonParameterLocal(Variable* var) {
   ASSERT(var->scope() == this);
   ASSERT(var->rewrite() == NULL ||
-         (!var->IsVariable(FACTORY->result_symbol())) ||
-         (var->AsSlot() == NULL || var->AsSlot()->type() != Slot::LOCAL));
+         !var->IsVariable(FACTORY->result_symbol()) ||
+         var->AsSlot() == NULL ||
+         var->AsSlot()->type() != Slot::LOCAL);
   if (var->rewrite() == NULL && MustAllocate(var)) {
     if (MustAllocateInContext(var)) {
       AllocateHeapSlot(var);