Don't cache Ternary.Maybe results when recursion is encountered during variance measurement (microsoft#41218)

ahejlsberg · web-flow · commit f9dcd9ef986c · 2020-10-27T10:53:13.000-07:00
* Don't record Ternary.Maybe results in cache during recursive variance measurement

* Add regression test

* Accept new baselines

* Use Ternary.Unknown to signal variance recursion

* Add comments

* Fix comment
diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts
@@ -17287,9 +17287,12 @@ namespace ts {
                 depth--;
                 if (result) {
                     if (result === Ternary.True || depth === 0) {
-                        // If result is definitely true, record all maybe keys as having succeeded
-                        for (let i = maybeStart; i < maybeCount; i++) {
-                            relation.set(maybeKeys[i], RelationComparisonResult.Succeeded | propagatingVarianceFlags);
+                        if (result === Ternary.True || result === Ternary.Maybe) {
+                            // If result is definitely true, record all maybe keys as having succeeded. Also, record Ternary.Maybe
+                            // results as having succeeded once we reach depth 0, but never record Ternary.Unknown results.
+                            for (let i = maybeStart; i < maybeCount; i++) {
+                                relation.set(maybeKeys[i], RelationComparisonResult.Succeeded | propagatingVarianceFlags);
+                            }
                         }
                         maybeCount = maybeStart;
                     }
@@ -17359,7 +17362,7 @@ namespace ts {
                     !(source.aliasTypeArgumentsContainsMarker || target.aliasTypeArgumentsContainsMarker)) {
                     const variances = getAliasVariances(source.aliasSymbol);
                     if (variances === emptyArray) {
-                        return Ternary.Maybe;
+                        return Ternary.Unknown;
                     }
                     const varianceResult = relateVariances(source.aliasTypeArguments, target.aliasTypeArguments, variances, intersectionState);
                     if (varianceResult !== undefined) {
@@ -17632,7 +17635,7 @@ namespace ts {
                         // effectively means we measure variance only from type parameter occurrences that aren't nested in
                         // recursive instantiations of the generic type.
                         if (variances === emptyArray) {
-                            return Ternary.Maybe;
+                            return Ternary.Unknown;
                         }
                         const varianceResult = relateVariances(getTypeArguments(<TypeReference>source), getTypeArguments(<TypeReference>target), variances, intersectionState);
                         if (varianceResult !== undefined) {
diff --git a/src/compiler/types.ts b/src/compiler/types.ts
@@ -5531,17 +5531,17 @@ namespace ts {
 
     /**
      * Ternary values are defined such that
-     * x & y is False if either x or y is False.
-     * x & y is Maybe if either x or y is Maybe, but neither x or y is False.
-     * x & y is True if both x and y are True.
-     * x | y is False if both x and y are False.
-     * x | y is Maybe if either x or y is Maybe, but neither x or y is True.
-     * x | y is True if either x or y is True.
+     * x & y picks the lesser in the order False < Unknown < Maybe < True, and
+     * x | y picks the greater in the order False < Unknown < Maybe < True.
+     * Generally, Ternary.Maybe is used as the result of a relation that depends on itself, and
+     * Ternary.Unknown is used as the result of a variance check that depends on itself. We make
+     * a distinction because we don't want to cache circular variance check results.
      */
     /* @internal */
     export const enum Ternary {
         False = 0,
-        Maybe = 1,
+        Unknown = 1,
+        Maybe = 3,
         True = -1
     }
 
diff --git a/tests/baselines/reference/varianceMeasurement.errors.txt b/tests/baselines/reference/varianceMeasurement.errors.txt
@@ -22,9 +22,11 @@ tests/cases/compiler/varianceMeasurement.ts(57,7): error TS2322: Type 'Fn<string
   Type 'unknown' is not assignable to type 'string'.
 tests/cases/compiler/varianceMeasurement.ts(62,7): error TS2322: Type 'Fn<string, number>' is not assignable to type 'Fn<string, 0>'.
   Type 'number' is not assignable to type '0'.
+tests/cases/compiler/varianceMeasurement.ts(75,7): error TS2322: Type 'C<unknown, number>' is not assignable to type 'C<unknown, string>'.
+  Type 'number' is not assignable to type 'string'.
 
 
-==== tests/cases/compiler/varianceMeasurement.ts (8 errors) ====
+==== tests/cases/compiler/varianceMeasurement.ts (9 errors) ====
     // The type below should be invariant in T but is measured as covariant because
     // we don't analyze recursive references.
     
@@ -119,4 +121,20 @@ tests/cases/compiler/varianceMeasurement.ts(62,7): error TS2322: Type 'Fn<string
           ~~~
 !!! error TS2322: Type 'Fn<string, number>' is not assignable to type 'Fn<string, 0>'.
 !!! error TS2322:   Type 'number' is not assignable to type '0'.
+    
+    // Repro from #39947
+    
+    interface I<Dummy, V> {
+      c: C<Dummy, V>;
+    }
+    
+    class C<Dummy, V> {
+      declare sub: I<Dummy, V>;
+      declare covariance: V;
+    }
+    
+    const c1: C<unknown, string> = new C<unknown, number>();  // Error
+          ~~
+!!! error TS2322: Type 'C<unknown, number>' is not assignable to type 'C<unknown, string>'.
+!!! error TS2322:   Type 'number' is not assignable to type 'string'.
     
diff --git a/tests/baselines/reference/varianceMeasurement.js b/tests/baselines/reference/varianceMeasurement.js
@@ -61,6 +61,19 @@ const fn2: Fn<'a', number> = fn;
 // Covariant in B
 const fn3: Fn<string, unknown> = fn;
 const fn4: Fn<string, 0> = fn;  // Error
+
+// Repro from #39947
+
+interface I<Dummy, V> {
+  c: C<Dummy, V>;
+}
+
+class C<Dummy, V> {
+  declare sub: I<Dummy, V>;
+  declare covariance: V;
+}
+
+const c1: C<unknown, string> = new C<unknown, number>();  // Error
 
 
 //// [varianceMeasurement.js]
@@ -81,3 +94,9 @@ var fn2 = fn;
 // Covariant in B
 var fn3 = fn;
 var fn4 = fn; // Error
+var C = /** @class */ (function () {
+    function C() {
+    }
+    return C;
+}());
+var c1 = new C(); // Error
diff --git a/tests/baselines/reference/varianceMeasurement.symbols b/tests/baselines/reference/varianceMeasurement.symbols
@@ -183,3 +183,38 @@ const fn4: Fn<string, 0> = fn;  // Error
 >Fn : Symbol(Fn, Decl(varianceMeasurement.ts, 44, 31))
 >fn : Symbol(fn, Decl(varianceMeasurement.ts, 53, 13))
 
+// Repro from #39947
+
+interface I<Dummy, V> {
+>I : Symbol(I, Decl(varianceMeasurement.ts, 61, 30))
+>Dummy : Symbol(Dummy, Decl(varianceMeasurement.ts, 65, 12))
+>V : Symbol(V, Decl(varianceMeasurement.ts, 65, 18))
+
+  c: C<Dummy, V>;
+>c : Symbol(I.c, Decl(varianceMeasurement.ts, 65, 23))
+>C : Symbol(C, Decl(varianceMeasurement.ts, 67, 1))
+>Dummy : Symbol(Dummy, Decl(varianceMeasurement.ts, 65, 12))
+>V : Symbol(V, Decl(varianceMeasurement.ts, 65, 18))
+}
+
+class C<Dummy, V> {
+>C : Symbol(C, Decl(varianceMeasurement.ts, 67, 1))
+>Dummy : Symbol(Dummy, Decl(varianceMeasurement.ts, 69, 8))
+>V : Symbol(V, Decl(varianceMeasurement.ts, 69, 14))
+
+  declare sub: I<Dummy, V>;
+>sub : Symbol(C.sub, Decl(varianceMeasurement.ts, 69, 19))
+>I : Symbol(I, Decl(varianceMeasurement.ts, 61, 30))
+>Dummy : Symbol(Dummy, Decl(varianceMeasurement.ts, 69, 8))
+>V : Symbol(V, Decl(varianceMeasurement.ts, 69, 14))
+
+  declare covariance: V;
+>covariance : Symbol(C.covariance, Decl(varianceMeasurement.ts, 70, 27))
+>V : Symbol(V, Decl(varianceMeasurement.ts, 69, 14))
+}
+
+const c1: C<unknown, string> = new C<unknown, number>();  // Error
+>c1 : Symbol(c1, Decl(varianceMeasurement.ts, 74, 5))
+>C : Symbol(C, Decl(varianceMeasurement.ts, 67, 1))
+>C : Symbol(C, Decl(varianceMeasurement.ts, 67, 1))
+
diff --git a/tests/baselines/reference/varianceMeasurement.types b/tests/baselines/reference/varianceMeasurement.types
@@ -131,3 +131,25 @@ const fn4: Fn<string, 0> = fn;  // Error
 >fn4 : Fn<string, 0>
 >fn : Fn<string, number>
 
+// Repro from #39947
+
+interface I<Dummy, V> {
+  c: C<Dummy, V>;
+>c : C<Dummy, V>
+}
+
+class C<Dummy, V> {
+>C : C<Dummy, V>
+
+  declare sub: I<Dummy, V>;
+>sub : I<Dummy, V>
+
+  declare covariance: V;
+>covariance : V
+}
+
+const c1: C<unknown, string> = new C<unknown, number>();  // Error
+>c1 : C<unknown, string>
+>new C<unknown, number>() : C<unknown, number>
+>C : typeof C
+
diff --git a/tests/cases/compiler/varianceMeasurement.ts b/tests/cases/compiler/varianceMeasurement.ts
@@ -62,3 +62,16 @@ const fn2: Fn<'a', number> = fn;
 // Covariant in B
 const fn3: Fn<string, unknown> = fn;
 const fn4: Fn<string, 0> = fn;  // Error
+
+// Repro from #39947
+
+interface I<Dummy, V> {
+  c: C<Dummy, V>;
+}
+
+class C<Dummy, V> {
+  declare sub: I<Dummy, V>;
+  declare covariance: V;
+}
+
+const c1: C<unknown, string> = new C<unknown, number>();  // Error
