Skip to content

[libSyntax] Create deferred nodes in the ParsedRawSyntaxRecorder #36235

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

Conversation

ahoppen
Copy link
Member

@ahoppen ahoppen commented Mar 2, 2021

This is a multi-commit effort to push the responsibility of deferred node handling to the SyntaxParseActions which have more detailed knowledge of their requirements on deferred nodes and might perform additional optimisations. For example, the SyntaxTreeCreator can always create RawSyntax nodes (even for deferred nodes) and decide to simply not use them, should the deferred nodes not get recorded.

@ahoppen ahoppen marked this pull request as ready for review March 2, 2021 17:31
@ahoppen ahoppen requested a review from rintaro March 2, 2021 17:31
@ahoppen ahoppen force-pushed the pr/deferred-nodes-from-parsedrawsyntaxrecorder branch from 8c5caa9 to 0a820fe Compare March 2, 2021 21:47
@swiftlang swiftlang deleted a comment from swift-ci Mar 2, 2021
@ahoppen
Copy link
Member Author

ahoppen commented Mar 2, 2021

@swift-ci Please test

@ahoppen
Copy link
Member Author

ahoppen commented Mar 3, 2021

@swift-ci Please test Windows

This is a multi-commit effort to push the responsibility of deferred
node handling to the SyntaxParseActions which have more detailed
knowledge of their requirements on deferred nodes and might perform
additional optimisations. For example, the SyntaxTreeCreator can always
create RawSyntax nodes (even for deferred nodes) and decide to simply
not use them, should the deferred nodes not get recorded.
@ahoppen ahoppen force-pushed the pr/deferred-nodes-from-parsedrawsyntaxrecorder branch from 0a820fe to ef485d4 Compare March 3, 2021 08:08
@ahoppen
Copy link
Member Author

ahoppen commented Mar 3, 2021

Full testing passed earlier. Just running smoke tests after rebasing onto main and resolving merge conflicts.

@swift-ci Please smoke test

Copy link
Member

@rintaro rintaro left a comment

Choose a reason for hiding this comment

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

the SyntaxTreeCreator can always create RawSyntax nodes (even for deferred nodes) and decide to simply not use them, should the deferred nodes not get recorded.

We should be more careful about enabling/disabling SytnaxContext. For example, isValidTrailingClosure() may consume many tokens. We should not create anything for them.

@ahoppen
Copy link
Member Author

ahoppen commented Mar 3, 2021

We should be more careful about enabling/disabling SytnaxContext. For example, isValidTrailingClosure() may consume many tokens. We should not create anything for them.

I would suggest, we discuss that on the later PR that implements the change. Maybe creating these (potentially unrecorded) nodes in a different arena and then discarding it entirely or not could be an idea. I’ll look into it..

@ahoppen ahoppen merged commit cf543e0 into swiftlang:main Mar 4, 2021
@ahoppen ahoppen deleted the pr/deferred-nodes-from-parsedrawsyntaxrecorder branch March 4, 2021 06:30
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