Skip to content
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.

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

Fixed an accidental undefined leak into getAccessibleSymbolChain's cache key #58669

Merged

Conversation

Andarist
Copy link
Contributor

No description provided.

@typescript-bot typescript-bot added the For Uncommitted Bug PR for untriaged, rejected, closed or missing bug label May 27, 2024
@typescript-bot
Copy link
Collaborator

This PR doesn't have any linked issues. Please open an issue that references this PR. From there we can discuss and prioritise.

@@ -5473,7 +5473,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
const cache = (links.accessibleChainCache ||= new Map());
// Go from enclosingDeclaration to the first scope we check, so the cache is keyed off the scope and thus shared more
const firstRelevantLocation = forEachSymbolTableInScope(enclosingDeclaration, (_, __, ___, node) => node);
const key = `${useOnlyExternalAliasing ? 0 : 1}|${firstRelevantLocation && getNodeId(firstRelevantLocation)}|${meaning}`;
const key = `${useOnlyExternalAliasing ? 0 : 1}|${firstRelevantLocation ? getNodeId(firstRelevantLocation) : 0}|${meaning}`;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I noticed that those keys could look smth like '0|undefined|788968' (I spotted an instance of that in the debugger). It bugged me enough to create this PR ;p

0 should be a safe fallback here as the lowest possible node id is 1

@jakebailey
Copy link
Member

@typescript-bot test it

Double checking this isn't depended on accidentally.

@typescript-bot
Copy link
Collaborator

typescript-bot commented Oct 31, 2024

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

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 Here are the results of running the user tests with tsc comparing main and refs/pull/58669/merge:

Everything looks good!

@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 31 31 ~ ~ ~ p=1.000 n=6
Symbols 62,340 62,340 ~ ~ ~ p=1.000 n=6
Types 50,379 50,379 ~ ~ ~ p=1.000 n=6
Memory used 194,617k (± 1.03%) 192,795k (± 0.01%) ~ 192,777k 192,808k p=0.229 n=6
Parse Time 1.31s (± 1.12%) 1.31s (± 0.62%) ~ 1.30s 1.32s p=1.000 n=6
Bind Time 0.72s (± 0.57%) 0.72s ~ ~ ~ p=0.405 n=6
Check Time 9.74s (± 0.35%) 9.75s (± 0.26%) ~ 9.72s 9.79s p=0.419 n=6
Emit Time 2.71s (± 1.39%) 2.71s (± 1.27%) ~ 2.64s 2.73s p=0.687 n=6
Total Time 14.48s (± 0.38%) 14.49s (± 0.34%) ~ 14.40s 14.55s p=0.742 n=6
angular-1 - node (v18.15.0, x64)
Errors 33 33 ~ ~ ~ p=1.000 n=6
Symbols 947,886 947,886 ~ ~ ~ p=1.000 n=6
Types 410,840 410,840 ~ ~ ~ p=1.000 n=6
Memory used 1,224,604k (± 0.00%) 1,224,610k (± 0.00%) ~ 1,224,562k 1,224,679k p=0.748 n=6
Parse Time 6.64s (± 0.73%) 6.65s (± 0.61%) ~ 6.61s 6.72s p=0.687 n=6
Bind Time 1.89s (± 0.29%) 1.88s (± 0.27%) ~ 1.88s 1.89s p=0.640 n=6
Check Time 31.88s (± 0.48%) 31.87s (± 0.33%) ~ 31.70s 31.99s p=0.810 n=6
Emit Time 15.23s (± 0.43%) 15.22s (± 0.40%) ~ 15.15s 15.31s p=1.000 n=6
Total Time 55.63s (± 0.22%) 55.63s (± 0.30%) ~ 55.40s 55.82s p=1.000 n=6
mui-docs - node (v18.15.0, x64)
Errors 0 0 ~ ~ ~ p=1.000 n=6
Symbols 2,495,026 2,495,026 ~ ~ ~ p=1.000 n=6
Types 908,645 908,645 ~ ~ ~ p=1.000 n=6
Memory used 2,307,184k (± 0.00%) 2,307,204k (± 0.00%) ~ 2,307,173k 2,307,233k p=0.378 n=6
Parse Time 9.32s (± 0.25%) 9.31s (± 0.20%) ~ 9.28s 9.33s p=0.683 n=6
Bind Time 2.14s (± 0.39%) 2.14s (± 0.19%) ~ 2.13s 2.14s p=0.527 n=6
Check Time 75.07s (± 0.57%) 75.20s (± 0.43%) ~ 74.75s 75.65s p=0.689 n=6
Emit Time 0.28s 0.28s (± 1.47%) ~ 0.27s 0.28s p=0.405 n=6
Total Time 86.81s (± 0.49%) 86.93s (± 0.38%) ~ 86.49s 87.39s p=0.748 n=6
self-build-src - node (v18.15.0, x64)
Errors 0 0 ~ ~ ~ p=1.000 n=6
Symbols 1,258,087 1,258,087 ~ ~ ~ p=1.000 n=6
Types 266,235 266,235 ~ ~ ~ p=1.000 n=6
Memory used 2,422,687k (± 0.03%) 2,422,754k (± 0.01%) ~ 2,422,385k 2,423,074k p=0.689 n=6
Parse Time 5.20s (± 0.77%) 5.20s (± 0.86%) ~ 5.14s 5.28s p=1.000 n=6
Bind Time 1.93s (± 0.61%) 1.92s (± 0.63%) ~ 1.91s 1.94s p=0.618 n=6
Check Time 35.47s (± 0.42%) 35.46s (± 0.26%) ~ 35.37s 35.60s p=0.936 n=6
Emit Time 3.00s (± 1.37%) 3.07s (± 4.20%) ~ 2.99s 3.33s p=0.336 n=6
Total Time 45.60s (± 0.37%) 45.66s (± 0.30%) ~ 45.48s 45.83s p=0.378 n=6
self-build-src-public-api - node (v18.15.0, x64)
Errors 0 0 ~ ~ ~ p=1.000 n=6
Symbols 1,258,087 1,258,087 ~ ~ ~ p=1.000 n=6
Types 266,235 266,235 ~ ~ ~ p=1.000 n=6
Memory used 2,620,075k (±11.19%) 2,526,611k (± 2.55%) ~ 2,498,240k 2,658,108k p=0.810 n=6
Parse Time 6.61s (± 2.71%) 6.59s (± 0.89%) ~ 6.52s 6.68s p=0.521 n=6
Bind Time 2.17s (± 0.35%) 2.17s (± 1.07%) ~ 2.14s 2.20s p=0.628 n=6
Check Time 43.14s (± 0.23%) 43.20s (± 0.70%) ~ 42.69s 43.44s p=0.336 n=6
Emit Time 3.58s (± 5.64%) 3.52s (± 6.45%) ~ 3.34s 3.97s p=0.297 n=6
Total Time 55.49s (± 0.58%) 55.51s (± 0.72%) ~ 55.03s 56.22s p=0.688 n=6
self-compiler - node (v18.15.0, x64)
Errors 0 0 ~ ~ ~ p=1.000 n=6
Symbols 261,754 261,754 ~ ~ ~ p=1.000 n=6
Types 106,477 106,477 ~ ~ ~ p=1.000 n=6
Memory used 438,832k (± 0.01%) 438,858k (± 0.01%) ~ 438,785k 438,905k p=0.298 n=6
Parse Time 3.54s (± 0.58%) 3.53s (± 0.72%) ~ 3.49s 3.56s p=0.416 n=6
Bind Time 1.31s (± 1.31%) 1.32s (± 0.75%) ~ 1.31s 1.33s p=0.673 n=6
Check Time 19.00s (± 0.41%) 18.95s (± 0.26%) ~ 18.89s 19.01s p=0.199 n=6
Emit Time 1.53s (± 0.89%) 1.54s (± 1.54%) ~ 1.50s 1.57s p=0.459 n=6
Total Time 25.39s (± 0.32%) 25.34s (± 0.24%) ~ 25.24s 25.42s p=0.332 n=6
ts-pre-modules - node (v18.15.0, x64)
Errors 68 68 ~ ~ ~ p=1.000 n=6
Symbols 225,919 225,919 ~ ~ ~ p=1.000 n=6
Types 94,415 94,415 ~ ~ ~ p=1.000 n=6
Memory used 371,075k (± 0.01%) 371,041k (± 0.01%) ~ 370,998k 371,076k p=0.093 n=6
Parse Time 2.90s (± 1.21%) 2.91s (± 0.60%) ~ 2.88s 2.93s p=0.747 n=6
Bind Time 1.59s (± 0.69%) 1.60s (± 2.36%) ~ 1.55s 1.64s p=0.935 n=6
Check Time 16.39s (± 0.21%) 16.40s (± 0.32%) ~ 16.34s 16.47s p=0.687 n=6
Emit Time 0.00s 0.00s ~ ~ ~ p=1.000 n=6
Total Time 20.88s (± 0.30%) 20.91s (± 0.37%) ~ 20.78s 20.98s p=0.422 n=6
vscode - node (v18.15.0, x64)
Errors 3 3 ~ ~ ~ p=1.000 n=6
Symbols 3,130,033 3,130,033 ~ ~ ~ p=1.000 n=6
Types 1,079,008 1,079,008 ~ ~ ~ p=1.000 n=6
Memory used 3,219,013k (± 0.01%) 3,219,240k (± 0.01%) ~ 3,218,627k 3,219,475k p=0.230 n=6
Parse Time 14.01s (± 0.25%) 14.03s (± 0.63%) ~ 13.95s 14.15s p=0.678 n=6
Bind Time 4.47s (± 2.09%) 4.54s (± 2.68%) ~ 4.43s 4.68s p=0.744 n=6
Check Time 86.18s (± 1.85%) 86.72s (± 3.23%) ~ 84.20s 91.42s p=1.000 n=6
Emit Time 25.80s (± 6.80%) 25.29s (± 8.65%) ~ 22.52s 27.17s p=0.810 n=6
Total Time 130.46s (± 2.31%) 130.58s (± 2.92%) ~ 126.16s 136.98s p=1.000 n=6
webpack - node (v18.15.0, x64)
Errors 0 0 ~ ~ ~ p=1.000 n=6
Symbols 287,028 287,028 ~ ~ ~ p=1.000 n=6
Types 116,316 116,316 ~ ~ ~ p=1.000 n=6
Memory used 438,455k (± 0.02%) 438,358k (± 0.02%) ~ 438,281k 438,441k p=0.128 n=6
Parse Time 4.07s (± 0.71%) 4.07s (± 0.50%) ~ 4.04s 4.09s p=0.870 n=6
Bind Time 1.75s (± 0.59%) 1.75s (± 1.64%) ~ 1.71s 1.79s p=0.739 n=6
Check Time 18.55s (± 0.45%) 18.50s (± 0.37%) ~ 18.37s 18.55s p=0.260 n=6
Emit Time 0.00s 0.00s ~ ~ ~ p=1.000 n=6
Total Time 24.36s (± 0.36%) 24.33s (± 0.25%) ~ 24.22s 24.39s p=0.298 n=6
xstate-main - node (v18.15.0, x64)
Errors 3 3 ~ ~ ~ p=1.000 n=6
Symbols 543,130 543,130 ~ ~ ~ p=1.000 n=6
Types 181,889 181,889 ~ ~ ~ p=1.000 n=6
Memory used 485,535k (± 0.01%) 485,527k (± 0.01%) ~ 485,464k 485,609k p=0.810 n=6
Parse Time 4.17s (± 0.52%) 4.17s (± 0.54%) ~ 4.15s 4.21s p=1.000 n=6
Bind Time 1.46s (± 1.18%) 1.47s (± 0.61%) ~ 1.46s 1.48s p=0.215 n=6
Check Time 23.73s (± 0.35%) 23.71s (± 0.52%) ~ 23.59s 23.91s p=0.748 n=6
Emit Time 0.00s 0.00s ~ ~ ~ p=1.000 n=6
Total Time 29.36s (± 0.25%) 29.35s (± 0.42%) ~ 29.22s 29.54s p=0.748 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/58669/merge:

Everything looks good!

@jakebailey jakebailey merged commit 32513a7 into microsoft:main Oct 31, 2024
28 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
For Uncommitted Bug PR for untriaged, rejected, closed or missing bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants