Skip to content

Conversation

@hamishknight
Copy link
Contributor

This just holds the position info, which is useful for clients that don't care about the extra state in Cursor (e.g the state stack). LexingDiagnostic is one such client, and regex literal lexing will eventually be another.

This just holds the position info, which is useful
for clients that don't care about the extra state
in Cursor (e.g the state stack). LexingDiagnostic
is one such client, and regex literal lexing will
eventually be another.
@hamishknight hamishknight requested a review from ahoppen as a code owner March 19, 2023 13:21
@hamishknight
Copy link
Contributor Author

@swift-ci please test

Copy link
Member

@ahoppen ahoppen left a comment

Choose a reason for hiding this comment

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

Nice. Just to be clear. Changing the state stack to use Lexer.Cursor.Position will be a follow-up PR?

Also out of curiosity: Did you check if changing the sate stack to use Lexer.Cursor.Postion improves performance?

@hamishknight
Copy link
Contributor Author

hamishknight commented Mar 20, 2023

@ahoppen Sorry, I realize now the PR description is ambiguously worded, when I mentioned the state stack, I'm referring to it as state that certain clients don't care about (and so should use Cursor.Position instead). AFAIK the state stack does not currently store Cursors, so there shouldn't be any work to do there. That being said, one of the reasons I'm adding this is to avoid storing a whole Cursor as the endpoint for a regex literal lexeme, which will need to be stored in the state stack (I'm planning on doing some benchmarks to see whether storing that state inline vs indirectly is better).

If you're interested in where I am currently with the regex work, I have a WIP branch https://github.com/hamishknight/swift-syntax/compare/lets-go-back-to-the-island

@ahoppen
Copy link
Member

ahoppen commented Mar 20, 2023

Ah, I see. Makes sense 👍🏽

@hamishknight hamishknight merged commit 9fd3175 into swiftlang:main Mar 20, 2023
@hamishknight hamishknight deleted the position branch March 20, 2023 20:07
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