[strong] Stricter check for referring to other classes inside methods.

src/scopes.cc

 // 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.
 
 #include "src/v8.h"
 
 #include "src/accessors.h"
 #include "src/bootstrapper.h"
 #include "src/messages.h"
 #include "src/parser.h"
@@ skipping 14 matching lines @@
 
 VariableMap::VariableMap(Zone* zone)
     : ZoneHashMap(ZoneHashMap::PointersMatch, 8, ZoneAllocationPolicy(zone)),
       zone_(zone) {}
 VariableMap::~VariableMap() {}
 
 
 Variable* VariableMap::Declare(Scope* scope, const AstRawString* name,
                                VariableMode mode, Variable::Kind kind,
                                InitializationFlag initialization_flag,
-                               MaybeAssignedFlag maybe_assigned_flag) {
+                               MaybeAssignedFlag maybe_assigned_flag,
+                               int declaration_group_start) {
   // AstRawStrings are unambiguous, i.e., the same string is always represented
   // by the same AstRawString*.
   // FIXME(marja): fix the type of Lookup.
   Entry* p =
       ZoneHashMap::LookupOrInsert(const_cast<AstRawString*>(name), name->hash(),
                                   ZoneAllocationPolicy(zone()));
   if (p->value == NULL) {
     // The variable has not been declared yet -> insert it.
     DCHECK(p->key == name);
-    p->value = new (zone()) Variable(scope, name, mode, kind,
-                                     initialization_flag, maybe_assigned_flag);
+    if (kind == Variable::CLASS) {
+      p->value = new (zone())
+          ClassVariable(scope, name, mode, kind, initialization_flag,
+                        maybe_assigned_flag, declaration_group_start);
+    } else {
+      p->value = new (zone()) Variable(
+          scope, name, mode, kind, initialization_flag, maybe_assigned_flag);
+    }
   }
   return reinterpret_cast<Variable*>(p->value);
 }
 
 
 Variable* VariableMap::Lookup(const AstRawString* name) {
   Entry* p = ZoneHashMap::Lookup(const_cast<AstRawString*>(name), name->hash());
   if (p != NULL) {
     DCHECK(reinterpret_cast<const AstRawString*>(p->key) == name);
     DCHECK(p->value != NULL);
@@ skipping 12 matching lines @@
       variables_(zone),
       internals_(4, zone),
       temps_(4, zone),
       params_(4, zone),
       unresolved_(16, zone),
       decls_(4, zone),
       module_descriptor_(
           scope_type == MODULE_SCOPE ? ModuleDescriptor::New(zone) : NULL),
       already_resolved_(false),
       ast_value_factory_(ast_value_factory),
-      zone_(zone) {
+      zone_(zone),
+      class_declaration_group_start_(-1) {
   SetDefaults(scope_type, outer_scope, Handle<ScopeInfo>::null(),
               function_kind);
   // The outermost scope must be a script scope.
   DCHECK(scope_type == SCRIPT_SCOPE || outer_scope != NULL);
   DCHECK(!HasIllegalRedeclaration());
 }
 
 
 Scope::Scope(Zone* zone, Scope* inner_scope, ScopeType scope_type,
              Handle<ScopeInfo> scope_info, AstValueFactory* value_factory)
     : inner_scopes_(4, zone),
       variables_(zone),
       internals_(4, zone),
       temps_(4, zone),
       params_(4, zone),
       unresolved_(16, zone),
       decls_(4, zone),
       module_descriptor_(NULL),
       already_resolved_(true),
       ast_value_factory_(value_factory),
-      zone_(zone) {
+      zone_(zone),
+      class_declaration_group_start_(-1) {
   SetDefaults(scope_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_,
                         static_cast<int>(Context::MIN_CONTEXT_SLOTS));
   AddInnerScope(inner_scope);
 }
 
 
 Scope::Scope(Zone* zone, Scope* inner_scope,
              const AstRawString* catch_variable_name,
              AstValueFactory* value_factory)
     : inner_scopes_(1, zone),
       variables_(zone),
       internals_(0, zone),
       temps_(0, zone),
       params_(0, zone),
       unresolved_(0, zone),
       decls_(0, zone),
       module_descriptor_(NULL),
       already_resolved_(true),
       ast_value_factory_(value_factory),
-      zone_(zone) {
+      zone_(zone),
+      class_declaration_group_start_(-1) {
   SetDefaults(CATCH_SCOPE, NULL, Handle<ScopeInfo>::null());
   AddInnerScope(inner_scope);
   ++num_var_or_const_;
   num_heap_slots_ = Context::MIN_CONTEXT_SLOTS;
   Variable* variable = variables_.Declare(this,
                                           catch_variable_name,
                                           VAR,
                                           Variable::NORMAL,
                                           kCreatedInitialized);
   AllocateHeapSlot(variable);
@@ skipping 267 matching lines @@
 
     mode = DYNAMIC;
     location = Variable::LOOKUP;
     init_flag = kCreatedInitialized;
     // Be conservative and flag parameters as maybe assigned. Better information
     // would require ScopeInfo to serialize the maybe_assigned bit also for
     // parameters.
     maybe_assigned_flag = kMaybeAssigned;
   }
 
+  // TODO(marja, rossberg): Declare variables of the right Kind.
   Variable* var = variables_.Declare(this, name, mode, Variable::NORMAL,
                                      init_flag, maybe_assigned_flag);
   var->AllocateTo(location, index);
   return var;
 }
 
 
 Variable* Scope::LookupFunctionVar(const AstRawString* name,
                                    AstNodeFactory* factory) {
   if (function_ != NULL && function_->proxy()->raw_name() == name) {
@@ skipping 41 matching lines @@
   }
   // TODO(wingo): Avoid O(n^2) check.
   *is_duplicate = IsDeclaredParameter(name);
   params_.Add(var, zone());
   return var;
 }
 
 
 Variable* Scope::DeclareLocal(const AstRawString* name, VariableMode mode,
                               InitializationFlag init_flag, Variable::Kind kind,
-                              MaybeAssignedFlag maybe_assigned_flag) {
+                              MaybeAssignedFlag maybe_assigned_flag,
+                              int declaration_group_start) {
   DCHECK(!already_resolved());
   // This function handles VAR, LET, and CONST modes.  DYNAMIC variables are
   // introduces during variable allocation, INTERNAL variables are allocated
   // explicitly, and TEMPORARY variables are allocated via NewTemporary().
   DCHECK(IsDeclaredVariableMode(mode));
   ++num_var_or_const_;
   return variables_.Declare(this, name, mode, kind, init_flag,
-                            maybe_assigned_flag);
+                            maybe_assigned_flag, declaration_group_start);
 }
 
 
 Variable* Scope::DeclareDynamicGlobal(const AstRawString* name) {
   DCHECK(is_script_scope());
   return variables_.Declare(this,
                             name,
                             DYNAMIC_GLOBAL,
                             Variable::NORMAL,
                             kCreatedInitialized);
@@ skipping 632 matching lines @@
 
   return true;
 }
 
 
 bool Scope::CheckStrongModeDeclaration(VariableProxy* proxy, Variable* var) {
   // Check for declaration-after use (for variables) in strong mode. Note that
   // we can only do this in the case where we have seen the declaration. And we
   // always allow referencing functions (for now).
 
+  // This might happen during lazy compilation; we don't keep track of
+  // initializer positions for variables stored in ScopeInfo, so we cannot check
+  // bindings against them. TODO(marja, rossberg): remove this hack.
+  if (var->initializer_position() == RelocInfo::kNoPosition) return true;
+
   // Allow referencing the class name from methods of that class, even though
   // the initializer position for class names is only after the body.
   Scope* scope = this;
   while (scope) {
     if (scope->ClassVariableForMethod() == var) return true;
     scope = scope->outer_scope();
   }
 
   // Allow references from methods to classes declared later, if we detect no
-  // problematic dependency cycles.
+  // problematic dependency cycles. Note that we can be inside multiple methods
+  // at the same time, and it's enough if we find one where the reference is
+  // allowed.
+  if (var->is_class() &&
+      var->AsClassVariable()->declaration_group_start() >= 0) {
+    for (scope = this; scope && scope != var->scope();
+         scope = scope->outer_scope()) {
+      ClassVariable* class_var = scope->ClassVariableForMethod();
+      if (class_var) {
+        // A method is referring to some other class, possibly declared
+        // later. Referring to a class declared earlier is always OK and covered
+        // by the code outside this if. Here we only need to allow special cases
+        // for referring to a class which is declared later.
 
-  if (ClassVariableForMethod() && var->is_class()) {
-    // A method is referring to some other class, possibly declared
-    // later. Referring to a class declared earlier is always OK and covered by
-    // the code outside this if. Here we only need to allow special cases for
-    // referring to a class which is declared later.
+        // Referring to a class C declared later is OK under the following
+        // circumstances:
 
-    // Referring to a class C declared later is OK under the following
-    // circumstances:
+        // 1. The class declarations are in a consecutive group with no other
+        // declarations or statements in between, and
 
-    // 1. The class declarations are in a consecutive group with no other
-    // declarations or statements in between, and
+        // 2. There is no dependency cycle where the first edge is an
+        // initialization time dependency (computed property name or extends
+        // clause) from C to something that depends on this class directly or
+        // transitively.
 
-    // 2. There is no dependency cycle where the first edge is an initialization
-    // time dependency (computed property name or extends clause) from C to
-    // something that depends on this class directly or transitively.
+        // This is needed because a class ("class Name { }") creates two
+        // bindings (one in the outer scope, and one in the class scope). The
+        // method is a function scope inside the inner scope (class scope). The
+        // consecutive class declarations are in the outer scope.
+        if (class_var->corresponding_outer_class_variable()) {
+          class_var = class_var->corresponding_outer_class_variable();

[rossberg, 2015/04/23 13:17:20]

Nit: you could switch the two lines and avoid the duplication:

class_var = class_var>corresponding_outer_class_variable();
if (class_var) {

[marja, 2015/04/23 13:40:23]

Done.

+          if (class_var->declaration_group_start() ==
+              var->AsClassVariable()->declaration_group_start()) {
+            return true;
+          }
+        }
 
-    // TODO(marja,rossberg): implement these checks. Here we undershoot the
-    // target and allow referring to any class.
-    return true;
+        // TODO(marja,rossberg): implement the dependency cycle detection. Here
+        // we undershoot the target and allow referring to any class in the same
+        // consectuive declaration group.
+
+        // The cycle detection can work roughly like this: 1) detect init-time
+        // references here (they are free variables which are inside the class
+        // scope but not inside a method scope - no parser changes needed to
+        // detect them) 2) if we encounter an init-time reference here, allow
+        // it, but record it for a later dependency cycle check 3) also record
+        // non-init-time references here 4) after scope analysis is done,
+        // analyse the dependency cycles: an illegal cycle is one starting with
+        // an init-time reference and leading back to the starting point with
+        // either non-init-time and init-time references.
+      }
+    }
   }
 
   // If both the use and the declaration are inside an eval scope (possibly
   // indirectly), or one of them is, we need to check whether they are inside
   // the same eval scope or different ones.
 
   // TODO(marja,rossberg): Detect errors across different evals (depends on the
   // future of eval in strong mode).
   const Scope* eval_for_use = NearestOuterEvalScope();
   const Scope* eval_for_declaration = var->scope()->NearestOuterEvalScope();
 
   if (proxy->position() != RelocInfo::kNoPosition &&
       proxy->position() < var->initializer_position() && !var->is_function() &&
       eval_for_use == eval_for_declaration) {
     DCHECK(proxy->end_position() != RelocInfo::kNoPosition);
     ReportMessage(proxy->position(), proxy->end_position(),
                   "strong_use_before_declaration", proxy->raw_name());
     return false;
   }
   return true;
 }
 
 
-Variable* Scope::ClassVariableForMethod() const {
+ClassVariable* Scope::ClassVariableForMethod() const {
+  // TODO(marja, rossberg): This fails to find a class variable in the following
+  // cases:
+  // let A = class { ... }
+  // It needs to be investigated whether this causes any practical problems.
   if (!is_function_scope()) return nullptr;
   if (IsInObjectLiteral(function_kind_)) return nullptr;
   if (!IsConciseMethod(function_kind_) && !IsConstructor(function_kind_) &&
       !IsAccessorFunction(function_kind_)) {
     return nullptr;
   }
   DCHECK_NOT_NULL(outer_scope_);
   DCHECK(outer_scope_->is_class_scope());
   // The class scope contains at most one variable, the class name.
   DCHECK(outer_scope_->variables_.occupancy() <= 1);
   if (outer_scope_->variables_.occupancy() == 0) return nullptr;
   VariableMap::Entry* p = outer_scope_->variables_.Start();
-  return reinterpret_cast<Variable*>(p->value);
+  Variable* var = reinterpret_cast<Variable*>(p->value);
+  if (!var->is_class()) return nullptr;
+  return var->AsClassVariable();
 }
 
 
 bool Scope::ResolveVariablesRecursively(ParseInfo* info,
                                         AstNodeFactory* factory) {
   DCHECK(info->script_scope()->is_script_scope());
 
   // Resolve unresolved variables for this scope.
   for (int i = 0; i < unresolved_.length(); i++) {
     if (!ResolveVariable(info, unresolved_[i], factory)) return false;
@@ skipping 278 matching lines @@
       (function_ != NULL && function_->proxy()->var()->IsStackLocal() ? 1 : 0);
 }
 
 
 int Scope::ContextLocalCount() const {
   if (num_heap_slots() == 0) return 0;
   return num_heap_slots() - Context::MIN_CONTEXT_SLOTS -
       (function_ != NULL && function_->proxy()->var()->IsContextSlot() ? 1 : 0);
 }
 } }  // namespace v8::internal