Skip to content

[Parsing] NFC: metaprogram contextual decl keywords into Attr.def #15818

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

davezarzycki
Copy link
Contributor

@davezarzycki davezarzycki commented Apr 7, 2018

This makes it easier to add new contextual keywords to the compiler after updating Attr.def.

NEEDS REVIEW: open and __consuming were inconsistently handled across the compiler. Was/is this intentional?

This passes validation testing on my Linux box.

@davezarzycki
Copy link
Contributor Author

@swift-ci please smoke test

@davezarzycki
Copy link
Contributor Author

Hi @slavapestov – Thanks for the positive review! It looks like SourceKit didn't deal with __consuming, indirect, or open. Is this a SourceKit bug? Should I just fix the test?

@davezarzycki davezarzycki force-pushed the nfc_metaprogram_contextual_decl_keywords branch from 96b78d1 to f346f05 Compare April 7, 2018 21:20
@davezarzycki
Copy link
Contributor Author

@swift-ci please smoke test

@davezarzycki davezarzycki force-pushed the nfc_metaprogram_contextual_decl_keywords branch from f346f05 to 00edd55 Compare April 7, 2018 21:25
@davezarzycki
Copy link
Contributor Author

I made a mistake with git commit --amend. Let's try this again:

@swift-ci please smoke test

@davezarzycki davezarzycki force-pushed the nfc_metaprogram_contextual_decl_keywords branch from 00edd55 to 7174d14 Compare April 7, 2018 22:00
@davezarzycki
Copy link
Contributor Author

@swift-ci please smoke test

@davezarzycki davezarzycki merged commit d9f7d14 into swiftlang:master Apr 8, 2018
@davezarzycki davezarzycki deleted the nfc_metaprogram_contextual_decl_keywords branch April 8, 2018 10:52
@davezarzycki
Copy link
Contributor Author

I feel more confident now that this was merely a SourceKit bug. We can always revert this if somebody complains.

@nkcsgexi
Copy link
Contributor

nkcsgexi commented Apr 9, 2018

Yeah, I agree this is also a SourceKit enhancement. Thank you for fixing it, @davezarzycki !

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