Skip to content

Conversation

@CurtisFenner
Copy link

@CurtisFenner CurtisFenner commented Apr 13, 2025

Fixes #61429

The maybeTypeOfKind function is a simpler version of full type-checking which is used by arithmetic. Previously it was used to determine whether arithmetic operations such as +, <<, and ~ were acting on number types or bigint types.

Note that these type checks are somewhat complicated by a desire to preserve unsoundness: any * any should be number, rather than the (sound but exceedingly impractical) number | bigint.

Previously, the maybeTypeOfKind function did not return true when an operand has a type T, regardless of the bounds of T. This meant that when maybeTypeOfKind is passed T extends bigint and bigint, it returned false.

This caused expressions like a & b to be assigned the "default" type number instead of bigint.


To fix this issue, I updated the checks to instead use maybeTypeOfKindConsideringBaseConstraint. I suspect that some of the other uses of maybeTypeOfKind have similar issues, but if so they seem much more subtle and I wasn't able to construct any suspicious tests for them.


I added the bigintSubtypingTypeParameter test which includes a variety of unary and binary operators acting on such T extends bigint values.


This is my first PR, so apologies if I'm missing something important.

@github-project-automation github-project-automation bot moved this to Not started in PR Backlog Apr 13, 2025
@typescript-bot typescript-bot added the For Backlog Bug PRs that fix a backlog bug label Apr 13, 2025
@CurtisFenner
Copy link
Author

@microsoft-github-policy-service agree

@CurtisFenner CurtisFenner force-pushed the fix-61429-bigint-type-parameter-maybeTypeOfKind-bug branch from ae470a1 to 1390f5f Compare April 18, 2025 00:44
@CurtisFenner CurtisFenner force-pushed the fix-61429-bigint-type-parameter-maybeTypeOfKind-bug branch from 1390f5f to 71d535a Compare April 18, 2025 00:47
@CurtisFenner
Copy link
Author

CurtisFenner commented Apr 27, 2025

@jakebailey Hi, I'm not sure if you missed the new approach I took. I found a more appropriate maybeTypeOfKindConsideringBaseConstraint and audited all the existing callers of maybeTypeOfKind to see which obviously should be using it. (I also did an extraction of getBinaryArithmeticResultType into a function since it was a large code block in a large chain of alternatives, to be more similar to the getUnaryResultType)

Apologies if you were already aware, I'm sure you have more important tasks to attend to for TypeScript!

@jakebailey
Copy link
Member

@typescript-bot test it

@typescript-bot
Copy link
Collaborator

typescript-bot commented May 9, 2025

Starting jobs; this comment will be updated as builds start and complete.

Command Status Results
test top400 ✅ Started ✅ Results
user test this ✅ Started ✅ Results
run dt ✅ Started ✅ Results
perf test this faster ✅ Started 👀 Results

@typescript-bot
Copy link
Collaborator

@jakebailey Here are the results of running the user tests with tsc comparing main and refs/pull/61571/merge:

Everything looks good!

@typescript-bot
Copy link
Collaborator

Hey @jakebailey, the results of running the DT tests are ready.

Everything looks the same!

You can check the log here.

@typescript-bot
Copy link
Collaborator

@jakebailey
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
Compiler-Unions - node (v18.15.0, x64)
Errors 34 34 ~ ~ ~ p=1.000 n=6
Symbols 62,390 62,390 ~ ~ ~ p=1.000 n=6
Types 50,395 50,395 ~ ~ ~ p=1.000 n=6
Memory used 193,034k (± 0.07%) 194,151k (± 0.95%) ~ 192,930k 196,581k p=1.000 n=6
Parse Time 1.31s (± 0.62%) 1.31s (± 0.62%) ~ 1.29s 1.31s p=1.000 n=6
Bind Time 0.73s 0.73s ~ ~ ~ p=1.000 n=6
Check Time 9.76s (± 0.45%) 9.69s (± 0.36%) -0.08s (- 0.80%) 9.64s 9.73s p=0.020 n=6
Emit Time 2.73s (± 0.68%) 2.75s (± 0.84%) ~ 2.73s 2.78s p=0.292 n=6
Total Time 14.54s (± 0.42%) 14.48s (± 0.30%) ~ 14.42s 14.55s p=0.106 n=6
angular-1 - node (v18.15.0, x64)
Errors 56 56 ~ ~ ~ p=1.000 n=6
Symbols 949,240 949,240 ~ ~ ~ p=1.000 n=6
Types 411,065 411,065 ~ ~ ~ p=1.000 n=6
Memory used 1,225,135k (± 0.01%) 1,225,081k (± 0.00%) ~ 1,225,051k 1,225,130k p=0.093 n=6
Parse Time 6.66s (± 0.61%) 6.63s (± 1.13%) ~ 6.53s 6.69s p=0.627 n=6
Bind Time 1.88s (± 0.22%) 1.88s (± 0.22%) ~ 1.88s 1.89s p=1.000 n=6
Check Time 31.88s (± 0.19%) 31.95s (± 0.27%) ~ 31.89s 32.12s p=0.196 n=6
Emit Time 15.20s (± 0.63%) 15.15s (± 0.29%) ~ 15.09s 15.21s p=0.686 n=6
Total Time 55.62s (± 0.28%) 55.61s (± 0.29%) ~ 55.45s 55.88s p=0.810 n=6
mui-docs - node (v18.15.0, x64)
Errors 0 0 ~ ~ ~ p=1.000 n=6
Symbols 2,439,920 2,439,920 ~ ~ ~ p=1.000 n=6
Types 872,553 872,553 ~ ~ ~ p=1.000 n=6
Memory used 2,448,513k (± 0.00%) 2,448,450k (± 0.00%) ~ 2,448,361k 2,448,501k p=0.066 n=6
Parse Time 10.51s (± 0.57%) 10.53s (± 0.37%) ~ 10.46s 10.57s p=0.470 n=6
Bind Time 2.67s (± 1.03%) 2.68s (± 0.41%) ~ 2.67s 2.69s p=0.498 n=6
Check Time 87.44s (± 1.55%) 87.35s (± 1.56%) ~ 86.20s 89.50s p=0.689 n=6
Emit Time 0.37s (± 1.71%) 0.37s (± 2.04%) ~ 0.36s 0.38s p=0.718 n=6
Total Time 100.99s (± 1.34%) 100.93s (± 1.36%) ~ 99.72s 103.10s p=0.748 n=6
self-build-src - node (v18.15.0, x64)
Errors 0 0 ~ ~ ~ p=1.000 n=6
Symbols 1,228,052 1,228,055 +3 (+ 0.00%) ~ ~ p=0.001 n=6
Types 267,262 267,261 -1 (- 0.00%) ~ ~ p=0.001 n=6
Memory used 2,360,628k (± 0.02%) 2,361,074k (± 0.03%) ~ 2,360,348k 2,361,978k p=0.173 n=6
Parse Time 5.24s (± 0.79%) 5.20s (± 0.35%) ~ 5.18s 5.22s p=0.109 n=6
Bind Time 1.80s (± 0.65%) 1.80s (± 0.61%) ~ 1.78s 1.81s p=0.557 n=6
Check Time 35.36s (± 0.43%) 35.24s (± 0.24%) ~ 35.10s 35.33s p=0.173 n=6
Emit Time 3.03s (± 3.16%) 2.95s (± 1.92%) ~ 2.87s 3.03s p=0.128 n=6
Total Time 45.43s (± 0.43%) 45.20s (± 0.27%) ~ 45.00s 45.34s p=0.054 n=6
self-build-src-public-api - node (v18.15.0, x64)
Errors 0 0 ~ ~ ~ p=1.000 n=6
Symbols 1,228,052 1,228,055 +3 (+ 0.00%) ~ ~ p=0.001 n=6
Types 267,262 267,261 -1 (- 0.00%) ~ ~ p=0.001 n=6
Memory used 2,427,516k (± 0.03%) 2,427,798k (± 0.02%) ~ 2,427,085k 2,428,457k p=0.575 n=6
Parse Time 5.46s (± 1.25%) 5.42s (± 0.23%) ~ 5.41s 5.44s p=1.000 n=6
Bind Time 1.82s (± 0.92%) 1.82s (± 0.69%) ~ 1.80s 1.83s p=1.000 n=6
Check Time 35.48s (± 0.78%) 35.25s (± 0.20%) ~ 35.17s 35.37s p=0.261 n=6
Emit Time 3.10s (± 1.89%) 3.02s (± 1.56%) ~ 2.94s 3.06s p=0.076 n=6
Total Time 45.85s (± 0.71%) 45.52s (± 0.16%) ~ 45.44s 45.65s p=0.093 n=6
self-compiler - node (v18.15.0, x64)
Errors 0 0 ~ ~ ~ p=1.000 n=6
Symbols 263,419 263,422 +3 (+ 0.00%) ~ ~ p=0.001 n=6
Types 107,098 107,097 -1 (- 0.00%) ~ ~ p=0.001 n=6
Memory used 441,781k (± 0.02%) 441,837k (± 0.02%) ~ 441,767k 441,915k p=0.298 n=6
Parse Time 3.55s (± 0.99%) 3.55s (± 1.09%) ~ 3.50s 3.59s p=0.935 n=6
Bind Time 1.32s (± 0.63%) 1.33s (± 1.11%) ~ 1.31s 1.35s p=0.797 n=6
Check Time 19.06s (± 0.38%) 18.96s (± 0.46%) ~ 18.84s 19.06s p=0.078 n=6
Emit Time 1.53s (± 1.41%) 1.51s (± 0.97%) ~ 1.50s 1.54s p=0.217 n=6
Total Time 25.46s (± 0.25%) 25.35s (± 0.38%) -0.11s (- 0.45%) 25.20s 25.45s p=0.036 n=6
ts-pre-modules - node (v18.15.0, x64)
Errors 71 71 ~ ~ ~ p=1.000 n=6
Symbols 225,981 225,981 ~ ~ ~ p=1.000 n=6
Types 94,356 94,356 ~ ~ ~ p=1.000 n=6
Memory used 371,258k (± 0.01%) 371,331k (± 0.04%) ~ 371,173k 371,570k p=0.471 n=6
Parse Time 2.92s (± 0.73%) 2.90s (± 1.23%) ~ 2.86s 2.94s p=0.256 n=6
Bind Time 1.60s (± 1.00%) 1.61s (± 1.14%) ~ 1.59s 1.64s p=0.324 n=6
Check Time 16.50s (± 0.37%) 16.44s (± 0.36%) ~ 16.38s 16.52s p=0.172 n=6
Emit Time 0.00s (±244.70%) 0.00s ~ ~ ~ p=0.405 n=6
Total Time 21.02s (± 0.41%) 20.95s (± 0.40%) ~ 20.85s 21.08s p=0.228 n=6
vscode - node (v18.15.0, x64)
Errors 51 51 ~ ~ ~ p=1.000 n=6
Symbols 3,427,986 3,427,986 ~ ~ ~ p=1.000 n=6
Types 1,156,177 1,156,177 ~ ~ ~ p=1.000 n=6
Memory used 3,477,216k (± 0.01%) 3,477,330k (± 0.00%) ~ 3,477,123k 3,477,494k p=0.298 n=6
Parse Time 14.66s (± 0.32%) 14.74s (± 1.03%) ~ 14.62s 15.04s p=0.293 n=6
Bind Time 4.80s (± 1.06%) 4.76s (± 0.54%) ~ 4.74s 4.80s p=0.122 n=6
Check Time 94.61s (± 2.47%) 94.94s (± 2.81%) ~ 93.40s 100.35s p=0.575 n=6
Emit Time 30.48s (± 2.29%) 30.11s (± 2.20%) ~ 29.58s 31.43s p=0.108 n=6
Total Time 144.56s (± 2.07%) 144.55s (± 2.29%) ~ 142.86s 151.31s p=0.936 n=6
webpack - node (v18.15.0, x64)
Errors 2 2 ~ ~ ~ p=1.000 n=6
Symbols 317,848 317,848 ~ ~ ~ p=1.000 n=6
Types 140,485 140,485 ~ ~ ~ p=1.000 n=6
Memory used 473,035k (± 0.01%) 473,047k (± 0.04%) ~ 472,896k 473,370k p=0.471 n=6
Parse Time 5.17s (± 0.50%) 5.17s (± 0.87%) ~ 5.12s 5.25s p=1.000 n=6
Bind Time 2.26s (± 2.29%) 2.27s (± 1.16%) ~ 2.23s 2.31s p=1.000 n=6
Check Time 25.99s (± 0.29%) 25.93s (± 0.36%) ~ 25.80s 26.03s p=0.378 n=6
Emit Time 0.00s (±244.70%) 0.00s ~ ~ ~ p=0.405 n=6
Total Time 33.42s (± 0.28%) 33.37s (± 0.37%) ~ 33.21s 33.56s p=0.471 n=6
xstate-main - node (v18.15.0, x64)
Errors 5 5 ~ ~ ~ p=1.000 n=6
Symbols 570,733 570,733 ~ ~ ~ p=1.000 n=6
Types 191,446 191,446 ~ ~ ~ p=1.000 n=6
Memory used 501,157k (± 0.04%) 501,292k (± 0.01%) ~ 501,189k 501,350k p=0.298 n=6
Parse Time 4.31s (± 0.41%) 4.31s (± 0.32%) ~ 4.30s 4.34s p=0.934 n=6
Bind Time 1.53s (± 0.96%) 1.54s (± 0.71%) ~ 1.53s 1.56s p=0.452 n=6
Check Time 25.74s (± 3.60%) 24.85s (± 0.26%) ~ 24.78s 24.95s p=0.065 n=6
Emit Time 0.00s 0.00s ~ ~ ~ p=1.000 n=6
Total Time 31.58s (± 2.87%) 30.71s (± 0.18%) -0.88s (- 2.78%) 30.66s 30.81s p=0.037 n=6
System info unknown
Hosts
  • node (v18.15.0, x64)
Scenarios
  • Compiler-Unions - node (v18.15.0, x64)
  • angular-1 - node (v18.15.0, x64)
  • mui-docs - node (v18.15.0, x64)
  • self-build-src - node (v18.15.0, x64)
  • self-build-src-public-api - node (v18.15.0, x64)
  • self-compiler - node (v18.15.0, x64)
  • ts-pre-modules - node (v18.15.0, x64)
  • vscode - node (v18.15.0, x64)
  • webpack - node (v18.15.0, x64)
  • xstate-main - node (v18.15.0, x64)
Benchmark Name Iterations
Current pr 6
Baseline baseline 6

Developer Information:

Download Benchmarks

@typescript-bot
Copy link
Collaborator

@jakebailey Here are the results of running the top 400 repos with tsc comparing main and refs/pull/61571/merge:

Everything looks good!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

For Backlog Bug PRs that fix a backlog bug

Projects

Status: Not started

Development

Successfully merging this pull request may close these issues.

Arithmetic operations on type-parameters subtyping bigint are incorrectly inferred to have type number

3 participants