10 ms·

Swift: When Unused Code Is a Bug

137 points by ileitch 8y ago

wiradikusuma 8y ago
I wonder if this behavior is working as intended. It does look like a bug to the language itself, isn't?
- mgoblu3 8y ago
  There are some rules around this, as noted in https://nomothetis.svbtle.com/the-ghost-of-swift-bugs-future https://nomothetis.svbtle.com/the-ghost-of-swift-bugs-future: IF the inferred type of a variable is the protocol: AND the method is defined in the original protocol THEN the runtime type’s implementation is called, irrespective of whether there is a default implementation in the extension. AND the method is not defined in the original protocol, THEN the default implementation is called. ELSE IF the inferred type of the variable is the type THEN the type’s implementation is called.
- anon180719 8y ago
  It appears the correct action would have been to use <code>override func greet() { print("sup") }</code>. To be fair, I was primed to really think about the code execution given he'd indicated there was a trap in there somewhere, so it certainly would not have been immediately obvious on first glance. But I think, generally developers understand that <code>override</code> is necessary to override code from a parent class. Typically XCode helps out with this sort of thing – adding <code>override</code> where it seems like it should be added – but perhaps not when a parent class is using a default protocol implementation. But again, in this scenario where a class is inheriting from another class, it's typically known you need to override the function in order to use your own implementation, so this does seem a bit contrived.
  - joeblau 8y ago
    > It appears the correct action would have been to use <code>override func greet() { print("sup") }</code>. That actually doesn't work because the greet function is implemented on an extension. You can only override class functions.
    - anon180719 8y ago
      Oh, that's definitely weird then. Is there an RFC to have that fixed in some way?
      - joeblau 8y ago
        I don't think there is. I think the code is functioning as expected — it's just written in a super confusing way. What should probably happen is that the compiler should throw an error about redefining a function in a super classes extension. I'm not a compiler engineer but a check for that seems like it would be intensive unless that information is codified in the AST somehow.
        anon180719 8y ago
        hmph. I agree, the compiler should probably not let you compile in that case. Funny enough this reminds me of "responder chain politics" in dynamically typed languages with duck typing.
        slavapestov 8y ago
        It's a bug - https://bugs.swift.org/browse/SR-103 https://bugs.swift.org/browse/SR-103
mgoblu3 8y ago
Default implementations on protocols in Swift definitely can be a little dangerous. We've shied our team away from doing this unless the protocol is explicitly used as a mix-in type. There's been some proposals around fixing these, one that comes to mind is: https://forums.swift.org/t/introducing-role-keywords-to-reduce-hard-to-find-bugs/6113 https://forums.swift.org/t/introducing-role-keywords-to-redu...
- alrz 8y ago
  This doesn't seem to be about default implementations on protocols, I guess they do actually do virtual dispatch so you wouldn't hit this bug with default impls. The main issue is that we have an extension method on a type with the same name of a member of that type. So it can always be statically flagged whether or not the method is used anywhere else. This is easily reproducible in any language that supports extension methods (C#, for instance). I wonder if that could be useful at all.
Chazprime 8y ago
I've run afoul of this a couple of times in my Swift delegate protocols. A really nice hack to implement Objective-C's optional protocol methods in Swift is to use a protocol extension to create a default method that can come back to bite you when you subclass objects that conform to that protocol.
- otaviokz 8y ago
  Yep, the old `fatalError("You need to implement this!")`.
untog 8y ago
I'm a little undecided on Swift extensions as a whole, and particularly protocol extensions. Maybe I'm just paranoid but it feels like it leads to a more complicated code layout.
- iforgotpassword 8y ago
  I agree. I never worked with swift and had to look up the terminology. Most of what the article refers to maps to terms from C++ (which is already horribly complicated and offers way too many ways to shoot yourself in the foot) or Java. That PWT sounds an awful lot like a vtable. In C++ you mess up by forgetting to add "virtual" (luckily we recently got "override"), but it seems swift's protocols add another dimension to make it possible to mess up in. It's weird how people preach over and over how arcane and cumbersome old languages like C(++) are, with the neckbeards defending it with "well you just have to adhere to this list of best practices and it's absolutely fine", and then when some new and hip language comes along and offers just as many traps, the same people giving the "old languages" crap suddenly go all "well it's bad design to do that! Just adhere to this list of best practices here..."
  - otaviokz 8y ago
    As someone who transitioned from Java to C#/.Net to Objective-C and finally to Swift, AMEN TO THAT! I learned not to say anything as apparently every software engineer "generation" needs to follow something "hip" when very young. Some will grow to see the cycle as it repeats itself in front of them, others will die believing "my programming language was the best, you just had to stick to this list of best practices". Also, "bad design" quite often means "not the way I'm used to and feel comfortable with".
    - wool_gather 8y ago
      There's an element of this even within the community of a single language, too. E.g., Apple's been doing Swift talks at WWDC where they sort of introduce their blow-your-mind-paradigm-of-the-year: "Protocol Oriented Programming" was two years ago; "Embrace Algorithms (subtitle: delete your for loops)" was this year. It concerns me because these are such lush, ripe targets for cargoculting. The behavior in this article is problematic exactly because of the "Apple says we must not use inheritance for anything! I can just use a protocol extension; that's totally not inheritance" mindset.
joeblau 8y ago
If I saw this in a code review, I would flag it for re-design. Mixing composition and inheritance especially in this way is definitely going to confuse anyone who doesn't read every line to figure out what's going on.
- derefr 8y ago
  I don't think there's anything wrong with mixing composition and inheritance per se, if everything is obeying strict OOP principles (Open-Closed, etc.) If the mix-in was adding functionality to the base class, and then the subclass was there to specialize the behavior of the base class without breaking any of the contracts the base class itself makes (i.e. any unit test that works on a Greeter should work on a LazyGreeter) then I don't think anyone would be confused. But in this case, OOP principles aren't being followed—the subclass is attempting to override stuff from the protocol, which the superclass didn't override. Essentially, the superclass made an assertion, by not overriding that method, that it wanted the default behavior from the protocol extension. The subclass, by overriding that behavior, is breaking the contract that the superclass declares.
  - joeblau 8y ago
    Yeah, A static analyzer should be able to figure out what's going on and throw an error.
jolux 8y ago
This seems like a problem with using class inheritance more than anything else.
- timjver 8y ago
  Or a problem of mixing class inheritance with protocol conformance, perhaps.
jordansmithnz 8y ago
In general I’ve found protocol extensions to be an incredibly powerful concept. There’s a couple of WWDC talks about ‘protocol oriented programming’ that highlight some great, practical use cases. I’ve run into the author’s problem a few times, and I’d like to think that this is just a bug or behaviour detail that a future Swift version could correct. Swift hasn’t quite matured fully yet, and this is a great example of that (these examples are becoming fewer every year though).
kitsunesoba 8y ago
Interesting. Swift has been my primary language for a couple of years now, but I’ve never run into this, mainly because I use subclasses far, far less often than I did under Objective-C. If this bug had existed in Obj-C back when It was still my primary language it likely would have bitten me.
mpweiher 8y ago
Wow, Heisen-Swift, where the Heisenbugs are in the language specification! What an object is, which is roughly equivalent to its observed behavior, should never depend on how it is declared. let greeter = LazyGreeter() let greeter1: Greeter = greeter print(greeter) print(greeter1) greeter.greet() greeter1.greet() UPDATE: just in case it's not clear, this prints the following: greeter.LazyGreeter greeter.LazyGreeter sup Hello, World! So the same object responds differently to the same message, depending on how it is declared. Yikes!
- wool_gather 8y ago
  It seems this is what you get when you decide dynamic dispatch is a demon that must be cast out. :/ It's a good example of Swift's at-times-infuriating insistence on chasing performance at the expense of developer ergonomics. That said, I'm sure it will be mediated eventually (although probably with more compiler annotations).
  - mpweiher 8y ago
    > infuriating insistence on chasing performance at the expense of developer ergonomics It would be slightly less infuriating had that chase actually delivered, but it dramatically has not.
  - arcticbull 8y ago
    The problem here is that Swift is sort of this hybrid between two disparate worldviews, the old Objective-C world where inheritance is cool and dispatch is dynamic and the new world where inheritance is not even permitted and traits are used to provide dynamic dispatch. It's trying to be everything to everyone. This is one of those crossover points where the seams don't meet. If this was all dynamic, it would be fine because the messages would be dispatched properly. If this was all new world, you wouldn't even be allowed to subclass, would refer to your trait explicitly, and again, everything would work fine. This is a fundamental structural issue with the language, philosophically, and will definitely cost programmer productivity. I'd argue this isn't a problem so much as behaves correctly due to insufficient planning. It's not that they're taking a position on dynamic dispatch, it's that they're trying to take both positions, likely in support of Objective-C compatibility. IMO dynamic dispatch has caused way more programmer productivity issues than anything else - you write code the compiler can't validate because you explicitly chose not to give it the information it needs to do it's job. It can't tell you your code is right. Nothing really can. The new world is a better place, Go and Rust have it right, inheritance is dead.
lazulicurio 8y ago
Coming from the .NET world, where extension methods are just syntactic sugar for static methods, the "bug" described in the article wasn't surprising to me. However, it did make me stop and think about how the example would look in C#. This was what I came up with: public interface IGreeter { } public static class GreeterExtensions { public void Greet(this IGreeter greeter) { // If you have CA turned on, // you'll get a warning for not // using the parameter "greeter" Console.WriteLine("Hello, World!"); } } public class BaseGreeter : IGreeter { public abstract void Greet(); } public class LazyGreeter : BaseGreeter { public override void Greet() { Console.WriteLine("sup"); } } IGreeter greeter = new LazyGreeter(); // This has to be GreeterExtensions.Greet(greeter), // because IGreeter is an empty interface greeter.Greet(); I think the syntax in C# makes it clearer that you're doing something funky mixing inheritance with extension methods. If you wanted to do it the "correct" way, I think it would look something like: public interface IGreeter { void Greet(); } public class BaseGreeter : IGreeter { // Edit: abstract method wouldn't be equivalent to example // public abstract void Greet(); public virtual void Greet() { Console.WriteLine("Hello, World!"); } } public class LazyGreeter : BaseGreeter { public override void Greet() { Console.WriteLine("sup"); } } IGreeter greeter = new LazyGreeter(); greeter.Greet(); Which is more complicated than the idiomatic way of providing default behavior with inheritance: public class Greeter { public virtual void Greet() { Console.WriteLine("Hello, World!"); } } public class LazyGreeter : Greeter { public override void Greet() { Console.WriteLine("sup"); } } Greeter greeter = new LazyGreeter(); greeter.Greet(); I'm not a huge fan of the Swift syntax; specifically, IMO, the declaration `class BaseGreeter: Greeter {}` hides where the implementation of `greet` is coming from. Without the method declaration it looks like it's being inherited from the protocol extension, when it's really not.
- sbjs 8y ago
  C# is one of those languages that I absolutely love and wish I had any reason or excuse to use. But it only seems used in .NET apps which are inherently harder to write than web apps which is why I learned Electron instead.
dtech 8y ago
Can anyone explain to me what the rationale for this is? I can understand the advantage of using static dispatch, i.e. Greeter#greet(object), but I assume there is some mechanism that avoids calling Greeter#greet if BaseGreeter would implement greet and call BaseGreeter#greet(object) instead. Why does extending a class which extends a protocol not make the extending class implement that protocol in Swift?
sonnyblarney 8y ago
The explanation is counterintuitive and it's one of those many Swift-ish things I've learned to be wary of. I find that Swift tries to be 'all things' and to often I get lost trying to cobble together a mental picture of what a class is supposed to be given all the various ways to annotate it.
wux 8y ago
This is an interesting demonstration of a hard edge of the language. It's not because Swift holds some sort of grudge against dynamic dispatch, but rather because it's trying to reconcile some subtly incompatible design goals: 1. Adding a default implementation for a protocol requirement shouldn't stop existing types that correctly conform to that protocol from compiling. This is a reasonable user expectation and makes protocol composition a more powerful feature. 2. "Retroactive conformance" should be possible: someone should be able to conform a type they didn't write to a protocol of their own creation. It's a nifty feature that makes Swift extensions very powerful but also sometimes difficult to reason about. 3. Classes should inherit protocol conformances from their superclasses. As some of the Swift core team members have explained, this is not something that absolutely had to be the case, but it seemed sensible initially. Points (1) and (2) preclude the language from requiring the `override` keyword for the subclass `LazyGreeter` to override a default implementation of a protocol requirement not itself overridden in the superclass `BaseGreeter`, since that would mean that (1) and/or (2) would become impossible. Getting rid of (3) would remove the surprising behavior but it is rather late in the evolution of the language to make such a large change. The way to reason about this behavior is to consider what are intended "customization points" in the language. It isn't really covered in The Swift Programming Language, but (although not intuitive) it's a fairly teachable concept: A separate but similar issue exists with the distinction between default implementations of protocol requirements (methods declared in both a protocol and a protocol extension) versus extension methods (methods not declared in a protocol but found in a protocol extension). The former is dynamically dispatched and is considered a customization point for conforming types, whereas the latter can only be shadowed by conforming types but is not a customization point. As described here: https://nomothetis.svbtle.com/the-ghost-of-swift-bugs-future https://nomothetis.svbtle.com/the-ghost-of-swift-bugs-future (The Swift standard library actually uses this concept to its advantage. For instance, the inequality operator != is not a customization point for the Equatable protocol and is implemented in an extension method, guaranteeing that !(a == b) is always equivalent to (a != b) in the generic context.) When implementing subclasses of a third-party open class, only open methods are customization points. In that case, Swift provides compile-time checking to prohibit overriding non-open public methods. This is possible because Swift does not support "retroactive inheritance" from a superclass as it does retroactive conformance to a protocol. Also, the language allows later versions of a third-party open class to stop your existing subclasses from compiling because of a conflict between a later-added superclass method and a pre-existing method in your subclass that now lacks the `override` keyword. In other words, the compiler-enforced prohibition is possible because points (1) and (2) above were not design goals for subclassing as they were for protocol conformance. In the case illustrated by the present article, where protocols are used in class hierarchies, a third notion of customization points arises. As demonstrated here, a protocol requirement not overridden by a base class is not a customization point for a subclass. The protocol requirement can be shadowed by the subclass but not overridden. Consistent with the design goals enumerated above, this allows for the vendor of a library protocol to provide additional default implementations at a later point without breaking user code. (But if the vendor of a conforming superclass then implements an overriding implementation, your subclass would cease to compile just as it would for any other conflict between superclass methods and subclass methods.)
ljm 8y ago
I'm reading the comments here about how the languages fails, or what it should have done better, but at the same time I'm feeling the same as I do when building up something with Kubernetes and the problem isn't really the language, it's the shortcuts we take with it to stay concise. e.g. in Kubernetes you can give a Pod, Deployment, Service, Ingress, Volume, etc. etc. the exact same name and in fact many examples encourage that (because they are namespaced on the type). However this is practically just an artefact of a previous choice and when you're looking at your YAML files you may not immediately know that distinction unless you have some familiarity with the tech. What it says is that you can call everything the exact same thing. And it will work, and it will seem nice or elegant. So it is with this Swift example and having a protocol, an extension, and a base class all declaring the exact same thing. Even if the compiler figures it out, how would you without reading the spec or knowing where those files live (because in the real world they would be spread across the filesystem)? It's just poor consideration for comprehension.
- wool_gather 8y ago
  You make a fair point, but isn't it part of a platform's responsibility to not make footguns quite so easy to grab? Especially given Swift's general (and mostly helpful) attitude of insistence on safe, explicit constructs. For example, it's a warning to ignore a function call result; it's an error to not chain up to a superclass's initializer; it's an error to re-implement a class member without the `override` keyword. Doesn't this situation deserve similar compiler scrutiny?
  - xyzzy_plugh 8y ago
    The problem with this angle is that while sure, you can always make a compiler more intelligent in ambiguous situations, you must also now train all the humans interfacing with it to be equally as intelligent (or at least, able to sufficiently disambiguate). We don't see many context-sensitive grammars for programming languages, not because very difficult to implement, but I suspect because they're hard for humans to understand. There is something to be said for the simplicity of naming things to describe what they are (e.g. Hungarian notation), but I really think it's a mistake for languages (configuration, programming, etc.) to even allow identifiers to be reused for differing types.
didibus 8y ago
I don't know that you can really call this a bug? It just seems to be the design choice. Though I agree, it seems odd at first, and appears possibly accidental, it could have been on purpose. My question to swifters: what if LazyGreeter had another method, say lazyGreet, and you typed it as Greeter? Then you called let greeter : Greeter = LazyGreeter(); greeter.lazyGreet(); Would you expect this to work or fail? I'd expect it to fail. In that sense, I assume its simply that extension methods override child overrides. And its just something you need to know. That can even have interesting utility in some scenarios.
- syrrim 8y ago
  In python, `greeter.lazyGreet()` would succeed, because python only has types at runtime. If you used mypy, you would get a warning for this. No warning would be issued for `greeter.greet()`, but since function dispatch is still done at runtime, this would call `LazyGreeter.greet()`. I concur that the behaviour is a design decision, not a bug, but I think most programmers would in general prefer design decisions to bring the language closer to python.
- nangtrongvuon 8y ago
  I’m just a wee lad still dabbling in Swift, but I would say this would work, since the lazyGreet method is separate from the Greeter protocol.
Legogris 8y ago
Another "unused code bug" would be when a locally scoped variable which is not supposed to be there shadows the intended global variable. It's just a matter of global scoping and precedence. But yeah, sharing notation between inheritance and extension is a tad bit reductionist.
jankins 8y ago
This is my favorite Swift gotcha :D This bug bit me one time back before remote debugging came out when I was developing an app for a device that plugged into the lightning port. The only way to debug was to send log messages over the network to a log server running on my laptop, and the connected device sent messages to satellites where there was often a 10-minute gap in connectivity. And I couldn’t reproduce it in tests. That took some time to diagnose! I like to bring this issue up if an interview candidate seems extremely bought in to protocol-driven development. I wouldn’t be surprised if any level of dev wasn’t aware of it, but would be impressed if anybody identified it.
delinka 8y ago
Providing a protocol with a default concrete implementation of the interface? That's not passing code review here. Would I have caught the bug? Probably not. But it looks like bad form to me to provide that default implementation to Greeter, so please rewrite your code.
- sharp11 8y ago
  I’m curious to know why... Looks like typical mixin pattern, no?
  - HNLogInShit 8y ago
    mix-in
  - delinka 8y ago
    An interface declaration is not the right place for implementation. If you want a default implementation, use a base class and inherit, overriding if you don't want the default. This bug is now another good reason not to use default implementations on interfaces.
    - sharp11 8y ago
      Ok, thanks. Sounds like you are not a fan of mixins! :)
    - jolux 8y ago
      Protocol extensions are extremely powerful and safe if used correctly and not mixed with class inheritance as in this example.
Buge 8y ago
It's good that Periphery catches the unused code. But if that's the only reason it catches the problem, I think it could have room for improvement. For example in one place the error could exist, but at a different place, the real function could be called because the variable is declared as a LazyGreeter. Then Periphery wouldn't catch the problem. One way to fully catch it would be to give a warning if a base class implements a protocol but a derived class doesn't. I'm not sure how many false positives this would have. Possibly you might only do this if the derived class tries to override a function from the base class's protocol.
a_t48 8y ago
How much different is this from slicing in C++? IE: #include <iostream> using namespace std; struct Greeter { void greet() { cout << "Hello, World!" << endl; } }; struct BaseGreeter: Greeter {}; struct LazyGreeter: BaseGreeter { void greet() { cout << "sup" << endl; } }; int main() { Greeter greeter = LazyGreeter(); greeter.greet(); // Prints "Hello, World" return 0; }
- GRiMe2D 8y ago
  In C++ world this is expected behavior AND it's documented. But in swift, protocols cannot declare default implementations but it's possible to define implementation to protocol using extensions. So in C++, it might look like this struct Greeter { virtual void greet() { cout << "Hello, world!" << endl; } }; struct BaseGreeter: Greeter {}; struct LazyGreeter: BaseGreeter { void greet() override { cout << "sup" << endl; } }; int main() { Greeter *greeter = new LazyGreeter(); greeter->greet(); // In C++ prints "sup", but in Swift "Hello, World!" delete greeter; return 0; }
mmfl 8y ago
If the concept is that protocol requirements represent the "customisation points" of a protocol, shouldn't this apply equally to conforming classes and subclasses with inherited conformance? So I would be in favour of fixing this. Conversely, perhaps protocol extension methods etc not declared as protocol requirements could e.g. elicit a compiler warning when "overridden" by conforming classes/subclasses with inherited conformance (since they are not intended as customisation points)? I think these changes would aid reasoning about protocols and reduce bugs.

137 points by ileitch 8y ago

wiradikusuma 8y ago

I wonder if this behavior is working as intended. It does look like a bug to the language itself, isn't?

mgoblu3 8y ago
There are some rules around this, as noted in https://nomothetis.svbtle.com/the-ghost-of-swift-bugs-future https://nomothetis.svbtle.com/the-ghost-of-swift-bugs-future: IF the inferred type of a variable is the protocol: AND the method is defined in the original protocol THEN the runtime type’s implementation is called, irrespective of whether there is a default implementation in the extension. AND the method is not defined in the original protocol, THEN the default implementation is called. ELSE IF the inferred type of the variable is the type THEN the type’s implementation is called.
anon180719 8y ago
It appears the correct action would have been to use <code>override func greet() { print("sup") }</code>. To be fair, I was primed to really think about the code execution given he'd indicated there was a trap in there somewhere, so it certainly would not have been immediately obvious on first glance. But I think, generally developers understand that <code>override</code> is necessary to override code from a parent class. Typically XCode helps out with this sort of thing – adding <code>override</code> where it seems like it should be added – but perhaps not when a parent class is using a default protocol implementation. But again, in this scenario where a class is inheriting from another class, it's typically known you need to override the function in order to use your own implementation, so this does seem a bit contrived.
- joeblau 8y ago
  > It appears the correct action would have been to use <code>override func greet() { print("sup") }</code>. That actually doesn't work because the greet function is implemented on an extension. You can only override class functions.
  - anon180719 8y ago
    Oh, that's definitely weird then. Is there an RFC to have that fixed in some way?
    - joeblau 8y ago
      I don't think there is. I think the code is functioning as expected — it's just written in a super confusing way. What should probably happen is that the compiler should throw an error about redefining a function in a super classes extension. I'm not a compiler engineer but a check for that seems like it would be intensive unless that information is codified in the AST somehow.
      - anon180719 8y ago
        hmph. I agree, the compiler should probably not let you compile in that case. Funny enough this reminds me of "responder chain politics" in dynamically typed languages with duck typing.
      - slavapestov 8y ago
        It's a bug - https://bugs.swift.org/browse/SR-103 https://bugs.swift.org/browse/SR-103

mgoblu3 8y ago

Default implementations on protocols in Swift definitely can be a little dangerous. We've shied our team away from doing this unless the protocol is explicitly used as a mix-in type. There's been some proposals around fixing these, one that comes to mind is: https://forums.swift.org/t/introducing-role-keywords-to-reduce-hard-to-find-bugs/6113 https://forums.swift.org/t/introducing-role-keywords-to-redu...

alrz 8y ago
This doesn't seem to be about default implementations on protocols, I guess they do actually do virtual dispatch so you wouldn't hit this bug with default impls. The main issue is that we have an extension method on a type with the same name of a member of that type. So it can always be statically flagged whether or not the method is used anywhere else. This is easily reproducible in any language that supports extension methods (C#, for instance). I wonder if that could be useful at all.

Chazprime 8y ago

I've run afoul of this a couple of times in my Swift delegate protocols. A really nice hack to implement Objective-C's optional protocol methods in Swift is to use a protocol extension to create a default method that can come back to bite you when you subclass objects that conform to that protocol.

otaviokz 8y ago
Yep, the old `fatalError("You need to implement this!")`.

untog 8y ago

I'm a little undecided on Swift extensions as a whole, and particularly protocol extensions. Maybe I'm just paranoid but it feels like it leads to a more complicated code layout.

iforgotpassword 8y ago
I agree. I never worked with swift and had to look up the terminology. Most of what the article refers to maps to terms from C++ (which is already horribly complicated and offers way too many ways to shoot yourself in the foot) or Java. That PWT sounds an awful lot like a vtable. In C++ you mess up by forgetting to add "virtual" (luckily we recently got "override"), but it seems swift's protocols add another dimension to make it possible to mess up in. It's weird how people preach over and over how arcane and cumbersome old languages like C(++) are, with the neckbeards defending it with "well you just have to adhere to this list of best practices and it's absolutely fine", and then when some new and hip language comes along and offers just as many traps, the same people giving the "old languages" crap suddenly go all "well it's bad design to do that! Just adhere to this list of best practices here..."
- otaviokz 8y ago
  As someone who transitioned from Java to C#/.Net to Objective-C and finally to Swift, AMEN TO THAT! I learned not to say anything as apparently every software engineer "generation" needs to follow something "hip" when very young. Some will grow to see the cycle as it repeats itself in front of them, others will die believing "my programming language was the best, you just had to stick to this list of best practices". Also, "bad design" quite often means "not the way I'm used to and feel comfortable with".
  - wool_gather 8y ago
    There's an element of this even within the community of a single language, too. E.g., Apple's been doing Swift talks at WWDC where they sort of introduce their blow-your-mind-paradigm-of-the-year: "Protocol Oriented Programming" was two years ago; "Embrace Algorithms (subtitle: delete your for loops)" was this year. It concerns me because these are such lush, ripe targets for cargoculting. The behavior in this article is problematic exactly because of the "Apple says we must not use inheritance for anything! I can just use a protocol extension; that's totally not inheritance" mindset.

joeblau 8y ago

If I saw this in a code review, I would flag it for re-design. Mixing composition and inheritance especially in this way is definitely going to confuse anyone who doesn't read every line to figure out what's going on.

derefr 8y ago
I don't think there's anything wrong with mixing composition and inheritance per se, if everything is obeying strict OOP principles (Open-Closed, etc.) If the mix-in was adding functionality to the base class, and then the subclass was there to specialize the behavior of the base class without breaking any of the contracts the base class itself makes (i.e. any unit test that works on a Greeter should work on a LazyGreeter) then I don't think anyone would be confused. But in this case, OOP principles aren't being followed—the subclass is attempting to override stuff from the protocol, which the superclass didn't override. Essentially, the superclass made an assertion, by not overriding that method, that it wanted the default behavior from the protocol extension. The subclass, by overriding that behavior, is breaking the contract that the superclass declares.
- joeblau 8y ago
  Yeah, A static analyzer should be able to figure out what's going on and throw an error.

jolux 8y ago

This seems like a problem with using class inheritance more than anything else.

timjver 8y ago
Or a problem of mixing class inheritance with protocol conformance, perhaps.

jordansmithnz 8y ago

In general I’ve found protocol extensions to be an incredibly powerful concept. There’s a couple of WWDC talks about ‘protocol oriented programming’ that highlight some great, practical use cases. I’ve run into the author’s problem a few times, and I’d like to think that this is just a bug or behaviour detail that a future Swift version could correct. Swift hasn’t quite matured fully yet, and this is a great example of that (these examples are becoming fewer every year though).

kitsunesoba 8y ago

Interesting. Swift has been my primary language for a couple of years now, but I’ve never run into this, mainly because I use subclasses far, far less often than I did under Objective-C. If this bug had existed in Obj-C back when It was still my primary language it likely would have bitten me.

mpweiher 8y ago

Wow, Heisen-Swift, where the Heisenbugs are in the language specification! What an object is, which is roughly equivalent to its observed behavior, should never depend on how it is declared. let greeter = LazyGreeter() let greeter1: Greeter = greeter print(greeter) print(greeter1) greeter.greet() greeter1.greet() UPDATE: just in case it's not clear, this prints the following: greeter.LazyGreeter greeter.LazyGreeter sup Hello, World! So the same object responds differently to the same message, depending on how it is declared. Yikes!

wool_gather 8y ago
It seems this is what you get when you decide dynamic dispatch is a demon that must be cast out. :/ It's a good example of Swift's at-times-infuriating insistence on chasing performance at the expense of developer ergonomics. That said, I'm sure it will be mediated eventually (although probably with more compiler annotations).
- mpweiher 8y ago
  > infuriating insistence on chasing performance at the expense of developer ergonomics It would be slightly less infuriating had that chase actually delivered, but it dramatically has not.
- arcticbull 8y ago
  The problem here is that Swift is sort of this hybrid between two disparate worldviews, the old Objective-C world where inheritance is cool and dispatch is dynamic and the new world where inheritance is not even permitted and traits are used to provide dynamic dispatch. It's trying to be everything to everyone. This is one of those crossover points where the seams don't meet. If this was all dynamic, it would be fine because the messages would be dispatched properly. If this was all new world, you wouldn't even be allowed to subclass, would refer to your trait explicitly, and again, everything would work fine. This is a fundamental structural issue with the language, philosophically, and will definitely cost programmer productivity. I'd argue this isn't a problem so much as behaves correctly due to insufficient planning. It's not that they're taking a position on dynamic dispatch, it's that they're trying to take both positions, likely in support of Objective-C compatibility. IMO dynamic dispatch has caused way more programmer productivity issues than anything else - you write code the compiler can't validate because you explicitly chose not to give it the information it needs to do it's job. It can't tell you your code is right. Nothing really can. The new world is a better place, Go and Rust have it right, inheritance is dead.

lazulicurio 8y ago

Coming from the .NET world, where extension methods are just syntactic sugar for static methods, the "bug" described in the article wasn't surprising to me. However, it did make me stop and think about how the example would look in C#. This was what I came up with: public interface IGreeter { } public static class GreeterExtensions { public void Greet(this IGreeter greeter) { // If you have CA turned on, // you'll get a warning for not // using the parameter "greeter" Console.WriteLine("Hello, World!"); } } public class BaseGreeter : IGreeter { public abstract void Greet(); } public class LazyGreeter : BaseGreeter { public override void Greet() { Console.WriteLine("sup"); } } IGreeter greeter = new LazyGreeter(); // This has to be GreeterExtensions.Greet(greeter), // because IGreeter is an empty interface greeter.Greet(); I think the syntax in C# makes it clearer that you're doing something funky mixing inheritance with extension methods. If you wanted to do it the "correct" way, I think it would look something like: public interface IGreeter { void Greet(); } public class BaseGreeter : IGreeter { // Edit: abstract method wouldn't be equivalent to example // public abstract void Greet(); public virtual void Greet() { Console.WriteLine("Hello, World!"); } } public class LazyGreeter : BaseGreeter { public override void Greet() { Console.WriteLine("sup"); } } IGreeter greeter = new LazyGreeter(); greeter.Greet(); Which is more complicated than the idiomatic way of providing default behavior with inheritance: public class Greeter { public virtual void Greet() { Console.WriteLine("Hello, World!"); } } public class LazyGreeter : Greeter { public override void Greet() { Console.WriteLine("sup"); } } Greeter greeter = new LazyGreeter(); greeter.Greet(); I'm not a huge fan of the Swift syntax; specifically, IMO, the declaration `class BaseGreeter: Greeter {}` hides where the implementation of `greet` is coming from. Without the method declaration it looks like it's being inherited from the protocol extension, when it's really not.

sbjs 8y ago
C# is one of those languages that I absolutely love and wish I had any reason or excuse to use. But it only seems used in .NET apps which are inherently harder to write than web apps which is why I learned Electron instead.

dtech 8y ago

Can anyone explain to me what the rationale for this is? I can understand the advantage of using static dispatch, i.e. Greeter#greet(object), but I assume there is some mechanism that avoids calling Greeter#greet if BaseGreeter would implement greet and call BaseGreeter#greet(object) instead. Why does extending a class which extends a protocol not make the extending class implement that protocol in Swift?

sonnyblarney 8y ago

The explanation is counterintuitive and it's one of those many Swift-ish things I've learned to be wary of. I find that Swift tries to be 'all things' and to often I get lost trying to cobble together a mental picture of what a class is supposed to be given all the various ways to annotate it.

wux 8y ago

This is an interesting demonstration of a hard edge of the language. It's not because Swift holds some sort of grudge against dynamic dispatch, but rather because it's trying to reconcile some subtly incompatible design goals: 1. Adding a default implementation for a protocol requirement shouldn't stop existing types that correctly conform to that protocol from compiling. This is a reasonable user expectation and makes protocol composition a more powerful feature. 2. "Retroactive conformance" should be possible: someone should be able to conform a type they didn't write to a protocol of their own creation. It's a nifty feature that makes Swift extensions very powerful but also sometimes difficult to reason about. 3. Classes should inherit protocol conformances from their superclasses. As some of the Swift core team members have explained, this is not something that absolutely had to be the case, but it seemed sensible initially. Points (1) and (2) preclude the language from requiring the `override` keyword for the subclass `LazyGreeter` to override a default implementation of a protocol requirement not itself overridden in the superclass `BaseGreeter`, since that would mean that (1) and/or (2) would become impossible. Getting rid of (3) would remove the surprising behavior but it is rather late in the evolution of the language to make such a large change. The way to reason about this behavior is to consider what are intended "customization points" in the language. It isn't really covered in The Swift Programming Language, but (although not intuitive) it's a fairly teachable concept: A separate but similar issue exists with the distinction between default implementations of protocol requirements (methods declared in both a protocol and a protocol extension) versus extension methods (methods not declared in a protocol but found in a protocol extension). The former is dynamically dispatched and is considered a customization point for conforming types, whereas the latter can only be shadowed by conforming types but is not a customization point. As described here: https://nomothetis.svbtle.com/the-ghost-of-swift-bugs-future https://nomothetis.svbtle.com/the-ghost-of-swift-bugs-future (The Swift standard library actually uses this concept to its advantage. For instance, the inequality operator != is not a customization point for the Equatable protocol and is implemented in an extension method, guaranteeing that !(a == b) is always equivalent to (a != b) in the generic context.) When implementing subclasses of a third-party open class, only open methods are customization points. In that case, Swift provides compile-time checking to prohibit overriding non-open public methods. This is possible because Swift does not support "retroactive inheritance" from a superclass as it does retroactive conformance to a protocol. Also, the language allows later versions of a third-party open class to stop your existing subclasses from compiling because of a conflict between a later-added superclass method and a pre-existing method in your subclass that now lacks the `override` keyword. In other words, the compiler-enforced prohibition is possible because points (1) and (2) above were not design goals for subclassing as they were for protocol conformance. In the case illustrated by the present article, where protocols are used in class hierarchies, a third notion of customization points arises. As demonstrated here, a protocol requirement not overridden by a base class is not a customization point for a subclass. The protocol requirement can be shadowed by the subclass but not overridden. Consistent with the design goals enumerated above, this allows for the vendor of a library protocol to provide additional default implementations at a later point without breaking user code. (But if the vendor of a conforming superclass then implements an overriding implementation, your subclass would cease to compile just as it would for any other conflict between superclass methods and subclass methods.)

ljm 8y ago

I'm reading the comments here about how the languages fails, or what it should have done better, but at the same time I'm feeling the same as I do when building up something with Kubernetes and the problem isn't really the language, it's the shortcuts we take with it to stay concise. e.g. in Kubernetes you can give a Pod, Deployment, Service, Ingress, Volume, etc. etc. the exact same name and in fact many examples encourage that (because they are namespaced on the type). However this is practically just an artefact of a previous choice and when you're looking at your YAML files you may not immediately know that distinction unless you have some familiarity with the tech. What it says is that you can call everything the exact same thing. And it will work, and it will seem nice or elegant. So it is with this Swift example and having a protocol, an extension, and a base class all declaring the exact same thing. Even if the compiler figures it out, how would you without reading the spec or knowing where those files live (because in the real world they would be spread across the filesystem)? It's just poor consideration for comprehension.

wool_gather 8y ago
You make a fair point, but isn't it part of a platform's responsibility to not make footguns quite so easy to grab? Especially given Swift's general (and mostly helpful) attitude of insistence on safe, explicit constructs. For example, it's a warning to ignore a function call result; it's an error to not chain up to a superclass's initializer; it's an error to re-implement a class member without the `override` keyword. Doesn't this situation deserve similar compiler scrutiny?
- xyzzy_plugh 8y ago
  The problem with this angle is that while sure, you can always make a compiler more intelligent in ambiguous situations, you must also now train all the humans interfacing with it to be equally as intelligent (or at least, able to sufficiently disambiguate). We don't see many context-sensitive grammars for programming languages, not because very difficult to implement, but I suspect because they're hard for humans to understand. There is something to be said for the simplicity of naming things to describe what they are (e.g. Hungarian notation), but I really think it's a mistake for languages (configuration, programming, etc.) to even allow identifiers to be reused for differing types.

didibus 8y ago

I don't know that you can really call this a bug? It just seems to be the design choice. Though I agree, it seems odd at first, and appears possibly accidental, it could have been on purpose. My question to swifters: what if LazyGreeter had another method, say lazyGreet, and you typed it as Greeter? Then you called let greeter : Greeter = LazyGreeter(); greeter.lazyGreet(); Would you expect this to work or fail? I'd expect it to fail. In that sense, I assume its simply that extension methods override child overrides. And its just something you need to know. That can even have interesting utility in some scenarios.

syrrim 8y ago
In python, `greeter.lazyGreet()` would succeed, because python only has types at runtime. If you used mypy, you would get a warning for this. No warning would be issued for `greeter.greet()`, but since function dispatch is still done at runtime, this would call `LazyGreeter.greet()`. I concur that the behaviour is a design decision, not a bug, but I think most programmers would in general prefer design decisions to bring the language closer to python.
nangtrongvuon 8y ago
I’m just a wee lad still dabbling in Swift, but I would say this would work, since the lazyGreet method is separate from the Greeter protocol.

Legogris 8y ago

Another "unused code bug" would be when a locally scoped variable which is not supposed to be there shadows the intended global variable. It's just a matter of global scoping and precedence. But yeah, sharing notation between inheritance and extension is a tad bit reductionist.

jankins 8y ago

This is my favorite Swift gotcha :D This bug bit me one time back before remote debugging came out when I was developing an app for a device that plugged into the lightning port. The only way to debug was to send log messages over the network to a log server running on my laptop, and the connected device sent messages to satellites where there was often a 10-minute gap in connectivity. And I couldn’t reproduce it in tests. That took some time to diagnose! I like to bring this issue up if an interview candidate seems extremely bought in to protocol-driven development. I wouldn’t be surprised if any level of dev wasn’t aware of it, but would be impressed if anybody identified it.

delinka 8y ago

Providing a protocol with a default concrete implementation of the interface? That's not passing code review here. Would I have caught the bug? Probably not. But it looks like bad form to me to provide that default implementation to Greeter, so please rewrite your code.

sharp11 8y ago
I’m curious to know why... Looks like typical mixin pattern, no?
- HNLogInShit 8y ago
  mix-in
- delinka 8y ago
  An interface declaration is not the right place for implementation. If you want a default implementation, use a base class and inherit, overriding if you don't want the default. This bug is now another good reason not to use default implementations on interfaces.
  - sharp11 8y ago
    Ok, thanks. Sounds like you are not a fan of mixins! :)
  - jolux 8y ago
    Protocol extensions are extremely powerful and safe if used correctly and not mixed with class inheritance as in this example.

Buge 8y ago

It's good that Periphery catches the unused code. But if that's the only reason it catches the problem, I think it could have room for improvement. For example in one place the error could exist, but at a different place, the real function could be called because the variable is declared as a LazyGreeter. Then Periphery wouldn't catch the problem. One way to fully catch it would be to give a warning if a base class implements a protocol but a derived class doesn't. I'm not sure how many false positives this would have. Possibly you might only do this if the derived class tries to override a function from the base class's protocol.

a_t48 8y ago

How much different is this from slicing in C++? IE: #include <iostream> using namespace std; struct Greeter { void greet() { cout << "Hello, World!" << endl; } }; struct BaseGreeter: Greeter {}; struct LazyGreeter: BaseGreeter { void greet() { cout << "sup" << endl; } }; int main() { Greeter greeter = LazyGreeter(); greeter.greet(); // Prints "Hello, World" return 0; }

GRiMe2D 8y ago
In C++ world this is expected behavior AND it's documented. But in swift, protocols cannot declare default implementations but it's possible to define implementation to protocol using extensions. So in C++, it might look like this struct Greeter { virtual void greet() { cout << "Hello, world!" << endl; } }; struct BaseGreeter: Greeter {}; struct LazyGreeter: BaseGreeter { void greet() override { cout << "sup" << endl; } }; int main() { Greeter *greeter = new LazyGreeter(); greeter->greet(); // In C++ prints "sup", but in Swift "Hello, World!" delete greeter; return 0; }

mmfl 8y ago

If the concept is that protocol requirements represent the "customisation points" of a protocol, shouldn't this apply equally to conforming classes and subclasses with inherited conformance? So I would be in favour of fixing this. Conversely, perhaps protocol extension methods etc not declared as protocol requirements could e.g. elicit a compiler warning when "overridden" by conforming classes/subclasses with inherited conformance (since they are not intended as customisation points)? I think these changes would aid reasoning about protocols and reduce bugs.