Skip to content

Don't skip elaborations when reporting errors for cached failed relations #55234

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 9 commits into from
Closed

Don't skip elaborations when reporting errors for cached failed relations #55234

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
eaba5a1
Don't skip elaborations when reporting errors for cached failed relat…
Andarist Aug 1, 2023
c7e33ff
Merge remote-tracking branch 'origin/main' into dont-skip-elaboration…
Andarist Aug 6, 2023
8a0b6c0
update existing baselines
Andarist Aug 6, 2023
16085d5
Remove `RelationComparisonResult.Reported` entirely
Andarist Aug 6, 2023
4a88fbd
new test case
Andarist Aug 6, 2023
add4655
Merge remote-tracking branch 'origin/main' into dont-skip-elaboration…
Andarist Aug 16, 2023
09a199d
Merge remote-tracking branch 'origin/main' into dont-skip-elaboration…
Andarist Nov 30, 2023
b29b639
Update `RelationComparisonResult`
Andarist Nov 30, 2023
0611fbb
update baseline
Andarist Nov 30, 2023
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
14 changes: 6 additions & 8 deletions src/compiler/checker.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20738,20 +20738,18 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
}
const id = getSymbolId(sourceSymbol) + "," + getSymbolId(targetSymbol);
const entry = enumRelation.get(id);
if (entry !== undefined && !(!(entry & RelationComparisonResult.Reported) && entry & RelationComparisonResult.Failed && errorReporter)) {
if (entry !== undefined && !(entry & RelationComparisonResult.Failed && errorReporter)) {
return !!(entry & RelationComparisonResult.Succeeded);
}
const targetEnumType = getTypeOfSymbol(targetSymbol);
for (const property of getPropertiesOfType(getTypeOfSymbol(sourceSymbol))) {
if (property.flags & SymbolFlags.EnumMember) {
const targetProperty = getPropertyOfType(targetEnumType, property.escapedName);
if (!targetProperty || !(targetProperty.flags & SymbolFlags.EnumMember)) {
enumRelation.set(id, RelationComparisonResult.Failed);

if (errorReporter) {
errorReporter(Diagnostics.Property_0_is_missing_in_type_1, symbolName(property), typeToString(getDeclaredTypeOfSymbol(targetSymbol), /*enclosingDeclaration*/ undefined, TypeFormatFlags.UseFullyQualifiedType));
enumRelation.set(id, RelationComparisonResult.Failed | RelationComparisonResult.Reported);
}
else {
enumRelation.set(id, RelationComparisonResult.Failed);
}
return false;
}
Expand Down Expand Up @@ -20933,7 +20931,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
if (overflow) {
// Record this relation as having failed such that we don't attempt the overflowing operation again.
const id = getRelationKey(source, target, /*intersectionState*/ IntersectionState.None, relation, /*ignoreConstraints*/ false);
relation.set(id, RelationComparisonResult.Reported | RelationComparisonResult.Failed);
relation.set(id, RelationComparisonResult.Failed);
Comment on lines 20932 to +20934
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sandersn @ahejlsberg those are lines that were not at all here when I created this PR. When syncing with main now I removed the RelationComparisonResult.Reported - just like it was done everywhere else here but I'm not sure if this should actually be done here.

Those lines come from #55851

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, the CI shows that this actually is important as now it's doing a lot of heavy work twice in this pathological case and the CI timeouts.

If we take a closer look at this baseline (even on main) we might notice that it's actually reporting 2 errors at the same location:

TypeScript/tests/baselines/reference/relationComplexityError.errors.txt

Lines 1 to 2 in 7216ee0

relationComplexityError.ts(12,5): error TS2322: Type 'T1 & T2' is not assignable to type 'T1 | null'.
relationComplexityError.ts(12,5): error TS2859: Excessive complexity comparing types 'T1 & T2' and 'T1 | null'.

So perhaps If I manage to report this once, the perf problems will go away naturally.

tracing?.instant(tracing.Phase.CheckTypes, "checkTypeRelatedTo_DepthLimit", { sourceId: source.id, targetId: target.id, depth: sourceDepth, targetDepth });
const message = relationCount <= 0 ?
Diagnostics.Excessive_complexity_comparing_types_0_and_1 :
Expand Down Expand Up @@ -21820,7 +21818,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
const id = getRelationKey(source, target, intersectionState, relation, /*ignoreConstraints*/ false);
const entry = relation.get(id);
if (entry !== undefined) {
if (reportErrors && entry & RelationComparisonResult.Failed && !(entry & RelationComparisonResult.Reported)) {
if (reportErrors && entry & RelationComparisonResult.Failed) {
// We are elaborating errors and the cached result is an unreported failure. The result will be reported
// as a failure, and should be updated as a reported failure by the bottom of this function.
}
Expand Down Expand Up @@ -21938,7 +21936,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
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.Reported : 0) | RelationComparisonResult.Failed | propagatingVarianceFlags);
relation.set(id, RelationComparisonResult.Failed | propagatingVarianceFlags);
relationCount--;
resetMaybeStack(/*markAllAsSucceeded*/ false);
}
Expand Down
5 changes: 2 additions & 3 deletions src/compiler/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -918,10 +918,9 @@ export const enum JsxFlags {
export const enum RelationComparisonResult {
Succeeded = 1 << 0, // Should be truthy
Failed = 1 << 1,
Reported = 1 << 2,

ReportsUnmeasurable = 1 << 3,
ReportsUnreliable = 1 << 4,
ReportsUnmeasurable = 1 << 2,
ReportsUnreliable = 1 << 3,
ReportsMask = ReportsUnmeasurable | ReportsUnreliable,
}

Expand Down
3 changes: 3 additions & 0 deletions tests/baselines/reference/arrayFrom.errors.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
arrayFrom.ts(20,7): error TS2322: Type 'A[]' is not assignable to type 'B[]'.
Property 'b' is missing in type 'A' but required in type 'B'.
arrayFrom.ts(23,7): error TS2322: Type 'A[]' is not assignable to type 'B[]'.
Property 'b' is missing in type 'A' but required in type 'B'.


==== arrayFrom.ts (2 errors) ====
Expand Down Expand Up @@ -33,6 +34,8 @@ arrayFrom.ts(23,7): error TS2322: Type 'A[]' is not assignable to type 'B[]'.
const result6: B[] = Array.from(inputALike); // expect error
~~~~~~~
!!! error TS2322: Type 'A[]' is not assignable to type 'B[]'.
!!! error TS2322: Property 'b' is missing in type 'A' but required in type 'B'.
!!! related TS2728 arrayFrom.ts:9:3: 'b' is declared here.
const result7: B[] = Array.from(inputALike, ({ a }): B => ({ b: a }));
const result8: A[] = Array.from(inputARand);
const result9: B[] = Array.from(inputARand, ({ a }): B => ({ b: a }));
Expand Down
5 changes: 3 additions & 2 deletions tests/baselines/reference/arrayOfSubtypeIsAssignableToReadonlyArray.errors.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ arrayOfSubtypeIsAssignableToReadonlyArray.ts(13,1): error TS2322: Type 'A[]' is
arrayOfSubtypeIsAssignableToReadonlyArray.ts(18,1): error TS2322: Type 'C<A>' is not assignable to type 'readonly B[]'.
The types returned by 'concat(...)' are incompatible between these types.
Type 'A[]' is not assignable to type 'B[]'.
Type 'A' is not assignable to type 'B'.
Property 'b' is missing in type 'A' but required in type 'B'.


==== arrayOfSubtypeIsAssignableToReadonlyArray.ts (2 errors) ====
Expand Down Expand Up @@ -33,5 +33,6 @@ arrayOfSubtypeIsAssignableToReadonlyArray.ts(18,1): error TS2322: Type 'C<A>' is
!!! error TS2322: Type 'C<A>' is not assignable to type 'readonly B[]'.
!!! error TS2322: The types returned by 'concat(...)' are incompatible between these types.
!!! error TS2322: Type 'A[]' is not assignable to type 'B[]'.
!!! error TS2322: Type 'A' is not assignable to type 'B'.
!!! error TS2322: Property 'b' is missing in type 'A' but required in type 'B'.
!!! related TS2728 arrayOfSubtypeIsAssignableToReadonlyArray.ts:2:21: 'b' is declared here.

9 changes: 5 additions & 4 deletions tests/baselines/reference/assignmentCompatWithNumericIndexer.errors.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,15 @@ assignmentCompatWithNumericIndexer.ts(32,9): error TS2322: Type '{ [x: number]:
assignmentCompatWithNumericIndexer.ts(33,9): error TS2322: Type 'A<T>' is not assignable to type '{ [x: number]: Derived; }'.
'number' index signatures are incompatible.
Type 'T' is not assignable to type 'Derived'.
Type 'Base' is not assignable to type 'Derived'.
Property 'bar' is missing in type 'Base' but required in type 'Derived'.
assignmentCompatWithNumericIndexer.ts(36,9): error TS2322: Type '{ [x: number]: Derived2; }' is not assignable to type 'A<T>'.
'number' index signatures are incompatible.
Type 'Derived2' is not assignable to type 'T'.
'Derived2' is assignable to the constraint of type 'T', but 'T' could be instantiated with a different subtype of constraint 'Base'.
assignmentCompatWithNumericIndexer.ts(37,9): error TS2322: Type 'A<T>' is not assignable to type '{ [x: number]: Derived2; }'.
'number' index signatures are incompatible.
Type 'T' is not assignable to type 'Derived2'.
Type 'Base' is not assignable to type 'Derived2'.
Type 'Base' is missing the following properties from type 'Derived2': baz, bar


==== assignmentCompatWithNumericIndexer.ts (6 errors) ====
Expand Down Expand Up @@ -74,7 +74,8 @@ assignmentCompatWithNumericIndexer.ts(37,9): error TS2322: Type 'A<T>' is not as
!!! error TS2322: Type 'A<T>' is not assignable to type '{ [x: number]: Derived; }'.
!!! error TS2322: 'number' index signatures are incompatible.
!!! error TS2322: Type 'T' is not assignable to type 'Derived'.
!!! error TS2322: Type 'Base' is not assignable to type 'Derived'.
!!! error TS2322: Property 'bar' is missing in type 'Base' but required in type 'Derived'.
!!! related TS2728 assignmentCompatWithNumericIndexer.ts:4:34: 'bar' is declared here.

var b2: { [x: number]: Derived2; }
a = b2; // error
Expand All @@ -88,7 +89,7 @@ assignmentCompatWithNumericIndexer.ts(37,9): error TS2322: Type 'A<T>' is not as
!!! error TS2322: Type 'A<T>' is not assignable to type '{ [x: number]: Derived2; }'.
!!! error TS2322: 'number' index signatures are incompatible.
!!! error TS2322: Type 'T' is not assignable to type 'Derived2'.
!!! error TS2322: Type 'Base' is not assignable to type 'Derived2'.
!!! error TS2322: Type 'Base' is missing the following properties from type 'Derived2': baz, bar

var b3: { [x: number]: T; }
a = b3; // ok
Expand Down
9 changes: 5 additions & 4 deletions tests/baselines/reference/assignmentCompatWithNumericIndexer2.errors.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,15 @@ assignmentCompatWithNumericIndexer2.ts(32,9): error TS2322: Type '{ [x: number]:
assignmentCompatWithNumericIndexer2.ts(33,9): error TS2322: Type 'A<T>' is not assignable to type '{ [x: number]: Derived; }'.
'number' index signatures are incompatible.
Type 'T' is not assignable to type 'Derived'.
Type 'Base' is not assignable to type 'Derived'.
Property 'bar' is missing in type 'Base' but required in type 'Derived'.
assignmentCompatWithNumericIndexer2.ts(36,9): error TS2322: Type '{ [x: number]: Derived2; }' is not assignable to type 'A<T>'.
'number' index signatures are incompatible.
Type 'Derived2' is not assignable to type 'T'.
'Derived2' is assignable to the constraint of type 'T', but 'T' could be instantiated with a different subtype of constraint 'Base'.
assignmentCompatWithNumericIndexer2.ts(37,9): error TS2322: Type 'A<T>' is not assignable to type '{ [x: number]: Derived2; }'.
'number' index signatures are incompatible.
Type 'T' is not assignable to type 'Derived2'.
Type 'Base' is not assignable to type 'Derived2'.
Type 'Base' is missing the following properties from type 'Derived2': baz, bar


==== assignmentCompatWithNumericIndexer2.ts (6 errors) ====
Expand Down Expand Up @@ -74,7 +74,8 @@ assignmentCompatWithNumericIndexer2.ts(37,9): error TS2322: Type 'A<T>' is not a
!!! error TS2322: Type 'A<T>' is not assignable to type '{ [x: number]: Derived; }'.
!!! error TS2322: 'number' index signatures are incompatible.
!!! error TS2322: Type 'T' is not assignable to type 'Derived'.
!!! error TS2322: Type 'Base' is not assignable to type 'Derived'.
!!! error TS2322: Property 'bar' is missing in type 'Base' but required in type 'Derived'.
!!! related TS2728 assignmentCompatWithNumericIndexer2.ts:4:34: 'bar' is declared here.

var b2: { [x: number]: Derived2; }
a = b2; // error
Expand All @@ -88,7 +89,7 @@ assignmentCompatWithNumericIndexer2.ts(37,9): error TS2322: Type 'A<T>' is not a
!!! error TS2322: Type 'A<T>' is not assignable to type '{ [x: number]: Derived2; }'.
!!! error TS2322: 'number' index signatures are incompatible.
!!! error TS2322: Type 'T' is not assignable to type 'Derived2'.
!!! error TS2322: Type 'Base' is not assignable to type 'Derived2'.
!!! error TS2322: Type 'Base' is missing the following properties from type 'Derived2': baz, bar

var b3: { [x: number]: T; }
a = b3; // ok
Expand Down
Loading