[Completing Generics] Completing protocol extension diagnostics


(Joe Groff) #1

Under the umbrella of completing generics, I think we should make room for improving our diagnostics around protocol extensions. They're a well-received feature, but they introduce a lot of surprising behavior, and introduce opportunity for subtle bugs. We didn't have time to put much diagnostic work in last year, but now that users have had time to work with the feature, we have evidence of some of the more surprising and problematic behavior. Among the most common things we've seen reported:

A) When a protocol requirement has an extension implementation requirement available, we'll silently ignore when a conforming type attempts to conform to the protocol but misses, by type or spelling:

protocol Channel {
  func receive() -> NSData
}
extension Channel {
  func receive() -> NSData { return NSData() }
}

// Silently fails to replace the default implementation:
struct ClumsyChannel: Channel {
  // Wrong spelling
  func recieve() -> NSData { return NSData(bytes: "oops", length: 4) }
  // Wrong return type
  func receive() -> [UInt8] { return Array("whoopsie".utf8) }
  // Wrong throwiness
  func receive() throws -> NSData { throw "doh" }
}

B) Protocol requirements aren't real class members, and can't be overridden by subclasses unless the base class satisfies the requirement with one of its own methods rather than with a protocol extension method, but we silently allow subclasses to shadow:

class BaseChannel: Channel { } // gets default imp from extension

class SubChannel: BaseChannel {
  // Doesn't 'override' protocol requirement; silently shadows it
  func receive() -> NSData { return NSData(bytes: "oof", length: 3) }
}

C) Similarly, protocol extension methods aren't protocol requirements, so extension methods that don't match a requirement can't be specialized in a way available to generic code, but we silently allow concrete type implementations to shadow:

extension Channel {
  func receiveAsString() -> String {
    return String(data: receive(), encoding: NSUTF8Encoding)
  }
}

struct StringyChannel {
  func receive() -> NSData { return NSData(bytes: "data", 4) }
  // Does not affect generic code calling receiveAsString
  func receiveAsString() -> String { return "string" }
}

func foo<T: Channel>(chan: T) {
  print(chan.receiveAsString())
}

foo(StringyChannel()) // Prints "data"

(B) and (C) could be mitigated by shadowing warnings, and we've also floated ideas for making them behave as intended, by implicitly forwarding protocol requirements into class methods to address (B) and/or introducing dynamic dispatch for protocol extensions to address (C). (A) is a bit trickier, since with overloading it's tricky to divine whether a declaration was really intended to match another one with a different type in isolation. We've discussed a couple approaches to this problem:

- Adopting an explicit 'implements' modifier, in the spirit of 'override', to mark a declaration as being intended to fulfill a requirement. This adds boilerplate we'd like to avoid, and also interferes with retroactive modeling.
- Encourage "one extension per conformance" style, where each protocol conformance's requirements are defined in a dedicated extension. We can then warn about any declarations in an extension that don't satisfy a requirement:

struct InconsistentChannel {}

extension InconsistentChannel: Channel {
  func receive() -> NSData { ... } // OK
  func recieve() -> NSData { ... } // Warning: Declaration in conformance extension doesn't satisfy any requirements from 'Channel'
  func receive() -> NSData? { ... } // Warning
}

There are likely others too. It'd be great if we could give users better guidance about this feature.

-Joe


(Brent Royal-Gordon) #2

(B) and (C) could be mitigated by shadowing warnings, and we've also floated ideas for making them behave as intended, by implicitly forwarding protocol requirements into class methods to address (B) and/or introducing dynamic dispatch for protocol extensions to address (C).

For what it's worth, I've posted threads several times aimed at addressing (C) through better warnings and keywording, and each time I've gotten lost of feedback from people who just want it dynamically dispatched. I wasn't aware of (B), but my guess is that I'd have heard the same thing if I'd brought it up.

- Adopting an explicit 'implements' modifier, in the spirit of 'override', to mark a declaration as being intended to fulfill a requirement. This adds boilerplate we'd like to avoid, and also interferes with retroactive modeling.

One possible way around that second issue is to permit you to mark the conformance itself as `implements` when you're doing retroactive modeling.

  protocol Arithmetic {
    func + (lhs: Self, rhs: Self) -> Self
    func - (lhs: Self, rhs: Self) -> Self
    func * (lhs: Self, rhs: Self) -> Self
    func / (lhs: Self, rhs: Self) -> Self
  }
  
  extension Int: implements Arithmetic {}
  extension Double: implements Arithmetic {}

If we just turned off the keyword requirement when the conformance was marked, people would abuse the feature, so perhaps it merely indicates that one or more members defined *outside* the current module implement the requirements. Any members *inside* the current module would still have to be individually marked. (And if the protocol wasn't at least partially implemented by external members, the `implements` in the conformance would be illegal.)

···

--
Brent Royal-Gordon
Architechies


(Haravikk) #3

I like the idea of encouraging "one extension per conformance", though I’d maybe adjust it to simply “add conformance through extension”, as sometimes it makes sense for a single extension to add conformance to multiple protocols at a time, for example an extension to add conformance to CustomDebugStringConvertible and CustomStringConvertible as a single item. I’m not sure if you intended to prevent that, but I think it should still be possible to allow multiple protocols with the same basic concept that conformance must be exact.

Regarding keyword usage for implementations, I’d say that we should have the override keyword at least on methods that shadow default implementations provided for protocol methods, as this should make things clearer. It could also help to tidy up documentation as currently default implementations that have been shadowed still show up in many places, even though they’re no longer strictly relevant.

···

On 4 Mar 2016, at 00:08, Joe Groff via swift-evolution <swift-evolution@swift.org> wrote:

Under the umbrella of completing generics, I think we should make room for improving our diagnostics around protocol extensions. They're a well-received feature, but they introduce a lot of surprising behavior, and introduce opportunity for subtle bugs. We didn't have time to put much diagnostic work in last year, but now that users have had time to work with the feature, we have evidence of some of the more surprising and problematic behavior. Among the most common things we've seen reported:

A) When a protocol requirement has an extension implementation requirement available, we'll silently ignore when a conforming type attempts to conform to the protocol but misses, by type or spelling:

protocol Channel {
  func receive() -> NSData
}
extension Channel {
  func receive() -> NSData { return NSData() }
}

// Silently fails to replace the default implementation:
struct ClumsyChannel: Channel {
  // Wrong spelling
  func recieve() -> NSData { return NSData(bytes: "oops", length: 4) }
  // Wrong return type
  func receive() -> [UInt8] { return Array("whoopsie".utf8) }
  // Wrong throwiness
  func receive() throws -> NSData { throw "doh" }
}

B) Protocol requirements aren't real class members, and can't be overridden by subclasses unless the base class satisfies the requirement with one of its own methods rather than with a protocol extension method, but we silently allow subclasses to shadow:

class BaseChannel: Channel { } // gets default imp from extension

class SubChannel: BaseChannel {
  // Doesn't 'override' protocol requirement; silently shadows it
  func receive() -> NSData { return NSData(bytes: "oof", length: 3) }
}

C) Similarly, protocol extension methods aren't protocol requirements, so extension methods that don't match a requirement can't be specialized in a way available to generic code, but we silently allow concrete type implementations to shadow:

extension Channel {
  func receiveAsString() -> String {
    return String(data: receive(), encoding: NSUTF8Encoding)
  }
}

struct StringyChannel {
  func receive() -> NSData { return NSData(bytes: "data", 4) }
  // Does not affect generic code calling receiveAsString
  func receiveAsString() -> String { return "string" }
}

func foo<T: Channel>(chan: T) {
  print(chan.receiveAsString())
}

foo(StringyChannel()) // Prints "data"

(B) and (C) could be mitigated by shadowing warnings, and we've also floated ideas for making them behave as intended, by implicitly forwarding protocol requirements into class methods to address (B) and/or introducing dynamic dispatch for protocol extensions to address (C). (A) is a bit trickier, since with overloading it's tricky to divine whether a declaration was really intended to match another one with a different type in isolation. We've discussed a couple approaches to this problem:

- Adopting an explicit 'implements' modifier, in the spirit of 'override', to mark a declaration as being intended to fulfill a requirement. This adds boilerplate we'd like to avoid, and also interferes with retroactive modeling.
- Encourage "one extension per conformance" style, where each protocol conformance's requirements are defined in a dedicated extension. We can then warn about any declarations in an extension that don't satisfy a requirement:

struct InconsistentChannel {}

extension InconsistentChannel: Channel {
  func receive() -> NSData { ... } // OK
  func recieve() -> NSData { ... } // Warning: Declaration in conformance extension doesn't satisfy any requirements from 'Channel'
  func receive() -> NSData? { ... } // Warning
}

There are likely others too. It'd be great if we could give users better guidance about this feature.

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


(Howard Lovatt) #4

For type A problems: why not require that you annotate with overload? In
Java they used to be like Swift; where a method that overrode an interface
declaration, no override was required. But they changed, due to these
problems, to always require override when implementing a previously
declared method, whether this was the first implementation of not. IE:

protocol Channel {
  func receive() -> NSData
}

extension Channel {
  func receive() -> NSData { return NSData() } // Error - no override and
declaration in protocol

  override func receive() -> NSData { return NSData() } // OK - note
override
}

// Silently fails to replace the default implementation:
struct ClumsyChannel: Channel {
  override func recieve() -> NSData { return NSData(bytes: "oops", length:
4) } // Error - Wrong spelling

  override func receive() -> [UInt8] { return Array("whoopsie".utf8) } //
Error - Wrong return type

  override func receive() throws -> NSData { throw "doh" } // Error - Wrong
throwiness

  func receive() -> NSData { return NSData(bytes: "Forgot", length: 6) } //
Error - no override and declaration in protocol

  override func receive() -> NSData { return NSData(bytes: "OK", length: 2) }
// OK - note override
}

This implies that methods implemented in extensions gain dynamic dispatch;
which I believe is under consideration anyway.

As an aside: I think that protocol extensions are only popular because you
can't put bodies in protocols. If bodies are allowed, I would think that
extensions to protocols will become the exception rather than the rule.

  -- Howard.

···

On 4 March 2016 at 11:08, Joe Groff via swift-evolution < swift-evolution@swift.org> wrote:

Under the umbrella of completing generics, I think we should make room for
improving our diagnostics around protocol extensions. They're a
well-received feature, but they introduce a lot of surprising behavior, and
introduce opportunity for subtle bugs. We didn't have time to put much
diagnostic work in last year, but now that users have had time to work with
the feature, we have evidence of some of the more surprising and
problematic behavior. Among the most common things we've seen reported:

A) When a protocol requirement has an extension implementation requirement
available, we'll silently ignore when a conforming type attempts to conform
to the protocol but misses, by type or spelling:

protocol Channel {
  func receive() -> NSData
}
extension Channel {
  func receive() -> NSData { return NSData() }
}

// Silently fails to replace the default implementation:
struct ClumsyChannel: Channel {
  // Wrong spelling
  func recieve() -> NSData { return NSData(bytes: "oops", length: 4) }
  // Wrong return type
  func receive() -> [UInt8] { return Array("whoopsie".utf8) }
  // Wrong throwiness
  func receive() throws -> NSData { throw "doh" }
}

B) Protocol requirements aren't real class members, and can't be
overridden by subclasses unless the base class satisfies the requirement
with one of its own methods rather than with a protocol extension method,
but we silently allow subclasses to shadow:

class BaseChannel: Channel { } // gets default imp from extension

class SubChannel: BaseChannel {
  // Doesn't 'override' protocol requirement; silently shadows it
  func receive() -> NSData { return NSData(bytes: "oof", length: 3) }
}

C) Similarly, protocol extension methods aren't protocol requirements, so
extension methods that don't match a requirement can't be specialized in a
way available to generic code, but we silently allow concrete type
implementations to shadow:

extension Channel {
  func receiveAsString() -> String {
    return String(data: receive(), encoding: NSUTF8Encoding)
  }
}

struct StringyChannel {
  func receive() -> NSData { return NSData(bytes: "data", 4) }
  // Does not affect generic code calling receiveAsString
  func receiveAsString() -> String { return "string" }
}

func foo<T: Channel>(chan: T) {
  print(chan.receiveAsString())
}

foo(StringyChannel()) // Prints "data"

(B) and (C) could be mitigated by shadowing warnings, and we've also
floated ideas for making them behave as intended, by implicitly forwarding
protocol requirements into class methods to address (B) and/or introducing
dynamic dispatch for protocol extensions to address (C). (A) is a bit
trickier, since with overloading it's tricky to divine whether a
declaration was really intended to match another one with a different type
in isolation. We've discussed a couple approaches to this problem:

- Adopting an explicit 'implements' modifier, in the spirit of 'override',
to mark a declaration as being intended to fulfill a requirement. This adds
boilerplate we'd like to avoid, and also interferes with retroactive
modeling.
- Encourage "one extension per conformance" style, where each protocol
conformance's requirements are defined in a dedicated extension. We can
then warn about any declarations in an extension that don't satisfy a
requirement:

struct InconsistentChannel {}

extension InconsistentChannel: Channel {
  func receive() -> NSData { ... } // OK
  func recieve() -> NSData { ... } // Warning: Declaration in conformance
extension doesn't satisfy any requirements from 'Channel'
  func receive() -> NSData? { ... } // Warning
}

There are likely others too. It'd be great if we could give users better
guidance about this feature.

-Joe

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


(Goffredo Marocchi) #5

Brent, why is dynamic dispatching for protocol extension default implementations wrong in your mind? Wouldn't you agree that when static dispatching introduces such a side effect that it should not be automatically applied and perhaps a keyword should be added if you really wanted static dispatching nonetheless?

I think that code execution should not be affected by type casting, it feels like a very confusing part of the language.

···

Sent from my iPhone

On 4 Mar 2016, at 07:06, Brent Royal-Gordon via swift-evolution <swift-evolution@swift.org> wrote:

(B) and (C) could be mitigated by shadowing warnings, and we've also floated ideas for making them behave as intended, by implicitly forwarding protocol requirements into class methods to address (B) and/or introducing dynamic dispatch for protocol extensions to address (C).

For what it's worth, I've posted threads several times aimed at addressing (C) through better warnings and keywording, and each time I've gotten lost of feedback from people who just want it dynamically dispatched. I wasn't aware of (B), but my guess is that I'd have heard the same thing if I'd brought it up.

- Adopting an explicit 'implements' modifier, in the spirit of 'override', to mark a declaration as being intended to fulfill a requirement. This adds boilerplate we'd like to avoid, and also interferes with retroactive modeling.

One possible way around that second issue is to permit you to mark the conformance itself as `implements` when you're doing retroactive modeling.

   protocol Arithmetic {
       func + (lhs: Self, rhs: Self) -> Self
       func - (lhs: Self, rhs: Self) -> Self
       func * (lhs: Self, rhs: Self) -> Self
       func / (lhs: Self, rhs: Self) -> Self
   }
   
   extension Int: implements Arithmetic {}
   extension Double: implements Arithmetic {}

If we just turned off the keyword requirement when the conformance was marked, people would abuse the feature, so perhaps it merely indicates that one or more members defined *outside* the current module implement the requirements. Any members *inside* the current module would still have to be individually marked. (And if the protocol wasn't at least partially implemented by external members, the `implements` in the conformance would be illegal.)

--
Brent Royal-Gordon
Architechies

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


(Tim Schmelter) #6

If we adopt the "one extension per conformance" model, would the compiler
then warn us if we include, say, helper functions that exist only to aid in
that conformance (and would therefore be appropriate to include in the
extension)? Wouldn't that lead to polluting the main type with those helper
methods?

--T

···

On Fri, Mar 4, 2016 at 3:38 AM, Haravikk via swift-evolution < swift-evolution@swift.org> wrote:

I like the idea of encouraging "one extension per conformance", though I’d
maybe adjust it to simply “add conformance through extension”, as sometimes
it makes sense for a single extension to add conformance to multiple
protocols at a time, for example an extension to add conformance to
CustomDebugStringConvertible and CustomStringConvertible as a single item.
I’m not sure if you intended to prevent that, but I think it should still
be possible to allow multiple protocols with the same basic concept that
conformance must be exact.

Regarding keyword usage for implementations, I’d say that we should have
the override keyword at least on methods that shadow default
implementations provided for protocol methods, as this should make things
clearer. It could also help to tidy up documentation as currently default
implementations that have been shadowed still show up in many places, even
though they’re no longer strictly relevant.

On 4 Mar 2016, at 00:08, Joe Groff via swift-evolution < > swift-evolution@swift.org> wrote:

Under the umbrella of completing generics, I think we should make room for
improving our diagnostics around protocol extensions. They're a
well-received feature, but they introduce a lot of surprising behavior, and
introduce opportunity for subtle bugs. We didn't have time to put much
diagnostic work in last year, but now that users have had time to work with
the feature, we have evidence of some of the more surprising and
problematic behavior. Among the most common things we've seen reported:

A) When a protocol requirement has an extension implementation requirement
available, we'll silently ignore when a conforming type attempts to conform
to the protocol but misses, by type or spelling:

protocol Channel {
  func receive() -> NSData
}
extension Channel {
  func receive() -> NSData { return NSData() }
}

// Silently fails to replace the default implementation:
struct ClumsyChannel: Channel {
  // Wrong spelling
  func recieve() -> NSData { return NSData(bytes: "oops", length: 4) }
  // Wrong return type
  func receive() -> [UInt8] { return Array("whoopsie".utf8) }
  // Wrong throwiness
  func receive() throws -> NSData { throw "doh" }
}

B) Protocol requirements aren't real class members, and can't be
overridden by subclasses unless the base class satisfies the requirement
with one of its own methods rather than with a protocol extension method,
but we silently allow subclasses to shadow:

class BaseChannel: Channel { } // gets default imp from extension

class SubChannel: BaseChannel {
  // Doesn't 'override' protocol requirement; silently shadows it
  func receive() -> NSData { return NSData(bytes: "oof", length: 3) }
}

C) Similarly, protocol extension methods aren't protocol requirements, so
extension methods that don't match a requirement can't be specialized in a
way available to generic code, but we silently allow concrete type
implementations to shadow:

extension Channel {
  func receiveAsString() -> String {
    return String(data: receive(), encoding: NSUTF8Encoding)
  }
}

struct StringyChannel {
  func receive() -> NSData { return NSData(bytes: "data", 4) }
  // Does not affect generic code calling receiveAsString
  func receiveAsString() -> String { return "string" }
}

func foo<T: Channel>(chan: T) {
  print(chan.receiveAsString())
}

foo(StringyChannel()) // Prints "data"

(B) and (C) could be mitigated by shadowing warnings, and we've also
floated ideas for making them behave as intended, by implicitly forwarding
protocol requirements into class methods to address (B) and/or introducing
dynamic dispatch for protocol extensions to address (C). (A) is a bit
trickier, since with overloading it's tricky to divine whether a
declaration was really intended to match another one with a different type
in isolation. We've discussed a couple approaches to this problem:

- Adopting an explicit 'implements' modifier, in the spirit of 'override',
to mark a declaration as being intended to fulfill a requirement. This adds
boilerplate we'd like to avoid, and also interferes with retroactive
modeling.
- Encourage "one extension per conformance" style, where each protocol
conformance's requirements are defined in a dedicated extension. We can
then warn about any declarations in an extension that don't satisfy a
requirement:

struct InconsistentChannel {}

extension InconsistentChannel: Channel {
  func receive() -> NSData { ... } // OK
  func recieve() -> NSData { ... } // Warning: Declaration in conformance
extension doesn't satisfy any requirements from 'Channel'
  func receive() -> NSData? { ... } // Warning
}

There are likely others too. It'd be great if we could give users better
guidance about this feature.

-Joe
_______________________________________________
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


(Brent Royal-Gordon) #7

For type A problems: why not require that you annotate with overload?

This is basically the same thing as the suggestion to use `implements`, except that `override` (which I assume is what you meant) is not really accurate since you only override inherited methods.

···

--
Brent Royal-Gordon
Architechies


(Howard Lovatt) #8

Yes. Just really saying that in another language, Java, they went through
the same discussion, chose override, and that it worked out well.

  -- Howard.

···

On 10 March 2016 at 14:55, Brent Royal-Gordon <brent@architechies.com> wrote:

> For type A problems: why not require that you annotate with overload?

This is basically the same thing as the suggestion to use `implements`,
except that `override` (which I assume is what you meant) is not really
accurate since you only override inherited methods.

--
Brent Royal-Gordon
Architechies


(Brent Royal-Gordon) #9

Brent, why is dynamic dispatching for protocol extension default implementations wrong in your mind? Wouldn't you agree that when static dispatching introduces such a side effect that it should not be automatically applied and perhaps a keyword should be added if you really wanted static dispatching nonetheless?

I think that code execution should not be affected by type casting, it feels like a very confusing part of the language.

I don't think dynamic dispatch is wrong; I think it's a large and technically challenging change. So in the spirit of incrementalism, I was trying to make cautious proposals which kept existing semantics intact but made them clearer, in preparation for perhaps eventually introducing dynamic dispatch. (Basically, I suggested that non-overridable protocol extension members should be marked `final` and it should be illegal to shadow them.)

But the feedback I got indicated that most people wanted a more aggressive proposal which introduced dynamic dispatch immediately. That's much harder to propose because it touches on all sorts of runtime implementation details I know nothing about, so I didn't try to draft a proposal.

(You are, perhaps inadvertently, currently demonstrating exactly what happened in those previous threads!)

···

--
Brent Royal-Gordon
Architechies


(Goffredo Marocchi) #10

Sorry, I understand and appreciate your pragmatism. Right now it feels very much like a fight to the ideological death between POP and OOP and it may get really bad results this way.

···

Sent from my iPhone

On 4 Mar 2016, at 08:58, Brent Royal-Gordon <brent@architechies.com> wrote:

Brent, why is dynamic dispatching for protocol extension default implementations wrong in your mind? Wouldn't you agree that when static dispatching introduces such a side effect that it should not be automatically applied and perhaps a keyword should be added if you really wanted static dispatching nonetheless?

I think that code execution should not be affected by type casting, it feels like a very confusing part of the language.

I don't think dynamic dispatch is wrong; I think it's a large and technically challenging change. So in the spirit of incrementalism, I was trying to make cautious proposals which kept existing semantics intact but made them clearer, in preparation for perhaps eventually introducing dynamic dispatch. (Basically, I suggested that non-overridable protocol extension members should be marked `final` and it should be illegal to shadow them.)

But the feedback I got indicated that most people wanted a more aggressive proposal which introduced dynamic dispatch immediately. That's much harder to propose because it touches on all sorts of runtime implementation details I know nothing about, so I didn't try to draft a proposal.

(You are, perhaps inadvertently, currently demonstrating exactly what happened in those previous threads!)

--
Brent Royal-Gordon
Architechies


(TJ Usiyan) #11
struct InconsistentChannel {}

extension InconsistentChannel: Channel {
  func receive() -> NSData { ... } // OK
  func recieve() -> NSData { ... } // Warning: Declaration in conformance
extension doesn't satisfy any requirements from 'Channel'
  func doSomething() -> NSData? { ... } // Warning
}

Would/Could the warning be silenced if `doSomething()` is called by one of
the methods that do satisfy requirements? (I know that it is possible, is
it reasonable?)

TJ

···

On Tue, Mar 8, 2016 at 11:34 AM, Tim Schmelter via swift-evolution < swift-evolution@swift.org> wrote:

If we adopt the "one extension per conformance" model, would the compiler
then warn us if we include, say, helper functions that exist only to aid in
that conformance (and would therefore be appropriate to include in the
extension)? Wouldn't that lead to polluting the main type with those helper
methods?

--T

On Fri, Mar 4, 2016 at 3:38 AM, Haravikk via swift-evolution < > swift-evolution@swift.org> wrote:

I like the idea of encouraging "one extension per conformance", though
I’d maybe adjust it to simply “add conformance through extension”, as
sometimes it makes sense for a single extension to add conformance to
multiple protocols at a time, for example an extension to add conformance
to CustomDebugStringConvertible and CustomStringConvertible as a single
item. I’m not sure if you intended to prevent that, but I think it should
still be possible to allow multiple protocols with the same basic concept
that conformance must be exact.

Regarding keyword usage for implementations, I’d say that we should have
the override keyword at least on methods that shadow default
implementations provided for protocol methods, as this should make things
clearer. It could also help to tidy up documentation as currently default
implementations that have been shadowed still show up in many places, even
though they’re no longer strictly relevant.

On 4 Mar 2016, at 00:08, Joe Groff via swift-evolution < >> swift-evolution@swift.org> wrote:

Under the umbrella of completing generics, I think we should make room
for improving our diagnostics around protocol extensions. They're a
well-received feature, but they introduce a lot of surprising behavior, and
introduce opportunity for subtle bugs. We didn't have time to put much
diagnostic work in last year, but now that users have had time to work with
the feature, we have evidence of some of the more surprising and
problematic behavior. Among the most common things we've seen reported:

A) When a protocol requirement has an extension implementation
requirement available, we'll silently ignore when a conforming type
attempts to conform to the protocol but misses, by type or spelling:

protocol Channel {
  func receive() -> NSData
}
extension Channel {
  func receive() -> NSData { return NSData() }
}

// Silently fails to replace the default implementation:
struct ClumsyChannel: Channel {
  // Wrong spelling
  func recieve() -> NSData { return NSData(bytes: "oops", length: 4) }
  // Wrong return type
  func receive() -> [UInt8] { return Array("whoopsie".utf8) }
  // Wrong throwiness
  func receive() throws -> NSData { throw "doh" }
}

B) Protocol requirements aren't real class members, and can't be
overridden by subclasses unless the base class satisfies the requirement
with one of its own methods rather than with a protocol extension method,
but we silently allow subclasses to shadow:

class BaseChannel: Channel { } // gets default imp from extension

class SubChannel: BaseChannel {
  // Doesn't 'override' protocol requirement; silently shadows it
  func receive() -> NSData { return NSData(bytes: "oof", length: 3) }
}

C) Similarly, protocol extension methods aren't protocol requirements, so
extension methods that don't match a requirement can't be specialized in a
way available to generic code, but we silently allow concrete type
implementations to shadow:

extension Channel {
  func receiveAsString() -> String {
    return String(data: receive(), encoding: NSUTF8Encoding)
  }
}

struct StringyChannel {
  func receive() -> NSData { return NSData(bytes: "data", 4) }
  // Does not affect generic code calling receiveAsString
  func receiveAsString() -> String { return "string" }
}

func foo<T: Channel>(chan: T) {
  print(chan.receiveAsString())
}

foo(StringyChannel()) // Prints "data"

(B) and (C) could be mitigated by shadowing warnings, and we've also
floated ideas for making them behave as intended, by implicitly forwarding
protocol requirements into class methods to address (B) and/or introducing
dynamic dispatch for protocol extensions to address (C). (A) is a bit
trickier, since with overloading it's tricky to divine whether a
declaration was really intended to match another one with a different type
in isolation. We've discussed a couple approaches to this problem:

- Adopting an explicit 'implements' modifier, in the spirit of
'override', to mark a declaration as being intended to fulfill a
requirement. This adds boilerplate we'd like to avoid, and also interferes
with retroactive modeling.
- Encourage "one extension per conformance" style, where each protocol
conformance's requirements are defined in a dedicated extension. We can
then warn about any declarations in an extension that don't satisfy a
requirement:

struct InconsistentChannel {}

extension InconsistentChannel: Channel {
  func receive() -> NSData { ... } // OK
  func recieve() -> NSData { ... } // Warning: Declaration in conformance
extension doesn't satisfy any requirements from 'Channel'
  func receive() -> NSData? { ... } // Warning
}

There are likely others too. It'd be great if we could give users better
guidance about this feature.

-Joe
_______________________________________________
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


(Howard Lovatt) #12

+1 to all of Goffredo's comments.

  -- Howard.

···

On 4 March 2016 at 18:41, Goffredo Marocchi via swift-evolution < swift-evolution@swift.org> wrote:

Brent, why is dynamic dispatching for protocol extension default
implementations wrong in your mind? Wouldn't you agree that when static
dispatching introduces such a side effect that it should not be
automatically applied and perhaps a keyword should be added if you really
wanted static dispatching nonetheless?

I think that code execution should not be affected by type casting, it
feels like a very confusing part of the language.

Sent from my iPhone

On 4 Mar 2016, at 07:06, Brent Royal-Gordon via swift-evolution < > swift-evolution@swift.org> wrote:

>> (B) and (C) could be mitigated by shadowing warnings, and we've also
floated ideas for making them behave as intended, by implicitly forwarding
protocol requirements into class methods to address (B) and/or introducing
dynamic dispatch for protocol extensions to address (C).
>
> For what it's worth, I've posted threads several times aimed at
addressing (C) through better warnings and keywording, and each time I've
gotten lost of feedback from people who just want it dynamically
dispatched. I wasn't aware of (B), but my guess is that I'd have heard the
same thing if I'd brought it up.
>
>> - Adopting an explicit 'implements' modifier, in the spirit of
'override', to mark a declaration as being intended to fulfill a
requirement. This adds boilerplate we'd like to avoid, and also interferes
with retroactive modeling.
>
> One possible way around that second issue is to permit you to mark the
conformance itself as `implements` when you're doing retroactive modeling.
>
> protocol Arithmetic {
> func + (lhs: Self, rhs: Self) -> Self
> func - (lhs: Self, rhs: Self) -> Self
> func * (lhs: Self, rhs: Self) -> Self
> func / (lhs: Self, rhs: Self) -> Self
> }
>
> extension Int: implements Arithmetic {}
> extension Double: implements Arithmetic {}
>
> If we just turned off the keyword requirement when the conformance was
marked, people would abuse the feature, so perhaps it merely indicates that
one or more members defined *outside* the current module implement the
requirements. Any members *inside* the current module would still have to
be individually marked. (And if the protocol wasn't at least partially
implemented by external members, the `implements` in the conformance would
be illegal.)
>
> --
> Brent Royal-Gordon
> Architechies
>
> _______________________________________________
> 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


(Joe Groff) #13
struct InconsistentChannel {}

extension InconsistentChannel: Channel {
  func receive() -> NSData { ... } // OK
  func recieve() -> NSData { ... } // Warning: Declaration in conformance extension doesn't satisfy any requirements from 'Channel'
  func doSomething() -> NSData? { ... } // Warning
}

Would/Could the warning be silenced if `doSomething()` is called by one of the methods that do satisfy requirements? (I know that it is possible, is it reasonable?)

That's an interesting idea. It seems reasonable to me.

-Joe

···

On Mar 8, 2016, at 9:24 AM, T.J. Usiyan via swift-evolution <swift-evolution@swift.org> wrote:

TJ

On Tue, Mar 8, 2016 at 11:34 AM, Tim Schmelter via swift-evolution <swift-evolution@swift.org <mailto:swift-evolution@swift.org>> wrote:
If we adopt the "one extension per conformance" model, would the compiler then warn us if we include, say, helper functions that exist only to aid in that conformance (and would therefore be appropriate to include in the extension)? Wouldn't that lead to polluting the main type with those helper methods?

--T

On Fri, Mar 4, 2016 at 3:38 AM, Haravikk via swift-evolution <swift-evolution@swift.org <mailto:swift-evolution@swift.org>> wrote:
I like the idea of encouraging "one extension per conformance", though I’d maybe adjust it to simply “add conformance through extension”, as sometimes it makes sense for a single extension to add conformance to multiple protocols at a time, for example an extension to add conformance to CustomDebugStringConvertible and CustomStringConvertible as a single item. I’m not sure if you intended to prevent that, but I think it should still be possible to allow multiple protocols with the same basic concept that conformance must be exact.

Regarding keyword usage for implementations, I’d say that we should have the override keyword at least on methods that shadow default implementations provided for protocol methods, as this should make things clearer. It could also help to tidy up documentation as currently default implementations that have been shadowed still show up in many places, even though they’re no longer strictly relevant.

On 4 Mar 2016, at 00:08, Joe Groff via swift-evolution <swift-evolution@swift.org <mailto:swift-evolution@swift.org>> wrote:

Under the umbrella of completing generics, I think we should make room for improving our diagnostics around protocol extensions. They're a well-received feature, but they introduce a lot of surprising behavior, and introduce opportunity for subtle bugs. We didn't have time to put much diagnostic work in last year, but now that users have had time to work with the feature, we have evidence of some of the more surprising and problematic behavior. Among the most common things we've seen reported:

A) When a protocol requirement has an extension implementation requirement available, we'll silently ignore when a conforming type attempts to conform to the protocol but misses, by type or spelling:

protocol Channel {
  func receive() -> NSData
}
extension Channel {
  func receive() -> NSData { return NSData() }
}

// Silently fails to replace the default implementation:
struct ClumsyChannel: Channel {
  // Wrong spelling
  func recieve() -> NSData { return NSData(bytes: "oops", length: 4) }
  // Wrong return type
  func receive() -> [UInt8] { return Array("whoopsie".utf8) }
  // Wrong throwiness
  func receive() throws -> NSData { throw "doh" }
}

B) Protocol requirements aren't real class members, and can't be overridden by subclasses unless the base class satisfies the requirement with one of its own methods rather than with a protocol extension method, but we silently allow subclasses to shadow:

class BaseChannel: Channel { } // gets default imp from extension

class SubChannel: BaseChannel {
  // Doesn't 'override' protocol requirement; silently shadows it
  func receive() -> NSData { return NSData(bytes: "oof", length: 3) }
}

C) Similarly, protocol extension methods aren't protocol requirements, so extension methods that don't match a requirement can't be specialized in a way available to generic code, but we silently allow concrete type implementations to shadow:

extension Channel {
  func receiveAsString() -> String {
    return String(data: receive(), encoding: NSUTF8Encoding)
  }
}

struct StringyChannel {
  func receive() -> NSData { return NSData(bytes: "data", 4) }
  // Does not affect generic code calling receiveAsString
  func receiveAsString() -> String { return "string" }
}

func foo<T: Channel>(chan: T) {
  print(chan.receiveAsString())
}

foo(StringyChannel()) // Prints "data"

(B) and (C) could be mitigated by shadowing warnings, and we've also floated ideas for making them behave as intended, by implicitly forwarding protocol requirements into class methods to address (B) and/or introducing dynamic dispatch for protocol extensions to address (C). (A) is a bit trickier, since with overloading it's tricky to divine whether a declaration was really intended to match another one with a different type in isolation. We've discussed a couple approaches to this problem:

- Adopting an explicit 'implements' modifier, in the spirit of 'override', to mark a declaration as being intended to fulfill a requirement. This adds boilerplate we'd like to avoid, and also interferes with retroactive modeling.
- Encourage "one extension per conformance" style, where each protocol conformance's requirements are defined in a dedicated extension. We can then warn about any declarations in an extension that don't satisfy a requirement:

struct InconsistentChannel {}

extension InconsistentChannel: Channel {
  func receive() -> NSData { ... } // OK
  func recieve() -> NSData { ... } // Warning: Declaration in conformance extension doesn't satisfy any requirements from 'Channel'
  func receive() -> NSData? { ... } // Warning
}

There are likely others too. It'd be great if we could give users better guidance about this feature.

-Joe
_______________________________________________
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

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


(Brent Royal-Gordon) #14

Would/Could the warning be silenced if `doSomething()` is called by one of the methods that do satisfy requirements? (I know that it is possible, is it reasonable?)

If the `scoped` proposal passes (note: I am still -1 on that proposal), that might be another criterion for permitting a non-conforming member in an extension.

···

--
Brent Royal-Gordon
Architechies


(TJ Usiyan) #15

I strongly suggest against requiring the scope keyword. This would force
you to move a method *out* of the scope if you also wanted to use it
outside of the conforming extension, which seems needlessly punitive.

TJ

···

On Tue, Mar 8, 2016 at 6:48 PM, Brent Royal-Gordon <brent@architechies.com> wrote:

> Would/Could the warning be silenced if `doSomething()` is called by one
of the methods that do satisfy requirements? (I know that it is possible,
is it reasonable?)

If the `scoped` proposal passes (note: I am still -1 on that proposal),
that might be another criterion for permitting a non-conforming member in
an extension.

--
Brent Royal-Gordon
Architechies