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

[Slava Pestov]

> On Nov 27, 2017, at 11:04 AM, Ben Langmuir <blangmuir at apple.com> wrote:
> 
> 
> 
>> On Nov 17, 2017, at 5:59 PM, Slava Pestov <spestov at apple.com <mailto:spestov at apple.com>> wrote:
>> 
>> 
>> 
>>> On Nov 17, 2017, at 8:57 PM, Jordan Rose via swift-evolution <swift-evolution at swift.org <mailto:swift-evolution at swift.org>> wrote:
>>> 
>>> 
>>> 
>>>> On Nov 17, 2017, at 15:20, Ben Langmuir <blangmuir at apple.com <mailto:blangmuir at apple.com>> wrote:
>>>> 
>>>> Hi Jordan,
>>>> 
>>>> First off, this is clearly the model we should have had all along ;-)  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 at swift.org <mailto:swift-evolution at swift.org>
>>> https://lists.swift.org/mailman/listinfo/swift-evolution <https://lists.swift.org/mailman/listinfo/swift-evolution>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.swift.org/pipermail/swift-evolution/attachments/20171127/ae92855f/attachment.html>