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

Implement tracking of syntax nodes if the syntax tree is edited #2118

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

Conversation

ahoppen
Copy link
Collaborator

@ahoppen ahoppen commented Aug 30, 2023

On any syntax tree, you can call .tracked. Any tree that is derived from that tracked tree is able to find the original syntax node in the tracked tree. Use cases of this could be to find the original source ranges of syntax nodes that have been re-written in macros or to allow a syntax rewriter to modify a syntax tree while still being able to get the original source locations for any node that was kept.

The basic idea is as follows: Every syntax node within the tree already contains an indexInTree, which is its index in a depth-first traversal. The node of each tree stores the ranges of indexInTree that have been re-used from the original tree and the offset my which their indexInTree has been shifted compared to the original tree.

For example, if you have the following tree (index in tree in parentheses)

      a (0)
     / \
(1) b   c (2)

And you construct a new tree a follows

       x (0)
     /   \
(1) y     a (2)
         / \
    (3) b   c (4)

Then the new tree would have a single syntax tracking range of 2...4: -2. This defines that nodes with indexInTree 2 to 4 occur in the tracked tree and that to receive the indexInTree within the tracked tree, 2 needs to be subtracted from the node in the derived tree.


I still need to investigate the performance implications of this change. My assumption is that

  • I need to get rid of the array that is now being created in the syntax node initializers because arrays usually result in a costly memory allocation
  • After that, the performance impact should be minimal when tracking is disabled.
    • Node modification requires two pointer dereferences plus null check (rootInfo.syntaxTracking?.trackedTree != nil from trackedTree != nil in replacingChild)
    • Node creation requires #-of-nodes pointer dereferences and null checks to check if any of them have a trackedTree. The cache should be able to serve these fairly quickly if you assume that many of these nodes have been recently created or are from the same original tree and thus share the same node

@ahoppen ahoppen force-pushed the ahoppen/node-tracking-in-root branch from 3b274ef to c41f24c Compare August 31, 2023 21:14
@ahoppen ahoppen marked this pull request as ready for review August 31, 2023 21:31
@ahoppen
Copy link
Collaborator Author

ahoppen commented Aug 31, 2023

@swift-ci Please test

@ahoppen ahoppen force-pushed the ahoppen/node-tracking-in-root branch from 8b7c8f5 to 489827b Compare September 1, 2023 20:23
@ahoppen
Copy link
Collaborator Author

ahoppen commented Sep 1, 2023

@swift-ci Please test

@ahoppen
Copy link
Collaborator Author

ahoppen commented Sep 1, 2023

@swift-ci Please test Windows

@ahoppen ahoppen force-pushed the ahoppen/node-tracking-in-root branch from 489827b to 4cab621 Compare September 6, 2023 00:47
@ahoppen
Copy link
Collaborator Author

ahoppen commented Sep 6, 2023

@swift-ci Please test

@ahoppen ahoppen force-pushed the ahoppen/node-tracking-in-root branch from 4cab621 to 6b1895b Compare September 6, 2023 01:21
@ahoppen
Copy link
Collaborator Author

ahoppen commented Sep 6, 2023

@swift-ci Please test

@ahoppen
Copy link
Collaborator Author

ahoppen commented Sep 6, 2023

@swift-ci Please test Windows

Passing a `UInt` around has always felt a little unsafe and didn’t give any semantic meaning that describes what the `UInt` represents.
…f structs

This will allow us to add more data to `SyntaxData.Info.Root` without increasing the size of `SyntaxData`. It shouldn’t have a big performance impact at the moment since `Root.arena` is never accessed and only exists to keep `arena` a live.
@ahoppen ahoppen force-pushed the ahoppen/node-tracking-in-root branch from 6b1895b to ba55508 Compare September 8, 2023 16:23
@ahoppen
Copy link
Collaborator Author

ahoppen commented Sep 8, 2023

@swift-ci Please test

@ahoppen
Copy link
Collaborator Author

ahoppen commented Sep 8, 2023

@swift-ci Please test Windows

@ahoppen
Copy link
Collaborator Author

ahoppen commented Sep 11, 2023

@swift-ci Please test macOS

@ahoppen ahoppen closed this Nov 8, 2023
@ahoppen ahoppen deleted the ahoppen/node-tracking-in-root branch November 8, 2023 15:40
@ahoppen ahoppen restored the ahoppen/node-tracking-in-root branch November 14, 2023 17:55
@ahoppen ahoppen reopened this Nov 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant