Skip to content

Cache most calls to isRelatedTo #17950

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 3 commits into from
Closed

Cache most calls to isRelatedTo #17950

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7831db9
cache relationships, needs work on preserving diagnostic output
weswigham Aug 18, 2017
d4143b7
Performance improvements
ahejlsberg Aug 25, 2017
fd8df1e
Accept new baseline
weswigham Aug 26, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
76 changes: 45 additions & 31 deletions src/compiler/checker.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8885,6 +8885,16 @@ namespace ts {
return false;
}

function reportElaboratedRelationError(headMessage: DiagnosticMessage | undefined, source: Type, target: Type) {
if (source.flags & TypeFlags.Object && target.flags & TypeFlags.Primitive) {
tryElaborateErrorsForPrimitivesAndObjects(source, target);
}
else if (source.symbol && source.flags & TypeFlags.Object && globalObjectType === source) {
reportError(Diagnostics.The_Object_type_is_assignable_to_very_few_other_types_Did_you_mean_to_use_the_any_type_instead);
}
reportRelationError(headMessage, source, target);
}

/**
* Compare two types and return
* * Ternary.True if they are related with no assumptions,
Expand All @@ -8899,20 +8909,36 @@ namespace ts {
target = (<LiteralType>target).regularType;
}
// both types are the same - covers 'they are the same primitive type or both are Any' or the same type parameter cases
if (source === target) return Ternary.True;
if (source === target || relation !== identityRelation && isSimpleTypeRelatedTo(source, target, relation, reportErrors ? reportError : undefined)) {
return Ternary.True;
}

if (relation === identityRelation) {
return isIdenticalTo(source, target);
const id = getRelationKey(source, target, relation);
const related = relation.get(id);
if (related !== undefined) {
// If we need to report errors, and the result is RelationComparisonResult.Failed, then we need
// to redo our work to generate an error message. Otherwise, we can just return the cached result.
if (!reportErrors || related !== RelationComparisonResult.Failed) {
if (reportErrors && related !== RelationComparisonResult.Succeeded) {
reportElaboratedRelationError(headMessage, source, target);
}
return related === RelationComparisonResult.Succeeded ? Ternary.True : Ternary.False;
}
if (reportErrors && related === RelationComparisonResult.Failed) {
relation.set(id, RelationComparisonResult.FailedAndReported);
}
}

if (isSimpleTypeRelatedTo(source, target, relation, reportErrors ? reportError : undefined)) return Ternary.True;
if (relation === identityRelation) {
return cacheResult(isIdenticalTo(source, target, id), id, /*reportErrors*/ false);
}

if (getObjectFlags(source) & ObjectFlags.ObjectLiteral && source.flags & TypeFlags.FreshLiteral) {
if (hasExcessProperties(<FreshObjectLiteralType>source, target, reportErrors)) {
if (reportErrors) {
reportRelationError(headMessage, source, target);
}
return Ternary.False;
return cacheResult(Ternary.False, id, reportErrors);
}
// Above we check for excess properties with respect to the entire target type. When union
// and intersection types are further deconstructed on the target side, we don't want to
Expand Down Expand Up @@ -8944,6 +8970,7 @@ namespace ts {
reportError(Diagnostics.Type_0_has_no_properties_in_common_with_type_1, typeToString(source), typeToString(target));
}
}
// Intentionally not cached, so common property errors are repeated
return Ternary.False;
}

Expand Down Expand Up @@ -8985,7 +9012,7 @@ namespace ts {
result = someTypeRelatedToType(<IntersectionType>source, target, /*reportErrors*/ false);
}
if (!result && (source.flags & TypeFlags.StructuredOrTypeVariable || target.flags & TypeFlags.StructuredOrTypeVariable)) {
if (result = recursiveTypeRelatedTo(source, target, reportErrors)) {
if (result = recursiveTypeRelatedTo(source, target, id, reportErrors)) {
errorInfo = saveErrorInfo;
}
}
Expand All @@ -8994,21 +9021,23 @@ namespace ts {
isIntersectionConstituent = saveIsIntersectionConstituent;

if (!result && reportErrors) {
if (source.flags & TypeFlags.Object && target.flags & TypeFlags.Primitive) {
tryElaborateErrorsForPrimitivesAndObjects(source, target);
}
else if (source.symbol && source.flags & TypeFlags.Object && globalObjectType === source) {
reportError(Diagnostics.The_Object_type_is_assignable_to_very_few_other_types_Did_you_mean_to_use_the_any_type_instead);
}
reportRelationError(headMessage, source, target);
reportElaboratedRelationError(headMessage, source, target);
}

return cacheResult(result, id, reportErrors);
}

function cacheResult(result: Ternary, comparisonId: string, reportErrors?: boolean) {
if (!relation.has(comparisonId) && result !== Ternary.Maybe) {
relation.set(comparisonId, result ? RelationComparisonResult.Succeeded : reportErrors ? RelationComparisonResult.FailedAndReported : RelationComparisonResult.Failed);
}
return result;
}

function isIdenticalTo(source: Type, target: Type): Ternary {
function isIdenticalTo(source: Type, target: Type, id: string): Ternary {
let result: Ternary;
if (source.flags & TypeFlags.Object && target.flags & TypeFlags.Object) {
return recursiveTypeRelatedTo(source, target, /*reportErrors*/ false);
return recursiveTypeRelatedTo(source, target, id, /*reportErrors*/ false);
}
if (source.flags & TypeFlags.Union && target.flags & TypeFlags.Union ||
source.flags & TypeFlags.Intersection && target.flags & TypeFlags.Intersection) {
Expand Down Expand Up @@ -9184,22 +9213,10 @@ namespace ts {
// Third, check if both types are part of deeply nested chains of generic type instantiations and if so assume the types are
// equal and infinitely expanding. Fourth, if we have reached a depth of 100 nested comparisons, assume we have runaway recursion
// and issue an error. Otherwise, actually compare the structure of the two types.
function recursiveTypeRelatedTo(source: Type, target: Type, reportErrors: boolean): Ternary {
function recursiveTypeRelatedTo(source: Type, target: Type, id: string, reportErrors: boolean): Ternary {
if (overflow) {
return Ternary.False;
}
const id = getRelationKey(source, target, relation);
const related = relation.get(id);
if (related !== undefined) {
if (reportErrors && related === RelationComparisonResult.Failed) {
// We are elaborating errors and the cached result is an unreported failure. Record the result as a reported
// failure and continue computing the relation such that errors get reported.
relation.set(id, RelationComparisonResult.FailedAndReported);
}
else {
return related === RelationComparisonResult.Succeeded ? Ternary.True : Ternary.False;
}
}
if (!maybeKeys) {
maybeKeys = [];
sourceStack = [];
Expand Down Expand Up @@ -9239,9 +9256,6 @@ namespace ts {
}
}
else {
// A false result goes straight into global cache (when something is false under
// assumptions it will also be false without assumptions)
relation.set(id, reportErrors ? RelationComparisonResult.FailedAndReported : RelationComparisonResult.Failed);
maybeCount = maybeStart;
}
return result;
Expand Down
2 changes: 0 additions & 2 deletions tests/baselines/reference/arityAndOrderCompatibility01.errors.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ tests/cases/conformance/types/tuple/arityAndOrderCompatibility01.ts(26,5): error
Types of property 'pop' are incompatible.
Type '() => string | number' is not assignable to type '() => string'.
Type 'string | number' is not assignable to type 'string'.
Type 'number' is not assignable to type 'string'.
tests/cases/conformance/types/tuple/arityAndOrderCompatibility01.ts(27,5): error TS2322: Type '{ 0: string; 1: number; }' is not assignable to type '[string]'.
Property 'length' is missing in type '{ 0: string; 1: number; }'.
tests/cases/conformance/types/tuple/arityAndOrderCompatibility01.ts(28,5): error TS2322: Type '[string, number]' is not assignable to type '[number, string]'.
Expand Down Expand Up @@ -118,7 +117,6 @@ tests/cases/conformance/types/tuple/arityAndOrderCompatibility01.ts(30,5): error
!!! error TS2322: Types of property 'pop' are incompatible.
!!! error TS2322: Type '() => string | number' is not assignable to type '() => string'.
!!! error TS2322: Type 'string | number' is not assignable to type 'string'.
!!! error TS2322: Type 'number' is not assignable to type 'string'.
var m3: [string] = z;
~~
!!! error TS2322: Type '{ 0: string; 1: number; }' is not assignable to type '[string]'.
Expand Down
10 changes: 0 additions & 10 deletions tests/baselines/reference/callWithSpread2.errors.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,15 +1,10 @@
tests/cases/conformance/expressions/functionCalls/callWithSpread2.ts(30,5): error TS2345: Argument of type 'string | number' is not assignable to parameter of type 'number'.
Type 'string' is not assignable to type 'number'.
tests/cases/conformance/expressions/functionCalls/callWithSpread2.ts(31,5): error TS2345: Argument of type 'string | number' is not assignable to parameter of type 'number'.
Type 'string' is not assignable to type 'number'.
tests/cases/conformance/expressions/functionCalls/callWithSpread2.ts(32,13): error TS2345: Argument of type 'string | number' is not assignable to parameter of type 'number'.
Type 'string' is not assignable to type 'number'.
tests/cases/conformance/expressions/functionCalls/callWithSpread2.ts(33,13): error TS2345: Argument of type 'string | number' is not assignable to parameter of type 'number'.
Type 'string' is not assignable to type 'number'.
tests/cases/conformance/expressions/functionCalls/callWithSpread2.ts(34,11): error TS2345: Argument of type 'string | number' is not assignable to parameter of type 'number'.
Type 'string' is not assignable to type 'number'.
tests/cases/conformance/expressions/functionCalls/callWithSpread2.ts(35,11): error TS2345: Argument of type 'string | number' is not assignable to parameter of type 'number'.
Type 'string' is not assignable to type 'number'.
tests/cases/conformance/expressions/functionCalls/callWithSpread2.ts(36,1): error TS2556: Expected 1-3 arguments, but got a minimum of 0.
tests/cases/conformance/expressions/functionCalls/callWithSpread2.ts(37,1): error TS2556: Expected 1-3 arguments, but got a minimum of 0.
tests/cases/conformance/expressions/functionCalls/callWithSpread2.ts(38,1): error TS2556: Expected 1-3 arguments, but got a minimum of 0.
Expand Down Expand Up @@ -52,23 +47,18 @@ tests/cases/conformance/expressions/functionCalls/callWithSpread2.ts(38,1): erro
all(...tuple)
~~~~~~~~
!!! error TS2345: Argument of type 'string | number' is not assignable to parameter of type 'number'.
!!! error TS2345: Type 'string' is not assignable to type 'number'.
prefix("b", ...mixed)
~~~~~~~~
!!! error TS2345: Argument of type 'string | number' is not assignable to parameter of type 'number'.
!!! error TS2345: Type 'string' is not assignable to type 'number'.
prefix("c", ...tuple)
~~~~~~~~
!!! error TS2345: Argument of type 'string | number' is not assignable to parameter of type 'number'.
!!! error TS2345: Type 'string' is not assignable to type 'number'.
rest("e", ...mixed)
~~~~~~~~
!!! error TS2345: Argument of type 'string | number' is not assignable to parameter of type 'number'.
!!! error TS2345: Type 'string' is not assignable to type 'number'.
rest("f", ...tuple)
~~~~~~~~
!!! error TS2345: Argument of type 'string | number' is not assignable to parameter of type 'number'.
!!! error TS2345: Type 'string' is not assignable to type 'number'.
prefix(...ns) // required parameters are required
~~~~~~~~~~~~~
!!! error TS2556: Expected 1-3 arguments, but got a minimum of 0.
Expand Down
4 changes: 0 additions & 4 deletions tests/baselines/reference/checkJsxChildrenProperty7.errors.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,10 @@ tests/cases/conformance/jsx/file.tsx(25,16): error TS2322: Type '{ a: 10; b: "hi
Type '{ a: 10; b: "hi"; children: (string | Element)[]; }' is not assignable to type 'Prop'.
Types of property 'children' are incompatible.
Type '(string | Element)[]' is not assignable to type 'Element | Element[]'.
Type '(string | Element)[]' is not assignable to type 'Element[]'.
tests/cases/conformance/jsx/file.tsx(27,16): error TS2322: Type '{ a: 10; b: "hi"; children: (string | Element)[]; }' is not assignable to type 'IntrinsicAttributes & Prop'.
Type '{ a: 10; b: "hi"; children: (string | Element)[]; }' is not assignable to type 'Prop'.
Types of property 'children' are incompatible.
Type '(string | Element)[]' is not assignable to type 'Element | Element[]'.
Type '(string | Element)[]' is not assignable to type 'Element[]'.


==== tests/cases/conformance/jsx/file.tsx (3 errors) ====
Expand Down Expand Up @@ -56,13 +54,11 @@ tests/cases/conformance/jsx/file.tsx(27,16): error TS2322: Type '{ a: 10; b: "hi
!!! error TS2322: Type '{ a: 10; b: "hi"; children: (string | Element)[]; }' is not assignable to type 'Prop'.
!!! error TS2322: Types of property 'children' are incompatible.
!!! error TS2322: Type '(string | Element)[]' is not assignable to type 'Element | Element[]'.
!!! error TS2322: Type '(string | Element)[]' is not assignable to type 'Element[]'.
<AnotherButton /> </Comp>;
let k3 = <Comp a={10} b="hi"> <Button />
~~~~~~~~~~~~~
!!! error TS2322: Type '{ a: 10; b: "hi"; children: (string | Element)[]; }' is not assignable to type 'IntrinsicAttributes & Prop'.
!!! error TS2322: Type '{ a: 10; b: "hi"; children: (string | Element)[]; }' is not assignable to type 'Prop'.
!!! error TS2322: Types of property 'children' are incompatible.
!!! error TS2322: Type '(string | Element)[]' is not assignable to type 'Element | Element[]'.
!!! error TS2322: Type '(string | Element)[]' is not assignable to type 'Element[]'.
<AnotherButton /></Comp>;
4 changes: 1 addition & 3 deletions tests/baselines/reference/contextualTypeWithTuple.errors.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ tests/cases/conformance/types/tuple/contextualTypeWithTuple.ts(24,1): error TS23
Property '2' is missing in type '[C, string | number]'.
tests/cases/conformance/types/tuple/contextualTypeWithTuple.ts(25,1): error TS2322: Type '[number, string | number]' is not assignable to type '[number, string]'.
Type 'string | number' is not assignable to type 'string'.
Type 'number' is not assignable to type 'string'.


==== tests/cases/conformance/types/tuple/contextualTypeWithTuple.ts (7 errors) ====
Expand Down Expand Up @@ -73,5 +72,4 @@ tests/cases/conformance/types/tuple/contextualTypeWithTuple.ts(25,1): error TS23
numStrTuple = unionTuple3;
~~~~~~~~~~~
!!! error TS2322: Type '[number, string | number]' is not assignable to type '[number, string]'.
!!! error TS2322: Type 'string | number' is not assignable to type 'string'.
!!! error TS2322: Type 'number' is not assignable to type 'string'.
!!! error TS2322: Type 'string | number' is not assignable to type 'string'.
Loading