Introducing role keywords to reduce hard-to-find bugs

I’d support the breaking change here to allow the language to evolve properly instead of introducing halve baked opt-ins and workarounds for issues that totally deserve a fix. The first thought that I had when reading the proposal the first time was, that extend does not add any benefit to the language and should be implicit by default, where on the other hand default solves a real problem we’re having today. David was quicker and pitched the ideal solution by making default necessary in Swift 4 which is a breaking change but it solves the issue nicely.

That said, personally I would not support the opt-in version.

···

--
Adrian Zubarev
Sent with Airmail

Am 16. Juni 2017 um 21:06:58, Paul Cantrell via swift-evolution (swift-evolution@swift.org) schrieb:

On Jun 16, 2017, at 1:14 PM, Erica Sadun via swift-evolution <swift-evolution@swift.org> wrote:

On Jun 16, 2017, at 8:44 AM, David Hart <davidhart@fastmail.com> wrote:

Okay, I undertand. I’m just worried that the proposal is a net negative if the keywords stay optional. I’ll mention it in more detail once we get to the review period.

Thanks for the work on the proposal!!

I believe a breaking change has little chance of being accepted at this point in the language lifecycle. Adding opt-in compiler auditing to increase safety is, IMO, a net positive. It's a deliberate trade-off. We have included other designs to allow the core team to choose an alternative they feel is best for the philosophy and direction of Swift. This doesn't close the door to future language releases enhancing the concept, phasing out the second keyword, or introducing keywords for additional safety auditing.

I find it a dangerous philosophy to insist that any new proposal be ideologically pure. Imperfect proposals can still improve the language within the realities of the timelines, user base, and code base of the Swift community.

-- E

I share David’s concern. I also tend to think Erica’s correct about breaking changes. If the core team says “go ahead and break it,” then I’m all for it, but that seems unlikely.

FWIW, if we’re sticking with the two-keyword approach, the language could emit warnings for _any_ extension members that aren’t marked with either `extend` or `default`.

P

On 16 Jun 2017, at 16:33, Erica Sadun <erica@ericasadun.com> wrote:

As we say in our introduction, we're pitching the most conservative approach.

The proposal was designed for minimal language impact. It chooses a conservative approach that can be phased in first over time and language release over more succinct alternatives that would impact existing code bases.

We discuss the one keyword version (which most of us are a fan of) in the alternatives. The core team has to decide how much they're willing to allow existing code to warn and/or break, which is the consequence of the one keyword solution.

-- E

On Jun 16, 2017, at 7:44 AM, David Hart <davidhart@fastmail.com> wrote:

Erica, any thoughts on only having default and making it an error in a future version of Swift like was discussed on this thread? The idea seems to have a few supporters.

On 16 Jun 2017, at 15:33, Erica Sadun via swift-evolution <swift-evolution@swift.org> wrote:

On Jun 14, 2017, at 11:46 PM, Dave Abrahams via swift-evolution <swift-evolution@swift.org> wrote:

on Wed Jun 14 2017, Chris Lattner <swift-evolution@swift.org> wrote:

On Jun 14, 2017, at 10:11 AM, Erica Sadun via swift-evolution <swift-evolution@swift.org> wrote:

Some pals and I have been kicking an idea around about introducing
better ways to support the compiler in protocol extensions. We want

to eliminate some hard-to-detect bugs. We've been brainstorming on
how to do this without affecting backward compatibility and
introducing a minimal impact on keywords.

We'd love to know what you think of our idea, which is to introduce
"role" keywords. Roles allow the compiler to automatically check the
intended use of a extension member definition against its protocol
declarations, and emit errors, warnings, and fixits as needed. We
think it's a pretty straightforward approach that, if adopted,
eliminates an entire category of bugs.

The draft proposal is here:
https://gist.github.com/erica/14283fe18254489c1498a7069b7760c4
<https://gist.github.com/erica/14283fe18254489c1498a7069b7760c4>

Thanks in advance for your thoughtful feedback,

+1 on the idea of this.

ditto. IMO it also makes the protocol extension much more expressive
and easy to read.

--
-Dave

Pull request: https://github.com/apple/swift-evolution/pull/724

-- E

_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution

_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution

_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution

I think there's a lot of people who want this. I'd have to do a search through the archives but hasn't it been said that this isn't likely to happen soon?

-- E

···

On Jun 16, 2017, at 12:21 PM, Tino Heth <2th@gmx.de> wrote:

The described problem might be one of the most famous itches of the language, but imho the bar for new keywords* should be higher than that — and there are alternatives:

First, I guess many would like to see this to be valid Swift:

protocol Foo {
  func bar() {
    print("Default implementation called")
  }
}

Okay, I undertand. I’m just worried that the proposal is a net negative if the keywords stay optional. I’ll mention it in more detail once we get to the review period.

Thanks for the work on the proposal!!

I believe a breaking change has little chance of being accepted at this point in the language lifecycle. Adding opt-in compiler auditing to increase safety is, IMO, a net positive. It's a deliberate trade-off. We have included other designs to allow the core team to choose an alternative they feel is best for the philosophy and direction of Swift. This doesn't close the door to future language releases enhancing the concept, phasing out the second keyword, or introducing keywords for additional safety auditing.

I find it a dangerous philosophy to insist that any new proposal be ideologically pure. Imperfect proposals can still improve the language within the realities of the timelines, user base, and code base of the Swift community.

-- E

I share David’s concern. I also tend to think Erica’s correct about breaking changes. If the core team says “go ahead and break it,” then I’m all for it, but that seems unlikely.

FWIW, if we’re sticking with the two-keyword approach, the language could emit warnings for _any_ extension members that aren’t marked with either `extend` or `default`.

If the language were to do that, then it would certainly be superior to use the one-keyword approach, since this is equal breakage with an extra keyword.

I wouldn’t consider new warnings to be “breakage.” Warnings offer a gentler migration path than non-compilation.

Unfortunately, the single keyword approach wouldn’t support warnings the same way. So it’s really a question of what the tolerance for breakage is among the core team.

···

On Jun 16, 2017, at 2:26 PM, Xiaodi Wu <xiaodi.wu@gmail.com> wrote:
On Fri, Jun 16, 2017 at 14:06 Paul Cantrell via swift-evolution <swift-evolution@swift.org <mailto:swift-evolution@swift.org>> wrote:

On Jun 16, 2017, at 1:14 PM, Erica Sadun via swift-evolution <swift-evolution@swift.org <mailto:swift-evolution@swift.org>> wrote:

On Jun 16, 2017, at 8:44 AM, David Hart <davidhart@fastmail.com <mailto:davidhart@fastmail.com>> wrote:

P

On 16 Jun 2017, at 16:33, Erica Sadun <erica@ericasadun.com <mailto:erica@ericasadun.com>> wrote:

As we say in our introduction, we're pitching the most conservative approach.

The proposal was designed for minimal language impact. It chooses a conservative approach that can be phased in first over time and language release over more succinct alternatives that would impact existing code bases.

We discuss the one keyword version (which most of us are a fan of) in the alternatives. The core team has to decide how much they're willing to allow existing code to warn and/or break, which is the consequence of the one keyword solution.

-- E

On Jun 16, 2017, at 7:44 AM, David Hart <davidhart@fastmail.com <mailto:davidhart@fastmail.com>> wrote:

Erica, any thoughts on only having default and making it an error in a future version of Swift like was discussed on this thread? The idea seems to have a few supporters.

On 16 Jun 2017, at 15:33, Erica Sadun via swift-evolution <swift-evolution@swift.org <mailto:swift-evolution@swift.org>> wrote:

On Jun 14, 2017, at 11:46 PM, Dave Abrahams via swift-evolution <swift-evolution@swift.org <mailto:swift-evolution@swift.org>> wrote:

on Wed Jun 14 2017, Chris Lattner <swift-evolution@swift.org <mailto:swift-evolution@swift.org>> wrote:

On Jun 14, 2017, at 10:11 AM, Erica Sadun via swift-evolution >>>>>>>>> <swift-evolution@swift.org <mailto:swift-evolution@swift.org>> wrote:

Some pals and I have been kicking an idea around about introducing
better ways to support the compiler in protocol extensions. We want

to eliminate some hard-to-detect bugs. We've been brainstorming on
how to do this without affecting backward compatibility and
introducing a minimal impact on keywords.

We'd love to know what you think of our idea, which is to introduce
"role" keywords. Roles allow the compiler to automatically check the
intended use of a extension member definition against its protocol
declarations, and emit errors, warnings, and fixits as needed. We
think it's a pretty straightforward approach that, if adopted,
eliminates an entire category of bugs.

The draft proposal is here:
https://gist.github.com/erica/14283fe18254489c1498a7069b7760c4
<https://gist.github.com/erica/14283fe18254489c1498a7069b7760c4>

Thanks in advance for your thoughtful feedback,

+1 on the idea of this.

ditto. IMO it also makes the protocol extension much more expressive
and easy to read.

--
-Dave

Pull request: https://github.com/apple/swift-evolution/pull/724

-- E

_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org <mailto:swift-evolution@swift.org>
https://lists.swift.org/mailman/listinfo/swift-evolution

_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org <mailto:swift-evolution@swift.org>
https://lists.swift.org/mailman/listinfo/swift-evolution

_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org <mailto:swift-evolution@swift.org>
https://lists.swift.org/mailman/listinfo/swift-evolution

Okay, I undertand. I’m just worried that the proposal is a net negative
if the keywords stay optional. I’ll mention it in more detail once we get
to the review period.

Thanks for the work on the proposal!!

I believe a breaking change has little chance of being accepted at this
point in the language lifecycle. Adding opt-in compiler auditing to
increase safety is, IMO, a net positive. It's a deliberate trade-off. We
have included other designs to allow the core team to choose an alternative
they feel is best for the philosophy and direction of Swift. This doesn't
close the door to future language releases enhancing the concept, phasing
out the second keyword, or introducing keywords for additional safety
auditing.

I find it a dangerous philosophy to insist that any new proposal be
ideologically pure. Imperfect proposals can still improve the language
within the realities of the timelines, user base, and code base of the
Swift community.

-- E

I share David’s concern. I also tend to think Erica’s correct about
breaking changes. If the core team says “go ahead and break it,” then I’m
all for it, but that seems unlikely.

FWIW, if we’re sticking with the two-keyword approach, the language could
emit warnings for _any_ extension members that aren’t marked with either
`extend` or `default`.

If the language were to do that, then it would certainly be superior to
use the one-keyword approach, since this is equal breakage with an extra
keyword.

I wouldn’t consider new warnings to be “breakage.” Warnings offer a
gentler migration path than non-compilation.

Unfortunately, the single keyword approach wouldn’t support warnings the
same way.

No, but even that could offer benefits, and if transitional it would phase
in the benefits while phasing in the change.

So it’s really a question of what the tolerance for breakage is among the

core team.

Indeed.

···

On Fri, Jun 16, 2017 at 14:42 Paul Cantrell <cantrell@pobox.com> wrote:

On Jun 16, 2017, at 2:26 PM, Xiaodi Wu <xiaodi.wu@gmail.com> wrote:
On Fri, Jun 16, 2017 at 14:06 Paul Cantrell via swift-evolution < > swift-evolution@swift.org> wrote:

On Jun 16, 2017, at 1:14 PM, Erica Sadun via swift-evolution < >> swift-evolution@swift.org> wrote:
On Jun 16, 2017, at 8:44 AM, David Hart <davidhart@fastmail.com> wrote:

P

On 16 Jun 2017, at 16:33, Erica Sadun <erica@ericasadun.com> wrote:

As we say in our introduction, we're pitching the most conservative
approach.

The proposal was designed for minimal language impact. It chooses a
conservative approach that can be phased in first over time and language
release over more succinct alternatives that would impact existing code
bases.

We discuss the one keyword version (which most of us are a fan of) in the
alternatives. The core team has to decide how much they're willing to allow
existing code to warn and/or break, which is the consequence of the one
keyword solution.

-- E

On Jun 16, 2017, at 7:44 AM, David Hart <davidhart@fastmail.com> wrote:

Erica, any thoughts on only having default and making it an error in a
future version of Swift like was discussed on this thread? The idea seems
to have a few supporters.

On 16 Jun 2017, at 15:33, Erica Sadun via swift-evolution < >> swift-evolution@swift.org> wrote:

On Jun 14, 2017, at 11:46 PM, Dave Abrahams via swift-evolution < >> swift-evolution@swift.org> wrote:

on Wed Jun 14 2017, Chris Lattner <swift-evolution@swift.org> wrote:

On Jun 14, 2017, at 10:11 AM, Erica Sadun via swift-evolution >> <swift-evolution@swift.org> wrote:

Some pals and I have been kicking an idea around about introducing
better ways to support the compiler in protocol extensions. We want

to eliminate some hard-to-detect bugs. We've been brainstorming on
how to do this without affecting backward compatibility and
introducing a minimal impact on keywords.

We'd love to know what you think of our idea, which is to introduce
"role" keywords. Roles allow the compiler to automatically check the
intended use of a extension member definition against its protocol
declarations, and emit errors, warnings, and fixits as needed. We
think it's a pretty straightforward approach that, if adopted,
eliminates an entire category of bugs.

The draft proposal is here:
https://gist.github.com/erica/14283fe18254489c1498a7069b7760c4
<https://gist.github.com/erica/14283fe18254489c1498a7069b7760c4>

Thanks in advance for your thoughtful feedback,

+1 on the idea of this.

ditto. IMO it also makes the protocol extension much more expressive
and easy to read.

--
-Dave

Pull request: https://github.com/apple/swift-evolution/pull/724

-- E

_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution

_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution

_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution

I’ve already mentioned this in this thread. This is one of the missing features from the generics manifesto, however it’s (almost) a syntactic sugar only. It means that if we would enforce default in Swift 4, then having default implementation directly inside protocols would be really a syntactic and additional change only, which is a good sign. :wink:

···

--
Adrian Zubarev
Sent with Airmail

Am 16. Juni 2017 um 20:39:21, Tino Heth via swift-evolution (swift-evolution@swift.org) schrieb:

The described problem might be one of the most famous itches of the language, but imho the bar for new keywords* should be higher than that — and there are alternatives:

First, I guess many would like to see this to be valid Swift:

protocol Foo {
func bar() {
print("Default implementation called")
}
}

It's the most convenient way of avoiding typos: avoid to type :wink:
Imho this might already be enough, but for a full alternative for "default", I'd suggest something like this:

extension Foo {
func Foo.bar() {
print("String has its own implementation")
}
}

(to make it more familiar for those with a C++ background, "Foo::bar" could be used instead :wink:

Additional benefit: This wouldn't be limited to protocols — and it could possibly help in weird situations when two protocols declare functions with identical signature...

extension String: Foo {
func Foo.bar() {
print("String has its own implementation")
}

func Foo.barr() {
// compiler error, Foo defines no function "barr"
}

func barr() {
// this is fine, no connection to a protocol
}
}

- Tino

* well, I guess "default" is not really a new keyword… but you get the point
_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution

1 Like

The described problem might be one of the most famous itches of the language, but imho the bar for new keywords* should be higher than that — and there are alternatives:

First, I guess many would like to see this to be valid Swift:

protocol Foo {
  func bar() {
    print("Default implementation called")
  }
}

It's the most convenient way of avoiding typos: avoid to type :wink:

Absolutely. This is the more natural way to describe most default implementations; it’s more concise and eliminates the possibility of errors for the common case.

Imho this might already be enough, but for a full alternative for "default", I'd suggest something like this:

extension Foo {
  func Foo.bar() {
    print("String has its own implementation")
  }
}

On top of your first syntax, this would be useful when the extension is further constrained, e.g.,

extension Foo where Self: Comparable {
  func Foo.bar() {
    print(“I use Comparable for my Foo”)
  }
}

(to make it more familiar for those with a C++ background, "Foo::bar" could be used instead :wink:

Joking aside, “Foo::bar” has one advantage if it’s applied universally: it’s unambiguous if we allow it in method references. For example, we could say

  someString.Foo::bar()

to mean “call the function that String used to satisfy the requirement Foo.bar()”. If instead it were

  someString.Foo.Foo.bar()

it looks like we’re referring to a member named “Foo” within String, and a “bar” inside that. One would end up having to write the example differently, e.g.,

  (someString as Foo).bar()

C# has set some precedent for using “.” when declaring the function, though, and there are obvious advantages to not introducing a new sigil like “::” into Swift because it brings complexity and the potential for confusion with “.”.

Additional benefit: This wouldn't be limited to protocols — and it could possibly help in weird situations when two protocols declare functions with identical signature...

extension String: Foo {
  func Foo.bar() {
    print("String has its own implementation")
  }

  func Foo.barr() {
    // compiler error, Foo defines no function "barr"
  }

  func barr() {
    // this is fine, no connection to a protocol
  }
}

Absolutely.

Thanks for writing this up, Tino; I was going to send a very similar response :slight_smile:

  - Doug

···

On Jun 16, 2017, at 11:21 AM, Tino Heth via swift-evolution <swift-evolution@swift.org> wrote:

3 Likes

I wrote about how the problem this addresses has affected us at Tumblr:

http://developear.com/blog/2017/02/26/swift-protocols.html

TL/DR:

This is a massive problem in large Swift codebases

3 Likes

This has been covered in detail elsewhere (and probably in this thread) but there are several issues with making this a compile-time error, e.g. around library evolution, retroactive conformances, etc. I think it is generally agreed, however, that warnings are appropriate for situations like these (near misses with protocol methods, perhaps some sort of annotation that can provide warnings, etc).

One thing I am interested in hearing is how these default implementations have been “dangerous” in practice. You mention that this has hit you several times at Tumblr, but your example is a contrived one to demonstrate the general issue and not a specific practical example. In my head, a default implementation should only be provided if the implementation is obvious and can be implemented semantically correctly solely by composition of the other protocol requirements. I see overriding the default implementation as a way of optimising the implementation for a specific type, rather than doing something semantically entirely different. I think this is in line with the use of default implementations in the standard library and the accepted view of protocols in Swift as specifying semantics rather than just syntax. I think if you agree with this viewpoint then these issues become mainly “just” issues with run-time performance, rather than “dangerous bugs”.

I guess in your example you are saying that you might want to insert some analytics code around the implementation, so wanting to perform some side effect that is totally unrelated to the semantics of the protocol is perhaps one example. Are there other cases you have come across?

2 Likes

@jawbroken One specific example I have in mind is with my Reusable pod, which, in short, provides a default implementation to conforming UITableViewCells for reuseIdentifier:

public protocol Reusable: class {
  static var reuseIdentifier: String { get }
}

public extension UITableView {
  final func dequeueReusableCell<T: UITableViewCell>(for indexPath: IndexPath, cellType: T.Type = T.self) -> T where T: Reusable {
      guard let cell = self.dequeueReusableCell(withIdentifier: cellType.reuseIdentifier, for: indexPath) as? T else {
        fatalError("Failed to dequeue a cell with identifier \(cellType.reuseIdentifier) matching type \(cellType.self). Check that the reuseIdentifier is set properly in your XIB/Storyboard and that you registered the cell beforehand")
      }
      return cell
  }
}

This mixin allows you to do extension MyTableViewCell: Reusable { } and then magically be able to let cell = tableView.dequeueReusableCell(for: indexPath) as MyTableViewCell. But if you need to use an alternative reuseIdentifier for one of your custom cell you can override the default implementation.

The issue I had here is that I previously named that static var identifier: String { get } in the protocol, and used that in a bunch of projects. Then I later decided to rename that property reuseIdentifier instead.

But then suddenly every extension MyCustomTableViewCell: Reusable { static var identifier = "CustomID" } which intended to override their identifier ended up generating a fatalError and crash only at runtime (because it used the default implementation instead of the specialisation)… instead of having a compiler error or warning to tell me that the name mismatched, which would have saved me a lot of crashes

This is a real use case where missing that error is not just a missing opportunity for optimisation, but clearly introducing a new bug.

1 Like

For the benefit of others reading this, I dug up the code in question and there actually is a default implementation not shown above

public extension Reusable {
  /// By default, use the name of the class as String for its reuseIdentifier
  static var reuseIdentifier: String {
    return String(describing: self)
  }
}

I’m a little unclear as to how this fundamentally caused the bug, though. Your registration code (e.g. in UICollectionView+Reusable.swift) also uses reuseIdentifier to register the cell, so failing to override the default implementation would still seem to work fine (albeit with an unexpected reuse identifier) unless you were mixing protocol functionality with direct registration, which is probably generally a bad idea regardless of default implementations. Thanks for the example.

@jawbroken Whoops you’re right I forgot to quote the default impl, thanks for quoting it!

Are there other cases you have come across?

Many times that this has hit us has been during Swift migrations - where the function grammar changes (especially Swift 3) and has left us with hanging implementations that are never called.

Other times it’s usually just programmer error and its hard to reason about in a codebase like ours (~190k lines of Swift code). It is also hard to debug these errors since the function signature can look almost identical leaving engineers pulling their hair out wondering why their implementation isn’t the one being called but the default one is (or, in extreme cases - the opposite is totally possible by complete accident!)

1 Like

Apparently, I replied to this thread in June 2017 with lots of thoughts. I have no idea what this conversation is about anymore, and I suspect few others do. On mobile, I can’t even see the title of the thread.

Instead of resurrecting an extensive old conversation and referring to all of it as “this,” it’d be nice to have a new thread summarizing what’s already been said.

1 Like

Instead of resurrecting an extensive old conversation and referring to all of it as “this,” it’d be nice to have a new thread summarizing what’s already been said.

I am totally down to do so. What does everyone think about that?

1 Like

This is a great proposal, wish to see these become required keywords which perhaps would help the compiler fix the subclass issue too. I especially love example 4 about orphaned implementations, which are an issue even in non-default implementation conformances. I think an even safer and Swiftier approach is to require protocol conformance methods to be annotated as such with their protocol (similar to the override keyword for subclass methods).

3 Likes

If such an annotation would be mandatory, what about retroactive conformance?

This has been the crux of the issue ever since this topic was first raised. Retroactive conformance must be possible and nobody has proposed an acceptable design that allows for it.

One possibility I remember being raised a long time ago, is to let developers mark *extensions* with some annotation, whereby anything declared within that extension must be either a requirement of a certain protocol, or a helper method with restricted visibility. For example:

@exclusivelyForProtocol(SomeProtocol)
extension Foo {
  // Everything in here must be part of SomeProtocol
  // or a private helper method.
  // Anything else is an error.
}

I believe this is the main word. IMO It is possible to suggest a solution here, which respects the retroactive conformance, the question is if such suggestions will have “acceptable” requirements. Is new keyword acceptable? Is new attribute acceptable? Is new modifier acceptable? etc
Personally I believe the discussed problem worth new keyword/new attribute etc, but not sure what others think.

Unfortunately, I am at an impasse. My C++ skills are low to nonexistent. I cannot move forward without a prototype implementation and this is quite a complex task for anyone who is not an Swift information silo (thanks @Joe_Groff for the terminology). I haven’t the slightest idea how to find someone to help build this, so I’m limited in my proposal pull requests to items I have actually coded.

I think this proposal could have a significant contribution to language safety.

6 Likes