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

Check if for-of iterated element types are disposable when using using declarations #60354

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

Andarist
Copy link
Contributor

fixes #60348

cc @rbuckton

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

@typescript-bot test it

(not that there are a major number of these in the wild, or even using in general yet)

@jakebailey
Copy link
Member

@typescript-bot test it

@typescript-bot
Copy link
Collaborator

typescript-bot commented Oct 28, 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

@jakebailey Here are the results of running the user tests with tsc comparing main and refs/pull/60354/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 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 195,832k (± 0.72%) 196,434k (± 0.08%) ~ 196,140k 196,588k p=0.575 n=6
Parse Time 1.61s (± 1.20%) 1.58s (± 1.77%) ~ 1.53s 1.61s p=0.076 n=6
Bind Time 0.87s (± 1.74%) 0.87s (± 0.63%) ~ 0.86s 0.87s p=0.868 n=6
Check Time 11.73s (± 0.18%) 11.76s (± 0.44%) ~ 11.66s 11.80s p=0.077 n=6
Emit Time 3.34s (± 2.27%) 3.32s (± 3.27%) ~ 3.24s 3.54s p=0.414 n=6
Total Time 17.54s (± 0.38%) 17.54s (± 0.58%) ~ 17.44s 17.73s p=0.688 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,798k (± 0.01%) 1,224,795k (± 0.00%) ~ 1,224,685k 1,224,852k p=0.810 n=6
Parse Time 8.09s (± 0.81%) 8.11s (± 1.09%) ~ 7.98s 8.19s p=0.228 n=6
Bind Time 2.28s (± 0.73%) 2.27s (± 1.13%) ~ 2.24s 2.31s p=0.746 n=6
Check Time 37.85s (± 0.30%) 37.94s (± 0.32%) ~ 37.76s 38.13s p=0.173 n=6
Emit Time 18.29s (± 0.44%) 18.29s (± 0.52%) ~ 18.20s 18.42s p=0.810 n=6
Total Time 66.51s (± 0.17%) 66.63s (± 0.29%) ~ 66.43s 66.98s p=0.228 n=6
mui-docs - node (v18.15.0, x64)
Errors 0 0 ~ ~ ~ p=1.000 n=6
Symbols 2,494,603 2,494,603 ~ ~ ~ p=1.000 n=6
Types 908,300 908,300 ~ ~ ~ p=1.000 n=6
Memory used 2,307,415k (± 0.01%) 2,307,320k (± 0.01%) ~ 2,307,126k 2,307,467k p=0.230 n=6
Parse Time 11.19s (± 0.67%) 11.20s (± 0.35%) ~ 11.15s 11.25s p=0.936 n=6
Bind Time 2.61s (± 0.71%) 2.64s (± 0.88%) ~ 2.60s 2.66s p=0.105 n=6
Check Time 90.84s (± 1.70%) 91.47s (± 1.97%) ~ 89.06s 93.79s p=0.689 n=6
Emit Time 0.35s (± 1.17%) 0.35s (± 2.14%) ~ 0.34s 0.36s p=0.389 n=6
Total Time 104.99s (± 1.53%) 105.66s (± 1.70%) ~ 103.24s 107.92s p=0.689 n=6
self-build-src - node (v18.15.0, x64)
Errors 0 0 ~ ~ ~ p=1.000 n=6
Symbols 1,258,044 1,258,086 +42 (+ 0.00%) ~ ~ p=0.001 n=6
Types 266,229 266,247 +18 (+ 0.01%) ~ ~ p=0.001 n=6
Memory used 3,147,088k (± 0.03%) 3,147,144k (± 0.03%) ~ 3,145,928k 3,149,044k p=0.936 n=6
Parse Time 6.65s (± 1.14%) 6.65s (± 0.31%) ~ 6.62s 6.67s p=0.810 n=6
Bind Time 2.29s (± 4.02%) 2.33s (± 1.19%) ~ 2.29s 2.36s p=0.469 n=6
Check Time 43.07s (± 0.34%) 43.19s (± 0.34%) ~ 43.01s 43.35s p=0.336 n=6
Emit Time 3.48s (± 1.70%) 3.53s (± 1.43%) ~ 3.44s 3.57s p=0.128 n=6
Total Time 55.49s (± 0.19%) 55.69s (± 0.32%) ~ 55.41s 55.86s p=0.128 n=6
self-build-src-public-api - node (v18.15.0, x64)
Errors 0 0 ~ ~ ~ p=1.000 n=6
Symbols 1,258,044 1,258,086 +42 (+ 0.00%) ~ ~ p=0.001 n=6
Types 266,229 266,247 +18 (+ 0.01%) ~ ~ p=0.001 n=6
Memory used 2,672,426k (±10.44%) 2,620,000k (±11.19%) ~ 2,498,969k 3,218,977k p=0.378 n=6
Parse Time 8.29s (± 1.39%) 8.19s (± 1.64%) ~ 8.08s 8.45s p=0.091 n=6
Bind Time 2.67s (± 1.46%) 2.68s (± 2.37%) ~ 2.58s 2.77s p=0.748 n=6
Check Time 53.26s (± 0.50%) 53.68s (± 0.78%) ~ 53.35s 54.50s p=0.128 n=6
Emit Time 4.41s (± 2.18%) 4.41s (± 3.72%) ~ 4.25s 4.64s p=1.000 n=6
Total Time 68.63s (± 0.34%) 68.96s (± 0.78%) ~ 68.50s 69.90s p=0.230 n=6
self-compiler - node (v18.15.0, x64)
Errors 0 0 ~ ~ ~ p=1.000 n=6
Symbols 261,754 261,774 +20 (+ 0.01%) ~ ~ p=0.001 n=6
Types 106,477 106,493 +16 (+ 0.02%) ~ ~ p=0.001 n=6
Memory used 438,828k (± 0.01%) 438,925k (± 0.02%) +97k (+ 0.02%) 438,863k 439,061k p=0.013 n=6
Parse Time 4.38s (± 0.93%) 4.38s (± 1.06%) ~ 4.34s 4.45s p=1.000 n=6
Bind Time 1.61s (± 0.76%) 1.61s (± 0.47%) ~ 1.60s 1.62s p=0.555 n=6
Check Time 23.38s (± 0.34%) 23.48s (± 0.30%) ~ 23.39s 23.59s p=0.065 n=6
Emit Time 1.91s (± 0.66%) 1.91s (± 1.13%) ~ 1.89s 1.95s p=1.000 n=6
Total Time 31.29s (± 0.37%) 31.39s (± 0.40%) ~ 31.28s 31.60s p=0.128 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,141k (± 0.04%) 371,093k (± 0.01%) ~ 371,049k 371,143k p=0.688 n=6
Parse Time 2.89s (± 1.23%) 2.92s (± 1.35%) ~ 2.86s 2.98s p=0.745 n=6
Bind Time 1.59s (± 1.85%) 1.61s (± 1.22%) ~ 1.58s 1.64s p=0.370 n=6
Check Time 16.39s (± 0.33%) 16.35s (± 0.24%) ~ 16.31s 16.40s p=0.261 n=6
Emit Time 0.00s 0.00s ~ ~ ~ p=1.000 n=6
Total Time 20.88s (± 0.41%) 20.88s (± 0.27%) ~ 20.77s 20.92s p=0.872 n=6
vscode - node (v18.15.0, x64)
Errors 3 3 ~ ~ ~ p=1.000 n=6
Symbols 3,128,472 3,128,472 ~ ~ ~ p=1.000 n=6
Types 1,078,305 1,078,305 ~ ~ ~ p=1.000 n=6
Memory used 3,219,965k (± 0.01%) 3,219,985k (± 0.02%) ~ 3,219,338k 3,220,691k p=0.810 n=6
Parse Time 11.58s (± 0.46%) 11.60s (± 0.54%) ~ 11.51s 11.67s p=0.685 n=6
Bind Time 3.73s (± 2.05%) 3.70s (± 0.24%) ~ 3.69s 3.71s p=1.000 n=6
Check Time 72.03s (± 0.26%) 72.06s (± 0.32%) ~ 71.64s 72.29s p=0.748 n=6
Emit Time 22.98s (± 1.34%) 21.76s (± 9.57%) ~ 19.04s 23.30s p=0.810 n=6
Total Time 110.32s (± 0.33%) 109.12s (± 1.85%) ~ 106.43s 110.63s p=0.810 n=6
webpack - node (v18.15.0, x64)
Errors 0 0 ~ ~ ~ p=1.000 n=6
Symbols 286,866 286,866 ~ ~ ~ p=1.000 n=6
Types 116,245 116,245 ~ ~ ~ p=1.000 n=6
Memory used 437,893k (± 0.01%) 437,926k (± 0.02%) ~ 437,824k 438,044k p=0.378 n=6
Parse Time 3.29s (± 0.46%) 3.28s (± 0.54%) ~ 3.27s 3.31s p=0.866 n=6
Bind Time 1.46s (± 0.35%) 1.47s (± 0.35%) ~ 1.46s 1.47s p=0.311 n=6
Check Time 15.50s (± 0.56%) 15.53s (± 0.42%) ~ 15.46s 15.62s p=0.748 n=6
Emit Time 0.00s 0.00s ~ ~ ~ p=1.000 n=6
Total Time 20.25s (± 0.39%) 20.29s (± 0.36%) ~ 20.19s 20.37s p=0.521 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,546k (± 0.02%) 485,477k (± 0.02%) ~ 485,283k 485,548k p=0.378 n=6
Parse Time 3.37s (± 0.45%) 3.37s (± 0.55%) ~ 3.35s 3.40s p=0.744 n=6
Bind Time 1.18s (± 0.46%) 1.19s (± 0.75%) ~ 1.18s 1.20s p=0.341 n=6
Check Time 19.12s (± 0.35%) 19.25s (± 1.44%) ~ 19.08s 19.81s p=0.520 n=6
Emit Time 0.00s 0.00s ~ ~ ~ p=1.000 n=6
Total Time 23.67s (± 0.27%) 23.81s (± 1.24%) ~ 23.66s 24.41s p=0.467 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/60354/merge:

Everything looks good!

@@ -44058,6 +44058,34 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
return type === autoType ? anyType : type === autoArrayType ? anyArrayType : type;
}

function checkIfUsingDisposables(declaration: Declaration, type: Type) {
Copy link
Member

Choose a reason for hiding this comment

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

This check would be handled by checkVariableLikeDeclaration were it not for the fact that const initializer in that function results in undefined. It seems like we could fit this into that function as an else if case for if (initializer) to avoid repeating the same logic just for for..of.

Something like:

let errorNode: Node | undefined;
let initializerType: Type | undefined;
if (initializer) {
  const isJSObjectLiteralInitializer = ...;
  if (!isJSObjectLiteralInitializer && node.parent.parent.kind !== SyntaxKind.ForInStatement) {
    errorNode = initializer;
    initializerType = checkExpressionCached(initializer);
    checkTypeAssignableToAndOptionallyElaborate(initializerType, type, node, initializer, /*headMessage*/ undefined);
  }
}
else if (node.parent.parent.kind === SyntaxKind.ForOfStatement) {
  errorNode = (node.parent.parent as ForOfStatement).expression;
  initializerType = checkRightHandSideOfForOf(node.parent.parent as ForOfStatement);
}
if (initializerType && errorNode) {
  const blockScopeKind = getCombinedNodeFlagsCached(node) & NodeFlags.BlockScoped;
  ...
}

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
Status: Waiting on author
Development

Successfully merging this pull request may close these issues.

Should validate iteree when using using/await using in loop initializer
4 participants