<!DOCTYPE html>
<html>
<head>
<title></title>
</head>
<body><div>On Sat, Jan 2, 2016, at 11:39 AM, Douglas Gregor via swift-dev wrote:<br></div>
<blockquote type="cite"><div> </div>
<div><div> </div>
<div style="direction:ltr;"><blockquote type="cite"><div>On Dec 31, 2015, at 3:15 PM, Jesse Rusak <<a href="mailto:me@jesserusak.com">me@jesserusak.com</a>> wrote:<br></div>
<div> </div>
<div><div><div>Hi Doug,<br></div>
<div> </div>
<div>I’ve been playing around with an implementation of the warning you referenced here: <a href="https://lists.swift.org/pipermail/swift-evolution/Week-of-Mon-20151207/001584.html">https://lists.swift.org/pipermail/swift-evolution/Week-of-Mon-20151207/001584.html</a><br></div>
<div> </div>
<div>Would it be helpful for me to take this on?<br></div>
</div>
</div>
</blockquote><div style="direction:ltr;"> </div>
<div style="direction:ltr;">Yes, absolutely!<br></div>
<div> </div>
<blockquote type="cite"><div><div>If so, is there any detail in the radar assigned to you about what exactly should trigger such a warning?<br></div>
</div>
</blockquote><blockquote type="cite"><div><div>For example, I have it currently triggering whenever there’s a method with a matching name, ignoring parameter/return types; it’s not obvious to me how closely they should have to match, if at all, to trigger the warning.<br></div>
</div>
</blockquote></div>
<div style="direction:ltr;">I just realized that I wrote up a big discussion of a related-but-not-identical warning. In either case, there is some kind of radar, and neither gives a lot of detail. <br></div>
<div style="direction:ltr;"> </div>
<div style="direction:ltr;">In general: just matching on name alone feels like it might produce too many false positives, but exactly matching parameter/return types feels like it might miss cases where this warning would be important, so… I think it’s going to come down to coming up with cases where you do/don’t want to see the warning and tuning the warning to do the right thing. It might be that you want to do some simplistic matching (perhaps akin to what lib/Sema/TypeCheckProtocol.cpp does when inferring type witnesses) that ignores uses of associated types that might have been deduced differently from what the user expected.<br></div>
<div style="direction:ltr;"> </div>
<div style="direction:ltr;">That leads to my #1 example I’d love to get a warning for, which is when you intended to provide something to satisfy a protocol requirement, but you ended up getting a default implementation:<br></div>
<div style="direction:ltr;"> </div>
<blockquote style="margin-top:0px;margin-right:0px;margin-bottom:0px;margin-left:40px;border-top-style:none;border-right-style:none;border-bottom-style:none;border-left-style:none;border-top-width:initial;border-right-width:initial;border-bottom-width:initial;border-left-width:initial;border-top-color:initial;border-right-color:initial;border-bottom-color:initial;border-left-color:initial;border-image-source:initial;border-image-slice:initial;border-image-width:initial;border-image-outset:initial;border-image-repeat:initial;padding-top:0px;padding-right:0px;padding-bottom:0px;padding-left:0px;"><div style="direction:ltr;"><div style="direction:ltr;">struct MyGenerator {<br></div>
</div>
<div style="direction:ltr;"><div style="direction:ltr;"> mutating func next() -> Int? { return nil }<br></div>
</div>
<div style="direction:ltr;"><div style="direction:ltr;">}<br></div>
</div>
<div style="direction:ltr;"><div style="direction:ltr;"> </div>
</div>
<div style="direction:ltr;"><div style="direction:ltr;">struct MyCollection : CollectionType {<br></div>
</div>
<div style="direction:ltr;"><div style="direction:ltr;"> typealias Index = Int<br></div>
</div>
<div style="direction:ltr;"><div style="direction:ltr;"> </div>
</div>
<div style="direction:ltr;"><div style="direction:ltr;"> var startIndex: Int { return 0 }<br></div>
</div>
<div style="direction:ltr;"><div style="direction:ltr;"> var endIndex: Int { return 10 }<br></div>
</div>
<div style="direction:ltr;"><div style="direction:ltr;"> </div>
</div>
<div style="direction:ltr;"><div style="direction:ltr;"> subscript (index: Int) -> Int {<br></div>
</div>
<div style="direction:ltr;"><div style="direction:ltr;"> return index<br></div>
</div>
<div style="direction:ltr;"><div style="direction:ltr;"> }<br></div>
</div>
<div style="direction:ltr;"><div style="direction:ltr;"> </div>
</div>
<div style="direction:ltr;"><div style="direction:ltr;"> func generate() -> MyGenerator {<br></div>
</div>
<div style="direction:ltr;"><div style="direction:ltr;"> print("using MyGenerator")<br></div>
</div>
<div style="direction:ltr;"><div style="direction:ltr;"> return MyGenerator()<br></div>
</div>
<div style="direction:ltr;"><div style="direction:ltr;"> }<br></div>
</div>
<div style="direction:ltr;"><div style="direction:ltr;">}<br></div>
</div>
<div style="direction:ltr;"><div style="direction:ltr;"> </div>
</div>
<div style="direction:ltr;"><div style="direction:ltr;">func foo<C: CollectionType>(c: C) {<br></div>
</div>
<div style="direction:ltr;"><div style="direction:ltr;"> c.generate()<br></div>
</div>
<div style="direction:ltr;"><div style="direction:ltr;">}<br></div>
</div>
<div style="direction:ltr;"><div style="direction:ltr;"> </div>
</div>
<div style="direction:ltr;"><div style="direction:ltr;">foo(MyCollection())<br></div>
</div>
</blockquote><div style="direction:ltr;"> </div>
<div style="direction:ltr;">Note that there is no output from this program, although one would expect to see “using MyGenerator”.<br></div>
<div style="direction:ltr;"> </div>
<div style="direction:ltr;">The root of the problem is annoying simple (I “forgot” to state that MyGenerator conforms to GeneratorType). The result is that the implied SequenceType conformance gets a default implementation of “generate” from a protocol extension in the standard library (that produces default generator for any SequenceType that is also a CollectionType). Our place to warn about this is at the point where we decide to use a “generate” from a protocol extension rather than the “generate” in the same “struct” that declares conformance to CollectionType. Obviously, lots of bonus points if we could say why the generate() in the struct wasn’t picked :)<br></div>
<div style="direction:ltr;"> </div>
<div style="direction:ltr;">That brings up another point about warnings: it’s useful to have a way to suppress them. Let’s say we got a warning for my example above (huzzah!) but I wanted to silence it. A fairly natural way to do so would be to move the “generate” function I defined into a separate extension, so it’s away from where we state conformance to CollectionType:<br></div>
<div style="direction:ltr;"> </div>
<div style="direction:ltr;"> </div>
<blockquote style="margin-top:0px;margin-right:0px;margin-bottom:0px;margin-left:40px;border-top-style:none;border-right-style:none;border-bottom-style:none;border-left-style:none;border-top-width:initial;border-right-width:initial;border-bottom-width:initial;border-left-width:initial;border-top-color:initial;border-right-color:initial;border-bottom-color:initial;border-left-color:initial;border-image-source:initial;border-image-slice:initial;border-image-width:initial;border-image-outset:initial;border-image-repeat:initial;padding-top:0px;padding-right:0px;padding-bottom:0px;padding-left:0px;"><div style="direction:ltr;"><div style="direction:ltr;"><div style="direction:ltr;">struct MyGenerator {<br></div>
</div>
</div>
<div style="direction:ltr;"><div style="direction:ltr;"><div style="direction:ltr;"> mutating func next() -> Int? { return nil }<br></div>
</div>
</div>
<div style="direction:ltr;"><div style="direction:ltr;"><div style="direction:ltr;">}<br></div>
</div>
</div>
<div style="direction:ltr;"><div style="direction:ltr;"><div style="direction:ltr;"> </div>
</div>
</div>
<div style="direction:ltr;"><div style="direction:ltr;"><div style="direction:ltr;">struct MyCollection : CollectionType {<br></div>
</div>
</div>
<div style="direction:ltr;"><div style="direction:ltr;"><div style="direction:ltr;"> typealias Index = Int<br></div>
</div>
</div>
<div style="direction:ltr;"><div style="direction:ltr;"><div style="direction:ltr;"> </div>
</div>
</div>
<div style="direction:ltr;"><div style="direction:ltr;"><div style="direction:ltr;"> var startIndex: Int { return 0 }<br></div>
</div>
</div>
<div style="direction:ltr;"><div style="direction:ltr;"><div style="direction:ltr;"> var endIndex: Int { return 10 }<br></div>
</div>
</div>
<div style="direction:ltr;"><div style="direction:ltr;"><div style="direction:ltr;"> </div>
</div>
</div>
<div style="direction:ltr;"><div style="direction:ltr;"><div style="direction:ltr;"> subscript (index: Int) -> Int {<br></div>
</div>
</div>
<div style="direction:ltr;"><div style="direction:ltr;"><div style="direction:ltr;"> return index<br></div>
</div>
</div>
<div style="direction:ltr;"><div style="direction:ltr;"><div style="direction:ltr;"> }<br></div>
</div>
</div>
<div style="direction:ltr;"><div style="direction:ltr;"><div style="direction:ltr;">}<br></div>
<div style="direction:ltr;"> </div>
<div style="direction:ltr;">extension MyCollection {<br></div>
</div>
</div>
<div style="direction:ltr;"><div style="direction:ltr;"><div style="direction:ltr;"> func generate() -> MyGenerator { // no warning<br></div>
</div>
</div>
<div style="direction:ltr;"><div style="direction:ltr;"><div style="direction:ltr;"> print("using MyGenerator")<br></div>
</div>
</div>
<div style="direction:ltr;"><div style="direction:ltr;"><div style="direction:ltr;"> return MyGenerator()<br></div>
</div>
</div>
<div style="direction:ltr;"><div style="direction:ltr;"><div style="direction:ltr;"> }<br></div>
</div>
</div>
<div style="direction:ltr;"><div style="direction:ltr;"><div style="direction:ltr;">}<br></div>
<div style="direction:ltr;"> </div>
</div>
</div>
</blockquote><div>Effectively, we’re using the declaration of conformance to a protocol as indicating user intent that the contents of this particular definition/extension involve that conformance.<br></div>
</div>
</blockquote><div> </div>
<div>This isn't going to work well for cases where the protocol declares a property (with a default computed property impl) and a conforming type attempts to use a stored property. Stored properties must be declared in the initial type declaration, not in extensions. The only way to suppress it then would be to suppress the warning in general for members provided in the initial type declaration when the protocol conformance is part of an extension (sort of the opposite of the suggested way of suppressing warnings), and maybe that's fine, but it does mean that there's no way to suppress the warning for a single stored property without suppressing it for all stored properties.<br></div>
<div> </div>
<blockquote type="cite"><div><div>The actual warning you are talking about is very, very similar, and would likely use most of the same logic. The part that differs is the trigger: whenever one declares something within a type, perform a lookup in that type to determine whether there are any similar members of extensions of one of the protocols that type conforms to. You'll want to think about how to suppress the warning when the user wants to. <br></div>
</div>
</blockquote><div> </div>
<div>The warning you documented in most of this email seems perfectly reasonable; if I declare a function that looks like it's meant to match a protocol method, there's a good chance I expected it to actually do so (and, as you suggested, there's a reasonable way to suppress it for methods).<br></div>
<div> </div>
<div>But the warning that Jesse was talking about, the one that was discussed in the older threads, I think is completely different. If I declare a method on my type that matches a protocol extension method, it is <i>not</i> safe to assume I was trying to override the (non-overridable) method. It's very likely that I'm simply providing a specialization of the method that will be called by anyone who's working with my type directly (with the acknowledgement that working with the type generically won't call my type's version). Not only that, but it's not even possible by looking at the definition of the protocol to determine if any given method I add to my type will even trigger a warning! The warning will be triggered by the existence of any visible extension to the protocol, even one the programmer isn't aware of.<br></div>
<div> </div>
<div>I think a much better approach to handling this is simply to update the documentation to make it much more obvious which methods/properties are "part of" the protocol and which are extensions. The current stdlib docs (in the iOS Developer Library) adds the text "Default implementation" next to any default method, but it doesn't actually separate out the default implementations from the required ones. This also means that when Xcode shows the method list in the documentation viewer there's no indication as to which ones are default and which are not. <a href="http://swiftdoc.org">http://swiftdoc.org</a> provides a <i>much</i> better display where "Instance methods" are in a separate section from "Default Implementations" (see <a href="http://swiftdoc.org/v2.1/protocol/SequenceType/">http://swiftdoc.org/v2.1/protocol/SequenceType/</a> for an example).<br></div>
<div> </div>
<div>-Kevin Ballard</div>
</body>
</html>