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

test/mjsunit/strong/mutually-recursive-classes.js

Index: test/mjsunit/strong/mutually-recursive-classes.js
diff --git a/test/mjsunit/strong/mutually-recursive-classes.js b/test/mjsunit/strong/mutually-recursive-classes.js
index c8d6fad260fd9fad4cfc8bc7d2ca1868df8143e3..3755cbaf034a7099bfa88b6544aa6b3052739229 100644
--- a/test/mjsunit/strong/mutually-recursive-classes.js
+++ b/test/mjsunit/strong/mutually-recursive-classes.js
@@ -2,23 +2,20 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-// Flags: --strong-mode
+// Flags: --strong-mode --harmony-arrow-functions
+"use strict"
 
-// Note that it's essential for these tests that the reference is inside dead
-// code (because we already produce ReferenceErrors for run-time unresolved
-// variables and don't want to confuse those with strong mode errors). But the
-// errors should *not* be inside lazy, unexecuted functions, since lazy parsing
-// doesn't produce strong mode scoping errors).
+let prologue_dead = "(function outer() { if (false) { ";
+let epilogue_dead = " } })();";
 
-// In addition, assertThrows will call eval and that changes variable binding
-// types (see e.g., UNBOUND_EVAL_SHADOWED). We can avoid unwanted side effects
-// by wrapping the code to be tested inside an outer function.
-function assertThrowsHelper(code) {
-  "use strict";
-  let prologue_dead = "(function outer() { if (false) { ";
-  let epilogue_dead = " } })();";
+let prologue_live = "(function outer() { ";
+let epilogue_live = "})();";
 
-  assertThrows("'use strong'; " + prologue_dead + code + epilogue_dead, ReferenceError);
+// For code which already throws a run-time error in non-strong mode; we assert
+// that we now get the error already compilation time.
+function assertThrowsEarlyErrors(code) {

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

How about assertLateErrorsBecomeEarly?

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

Done.

+  assertThrows("'use strong'; " + prologue_dead + code + epilogue_dead,
+               ReferenceError);
 
   // Make sure the error happens only in strong mode (note that we need strict
   // mode here because of let).
@@ -26,24 +23,35 @@ function assertThrowsHelper(code) {
 
   // But if we don't put the references inside a dead code, it throws a run-time
   // error (also in strict mode).
-  let prologue_live = "(function outer() { ";
-  let epilogue_live = "})();";
+  assertThrows("'use strong'; " + prologue_live + code + epilogue_live,
+               ReferenceError);
+  assertThrows("'use strict'; " + prologue_live + code + epilogue_live,
+               ReferenceError);
+}
+
+// For code which doesn't throw an error at all in non-strong mode.
+function assertThrowsAddedErrors(code) {

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

assertNonErrorsBecomeEarly?

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

Done.

+  assertThrows("'use strong'; " + prologue_dead + code + epilogue_dead,
+               ReferenceError);
+  assertDoesNotThrow("'use strict'; " + prologue_dead + code + epilogue_dead);
 
-  assertThrows("'use strong'; " + prologue_live + code + epilogue_live, ReferenceError);
-  assertThrows("'use strict'; " + prologue_live + code + epilogue_live, ReferenceError);
+  assertThrows("'use strong'; " + prologue_live + code + epilogue_live,
+               ReferenceError);
+  assertDoesNotThrow("'use strict'; " + prologue_live + code + epilogue_live,
+                     ReferenceError);
 }
 
 (function InitTimeReferenceForward() {
   // It's never OK to have an init time reference to a class which hasn't been
   // declared.
-  assertThrowsHelper(
-      `class A extends B { };
+  assertThrowsEarlyErrors(
+      `class A extends B { }
       class B {}`);
 
-  assertThrowsHelper(
+  assertThrowsEarlyErrors(
       `class A {
         [B.sm()]() { }
-      };
+      }
       class B {
         static sm() { return 0; }
       }`);
@@ -54,7 +62,7 @@ function assertThrowsHelper(code) {
   "use strong";
   class A {
     static sm() { return 0; }
-  };
+  }
   let i = "making these classes non-consecutive";
   class B extends A {};
   "by inserting statements and declarations in between";
@@ -68,10 +76,109 @@ function assertThrowsHelper(code) {
   class A {
     m() { B; }
     static sm() { B; }
-  };
+  }
   // No statements or declarations between the classes.
   class B {
     m() { A; }
     static sm() { A; }
-  };
+  }
+})();
+
+(function MutualRecursionWithMoreClasses() {
+  "use strong";
+  class A {
+    m() { B; C; }
+    static sm() { B; C; }
+  }
+  class B {
+    m() { A; C; }
+    static sm() { A; C; }
+  }
+  class C {
+    m() { A; B; }
+    static sm() { A; B; }
+  }
+})();
+
+(function ReferringForwardInDeeperScopes() {
+  "use strong";
+
+  function foo() {
+    class A1 {
+      m() { B1; }
+    }
+    class B1 { }
+  }
+
+  class Outer {
+    m() {
+      class A2 {
+        m() { B2; }
+      }
+      class B2 { }
+    }
+  }
+
+  for (let i = 0; i < 1; ++i) {
+    class A3 {
+      m() { B3; }
+    }
+    class B3 { }
+  }
+
+  (a, b) => {
+    class A4 {
+      m() { B4; }
+    }
+    class B4 { }
+  }
+})();
+
+(function ReferringForwardButClassesNotConsecutive() {
+  assertThrowsAddedErrors(
+      `class A {
+        m() { B; }
+      }
+      ;
+      class B {}`);
+
+  assertThrowsAddedErrors(
+      `class A {
+        m() { B1; } // Just a normal use-before-declaration.
+      }
+      let B1 = class B2 {}`);
+
+  // Note that this is also an error:

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

I think the comment only makes sense if A refers to B1. Otherwise it's just a
plain scoping error unrelated to class recursion -- obviously, you cannot access
local variables outside their scope, forwards or backwards.

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

Yes, the comment "B2 will be unbound" tried to explain that. But maybe having it
here is more confusing than clarifying?

[rossberg, 2015/04/23 12:20:53]

Yeah, I would just remove it. It certainly confused me, although that might not
mean much. :)

+  // if (false) {
+  //   class A {
+  //     m() { B2; }
+  //   }
+  //   // Class declared inside a let statement -> not a consecutive batch.
+  //   let B1 = class B2 {}
+  // }
+  // But we cannot test it here, because B2 will be unbound (potentially bound
+  // to the global object), and not subject to static strong mode scoping
+  // checks.
+
+  assertThrowsAddedErrors(
+      `class A {
+        m() { B; }
+      }
+      let i = 0;
+      class B {}`);
+
+  assertThrowsAddedErrors(
+      `class A {
+        m() { B; }
+      }
+      function foo() {}
+      class B {}`);
+
+  assertThrowsAddedErrors(
+      `function foo() {
+        class A {
+          m() { B; }
+        }
+      }
+      class B {}`);
 })();