Resolve references to "this" the same way as normal variables

src/compiler/ast-graph-builder.cc

 // Copyright 2014 the V8 project authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "src/compiler/ast-graph-builder.h"
 
 #include "src/compiler.h"
 #include "src/compiler/ast-loop-assignment-analyzer.h"
 #include "src/compiler/control-builders.h"
 #include "src/compiler/js-type-feedback.h"
@@ skipping 450 matching lines @@
 
   // Initialize the incoming context.
   CreateFunctionContext(constant_context);
   ContextScope incoming(this, scope, function_context_.get());
 
   // Initialize control scope.
   ControlScope control(this);
 
   // Build receiver check for sloppy mode if necessary.
   // TODO(mstarzinger/verwaest): Should this be moved back into the CallIC?
-  Node* original_receiver = env.Lookup(scope->receiver());
-  Node* patched_receiver = BuildPatchReceiverToGlobalProxy(original_receiver);
-  env.Bind(scope->receiver(), patched_receiver);
+  if (scope->has_this_declaration()) {
+    Node* original_receiver = env.Lookup(scope->receiver());
+    Node* patched_receiver = BuildPatchReceiverToGlobalProxy(original_receiver);
+    env.Bind(scope->receiver(), patched_receiver);
+  }
 
   // Build function context only if there are context allocated variables.
   int heap_slots = info()->num_heap_slots() - Context::MIN_CONTEXT_SLOTS;
   if (heap_slots > 0) {
     // Push a new inner context scope for the function.
     Node* inner_context = BuildLocalFunctionContext(function_context_.get());
     ContextScope top_context(this, scope, inner_context);
     CreateGraphBody(stack_check);
   } else {
     // Simply use the outer function context in building the graph.
@@ skipping 148 matching lines @@
     // Split the liveness blocks.
     copy->liveness_block_ =
         builder_->liveness_analyzer()->NewBlock(copy->liveness_block());
     liveness_block_ =
         builder_->liveness_analyzer()->NewBlock(copy->liveness_block());
   }
 }
 
 
 void AstGraphBuilder::Environment::Bind(Variable* variable, Node* node) {
+  // "this" may be context-allocated, but it's possible for us to look it up and
+  // patch it on the stack before it has been hoisted to the context.
+  if (variable->is_this()) {

[Michael Starzinger, 2015/04/21 18:13:44]

AFAICT, this is only needed for patching the receiver to the global proxy in the
method prologue. Please don't "extend" Environment::Bind this way, but rather
adapt BuildPatchReceiverToGlobalProxy accordingly.

[wingo, 2015/04/22 08:04:00]

I had tried to do that, but I couldn't get at environment->values()->at(0), as
it's private.  I realize this is an awful hack and something different should go
in :)  I guess common()->Parameter(0) is the thing to use, then?

[Michael Starzinger, 2015/04/22 10:59:21]

Yes, I think for patching purposes Parameter(0) is always correct.

+    values()->at(0) = node;
+    return;
+  }
+
   DCHECK(variable->IsStackAllocated());
   if (variable->IsParameter()) {
     // The parameter indices are shifted by 1 (receiver is parameter
     // index -1 but environment index 0).
     values()->at(variable->index() + 1) = node;
   } else {
     DCHECK(variable->IsStackLocal());
     values()->at(variable->index() + parameters_count_) = node;
     if (FLAG_analyze_environment_liveness) {
       liveness_block()->Bind(variable->index());
     }
   }
 }
 
 
 Node* AstGraphBuilder::Environment::Lookup(Variable* variable) {
+  // "this" may be context-allocated, but it's possible for us to look it up and
+  // patch it on the stack before it has been hoisted to the context.
+  if (variable->is_this()) {

[Michael Starzinger, 2015/04/21 18:13:44]

Likewise.

[wingo, 2015/04/22 08:04:00]

Acknowledged.

+    return values()->at(0);
+  }
+
   DCHECK(variable->IsStackAllocated());
   if (variable->IsParameter()) {
     // The parameter indices are shifted by 1 (receiver is parameter
     // index -1 but environment index 0).
     return values()->at(variable->index() + 1);
   } else {
     DCHECK(variable->IsStackLocal());
     if (FLAG_analyze_environment_liveness) {
       liveness_block()->Lookup(variable->index());
     }
@@ skipping 1473 matching lines @@
   if (possibly_eval && args->length() > 0) {
     int arg_count = args->length();
 
     // Extract callee and source string from the environment.
     Node* callee = environment()->Peek(arg_count + 1);
     Node* source = environment()->Peek(arg_count - 1);
 
     // Create node to ask for help resolving potential eval call. This will
     // provide a fully resolved callee and the corresponding receiver.
     Node* function = GetFunctionClosure();
-    Node* receiver = environment()->Lookup(info()->scope()->receiver());
+    Node* receiver = environment()->Lookup(info()->scope()->LookupThis());
     Node* language = jsgraph()->Constant(language_mode());
     Node* position = jsgraph()->Constant(info()->scope()->start_position());
     const Operator* op =
         javascript()->CallRuntime(Runtime::kResolvePossiblyDirectEval, 6);
     Node* pair =
         NewNode(op, callee, source, function, receiver, language, position);
     PrepareFrameState(pair, expr->EvalOrLookupId(),
                       OutputFrameStateCombine::PokeAt(arg_count + 1));
     Node* new_callee = NewNode(common()->Projection(0), pair);
     Node* new_receiver = NewNode(common()->Projection(1), pair);
@@ skipping 482 matching lines @@
   Node* closure = GetFunctionClosure();
 
   // Allocate a new local context.
   Node* local_context =
       info()->scope()->is_script_scope()
           ? BuildLocalScriptContext(info()->scope())
           : NewNode(javascript()->CreateFunctionContext(), closure);
 
   // Copy parameters into context if necessary.
   int num_parameters = info()->scope()->num_parameters();
-  for (int i = 0; i < num_parameters; i++) {
-    Variable* variable = info()->scope()->parameter(i);
+  int first_parameter = info()->scope()->has_this_declaration() ? -1 : 0;
+  for (int i = first_parameter; i < num_parameters; i++) {
+    Variable* variable =
+        (i == -1) ? info()->scope()->receiver() : info()->scope()->parameter(i);

[Michael Starzinger, 2015/04/21 18:13:44]

This is getting out of hand, please hoist handling of the receiver out into a
separate block (preferably placed at line 2666 above) and rather duplicate
creation of the StoreContext node below.

     if (!variable->IsContextSlot()) continue;
     // Temporary parameter node. The parameter indices are shifted by 1
     // (receiver is parameter index -1 but environment index 0).
     Node* parameter = NewNode(common()->Parameter(i + 1), graph()->start());

[Michael Starzinger, 2015/04/21 18:13:44]

If the receiver parameter has been patched, then using Patameter(0) he is
incorrect and will use the un-patched version.

[wingo, 2015/04/22 08:04:00]

I see.  Thanks, I didn't realize that.  Will update with a new patch.

     // Context variable (at bottom of the context chain).
     DCHECK_EQ(0, info()->scope()->ContextChainLength(variable->scope()));
     const Operator* op = javascript()->StoreContext(0, variable->index());
     NewNode(op, local_context, parameter);
   }
 
   return local_context;
 }
 
 
@@ skipping 880 matching lines @@
     // Phi does not exist yet, introduce one.
     value = NewPhi(inputs, value, control);
     value->ReplaceInput(inputs - 1, other);
   }
   return value;
 }
 
 }  // namespace compiler
 }  // namespace internal
 }  // namespace v8