Implement proper scoping for "this" in arrow functions

src/scopes.h

 // Copyright 2012 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.
 
 #ifndef V8_SCOPES_H_
 #define V8_SCOPES_H_
 
 #include "src/ast.h"
 #include "src/zone.h"
 
@@ skipping 127 matching lines @@
   // Declare an implicit global variable in this scope which must be a
   // script scope.  The variable was introduced (possibly from an inner
   // scope) by a reference to an unresolved variable with no intervening
   // with statements or eval calls.
   Variable* DeclareDynamicGlobal(const AstRawString* name);
 
   // Create a new unresolved variable.
   VariableProxy* NewUnresolved(AstNodeFactory* factory,
                                const AstRawString* name,
                                Interface* interface = Interface::NewValue(),
-                               int position = RelocInfo::kNoPosition) {
+                               int position = RelocInfo::kNoPosition,
+                               bool is_this = false) {

[wingo, 2015/02/04 10:02:13]

Boolean parameters are an antipattern, especially optional boolean parameters,
because they read so poorly.  Please create a small enum somewhere.  I know that
is_this is used in other parts of the API but it's bad there too.

[aperez, 2015/02/04 21:00:42]

Acknowledged. Even better: the actual name of the variable is always passed in,
so why not checking that “name == ast_value_factory->this_string()”?  I am gonna
go with that, and it will save one argument being passed around.

     // Note that we must not share the unresolved variables with
     // the same name because they may be removed selectively via
     // RemoveUnresolved().
     DCHECK(!already_resolved());
     VariableProxy* proxy =
-        factory->NewVariableProxy(name, false, interface, position);
+        factory->NewVariableProxy(name, is_this, interface, position);
     unresolved_.Add(proxy, zone_);
     return proxy;
   }
 
   // Remove a unresolved variable. During parsing, an unresolved variable
   // may have been added optimistically, but then only the variable name
   // was used (typically for labels). If the variable was not declared, the
   // addition introduced a new unresolved variable which may end up being
   // allocated globally as a "ghost" variable. RemoveUnresolved removes
   // such a variable again if it was added; otherwise this is a no-op.
@@ skipping 50 matching lines @@
   void RecordArgumentsUsage() { scope_uses_arguments_ = true; }
 
   // Inform the scope that the corresponding code uses "super".
   void RecordSuperPropertyUsage() { scope_uses_super_property_ = true; }
 
   // Inform the scope that the corresponding code invokes "super" constructor.
   void RecordSuperConstructorCallUsage() {
     scope_uses_super_constructor_call_ = true;
   }
 
-  // Inform the scope that the corresponding code uses "this".
-  void RecordThisUsage() { scope_uses_this_ = true; }
-
   // Set the strict mode flag (unless disabled by a global flag).
   void SetStrictMode(StrictMode strict_mode) { strict_mode_ = strict_mode; }
 
   // Set the ASM module flag.
   void SetAsmModule() { asm_module_ = true; }
 
   // Position in the source where this scope begins and ends.
   //
   // * For the scope of a with statement
   //     with (obj) stmt
@@ skipping 83 matching lines @@
   }
   // Does this scope calls "super" constructor.
   bool uses_super_constructor_call() const {
     return scope_uses_super_constructor_call_;
   }
   // Does  any inner scope calls "super" constructor.
   bool inner_uses_super_constructor_call() const {
     return inner_scope_uses_super_constructor_call_;
   }
   // Does this scope access "this".
-  bool uses_this() const { return scope_uses_this_; }
+  bool uses_this() const { return receiver_->is_used(); }
   // Does any inner scope access "this".
-  bool inner_uses_this() const { return inner_scope_uses_this_; }
 
   // ---------------------------------------------------------------------------
   // Accessors.
 
   // The type of this scope.
   ScopeType scope_type() const { return scope_type_; }
 
   // The language mode of this scope.
   StrictMode strict_mode() const { return strict_mode_; }
 
   // The variable corresponding the 'this' value.
-  Variable* receiver() { return receiver_; }
+  Variable* receiver() {
+    DCHECK(has_this_declaration());

[wingo, 2015/02/04 10:02:13]

DCHECK(receiver_) also perhaps?

[aperez, 2015/02/04 21:00:42]

Acknowledged.

+    return receiver_;
+  }
+
+  bool has_this_declaration() const {
+    return !is_arrow_scope() && is_declaration_scope();
+  }
 
   // The variable holding the function literal for named function
   // literals, or NULL.  Only valid for function scopes.
   VariableDeclaration* function() const {
     DCHECK(is_function_scope());
     return function_;
   }
 
   // Parameters. The left-most parameter has index 0.
   // Only valid for function scopes.
@@ skipping 153 matching lines @@
   bool scope_contains_with_;
   // This scope or a nested catch scope or with scope contain an 'eval' call. At
   // the 'eval' call site this scope is the declaration scope.
   bool scope_calls_eval_;
   // This scope uses "arguments".
   bool scope_uses_arguments_;
   // This scope uses "super" property ('super.foo').
   bool scope_uses_super_property_;
   // This scope uses "super" constructor ('super(..)').
   bool scope_uses_super_constructor_call_;
-  // This scope uses "this".
-  bool scope_uses_this_;
   // This scope contains an "use asm" annotation.
   bool asm_module_;
   // This scope's outer context is an asm module.
   bool asm_function_;
   // The strict mode of this scope.
   StrictMode strict_mode_;
   // Source positions.
   int start_position_;
   int end_position_;
 
   // Computed via PropagateScopeInfo.
   bool outer_scope_calls_sloppy_eval_;
   bool inner_scope_calls_eval_;
   bool inner_scope_uses_arguments_;
   bool inner_scope_uses_super_property_;
   bool inner_scope_uses_super_constructor_call_;
-  bool inner_scope_uses_this_;
   bool force_eager_compilation_;
   bool force_context_allocation_;
 
   // True if it doesn't need scope resolution (e.g., if the scope was
   // constructed based on a serialized scope info or a catch context).
   bool already_resolved_;
 
   // Computed as variables are declared.
   int num_var_or_const_;
 
@@ skipping 79 matching lines @@
 
   // Predicates.
   bool MustAllocate(Variable* var);
   bool MustAllocateInContext(Variable* var);
   bool HasArgumentsParameter();
 
   // Variable allocation.
   void AllocateStackSlot(Variable* var);
   void AllocateHeapSlot(Variable* var);
   void AllocateParameterLocals();
+  void AllocateParameter(Variable* var, int index);
+  void AllocateReceiver();
   void AllocateNonParameterLocal(Variable* var);
   void AllocateNonParameterLocals();
   void AllocateVariablesRecursively();
   void AllocateModulesRecursively(Scope* host_scope);
 
   // Resolve and fill in the allocation information for all variables
   // in this scopes. Must be called *after* all scopes have been
   // processed (parsed) to ensure that unresolved variables can be
   // resolved properly.
   //
@@ skipping 24 matching lines @@
                    Scope* outer_scope,
                    Handle<ScopeInfo> scope_info);
 
   AstValueFactory* ast_value_factory_;
   Zone* zone_;
 };
 
 } }  // namespace v8::internal
 
 #endif  // V8_SCOPES_H_