Fixed a regression with reporting unused parameters in potential predicates #58514

Andarist · 2024-05-13T08:00:45Z

…icates

jakebailey · 2024-05-13T21:37:20Z

typescript-bot · 2024-05-13T21:37:22Z

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-05-13T22:08:09Z

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

Everything looks the same!

You can check the log here.

typescript-bot · 2024-05-13T22:13:22Z

@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	₃₀	₃₀	_~	_~	_~	_{p=1.000 n=6}
_Symbols	_62,154	_62,154	_~	_~	_~	_{p=1.000 n=6}
_Types	_50,248	_50,248	_~	_~	_~	_{p=1.000 n=6}
_{Memory used}	_{194,058k (± 1.03%)}	_{193,425k (± 0.99%)}	_~	_192,164k	_195,959k	_{p=0.173 n=6}
_{Parse Time}	_{1.30s (± 0.69%)}	_{1.30s (± 1.57%)}	_~	_1.27s	_1.32s	_{p=1.000 n=6}
_{Bind Time}	_0.72s	_0.72s	_~	_~	_~	_{p=1.000 n=6}
_{Check Time}	_{9.55s (± 0.34%)}	_{9.54s (± 0.38%)}	_~	_9.48s	_9.58s	_{p=0.627 n=6}
_{Emit Time}	_{2.65s (± 0.44%)}	_{2.64s (± 1.01%)}	_~	_2.59s	_2.66s	_{p=0.236 n=6}
_{Total Time}	_{14.22s (± 0.22%)}	_{14.19s (± 0.36%)}	_~	_14.13s	_14.26s	_{p=0.377 n=6}
_{angular-1 - node (v18.15.0, x64)}
_Errors	₅	₅	_~	_~	_~	_{p=1.000 n=6}
_Symbols	_944,110	_944,110	_~	_~	_~	_{p=1.000 n=6}
_Types	_407,140	_407,140	_~	_~	_~	_{p=1.000 n=6}
_{Memory used}	_{1,222,106k (± 0.00%)}	_{1,222,086k (± 0.01%)}	_~	_1,222,021k	_1,222,196k	_{p=0.471 n=6}
_{Parse Time}	_{6.77s (± 0.59%)}	_{6.78s (± 0.62%)}	_~	_6.73s	_6.84s	_{p=1.000 n=6}
_{Bind Time}	_{1.87s (± 0.44%)}	_{1.88s (± 0.52%)}	_~	_1.87s	_1.89s	_{p=0.498 n=6}
_{Check Time}	_{31.15s (± 0.27%)}	_{31.26s (± 0.21%)}	_{+0.11s (+ 0.34%)}	_31.14s	_31.32s	_{p=0.045 n=6}
_{Emit Time}	_{14.72s (± 0.53%)}	_{14.80s (± 0.10%)}	_~	_14.78s	_14.82s	_{p=0.064 n=6}
_{Total Time}	_{54.51s (± 0.30%)}	_{54.72s (± 0.13%)}	_{+0.20s (+ 0.38%)}	_54.59s	_54.80s	_{p=0.013 n=6}
_{mui-docs - node (v18.15.0, x64)}
_Errors	₅	₅	_~	_~	_~	_{p=1.000 n=6}
_Symbols	_1,961,349	_1,961,349	_~	_~	_~	_{p=1.000 n=6}
_Types	_696,910	_696,910	_~	_~	_~	_{p=1.000 n=6}
_{Memory used}	_{1,778,083k (± 0.00%)}	_{1,778,093k (± 0.00%)}	_~	_1,778,052k	_1,778,114k	_{p=0.575 n=6}
_{Parse Time}	_{6.77s (± 0.18%)}	_{6.78s (± 0.29%)}	_~	_6.76s	_6.81s	_{p=0.797 n=6}
_{Bind Time}	_{2.30s (± 0.65%)}	_{2.30s (± 0.53%)}	_~	_2.29s	_2.32s	_{p=1.000 n=6}
_{Check Time}	_{56.95s (± 0.37%)}	_{57.10s (± 0.31%)}	_~	_56.85s	_57.34s	_{p=0.261 n=6}
_{Emit Time}	_0.14s	_0.14s	_~	_~	_~	_{p=1.000 n=6}
_{Total Time}	_{66.17s (± 0.32%)}	_{66.32s (± 0.27%)}	_~	_66.07s	_66.56s	_{p=0.261 n=6}
_{self-build-src - node (v18.15.0, x64)}
_Errors	₀	₀	_~	_~	_~	_{p=1.000 n=6}
_Symbols	_1,221,120	_1,221,121	_{+1 (+ 0.00%)}	_~	_~	_{p=0.001 n=6}
_Types	_259,503	_259,503	_~	_~	_~	_{p=1.000 n=6}
_{Memory used}	_{2,336,352k (± 0.03%)}	_{2,337,014k (± 0.04%)}	_~	_2,336,098k	_2,338,264k	_{p=0.173 n=6}
_{Parse Time}	_{5.04s (± 1.54%)}	_{5.05s (± 0.38%)}	_~	_5.03s	_5.08s	_{p=0.471 n=6}
_{Bind Time}	_{1.88s (± 0.64%)}	_{1.87s (± 0.99%)}	_~	_1.85s	_1.89s	_{p=0.934 n=6}
_{Check Time}	_{33.78s (± 0.73%)}	_{33.81s (± 0.21%)}	_~	_33.73s	_33.91s	_{p=0.748 n=6}
_{Emit Time}	_{2.68s (± 3.44%)}	_{2.63s (± 1.61%)}	_~	_2.57s	_2.69s	_{p=0.128 n=6}
_{Total Time}	_{43.41s (± 0.41%)}	_{43.38s (± 0.26%)}	_~	_43.25s	_43.56s	_{p=0.936 n=6}
_{self-build-src-public-api - node (v18.15.0, x64)}
_Errors	₀	₀	_~	_~	_~	_{p=1.000 n=6}
_Symbols	_1,221,120	_1,221,121	_{+1 (+ 0.00%)}	_~	_~	_{p=0.001 n=6}
_Types	_259,503	_259,503	_~	_~	_~	_{p=1.000 n=6}
_{Memory used}	_{2,412,625k (± 0.02%)}	_{2,413,238k (± 0.03%)}	_~	_2,412,040k	_2,414,007k	_{p=0.093 n=6}
_{Parse Time}	_{6.28s (± 1.24%)}	_{6.20s (± 1.64%)}	_~	_6.01s	_6.29s	_{p=0.173 n=6}
_{Bind Time}	_{2.04s (± 0.67%)}	_{2.06s (± 0.81%)}	_{+0.02s (+ 1.15%)}	_2.04s	_2.08s	_{p=0.043 n=6}
_{Check Time}	_{40.20s (± 0.29%)}	_{40.31s (± 0.42%)}	_~	_40.08s	_40.49s	_{p=0.336 n=6}
_{Emit Time}	_{3.18s (± 2.27%)}	_{3.11s (± 2.90%)}	_~	_3.00s	_3.26s	_{p=0.173 n=6}
_{Total Time}	_{51.71s (± 0.26%)}	_{51.68s (± 0.45%)}	_~	_51.31s	_51.91s	_{p=0.936 n=6}
_{self-compiler - node (v18.15.0, x64)}
_Errors	₀	₀	_~	_~	_~	_{p=1.000 n=6}
_Symbols	_256,716	_256,717	_{+1 (+ 0.00%)}	_~	_~	_{p=0.001 n=6}
_Types	_104,580	_104,580	_~	_~	_~	_{p=1.000 n=6}
_{Memory used}	_{425,973k (± 0.02%)}	_{425,883k (± 0.01%)}	_~	_425,813k	_425,966k	_{p=0.093 n=6}
_{Parse Time}	_{4.17s (± 0.58%)}	_{4.16s (± 0.42%)}	_~	_4.13s	_4.18s	_{p=0.168 n=6}
_{Bind Time}	_{1.62s (± 0.50%)}	_{1.63s (± 0.85%)}	_~	_1.61s	_1.65s	_{p=0.270 n=6}
_{Check Time}	_{22.15s (± 0.49%)}	_{22.11s (± 0.15%)}	_~	_22.06s	_22.15s	_{p=1.000 n=6}
_{Emit Time}	_{1.70s (± 1.05%)}	_{1.73s (± 1.08%)}	_~	_1.71s	_1.75s	_{p=0.056 n=6}
_{Total Time}	_{29.64s (± 0.36%)}	_{29.61s (± 0.17%)}	_~	_29.54s	_29.67s	_{p=0.936 n=6}
_{ts-pre-modules - node (v18.15.0, x64)}
_Errors	₃₅	₃₅	_~	_~	_~	_{p=1.000 n=6}
_Symbols	_224,575	_224,575	_~	_~	_~	_{p=1.000 n=6}
_Types	_93,785	_93,785	_~	_~	_~	_{p=1.000 n=6}
_{Memory used}	_{369,887k (± 0.02%)}	_{369,847k (± 0.04%)}	_~	_369,711k	_370,106k	_{p=0.230 n=6}
_{Parse Time}	_{2.85s (± 0.67%)}	_{2.84s (± 0.82%)}	_~	_2.81s	_2.88s	_{p=0.624 n=6}
_{Bind Time}	_{1.58s (± 1.01%)}	_{1.58s (± 0.80%)}	_~	_1.56s	_1.59s	_{p=0.869 n=6}
_{Check Time}	_{15.64s (± 0.44%)}	_{15.64s (± 0.24%)}	_~	_15.60s	_15.70s	_{p=1.000 n=6}
_{Emit Time}	_0.00s	_0.00s	_~	_~	_~	_{p=1.000 n=6}
_{Total Time}	_{20.07s (± 0.41%)}	_{20.06s (± 0.24%)}	_~	_19.99s	_20.14s	_{p=1.000 n=6}
_{vscode - node (v18.15.0, x64)}
_Errors	₀	₀	_~	_~	_~	_{p=1.000 n=6}
_Symbols	_2,822,415	_2,822,415	_~	_~	_~	_{p=1.000 n=6}
_Types	_957,541	_957,541	_~	_~	_~	_{p=1.000 n=6}
_{Memory used}	_{2,994,941k (± 0.00%)}	_{2,995,024k (± 0.00%)}	_~	_2,994,981k	_2,995,122k	_{p=0.128 n=6}
_{Parse Time}	_{13.82s (± 0.19%)}	_{13.82s (± 0.13%)}	_~	_13.79s	_13.84s	_{p=0.508 n=6}
_{Bind Time}	_{4.14s (± 0.33%)}	_{4.14s (± 0.12%)}	_~	_4.14s	_4.15s	_{p=0.928 n=6}
_{Check Time}	_{73.44s (± 0.46%)}	_{73.44s (± 0.43%)}	_~	_73.06s	_74.02s	_{p=0.936 n=6}
_{Emit Time}	_{23.61s (± 0.45%)}	_{23.47s (± 0.48%)}	_~	_23.26s	_23.57s	_{p=0.078 n=6}
_{Total Time}	_{115.02s (± 0.35%)}	_{114.88s (± 0.30%)}	_~	_114.53s	_115.47s	_{p=0.471 n=6}
_{webpack - node (v18.15.0, x64)}
_Errors	₀	₀	_~	_~	_~	_{p=1.000 n=6}
_Symbols	_265,866	_265,866	_~	_~	_~	_{p=1.000 n=6}
_Types	_108,401	_108,401	_~	_~	_~	_{p=1.000 n=6}
_{Memory used}	_{410,516k (± 0.01%)}	_{410,514k (± 0.01%)}	_~	_410,452k	_410,570k	_{p=0.748 n=6}
_{Parse Time}	_{3.84s (± 1.39%)}	_{3.83s (± 0.86%)}	_~	_3.79s	_3.87s	_{p=1.000 n=6}
_{Bind Time}	_{1.66s (± 0.59%)}	_{1.66s (± 1.04%)}	_~	_1.64s	_1.68s	_{p=0.805 n=6}
_{Check Time}	_{16.98s (± 0.32%)}	_{16.96s (± 0.32%)}	_~	_16.89s	_17.02s	_{p=0.810 n=6}
_{Emit Time}	_0.00s	_0.00s	_~	_~	_~	_{p=1.000 n=6}
_{Total Time}	_{22.49s (± 0.38%)}	_{22.45s (± 0.36%)}	_~	_22.34s	_22.56s	_{p=0.748 n=6}
_{xstate-main - node (v18.15.0, x64)}
_Errors	₀	₀	_~	_~	_~	_{p=1.000 n=6}
_Symbols	_524,639	_524,639	_~	_~	_~	_{p=1.000 n=6}
_Types	_178,906	_178,906	_~	_~	_~	_{p=1.000 n=6}
_{Memory used}	_{462,675k (± 0.02%)}	_{462,662k (± 0.01%)}	_~	_462,595k	_462,744k	_{p=0.689 n=6}
_{Parse Time}	_{3.89s (± 0.46%)}	_{3.88s (± 0.36%)}	_~	_3.86s	_3.90s	_{p=0.370 n=6}
_{Bind Time}	_{1.44s (± 1.04%)}	_{1.44s (± 1.57%)}	_~	_1.42s	_1.48s	_{p=0.285 n=6}
_{Check Time}	_{22.53s (± 0.33%)}	_{22.57s (± 0.54%)}	_~	_22.36s	_22.72s	_{p=0.377 n=6}
_{Emit Time}	_0.00s	_0.00s	_~	_~	_~	_{p=1.000 n=6}
_{Total Time}	_{27.87s (± 0.26%)}	_{27.89s (± 0.38%)}	_~	_27.74s	_28.06s	_{p=0.810 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	₆
_Baseline	_baseline	₆

Developer Information:

Download Benchmarks

typescript-bot · 2024-05-13T22:14:54Z

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

Everything looks good!

typescript-bot · 2024-05-13T23:24:19Z

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

Everything looks good!

sandersn

@danvk guessed that the source of the problem was a speculative getTypeOfSymbol. But this PR fixes resolution for all parameters. If it applies to all parameters, why wasn't this observed before Dan's change?

Need tests of used/unused binding patterns. These probably exist already, but please double-check.
If this implementation is correct, I think it would be easier to read in isSelfReferenceLocation, as long as that doesn't make performance worse (from adding a second parameter).

Edit: having looked at checkIfExpressionRefinesAnyParameter, Dan's specific guess doesn't make sense to me. My (fuzzier) guess is that we haven't tried to resolve a parameter name from itself before and never observed that it mistakenly caused the parameter to be marked as used. But I'm not sure what in the new code is causing that.

sandersn · 2024-05-17T21:27:41Z

src/compiler/utilities.ts

@@ -11360,18 +11360,23 @@ export function createNameResolver({
                        location = root.parent;
                    }
                    break;
-                case SyntaxKind.Parameter:
+                case SyntaxKind.Parameter: {
+                    const param = location as ParameterDeclaration;


style question: is there a reason for all the casts in this code? Maybe it's better to drop this alias and use location as ParameterDeclaration) everywhere.

sandersn · 2024-05-17T21:38:25Z

src/compiler/utilities.ts

+                case SyntaxKind.Parameter: {
+                    const param = location as ParameterDeclaration;
+                    if (lastLocation === param.name) {
+                        lastSelfReferenceLocation = param;


I'd almost prefer a new parameter and entry in isSelfReferenceLocation like

function isSelfReferenceLocation(node: Node, lastLocation: Node): node is SelfReferenceLocation { switch (node.kind) { case SyntaxKind.Parameter: return lastLocation === (node as ParameterDeclaration).name // ...

sandersn · 2024-05-17T21:45:44Z

src/compiler/utilities.ts

-                case SyntaxKind.Parameter:
+                case SyntaxKind.Parameter: {
+                    const param = location as ParameterDeclaration;
+                    if (lastLocation === param.name) {


what does this do when param.name is a BindingPattern? Do we have tests to make sure that they still correctly get marked unused (and used when used)?

Fixed a regression with reporting unused parameters in potential pred…

27ad6f1

…icates

typescript-bot assigned sandersn May 13, 2024

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

danvk mentioned this pull request May 15, 2024

Infer AssertsIdentifier type predicates #58495

Open

sandersn requested changes May 17, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixed a regression with reporting unused parameters in potential predicates #58514

Fixed a regression with reporting unused parameters in potential predicates #58514

Andarist commented May 13, 2024

jakebailey commented May 13, 2024

typescript-bot commented May 13, 2024 •

edited

typescript-bot commented May 13, 2024

typescript-bot commented May 13, 2024

tsc

typescript-bot commented May 13, 2024

typescript-bot commented May 13, 2024

sandersn left a comment •

edited

sandersn May 17, 2024

sandersn May 17, 2024

sandersn May 17, 2024

Fixed a regression with reporting unused parameters in potential predicates #58514

Are you sure you want to change the base?

Fixed a regression with reporting unused parameters in potential predicates #58514

Conversation

Andarist commented May 13, 2024

jakebailey commented May 13, 2024

typescript-bot commented May 13, 2024 • edited

typescript-bot commented May 13, 2024

typescript-bot commented May 13, 2024

tsc

typescript-bot commented May 13, 2024

typescript-bot commented May 13, 2024

sandersn left a comment • edited

Choose a reason for hiding this comment

sandersn May 17, 2024

Choose a reason for hiding this comment

sandersn May 17, 2024

Choose a reason for hiding this comment

sandersn May 17, 2024

Choose a reason for hiding this comment

typescript-bot commented May 13, 2024 •

edited

sandersn left a comment •

edited