Support synthesis of protocol conformances (including conditional ones) in extensions #16376
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@swift-ci please test |
|
@swift-ci please test source compatibility |
|
Build failed |
|
For future reference/if the builds disappear before I get back to this: Swift(linux-x86_64).stdlib.KeyPath.swiftSwift(linux-x86_64).stdlib.KeyPathImplementation.swift(They could be constantly failing, from a quick glance at the other builds.) |
|
Build failed |
itaiferber
approved these changes
May 4, 2018
itaiferber
reviewed
May 4, 2018
| "implementation of %0 cannot be automatically synthesized in an extension", (Type)) | ||
| ERROR(cannot_synthesize_init_in_extension_of_nonfinal,none, | ||
| "implementation of %0 for non-final class cannot be automatically " | ||
| "synthesized in an extension due to 'init' requirement", |
There was a problem hiding this comment.
One question here — excuse my mental lapse, but what is preventing us from synthesizing these? (I suspect that folks are going to hit this and be confused; we might want to be a bit more specific)
There was a problem hiding this comment.
In non-final classes, the inits satisfying protocol requirements must be required and those can't occur in extensions. I phrase the message this way because I couldn't get something more explanatory to be shorter, and it was pointed out to me that error messages aren't the place to teach the language. I'd love some alternative phrasings though, do you have any ideas?
There was a problem hiding this comment.
Ah, duh 🤦♂️ I haven't thought about it much, but I think cribbing some of the messaging we already have (e.g. the existing "error: initializer requirement 'init(foo:)' can only be satisfied by a required initializer in the definition of non-final class 'Foo'") can help.
For instance, "implementation of %0 for non-final class '%1' cannot be synthesized because initializer requirement 'required %2' cannot be implemented in an extension" gets the user a little bit closer to understanding why this can't be done. Of course, the next question people have is "why does Swift treat a same-file extension any differently than writing things in the original declaration", but that's something for another day. 😅
There was a problem hiding this comment.
Yeah, that's a great idea! I've gone with:
implementation of 'Decodable' for non-final class cannot be automatically synthesized in extension because initializer requirement 'init(from:)' can only be be satisfied by a 'required' initializer in the class definition
|
🎉 this is huge |
|
@swift-ci please test OSX platform |
|
@swift-ci please test OS X platform |
1 similar comment
|
@swift-ci please test OS X platform |
Instead of passing around a TypeChecker and three Decls (the nominal type, the protocol, and the decl declaring the conformance) everywhere, we can just pass one object. This should be [NFC].
This works for all protocols except for Decodable on non-final classes, because the init requirement has to be 'required' and thus in the type's declaration. Fixes most of https://bugs.swift.org/browse/SR-6803.
…c types. Fixes https://bugs.swift.org/browse/SR-6803 and rdar://problem/39199726.
huonw
force-pushed
the
same-file-extension-restructure
branch
from
4ec037f
to
27d8f17
Compare
|
@swift-ci please test |
|
Build failed |
|
Build failed |
|
@swift-ci please test |
DougGregor
reviewed
May 7, 2018
| @@ -1193,9 +1172,9 @@ static bool canSynthesize(TypeChecker &tc, NominalTypeDecl *target, | |||
| diag::decodable_super_init_not_designated_here, | |||
| requirement->getFullName(), memberName); | |||
| return false; | |||
| } else if (!initializer->isAccessibleFrom(target)) { | |||
| } else if (!initializer->isAccessibleFrom(classDecl)) { | |||
There was a problem hiding this comment.
I think this should be initializer->isAccessibleFrom(conformanceDC), although I'm not convinced that it will ever matter.
| // Cannot call an inaccessible method. | ||
| auto accessScope = initializer->getFormalAccessScope(target); | ||
| auto accessScope = initializer->getFormalAccessScope(classDecl); |
| tc.diagnose(requirement, diag::no_witnesses, diag::RequirementKind::Func, | ||
| requirement->getFullName(), encodableType, /*AddFixIt=*/false); | ||
| auto diagnosticTransaction = DiagnosticTransaction(TC.Context.Diags); | ||
| TC.diagnose(Nominal, diag::type_does_not_conform, Nominal->getDeclaredType(), |
There was a problem hiding this comment.
Don't we want this diagnostic to point at the place where the conformance was declared, rather than always pointing at the nominal type declaration?
| decodableType); | ||
| tc.diagnose(requirement, diag::no_witnesses, | ||
| auto diagnosticTransaction = DiagnosticTransaction(TC.Context.Diags); | ||
| TC.diagnose(Nominal, diag::type_does_not_conform, Nominal->getDeclaredType(), |
There was a problem hiding this comment.
Same comment regarding the location of the diagnostic.
| @@ -635,14 +635,15 @@ deriveEquatable_eq(TypeChecker &tc, Decl *parentDecl, NominalTypeDecl *typeDecl, | |||
| DeclNameLoc())); | |||
|
|
|||
| if (!C.getEqualIntDecl()) { | |||
| tc.diagnose(parentDecl->getLoc(), diag::no_equal_overload_for_int); | |||
| derived.TC.diagnose(derived.ConformanceDecl->getLoc(), | |||
There was a problem hiding this comment.
Ah, this is the location I was expecting for the other diagnostics above.
| if (!tc.conformsToProtocol(intType, intLiteralProto, typeDecl, None)) { | ||
| tc.diagnose(typeDecl->getLoc(), | ||
| if (!tc.conformsToProtocol(intType, intLiteralProto, derived.Nominal, None)) { | ||
| tc.diagnose(derived.Nominal->getLoc(), |
There was a problem hiding this comment.
This diagnostic probably never fires, but derived.ConformanceDecl would be more appropriate for these two uses.
| @@ -1110,15 +1095,17 @@ deriveHashable_hashValue(TypeChecker &tc, Decl *parentDecl, | |||
|
|
|||
| getterDecl->setInterfaceType(interfaceType); | |||
| getterDecl->setValidationStarted(); | |||
| getterDecl->copyFormalAccessFrom(typeDecl, /*sourceIsParentContext*/true); | |||
| getterDecl->copyFormalAccessFrom(derived.Nominal, | |||
There was a problem hiding this comment.
Same comment about using derived.ConformanceDecl.
|
|
||
| // Finish creating the property. | ||
| hashValueDecl->setImplicit(); | ||
| hashValueDecl->setInterfaceType(intType); | ||
| hashValueDecl->setValidationStarted(); | ||
| hashValueDecl->makeComputed(SourceLoc(), getterDecl, | ||
| nullptr, nullptr, SourceLoc()); | ||
| hashValueDecl->copyFormalAccessFrom(typeDecl, /*sourceIsParentContext*/true); | ||
| hashValueDecl->copyFormalAccessFrom(derived.Nominal, |
|
|
||
| VarDecl *propDecl = new (C) VarDecl(/*IsStatic*/isStatic, VarDecl::Specifier::Var, | ||
| /*IsCaptureList*/false, SourceLoc(), name, | ||
| propertyContextType, parentDC); | ||
| propDecl->setImplicit(); | ||
| propDecl->copyFormalAccessFrom(typeDecl, /*sourceIsParentContext*/true); | ||
| propDecl->copyFormalAccessFrom(Nominal, /*sourceIsParentContext*/ true); |
| } | ||
| } | ||
|
|
||
| return false; |
There was a problem hiding this comment.
In Swift 3 mode, we should disallow synthesis of conformances declared on extensions, because the Swift 3 meaning of private doesn't jive well with synthesis, e.g.,
struct X {
private var foo: Int
}
extension X: Equatable { /* cannot access foo in Swift 3! */ }Developers can migrate to Swift 4.x to get this feature.
This was referenced May 8, 2018
This was referenced May 9, 2018
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This works for
Equatable,Hashable,Encodable,Decodable(and thusCodable), andCaseIterable. For instance,Fixes rdar://problem/39199726 and https://bugs.swift.org/browse/SR-6803