Detect comparisons between large unions or intersections #41574

amcasey · 2020-11-18T01:40:47Z

If their multiplied size is greater than 1E7 (chosen based on the repro
in #41517), then we'll expend a large amount of time and memory
comparing them, so alert the user instead.

Fixes #41517

amcasey · 2020-11-18T01:40:57Z

@typescript-bot test this

amcasey · 2020-11-18T01:41:06Z

@typescript-bot pack this

typescript-bot · 2020-11-18T01:41:10Z

Heya @amcasey, I've started to run the tarball bundle task on this PR at 935e27a. You can monitor the build here.

typescript-bot · 2020-11-18T01:41:22Z

Heya @amcasey, I've started to run the extended test suite on this PR at 935e27a. You can monitor the build here.

amcasey · 2020-11-18T01:42:02Z

@NoPhaseNoKill the bot should post a link to a private build shortly. If you have a minute, can you please confirm that this change correctly flags the problematic expression in your real code? (I've already confirmed that it works in the toy repro.)

typescript-bot · 2020-11-18T01:58:24Z

Hey @amcasey, I've packed this into an installable tgz. You can install it for testing by referencing it in your package.json like so:

{
    "devDependencies": {
        "typescript": "https://typescript.visualstudio.com/cf7ac146-d525-443c-b23c-0d58337efebc/_apis/build/builds/89455/artifacts?artifactName=tgz&fileId=618CA2EA1F4B47196F4BF57EC3167658E429AB9FDB6380D977C766FBCFB1546F02&fileName=/typescript-4.2.0-insiders.20201118.tgz"
    }
}

and then running npm install.

There is also a playground for this build.

NoPhaseNoKill · 2020-11-18T20:44:37Z

The error I'm now seeing in the compiler is:

ERROR in /[redacted]src/IconButton.tsx
ERROR in /[redacted]/src/IconButton.tsx(23,17):
TS2769: No overload matches this call.
  Overload 1 of 2, '(props: Readonly<IconProps>): Icon', gave the following error.
    Expression requires comparison of excessively large unions or intersections.
  Overload 2 of 2, '(props: IconProps, context?: any): Icon', gave the following error.
    Expression requires comparison of excessively large unions or intersections.

Looks good to me :)

NoPhaseNoKill · 2020-11-18T20:47:56Z

Are there any places I can lodge/put in feedback for the help you've given me? I just want to say what an excellent experience this has been for me.

Normally, dealing with large companies/significant repo's it's an absolute nightmare. So thank you very much for all your help, you deserve to be recognised formally.

amcasey · 2020-11-18T20:50:00Z

Thanks for confirming! I'm glad we could get that sorted out for you. And, no, there won't be a satisfaction survey - if you're happy, I'm happy. 😄

NoPhaseNoKill · 2020-11-18T20:53:19Z

Sounds good man, thanks for the help again. I'll reach out if we find any unusual behaviour in the future. Keep in touch, Tom

amcasey · 2020-11-18T22:00:18Z

@ahejlsberg pointed out that we probably want different limits for primitive and non-primitive unions/intersections. I'll try to work some out empirically.

amcasey · 2020-11-19T22:42:53Z

@typescript-bot test this

typescript-bot · 2020-11-19T22:42:56Z

Heya @amcasey, I've started to run the extended test suite on this PR at 64a1229. You can monitor the build here.

src/compiler/checker.ts

src/compiler/diagnosticMessages.json

tests/cases/compiler/largePrimitiveUnionComparisons.ts

amcasey · 2020-11-20T01:25:50Z

RWC shows no instances of the new error text.

amcasey · 2020-12-09T17:09:23Z

@typescript-bot pack this

typescript-bot · 2020-12-09T17:09:26Z

Heya @amcasey, I've started to run the tarball bundle task on this PR at 8f33b9d. You can monitor the build here.

weswigham · 2020-12-09T17:58:37Z

...ference/conditionalTypeDiscriminatingLargeUnionRegularTypeFetchingSpeedReasonable.errors.txt

@@ -0,0 +1,8018 @@
+tests/cases/compiler/conditionalTypeDiscriminatingLargeUnionRegularTypeFetchingSpeedReasonable.ts(8011,26): error TS2797: Expression requires comparison of excessively large union or intersection types.


I'm pretty sure adding an error into this test kinda invalidates it, since it was here to test that we could do a thing without timing out - not being able to do it at all is kinda strictly worse (and, moreover, means we're no longer testing for a timeout). Is it just a matter of the limit being too low? #37749 made it so this is actually quite cheap for us (and this is just a simplified form of a common reactish pattern), so we shouldn't have an error here.

Specifically, regarding the limit, these kinda of errors are "hard blocks" in scenarios which may otherwise work, so we should ensure we only ever issue them them in cases where we'd either time out, stack out, or OOM without the error. If we can process the comparison without one of those three things occurring, we shouldn't be erroring, IMO.

If their multiplied size is greater than 1E7 (chosen based on the repro in microsoft#41517), then we'll expend a large amount of time and memory comparing them, so alert the user instead. Fixes microsoft#41517

amcasey · 2020-12-10T00:48:40Z

Force pushed a rebase, so that feedback-driven changes would show up cleanly in subsequent commits.

weswigham

👍 tracepoint

src/compiler/checker.ts

amcasey requested review from weswigham and ahejlsberg November 18, 2020 01:40

typescript-bot assigned amcasey Nov 18, 2020

typescript-bot added the Author: Team label Nov 18, 2020

typescript-bot added the For Milestone Bug PRs that fix a bug with a specific milestone label Nov 18, 2020

ahejlsberg requested changes Nov 19, 2020

View reviewed changes

src/compiler/checker.ts Outdated Show resolved Hide resolved

src/compiler/diagnosticMessages.json Outdated Show resolved Hide resolved

tests/cases/compiler/largePrimitiveUnionComparisons.ts Outdated Show resolved Hide resolved

amcasey requested a review from ahejlsberg November 24, 2020 22:31

amcasey mentioned this pull request Dec 9, 2020

Slow compilation issues (40-50s) - --skipLibCheck not working #41517

Closed

weswigham reviewed Dec 9, 2020

View reviewed changes

amcasey added 4 commits December 9, 2020 16:36

Detect comparisons between large unions or intersections

c9bceb8

If their multiplied size is greater than 1E7 (chosen based on the repro in microsoft#41517), then we'll expend a large amount of time and memory comparing them, so alert the user instead. Fixes microsoft#41517

Make an exception for primitive union comparisons

ccd945a

Address PR feedback

47e9682

Pick up baseline change from master

833a268

amcasey force-pushed the UnionCompLimit branch from 8f33b9d to 833a268 Compare December 10, 2020 00:47

Eliminate diagnostic and only trace

3a6def2

amcasey requested a review from weswigham December 10, 2020 18:52

weswigham approved these changes Dec 10, 2020

View reviewed changes

src/compiler/checker.ts Show resolved Hide resolved

Don't check reportErrors

da98c00

amcasey merged commit 3e72526 into microsoft:master Dec 10, 2020

amcasey deleted the UnionCompLimit branch December 10, 2020 21:53

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Detect comparisons between large unions or intersections #41574

Detect comparisons between large unions or intersections #41574

amcasey commented Nov 18, 2020

amcasey commented Nov 18, 2020

amcasey commented Nov 18, 2020

typescript-bot commented Nov 18, 2020 •

edited

Loading

typescript-bot commented Nov 18, 2020 •

edited

Loading

amcasey commented Nov 18, 2020

typescript-bot commented Nov 18, 2020 •

edited

Loading

NoPhaseNoKill commented Nov 18, 2020 •

edited

Loading

NoPhaseNoKill commented Nov 18, 2020

amcasey commented Nov 18, 2020

NoPhaseNoKill commented Nov 18, 2020

amcasey commented Nov 18, 2020

amcasey commented Nov 19, 2020

typescript-bot commented Nov 19, 2020 •

edited

Loading

amcasey commented Nov 20, 2020

amcasey commented Dec 9, 2020

typescript-bot commented Dec 9, 2020 •

edited

Loading

weswigham Dec 9, 2020

weswigham Dec 9, 2020 •

edited

Loading

amcasey commented Dec 10, 2020

weswigham left a comment

		@@ -0,0 +1,8018 @@
		tests/cases/compiler/conditionalTypeDiscriminatingLargeUnionRegularTypeFetchingSpeedReasonable.ts(8011,26): error TS2797: Expression requires comparison of excessively large union or intersection types.

Detect comparisons between large unions or intersections #41574

Detect comparisons between large unions or intersections #41574

Conversation

amcasey commented Nov 18, 2020

amcasey commented Nov 18, 2020

amcasey commented Nov 18, 2020

typescript-bot commented Nov 18, 2020 • edited Loading

typescript-bot commented Nov 18, 2020 • edited Loading

amcasey commented Nov 18, 2020

typescript-bot commented Nov 18, 2020 • edited Loading

NoPhaseNoKill commented Nov 18, 2020 • edited Loading

NoPhaseNoKill commented Nov 18, 2020

amcasey commented Nov 18, 2020

NoPhaseNoKill commented Nov 18, 2020

amcasey commented Nov 18, 2020

amcasey commented Nov 19, 2020

typescript-bot commented Nov 19, 2020 • edited Loading

amcasey commented Nov 20, 2020

amcasey commented Dec 9, 2020

typescript-bot commented Dec 9, 2020 • edited Loading

weswigham Dec 9, 2020

Choose a reason for hiding this comment

weswigham Dec 9, 2020 • edited Loading

Choose a reason for hiding this comment

amcasey commented Dec 10, 2020

weswigham left a comment

Choose a reason for hiding this comment

typescript-bot commented Nov 18, 2020 •

edited

Loading

typescript-bot commented Nov 18, 2020 •

edited

Loading

typescript-bot commented Nov 18, 2020 •

edited

Loading

NoPhaseNoKill commented Nov 18, 2020 •

edited

Loading

typescript-bot commented Nov 19, 2020 •

edited

Loading

typescript-bot commented Dec 9, 2020 •

edited

Loading

weswigham Dec 9, 2020 •

edited

Loading