-
Notifications
You must be signed in to change notification settings - Fork 439
Teach TokenSpecSet about experimental features #2211
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
I can't say I love having to pass down experimental features to each TokenSpecSet, but I think it's ultimately better having them automatically handle experimental features instead of needing ad-hoc checks in the parser which, as demonstrated recently, can be error prone |
840a614
to
287717d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for doing this. I agree that passing the experimental feature list to TokenSpecSet
isn’t great but it’s definitely less error-prone.
I measured performance and couldn’t see any regression when parsing NetNewsWire.
Pass down the current set of experimental features to TokenSpecSets, and have generated TokenSpecSets automatically account for experimental features by returning `nil` if a given feature isn't active.
We now check these with the spec set.
287717d
to
4695dc7
Compare
@swift-ci please test |
@swift-ci please test macOS |
Pass down the current set of experimental features to TokenSpecSets, and have generated TokenSpecSets automatically account for experimental features by returning
nil
if a given feature isn't active. Should be NFC.