Skip to content

Keep accessors as accessors in emitted anonymous class declarations #60853

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

Conversation

Andarist
Copy link
Contributor

@Andarist Andarist commented Dec 25, 2024

fixes #60672
fixes #44938
fixes #58790
fixes #61967

@typescript-bot typescript-bot added For Backlog Bug PRs that fix a backlog bug For Milestone Bug PRs that fix a bug with a specific milestone labels Dec 25, 2024
@Andarist Andarist force-pushed the keep-accessors-as-accessors-on-emitted-anonymous-class-declarations branch from d466c50 to 1baa828 Compare January 1, 2025 19:11
@@ -14939,6 +14966,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
}
if (!singleProp) {
singleProp = prop;
propFlags = (prop.flags & SymbolFlags.Accessor) || SymbolFlags.Property;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Perhaps the accessor case should be handled through checkFlags. I'm worried that it would require looking carefully at all call sites that handle accessors to cover for this new case - but maybe I'm worried about it prematurely. Anyway, let me know what you think about it ;)

Comment on lines 14992 to 14993
// classes created by mixins are represented as intersections and overriding a property in a derived class redefines it completely at runtime
// so a get accessor can't be merged with a set accessor in a base class, for that reason the accessor flags are only used when they are the same in all consistuents
Copy link
Contributor Author

Choose a reason for hiding this comment

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

perhaps if I'd use a new CheckFlags.SyntheticAccessor that wouldn't really matter as much? I'm not sure if it's even that important of a rule to begin with, I just erred on the safer side of things and maintained the current behavior for this ambiguous case

@Andarist Andarist force-pushed the keep-accessors-as-accessors-on-emitted-anonymous-class-declarations branch from bfbf4b6 to b43f9dd Compare June 30, 2025 09:30
@rbuckton rbuckton removed their assignment Jun 30, 2025
@jakebailey
Copy link
Member

Doubtful this will catch anything, but

@typescript-bot test it

@typescript-bot
Copy link
Collaborator

typescript-bot commented Jun 30, 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

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/60853/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 34 34 ~ ~ ~ p=1.000 n=6
Symbols 62,370 62,370 ~ ~ ~ p=1.000 n=6
Types 50,386 50,386 ~ ~ ~ p=1.000 n=6
Memory used 195,949k (± 0.71%) 196,574k (± 0.03%) ~ 196,500k 196,640k p=0.298 n=6
Parse Time 1.60s (± 0.47%) 1.60s (± 1.56%) ~ 1.57s 1.63s p=0.805 n=6
Bind Time 0.88s (± 1.02%) 0.88s (± 1.18%) ~ 0.86s 0.89s p=0.673 n=6
Check Time 11.78s (± 0.37%) 11.83s (± 0.94%) ~ 11.70s 11.96s p=0.521 n=6
Emit Time 3.33s (± 3.68%) 3.27s (± 0.77%) ~ 3.23s 3.31s p=0.079 n=6
Total Time 17.58s (± 0.73%) 17.58s (± 0.62%) ~ 17.47s 17.71s p=1.000 n=6
angular-1 - node (v18.15.0, x64)
Errors 56 56 ~ ~ ~ p=1.000 n=6
Symbols 948,753 948,753 ~ ~ ~ p=1.000 n=6
Types 410,846 410,846 ~ ~ ~ p=1.000 n=6
Memory used 1,225,207k (± 0.00%) 1,225,195k (± 0.01%) ~ 1,225,077k 1,225,257k p=0.689 n=6
Parse Time 7.86s (± 1.11%) 7.98s (± 0.48%) ~ 7.92s 8.02s p=0.064 n=6
Bind Time 2.28s (± 0.43%) 2.29s (± 1.13%) ~ 2.24s 2.31s p=0.324 n=6
Check Time 37.95s (± 0.37%) 38.01s (± 0.36%) ~ 37.85s 38.19s p=0.688 n=6
Emit Time 17.82s (± 0.66%) 17.90s (± 0.90%) ~ 17.76s 18.20s p=0.423 n=6
Total Time 65.92s (± 0.43%) 66.17s (± 0.29%) ~ 65.91s 66.43s p=0.128 n=6
mui-docs - node (v18.15.0, x64)
Errors 0 0 ~ ~ ~ p=1.000 n=6
Symbols 2,509,183 2,509,183 ~ ~ ~ p=1.000 n=6
Types 892,716 892,716 ~ ~ ~ p=1.000 n=6
Memory used 2,804,482k (± 0.00%) 2,804,373k (± 0.00%) ~ 2,804,132k 2,804,508k p=0.173 n=6
Parse Time 10.53s (± 0.37%) 10.49s (± 0.16%) ~ 10.47s 10.51s p=0.090 n=6
Bind Time 2.74s (± 0.15%) 2.74s (± 0.84%) ~ 2.72s 2.78s p=0.498 n=6
Check Time 101.08s (± 0.35%) 101.48s (± 2.19%) ~ 99.83s 105.95s p=0.230 n=6
Emit Time 0.37s (± 2.04%) 0.37s (± 2.76%) ~ 0.36s 0.39s p=0.437 n=6
Total Time 114.71s (± 0.33%) 115.09s (± 1.94%) ~ 113.40s 119.59s p=0.229 n=6
self-build-src - node (v18.15.0, x64)
Errors 0 0 ~ ~ ~ p=1.000 n=6
Symbols 1,227,080 1,227,095 +15 (+ 0.00%) ~ ~ p=0.001 n=6
Types 267,443 267,448 +5 (+ 0.00%) ~ ~ p=0.001 n=6
Memory used 2,781,556k (±12.57%) 2,756,565k (±13.68%) ~ 2,364,729k 3,097,767k p=1.000 n=6
Parse Time 6.61s (± 1.29%) 6.59s (± 1.40%) ~ 6.46s 6.69s p=0.575 n=6
Bind Time 2.17s (± 0.63%) 2.16s (± 1.49%) ~ 2.12s 2.21s p=0.193 n=6
Check Time 42.62s (± 0.63%) 42.75s (± 0.88%) ~ 41.99s 42.96s p=0.298 n=6
Emit Time 3.40s (± 2.56%) 3.54s (± 2.04%) 🔻+0.14s (+ 4.27%) 3.44s 3.60s p=0.045 n=6
Total Time 54.81s (± 0.70%) 55.05s (± 0.78%) ~ 54.18s 55.30s p=0.471 n=6
self-build-src-public-api - node (v18.15.0, x64)
Errors 0 0 ~ ~ ~ p=1.000 n=6
Symbols 1,227,080 1,227,095 +15 (+ 0.00%) ~ ~ p=0.001 n=6
Types 267,443 267,448 +5 (+ 0.00%) ~ ~ p=0.001 n=6
Memory used 2,674,885k (±14.05%) 2,796,572k (±14.27%) ~ 2,431,253k 3,161,462k p=0.471 n=6
Parse Time 6.82s (± 1.24%) 6.87s (± 0.56%) ~ 6.81s 6.90s p=0.378 n=6
Bind Time 2.21s (± 1.31%) 2.21s (± 1.06%) ~ 2.18s 2.24s p=1.000 n=6
Check Time 42.80s (± 0.53%) 42.78s (± 0.46%) ~ 42.58s 43.02s p=0.810 n=6
Emit Time 3.60s (± 3.35%) 3.53s (± 2.78%) ~ 3.41s 3.70s p=0.297 n=6
Total Time 55.43s (± 0.53%) 55.39s (± 0.50%) ~ 55.04s 55.71s p=0.873 n=6
self-compiler - node (v18.15.0, x64)
Errors 0 0 ~ ~ ~ p=1.000 n=6
Symbols 262,529 262,544 +15 (+ 0.01%) ~ ~ p=0.001 n=6
Types 107,150 107,155 +5 (+ 0.00%) ~ ~ p=0.001 n=6
Memory used 441,796k (± 0.01%) 441,855k (± 0.03%) ~ 441,640k 442,078k p=0.093 n=6
Parse Time 3.52s (± 0.47%) 3.54s (± 0.94%) ~ 3.50s 3.59s p=0.226 n=6
Bind Time 1.32s (± 1.07%) 1.32s (± 1.11%) ~ 1.30s 1.34s p=0.870 n=6
Check Time 18.92s (± 0.28%) 18.84s (± 0.32%) ~ 18.77s 18.95s p=0.063 n=6
Emit Time 1.53s (± 1.62%) 1.51s (± 1.16%) ~ 1.49s 1.54s p=0.413 n=6
Total Time 25.28s (± 0.24%) 25.22s (± 0.37%) ~ 25.10s 25.36s p=0.261 n=6
ts-pre-modules - node (v18.15.0, x64)
Errors 71 71 ~ ~ ~ p=1.000 n=6
Symbols 225,367 225,367 ~ ~ ~ p=1.000 n=6
Types 94,290 94,290 ~ ~ ~ p=1.000 n=6
Memory used 371,088k (± 0.01%) 371,143k (± 0.02%) ~ 371,080k 371,303k p=0.298 n=6
Parse Time 2.88s (± 0.97%) 2.88s (± 0.87%) ~ 2.83s 2.90s p=1.000 n=6
Bind Time 1.61s (± 1.28%) 1.59s (± 0.77%) ~ 1.58s 1.61s p=0.365 n=6
Check Time 16.36s (± 0.33%) 16.34s (± 0.30%) ~ 16.27s 16.39s p=0.571 n=6
Emit Time 0.00s 0.00s ~ ~ ~ p=1.000 n=6
Total Time 20.85s (± 0.34%) 20.81s (± 0.37%) ~ 20.73s 20.90s p=0.466 n=6
vscode - node (v18.15.0, x64)
Errors 33 33 ~ ~ ~ p=1.000 n=6
Symbols 3,498,707 3,498,707 ~ ~ ~ p=1.000 n=6
Types 1,176,728 1,176,728 ~ ~ ~ p=1.000 n=6
Memory used 3,548,716k (± 0.00%) 3,548,740k (± 0.00%) ~ 3,548,497k 3,548,913k p=0.810 n=6
Parse Time 18.80s (± 3.28%) 18.47s (± 0.26%) ~ 18.40s 18.53s p=0.230 n=6
Bind Time 6.00s (± 0.33%) 5.99s (± 0.57%) ~ 5.94s 6.02s p=0.803 n=6
Check Time 120.15s (± 4.48%) 115.69s (± 3.07%) ~ 113.63s 122.89s p=0.298 n=6
Emit Time 37.17s (± 2.67%) 36.40s (± 1.93%) ~ 36.02s 37.83s p=0.471 n=6
Total Time 182.11s (± 3.51%) 176.54s (± 2.43%) ~ 174.11s 185.23s p=0.378 n=6
webpack - node (v18.15.0, x64)
Errors 2 2 ~ ~ ~ p=1.000 n=6
Symbols 318,019 318,019 ~ ~ ~ p=1.000 n=6
Types 137,705 137,705 ~ ~ ~ p=1.000 n=6
Memory used 472,300k (± 0.02%) 472,088k (± 0.01%) -212k (- 0.04%) 472,009k 472,187k p=0.008 n=6
Parse Time 4.35s (± 0.52%) 4.36s (± 1.12%) ~ 4.30s 4.45s p=0.685 n=6
Bind Time 1.78s (± 0.69%) 1.77s (± 0.97%) ~ 1.75s 1.80s p=0.137 n=6
Check Time 20.78s (± 0.52%) 20.69s (± 0.41%) ~ 20.63s 20.84s p=0.108 n=6
Emit Time 0.00s 0.00s ~ ~ ~ p=1.000 n=6
Total Time 26.92s (± 0.46%) 26.82s (± 0.48%) ~ 26.70s 27.05s p=0.128 n=6
xstate-main - node (v18.15.0, x64)
Errors 30 30 ~ ~ ~ p=1.000 n=6
Symbols 665,978 665,978 ~ ~ ~ p=1.000 n=6
Types 199,345 199,345 ~ ~ ~ p=1.000 n=6
Memory used 570,374k (± 0.01%) 570,277k (± 0.03%) ~ 569,930k 570,378k p=0.230 n=6
Parse Time 4.25s (± 0.77%) 4.25s (± 0.72%) ~ 4.22s 4.30s p=1.000 n=6
Bind Time 1.34s (± 0.73%) 1.34s (± 0.87%) ~ 1.33s 1.36s p=1.000 n=6
Check Time 19.93s (± 0.38%) 19.94s (± 0.30%) ~ 19.89s 20.04s p=0.936 n=6
Emit Time 0.00s 0.00s ~ ~ ~ p=1.000 n=6
Total Time 25.52s (± 0.36%) 25.54s (± 0.25%) ~ 25.47s 25.64s p=0.936 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/60853/merge:

Everything looks good!

@github-project-automation github-project-automation bot moved this from Not started to Needs merge in PR Backlog Jun 30, 2025
@jakebailey jakebailey merged commit c35b143 into microsoft:main Jun 30, 2025
32 checks passed
@github-project-automation github-project-automation bot moved this from Needs merge to Done in PR Backlog Jun 30, 2025
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 For Milestone Bug PRs that fix a bug with a specific milestone
Projects
Status: Done
5 participants