-
Notifications
You must be signed in to change notification settings - Fork 10.4k
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
Sema: Gut synthesizeMemberForLookup() #16054
Sema: Gut synthesizeMemberForLookup() #16054
Conversation
43f77de
to
baa01da
Compare
@swift-ci Please smoke test |
baa01da
to
b2d0124
Compare
@swift-ci Please test source compatibility |
@swift-ci Please test |
Build failed |
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.
Looks good from my (limited) end! Significant cleanup here, and it's more elegant to boot. Thanks 🙂
… checking This is a more elegant path to ensuring that init(from:) and encode(to:) get synthesized than synthesizeMemberForLookup().
…irements Since init(from:) and encode(to:) are protocol requirements, any name lookup of them will find the protocol requirement, and if the witness is not present we will synthesize it on the spot. Only CodingKeys requires special handling here, since it is not a protocol requirement, but has to get synthesized anyway.
b2d0124
to
d8b4d99
Compare
Thanks for taking a look! As you’ve noticed this cleanup caused a test failure. I merged the other fixes though since they don’t depend on this, they should all be in swift-4.2-branch now. I’ll revisit this PR later. |
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.
LGTM! Small nit - there is a typo (than
right before synthesizeMemberForLookup
) in the first commit message.
More cleanups based off of #16053.