GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — Ensure that we copy empty NodeArrays during transform by jakebailey · Pull Request #48490 · microsoft/TypeScript

Conversation

@jakebailey

@jakebailey Jake Bailey (jakebailey) commented Mar 30, 2022 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

Fixes #48379
Fixes #45744 (most likely)

In order to offer snippet completion for method overrides, we emit code. During emit, each node gets __pos and __end properties as a side effect, so that later those positions relative to the just-emitted text can be used.

To produce the code for the override, we convert signatures, which involves converting types to nodes. Unfortunately, the empty NodeArray inside of the type {} was getting cached then reused instead of cached then cloned. This led to every instance of {} in the transformed tree being the same object, which then caused emit to set position info more than once on the same node, which then caused debug assertions because earlier {}s would have the position info from the last {}, which isn't legal.

So, during transform, when we are explicitly doing that "deep copy or reuse", actually ensure that a copy happens for empty NodeArrays.

I feel like it would be better for visitNodes to do this, or createNodeArray to do it, or similar, but doing so breaks some cases where it appears as though callers of visitNodes assume that the array is returned unmodified if empty, breaking some 25 cases.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Jake Bailey (jakebailey) added 2 commits March 29, 2022 17:34
@jakebailey
Add failing test case
d273704
@jakebailey
Ensure that we copy empty NodeArrays during transform
944315e
@typescript-bot TypeScript Bot (typescript-bot) assigned Jake Bailey (jakebailey) Mar 30, 2022
@typescript-bot TypeScript Bot (typescript-bot) added Author: Team For Milestone Bug PRs that fix a bug with a specific milestone labels Mar 30, 2022
@jakebailey

Jake Bailey (jakebailey) commented Mar 30, 2022

Copy link
Copy Markdown
Member Author

TypeScript Bot (@typescript-bot) perf test this inline
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@typescript-bot

TypeScript Bot (typescript-bot) commented Mar 30, 2022 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

Heya Jake Bailey (@jakebailey), I've started to run the perf test suite on this PR at 944315e. You can monitor the build here.

Update: The results are in!
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@typescript-bot

TypeScript Bot (typescript-bot) commented Mar 30, 2022

Copy link
Copy Markdown
Contributor

Jake Bailey (@jakebailey)
The results of the perf run you requested are in!

Here they are:

Comparison Report - main..48490

Metric main 48490 Delta Best Worst
Angular - node (v10.16.3, x64)
Memory used 356,635k (± 0.02%) 356,664k (± 0.02%) +29k (+ 0.01%) 356,453k 356,764k
Parse Time 2.05s (± 0.52%) 2.05s (± 0.56%) -0.01s (- 0.29%) 2.02s 2.07s
Bind Time 0.86s (± 0.93%) 0.86s (± 0.72%) +0.00s (+ 0.35%) 0.85s 0.87s
Check Time 5.81s (± 0.53%) 5.81s (± 0.58%) -0.01s (- 0.12%) 5.74s 5.86s
Emit Time 6.00s (± 0.57%) 5.99s (± 0.63%) -0.02s (- 0.28%) 5.92s 6.09s
Total Time 14.73s (± 0.36%) 14.71s (± 0.36%) -0.02s (- 0.17%) 14.59s 14.81s
Compiler-Unions - node (v10.16.3, x64)
Memory used 205,783k (± 0.03%) 205,760k (± 0.03%) -23k (- 0.01%) 205,595k 205,874k
Parse Time 0.85s (± 0.58%) 0.85s (± 0.56%) +0.00s (+ 0.12%) 0.84s 0.86s
Bind Time 0.51s (± 0.78%) 0.52s (± 0.66%) +0.01s (+ 1.17%) 0.51s 0.52s
Check Time 7.92s (± 0.49%) 7.92s (± 0.44%) +0.00s (+ 0.01%) 7.84s 8.03s
Emit Time 2.50s (± 1.06%) 2.52s (± 0.72%) +0.02s (+ 0.80%) 2.49s 2.57s
Total Time 11.78s (± 0.44%) 11.81s (± 0.35%) +0.03s (+ 0.25%) 11.70s 11.92s
Monaco - node (v10.16.3, x64)
Memory used 343,621k (± 0.02%) 343,617k (± 0.02%) -4k (- 0.00%) 343,441k 343,724k
Parse Time 1.57s (± 0.42%) 1.57s (± 0.56%) +0.00s (+ 0.00%) 1.54s 1.58s
Bind Time 0.75s (± 0.49%) 0.75s (± 0.74%) -0.00s (- 0.40%) 0.74s 0.76s
Check Time 5.75s (± 0.38%) 5.75s (± 0.29%) +0.00s (+ 0.02%) 5.71s 5.78s
Emit Time 3.24s (± 0.62%) 3.23s (± 0.60%) -0.01s (- 0.19%) 3.16s 3.25s
Total Time 11.30s (± 0.28%) 11.30s (± 0.20%) -0.01s (- 0.04%) 11.25s 11.36s
TFS - node (v10.16.3, x64)
Memory used 305,299k (± 0.02%) 305,302k (± 0.03%) +3k (+ 0.00%) 305,020k 305,577k
Parse Time 1.28s (± 0.78%) 1.28s (± 0.63%) -0.01s (- 0.47%) 1.26s 1.30s
Bind Time 0.72s (± 0.77%) 0.71s (± 1.41%) -0.01s (- 0.97%) 0.69s 0.74s
Check Time 5.25s (± 0.69%) 5.25s (± 0.45%) -0.00s (- 0.04%) 5.20s 5.30s
Emit Time 3.43s (± 1.10%) 3.43s (± 1.93%) -0.00s (- 0.12%) 3.31s 3.55s
Total Time 10.69s (± 0.50%) 10.67s (± 0.57%) -0.02s (- 0.15%) 10.52s 10.79s
material-ui - node (v10.16.3, x64)
Memory used 469,833k (± 0.01%) 469,821k (± 0.01%) -12k (- 0.00%) 469,676k 469,966k
Parse Time 1.81s (± 0.49%) 1.81s (± 0.42%) +0.01s (+ 0.39%) 1.80s 1.83s
Bind Time 0.68s (± 1.13%) 0.68s (± 0.98%) +0.00s (+ 0.74%) 0.66s 0.69s
Check Time 14.30s (± 0.56%) 14.33s (± 0.64%) +0.03s (+ 0.20%) 14.11s 14.58s
Emit Time 0.00s (± 0.00%) 0.00s (± 0.00%) 0.00s ( NaN%) 0.00s 0.00s
Total Time 16.78s (± 0.48%) 16.82s (± 0.55%) +0.04s (+ 0.24%) 16.60s 17.06s
xstate - node (v10.16.3, x64)
Memory used 573,939k (± 1.25%) 574,052k (± 1.25%) +113k (+ 0.02%) 570,590k 602,991k
Parse Time 2.57s (± 0.32%) 2.58s (± 0.25%) +0.00s (+ 0.16%) 2.57s 2.59s
Bind Time 1.01s (± 0.68%) 1.00s (± 0.47%) -0.01s (- 0.60%) 0.99s 1.01s
Check Time 1.51s (± 0.49%) 1.51s (± 0.50%) +0.00s (+ 0.07%) 1.50s 1.53s
Emit Time 0.07s (± 4.13%) 0.07s (± 0.00%) -0.00s (- 2.78%) 0.07s 0.07s
Total Time 5.16s (± 0.23%) 5.16s (± 0.21%) +0.00s (+ 0.04%) 5.14s 5.18s
Angular - node (v12.1.0, x64)
Memory used 334,451k (± 0.01%) 334,509k (± 0.02%) +57k (+ 0.02%) 334,413k 334,692k
Parse Time 2.04s (± 0.69%) 2.04s (± 0.24%) -0.01s (- 0.29%) 2.03s 2.05s
Bind Time 0.82s (± 0.36%) 0.83s (± 1.23%) +0.01s (+ 1.09%) 0.82s 0.86s
Check Time 5.67s (± 1.00%) 5.65s (± 0.50%) -0.01s (- 0.26%) 5.61s 5.73s
Emit Time 6.26s (± 0.56%) 6.25s (± 0.32%) -0.00s (- 0.02%) 6.21s 6.29s
Total Time 14.78s (± 0.64%) 14.77s (± 0.27%) -0.01s (- 0.05%) 14.71s 14.90s
Compiler-Unions - node (v12.1.0, x64)
Memory used 193,168k (± 0.12%) 193,167k (± 0.12%) -1k (- 0.00%) 192,735k 193,461k
Parse Time 0.84s (± 0.53%) 0.84s (± 0.81%) +0.01s (+ 0.72%) 0.83s 0.86s
Bind Time 0.53s (± 0.68%) 0.53s (± 0.84%) -0.00s (- 0.37%) 0.52s 0.54s
Check Time 7.42s (± 0.33%) 7.49s (± 0.52%) +0.07s (+ 0.88%) 7.39s 7.56s
Emit Time 2.57s (± 1.45%) 2.57s (± 2.28%) +0.00s (+ 0.12%) 2.49s 2.79s
Total Time 11.36s (± 0.47%) 11.43s (± 0.70%) +0.07s (+ 0.64%) 11.31s 11.71s
Monaco - node (v12.1.0, x64)
Memory used 326,574k (± 0.08%) 326,581k (± 0.06%) +7k (+ 0.00%) 325,870k 326,784k
Parse Time 1.55s (± 0.40%) 1.55s (± 0.86%) +0.00s (+ 0.26%) 1.52s 1.58s
Bind Time 0.74s (± 1.10%) 0.74s (± 0.54%) +0.00s (+ 0.14%) 0.73s 0.75s
Check Time 5.61s (± 0.62%) 5.60s (± 0.36%) -0.00s (- 0.09%) 5.56s 5.64s
Emit Time 3.28s (± 1.34%) 3.27s (± 0.88%) -0.01s (- 0.30%) 3.23s 3.36s
Total Time 11.18s (± 0.63%) 11.17s (± 0.36%) -0.01s (- 0.05%) 11.12s 11.29s
TFS - node (v12.1.0, x64)
Memory used 289,873k (± 0.06%) 289,915k (± 0.01%) +42k (+ 0.01%) 289,838k 289,988k
Parse Time 1.29s (± 0.76%) 1.29s (± 0.59%) -0.00s (- 0.16%) 1.27s 1.30s
Bind Time 0.71s (± 0.73%) 0.71s (± 0.85%) -0.00s (- 0.56%) 0.69s 0.72s
Check Time 5.18s (± 0.58%) 5.19s (± 0.39%) +0.00s (+ 0.08%) 5.15s 5.25s
Emit Time 3.45s (± 1.07%) 3.47s (± 0.70%) +0.02s (+ 0.43%) 3.38s 3.50s
Total Time 10.63s (± 0.43%) 10.64s (± 0.34%) +0.01s (+ 0.14%) 10.52s 10.69s
material-ui - node (v12.1.0, x64)
Memory used 448,864k (± 0.06%) 448,825k (± 0.07%) -38k (- 0.01%) 447,640k 449,078k
Parse Time 1.81s (± 0.33%) 1.80s (± 0.37%) -0.00s (- 0.28%) 1.79s 1.82s
Bind Time 0.64s (± 0.52%) 0.64s (± 0.69%) +0.00s (+ 0.31%) 0.63s 0.65s
Check Time 12.90s (± 0.68%) 12.85s (± 0.43%) -0.04s (- 0.32%) 12.77s 12.99s
Emit Time 0.00s (± 0.00%) 0.00s (± 0.00%) 0.00s ( NaN%) 0.00s 0.00s
Total Time 15.34s (± 0.55%) 15.30s (± 0.41%) -0.04s (- 0.29%) 15.21s 15.45s
xstate - node (v12.1.0, x64)
Memory used 536,345k (± 0.02%) 536,294k (± 0.01%) -51k (- 0.01%) 536,132k 536,430k
Parse Time 2.52s (± 0.56%) 2.51s (± 0.55%) -0.01s (- 0.44%) 2.48s 2.54s
Bind Time 1.03s (± 0.46%) 1.03s (± 0.64%) -0.00s (- 0.29%) 1.02s 1.04s
Check Time 1.47s (± 0.41%) 1.46s (± 0.64%) -0.01s (- 0.34%) 1.44s 1.48s
Emit Time 0.07s (± 0.00%) 0.07s (± 0.00%) 0.00s ( 0.00%) 0.07s 0.07s
Total Time 5.09s (± 0.33%) 5.07s (± 0.26%) -0.02s (- 0.39%) 5.05s 5.11s
Angular - node (v14.15.1, x64)
Memory used 332,666k (± 0.08%) 332,806k (± 0.01%) +140k (+ 0.04%) 332,771k 332,851k
Parse Time 2.02s (± 0.77%) 2.02s (± 0.35%) -0.00s (- 0.05%) 2.00s 2.03s
Bind Time 0.86s (± 0.57%) 0.87s (± 0.57%) +0.00s (+ 0.23%) 0.86s 0.88s
Check Time 5.65s (± 0.63%) 5.63s (± 0.30%) -0.02s (- 0.32%) 5.58s 5.66s
Emit Time 6.32s (± 0.73%) 6.31s (± 0.57%) -0.01s (- 0.14%) 6.22s 6.38s
Total Time 14.85s (± 0.52%) 14.83s (± 0.28%) -0.02s (- 0.13%) 14.73s 14.92s
Compiler-Unions - node (v14.15.1, x64)
Memory used 194,015k (± 0.61%) 194,628k (± 0.50%) +614k (+ 0.32%) 191,993k 195,378k
Parse Time 0.86s (± 0.68%) 0.86s (± 0.61%) +0.00s (+ 0.23%) 0.85s 0.87s
Bind Time 0.56s (± 0.65%) 0.56s (± 0.80%) +0.00s (+ 0.36%) 0.55s 0.57s
Check Time 7.49s (± 0.59%) 7.52s (± 0.79%) +0.04s (+ 0.47%) 7.39s 7.72s
Emit Time 2.51s (± 0.86%) 2.50s (± 0.92%) -0.01s (- 0.36%) 2.46s 2.55s
Total Time 11.41s (± 0.39%) 11.44s (± 0.58%) +0.03s (+ 0.26%) 11.32s 11.64s
Monaco - node (v14.15.1, x64)
Memory used 325,399k (± 0.00%) 325,402k (± 0.00%) +3k (+ 0.00%) 325,372k 325,429k
Parse Time 1.58s (± 0.55%) 1.57s (± 0.66%) -0.01s (- 0.63%) 1.55s 1.59s
Bind Time 0.77s (± 0.58%) 0.78s (± 0.48%) +0.00s (+ 0.39%) 0.77s 0.78s
Check Time 5.51s (± 0.43%) 5.51s (± 0.56%) -0.00s (- 0.04%) 5.46s 5.58s
Emit Time 3.30s (± 0.59%) 3.31s (± 0.68%) +0.01s (+ 0.33%) 3.26s 3.36s
Total Time 11.16s (± 0.24%) 11.16s (± 0.34%) -0.00s (- 0.00%) 11.08s 11.24s
TFS - node (v14.15.1, x64)
Memory used 288,906k (± 0.01%) 288,903k (± 0.01%) -3k (- 0.00%) 288,866k 288,949k
Parse Time 1.33s (± 2.11%) 1.31s (± 0.56%) -0.02s (- 1.88%) 1.30s 1.33s
Bind Time 0.73s (± 0.76%) 0.73s (± 0.81%) +0.00s (+ 0.41%) 0.72s 0.75s
Check Time 5.17s (± 0.27%) 5.17s (± 0.74%) +0.01s (+ 0.19%) 5.08s 5.28s
Emit Time 3.51s (± 2.04%) 3.50s (± 1.87%) -0.01s (- 0.26%) 3.38s 3.62s
Total Time 10.74s (± 0.75%) 10.72s (± 0.87%) -0.02s (- 0.20%) 10.51s 10.93s
material-ui - node (v14.15.1, x64)
Memory used 447,018k (± 0.08%) 447,143k (± 0.07%) +125k (+ 0.03%) 445,962k 447,324k
Parse Time 1.85s (± 0.45%) 1.86s (± 0.42%) +0.01s (+ 0.32%) 1.85s 1.88s
Bind Time 0.70s (± 0.79%) 0.70s (± 1.16%) +0.00s (+ 0.14%) 0.69s 0.72s
Check Time 13.07s (± 0.76%) 13.01s (± 0.95%) -0.07s (- 0.50%) 12.88s 13.48s
Emit Time 0.00s (± 0.00%) 0.00s (± 0.00%) 0.00s ( NaN%) 0.00s 0.00s
Total Time 15.63s (± 0.63%) 15.57s (± 0.83%) -0.06s (- 0.36%) 15.43s 16.07s
xstate - node (v14.15.1, x64)
Memory used 534,078k (± 0.00%) 534,070k (± 0.00%) -8k (- 0.00%) 534,014k 534,104k
Parse Time 2.58s (± 0.52%) 2.56s (± 0.36%) -0.02s (- 0.81%) 2.54s 2.58s
Bind Time 1.16s (± 1.04%) 1.14s (± 0.87%) -0.01s (- 1.12%) 1.12s 1.16s
Check Time 1.50s (± 0.45%) 1.50s (± 0.27%) -0.00s (- 0.27%) 1.49s 1.51s
Emit Time 0.07s (± 0.00%) 0.07s (± 0.00%) 0.00s ( 0.00%) 0.07s 0.07s
Total Time 5.31s (± 0.32%) 5.28s (± 0.26%) -0.03s (- 0.58%) 5.25s 5.31s
System
Machine Namets-ci-ubuntu
Platformlinux 4.4.0-210-generic
Architecturex64
Available Memory16 GB
Available Memory3 GB
CPUs4 × Intel(R) Core(TM) i7-4770 CPU @ 3.40GHz
Hosts
  • node (v10.16.3, x64)
  • node (v12.1.0, x64)
  • node (v14.15.1, x64)
Scenarios
  • Angular - node (v10.16.3, x64)
  • Angular - node (v12.1.0, x64)
  • Angular - node (v14.15.1, x64)
  • Compiler-Unions - node (v10.16.3, x64)
  • Compiler-Unions - node (v12.1.0, x64)
  • Compiler-Unions - node (v14.15.1, x64)
  • Monaco - node (v10.16.3, x64)
  • Monaco - node (v12.1.0, x64)
  • Monaco - node (v14.15.1, x64)
  • TFS - node (v10.16.3, x64)
  • TFS - node (v12.1.0, x64)
  • TFS - node (v14.15.1, x64)
  • material-ui - node (v10.16.3, x64)
  • material-ui - node (v12.1.0, x64)
  • material-ui - node (v14.15.1, x64)
  • xstate - node (v10.16.3, x64)
  • xstate - node (v12.1.0, x64)
  • xstate - node (v14.15.1, x64)
Benchmark Name Iterations
Current 48490 10
Baseline main 10

Developer Information:

Download Benchmark
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@jakebailey
Jake Bailey (jakebailey) requested review from Gabriela Araujo Britto (gabritto) and Wesley Wigham (weswigham) March 30, 2022 21:43
gabritto
Gabriela Araujo Britto (gabritto) approved these changes Mar 30, 2022
View reviewed changes
weswigham
Wesley Wigham (weswigham) approved these changes Mar 30, 2022
View reviewed changes
@jakebailey
Jake Bailey (jakebailey) merged commit 3c6c279 into microsoft:main Mar 30, 2022
@jakebailey
Jake Bailey (jakebailey) deleted the fix-48379 branch March 30, 2022 22:33
Sidharth Vinod (sidharthv96) added a commit to sidharthv96/TypeScript that referenced this pull request Apr 1, 2022
@sidharthv96
Merge remote-tracking branch 'upstream/main' into fix/46149
e149cc3 
* upstream/main: (473 commits)
  Correct node used for isDefinition calculation (microsoft#48499)
  fix(48405): emit dummy members from a mapped type (microsoft#48481)
  CFA for dependent parameters typed by generic constraints (microsoft#48411)
  No contextual typing from return types for boolean literals (microsoft#48380)
  fix(47733): omit JSDoc comment template suggestion on node with existing JSDoc (microsoft#47748)
  Ensure that we copy empty NodeArrays during transform (microsoft#48490)
  Add a new compiler option `moduleSuffixes` to expand the node module resolver's search algorithm (microsoft#48189)
  feat(27615): Add missing member fix should work for type literals (microsoft#47212)
  Add label details to completion entry (microsoft#48429)
  Enable method signature completion for object literals (microsoft#48168)
  Fix string literal completions when a partially-typed string fixes inference to a type parameter (microsoft#48410)
  fix(48445): show errors on type-only import/export specifiers in JavaScript files (microsoft#48449)
  Fix newline inserted in empty block at end of formatting range (microsoft#48463)
  Prevent looking up symbol for as const from triggering an error (microsoft#48464)
  Revise accessor resolution logic and error reporting (microsoft#48459)
  fix(48166): skip checking module.exports in a truthiness call expression (microsoft#48337)
  LEGO: Merge pull request 48450
  LEGO: Merge pull request 48436
  fix(48031): show circularity error for self referential get accessor annotations (microsoft#48050)
  Revert "Fix contextual discrimination for omitted members (microsoft#43937)" (microsoft#48426)
  ...
@jakebailey Jake Bailey (jakebailey) mentioned this pull request Jul 21, 2022
Error in 'setTextPos' during 'getCodeFixes' #49310
Closed
@microsoft Microsoft (microsoft) locked as resolved and limited conversation to collaborators Oct 22, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@weswigham Wesley Wigham (weswigham) weswigham approved these changes
@gabritto Gabriela Araujo Britto (gabritto) gabritto approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

@jakebailey Jake Bailey (jakebailey)

Labels

Author: Team For Milestone Bug PRs that fix a bug with a specific milestone

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Assertion failure in method snippet completions for subclasses with multiple type parameter references Error in 'consumeTokenAndAdvanceScanner' during 'getCodeFixes'

Uh oh!

There was an error while loading. Please reload this page.

4 participants

@jakebailey @typescript-bot @weswigham @gabritto
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.