<!DOCTYPE html>
<html>
<head>
<title></title>
</head>
<body><div>On Sun, Jan 3, 2016, at 09:30 AM, Jesse Rusak wrote:<br></div>
<blockquote type="cite"><div><div><div><div>Some things from Kevin:<br></div>
<div> </div>
<div><blockquote type="cite">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).<br></blockquote></div>
<div> </div>
<div>Can I ask for examples of cases where you’d want to do this intentionally? In those cases, would moving such members (or the protocol conformance) to an extension be a hassle? The point about stored properties is one way this is awkward; are there others?<br></div>
</div>
</div>
</div>
</blockquote><div> </div>
<div>The suggested way to suppress the warning wasn't for this warning. It was for the warning about trying to conform to a protocol but having a member that doesn't actually match the protocol requirements (e.g. having a CollectionType with a `func generate()` that returns a type that doesn't actually conform to `GeneratorType`).<br></div>
<div> </div>
<div>The warning we're discussing here, of attempting to override a protocol extension method even though it's not a member of the protocol, can't work with this suppression mechanism unless the warning simply doesn't work at all for subclasses who inherit the protocol from their superclass. If you inherit the protocol declaration, then you can't possibly put the protocol conformance into its own extension because you're not declaring the conformance to begin with. Even if you ignore classes, the suppression mechanism doesn't really work for this warning anyway as it would suppress a lot of the cases that you actually want to warn about. I say that because it seems quite reasonable for people to declare things like `var last` in an extension on a type (which would trigger this suppression mechanism). This is doubly true for members declared with where clauses, because those are required to be in extensions (and can't declare protocol conformance). So e.g. if I say<br></div>
<div> </div>
<div>struct MyCollection<Base : CollectionType> : CollectionType {<br></div>
<div> // ...<br></div>
<div>}<br></div>
<div> </div>
<div>extension MyCollection where Base.Index : BidirectionalIndexType {<br></div>
<div> var last: Generator.Element? {<br></div>
<div> // ...<br></div>
<div> }<br></div>
<div>}<br></div>
<div> </div>
<div>then, assuming you think the warning is valid to begin with, you'd want to warn about that `var last`.</div>
<div> </div>
<blockquote type="cite"><div><div><div><blockquote type="cite"><div><div>Basically, the warning we're discussing is the same thing as saying "if you have a misconception about how protocol extension methods work, then you might be attempting to do something that won't work", which is basically just a punishment for people who don't have that misconception at all because it means their perfectly legitimate code is throwing warnings because someone happened to declare a protocol extension with the same method. They may not even be aware the extension methods exists at all, which makes the warning even more surprising.<br></div>
</div>
</blockquote><div> </div>
<div>It’s true that if you’re sophisticated enough to understand the behavior and want to take advantage of it, you’d be inconvenienced by a warning here. I think it comes down to the frequency of people intending to do this (who’d have to move some code around?) versus the frequency of people not intending to do this (who'd get potentially-surprising behavior).<br></div>
</div>
</div>
</div>
</blockquote><div> </div>
<div>Since the discussed suppression behavior won't work for this particular warning, it's going to more than just "mov[ing] some code around" to work around it.<br></div>
<div> </div>
<blockquote type="cite"><div><div><div><blockquote type="cite"><div><div>My belief is that a warning in this case would end up being unwanted in a disproportionate number of cases. I believe it's better to clearly differentiate between protocol methods and extension methods and how they behave in our documentation and education for programmers than to add spurious warnings that will annoy everyone who does understand the distinction.<br></div>
</div>
</blockquote><div> </div>
<div>How often are you doing this intentionally in your code? If it’s frequent, we should see if there’s a way to suppress it for your common cases. <br></div>
</div>
</div>
</div>
</blockquote><div> </div>
<div>I don't currently have any examples of this in my code. But I've also never had any cases where the warning would have triggered either. But the standard library provides examples of this!<br></div>
<div> </div>
<div>SequenceType has an extension that declares a `var lazy`. LazySequence also declares `var lazy`. It's obviously not overriding SequenceType's `lazy` property (especially because the type is slightly different), but it is very intentionally trying to shadow it, to make it so anyone who types `foo.lazy.lazy` gets back a `LazySequence<Foo>` instead of a `LazySequence<LazySequence<Foo>>`.<br></div>
<div> </div>
<div>Same thing also happens with `SequenceType.flatMap` and `LazySequence.flatMap`.<br></div>
<div> </div>
<div>CollectionType and LazyCollection is another example. At the very least the `lazy` property is an example. I'm not sure offhand if there's any other examples with those types but I wouldn't be surprised to find that there are.<br></div>
<div> </div>
<div>Set defines a method `contains(_ element:)` that's an exact match for the SequenceType extension method `contains(_ element:)`, but that's not actually an override. It also has `indexOf(_:)` which matches CollectionType's extension method `indexOf(_:)`.<br></div>
<div> </div>
<div>There may be others, I didn't do an exhaustive search of the standard library. But these should illustrate legitimate use-cases for shadowing protocol extension methods.</div>
<div> </div>
<blockquote type="cite"><div><div><div><blockquote type="cite"><div><div>Another concern with this warning is it means that adding a simple `import Foo` to the top of a file can cause my otherwise-valid type definition to sprout warnings (because Foo may declare an extension to a protocol I'm using). I realize there's already cases where adding an import can cause errors (e.g. ambiguity errors), but I think we should think very carefully before adding anything else that causes imports to cause problems. And with the recently-proposed changes to import syntax we can even resolve ambiguity errors at the import line, but this proposed warning couldn't possibly be fixed that way (you can't name extensions so importing a module must pull in all its defined extensions).<br></div>
</div>
</blockquote><div>One way to prevent this warning in general (if we go with the above silencing mechanism) is to declare each protocol conformance in a separate extension with just the methods that implement that protocol. (I think many people do this already for organizational reasons, though with the same caveat about stored properties.) Then importing new modules won't cause additional warnings.<br></div>
</div>
</div>
</div>
</blockquote><div> </div>
<div>As talked about above, this suppression mechanism doesn't work well for this warning, because it will suppress far more than is intended.<br></div>
<div> </div>
<div>-Kevin Ballard<br></div>
</body>
</html>