From 156d219b0a71d8bd30dd7e648e9edaf8c0242815 Mon Sep 17 00:00:00 2001 From: Anders Hejlsberg Date: Wed, 1 Dec 2021 19:52:00 +0000 Subject: [PATCH] Cherry-pick PR #46599 into release-4.5 Component commits: ddc106b854 Decrease recursion depth limit to 3 + smarter check for recursion 86185ad96b Accept new baselines 52e10d3a98 Always set last type id 5f37d89c88 Keep indexed access recursion depth check 9df07a8482 Less expensive and corrected check for broadest equivalent keys --- src/compiler/checker.ts | 109 ++++++++++-------- ...nvariantGenericErrorElaboration.errors.txt | 12 +- 2 files changed, 67 insertions(+), 54 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index b6e35f7d6c8e4..e5310a8afec50 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -17804,7 +17804,7 @@ namespace ts { if (source.flags & TypeFlags.Singleton) return true; } if (source.flags & TypeFlags.Object && target.flags & TypeFlags.Object) { - const related = relation.get(getRelationKey(source, target, IntersectionState.None, relation)); + const related = relation.get(getRelationKey(source, target, IntersectionState.None, relation, /*ignoreConstraints*/ false)); if (related !== undefined) { return !!(related & RelationComparisonResult.Succeeded); } @@ -18704,7 +18704,8 @@ namespace ts { if (overflow) { return Ternary.False; } - const id = getRelationKey(source, target, intersectionState | (inPropertyCheck ? IntersectionState.InPropertyCheck : 0), relation); + const keyIntersectionState = intersectionState | (inPropertyCheck ? IntersectionState.InPropertyCheck : 0); + const id = getRelationKey(source, target, keyIntersectionState, relation, /*ingnoreConstraints*/ false); const entry = relation.get(id); if (entry !== undefined) { if (reportErrors && entry & RelationComparisonResult.Failed && !(entry & RelationComparisonResult.Reported)) { @@ -18731,16 +18732,13 @@ namespace ts { targetStack = []; } else { - // generate a key where all type parameter id positions are replaced with unconstrained type parameter ids - // this isn't perfect - nested type references passed as type arguments will muck up the indexes and thus - // prevent finding matches- but it should hit up the common cases - const broadestEquivalentId = id.split(",").map(i => i.replace(/-\d+/g, (_match, offset: number) => { - const index = length(id.slice(0, offset).match(/[-=]/g) || undefined); - return `=${index}`; - })).join(","); + // A key that starts with "*" is an indication that we have type references that reference constrained + // type parameters. For such keys we also check against the key we would have gotten if all type parameters + // were unconstrained. + const broadestEquivalentId = id.startsWith("*") ? getRelationKey(source, target, keyIntersectionState, relation, /*ignoreConstraints*/ true) : undefined; for (let i = 0; i < maybeCount; i++) { // If source and target are already being compared, consider them related with assumptions - if (id === maybeKeys[i] || broadestEquivalentId === maybeKeys[i]) { + if (id === maybeKeys[i] || broadestEquivalentId && broadestEquivalentId === maybeKeys[i]) { return Ternary.Maybe; } } @@ -20295,47 +20293,55 @@ namespace ts { return isNonDeferredTypeReference(type) && some(getTypeArguments(type), t => !!(t.flags & TypeFlags.TypeParameter) || isTypeReferenceWithGenericArguments(t)); } - /** - * getTypeReferenceId(A) returns "111=0-12=1" - * where A.id=111 and number.id=12 - */ - function getTypeReferenceId(type: TypeReference, typeParameters: Type[], depth = 0) { - let result = "" + type.target.id; - for (const t of getTypeArguments(type)) { - if (isUnconstrainedTypeParameter(t)) { - let index = typeParameters.indexOf(t); - if (index < 0) { - index = typeParameters.length; - typeParameters.push(t); + function getGenericTypeReferenceRelationKey(source: TypeReference, target: TypeReference, postFix: string, ignoreConstraints: boolean) { + const typeParameters: Type[] = []; + let constraintMarker = ""; + const sourceId = getTypeReferenceId(source, 0); + const targetId = getTypeReferenceId(target, 0); + return `${constraintMarker}${sourceId},${targetId}${postFix}`; + // getTypeReferenceId(A) returns "111=0-12=1" + // where A.id=111 and number.id=12 + function getTypeReferenceId(type: TypeReference, depth = 0) { + let result = "" + type.target.id; + for (const t of getTypeArguments(type)) { + if (t.flags & TypeFlags.TypeParameter) { + if (ignoreConstraints || isUnconstrainedTypeParameter(t)) { + let index = typeParameters.indexOf(t); + if (index < 0) { + index = typeParameters.length; + typeParameters.push(t); + } + result += "=" + index; + continue; + } + // We mark type references that reference constrained type parameters such that we know to obtain + // and look for a "broadest equivalent key" in the cache. + constraintMarker = "*"; + } + else if (depth < 4 && isTypeReferenceWithGenericArguments(t)) { + result += "<" + getTypeReferenceId(t as TypeReference, depth + 1) + ">"; + continue; } - result += "=" + index; - } - else if (depth < 4 && isTypeReferenceWithGenericArguments(t)) { - result += "<" + getTypeReferenceId(t as TypeReference, typeParameters, depth + 1) + ">"; - } - else { result += "-" + t.id; } + return result; } - return result; } /** * To improve caching, the relation key for two generic types uses the target's id plus ids of the type parameters. * For other cases, the types ids are used. */ - function getRelationKey(source: Type, target: Type, intersectionState: IntersectionState, relation: ESMap) { + function getRelationKey(source: Type, target: Type, intersectionState: IntersectionState, relation: ESMap, ignoreConstraints: boolean) { if (relation === identityRelation && source.id > target.id) { const temp = source; source = target; target = temp; } const postFix = intersectionState ? ":" + intersectionState : ""; - if (isTypeReferenceWithGenericArguments(source) && isTypeReferenceWithGenericArguments(target)) { - const typeParameters: Type[] = []; - return getTypeReferenceId(source as TypeReference, typeParameters) + "," + getTypeReferenceId(target as TypeReference, typeParameters) + postFix; - } - return source.id + "," + target.id + postFix; + return isTypeReferenceWithGenericArguments(source) && isTypeReferenceWithGenericArguments(target) ? + getGenericTypeReferenceRelationKey(source as TypeReference, target as TypeReference, postFix, ignoreConstraints) : + `${source.id},${target.id}${postFix}`; } // Invoke the callback for each underlying property symbol of the given symbol and return the first @@ -20389,27 +20395,34 @@ namespace ts { } // Return true if the given type is deeply nested. We consider this to be the case when structural type comparisons - // for 5 or more occurrences or instantiations of the type have been recorded on the given stack. It is possible, + // for maxDepth or more occurrences or instantiations of the type have been recorded on the given stack. It is possible, // though highly unlikely, for this test to be true in a situation where a chain of instantiations is not infinitely - // expanding. Effectively, we will generate a false positive when two types are structurally equal to at least 5 + // expanding. Effectively, we will generate a false positive when two types are structurally equal to at least maxDepth // levels, but unequal at some level beyond that. - // In addition, this will also detect when an indexed access has been chained off of 5 or more times (which is essentially - // the dual of the structural comparison), and likewise mark the type as deeply nested, potentially adding false positives - // for finite but deeply expanding indexed accesses (eg, for `Q[P1][P2][P3][P4][P5]`). - // It also detects when a recursive type reference has expanded 5 or more times, eg, if the true branch of + // In addition, this will also detect when an indexed access has been chained off of maxDepth more times (which is + // essentially the dual of the structural comparison), and likewise mark the type as deeply nested, potentially adding + // false positives for finite but deeply expanding indexed accesses (eg, for `Q[P1][P2][P3][P4][P5]`). + // It also detects when a recursive type reference has expanded maxDepth or more times, e.g. if the true branch of // `type A = null extends T ? [A>] : [T]` - // has expanded into `[A>>>>>]` - // in such cases we need to terminate the expansion, and we do so here. - function isDeeplyNestedType(type: Type, stack: Type[], depth: number, maxDepth = 5): boolean { + // has expanded into `[A>>>>>]`. In such cases we need + // to terminate the expansion, and we do so here. + function isDeeplyNestedType(type: Type, stack: Type[], depth: number, maxDepth = 3): boolean { if (depth >= maxDepth) { const identity = getRecursionIdentity(type); let count = 0; + let lastTypeId = 0; for (let i = 0; i < depth; i++) { - if (getRecursionIdentity(stack[i]) === identity) { - count++; - if (count >= maxDepth) { - return true; + const t = stack[i]; + if (getRecursionIdentity(t) === identity) { + // We only count occurrences with a higher type id than the previous occurrence, since higher + // type ids are an indicator of newer instantiations caused by recursion. + if (t.id >= lastTypeId) { + count++; + if (count >= maxDepth) { + return true; + } } + lastTypeId = t.id; } } } diff --git a/tests/baselines/reference/invariantGenericErrorElaboration.errors.txt b/tests/baselines/reference/invariantGenericErrorElaboration.errors.txt index 7ab83c61bcdd5..1113d3e0eae79 100644 --- a/tests/baselines/reference/invariantGenericErrorElaboration.errors.txt +++ b/tests/baselines/reference/invariantGenericErrorElaboration.errors.txt @@ -1,7 +1,7 @@ tests/cases/compiler/invariantGenericErrorElaboration.ts(3,7): error TS2322: Type 'Num' is not assignable to type 'Runtype'. - The types of 'constraint.constraint.constraint' are incompatible between these types. - Type 'Constraint>>' is not assignable to type 'Constraint>>>'. - Type 'Constraint>' is not assignable to type 'Constraint'. + The types of 'constraint.constraint' are incompatible between these types. + Type 'Constraint>' is not assignable to type 'Constraint>>'. + Type 'Runtype' is not assignable to type 'Num'. tests/cases/compiler/invariantGenericErrorElaboration.ts(4,19): error TS2322: Type 'Num' is not assignable to type 'Runtype'. @@ -11,9 +11,9 @@ tests/cases/compiler/invariantGenericErrorElaboration.ts(4,19): error TS2322: Ty const wat: Runtype = Num; ~~~ !!! error TS2322: Type 'Num' is not assignable to type 'Runtype'. -!!! error TS2322: The types of 'constraint.constraint.constraint' are incompatible between these types. -!!! error TS2322: Type 'Constraint>>' is not assignable to type 'Constraint>>>'. -!!! error TS2322: Type 'Constraint>' is not assignable to type 'Constraint'. +!!! error TS2322: The types of 'constraint.constraint' are incompatible between these types. +!!! error TS2322: Type 'Constraint>' is not assignable to type 'Constraint>>'. +!!! error TS2322: Type 'Runtype' is not assignable to type 'Num'. const Foo = Obj({ foo: Num }) ~~~ !!! error TS2322: Type 'Num' is not assignable to type 'Runtype'.