Ownership on protocol property requirements


(Greg Spiers) #1

Hello,

I hope this is the appropriate place to raise this discussion and see if
it's worth a proposal around which direction to take.

I've recently had some confusion around specifying weak for a property in a
protocol and surprised later of not being warned if the adopting type does
not also specify weak for the property. There is an open bug around this
behaviour [SR-479](https://bugs.swift.org/browse/SR-479). Example:

class A {}

protocol P {

    weak var weakVar: A? { get set }

}

class B: P {

    var weakVar: A? // Not declared weak, no compiler warning/error

}

Jordan Rose left the comment, "We need to figure out what weak means in a
protocol. We should probably ban it for now."

It is a little inconsistent to allow an ownership attribute in a protocol
but not enforce the same attribute in the adopting type I think. I guess
the point could be made that it's to document/hint that the type adopting
the protocol should use the ownership attribute suggested? The Swift
Programming Language reference has this to say about Property Requirements
in Protocols:

“A protocol can require any conforming type to provide an instance property
or type property with a particular name and type. The protocol doesn’t
specify whether the property should be a stored property or a computed
property—it only specifies the required property name and type.”

There’s no mention if the protocol can specify weak/unowned for a property.

My concern is that this is a warning when this situation happens in
Objective-C and there's another bug that makes this point [SR-1494](
https://bugs.swift.org/browse/SR-1494).

Would that be desirable behaviour to warn/error in Swift in the adopting
type? Or does it make more sense as Jordan commented on the original bug to
ban it in protocols (technically a source breaking change)?

I've done some initial investigation on how difficult it would be to
[implement banning it in protocols](
https://github.com/gspiers/swift/commit/ecde3ec5f61f259f8a396618e9973bac04536fd0).
This would be my first contribution so sorry the code will need some work
but I'm happy to try and get it in shape and work on a proposal if people
think it's worth fixing.

Thanks,

Greg Spiers


(Adrian Zubarev) #2

I had a short conversation on Twitter with Joe Groff, here is what he said about it: “nowned/weak are meaningless inside a protocol”.

···

--
Adrian Zubarev
Sent with Airmail

Am 7. Mai 2017 um 13:54:28, Greg Spiers via swift-evolution (swift-evolution@swift.org) schrieb:

Hello,

I hope this is the appropriate place to raise this discussion and see if it's worth a proposal around which direction to take.

I've recently had some confusion around specifying weak for a property in a protocol and surprised later of not being warned if the adopting type does not also specify weak for the property. There is an open bug around this behaviour [SR-479](https://bugs.swift.org/browse/SR-479). Example:

class A {}

protocol P {
weak var weakVar: A? { get set }
}

class B: P {
var weakVar: A? // Not declared weak, no compiler warning/error
}

Jordan Rose left the comment, "We need to figure out what weak means in a protocol. We should probably ban it for now."

It is a little inconsistent to allow an ownership attribute in a protocol but not enforce the same attribute in the adopting type I think. I guess the point could be made that it's to document/hint that the type adopting the protocol should use the ownership attribute suggested? The Swift Programming Language reference has this to say about Property Requirements in Protocols:

“A protocol can require any conforming type to provide an instance property or type property with a particular name and type. The protocol doesn’t specify whether the property should be a stored property or a computed property—it only specifies the required property name and type.”

There’s no mention if the protocol can specify weak/unowned for a property.

My concern is that this is a warning when this situation happens in Objective-C and there's another bug that makes this point [SR-1494](https://bugs.swift.org/browse/SR-1494).

Would that be desirable behaviour to warn/error in Swift in the adopting type? Or does it make more sense as Jordan commented on the original bug to ban it in protocols (technically a source breaking change)?

I've done some initial investigation on how difficult it would be to [implement banning it in protocols](https://github.com/gspiers/swift/commit/ecde3ec5f61f259f8a396618e9973bac04536fd0). This would be my first contribution so sorry the code will need some work but I'm happy to try and get it in shape and work on a proposal if people think it's worth fixing.

Thanks,

Greg Spiers

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


(Goffredo Marocchi) #3

It would be useful to have a longer discussion on this as... I think that weak has a place there and should be enforced as a protocol is the public facing interface/api for the types who decide to adopt it.

···

Sent from my iPhone

On 7 May 2017, at 15:41, Adrian Zubarev via swift-evolution <swift-evolution@swift.org> wrote:

browse


(Xiaodi Wu) #4

Today these keywords have no meaning inside a protocol, so clearly it
should be an error to use it in that context. I agree with Jordan that the
error should be on the protocol.

It's entirely a different conversation whether the keyword should have
meaning or not. If it should, it seems to me it should be limited to
protocols that are limited to classes. But that's an additive feature we
can discuss later.

The source-breaking bug fix that is more pressing today is removing
meaningless keywords that can be misleading to users, because they have no
effect but look like they should.

···

On Sun, May 7, 2017 at 11:00 Goffredo Marocchi via swift-evolution < swift-evolution@swift.org> wrote:

It would be useful to have a longer discussion on this as... I think that
weak has a place there and should be enforced as a protocol is the public
facing interface/api for the types who decide to adopt it.

Sent from my iPhone

> On 7 May 2017, at 15:41, Adrian Zubarev via swift-evolution < > swift-evolution@swift.org> wrote:
>
> browse
_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution


(Matthew Johnson) #5

Today these keywords have no meaning inside a protocol, so clearly it should be an error to use it in that context. I agree with Jordan that the error should be on the protocol.

It's entirely a different conversation whether the keyword should have meaning or not. If it should, it seems to me it should be limited to protocols that are limited to classes. But that's an additive feature we can discuss later.

Why would it make sense to limit this to class-constrained protocols? It would obviously make sense to limit it to properties of class or class-constrained type but I see no reason why an arbitrary restriction to class-constrained protocols would make sense even if that was how it is most commonly used.

···

Sent from my iPad

On May 7, 2017, at 1:12 PM, Xiaodi Wu via swift-evolution <swift-evolution@swift.org> wrote:

The source-breaking bug fix that is more pressing today is removing meaningless keywords that can be misleading to users, because they have no effect but look like they should.

On Sun, May 7, 2017 at 11:00 Goffredo Marocchi via swift-evolution <swift-evolution@swift.org> wrote:
It would be useful to have a longer discussion on this as... I think that weak has a place there and should be enforced as a protocol is the public facing interface/api for the types who decide to adopt it.

Sent from my iPhone

> On 7 May 2017, at 15:41, Adrian Zubarev via swift-evolution <swift-evolution@swift.org> wrote:
>
> browse
_______________________________________________
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


(Xiaodi Wu) #6

Sent from my iPad

Today these keywords have no meaning inside a protocol, so clearly it
should be an error to use it in that context. I agree with Jordan that the
error should be on the protocol.

It's entirely a different conversation whether the keyword should have
meaning or not. If it should, it seems to me it should be limited to
protocols that are limited to classes. But that's an additive feature we
can discuss later.

Why would it make sense to limit this to class-constrained protocols? It
would obviously make sense to limit it to properties of class or
class-constrained type but I see no reason why an arbitrary restriction to
class-constrained protocols would make sense even if that was how it is
most commonly used.

You're quite right.

···

On Sun, May 7, 2017 at 3:02 PM, Matthew Johnson <matthew@anandabits.com> wrote:

On May 7, 2017, at 1:12 PM, Xiaodi Wu via swift-evolution < > swift-evolution@swift.org> wrote:

The source-breaking bug fix that is more pressing today is removing
meaningless keywords that can be misleading to users, because they have no
effect but look like they should.
On Sun, May 7, 2017 at 11:00 Goffredo Marocchi via swift-evolution < > swift-evolution@swift.org> wrote:

It would be useful to have a longer discussion on this as... I think that
weak has a place there and should be enforced as a protocol is the public
facing interface/api for the types who decide to adopt it.

Sent from my iPhone

> On 7 May 2017, at 15:41, Adrian Zubarev via swift-evolution < >> swift-evolution@swift.org> wrote:
>
> browse
_______________________________________________
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


(David Hart) #7

Today these keywords have no meaning inside a protocol, so clearly it should be an error to use it in that context. I agree with Jordan that the error should be on the protocol.

It's entirely a different conversation whether the keyword should have meaning or not. If it should, it seems to me it should be limited to protocols that are limited to classes. But that's an additive feature we can discuss later.

The source-breaking bug fix that is more pressing today is removing meaningless keywords that can be misleading to users, because they have no effect but look like they should.

Yup, +1. Who wants to write a proposal?

···

On 7 May 2017, at 20:12, Xiaodi Wu via swift-evolution <swift-evolution@swift.org> wrote:

On Sun, May 7, 2017 at 11:00 Goffredo Marocchi via swift-evolution <swift-evolution@swift.org> wrote:
It would be useful to have a longer discussion on this as... I think that weak has a place there and should be enforced as a protocol is the public facing interface/api for the types who decide to adopt it.

Sent from my iPhone

> On 7 May 2017, at 15:41, Adrian Zubarev via swift-evolution <swift-evolution@swift.org> wrote:
>
> browse
_______________________________________________
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


(Greg Spiers) #8

Today these keywords have no meaning inside a protocol, so clearly it
should be an error to use it in that context. I agree with Jordan that the
error should be on the protocol.

It's entirely a different conversation whether the keyword should have
meaning or not. If it should, it seems to me it should be limited to
protocols that are limited to classes. But that's an additive feature we
can discuss later.

The source-breaking bug fix that is more pressing today is removing
meaningless keywords that can be misleading to users, because they have no
effect but look like they should.

Exactly the trap I fell into when I found this issue.

Yup, +1. Who wants to write a proposal?

I'd like to give it a try. I can write up the proposal to remove the
keywords in protocols and will post a draft here for further discussion.

···

On Mon, May 8, 2017 at 6:26 AM, David Hart via swift-evolution < swift-evolution@swift.org> wrote:

On 7 May 2017, at 20:12, Xiaodi Wu via swift-evolution < > swift-evolution@swift.org> wrote:

On Sun, May 7, 2017 at 11:00 Goffredo Marocchi via swift-evolution < > swift-evolution@swift.org> wrote:

It would be useful to have a longer discussion on this as... I think that
weak has a place there and should be enforced as a protocol is the public
facing interface/api for the types who decide to adopt it.

Sent from my iPhone

> On 7 May 2017, at 15:41, Adrian Zubarev via swift-evolution < >> swift-evolution@swift.org> wrote:
>
> browse
_______________________________________________
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


(David Hart) #9

Sounds great! It should be an easy one to get through,

···

On 8 May 2017, at 08:35, Greg Spiers <gspiers@gmail.com> wrote:

On Mon, May 8, 2017 at 6:26 AM, David Hart via swift-evolution <swift-evolution@swift.org <mailto:swift-evolution@swift.org>> wrote:

On 7 May 2017, at 20:12, Xiaodi Wu via swift-evolution <swift-evolution@swift.org <mailto:swift-evolution@swift.org>> wrote:

Today these keywords have no meaning inside a protocol, so clearly it should be an error to use it in that context. I agree with Jordan that the error should be on the protocol.

It's entirely a different conversation whether the keyword should have meaning or not. If it should, it seems to me it should be limited to protocols that are limited to classes. But that's an additive feature we can discuss later.

The source-breaking bug fix that is more pressing today is removing meaningless keywords that can be misleading to users, because they have no effect but look like they should.

Exactly the trap I fell into when I found this issue.

Yup, +1. Who wants to write a proposal?

I'd like to give it a try. I can write up the proposal to remove the keywords in protocols and will post a draft here for further discussion.

On Sun, May 7, 2017 at 11:00 Goffredo Marocchi via swift-evolution <swift-evolution@swift.org <mailto:swift-evolution@swift.org>> wrote:
It would be useful to have a longer discussion on this as... I think that weak has a place there and should be enforced as a protocol is the public facing interface/api for the types who decide to adopt it.

Sent from my iPhone

> On 7 May 2017, at 15:41, Adrian Zubarev via swift-evolution <swift-evolution@swift.org <mailto:swift-evolution@swift.org>> wrote:
>
> browse
_______________________________________________
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


(Goffredo Marocchi) #10

Over my dead body --random list dweller :wink:

Seriously though, I think the labels should be made to matter not removed if they do not matter now. I think this goes to a path where we should not take protocols as they should be true contracts for the API in question (default method in protocols make me think we have to write unit tests for a protocol which sounds mad... oh well) although some may argue the ownership info is implementation detail and on that point I may agree with you ;).

···

Sent from my iPhone

On 8 May 2017, at 07:57, David Hart via swift-evolution <swift-evolution@swift.org> wrote:

Sounds great! It should be an easy one to get through,

On 8 May 2017, at 08:35, Greg Spiers <gspiers@gmail.com> wrote:

On Mon, May 8, 2017 at 6:26 AM, David Hart via swift-evolution <swift-evolution@swift.org> wrote:

On 7 May 2017, at 20:12, Xiaodi Wu via swift-evolution <swift-evolution@swift.org> wrote:

Today these keywords have no meaning inside a protocol, so clearly it should be an error to use it in that context. I agree with Jordan that the error should be on the protocol.

It's entirely a different conversation whether the keyword should have meaning or not. If it should, it seems to me it should be limited to protocols that are limited to classes. But that's an additive feature we can discuss later.

The source-breaking bug fix that is more pressing today is removing meaningless keywords that can be misleading to users, because they have no effect but look like they should.

Exactly the trap I fell into when I found this issue.

Yup, +1. Who wants to write a proposal?

I'd like to give it a try. I can write up the proposal to remove the keywords in protocols and will post a draft here for further discussion.

On Sun, May 7, 2017 at 11:00 Goffredo Marocchi via swift-evolution <swift-evolution@swift.org> wrote:
It would be useful to have a longer discussion on this as... I think that weak has a place there and should be enforced as a protocol is the public facing interface/api for the types who decide to adopt it.

Sent from my iPhone

> On 7 May 2017, at 15:41, Adrian Zubarev via swift-evolution <swift-evolution@swift.org> wrote:
>
> browse
_______________________________________________
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

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


(David Hart) #11

Over my dead body --random list dweller :wink:

Seriously though, I think the labels should be made to matter not removed if they do not matter now. I think this goes to a path where we should not take protocols as they should be true contracts for the API in question (default method in protocols make me think we have to write unit tests for a protocol which sounds mad... oh well) although some may argue the ownership info is implementation detail and on that point I may agree with you ;).

Agreed. But we don’t have the time to bring meaning to them in time for Swift 4. Its better to make the language consistent now (disallowing a keyword which currently has no meaning) and allow ourselves to reintroduce later with correct semantics.

···

On 8 May 2017, at 09:03, Goffredo Marocchi <panajev@gmail.com> wrote:

Sent from my iPhone

On 8 May 2017, at 07:57, David Hart via swift-evolution <swift-evolution@swift.org <mailto:swift-evolution@swift.org>> wrote:

Sounds great! It should be an easy one to get through,

On 8 May 2017, at 08:35, Greg Spiers <gspiers@gmail.com <mailto:gspiers@gmail.com>> wrote:

On Mon, May 8, 2017 at 6:26 AM, David Hart via swift-evolution <swift-evolution@swift.org <mailto:swift-evolution@swift.org>> wrote:

On 7 May 2017, at 20:12, Xiaodi Wu via swift-evolution <swift-evolution@swift.org <mailto:swift-evolution@swift.org>> wrote:

Today these keywords have no meaning inside a protocol, so clearly it should be an error to use it in that context. I agree with Jordan that the error should be on the protocol.

It's entirely a different conversation whether the keyword should have meaning or not. If it should, it seems to me it should be limited to protocols that are limited to classes. But that's an additive feature we can discuss later.

The source-breaking bug fix that is more pressing today is removing meaningless keywords that can be misleading to users, because they have no effect but look like they should.

Exactly the trap I fell into when I found this issue.

Yup, +1. Who wants to write a proposal?

I'd like to give it a try. I can write up the proposal to remove the keywords in protocols and will post a draft here for further discussion.

On Sun, May 7, 2017 at 11:00 Goffredo Marocchi via swift-evolution <swift-evolution@swift.org <mailto:swift-evolution@swift.org>> wrote:
It would be useful to have a longer discussion on this as... I think that weak has a place there and should be enforced as a protocol is the public facing interface/api for the types who decide to adopt it.

Sent from my iPhone

> On 7 May 2017, at 15:41, Adrian Zubarev via swift-evolution <swift-evolution@swift.org <mailto:swift-evolution@swift.org>> wrote:
>
> browse
_______________________________________________
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 <mailto:swift-evolution@swift.org>
https://lists.swift.org/mailman/listinfo/swift-evolution


(Greg Spiers) #12

Thanks David, appreciate it :slight_smile: I've created a draft of the proposal.
Any feedback would be very welcome. I wasn't sure if there was an
effect on ABI stability or API resilience. I don't think there would
be as this is only correcting syntax.

Also, just adding the link again where I had a try at implementing the
warning/error. Hopefully that can be helpful in getting feedback about
the proposal as well.
https://github.com/gspiers/swift/commit/ecde3ec5f61f259f8a396618e9973bac04536fd0

# Remove ownership keyword support in protocols

* Proposal: [SE-NNNN](NNNN-remove-ownership-keyword-support-in-protocols.md)
* Authors: [Greg Spiers](https://github.com/gspiers)
* Review Manager: TBD
* Status:
* Bug: [SR-479](https://bugs.swift.org/browse/SR-479)

## Introduction

This proposal removes support for the keywords `weak` and `unowned`
for property declarations in a protocol.

Swift-evolution thread: [Ownership on protocol property
requirements](https://lists.swift.org/pipermail/swift-evolution/Week-of-Mon-20170501/036495.html)
thread. (TODO: Add link to rest of discussion)

## Motivation

Currently it's possible to use the weak/unowned keywords for a
property requirement in a protocol. This can lead to confusion as
specifying one of these keywords does not enforce or raise any
warnings in the adopting type of that protocol:


class A {}

protocol P {
    weak var weakVar: A? { get set }
}

class B: P {
    var weakVar: A? // Not declared weak, no compiler warning/error
}

This can lead to unexpected and surprising behaviour from the point of
view of users. They keywords themselves are currently meaningless
inside of a protocol but look like they would have an effect when the
protocol is adopted.

## Proposed solution

Although the case could be made that the keywords should have meaning
in a protocol, as they are currently implemented today they don't have
an effect. This proposal aims to cleanup the misleading syntax and
isn't meant to remove functionality only correct to existing
behaviour.

This proposal suggests removing support for `weak` and `unowned` in a protocol.

## Detailed design

The compiler will flag the use of `weak` and `unowned` in a protocol
and suggest a fix to remove the keyword.

## Source compatibility

This is a source breaking change but one that would only correct code
that is already has broken assumptions. For existing use the compiler
will raise a compilation error. When running in Swift 3 mode a warning
can be generated instead of an error. It could be possible to address
source compatibility through source migration as well.

## Effect on ABI stability

This proposal does not affect ABI stability.

## Effect on API resilience

This proposal should not effect API resilience.

## Alternatives considered

There is an argument in making `weak` and `unowned` have meaning in a
protocol but this does open up other questions and is probably better
as a topic of a separate discussion/proposal. As this would be
additive it can be addressed at a later point when we have a clearer
understanding.

···

On Mon, May 8, 2017 at 7:57 AM, David Hart <david@hartbit.com> wrote:

Sounds great! It should be an easy one to get through,


(Xiaodi Wu) #13

I can understand that, I am just wary of "let's do a partially detrimental
change

The key here is that there is no detriment to this change. There's no
functionality that's being removed, only misleading syntax.

no... we will... we will make it proper someday" kind of changes as they
seldom work out. Argument labels for stored closures and callbacks are
still lost for example :/...

That's a totally different issue. Someone needs to write the proposal and
implementation for that.

Also, while here we keep pushing things Core Team's way, Ted K. is asking

···

On Mon, May 8, 2017 at 2:40 AM, Goffredo Marocchi via swift-evolution < swift-evolution@swift.org> wrote:

for devs' help on swift-dev as there is concern that the currently accepted
proposals may not make it with this year's new Swift version (second year
in a row). Should we discuss features in scope for Swift 4.1+ only now?

Sent from my iPhone

On 8 May 2017, at 08:12, David Hart <david@hartbit.com> wrote:

On 8 May 2017, at 09:03, Goffredo Marocchi <panajev@gmail.com> wrote:

Over my dead body --random list dweller :wink:

Seriously though, I think the labels should be made to matter not removed
if they do not matter now. I think this goes to a path where we should not
take protocols as they should be true contracts for the API in question
(default method in protocols make me think we have to write unit tests for
a protocol which sounds mad... oh well) although some may argue the
ownership info is implementation detail and on that point I may agree with
you ;).

Agreed. But we don’t have the time to bring meaning to them in time for
Swift 4. Its better to make the language consistent now (disallowing a
keyword which currently has no meaning) and allow ourselves to reintroduce
later with correct semantics.

Sent from my iPhone

On 8 May 2017, at 07:57, David Hart via swift-evolution < > swift-evolution@swift.org> wrote:

Sounds great! It should be an easy one to get through,

On 8 May 2017, at 08:35, Greg Spiers <gspiers@gmail.com> wrote:

On Mon, May 8, 2017 at 6:26 AM, David Hart via swift-evolution < > swift-evolution@swift.org> wrote:

On 7 May 2017, at 20:12, Xiaodi Wu via swift-evolution < >> swift-evolution@swift.org> wrote:

Today these keywords have no meaning inside a protocol, so clearly it
should be an error to use it in that context. I agree with Jordan that the
error should be on the protocol.

It's entirely a different conversation whether the keyword should have
meaning or not. If it should, it seems to me it should be limited to
protocols that are limited to classes. But that's an additive feature we
can discuss later.

The source-breaking bug fix that is more pressing today is removing
meaningless keywords that can be misleading to users, because they have no
effect but look like they should.

Exactly the trap I fell into when I found this issue.

Yup, +1. Who wants to write a proposal?

I'd like to give it a try. I can write up the proposal to remove the
keywords in protocols and will post a draft here for further discussion.

On Sun, May 7, 2017 at 11:00 Goffredo Marocchi via swift-evolution < >> swift-evolution@swift.org> wrote:

It would be useful to have a longer discussion on this as... I think
that weak has a place there and should be enforced as a protocol is the
public facing interface/api for the types who decide to adopt it.

Sent from my iPhone

> On 7 May 2017, at 15:41, Adrian Zubarev via swift-evolution < >>> swift-evolution@swift.org> wrote:
>
> browse
_______________________________________________
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

_______________________________________________
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


(Xiaodi Wu) #14

Sent from my iPhone

I can understand that, I am just wary of "let's do a partially
detrimental change

The key here is that there is no detriment to this change. There's no
functionality that's being removed, only misleading syntax.

Why is it misleading? Because it is not enforced,

Right, and there is clearly no chance of it being enforced in Swift 4, so
it is either going to be unenforced and misleading, or not permitted and
not misleading.

but protocol extension default methods being shadowable but not
overridable and presenting no warning is a very similar issue...

That is totally another topic.

if the answer is the same there (at least for classes), I am more than
happy to get both changes ;).

no... we will... we will make it proper someday" kind of changes as they
seldom work out. Argument labels for stored closures and callbacks are
still lost for example :/...

That's a totally different issue. Someone needs to write the proposal and
implementation for that.

Would you and some other be willing to help me (Erica?)? A lot to ask, but
it would be important although a bit beyond the scope for "dweller's first
proposal" especially the implementation and grammar side. I do not think it
has a iota of chance for it to get integrate in time though :/.

It clearly doesn't have any chance of implementation for Swift 4. The
proposal itself doesn't need much as it's already spelled out; it's
entirely the implementation that needs work, for which I have neither the
time nor the expertise. Since it is an additive change, though, there
really is no rush as it can be done at any time. There are just so many
other priorities.

···

On Mon, May 8, 2017 at 2:52 AM, Goffredo Marocchi <panajev@gmail.com> wrote:

On 8 May 2017, at 08:44, Xiaodi Wu <xiaodi.wu@gmail.com> wrote:
On Mon, May 8, 2017 at 2:40 AM, Goffredo Marocchi via swift-evolution < > swift-evolution@swift.org> wrote:


(Goffredo Marocchi) #15

I can understand that, I am just wary of "let's do a partially detrimental change no... we will... we will make it proper someday" kind of changes as they seldom work out. Argument labels for stored closures and callbacks are still lost for example :/...

Also, while here we keep pushing things Core Team's way, Ted K. is asking for devs' help on swift-dev as there is concern that the currently accepted proposals may not make it with this year's new Swift version (second year in a row). Should we discuss features in scope for Swift 4.1+ only now?

···

Sent from my iPhone

On 8 May 2017, at 08:12, David Hart <david@hartbit.com> wrote:

On 8 May 2017, at 09:03, Goffredo Marocchi <panajev@gmail.com> wrote:

Over my dead body --random list dweller :wink:

Seriously though, I think the labels should be made to matter not removed if they do not matter now. I think this goes to a path where we should not take protocols as they should be true contracts for the API in question (default method in protocols make me think we have to write unit tests for a protocol which sounds mad... oh well) although some may argue the ownership info is implementation detail and on that point I may agree with you ;).

Agreed. But we don’t have the time to bring meaning to them in time for Swift 4. Its better to make the language consistent now (disallowing a keyword which currently has no meaning) and allow ourselves to reintroduce later with correct semantics.

Sent from my iPhone

On 8 May 2017, at 07:57, David Hart via swift-evolution <swift-evolution@swift.org> wrote:

Sounds great! It should be an easy one to get through,

On 8 May 2017, at 08:35, Greg Spiers <gspiers@gmail.com> wrote:

On Mon, May 8, 2017 at 6:26 AM, David Hart via swift-evolution <swift-evolution@swift.org> wrote:

On 7 May 2017, at 20:12, Xiaodi Wu via swift-evolution <swift-evolution@swift.org> wrote:

Today these keywords have no meaning inside a protocol, so clearly it should be an error to use it in that context. I agree with Jordan that the error should be on the protocol.

It's entirely a different conversation whether the keyword should have meaning or not. If it should, it seems to me it should be limited to protocols that are limited to classes. But that's an additive feature we can discuss later.

The source-breaking bug fix that is more pressing today is removing meaningless keywords that can be misleading to users, because they have no effect but look like they should.

Exactly the trap I fell into when I found this issue.

Yup, +1. Who wants to write a proposal?

I'd like to give it a try. I can write up the proposal to remove the keywords in protocols and will post a draft here for further discussion.

On Sun, May 7, 2017 at 11:00 Goffredo Marocchi via swift-evolution <swift-evolution@swift.org> wrote:
It would be useful to have a longer discussion on this as... I think that weak has a place there and should be enforced as a protocol is the public facing interface/api for the types who decide to adopt it.

Sent from my iPhone

> On 7 May 2017, at 15:41, Adrian Zubarev via swift-evolution <swift-evolution@swift.org> wrote:
>
> browse
_______________________________________________
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

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


(Goffredo Marocchi) #16

I can understand that, I am just wary of "let's do a partially detrimental change

The key here is that there is no detriment to this change. There's no functionality that's being removed, only misleading syntax.

Why is it misleading? Because it is not enforced, but protocol extension default methods being shadowable but not overridable and presenting no warning is a very similar issue... if the answer is the same there (at least for classes), I am more than happy to get both changes ;).

no... we will... we will make it proper someday" kind of changes as they seldom work out. Argument labels for stored closures and callbacks are still lost for example :/...

That's a totally different issue. Someone needs to write the proposal and implementation for that.

Would you and some other be willing to help me (Erica?)? A lot to ask, but it would be important although a bit beyond the scope for "dweller's first proposal" especially the implementation and grammar side. I do not think it has a iota of chance for it to get integrate in time though :/.

···

Sent from my iPhone

On 8 May 2017, at 08:44, Xiaodi Wu <xiaodi.wu@gmail.com> wrote:

On Mon, May 8, 2017 at 2:40 AM, Goffredo Marocchi via swift-evolution <swift-evolution@swift.org> wrote:

Also, while here we keep pushing things Core Team's way, Ted K. is asking for devs' help on swift-dev as there is concern that the currently accepted proposals may not make it with this year's new Swift version (second year in a row). Should we discuss features in scope for Swift 4.1+ only now?

Sent from my iPhone

On 8 May 2017, at 08:12, David Hart <david@hartbit.com> wrote:

On 8 May 2017, at 09:03, Goffredo Marocchi <panajev@gmail.com> wrote:

Over my dead body --random list dweller :wink:

Seriously though, I think the labels should be made to matter not removed if they do not matter now. I think this goes to a path where we should not take protocols as they should be true contracts for the API in question (default method in protocols make me think we have to write unit tests for a protocol which sounds mad... oh well) although some may argue the ownership info is implementation detail and on that point I may agree with you ;).

Agreed. But we don’t have the time to bring meaning to them in time for Swift 4. Its better to make the language consistent now (disallowing a keyword which currently has no meaning) and allow ourselves to reintroduce later with correct semantics.

Sent from my iPhone

On 8 May 2017, at 07:57, David Hart via swift-evolution <swift-evolution@swift.org> wrote:

Sounds great! It should be an easy one to get through,

On 8 May 2017, at 08:35, Greg Spiers <gspiers@gmail.com> wrote:

On Mon, May 8, 2017 at 6:26 AM, David Hart via swift-evolution <swift-evolution@swift.org> wrote:

On 7 May 2017, at 20:12, Xiaodi Wu via swift-evolution <swift-evolution@swift.org> wrote:

Today these keywords have no meaning inside a protocol, so clearly it should be an error to use it in that context. I agree with Jordan that the error should be on the protocol.

It's entirely a different conversation whether the keyword should have meaning or not. If it should, it seems to me it should be limited to protocols that are limited to classes. But that's an additive feature we can discuss later.

The source-breaking bug fix that is more pressing today is removing meaningless keywords that can be misleading to users, because they have no effect but look like they should.

Exactly the trap I fell into when I found this issue.

Yup, +1. Who wants to write a proposal?

I'd like to give it a try. I can write up the proposal to remove the keywords in protocols and will post a draft here for further discussion.

On Sun, May 7, 2017 at 11:00 Goffredo Marocchi via swift-evolution <swift-evolution@swift.org> wrote:
It would be useful to have a longer discussion on this as... I think that weak has a place there and should be enforced as a protocol is the public facing interface/api for the types who decide to adopt it.

Sent from my iPhone

> On 7 May 2017, at 15:41, Adrian Zubarev via swift-evolution <swift-evolution@swift.org> wrote:
>
> browse
_______________________________________________
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

_______________________________________________
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


(Greg Spiers) #17

Sent from my iPhone

I can understand that, I am just wary of "let's do a partially detrimental change

The key here is that there is no detriment to this change. There's no functionality that's being removed, only misleading syntax.

Why is it misleading? Because it is not enforced, but protocol extension default methods being shadowable but not overridable and presenting no warning is a very similar issue... if the answer is the same there (at least for classes), I am more than happy to get both changes ;).

When I found the problem I thought specifying weak on a property in a
protocol should have meaning. But I do agree that I think it needs
much more discussion around what those details mean. As you said, it
could be argued that ownership is just an implementation detail and
shouldn't be specified as part of the contract of a protocol.

There's also questions around how the conforming class satisfies the
protocol. What if I use a computed property, does it make sense to
specify ownership weak/unowned for a computed property if it was
specified as weak/unowned in the protocol?

no... we will... we will make it proper someday" kind of changes as they seldom work out. Argument labels for stored closures and callbacks are still lost for example :/...

That's a totally different issue. Someone needs to write the proposal and implementation for that.

Would you and some other be willing to help me (Erica?)? A lot to ask, but it would be important although a bit beyond the scope for "dweller's first proposal" especially the implementation and grammar side. I do not think it has a iota of chance for it to get integrate in time though :/.

I know I'm new to the list but I'm happy to help if I can.

I think the proposal to remove the keywords makes sense in the short
term as currently I agree it's misleading to users. The proposal
should be short, I'll finish it up and post it here to continue the
discussion. Thanks again for the feedback everyone.

···

On Mon, May 8, 2017 at 8:52 AM, Goffredo Marocchi via swift-evolution <swift-evolution@swift.org> wrote:

On 8 May 2017, at 08:44, Xiaodi Wu <xiaodi.wu@gmail.com> wrote:
On Mon, May 8, 2017 at 2:40 AM, Goffredo Marocchi via swift-evolution <swift-evolution@swift.org> wrote:


(David Hart) #18

LGTM! I'd go ahead with opening a PR. Time is running out!

···

On 8 May 2017, at 14:40, Greg Spiers <gspiers@gmail.com> wrote:

On Mon, May 8, 2017 at 7:57 AM, David Hart <david@hartbit.com> wrote:
Sounds great! It should be an easy one to get through,

Thanks David, appreciate it :slight_smile: I've created a draft of the proposal.
Any feedback would be very welcome. I wasn't sure if there was an
effect on ABI stability or API resilience. I don't think there would
be as this is only correcting syntax.

Also, just adding the link again where I had a try at implementing the
warning/error. Hopefully that can be helpful in getting feedback about
the proposal as well.
https://github.com/gspiers/swift/commit/ecde3ec5f61f259f8a396618e9973bac04536fd0

# Remove ownership keyword support in protocols

* Proposal: [SE-NNNN](NNNN-remove-ownership-keyword-support-in-protocols.md)
* Authors: [Greg Spiers](https://github.com/gspiers)
* Review Manager: TBD
* Status:
* Bug: [SR-479](https://bugs.swift.org/browse/SR-479)

## Introduction

This proposal removes support for the keywords `weak` and `unowned`
for property declarations in a protocol.

Swift-evolution thread: [Ownership on protocol property
requirements](https://lists.swift.org/pipermail/swift-evolution/Week-of-Mon-20170501/036495.html)
thread. (TODO: Add link to rest of discussion)

## Motivation

Currently it's possible to use the weak/unowned keywords for a
property requirement in a protocol. This can lead to confusion as
specifying one of these keywords does not enforce or raise any
warnings in the adopting type of that protocol:


class A {}

protocol P {
   weak var weakVar: A? { get set }
}

class B: P {
   var weakVar: A? // Not declared weak, no compiler warning/error
}

This can lead to unexpected and surprising behaviour from the point of
view of users. They keywords themselves are currently meaningless
inside of a protocol but look like they would have an effect when the
protocol is adopted.

## Proposed solution

Although the case could be made that the keywords should have meaning
in a protocol, as they are currently implemented today they don't have
an effect. This proposal aims to cleanup the misleading syntax and
isn't meant to remove functionality only correct to existing
behaviour.

This proposal suggests removing support for `weak` and `unowned` in a protocol.

## Detailed design

The compiler will flag the use of `weak` and `unowned` in a protocol
and suggest a fix to remove the keyword.

## Source compatibility

This is a source breaking change but one that would only correct code
that is already has broken assumptions. For existing use the compiler
will raise a compilation error. When running in Swift 3 mode a warning
can be generated instead of an error. It could be possible to address
source compatibility through source migration as well.

## Effect on ABI stability

This proposal does not affect ABI stability.

## Effect on API resilience

This proposal should not effect API resilience.

## Alternatives considered

There is an argument in making `weak` and `unowned` have meaning in a
protocol but this does open up other questions and is probably better
as a topic of a separate discussion/proposal. As this would be
additive it can be addressed at a later point when we have a clearer
understanding.


(Greg Spiers) #19

I've opened a pull request for the draft proposal. As others have said
it would be good to get this inconsistency fixed in time for Swift 4.
I'd be up for discussion around actually giving these keywords meaning
at some point, but there's lots of questions around that.

Thanks everyone for the feedback and help :slight_smile:

https://github.com/apple/swift-evolution/pull/707

···

On Mon, May 8, 2017 at 11:20 PM, David Hart <david@hartbit.com> wrote:

LGTM! I'd go ahead with opening a PR. Time is running out!

On 8 May 2017, at 14:40, Greg Spiers <gspiers@gmail.com> wrote:

On Mon, May 8, 2017 at 7:57 AM, David Hart <david@hartbit.com> wrote:
Sounds great! It should be an easy one to get through,

Thanks David, appreciate it :slight_smile: I've created a draft of the proposal.
Any feedback would be very welcome. I wasn't sure if there was an
effect on ABI stability or API resilience. I don't think there would
be as this is only correcting syntax.

Also, just adding the link again where I had a try at implementing the
warning/error. Hopefully that can be helpful in getting feedback about
the proposal as well.
https://github.com/gspiers/swift/commit/ecde3ec5f61f259f8a396618e9973bac04536fd0

# Remove ownership keyword support in protocols

* Proposal: [SE-NNNN](NNNN-remove-ownership-keyword-support-in-protocols.md)
* Authors: [Greg Spiers](https://github.com/gspiers)
* Review Manager: TBD
* Status:
* Bug: [SR-479](https://bugs.swift.org/browse/SR-479)

## Introduction

This proposal removes support for the keywords `weak` and `unowned`
for property declarations in a protocol.

Swift-evolution thread: [Ownership on protocol property
requirements](https://lists.swift.org/pipermail/swift-evolution/Week-of-Mon-20170501/036495.html)
thread. (TODO: Add link to rest of discussion)

## Motivation

Currently it's possible to use the weak/unowned keywords for a
property requirement in a protocol. This can lead to confusion as
specifying one of these keywords does not enforce or raise any
warnings in the adopting type of that protocol:


class A {}

protocol P {
   weak var weakVar: A? { get set }
}

class B: P {
   var weakVar: A? // Not declared weak, no compiler warning/error
}

This can lead to unexpected and surprising behaviour from the point of
view of users. They keywords themselves are currently meaningless
inside of a protocol but look like they would have an effect when the
protocol is adopted.

## Proposed solution

Although the case could be made that the keywords should have meaning
in a protocol, as they are currently implemented today they don't have
an effect. This proposal aims to cleanup the misleading syntax and
isn't meant to remove functionality only correct to existing
behaviour.

This proposal suggests removing support for `weak` and `unowned` in a protocol.

## Detailed design

The compiler will flag the use of `weak` and `unowned` in a protocol
and suggest a fix to remove the keyword.

## Source compatibility

This is a source breaking change but one that would only correct code
that is already has broken assumptions. For existing use the compiler
will raise a compilation error. When running in Swift 3 mode a warning
can be generated instead of an error. It could be possible to address
source compatibility through source migration as well.

## Effect on ABI stability

This proposal does not affect ABI stability.

## Effect on API resilience

This proposal should not effect API resilience.

## Alternatives considered

There is an argument in making `weak` and `unowned` have meaning in a
protocol but this does open up other questions and is probably better
as a topic of a separate discussion/proposal. As this would be
additive it can be addressed at a later point when we have a clearer
understanding.