> On Apr 25, 2016, at 1:13 PM, Erica Sadun <[email protected]> wrote: > > With apologies, I do not see near miss checks as a pressing need: > > * Quick Help lists of required members (including associated types and > inherited members) would be far more valuable to me than "near miss" > detection.
Understood. > > * I'd like the compiler to check for unsatisfied conformances and emit a list > of required conformances including whether they are found and their origin. > This would tangentially address the "near miss" case but only for unsatisfied > conformances. It would not help when conformances are satisfied. Yes, the experience here is terrible. What the compiler *should* do is give a serious of errors, each of which corresponds to a single requirement and has a Fix-It to stub out the appropriate method/property/etc. > * I believe "near miss" is less important than "intentional override", > requiring a signature of intent as in inheritance. > Here's what you said previously on the topic of default implementation > overrides that brought me here: >> >> This is a commonly-requested feature that I don’t think we need. The TL;DR >> version is that I feel like specifying the conformance explicitly (my type >> Foo conforms to protocol P) already expresses intent, and the compiler >> should help with the rest. I’ve recently been working on providing better >> warnings for cases where one has tried to implement an optional requirement >> for a protocol (but got the declaration wrong), and I think we can turn it >> on for cases where one got a default implementation instead: >> >> http://thread.gmane.org/gmane.comp.lang.swift.devel/1799 >> <http://thread.gmane.org/gmane.comp.lang.swift.devel/1799> My opinion is that the signature of intent is the explicitly-stated conformance to the protocol, but this is not universally agreed upon (even within the core team). It suffices to say that a proposal to add some kind of “implements” keyword won’t come from me :) - Doug > > -- Erica > > >> On Apr 22, 2016, at 4:33 PM, Douglas Gregor via swift-dev >> <[email protected] <mailto:[email protected]>> wrote: >> >> Hi all, >> >> A common complaint with protocol conformance checking is that it’s easy to >> make a little mistake when trying to implement a protocol requirement that >> has a default implementation. Here is a silly example: >> >> protocol P { >> func foo(value: Float) >> } >> >> extension P { >> func foo(value: Float) { } // default implementation of P.foo(value:) >> } >> >> struct S { } >> >> extension S : P { >> func foo(value: Double) { } // intended-but-incorrect attempt to satisfy >> the requirement P.foo(value:) >> } >> >> S satisfies the requirement for P.foo(value:) using the implementation from >> the protocol extension, although it certainly looks like the user intended >> to provide a different implementation. >> >> This kind of problem has prompted repeated calls for some kind of >> “implements” keyword to indicate that one is intending to implement a >> protocol requirement. I, personally, *really* don’t want yet another >> decorator keyword to indicate the intent here, because I believe the user >> has already indicated intent by stating conformance to the protocol P. I’ve >> recently committed a number of changes that provide “near-miss” checking for >> optional requirements of @objc protocols (which have the same problem). >> >> It might be worth enabling this functionality for cases like the above as >> well. The Swift compiler patch to do so is attached, and will produce the >> following warning for the code above: >> >> t2.swift:12:8: warning: instance method 'foo(value:)' nearly matches >> optional requirement 'foo(value:)' of protocol 'P' >> func foo(value: Double) { } >> ^ >> t2.swift:12:8: note: candidate has non-matching type '(value: Double) -> ()' >> func foo(value: Double) { } >> ^ >> t2.swift:12:8: note: move 'foo(value:)' to another extension to silence this >> warning >> func foo(value: Double) { } >> ^ >> t2.swift:12:8: note: make 'foo(value:)' private to silence this warning >> func foo(value: Double) { } >> ^ >> private >> t2.swift:2:8: note: requirement 'foo(value:)' declared here >> func foo(value: Float) >> ^ >> >> It’s unfortunate that it’s a lot of notes. The first says what is wrong with >> the candidate (and there is much we could do to improve the precision of >> this diagnostic!), while the next two are mitigation strategies: move it to >> another extension (which implies that it’s not part of the conformance to P) >> or explicitly mark it as having less visibility than the conformance (in >> this case, private), which feels like a good way to state “I intend this to >> be a helper”. This might nudge Swift developers toward a style where they >> write one conformance per extension, but I don’t think that’s necessarily a >> bad thing: it’s a fine way to organize code. >> >> Naturally, this handles typos as well, e.g., >> >> t2.swift:12:8: warning: instance method 'foob(value:)' nearly matches >> optional requirement 'foo(value:)' of protocol 'P' >> func foob(value: Float) { } >> ^ >> t2.swift:12:8: note: rename to 'foo(value:)' to satisfy this requirement >> func foob(value: Float) { } >> ^~~~ >> foo >> >> Running this on the standard library produces a number of results: >> >> /Users/dgregor/Projects/swift/swift/stdlib/public/core/Arrays.swift.gyb:726:24: >> warning: instance method 'removeLast()' nearly matches optional requirement >> 'removeFirst()' of protocol 'RangeReplaceableCollection' >> public mutating func removeLast() -> Element { >> ^ >> /Users/dgregor/Projects/swift/swift/stdlib/public/core/Arrays.swift.gyb:726:24: >> note: rename to 'removeFirst()' to satisfy this requirement >> public mutating func removeLast() -> Element { >> ^~~~~~~~~~ >> removeFirst >> /Users/dgregor/Projects/swift/swift/stdlib/public/core/Arrays.swift.gyb:726:24: >> note: move 'removeLast()' to another extension to silence this warning >> public mutating func removeLast() -> Element { >> ^ >> /Users/dgregor/Projects/swift/swift/stdlib/public/core/RangeReplaceableCollection.swift:158:17: >> note: requirement 'removeFirst()' declared here >> mutating func removeFirst() -> Iterator.Element >> ^ >> /Users/dgregor/Projects/swift/swift/stdlib/public/core/Arrays.swift.gyb:726:24: >> warning: instance method 'removeLast()' nearly matches optional requirement >> 'removeFirst()' of protocol 'RangeReplaceableCollection' >> public mutating func removeLast() -> Element { >> ^ >> /Users/dgregor/Projects/swift/swift/stdlib/public/core/Arrays.swift.gyb:726:24: >> note: rename to 'removeFirst()' to satisfy this requirement >> public mutating func removeLast() -> Element { >> ^~~~~~~~~~ >> removeFirst >> /Users/dgregor/Projects/swift/swift/stdlib/public/core/Arrays.swift.gyb:726:24: >> note: move 'removeLast()' to another extension to silence this warning >> public mutating func removeLast() -> Element { >> ^ >> /Users/dgregor/Projects/swift/swift/stdlib/public/core/RangeReplaceableCollection.swift:158:17: >> note: requirement 'removeFirst()' declared here >> mutating func removeFirst() -> Iterator.Element >> ^ >> /Users/dgregor/Projects/swift/swift/stdlib/public/core/Arrays.swift.gyb:726:24: >> warning: instance method 'removeLast()' nearly matches optional requirement >> 'removeFirst()' of protocol 'RangeReplaceableCollection' >> public mutating func removeLast() -> Element { >> ^ >> /Users/dgregor/Projects/swift/swift/stdlib/public/core/Arrays.swift.gyb:726:24: >> note: rename to 'removeFirst()' to satisfy this requirement >> public mutating func removeLast() -> Element { >> ^~~~~~~~~~ >> removeFirst >> /Users/dgregor/Projects/swift/swift/stdlib/public/core/Arrays.swift.gyb:726:24: >> note: move 'removeLast()' to another extension to silence this warning >> public mutating func removeLast() -> Element { >> ^ >> /Users/dgregor/Projects/swift/swift/stdlib/public/core/RangeReplaceableCollection.swift:158:17: >> note: requirement 'removeFirst()' declared here >> mutating func removeFirst() -> Iterator.Element >> ^ >> /Users/dgregor/Projects/swift/swift/stdlib/public/core/HashedCollections.swift.gyb:1077:10: >> warning: subscript 'subscript' nearly matches optional requirement >> 'subscript' of protocol 'Collection' >> public subscript(key: Key) -> Value? { >> ^ >> /Users/dgregor/Projects/swift/swift/stdlib/public/core/HashedCollections.swift.gyb:1077:10: >> note: candidate has non-matching type 'Key -> Value?' [with Iterator = >> DictionaryIterator<Key, Value>, SubSequence = Slice<Dictionary<Key, Value>>] >> public subscript(key: Key) -> Value? { >> ^ >> /Users/dgregor/Projects/swift/swift/stdlib/public/core/HashedCollections.swift.gyb:1077:10: >> note: move 'subscript' to an extension to silence this warning >> public subscript(key: Key) -> Value? { >> ^ >> /Users/dgregor/Projects/swift/swift/stdlib/public/core/Collection.swift:147:3: >> note: requirement 'subscript' declared here >> subscript(bounds: Range<Index>) -> SubSequence { get } >> ^ >> /Users/dgregor/Projects/swift/swift/stdlib/public/core/Range.swift:116:10: >> warning: subscript 'subscript' nearly matches optional requirement >> 'subscript' of protocol 'Collection' >> public subscript(_: Element._DisabledRangeIndex) -> Element { >> ^ >> /Users/dgregor/Projects/swift/swift/stdlib/public/core/Range.swift:116:10: >> note: candidate has non-matching type 'Element._DisabledRangeIndex -> >> Element' [with Iterator = RangeIterator<Element>, SubSequence = >> Slice<Range<Element>>] >> public subscript(_: Element._DisabledRangeIndex) -> Element { >> ^ >> /Users/dgregor/Projects/swift/swift/stdlib/public/core/Range.swift:116:10: >> note: move 'subscript' to an extension to silence this warning >> public subscript(_: Element._DisabledRangeIndex) -> Element { >> ^ >> /Users/dgregor/Projects/swift/swift/stdlib/public/core/Collection.swift:147:3: >> note: requirement 'subscript' declared here >> subscript(bounds: Range<Index>) -> SubSequence { get } >> ^ >> /Users/dgregor/Projects/swift/swift/stdlib/public/core/ExistentialCollection.swift.gyb:508:15: >> warning: instance method '_distance(to:)' nearly matches optional >> requirement 'distance(to:)' of protocol 'ForwardIndex' >> public func _distance(to other: AnyForwardIndex) -> >> AnyForwardIndex.Distance { >> ^ >> /Users/dgregor/Projects/swift/swift/stdlib/public/core/ExistentialCollection.swift.gyb:508:15: >> note: rename to 'distance(to:)' to satisfy this requirement [with >> _DisabledRangeIndex = _DisabledRangeIndex_] >> public func _distance(to other: AnyForwardIndex) -> >> AnyForwardIndex.Distance { >> ^~~~~~~~~ >> distance >> /Users/dgregor/Projects/swift/swift/stdlib/public/core/ExistentialCollection.swift.gyb:508:15: >> note: move '_distance(to:)' to an extension to silence this warning >> public func _distance(to other: AnyForwardIndex) -> >> AnyForwardIndex.Distance { >> ^ >> /Users/dgregor/Projects/swift/swift/stdlib/public/core/Index.swift:180:8: >> note: requirement 'distance(to:)' declared here >> func distance(to end: Self) -> Distance >> ^ >> /Users/dgregor/Projects/swift/swift/stdlib/public/core/ExistentialCollection.swift.gyb:508:15: >> warning: instance method '_distance(to:)' nearly matches optional >> requirement 'distance(to:)' of protocol 'ForwardIndex' >> public func _distance(to other: AnyBidirectionalIndex) -> >> AnyBidirectionalIndex.Distance { >> ^ >> /Users/dgregor/Projects/swift/swift/stdlib/public/core/ExistentialCollection.swift.gyb:508:15: >> note: rename to 'distance(to:)' to satisfy this requirement [with >> _DisabledRangeIndex = _DisabledRangeIndex_] >> public func _distance(to other: AnyBidirectionalIndex) -> >> AnyBidirectionalIndex.Distance { >> ^~~~~~~~~ >> distance >> /Users/dgregor/Projects/swift/swift/stdlib/public/core/ExistentialCollection.swift.gyb:508:15: >> note: move '_distance(to:)' to an extension to silence this warning >> public func _distance(to other: AnyBidirectionalIndex) -> >> AnyBidirectionalIndex.Distance { >> ^ >> /Users/dgregor/Projects/swift/swift/stdlib/public/core/Index.swift:180:8: >> note: requirement 'distance(to:)' declared here >> func distance(to end: Self) -> Distance >> ^ >> >> >> It’s somewhat frustrating that these are *all* false positives. However, >> they seem like “reasonable” false positives, in the sense that they’re close >> enough to the requirement to be justifiable, and the suggested recovery >> strategies look acceptable. >> >> Thoughts? Should we turn this on? >> >> - Doug >> >> <default-impl-near-miss.patch> >> _______________________________________________ >> swift-dev mailing list >> [email protected] <mailto:[email protected]> >> https://lists.swift.org/mailman/listinfo/swift-dev >
_______________________________________________ swift-evolution mailing list [email protected] https://lists.swift.org/mailman/listinfo/swift-evolution
