Eliminate SyntaxNode
#684
Merged
Eliminate SyntaxNode
#684
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@swift-ci Please test |
rintaro
commented
Sep 1, 2022
rintaro
force-pushed
the
syntaxnode-remove
branch
2 times, most recently
from
b2f9466 to
dff8f69
Compare
|
@swift-ci Please test |
rintaro
force-pushed
the
syntaxnode-remove
branch
from
dff8f69 to
2f03299
Compare
|
@swift-ci Please test |
`SyntaxNode` was introduced as a lighter `Syntax` when existentials were needed for creating `Syntax` instance. Now that `Syntax`/`SyntaxData` is just a struct that holds one class (`SyntaxBox`). So it's not that heavy. More importantly `SyntaxNode` was not safe to escape because it didn't retain the `SyntaxArena` of the `AbsoluteRawSyntax` `SyntaxNode` was only used in `SyntaxCursor` for `IncrementalParseLookup`. Use `SyntaxData` instead.
rintaro
force-pushed
the
syntaxnode-remove
branch
from
2f03299 to
c8ae3de
Compare
|
@swift-ci Please test |
ahoppen
approved these changes
Sep 5, 2022
| @@ -766,3 +663,6 @@ extension RawSyntaxNodeProtocol { | |||
| return Syntax(raw: raw).recursiveDescription | |||
| } | |||
| } | |||
|
|
|||
| @available(*, unavailable, message: "use 'Syntax' instead") | |||
| public struct SyntaxNode {} | |||
There was a problem hiding this comment.
Is this useful in any way or just an artifact from your local development?
There was a problem hiding this comment.
I left this only because SyntaxNode was a public type. Since the initializer was not public, I don't anticipate anyone is using this. But still.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
SyntaxNodewas introduced as a lighterSyntaxwhen existentials were needed for creatingSyntaxinstance.Now that
Syntax/SyntaxDatais just a struct that holds one class (SyntaxBox). So it's not that heavy.More importantly
SyntaxNodewas not safe to escape because it didn't retain theSyntaxArenaof theAbsoluteRawSyntaxSyntaxNodewas only used inSyntaxCursorforIncrementalParseLookup. UseSyntaxDatainstead.(Conflicts with #659 . Will rebase after merging it)
(No noticeable performance regression with a test case in rdar://48511326)