Fixed type parameter leak in union calls with mixed type parameter presence #57371

Andarist · 2024-02-11T09:43:38Z

Andarist · 2024-02-11T09:45:38Z

src/compiler/checker.ts

@@ -13540,6 +13540,9 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
        result.compositeSignatures = concatenate(left.compositeKind !== TypeFlags.Intersection && left.compositeSignatures || [left], [right]);
        if (paramMapper) {


the gist of the issue was that first 2 calls with type params were unionized here (so paramMapper was created and attached to became left.mapper in the next iteration) but then that composite signature was unionized with one without the type params so the left.mapper wasn't attached to the result (because paramMapper was not created this time)

Andarist · 2024-02-11T09:48:40Z

src/compiler/checker.ts

@@ -13540,6 +13540,9 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
        result.compositeSignatures = concatenate(left.compositeKind !== TypeFlags.Intersection && left.compositeSignatures || [left], [right]);
        if (paramMapper) {
            result.mapper = left.compositeKind !== TypeFlags.Intersection && left.mapper && left.compositeSignatures ? combineTypeMappers(left.mapper, paramMapper) : paramMapper;
+        } 
+        else if (left.compositeKind !== TypeFlags.Intersection && left.mapper && left.compositeSignatures) {


This whole function is very similar to combineSignaturesOfIntersectionMembers so I thought that there might be a similar problem there but so far I couldn't create a test case that would manifest an issue there.

It's also worth noting that both of those functions check left.compositeKind but it's never the "other one" in the existing test suite (in combineSignaturesOfIntersectionMembers we never witness left.compositeKind === TypeFlags.Union and in combineSignaturesOfUnionMembers we never witness left.compositeKind === TypeFlags.Intersection). So either some tests are missing or those branches are unused.

…esence

weswigham · 2024-02-12T21:07:38Z

@typescript-bot run dt
@typescript-bot test top100
@typescript-bot perf test public
@typescript-bot pack this

typescript-bot · 2024-02-12T21:07:40Z

Heya @weswigham, I've started to run the parallelized Definitely Typed test suite on this PR at a4f0d66. You can monitor the build here.

Update: The results are in!

typescript-bot · 2024-02-12T21:07:40Z

Heya @weswigham, I've started to run the public perf test suite on this PR at a4f0d66. You can monitor the build here.

Update: The results are in!

typescript-bot · 2024-02-12T21:07:40Z

Heya @weswigham, I've started to run the diff-based top-repos suite on this PR at a4f0d66. You can monitor the build here.

Update: The results are in!

typescript-bot · 2024-02-12T21:07:40Z

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

typescript-bot · 2024-02-12T21:09:57Z

Hey @weswigham, 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/159833/artifacts?artifactName=tgz&fileId=C8A9595F66800C629D865536766573392E2F7CB901DD45921CED66BF327B1D7A02&fileName=/typescript-5.4.0-insiders.20240212.tgz"
    }
}

and then running npm install.

There is also a playground for this build and an npm module you can use via "typescript": "npm:@typescript-deploys/pr-build@5.4.0-pr-57371-5".;

typescript-bot · 2024-02-12T21:38:23Z

@weswigham
The results of the perf run you requested are in!

Here they are:

tsc

Comparison Report - baseline..pr

_Metric	_baseline	_pr	_Delta	_Best	_Worst	_p-value
_{mui-docs - node (v20.5.1, x64)}
_{Memory used}	_{2,382,586k (± 0.00%)}	_{2,382,612k (± 0.01%)}	_~	_2,382,395k	_2,382,899k	_{p=0.521 n=6}
_{Parse Time}	_{12.24s (± 2.30%)}	_{12.16s (± 0.58%)}	_~	_12.06s	_12.25s	_{p=1.000 n=6}
_{Bind Time}	_{2.67s (± 0.94%)}	_{2.67s (± 0.73%)}	_~	_2.65s	_2.70s	_{p=0.808 n=6}
_{Check Time}	_{96.90s (± 0.98%)}	_{97.99s (± 1.49%)}	_~	_96.37s	_100.62s	_{p=0.173 n=6}
_{Emit Time}	_{0.31s (± 1.32%)}	_{0.31s (± 1.32%)}	_~	_0.30s	_0.31s	_{p=1.000 n=6}
_{Total Time}	_{112.12s (± 0.86%)}	_{113.13s (± 1.26%)}	_~	_111.54s	_115.68s	_{p=0.173 n=6}
_{self-build-src - node (v20.5.1, x64)}
_{Memory used}	_{2,639,803k (± 5.36%)}	_{2,581,515k (± 0.03%)}	_~	_2,580,775k	_2,582,539k	_{p=0.230 n=6}
_{Parse Time}	_{5.07s (± 1.15%)}	_{5.07s (± 0.95%)}	_~	_4.98s	_5.11s	_{p=0.471 n=6}
_{Bind Time}	_{1.99s (± 0.83%)}	_{1.99s (± 0.59%)}	_~	_1.97s	_2.00s	_{p=0.682 n=6}
_{Check Time}	_{32.08s (± 0.22%)}	_{32.17s (± 0.33%)}	_~	_32.10s	_32.38s	_{p=0.173 n=6}
_{Emit Time}	_{2.74s (± 2.14%)}	_{2.79s (± 2.41%)}	_~	_2.69s	_2.88s	_{p=0.199 n=6}
_{Total Time}	_{41.89s (± 0.20%)}	_{42.03s (± 0.20%)}	_{+0.14s (+ 0.34%)}	_41.93s	_42.12s	_{p=0.020 n=6}
_{self-compiler - node (v20.5.1, x64)}
_{Memory used}	_{418,456k (± 0.02%)}	_{418,424k (± 0.02%)}	_~	_418,334k	_418,536k	_{p=0.689 n=6}
_{Parse Time}	_{2.90s (± 0.61%)}	_{2.89s (± 0.51%)}	_~	_2.87s	_2.91s	_{p=0.118 n=6}
_{Bind Time}	_{1.14s (± 0.45%)}	_{1.14s (± 0.66%)}	_~	_1.13s	_1.15s	_{p=0.784 n=6}
_{Check Time}	_{14.14s (± 0.19%)}	_{14.16s (± 0.32%)}	_~	_14.10s	_14.21s	_{p=0.629 n=6}
_{Emit Time}	_{1.04s (± 1.49%)}	_{1.04s (± 1.18%)}	_~	_1.03s	_1.06s	_{p=0.340 n=6}
_{Total Time}	_{19.22s (± 0.27%)}	_{19.22s (± 0.25%)}	_~	_19.16s	_19.28s	_{p=0.936 n=6}
_{vscode - node (v20.5.1, x64)}
_{Memory used}	_{2,874,843k (± 0.00%)}	_{2,874,871k (± 0.00%)}	_~	_2,874,831k	_2,874,928k	_{p=0.810 n=6}
_{Parse Time}	_{10.86s (± 0.21%)}	_{10.84s (± 0.30%)}	_~	_10.79s	_10.88s	_{p=0.411 n=6}
_{Bind Time}	_{3.46s (± 0.35%)}	_{3.46s (± 0.64%)}	_~	_3.44s	_3.50s	_{p=0.684 n=6}
_{Check Time}	_{57.97s (± 0.63%)}	_{57.70s (± 0.31%)}	_~	_57.51s	_57.96s	_{p=0.298 n=6}
_{Emit Time}	_{17.01s (± 8.63%)}	_{16.41s (± 0.35%)}	_~	_16.34s	_16.48s	_{p=0.521 n=6}
_{Total Time}	_{89.31s (± 1.58%)}	_{88.42s (± 0.17%)}	_~	_88.19s	_88.57s	_{p=0.149 n=6}
_{webpack - node (v20.5.1, x64)}
_{Memory used}	_{397,204k (± 0.01%)}	_{397,200k (± 0.01%)}	_~	_397,138k	_397,244k	_{p=1.000 n=6}
_{Parse Time}	_{3.39s (± 0.43%)}	_{3.39s (± 0.31%)}	_~	_3.37s	_3.40s	_{p=0.805 n=6}
_{Bind Time}	_{1.45s (± 0.81%)}	_{1.45s (± 0.95%)}	_~	_1.43s	_1.46s	_{p=0.742 n=6}
_{Check Time}	_{13.10s (± 0.33%)}	_{13.10s (± 0.19%)}	_~	_13.08s	_13.13s	_{p=1.000 n=6}
_{Emit Time}	_{0.00s (± 0.00%)}	_{0.00s (± 0.00%)}	_~	_0.00s	_0.00s	_{p=1.000 n=6}
_{Total Time}	_{17.94s (± 0.24%)}	_{17.94s (± 0.14%)}	_~	_17.91s	_17.98s	_{p=1.000 n=6}

System info unknown

Hosts

node (v20.5.1, x64)

Scenarios

mui-docs - node (v20.5.1, x64)
self-build-src - node (v20.5.1, x64)
self-compiler - node (v20.5.1, x64)
vscode - node (v20.5.1, x64)
webpack - node (v20.5.1, x64)

_Benchmark	_Name	_Iterations
_Current	_pr	₆
_Baseline	_baseline	₆

Developer Information:

Download Benchmarks

typescript-bot · 2024-02-12T21:49:59Z

Hey @weswigham, the results of running the DT tests are ready.
Everything looks the same!
You can check the log here.

typescript-bot · 2024-02-12T22:02:44Z

@weswigham Here are the results of running the top-repos suite comparing main and refs/pull/57371/merge:

Everything looks good!

typescript-bot assigned weswigham Feb 11, 2024

typescript-bot added the For Milestone Bug PRs that fix a bug with a specific milestone label Feb 11, 2024

Andarist commented Feb 11, 2024

View reviewed changes

Fixed type parameter leak in union calls with mixed type parameter pr…

a4f0d66

…esence

Andarist force-pushed the fix-type-param-leak-union-signature branch from 8f10bf7 to a4f0d66 Compare February 11, 2024 09:50

weswigham merged commit 0b4966a into microsoft:main Feb 12, 2024

Andarist deleted the fix-type-param-leak-union-signature branch February 12, 2024 23:15

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixed type parameter leak in union calls with mixed type parameter presence #57371

Fixed type parameter leak in union calls with mixed type parameter presence #57371

Andarist commented Feb 11, 2024

Andarist Feb 11, 2024

Andarist Feb 11, 2024

weswigham commented Feb 12, 2024

typescript-bot commented Feb 12, 2024 •

edited

Loading

typescript-bot commented Feb 12, 2024 •

edited

Loading

typescript-bot commented Feb 12, 2024 •

edited

Loading

typescript-bot commented Feb 12, 2024 •

edited

Loading

typescript-bot commented Feb 12, 2024 •

edited

Loading

typescript-bot commented Feb 12, 2024

tsc

typescript-bot commented Feb 12, 2024

typescript-bot commented Feb 12, 2024

		@@ -13540,6 +13540,9 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
		result.compositeSignatures = concatenate(left.compositeKind !== TypeFlags.Intersection && left.compositeSignatures \|\| [left], [right]);
		if (paramMapper) {

Fixed type parameter leak in union calls with mixed type parameter presence #57371

Fixed type parameter leak in union calls with mixed type parameter presence #57371

Conversation

Andarist commented Feb 11, 2024

Andarist Feb 11, 2024

Choose a reason for hiding this comment

Andarist Feb 11, 2024

Choose a reason for hiding this comment

weswigham commented Feb 12, 2024

typescript-bot commented Feb 12, 2024 • edited Loading

typescript-bot commented Feb 12, 2024 • edited Loading

typescript-bot commented Feb 12, 2024 • edited Loading

typescript-bot commented Feb 12, 2024 • edited Loading

typescript-bot commented Feb 12, 2024 • edited Loading

typescript-bot commented Feb 12, 2024

tsc

typescript-bot commented Feb 12, 2024

typescript-bot commented Feb 12, 2024

typescript-bot commented Feb 12, 2024 •

edited

Loading

typescript-bot commented Feb 12, 2024 •

edited

Loading

typescript-bot commented Feb 12, 2024 •

edited

Loading

typescript-bot commented Feb 12, 2024 •

edited

Loading

typescript-bot commented Feb 12, 2024 •

edited

Loading