Skip to content

Make presence a token-only property #609

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 1 commit into from
Aug 23, 2022

Conversation

ahoppen
Copy link
Member

@ahoppen ahoppen commented Aug 19, 2022

It doesn’t really make sense to talk about presence for layout nodes. Are they missing if all of their tokens are missing? What if all tokens are missing but one has trivia attached to it?

To simplify the semantics, define presence for tokens only.

This slightly changes the semantics because layout nodes without any present children are now walked in the sourceAccurate view, but I think that’s a good change.

rdar://98818987

@ahoppen ahoppen requested review from rintaro and bnbarham August 19, 2022 12:18
@ahoppen
Copy link
Member Author

ahoppen commented Aug 22, 2022

@swift-ci Please test

@bnbarham
Copy link
Contributor

Seems reasonable to me. Note this is failing in the stress tester with on StressTesterToolTests.ActionGeneratorTests testRequestActionGenerator. Guess we now need to revert swiftlang/swift-stress-tester#183 😅

It doesn’t really make sense to talk about presence for layout nodes. Are they missing if all of their tokens are missing? What if all tokens are missing but one has trivia attached to it?

To simplify the semantics, define `presence` for tokens only.

This slightly changes the semantics because layout nodes without any present children are now walked in the `sourceAccurate` view, but I think that’s a good change.

rdar://98818987
@ahoppen ahoppen force-pushed the pr/presence-token-only branch from c40c836 to 7b61a46 Compare August 23, 2022 12:51
@ahoppen
Copy link
Member Author

ahoppen commented Aug 23, 2022

swiftlang/swift-stress-tester#187

@swift-ci Please test

@ahoppen ahoppen merged commit 950e97f into swiftlang:main Aug 23, 2022
@ahoppen ahoppen deleted the pr/presence-token-only branch August 23, 2022 21:04
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.

3 participants