[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 consecutive_declaration_batch_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);
+    p->value = new (zone())
+        Variable(scope, name, mode, kind, initialization_flag,
+                 maybe_assigned_flag, consecutive_declaration_batch_start);
   }
   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 405 matching lines @@
     rest_parameter_ = var;
     rest_index_ = num_parameters();
   }
   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 consecutive_declaration_batch_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,
+                            consecutive_declaration_batch_start);
 }
 
 
 Variable* Scope::DeclareDynamicGlobal(const AstRawString* name) {
   DCHECK(is_script_scope());
   return variables_.Declare(this,
                             name,
                             DYNAMIC_GLOBAL,
                             Variable::NORMAL,
                             kCreatedInitialized);
@@ skipping 642 matching lines @@
   // 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.
-
-  if (ClassVariableForMethod() && var->is_class()) {
+  Variable* class_var = ClassVariableForMethod();
+  if (class_var && 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:
 
     // 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.
 
-    // TODO(marja,rossberg): implement these checks. Here we undershoot the
-    // target and allow referring to any class.
-    return true;
+    // This is needed because a class normally creates two scopes (outer and

[rossberg, 2015/04/20 11:15:25]

Hm, a class only introduces a single scope. Do you mean it creates two bindings?

[marja, 2015/04/20 15:58:22]

Yes, s/two scopes/two bindings/ was what I meant; fixed.

+    // inner); the reference is coming from the inner one, but classes in the
+    // same consectutive class declaration batch are declared in the outer

[rossberg, 2015/04/20 11:15:25]

Nit: "consectutive class declaration batch" -> "declaration group" (here and
elsewhere)

[marja, 2015/04/20 15:58:22]

Done.

+    // one. We also need to compare the scopes to distinguish between two
+    // consecutive classes from a class which has another class inside its
+    // computed property name. This is pretty complicated.
+    if (class_var->scope()->outer_scope()) {
+      Variable* maybe_outer_class_var =
+          class_var->scope()->outer_scope()->LookupLocal(class_var->raw_name());
+      if (maybe_outer_class_var) {

[rossberg, 2015/04/20 11:15:25]

This may be too liberal. I suppose you somehow need to verify that this var is
in fact a class variable, for the same class. Otherwise you might incorrectly
capture

  let A = class C { ... }
  let C = class D { ... }

Unfortunately, we don't distinguish that in the declaration. Maybe some trick
with positions would work? If not, a TODO is fine for this CL.

[marja, 2015/04/20 15:58:22]

Ahh, true. The breaking case is this:

let A = class B {
  m() { C; }
}
;;;;
class C {}
class B {}

(Should produce an error, doesn't.)

Fixed that and added a test.

+        class_var = maybe_outer_class_var;
+      }
+    }
+
+    // TODO(marja,rossberg): implement the dependency cycle detection. Here we
+    // undershoot the target and allow referring to any class in the same
+    // consectuive declaration batch.
+    if (class_var->scope() == var->scope() &&
+        class_var->consecutive_declaration_batch_start() ==
+            var->consecutive_declaration_batch_start() &&
+        class_var->consecutive_declaration_batch_start() >= 0) {
+      return true;
+    }
   }
 
   // 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();
@@ skipping 315 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