Skip to content

Explicitly State CodingKeys as a Codable Requirement #28665

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 6 commits into from
Dec 11, 2019

Conversation

CodaFi
Copy link
Contributor

@CodaFi CodaFi commented Dec 9, 2019

Refactors the way we model the CodingKeys "requirement" in Codable. The idea is that it should be cheap and easy to synthesize CodingKeys. But it should also be clear that Codable synthesis has the unique ability to synthesize type witnesses to requirements that don't formally exist, hence "Phantom Requirements". This patch adds a new, terrifyingly-named, user-inaccessible attribute @_implicitly_synthesizes_nested_requirement, and attaches it to Encodable and Decodable. The synthesis machinery looks for the attribute and, if it must synthesize a value witness, attempts to install the phantom witness first.

This pull request and #28624 together will let us eliminate ImplicitMemberAction::ResolveEncodable and ImplicitMemberAction::ResolveDecodable.

Relates to rdar://56844567

@CodaFi
Copy link
Contributor Author

CodaFi commented Dec 9, 2019

@swift-ci please test

@CodaFi
Copy link
Contributor Author

CodaFi commented Dec 9, 2019

@swift-ci please test source compatibility

@CodaFi CodaFi requested a review from DougGregor December 9, 2019 23:14
@swift-ci
Copy link
Contributor

Build failed
Swift Test Linux Platform
Git Sha - 0bcc6864a307d0fad01bb22cbbcb6067b246bfe3

@CodaFi CodaFi force-pushed the the-phantom-menace branch from 0bcc686 to d2136a3 Compare December 10, 2019 19:02
@CodaFi
Copy link
Contributor Author

CodaFi commented Dec 10, 2019

@swift-ci please test

@CodaFi
Copy link
Contributor Author

CodaFi commented Dec 10, 2019

@swift-ci please test source compatibility

@swift-ci
Copy link
Contributor

Build failed
Swift Test OS X Platform
Git Sha - d2136a384ba3ba0bc3afec3fe007c4e22484f2da

@CodaFi
Copy link
Contributor Author

CodaFi commented Dec 10, 2019

Infrastructure problems with the macOS build

@swift-ci please smoke test macOS platform

State the previously unstated nested type requirement that CodingKeys adds to the witness requirements of a given type. The goal is to make this member cheap to synthesize, and independent of the expensive protocol conformance checks required to append it to the member list.

Further, this makes a clean conceptual separation between what I'm calling "nested type requirements" and actual type and value requirements.

With luck, we'll never have to use this attribute anywhere else.
CodableConformanceType::TypeNotValidated is unused since the InterfaceTypeRequest changes went through.  We can just use the validity of the ProtocolConformanceRef here.
The semantic checks for CodingKeys are being duplicated across the value witness synthesis code paths.  Just synthesize a CodingKeys enum and let validateCodingKeysEnum do the heavy lifting when we actually need to go emit diagnostics.
The presence of this attribute will be used to guide the protocol derivation machinery to synthesize CodingKeys.
@CodaFi CodaFi force-pushed the the-phantom-menace branch from d2136a3 to 3bda241 Compare December 11, 2019 00:29
@CodaFi
Copy link
Contributor Author

CodaFi commented Dec 11, 2019

Let's get this merged

@swift-ci please smoke test

@CodaFi
Copy link
Contributor Author

CodaFi commented Dec 11, 2019

⛵️

@CodaFi CodaFi merged commit 43a3ab7 into swiftlang:master Dec 11, 2019
@CodaFi CodaFi deleted the the-phantom-menace branch December 11, 2019 05:06
beccadax added a commit to beccadax/swift that referenced this pull request Jan 15, 2020
…nace"

This reverts commit 43a3ab7, reversing
changes made to 4f39d9c.

# Conflicts:
#	include/swift/AST/Attr.def
#	lib/AST/Attr.cpp
#	lib/Serialization/Deserialization.cpp
#	lib/Serialization/ModuleFormat.h
#	lib/Serialization/Serialization.cpp
beccadax added a commit that referenced this pull request Jan 16, 2020
Revert "Merge pull request #28665 from CodaFi/the-phantom-menace"
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