
Swift: When Unused Code Is a Bug - ileitch
https://www.peripheryapp.com/blog/2018/08/04/swift-when-unused-code-is-a-bug/
======
mpweiher
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!

~~~
torpidor
> What an object is, which is roughly equivalent to its observed behavior,
> should never depend on how it is declared.

This is actually a statement that strong typing should not exist. A
"declaration" (really a _type_ declaration) indicates the type, and if
observed behavior cannot depend on the types there is not much point to types.
Or stated positively: in a strongly typed language, the types are part of
'what an object is'.

Or, stated by example:

    
    
        let a: Int = 3
        let b: Float = 3
        print(a/2) //prints 1
        print(b/2) //prints 1.5
    

Both a and b are "the same" value, insofar as they are `==` and they model the
same underlying mathematical concept. But behavior varies because they differ
in type.

OP's snippet actually works quite a lot differently than the way you may
expect; LazyGreeter.greet() and Greeter.greet() are two completely independent
functions that share the same name. Name resolution is a complex topic in any
language, but in Swift if you wanted to override another function you would
say `override`, in which case the compiler will complain there is nothing you
can override in `BaseGreeter` at which point you will understand the whole
mistake.

There is actually no way to "pick" LazyGreeter's implementation of the
unrelated function (as perhaps you expect). The only function we can call on a
Greeter is Greeter.greet (or its overloads, and there are none). So if
Greeter.greet did not exist, we would not get LazyGreeter.greet() but rather a
type error.

I do think a case can be made that we need a similar `implements` keyword like
`override` to check that we are implementing a protocol requirement.

~~~
mpweiher
> This is actually a statement that strong typing should not exist.

Nope. It is a statement that objects shouldn't be different depending on how
you look at them.

The example is a class, so a reference type. That means that greeter and
greeter1 are just two references to the exact same underlying object.

Your example are two distinct value type instances that you happened to
initialize from the same literal.

So not even close to comparable situations. Speaking of comparable:

> Both a and b are "the same" value, insofar as they are `==`

Also nope.

    
    
       let a: Int = 3
       let b: Float = 3
    
       let r=a==b
       print(r)
    

Let's compile this:

    
    
       swiftc numbers.swift 
       numbers.swift:4:8: error: binary operator '==' cannot be applied to operands of type 'Int' and 'Float'
       let r=a==b
             ~^ ~
       numbers.swift:4:8: note: expected an argument list of type '(Int, Int)'
       let r=a==b
              ^

~~~
bsaul
You may not be familiar with swift but a protocol isn’t necessarily a class.
Structs ( so, value type) can also implements a protocol, and so in effect you
can not tell a lot about what you’re manipulating, beyond what’s declared at
the protocol level.

~~~
mpweiher
From TFA:

    
    
       class BaseGreeter: Greeter {}
    
       class LazyGreeter: BaseGreeter {

------
ljm
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
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
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.

~~~
wool_gather
> make a compiler more intelligent in ambiguous situations, [but] you must
> also now train all the humans interfacing with it to be equally as
> intelligent

I'm not sure what you mean here; probably I'm misunderstanding you somehow.

If you make the compiler more intelligent so that it can tell the humans about
subtle problems, that lets the humans be _less_ intelligent (or at least have
to think less). No?

------
mgoblu3
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-
redu...](https://forums.swift.org/t/introducing-role-keywords-to-reduce-hard-
to-find-bugs/6113)

~~~
alrz
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.

------
lazulicurio
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
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.

------
joeblau
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
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
Yeah, A static analyzer should be able to figure out what's going on and throw
an error.

------
jordansmithnz
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).

------
jankins
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.

------
dtech
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?

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

~~~
anon180719
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
> 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
Oh, that's definitely weird then. Is there an RFC to have that fixed in some
way?

~~~
joeblau
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.

~~~
slavapestov
It's a bug -
[https://bugs.swift.org/browse/SR-103](https://bugs.swift.org/browse/SR-103)

------
Buge
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.

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

~~~
timjver
Or a problem of mixing class inheritance with protocol conformance, perhaps.

------
Chazprime
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
Yep, the old `fatalError("You need to implement this!")`.

------
kitsunesoba
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.

------
delinka
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
I’m curious to know why... Looks like typical mixin pattern, no?

~~~
delinka
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.

~~~
jolux
Protocol extensions are extremely powerful and safe if used correctly and not
mixed with class inheritance as in this example.

------
mmfl
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.

------
didibus
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
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.

------
a_t48
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
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;
      }

------
sonnyblarney
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.

------
Legogris
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.

------
untog
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
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
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
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.

------
wux
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.)

~~~
wool_gather
> Adding a default implementation for a protocol requirement shouldn't stop
> existing types that correctly conform to that protocol from compiling.

I would disagree with that; adding that default implementation is not a simply
internal change. It flat-out changes compilibility; it makes invalid code into
valid code:

    
    
        protocol P {
            func f()
        }
    
        class C : P {}    // Error!
    

\---

    
    
        protocol P {
            func f()
        }
        extension P {
            func f() {}
        }
    
        class C : P {}    // Okay
    

Given the trap demonstrated here, the converse should hold as well. Doubly so
in light of the ways the compiler (mostly helpfully) enforces various aspects
of subclassing, as you pointed out.

In many ways, protocols (right now at least) feel like Swift having its cake
but not eating it: strictness floats around them in ways that do not help
developers (ugh, PATs!) but is absent where it would:

    
    
        // This all compiles without any warnings?!
        protocol P {
            var i: Int { get set }
        }
    
        protocol Q : P {
            var i: Int { get }
        }
    
        class C : Q, P {
            var i = 10
        }
    
        // Now make a protocol that inherits from another,
        // where both are class/AnyObject constrained
        
        //---
    
        struct S : Hashable {    // Swift 4.1, synthesized 
            let s: String
        }
        extension S : Equatable {
            static func == (lhs: S, rhs: S) -> Bool {
                // Hashable semantics, smashable semantics. Hold my beer.
                return lhs.s.first == rhs.s.first
            }
        }

