[Review] SE-0189: Restrict Cross-module Struct Initializers

The review of "SE-0189: Restrict Cross-module Struct Initializers" begins now and runs through November 21, 2017.

The proposal is available here:

https://github.com/apple/swift-evolution/blob/master/proposals/0189-restrict-cross-module-struct-initializers.md
Reviews are an important part of the Swift evolution process. All review feedback should be sent to the swift-evolution mailing list at:

https://lists.swift.org/mailman/listinfo/swift-evolution
or, if you would like to keep your feedback private, directly to the review manager.

When replying, please try to keep the proposal link at the top of the message:

Proposal link: https://github.com/apple/swift-evolution/blob/master/proposals/0189-restrict-cross-module-struct-initializers.md
...
Reply text
...
Other replies
What goes into a review of a proposal?

The goal of the review process is to improve the proposal under review through constructive criticism and, eventually, determine the direction of Swift.

When reviewing a proposal, here are some questions to consider:

What is your evaluation of the proposal?

Is the problem being addressed significant enough to warrant a change to Swift?

Does this proposal fit well with the feel and direction of Swift?

If you have used other languages or libraries with a similar feature, how do you feel that this proposal compares to those?

How much effort did you put into your review? A glance, a quick reading, or an in-depth study?

Thanks,
Ted Kremenek
Review Manager

+1 It should have been like this from the get go.

···

On Nov 14, 2017, at 11:31 AM, Ted Kremenek <kremenek@apple.com> wrote:

The review of "SE-0189: Restrict Cross-module Struct Initializers" begins now and runs through November 21, 2017.

The proposal is available here:

https://github.com/apple/swift-evolution/blob/master/proposals/0189-restrict-cross-module-struct-initializers.md
Reviews are an important part of the Swift evolution process. All review feedback should be sent to the swift-evolution mailing list at:

https://lists.swift.org/mailman/listinfo/swift-evolution
or, if you would like to keep your feedback private, directly to the review manager.

When replying, please try to keep the proposal link at the top of the message:

Proposal link: https://github.com/apple/swift-evolution/blob/master/proposals/0189-restrict-cross-module-struct-initializers.md
...
Reply text
...
Other replies
What goes into a review of a proposal?

The goal of the review process is to improve the proposal under review through constructive criticism and, eventually, determine the direction of Swift.

When reviewing a proposal, here are some questions to consider:

What is your evaluation of the proposal?

Is the problem being addressed significant enough to warrant a change to Swift?

Does this proposal fit well with the feel and direction of Swift?

If you have used other languages or libraries with a similar feature, how do you feel that this proposal compares to those?

How much effort did you put into your review? A glance, a quick reading, or an in-depth study?

Thanks,
Ted Kremenek
Review Manager
_______________________________________________
swift-evolution-announce mailing list
swift-evolution-announce@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution-announce

Hi, everyone, proposal author here. Caveat: I'll be on vacation for the last few days of this review period, so I won't see your comments after Saturday the 18th (and possibly not even then, if you send them too late in the day). This is fine, of course, but if you have questions for me I may not be able to answer them until the following Monday, after the review has formally ended. Apologies if this ends up affecting you.

Thank you for your consideration and feedback!
Jordan

P.S. The implementation PR includes some changes to the Apple SDK overlays that demonstrate what's being proposed for imported C structs. I encourage you to take a look.

···

On Nov 14, 2017, at 11:31, Ted Kremenek <kremenek@apple.com> wrote:

The review of "SE-0189: Restrict Cross-module Struct Initializers" begins now and runs through November 21, 2017.

The proposal is available here:

https://github.com/apple/swift-evolution/blob/master/proposals/0189-restrict-cross-module-struct-initializers.md
Reviews are an important part of the Swift evolution process. All review feedback should be sent to the swift-evolution mailing list at:

https://lists.swift.org/mailman/listinfo/swift-evolution
or, if you would like to keep your feedback private, directly to the review manager.

When replying, please try to keep the proposal link at the top of the message:

Proposal link: https://github.com/apple/swift-evolution/blob/master/proposals/0189-restrict-cross-module-struct-initializers.md
...
Reply text
...
Other replies
What goes into a review of a proposal?

The goal of the review process is to improve the proposal under review through constructive criticism and, eventually, determine the direction of Swift.

When reviewing a proposal, here are some questions to consider:

What is your evaluation of the proposal?

Is the problem being addressed significant enough to warrant a change to Swift?

Does this proposal fit well with the feel and direction of Swift?

If you have used other languages or libraries with a similar feature, how do you feel that this proposal compares to those?

How much effort did you put into your review? A glance, a quick reading, or an in-depth study?

Thanks,
Ted Kremenek
Review Manager
_______________________________________________
swift-evolution-announce mailing list
swift-evolution-announce@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution-announce

I was initially quite confused about the proposal's first sentence: "Adding a property to a public struct in Swift ought to not be a source-breaking change.” Because I nearly always rely (like many developers) on struct automatic initializers, adding a property is pretty much always a source-breaking if I don’t write an explicit initializer with the same signature as the old automatic. Can something be done to clarify the proposal in that regard or is it too late?

David.

···

On 14 Nov 2017, at 20:31, Ted Kremenek via swift-evolution <swift-evolution@swift.org> wrote:

The review of "SE-0189: Restrict Cross-module Struct Initializers" begins now and runs through November 21, 2017.

The proposal is available here:

https://github.com/apple/swift-evolution/blob/master/proposals/0189-restrict-cross-module-struct-initializers.md
Reviews are an important part of the Swift evolution process. All review feedback should be sent to the swift-evolution mailing list at:

https://lists.swift.org/mailman/listinfo/swift-evolution
or, if you would like to keep your feedback private, directly to the review manager.

When replying, please try to keep the proposal link at the top of the message:

Proposal link: https://github.com/apple/swift-evolution/blob/master/proposals/0189-restrict-cross-module-struct-initializers.md
...
Reply text
...
Other replies
What goes into a review of a proposal?

The goal of the review process is to improve the proposal under review through constructive criticism and, eventually, determine the direction of Swift.

When reviewing a proposal, here are some questions to consider:

What is your evaluation of the proposal?

Is the problem being addressed significant enough to warrant a change to Swift?

Does this proposal fit well with the feel and direction of Swift?

If you have used other languages or libraries with a similar feature, how do you feel that this proposal compares to those?

How much effort did you put into your review? A glance, a quick reading, or an in-depth study?

Thanks,
Ted Kremenek
Review Manager
_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution

What is your evaluation of the proposal?

It makes total sense. It’s a hole to plug in the resilience story.

Is the problem being addressed significant enough to warrant a change to Swift?

Yes.

Does this proposal fit well with the feel and direction of Swift?

It feels very Swift because it protects users from unknowingly placing themselves at risk of future source-breakage.

If you have used other languages or libraries with a similar feature, how do you feel that this proposal compares to those?

Not aware of any.

How much effort did you put into your review? A glance, a quick reading, or an in-depth study?

Several reads and made sure I understood everything.

···

Thanks,
Ted Kremenek
Review Manager
_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution

Hi Jordan,

First off, this is clearly the model we should have had all along :wink: That said, I have a concern about source-compat and our ability to automatically migrate code impacted by this change.

Source compatibility

This makes existing code invalid in Swift 5, which is a source compatibility break.

It's not just a source compatibility break, it's a break that cannot necessarily be fixed if you don't control the module that vended the struct. If a library doesn't expose an appropriate initializer, there is no way for the client to invent one. Similarly, this isn't going to be very amenable to automatic migration, since
a) there may not be a memberwise initializer to use
b) even if there is, it may change the semantics to use it

There are two classes that we could theoretically migrate automatically:
1) C structs, since we know the initializer and its semantics
2) when we are migrating the code that defines the struct at the same time

The latter case might be tricky though, since it requires more global knowledge than we have in today's migrator.

Any thoughts? Do we have an idea how common this is or what kinds of places it comes up in most often (in a single codebase with multiple modules vs external dependencies vs C structs vs ....)?

Ben

···

On Nov 14, 2017, at 11:31 AM, Ted Kremenek <kremenek@apple.com> wrote:

The review of "SE-0189: Restrict Cross-module Struct Initializers" begins now and runs through November 21, 2017.

The proposal is available here:

https://github.com/apple/swift-evolution/blob/master/proposals/0189-restrict-cross-module-struct-initializers.md
Reviews are an important part of the Swift evolution process. All review feedback should be sent to the swift-evolution mailing list at:

https://lists.swift.org/mailman/listinfo/swift-evolution
or, if you would like to keep your feedback private, directly to the review manager.

When replying, please try to keep the proposal link at the top of the message:

Proposal link: https://github.com/apple/swift-evolution/blob/master/proposals/0189-restrict-cross-module-struct-initializers.md
...
Reply text
...
Other replies
What goes into a review of a proposal?

The goal of the review process is to improve the proposal under review through constructive criticism and, eventually, determine the direction of Swift.

When reviewing a proposal, here are some questions to consider:

What is your evaluation of the proposal?

Is the problem being addressed significant enough to warrant a change to Swift?

Does this proposal fit well with the feel and direction of Swift?

If you have used other languages or libraries with a similar feature, how do you feel that this proposal compares to those?

How much effort did you put into your review? A glance, a quick reading, or an in-depth study?

Thanks,
Ted Kremenek
Review Manager
_______________________________________________
swift-evolution-announce mailing list
swift-evolution-announce@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution-announce

I'm "weak oppose" on this proposal.

The core premise here is to increase the encapsulation of a struct around its member variables. But I think the purview of encapsulation is more classes than structs. e.g. a struct leaks information about the mutation of its member variables, even if those variables are private. Structs are the obvious implementation for a named tuple (CGPoint CGAffineTransform UIColor etc.) where there is inherently a fixed set of members that are more conveniently accessed directly. Structs and classes have different purposes and so the argument for consistency with classes is weak.

With regard to the BalancedPair problem, I would prefer to see a "final struct" or "required init".

···

On November 14, 2017 at 1:31:25 PM, Ted Kremenek (kremenek@apple.com) wrote:

The review of "SE-0189: Restrict Cross-module Struct Initializers" begins now and runs through November 21, 2017.

The proposal is available here:

https://github.com/apple/swift-evolution/blob/master/proposals/0189-restrict-cross-module-struct-initializers.md
Reviews are an important part of the Swift evolution process. All review feedback should be sent to the swift-evolution mailing list at:

https://lists.swift.org/mailman/listinfo/swift-evolution
or, if you would like to keep your feedback private, directly to the review manager.

When replying, please try to keep the proposal link at the top of the message:

Proposal link: https://github.com/apple/swift-evolution/blob/master/proposals/0189-restrict-cross-module-struct-initializers.md
...
Reply text
...
Other replies
What goes into a review of a proposal?

The goal of the review process is to improve the proposal under review through constructive criticism and, eventually, determine the direction of Swift.

When reviewing a proposal, here are some questions to consider:

What is your evaluation of the proposal?

Is the problem being addressed significant enough to warrant a change to Swift?

Does this proposal fit well with the feel and direction of Swift?

If you have used other languages or libraries with a similar feature, how do you feel that this proposal compares to those?

How much effort did you put into your review? A glance, a quick reading, or an in-depth study?

Thanks,
Ted Kremenek
Review Manager
_______________________________________________
swift-evolution-announce mailing list
swift-evolution-announce@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution-announce

Hi, David. This only affects cross-module use cases, which means that the automatically synthesized initializer doesn’t come into play (because it’s not public). Is the clarification you’re looking for something like “a 'source-breaking change’ is something that can cause previously compiling code in another module to result in compile-time errors”?

Thanks for pointing out the potential for confusion here!
Jordan

···

On Nov 14, 2017, at 12:55, David Hart <david@hartbit.com> wrote:

I was initially quite confused about the proposal's first sentence: "Adding a property to a public struct in Swift ought to not be a source-breaking change.” Because I nearly always rely (like many developers) on struct automatic initializers, adding a property is pretty much always a source-breaking if I don’t write an explicit initializer with the same signature as the old automatic. Can something be done to clarify the proposal in that regard or is it too late?

David.

On 14 Nov 2017, at 20:31, Ted Kremenek via swift-evolution <swift-evolution@swift.org <mailto:swift-evolution@swift.org>> wrote:

The review of "SE-0189: Restrict Cross-module Struct Initializers" begins now and runs through November 21, 2017.

The proposal is available here:

https://github.com/apple/swift-evolution/blob/master/proposals/0189-restrict-cross-module-struct-initializers.md
Reviews are an important part of the Swift evolution process. All review feedback should be sent to the swift-evolution mailing list at:

https://lists.swift.org/mailman/listinfo/swift-evolution
or, if you would like to keep your feedback private, directly to the review manager.

When replying, please try to keep the proposal link at the top of the message:

Proposal link: https://github.com/apple/swift-evolution/blob/master/proposals/0189-restrict-cross-module-struct-initializers.md
...
Reply text
...
Other replies
What goes into a review of a proposal?

The goal of the review process is to improve the proposal under review through constructive criticism and, eventually, determine the direction of Swift.

When reviewing a proposal, here are some questions to consider:

What is your evaluation of the proposal?

Is the problem being addressed significant enough to warrant a change to Swift?

Does this proposal fit well with the feel and direction of Swift?

If you have used other languages or libraries with a similar feature, how do you feel that this proposal compares to those?

How much effort did you put into your review? A glance, a quick reading, or an in-depth study?

Thanks,
Ted Kremenek
Review Manager
_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org <mailto:swift-evolution@swift.org>
https://lists.swift.org/mailman/listinfo/swift-evolution

Hi, David. This only affects cross-module use cases, which means that the automatically synthesized initializer doesn’t come into play (because it’s not public). Is the clarification you’re looking for something like “a 'source-breaking change’ is something that can cause previously compiling code in another module to result in compile-time errors”?

Oh, I wasn’t aware that the automatically synthesised initialiser wasn’t public. I didn’t even test it :-/

···

On 14 Nov 2017, at 22:24, Jordan Rose <jordan_rose@apple.com> wrote:

Thanks for pointing out the potential for confusion here!
Jordan

On Nov 14, 2017, at 12:55, David Hart <david@hartbit.com <mailto:david@hartbit.com>> wrote:

I was initially quite confused about the proposal's first sentence: "Adding a property to a public struct in Swift ought to not be a source-breaking change.” Because I nearly always rely (like many developers) on struct automatic initializers, adding a property is pretty much always a source-breaking if I don’t write an explicit initializer with the same signature as the old automatic. Can something be done to clarify the proposal in that regard or is it too late?

David.

On 14 Nov 2017, at 20:31, Ted Kremenek via swift-evolution <swift-evolution@swift.org <mailto:swift-evolution@swift.org>> wrote:

The review of "SE-0189: Restrict Cross-module Struct Initializers" begins now and runs through November 21, 2017.

The proposal is available here:

https://github.com/apple/swift-evolution/blob/master/proposals/0189-restrict-cross-module-struct-initializers.md
Reviews are an important part of the Swift evolution process. All review feedback should be sent to the swift-evolution mailing list at:

https://lists.swift.org/mailman/listinfo/swift-evolution
or, if you would like to keep your feedback private, directly to the review manager.

When replying, please try to keep the proposal link at the top of the message:

Proposal link: https://github.com/apple/swift-evolution/blob/master/proposals/0189-restrict-cross-module-struct-initializers.md
...
Reply text
...
Other replies
What goes into a review of a proposal?

The goal of the review process is to improve the proposal under review through constructive criticism and, eventually, determine the direction of Swift.

When reviewing a proposal, here are some questions to consider:

What is your evaluation of the proposal?

Is the problem being addressed significant enough to warrant a change to Swift?

Does this proposal fit well with the feel and direction of Swift?

If you have used other languages or libraries with a similar feature, how do you feel that this proposal compares to those?

How much effort did you put into your review? A glance, a quick reading, or an in-depth study?

Thanks,
Ted Kremenek
Review Manager
_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org <mailto:swift-evolution@swift.org>
https://lists.swift.org/mailman/listinfo/swift-evolution

The proposal is available here:

https://github.com/apple/swift-evolution/blob/master/
proposals/0189-restrict-cross-module-struct-initializers.md

···

-

   What is your evaluation of the proposal?

+1, more an oversight in the original design rather than a change. Funny
how this problem was caught for classes and not structs.

   -

   Is the problem being addressed significant enough to warrant a change to
   Swift?

Yes, focus is on getting rid of inconsistencies and problems.

   -

   Does this proposal fit well with the feel and direction of Swift?

Yes, Swift is meant to be consistent.

   -

   If you have used other languages or libraries with a similar feature,
   how do you feel that this proposal compares to those?

No

   -

   How much effort did you put into your review? A glance, a quick reading,
   or an in-depth study?

Glance

  -- Howard.

On 15 November 2017 at 08:24, Jordan Rose via swift-evolution < swift-evolution@swift.org> wrote:

Hi, David. This only affects *cross-module* use cases, which means that
the automatically synthesized initializer doesn’t come into play (because
it’s not public). Is the clarification you’re looking for something like “a
'source-breaking change’ is something that can cause previously compiling
code in another module to result in compile-time errors”?

Thanks for pointing out the potential for confusion here!
Jordan

On Nov 14, 2017, at 12:55, David Hart <david@hartbit.com> wrote:

I was initially quite confused about the proposal's first sentence:
"Adding a property to a public struct in Swift ought to not be a
source-breaking change.” Because I nearly always rely (like many
developers) on struct automatic initializers, adding a property is pretty
much always a source-breaking if I don’t write an explicit initializer with
the same signature as the old automatic. Can something be done to clarify
the proposal in that regard or is it too late?

David.

On 14 Nov 2017, at 20:31, Ted Kremenek via swift-evolution < > swift-evolution@swift.org> wrote:

The review of "SE-0189: Restrict Cross-module Struct Initializers" begins
now and runs through *November 21, 2017*.

The proposal is available here:

https://github.com/apple/swift-evolution/blob/master/
proposals/0189-restrict-cross-module-struct-initializers.md

Reviews are an important part of the Swift evolution process. All review
feedback should be sent to the swift-evolution mailing list at:

https://lists.swift.org/mailman/listinfo/swift-evolution

or, if you would like to keep your feedback private, directly to the
review manager.

When replying, please try to keep the proposal link at the top of the
message:

Proposal link: https://github.com/apple/swift-evolution/blob/
master/proposals/0189-restrict-cross-module-struct-initializers.md
...
Reply text
...
Other replies

What goes into a review of a proposal?

The goal of the review process is to improve the proposal under review
through constructive criticism and, eventually, determine the direction of
Swift.

When reviewing a proposal, here are some questions to consider:

   -

   What is your evaluation of the proposal?
   -

   Is the problem being addressed significant enough to warrant a change
   to Swift?
   -

   Does this proposal fit well with the feel and direction of Swift?
   -

   If you have used other languages or libraries with a similar feature,
   how do you feel that this proposal compares to those?
   -

   How much effort did you put into your review? A glance, a quick
   reading, or an in-depth study?

Thanks,
Ted Kremenek
Review Manager
_______________________________________________
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

This is good to bring up, but I think "this can't be migrated" is the correct answer for Swift structs. It's equivalent in my mind to when someone was passing 'nil' to something that wasn't annotated for nullability and now is marked '_Nonnull': it's source-breaking, and what you had before might even have worked, but there's no guarantee that it would keep working in the future. That's harder to sell for multi-module projects or even test targets, though. I don't have a great answer there, but I don't think it's worth bending over backwards to handle the "I migrate everything at once" case.

The C case is a bit harder to sell, which is why I made sure there were fix-its to suggest inserting `self.init()` (the zeroing initializer) in most cases (both in Swift 4 mode and Swift 5 mode). Not all C structs have that initializer (specifically, if they have a member marked _Nonnull), but nearly all do, and that's something the migrator could insert fairly easily, as you note.

The mitigating factor I'm hoping for is that these become warnings in Swift 4.1, which is planned to ship in a mid-year Xcode (can I admit that publicly, swift-evolution?), and that therefore many people will have cleaned up their code well before they even think of switching to Swift 5. But yes, this is a breaking, non-migratable language change that's only strictly necessary for libraries with binary compatibility, which most libraries today are not, and that has to be acknowledged.

Jordan

···

On Nov 17, 2017, at 15:20, Ben Langmuir <blangmuir@apple.com> wrote:

Hi Jordan,

First off, this is clearly the model we should have had all along :wink: That said, I have a concern about source-compat and our ability to automatically migrate code impacted by this change.

Source compatibility

This makes existing code invalid in Swift 5, which is a source compatibility break.

It's not just a source compatibility break, it's a break that cannot necessarily be fixed if you don't control the module that vended the struct. If a library doesn't expose an appropriate initializer, there is no way for the client to invent one. Similarly, this isn't going to be very amenable to automatic migration, since
a) there may not be a memberwise initializer to use
b) even if there is, it may change the semantics to use it

There are two classes that we could theoretically migrate automatically:
1) C structs, since we know the initializer and its semantics
2) when we are migrating the code that defines the struct at the same time

The latter case might be tricky though, since it requires more global knowledge than we have in today's migrator.

Any thoughts? Do we have an idea how common this is or what kinds of places it comes up in most often (in a single codebase with multiple modules vs external dependencies vs C structs vs ....)?

I'm "weak oppose" on this proposal.

The core premise here is to increase the encapsulation of a struct around its member variables. But I think the purview of encapsulation is more classes than structs.

How so? It seems that encapsulation is orthogonal to reference/value semantics.

e.g. a struct leaks information about the mutation of its member variables, even if those variables are private.

Can you explain what you mean by this?

Structs are the obvious implementation for a named tuple (CGPoint CGAffineTransform UIColor etc.) where there is inherently a fixed set of members that are more conveniently accessed directly. Structs and classes have different purposes and so the argument for consistency with classes is weak.

With regard to the BalancedPair problem, I would prefer to see a "final struct" or "required init”.

The real reason we want to introduce this language change is to solve some problems related to resilience. We want to be able to add new stored properties to structs, or change existing stored properties to computed properties (and vice versa) without breaking binary or source compatibility. Since non-delegating initializers expose the exact set of stored properties to the client module, they break the encapsulation that we need to allow this.

Slava

···

On Nov 20, 2017, at 1:58 PM, Drew Crawford via swift-evolution <swift-evolution@swift.org> wrote:
On November 14, 2017 at 1:31:25 PM, Ted Kremenek (kremenek@apple.com <mailto:kremenek@apple.com>) wrote:

The review of "SE-0189: Restrict Cross-module Struct Initializers" begins now and runs through November 21, 2017.

The proposal is available here:

https://github.com/apple/swift-evolution/blob/master/proposals/0189-restrict-cross-module-struct-initializers.md
Reviews are an important part of the Swift evolution process. All review feedback should be sent to the swift-evolution mailing list at:

https://lists.swift.org/mailman/listinfo/swift-evolution
or, if you would like to keep your feedback private, directly to the review manager.

When replying, please try to keep the proposal link at the top of the message:

Proposal link: https://github.com/apple/swift-evolution/blob/master/proposals/0189-restrict-cross-module-struct-initializers.md
...
Reply text
...
Other replies
What goes into a review of a proposal?

The goal of the review process is to improve the proposal under review through constructive criticism and, eventually, determine the direction of Swift.

When reviewing a proposal, here are some questions to consider:

What is your evaluation of the proposal?

Is the problem being addressed significant enough to warrant a change to Swift?

Does this proposal fit well with the feel and direction of Swift?

If you have used other languages or libraries with a similar feature, how do you feel that this proposal compares to those?

How much effort did you put into your review? A glance, a quick reading, or an in-depth study?

Thanks,
Ted Kremenek
Review Manager
_______________________________________________
swift-evolution-announce mailing list
swift-evolution-announce@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution-announce

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

Hi Jordan,

First off, this is clearly the model we should have had all along :wink: That said, I have a concern about source-compat and our ability to automatically migrate code impacted by this change.

Source compatibility

This makes existing code invalid in Swift 5, which is a source compatibility break.

It's not just a source compatibility break, it's a break that cannot necessarily be fixed if you don't control the module that vended the struct. If a library doesn't expose an appropriate initializer, there is no way for the client to invent one. Similarly, this isn't going to be very amenable to automatic migration, since
a) there may not be a memberwise initializer to use
b) even if there is, it may change the semantics to use it

There are two classes that we could theoretically migrate automatically:
1) C structs, since we know the initializer and its semantics
2) when we are migrating the code that defines the struct at the same time

The latter case might be tricky though, since it requires more global knowledge than we have in today's migrator.

Any thoughts? Do we have an idea how common this is or what kinds of places it comes up in most often (in a single codebase with multiple modules vs external dependencies vs C structs vs ....)?

This is good to bring up, but I think "this can't be migrated" is the correct answer for Swift structs. It's equivalent in my mind to when someone was passing 'nil' to something that wasn't annotated for nullability and now is marked '_Nonnull': it's source-breaking, and what you had before might even have worked, but there's no guarantee that it would keep working in the future. That's harder to sell for multi-module projects or even test targets, though. I don't have a great answer there, but I don't think it's worth bending over backwards to handle the "I migrate everything at once" case.

The C case is a bit harder to sell, which is why I made sure there were fix-its to suggest inserting `self.init()` (the zeroing initializer) in most cases (both in Swift 4 mode and Swift 5 mode). Not all C structs have that initializer (specifically, if they have a member marked _Nonnull), but nearly all do, and that's something the migrator could insert fairly easily, as you note.

The mitigating factor I'm hoping for is that these become warnings in Swift 4.1, which is planned to ship in a mid-year Xcode (can I admit that publicly, swift-evolution?), and that therefore many people will have cleaned up their code well before they even think of switching to Swift 5. But yes, this is a breaking, non-migratable language change that's only strictly necessary for libraries with binary compatibility, which most libraries today are not, and that has to be acknowledged.

The migrator could also detect special cases, for example:

- there is a public member wise initializer — instead of initializing the fields directly, it could suggest adding a call to that instead
- there is a public no-argument initializer — here it might be sufficient to insert a call to self.init() prior to initializing fields

Slava

···

On Nov 17, 2017, at 8:57 PM, Jordan Rose via swift-evolution <swift-evolution@swift.org> wrote:

On Nov 17, 2017, at 15:20, Ben Langmuir <blangmuir@apple.com <mailto:blangmuir@apple.com>> wrote:

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

How so? It seems that encapsulation is orthogonal to reference/value semantics.

Consider this type

public struct Mailbox {
private var _messages: [String]
public mutating func append(message: String) { _messages.append(message)}
public var messages: [String] { return _messages}
public var sortedMessages: [String] { return _messages.sorted() }
}

We subsequently discover in the wild that although there are a variety of client patterns, certain "important" clients have the particular pattern

1. Add messages up front
2. Use sortedMessages accessor frequently and exclusively

It would improve the important clients without harming other clients to use the following implementation for our type: if the sorted accessor is used before the unsorted one, sort the underlying storage, and then serve that until the type is subsequently mutated (which important clients do not do). This amortizes the cost of the sort across many calls to the accessor.

Although the underlying storage is "private" and we may consider its sorted-arity an implementation detail of Mailbox, in fact there is no straightforward way to get to that implementation from this one. When we chose value semantics we promised clients that our "encapsulated" values will not change inside a getter; mutating the "hidden" storage breaks our promise.

I would argue this "encapsulation failure" of the _messages array is actually a feature, and part of the draw of value types is that they provide these kinds of guarantees to clients, and we subvert them at our peril. In cases where that is not desired reference types are available.

We want to be able to add new stored properties to structs, or change existing stored properties to computed properties (and vice versa) without breaking binary or source compatibility.

If CGAffineTransform, CGPoint, in_addr_t etc. are changing their members we have a problem. There are probably structs where this makes more sense, but I can't immediately think of an example.

···

On November 20, 2017 at 6:16:55 PM, Slava Pestov (spestov@apple.com) wrote:

On Nov 20, 2017, at 1:58 PM, Drew Crawford via swift-evolution <swift-evolution@swift.org> wrote:

I'm "weak oppose" on this proposal.

The core premise here is to increase the encapsulation of a struct around its member variables. But I think the purview of encapsulation is more classes than structs.

How so? It seems that encapsulation is orthogonal to reference/value semantics.

e.g. a struct leaks information about the mutation of its member variables, even if those variables are private.

Can you explain what you mean by this?

Structs are the obvious implementation for a named tuple (CGPoint CGAffineTransform UIColor etc.) where there is inherently a fixed set of members that are more conveniently accessed directly. Structs and classes have different purposes and so the argument for consistency with classes is weak.

With regard to the BalancedPair problem, I would prefer to see a "final struct" or "required init”.

The real reason we want to introduce this language change is to solve some problems related to resilience. We want to be able to add new stored properties to structs, or change existing stored properties to computed properties (and vice versa) without breaking binary or source compatibility. Since non-delegating initializers expose the exact set of stored properties to the client module, they break the encapsulation that we need to allow this.

Slava
On November 14, 2017 at 1:31:25 PM, Ted Kremenek (kremenek@apple.com) wrote:

The review of "SE-0189: Restrict Cross-module Struct Initializers" begins now and runs through November 21, 2017.

The proposal is available here:

https://github.com/apple/swift-evolution/blob/master/proposals/0189-restrict-cross-module-struct-initializers.md
Reviews are an important part of the Swift evolution process. All review feedback should be sent to the swift-evolution mailing list at:

https://lists.swift.org/mailman/listinfo/swift-evolution
or, if you would like to keep your feedback private, directly to the review manager.

When replying, please try to keep the proposal link at the top of the message:

Proposal link: https://github.com/apple/swift-evolution/blob/master/proposals/0189-restrict-cross-module-struct-initializers.md
...
Reply text
...
Other replies
What goes into a review of a proposal?

The goal of the review process is to improve the proposal under review through constructive criticism and, eventually, determine the direction of Swift.

When reviewing a proposal, here are some questions to consider:

What is your evaluation of the proposal?

Is the problem being addressed significant enough to warrant a change to Swift?

Does this proposal fit well with the feel and direction of Swift?

If you have used other languages or libraries with a similar feature, how do you feel that this proposal compares to those?

How much effort did you put into your review? A glance, a quick reading, or an in-depth study?

Thanks,
Ted Kremenek
Review Manager
_______________________________________________
swift-evolution-announce mailing list
swift-evolution-announce@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution-announce
_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution

It would improve the important clients without harming other clients to use the following implementation for our type: if the sorted accessor is used before the unsorted one, sort the underlying storage, and then serve that until the type is subsequently mutated (which important clients do not do). This amortizes the cost of the sort across many calls to the accessor.

Sure, but it’s hard to completely encapsulate this kind of caching behavior inside the type. For example, thread safety. But yes, there are different tradeoffs with reference and value types, and it’s hard to argue that one is better for encapsulation than the other.

> We want to be able to add new stored properties to structs, or change existing stored properties to computed properties (and vice versa) without breaking binary or source compatibility.

If CGAffineTransform, CGPoint, in_addr_t etc. are changing their members we have a problem. There are probably structs where this makes more sense, but I can't immediately think of an example.

in_addr_t is actually mostly opaque so it’s actually not a good example.I agree that CGAffineTransform and such are trivial. But it’s easy to think of cases where you want to change the internal representation of a data type without changing it’s API, and sometimes this means stored properties become computed properties. The only place in the language where the difference between stored and computed properties can be observed is inside a non-delegating initializer of a struct, so it makes sense to prohibit them from being defined outside of the original module.

Slava

···

On Nov 20, 2017, at 11:14 PM, Drew Crawford <drew@sealedabstract.com> wrote:

On November 20, 2017 at 6:16:55 PM, Slava Pestov (spestov@apple.com <mailto:spestov@apple.com>) wrote:

On Nov 20, 2017, at 1:58 PM, Drew Crawford via swift-evolution <swift-evolution@swift.org <mailto:swift-evolution@swift.org>> wrote:

I'm "weak oppose" on this proposal.

The core premise here is to increase the encapsulation of a struct around its member variables. But I think the purview of encapsulation is more classes than structs.

How so? It seems that encapsulation is orthogonal to reference/value semantics.

e.g. a struct leaks information about the mutation of its member variables, even if those variables are private.

Can you explain what you mean by this?

Structs are the obvious implementation for a named tuple (CGPoint CGAffineTransform UIColor etc.) where there is inherently a fixed set of members that are more conveniently accessed directly. Structs and classes have different purposes and so the argument for consistency with classes is weak.

With regard to the BalancedPair problem, I would prefer to see a "final struct" or "required init”.

The real reason we want to introduce this language change is to solve some problems related to resilience. We want to be able to add new stored properties to structs, or change existing stored properties to computed properties (and vice versa) without breaking binary or source compatibility. Since non-delegating initializers expose the exact set of stored properties to the client module, they break the encapsulation that we need to allow this.

Slava

On November 14, 2017 at 1:31:25 PM, Ted Kremenek (kremenek@apple.com <mailto:kremenek@apple.com>) wrote:

The review of "SE-0189: Restrict Cross-module Struct Initializers" begins now and runs through November 21, 2017.

The proposal is available here:

https://github.com/apple/swift-evolution/blob/master/proposals/0189-restrict-cross-module-struct-initializers.md
Reviews are an important part of the Swift evolution process. All review feedback should be sent to the swift-evolution mailing list at:

https://lists.swift.org/mailman/listinfo/swift-evolution
or, if you would like to keep your feedback private, directly to the review manager.

When replying, please try to keep the proposal link at the top of the message:

Proposal link: https://github.com/apple/swift-evolution/blob/master/proposals/0189-restrict-cross-module-struct-initializers.md
...
Reply text
...
Other replies
What goes into a review of a proposal?

The goal of the review process is to improve the proposal under review through constructive criticism and, eventually, determine the direction of Swift.

When reviewing a proposal, here are some questions to consider:

What is your evaluation of the proposal?

Is the problem being addressed significant enough to warrant a change to Swift?

Does this proposal fit well with the feel and direction of Swift?

If you have used other languages or libraries with a similar feature, how do you feel that this proposal compares to those?

How much effort did you put into your review? A glance, a quick reading, or an in-depth study?

Thanks,
Ted Kremenek
Review Manager
_______________________________________________
swift-evolution-announce mailing list
swift-evolution-announce@swift.org <mailto:swift-evolution-announce@swift.org>
https://lists.swift.org/mailman/listinfo/swift-evolution-announce

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

Hi Jordan,

First off, this is clearly the model we should have had all along :wink: That said, I have a concern about source-compat and our ability to automatically migrate code impacted by this change.

Source compatibility

This makes existing code invalid in Swift 5, which is a source compatibility break.

It's not just a source compatibility break, it's a break that cannot necessarily be fixed if you don't control the module that vended the struct. If a library doesn't expose an appropriate initializer, there is no way for the client to invent one. Similarly, this isn't going to be very amenable to automatic migration, since
a) there may not be a memberwise initializer to use
b) even if there is, it may change the semantics to use it

There are two classes that we could theoretically migrate automatically:
1) C structs, since we know the initializer and its semantics
2) when we are migrating the code that defines the struct at the same time

The latter case might be tricky though, since it requires more global knowledge than we have in today's migrator.

Any thoughts? Do we have an idea how common this is or what kinds of places it comes up in most often (in a single codebase with multiple modules vs external dependencies vs C structs vs ....)?

This is good to bring up, but I think "this can't be migrated" is the correct answer for Swift structs. It's equivalent in my mind to when someone was passing 'nil' to something that wasn't annotated for nullability and now is marked '_Nonnull': it's source-breaking, and what you had before might even have worked, but there's no guarantee that it would keep working in the future. That's harder to sell for multi-module projects or even test targets, though. I don't have a great answer there, but I don't think it's worth bending over backwards to handle the "I migrate everything at once" case.

The C case is a bit harder to sell, which is why I made sure there were fix-its to suggest inserting `self.init()` (the zeroing initializer) in most cases (both in Swift 4 mode and Swift 5 mode). Not all C structs have that initializer (specifically, if they have a member marked _Nonnull), but nearly all do, and that's something the migrator could insert fairly easily, as you note.

The mitigating factor I'm hoping for is that these become warnings in Swift 4.1, which is planned to ship in a mid-year Xcode (can I admit that publicly, swift-evolution?), and that therefore many people will have cleaned up their code well before they even think of switching to Swift 5. But yes, this is a breaking, non-migratable language change that's only strictly necessary for libraries with binary compatibility, which most libraries today are not, and that has to be acknowledged.

The migrator could also detect special cases, for example:

- there is a public member wise initializer — instead of initializing the fields directly, it could suggest adding a call to that instead

Are you suggesting we do this even when we don't know the semantics of the init we would be calling (e.g. it might introduce new side effects)? I was thinking this would be unsafe to transform automatically, or at least we'd want to draw the developer's attention to it to verify it didn't change the behaviour.

- there is a public no-argument initializer — here it might be sufficient to insert a call to self.init() prior to initializing fields

Good point about being able to use the no-arg init, although with the same caveat as the previous case I think.

Ben

···

On Nov 17, 2017, at 5:59 PM, Slava Pestov <spestov@apple.com> wrote:

On Nov 17, 2017, at 8:57 PM, Jordan Rose via swift-evolution <swift-evolution@swift.org <mailto:swift-evolution@swift.org>> wrote:

On Nov 17, 2017, at 15:20, Ben Langmuir <blangmuir@apple.com <mailto:blangmuir@apple.com>> wrote:

Slava

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

Hi Jordan,

First off, this is clearly the model we should have had all along :wink: That said, I have a concern about source-compat and our ability to automatically migrate code impacted by this change.

Source compatibility

This makes existing code invalid in Swift 5, which is a source compatibility break.

It's not just a source compatibility break, it's a break that cannot necessarily be fixed if you don't control the module that vended the struct. If a library doesn't expose an appropriate initializer, there is no way for the client to invent one. Similarly, this isn't going to be very amenable to automatic migration, since
a) there may not be a memberwise initializer to use
b) even if there is, it may change the semantics to use it

There are two classes that we could theoretically migrate automatically:
1) C structs, since we know the initializer and its semantics
2) when we are migrating the code that defines the struct at the same time

The latter case might be tricky though, since it requires more global knowledge than we have in today's migrator.

Any thoughts? Do we have an idea how common this is or what kinds of places it comes up in most often (in a single codebase with multiple modules vs external dependencies vs C structs vs ....)?

This is good to bring up, but I think "this can't be migrated" is the correct answer for Swift structs. It's equivalent in my mind to when someone was passing 'nil' to something that wasn't annotated for nullability and now is marked '_Nonnull': it's source-breaking, and what you had before might even have worked, but there's no guarantee that it would keep working in the future. That's harder to sell for multi-module projects or even test targets, though. I don't have a great answer there, but I don't think it's worth bending over backwards to handle the "I migrate everything at once" case.

The C case is a bit harder to sell, which is why I made sure there were fix-its to suggest inserting `self.init()` (the zeroing initializer) in most cases (both in Swift 4 mode and Swift 5 mode). Not all C structs have that initializer (specifically, if they have a member marked _Nonnull), but nearly all do, and that's something the migrator could insert fairly easily, as you note.

The mitigating factor I'm hoping for is that these become warnings in Swift 4.1, which is planned to ship in a mid-year Xcode (can I admit that publicly, swift-evolution?), and that therefore many people will have cleaned up their code well before they even think of switching to Swift 5. But yes, this is a breaking, non-migratable language change that's only strictly necessary for libraries with binary compatibility, which most libraries today are not, and that has to be acknowledged.

The migrator could also detect special cases, for example:

- there is a public member wise initializer — instead of initializing the fields directly, it could suggest adding a call to that instead

Are you suggesting we do this even when we don't know the semantics of the init we would be calling (e.g. it might introduce new side effects)? I was thinking this would be unsafe to transform automatically, or at least we'd want to draw the developer's attention to it to verify it didn't change the behaviour.

- there is a public no-argument initializer — here it might be sufficient to insert a call to self.init() prior to initializing fields

Good point about being able to use the no-arg init, although with the same caveat as the previous case I think.

Yeah in general the amount of existing code that would be affected by this is an open question. This is why we wanted to stage the warning into 4.1 and see if people complain first.

Slava

···

On Nov 27, 2017, at 11:04 AM, Ben Langmuir <blangmuir@apple.com> wrote:

On Nov 17, 2017, at 5:59 PM, Slava Pestov <spestov@apple.com <mailto:spestov@apple.com>> wrote:

On Nov 17, 2017, at 8:57 PM, Jordan Rose via swift-evolution <swift-evolution@swift.org <mailto:swift-evolution@swift.org>> wrote:

On Nov 17, 2017, at 15:20, Ben Langmuir <blangmuir@apple.com <mailto:blangmuir@apple.com>> wrote:

Ben

Slava

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

+1 from me.

I have, all along, wished that we could use the designated/convenience
initializer patterns with structs. This moves us a little bit closer, at
least.

···

On Mon, Nov 27, 2017 at 11:05 AM, Slava Pestov via swift-evolution < swift-evolution@swift.org> wrote:

On Nov 27, 2017, at 11:04 AM, Ben Langmuir <blangmuir@apple.com> wrote:

On Nov 17, 2017, at 5:59 PM, Slava Pestov <spestov@apple.com> wrote:

On Nov 17, 2017, at 8:57 PM, Jordan Rose via swift-evolution < > swift-evolution@swift.org> wrote:

On Nov 17, 2017, at 15:20, Ben Langmuir <blangmuir@apple.com> wrote:

Hi Jordan,

First off, this is clearly the model we should have had all along :wink:
That said, I have a concern about source-compat and our ability to
automatically migrate code impacted by this change.

Source compatibility

This makes existing code invalid in Swift 5, which is a source
compatibility break.

It's not just a source compatibility break, it's a break that cannot
necessarily be fixed if you don't control the module that vended the
struct. If a library doesn't expose an appropriate initializer, there is
no way for the client to invent one. Similarly, this isn't going to be
very amenable to automatic migration, since
a) there may not be a memberwise initializer to use
b) even if there is, it may change the semantics to use it

There are two classes that we could theoretically migrate automatically:
1) C structs, since we know the initializer and its semantics
2) when we are migrating the code that defines the struct at the same time

The latter case might be tricky though, since it requires more global
knowledge than we have in today's migrator.

Any thoughts? Do we have an idea how common this is or what kinds of
places it comes up in most often (in a single codebase with multiple
modules vs external dependencies vs C structs vs ....)?

This is good to bring up, but I think "this can't be migrated" is the
correct answer for Swift structs. It's equivalent in my mind to when
someone was passing 'nil' to something that wasn't annotated for
nullability and now is marked '_Nonnull': it's source-breaking, and what
you had before might even have worked, but there's no guarantee that it
would keep working in the future. That's harder to sell for multi-module
projects or even test targets, though. I don't have a great answer there,
but I don't think it's worth bending over backwards to handle the "I
migrate everything at once" case.

The C case is a bit harder to sell, which is why I made sure there were
fix-its to suggest inserting `self.init()` (the zeroing initializer) in
most cases (both in Swift 4 mode and Swift 5 mode). Not all C structs
*have* that initializer (specifically, if they have a member marked
_Nonnull), but nearly all do, and that's something the migrator could
insert fairly easily, as you note.

The mitigating factor I'm hoping for is that these become warnings in
Swift 4.1, which is planned to ship in a mid-year Xcode (can I admit that
publicly, swift-evolution?), and that therefore many people will have
cleaned up their code well before they even think of switching to Swift 5.
But yes, this is a breaking, non-migratable language change that's only
strictly necessary for libraries with binary compatibility, which most
libraries today are not, and that has to be acknowledged.

The migrator could also detect special cases, for example:

- there is a public member wise initializer — instead of initializing the
fields directly, it could suggest adding a call to that instead

Are you suggesting we do this even when we don't know the semantics of the
init we would be calling (e.g. it might introduce new side effects)? I was
thinking this would be unsafe to transform automatically, or at least we'd
want to draw the developer's attention to it to verify it didn't change the
behaviour.

- there is a public no-argument initializer — here it might be sufficient
to insert a call to self.init() prior to initializing fields

Good point about being able to use the no-arg init, although with the same
caveat as the previous case I think.

Yeah in general the amount of existing code that would be affected by this
is an open question. This is why we wanted to stage the warning into 4.1
and see if people complain first.

Slava

Ben

Slava

Jordan_______________________________________________
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