Skip to content

Some more improvements to BasicFormat + misc changes #1568

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

Merged
merged 4 commits into from
Apr 22, 2023

Conversation

ahoppen
Copy link
Member

@ahoppen ahoppen commented Apr 21, 2023

This is basically a grab bag of changes that I want to do before my final PR to improve BasicFormat. Splitting it so it’s easier to review:

Contains a couple of bug fixes:

  • Fix an access to freed memory when modifying token nodes
    • When a token node is modified, we would not add its arena as a child to the new arena. This is unsafe because the newly created token might still have its text stored in the original arena.
  • Fix a bug where allTokens would not include the last token if it is missing

Minor new features

  • Add a walk method to SyntaxRewriter
    • Does the same as visit but also re-attaches the rewritten node into the parent tree (I will need this in the next PR)

Biggest change:

  • Add placeholder children to all missing syntax nodes
    • It’s much easier to get correct formatting in the new BasicFormat if the MissingExprSyntax etc. node has a token inside of it. Because if it does, we know whether we should e.g. add a space after try in front of a MissingExprNode. This also required a few small changes to SwiftParserDiagnostics

@ahoppen ahoppen requested review from rintaro and bnbarham April 21, 2023 18:53
@ahoppen
Copy link
Member Author

ahoppen commented Apr 21, 2023

@swift-ci Please test

ahoppen added 2 commits April 21, 2023 15:06
When a token node is modified, we would not add its arena as a child to the new arena. This is unsafe because the newly created token might still have its text stored in the original arena.
@ahoppen ahoppen force-pushed the ahoppen/basic-format-2 branch from ab4b276 to 84b1372 Compare April 21, 2023 22:06
@ahoppen
Copy link
Member Author

ahoppen commented Apr 21, 2023

@swift-ci Please test

@ahoppen
Copy link
Member Author

ahoppen commented Apr 21, 2023

@swift-ci Please test Windows

@ahoppen
Copy link
Member Author

ahoppen commented Apr 22, 2023

@bnbarham @rintaro I’m merging this now because I’ve got two more PRs that depend on this and which I would like to get merged ASAP as well. I‘ll address review comments in a follow-up PR.

@ahoppen ahoppen merged commit 00cbee2 into swiftlang:main Apr 22, 2023
@ahoppen ahoppen deleted the ahoppen/basic-format-2 branch April 22, 2023 15:03
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.

2 participants