Bailout early from `isFunctionObjectType` for evolving arrays #58049

Andarist · 2024-04-03T07:33:53Z

Andarist · 2024-04-03T07:45:53Z

src/compiler/checker.ts

@@ -27044,6 27044,9 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
 }

 function isFunctionObjectType(type: ObjectType): boolean {
+ if (getObjectFlags(type) & ObjectFlags.EvolvingArray) {


resolveStructuredTypeMembers is still going to crash if an evolving array manages to reach it. I concluded that it's still better - at least in this case - to bail out early. IIUC, it avoid "fixing" this evolving type without a strong reason to do so - so the evolving array can continue evolving in certain situations.

Are there other paths whereby an evolving array could reach resolveStructuredTypeMembers? This seems like an important limitation that needs to be checked, either multiple places or maybe within resolveStructuredTypeMembers itself.

Are there other paths whereby an evolving array could reach resolveStructuredTypeMembers?

None that we are aware of. Any call with an evolving array to that function would throw and I didn't see any other crashes reported there. It's of course not completely unlikely that there are other ways to end up with that crash.

This seems like an important limitation that needs to be checked, either multiple places or maybe within resolveStructuredTypeMembers itself.

There is already an assert there. The only thing that potentially could be improved is to switch the assert to some other logic that would handle evolving arrays gracefully. I don't know what that logic would be and I think if this crash is a rare one then this fix is good enough.

jakebailey · 2024-04-03T14:54:35Z

@typescript-bot test it

typescript-bot · 2024-04-03T14:54:37Z

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 · 2024-04-03T15:24:45Z

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

Everything looks the same!

You can check the log here.

typescript-bot · 2024-04-03T15:29:32Z

@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
_{Angular - node (v18.15.0, x64)}
_{Memory used}	_{295,702k (± 0.01%)}	_{295,708k (± 0.01%)}	_~	_295,676k	_295,745k	_{p=0.809 n=6}
_{Parse Time}	_{2.66s (± 0.31%)}	_{2.65s (± 0.55%)}	_~	_2.63s	_2.67s	_{p=0.591 n=6}
_{Bind Time}	_{0.83s (± 0.49%)}	_{0.83s (± 1.24%)}	_~	_0.82s	_0.85s	_{p=0.924 n=6}
_{Check Time}	_{8.28s (± 0.33%)}	_{8.26s (± 0.30%)}	_~	_8.24s	_8.31s	_{p=0.284 n=6}
_{Emit Time}	_{7.03s (± 0.21%)}	_{7.05s (± 0.31%)}	_~	_7.01s	_7.07s	_{p=0.089 n=6}
_{Total Time}	_{18.79s (± 0.13%)}	_{18.80s (± 0.18%)}	_~	_18.74s	_18.83s	_{p=0.570 n=6}
_{Compiler-Unions - node (v18.15.0, x64)}
_{Memory used}	_{192,679k (± 0.72%)}	_{192,722k (± 0.75%)}	_~	_192,000k	_195,671k	_{p=1.000 n=6}
_{Parse Time}	_{1.36s (± 1.20%)}	_{1.36s (± 1.23%)}	_~	_1.34s	_1.38s	_{p=1.000 n=6}
_{Bind Time}	_{0.73s (± 0.00%)}	_{0.73s (± 0.56%)}	_~	_0.72s	_0.73s	_{p=0.405 n=6}
_{Check Time}	_{9.56s (± 0.73%)}	_{9.52s (± 0.22%)}	_~	_9.50s	_9.55s	_{p=0.260 n=6}
_{Emit Time}	_{2.63s (± 0.29%)}	_{2.63s (± 0.40%)}	_~	_2.62s	_2.65s	_{p=0.273 n=6}
_{Total Time}	_{14.27s (± 0.51%)}	_{14.24s (± 0.17%)}	_~	_14.20s	_14.26s	_{p=0.573 n=6}
_{Monaco - node (v18.15.0, x64)}
_{Memory used}	_{347,424k (± 0.01%)}	_{347,432k (± 0.01%)}	_~	_347,406k	_347,453k	_{p=0.630 n=6}
_{Parse Time}	_{3.70s (± 0.43%)}	_{3.67s (± 1.55%)}	_~	_3.56s	_3.71s	_{p=0.331 n=6}
_{Bind Time}	_{1.38s (± 0.37%)}	_{1.38s (± 0.30%)}	_~	_1.37s	_1.38s	_{p=0.595 n=6}
_{Check Time}	_{10.18s (± 0.53%)}	_{10.21s (± 0.33%)}	_~	_10.17s	_10.26s	_{p=0.289 n=6}
_{Emit Time}	_{6.03s (± 0.53%)}	_{6.02s (± 0.09%)}	_~	_6.02s	_6.03s	_{p=0.866 n=6}
_{Total Time}	_{21.29s (± 0.24%)}	_{21.28s (± 0.34%)}	_~	_21.17s	_21.37s	_{p=0.872 n=6}
_{TFS - node (v18.15.0, x64)}
_{Memory used}	_{302,803k (± 0.01%)}	_{302,799k (± 0.01%)}	_~	_302,755k	_302,825k	_{p=1.000 n=6}
_{Parse Time}	_{2.98s (± 1.29%)}	_{2.96s (± 0.95%)}	_~	_2.92s	_3.00s	_{p=0.627 n=6}
_{Bind Time}	_{1.48s (± 0.99%)}	_{1.47s (± 0.55%)}	_~	_1.46s	_1.48s	_{p=0.230 n=6}
_{Check Time}	_{9.24s (± 0.46%)}	_{9.26s (± 0.24%)}	_~	_9.23s	_9.30s	_{p=0.370 n=6}
_{Emit Time}	_{5.32s (± 0.76%)}	_{5.31s (± 0.65%)}	_~	_5.28s	_5.37s	_{p=0.744 n=6}
_{Total Time}	_{19.01s (± 0.44%)}	_{19.01s (± 0.31%)}	_~	_18.93s	_19.09s	_{p=0.936 n=6}
_{material-ui - node (v18.15.0, x64)}
_{Memory used}	_{510,050k (± 0.01%)}	_{510,043k (± 0.01%)}	_~	_510,016k	_510,095k	_{p=0.575 n=6}
_{Parse Time}	_{2.66s (± 0.34%)}	_{2.67s (± 0.31%)}	_~	_2.65s	_2.67s	_{p=0.339 n=6}
_{Bind Time}	_{0.99s (± 1.74%)}	_{0.99s (± 1.11%)}	_~	_0.97s	_1.00s	_{p=0.933 n=6}
_{Check Time}	_{17.32s (± 0.40%)}	_{17.34s (± 0.32%)}	_~	_17.30s	_17.43s	_{p=0.936 n=6}
_{Emit Time}	_{0.00s (± 0.00%)}	_{0.00s (± 0.00%)}	_~	_0.00s	_0.00s	_{p=1.000 n=6}
_{Total Time}	_{20.97s (± 0.37%)}	_{21.00s (± 0.28%)}	_~	_20.95s	_21.09s	_{p=0.936 n=6}
_{mui-docs - node (v18.15.0, x64)}
_{Memory used}	_{1,744,969k (± 0.00%)}	_{1,744,962k (± 0.00%)}	_~	_1,744,920k	_1,744,988k	_{p=0.689 n=6}
_{Parse Time}	_{9.64s (± 0.70%)}	_{9.59s (± 0.29%)}	_~	_9.56s	_9.64s	_{p=0.226 n=6}
_{Bind Time}	_{3.44s (± 0.63%)}	_{3.45s (± 0.40%)}	_~	_3.43s	_3.46s	_{p=1.000 n=6}
_{Check Time}	_{81.64s (± 0.45%)}	_{81.74s (± 0.18%)}	_~	_81.53s	_81.89s	_{p=1.000 n=6}
_{Emit Time}	_{0.19s (± 2.67%)}	_{0.19s (± 2.13%)}	_~	_0.19s	_0.20s	_{p=0.595 n=6}
_{Total Time}	_{94.91s (± 0.37%)}	_{94.97s (± 0.17%)}	_~	_94.74s	_95.12s	_{p=0.810 n=6}
_{self-build-src - node (v18.15.0, x64)}
_{Memory used}	_{2,400,032k (± 0.04%)}	_{2,400,431k (± 0.04%)}	_~	_2,399,214k	_2,401,568k	_{p=0.689 n=6}
_{Parse Time}	_{6.03s (± 0.99%)}	_{6.01s (± 0.91%)}	_~	_5.94s	_6.08s	_{p=0.748 n=6}
_{Bind Time}	_{2.26s (± 1.23%)}	_{2.26s (± 0.33%)}	_~	_2.25s	_2.27s	_{p=0.624 n=6}
_{Check Time}	_{39.88s (± 0.18%)}	_{39.97s (± 0.54%)}	_~	_39.69s	_40.24s	_{p=0.748 n=6}
_{Emit Time}	_{3.19s (± 1.60%)}	_{3.26s (± 3.71%)}	_~	_3.15s	_3.49s	_{p=0.261 n=6}
_{Total Time}	_{51.36s (± 0.15%)}	_{51.50s (± 0.59%)}	_~	_51.09s	_51.90s	_{p=0.378 n=6}
_{self-compiler - node (v18.15.0, x64)}
_{Memory used}	_{416,123k (± 0.00%)}	_{416,146k (± 0.01%)}	_~	_416,092k	_416,232k	_{p=0.873 n=6}
_{Parse Time}	_{2.77s (± 0.59%)}	_{2.75s (± 0.70%)}	_~	_2.73s	_2.78s	_{p=0.251 n=6}
_{Bind Time}	_{1.08s (± 0.76%)}	_{1.08s (± 0.76%)}	_~	_1.07s	_1.09s	_{p=0.787 n=6}
_{Check Time}	_{15.41s (± 0.12%)}	_{15.38s (± 0.41%)}	_~	_15.31s	_15.46s	_{p=0.747 n=6}
_{Emit Time}	_{1.11s (± 0.94%)}	_{1.13s (± 2.15%)}	_~	_1.10s	_1.17s	_{p=0.413 n=6}
_{Total Time}	_{20.37s (± 0.08%)}	_{20.33s (± 0.36%)}	_~	_20.23s	_20.42s	_{p=0.462 n=6}
_{vscode - node (v18.15.0, x64)}
_{Memory used}	_{2,900,631k (± 0.00%)}	_{2,900,624k (± 0.00%)}	_~	_2,900,520k	_2,900,664k	_{p=0.521 n=6}
_{Parse Time}	_{15.95s (± 0.56%)}	_{15.97s (± 0.34%)}	_~	_15.92s	_16.07s	_{p=1.000 n=6}
_{Bind Time}	_{5.07s (± 0.51%)}	_{5.05s (± 0.44%)}	_~	_5.02s	_5.08s	_{p=0.294 n=6}
_{Check Time}	_{87.99s (± 0.53%)}	_{87.87s (± 0.68%)}	_~	_87.12s	_88.83s	_{p=0.810 n=6}
_{Emit Time}	_{24.45s (± 7.81%)}	_{25.31s (± 9.42%)}	_~	_23.60s	_28.44s	_{p=0.336 n=6}
_{Total Time}	_{133.47s (± 1.49%)}	_{134.19s (± 2.13%)}	_~	_131.87s	_138.23s	_{p=0.936 n=6}
_{webpack - node (v18.15.0, x64)}
_{Memory used}	_{408,999k (± 0.02%)}	_{408,957k (± 0.02%)}	_~	_408,807k	_409,104k	_{p=0.471 n=6}
_{Parse Time}	_{4.81s (± 0.78%)}	_{4.81s (± 0.83%)}	_~	_4.75s	_4.86s	_{p=0.935 n=6}
_{Bind Time}	_{2.06s (± 0.51%)}	_{2.06s (± 0.81%)}	_~	_2.04s	_2.09s	_{p=0.405 n=6}
_{Check Time}	_{20.91s (± 0.29%)}	_{20.91s (± 0.56%)}	_~	_20.79s	_21.10s	_{p=0.810 n=6}
_{Emit Time}	_{0.00s (± 0.00%)}	_{0.00s (± 0.00%)}	_~	_0.00s	_0.00s	_{p=1.000 n=6}
_{Total Time}	_{27.79s (± 0.31%)}	_{27.78s (± 0.42%)}	_~	_27.64s	_27.96s	_{p=0.936 n=6}
_{xstate - node (v18.15.0, x64)}
_{Memory used}	_{513,586k (± 0.03%)}	_{513,534k (± 0.02%)}	_~	_513,417k	_513,653k	_{p=0.471 n=6}
_{Parse Time}	_{4.90s (± 0.65%)}	_{4.89s (± 0.95%)}	_~	_4.80s	_4.93s	_{p=0.805 n=6}
_{Bind Time}	_{2.31s (± 1.06%)}	_{2.29s (± 0.75%)}	_~	_2.27s	_2.32s	_{p=0.163 n=6}
_{Check Time}	_{4.31s (± 1.05%)}	_{4.31s (± 0.50%)}	_~	_4.29s	_4.35s	_{p=0.373 n=6}
_{Emit Time}	_{0.11s (± 0.00%)}	_{0.11s (± 0.00%)}	_~	_0.11s	_0.11s	_{p=1.000 n=6}
_{Total Time}	_{11.62s (± 0.58%)}	_{11.60s (± 0.34%)}	_~	_11.52s	_11.62s	_{p=0.630 n=6}

System info unknown

Hosts

node (v18.15.0, x64)

Scenarios

Angular - node (v18.15.0, x64)
Compiler-Unions - node (v18.15.0, x64)
Monaco - node (v18.15.0, x64)
TFS - node (v18.15.0, x64)
material-ui - node (v18.15.0, x64)
mui-docs - node (v18.15.0, x64)
self-build-src - node (v18.15.0, x64)
self-compiler - node (v18.15.0, x64)
vscode - node (v18.15.0, x64)
webpack - node (v18.15.0, x64)
xstate - node (v18.15.0, x64)

_Benchmark	_Name	_Iterations
_Current	_pr	₆
_Baseline	_baseline	₆

Developer Information:

Download Benchmarks

typescript-bot · 2024-04-03T15:32:27Z

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

Everything looks good!

typescript-bot · 2024-04-03T17:04:15Z

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

Everything looks good!

Bailout early from isFunctionObjectType for evolving arrays

6f29512

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

add extra test case

282b2ba

Andarist commented Apr 3, 2024

View reviewed changes

Merge remote-tracking branch 'origin/main' into fix/evolving-crash

31f2b7d

Andarist force-pushed the fix/evolving-crash branch from aa1e2df to 31f2b7d Compare May 1, 2024 22:26

sandersn requested review from sandersn and gabritto June 12, 2024 21:22

sandersn assigned gabritto Jun 12, 2024

DanielRosenwasser requested a review from andrewbranch July 18, 2024 17:33

andrewbranch approved these changes Jul 23, 2024

View reviewed changes

jakebailey approved these changes Jul 25, 2024

View reviewed changes

jakebailey added 2 commits July 25, 2024 16:46

Merge branch 'main' into fix/evolving-crash

d4fa10a

Fix baselines

6d4429e

jakebailey merged commit 1da9630 into microsoft:main Jul 26, 2024
32 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bailout early from `isFunctionObjectType` for evolving arrays #58049

Bailout early from `isFunctionObjectType` for evolving arrays #58049

Andarist commented Apr 3, 2024

Andarist Apr 3, 2024

sandersn Jun 12, 2024

Andarist Jun 12, 2024

jakebailey commented Apr 3, 2024

typescript-bot commented Apr 3, 2024 •

edited

Loading

typescript-bot commented Apr 3, 2024

typescript-bot commented Apr 3, 2024

tsc

typescript-bot commented Apr 3, 2024

typescript-bot commented Apr 3, 2024

Bailout early from isFunctionObjectType for evolving arrays #58049

Bailout early from isFunctionObjectType for evolving arrays #58049

Conversation

Andarist commented Apr 3, 2024

Andarist Apr 3, 2024

Choose a reason for hiding this comment

sandersn Jun 12, 2024

Choose a reason for hiding this comment

Andarist Jun 12, 2024

Choose a reason for hiding this comment

jakebailey commented Apr 3, 2024

typescript-bot commented Apr 3, 2024 • edited Loading

typescript-bot commented Apr 3, 2024

typescript-bot commented Apr 3, 2024

tsc

typescript-bot commented Apr 3, 2024

typescript-bot commented Apr 3, 2024

Bailout early from `isFunctionObjectType` for evolving arrays #58049

Bailout early from `isFunctionObjectType` for evolving arrays #58049

typescript-bot commented Apr 3, 2024 •

edited

Loading