Just getting around to this again. Thanks for putting a fair amount of thought 
in! I'm putting short section-by-section responses here, though the other 
discussion has already covered a fair amount. I'm planning to have a new 
revision of the proposal up tomorrow as well.


> On Sep 6, 2017, at 05:53, Brent Royal-Gordon <br...@architechies.com> wrote:
> 
>> On Sep 5, 2017, at 5:19 PM, Jordan Rose via swift-evolution 
>> <swift-evolution@swift.org <mailto:swift-evolution@swift.org>> wrote:
>> 
>> I've taken everyone's feedback into consideration and written this up as a 
>> proposal: 
>> https://github.com/jrose-apple/swift-evolution/blob/non-exhaustive-enums/proposals/nnnn-non-exhaustive-enums.md
>>  
>> <https://github.com/jrose-apple/swift-evolution/blob/non-exhaustive-enums/proposals/nnnn-non-exhaustive-enums.md>.
>>  The next step is working on an implementation, but if people have further 
>> pre-review comments I'd be happy to hear them.
> 
> I disagree with the choice of `exhaustive` and `nonexhaustive`. They are too 
> long; the more resilient keyword is longer than the more fragile one (and 
> difficult to read!); and they don't match the clang annotation. We may have 
> to compromise on one or two of these, but the combination of all three ought 
> to be considered disqualifying.
> 
> I think `final`/`nonfinal`, `total`/`partial`, `fixed`/? or `permanent`/? are 
> all better because they're shorter, although they all have problems with 
> their antonyms. `candid`/`coy` or `candid`/`shy` produce the right soft 
> default, but are kind of weirdly figurative.

I'm very happy for the review to end up settling on something else. It also 
sounds like the direction to go in will make 'nonexhaustive' the default in 
Swift 5 mode, so it at least wins on the second criterion in the long run.


> 
> But I don't think a change of keywords will fix everything here. 
> Fundamentally, I am not convinced that source compatibility of `switch` 
> statements should be weighed so heavily. Based on your survey of Foundation, 
> you suggest that the vast majority of imported enums should source-break all 
> switches in Swift 5. Why is that acceptable, but making Swift enums 
> source-breaking unacceptable?

Good point! I think it's acceptable because people were unlikely to be 
exhaustively switching over imported enums anyway, on the grounds that most of 
them aren't just non-exhaustive but actually don't make any sense to switch 
over in the first place. On the other hand, breaking Swift enums (when not 
migrated) is something that could cause pain for a while, rather than just 
being a one-time "go fix all the compiler issues". But you all are convincing 
me it's not as big a deal as I thought.


> 
> I suspect that, in practice, `public` enums tend to fall into two categories:
> 
>       1. "Data enums" which represent important data that happens to consist 
> of a set of alternatives. Outside users will frequently need to switch over 
> these, but they are not very likely to evolve or have private cases.
> 
>       2. "Mode enums" which tweak the behavior of an API. These are very 
> likely to evolve or have private cases, but outside users are not very likely 
> to need to switch over them.
> 
> An example of a data enum would be, as you mentioned, `NSComparisonResult`. 
> People really *do* need to be able to test against it, but barring some 
> fundamental break in the nature of reality, it will only ever have those 
> three cases. So it's fine to make it exhaustive.
> 
> An example of a mode enum would be `UIViewAnimationCurve`, which tells UIKit 
> how to ease an animation. I chose that example because I actually traced a 
> bug just last week to my mistaken impression that this enum had no private 
> cases. I was mapping values of this type to their corresponding 
> `UIViewAnimationOptions` values; because there were private cases, this was 
> Objective-C code, and I didn't include sufficiently aggressive assertions, I 
> ended up reading garbage data from memory. But while debugging this, it 
> struck me that this was actually *really weird* code. How often do you, as a 
> developer outside UIKit, need to interpret the value of a type like 
> `UIViewAnimationCurve`? If the compiler suddenly changed the exhaustiveness 
> behavior of `UIViewAnimationCurve`, probably less than 1% of apps would even 
> notice—and the affected code would probably have latent bugs!
> 
> Here's my point: Suddenly treating a mode enum as non-exhaustive is 
> *technically* source-breaking, but *people aren't doing things to them that 
> would break*. It is only the data enums that would actually experience source 
> breakage, and we both seem to agree those are relatively uncommon. So I would 
> argue the relatively rare source breaks are acceptable.

That's the hope! And still only when you're ready to migrate to Swift 5 mode.

> 
> Basically, what I would suggest is this:
> 
>       1. In Swift 4.1, we should add a permanent `exhaustive`* keyword and a 
> temporary `@nonexhaustive` attribute to Swift. These are no-ops, or maybe 
> `@nonexhaustive` simply silences the "unreachable default case" warning.
> 
>       2. In Swift 4.2 (or whatever Swift 5's Swift 4 mode is called), we 
> should warn about any enum which does not have either `exhaustive` or 
> `@nonexhaustive` attached to it, but publishes them as non-exhaustive. 
> `switch` requires a `default` case for any non-exhaustive public enum.
> 
>       3. Swift 5 in Swift 5 mode does the same thing, but does *not* warn 
> about the absence of `@nonexhaustive`.
> 
>       4. Swift 5 importing Objective-C treats enums as non-exhaustive by 
> default, unless marked with an attribute.
> 
> The dummy keywords in Swift 4.1 ensure that developers can write code that 
> works in both a true Swift 4 compiler and a Swift 5 compiler in Swift 4 mode. 
> (If we don't like that approach, though, we can bump the versions—give Swift 
> 4.2 the behavior I described for Swift 4, give Swift 5 the behavior I 
> described for 4.2, and plan to give Swift 6 the behavior I described for 
> Swift 5.)
> 
> * I'm still not super-happy with `exhaustive`, but since `@nonexhaustive` is 
> temporary in this scheme, that at least improves one of the complaints about 
> it. I think the keywords I discussed above would still be improvements.

I'm being a bit more casual and thinking that Swift 4 enums can stay 
exhaustive-by-default even in the Swift 4 mode of Swift 5. Joe Groff did 
suggest the warning even in that mode at one point, though. Similarly, switches 
over non-exhaustive enums declared in Swift 5 will produce only warnings in 
Swift 4 clients, not errors.


> 
>       * * *
> 
> But let's explore an entirely different design. This is a little bit loose; I 
> haven't thought it through totally rigorously.
> 
> `SKPaymentTransactionState`, which tells you the status of an in-app purchase 
> transaction, probably would have seemed like a data enum in iOS 3. After all, 
> what states could a transaction take besides `purchasing`, `purchased`, 
> `failed`, or `restored`? But in iOS 8, StoreKit introduced the `deferred` 
> state to handle a new parental-approval feature. Third-party developers did 
> not expect this and had to scramble to handle the unanticipated change.
> 
> The frameworks teams often solve this kind of issue by checking the linked 
> SDK version and falling back to compatible behavior in older versions. I 
> don't think StoreKit did this here, but it seems to me that they could have, 
> either by returning the `purchasing` state (which at worst would have stopped 
> users from doing anything else with the app until the purchase was approved 
> or declined) or by returning a `failed` state and then restoring the purchase 
> if it was later approved. At worst, if they had trapped when an incompatible 
> app had a purchase in the `deferred` state, developers might have fixed their 
> bugs more quickly.

For what it's worth, this particular solution is tricky when binary libraries 
come into play; the checks only look at when the main executable was linked. So 
it's not a panacea. (But you're not proposing that.)


> 
> I think we could imagine a similar solution being part of our resilience 
> system: Frameworks can add new cases to an enum, but they have to specify 
> compatibility behavior for old `switch` statements. Here's an example design:
> 
> A `public enum` may specify the `switch` keyword in its body. (I'm not 100% 
> happy with this keyword, but let's use it for now.) If it does, then the enum 
> is exhaustive:
> 
>       // A hypothetical pure-Swift version of `SKPaymentTransaction`.
>       @available(iOS 3.0)
>       public enum PaymentTransactionState {
>               case purchasing
>               case purchased(Purchase)
>               case restored(Purchase)
>               case error(Error)
>               
>               switch
>       }
> 
> If it later adds an additional case, or it has non-public cases, it must add 
> a block after the `switch` keyword. The block is called only if `self` is of 
> a case that the calling code doesn't know about; it must either return a 
> value that the caller *does* know about, or trap. So if we added `deferred`, 
> we might instead have:
> 
>       @available(iOS 3.0)
>       public enum PaymentTransactionState {
>               case purchasing
>               case purchased(Purchase)
>               case restored(Purchase)
>               case error(Error)
> 
>               @available(iOS 8.0)
>               case deferred
>               
>               switch {
>                       return .purchasing
>               }
>       }
> 
> (The same logic is applied to the value returned by the block, so if iOS 12 
> added another case, it could fall back to `deferred`, which would fall back 
> to `purchasing`.)
> 
> The `switch` keyword may be followed by a return type; public callers will 
> then need to write their `case` statements as though they were matching 
> against this type. So if, back in iOS 3, you had said this:
> 
>       @available(iOS 3.0)
>       public enum PaymentTransactionState {
>               case purchasing
>               case purchased(Purchase)
>               case restored(Purchase)
>               case error(Error)
>               
>               switch -> PaymentTransactionState?
>       }
> 
> Then every `switch` statement on a `PaymentTransactionState` would have had 
> to be written like:
> 
>       switch transaction.state {
>       case .purchasing?:
>               …
>       case .purchased?:
>               …
>       case .restored?:
>               …
>       case .error?:
>               …
>       case nil:
>               // Handle unexpected states
>       }
> 
> And then when you added a new case in iOS 8, you could say this, and 
> everyone's code would run through the `nil` path:
> 
>       @available(iOS 3.0)
>       public enum PaymentTransactionState {
>               case purchasing
>               case purchased(Purchase)
>               case restored(Purchase)
>               case error(Error)
> 
>               @available(iOS 8.0)
>               case deferred
>               
>               switch -> PaymentTransactionState? {
>                       return nil
>               }
>       }
> 
> An alternative design would have been to add a `case other` from the start, 
> anticipating that future versions would need to map unknown cases to that 
> one. (Or you could specify `switch -> Never` to forbid switching entirely, or 
> perhaps we could let you say `switch throws` to require the user to say `try 
> switch`. But you get the idea.)
> 
> Finally, the kicker: If you do *not* specify an `exhaustive` block, then it 
> is treated as though you had written `switch -> Self? { return nil }`. That 
> is, a "non-exhaustive" enum is just one which turns into an optional when you 
> switch over it, and returns `nil` for unknown cases. Thus, there basically 
> *are* no unknown cases.

Interesting. I think either Joe Groff or Slava Pestov has brought up ideas like 
this before, for mapping "new" cases to "older" ones. I suspect that the vast 
majority of enums fall into the "return nil" case, though, and then we're 
indeed roughly in the same position as the 'future' case alternative. I'll add 
this to the "alternatives considered" section, though.


> 
> Implementation-wise, I imagine that when switching over an enum from 
> `public`, you'd need to make a call which took a version parameter and 
> returned a value compatible with that version. (This might need to be some 
> sort of table of versions, depending on how we end up extending @available to 
> support versions for arbitrary modules.)

I'll note that this particular solution is tricky unless you version every 
build, but that's not really the main point here.


> 
>       * * *
> 
> As for the "untestable code path" problem…maybe we could let you mark certain 
> enum parameters as `@testable`, and then, when brought in through an 
> `@testable import`, allow a `#invalid` value to be passed to those parameters.
> 
>       // Library code
>       extension PurchasableItem {
>               func updateInventory(for state: @testable 
> PaymentTransactionState, quantity: Int) throws {
>                       switch state {
>                       case .purchasing:
>                               return
>                       case .purchased, .restored:
>                               inventory += quantity
>                       case .failed(let error):
>                               throw error
>                       default:
>                               throw ProductError.unknownTransactionState
>                       }
>               }
>       }
> 
>       // Test
>       func testUnknownTransactionState() {
>               XCTAssertThrowsError(myProduct.update(for: .#invalid) { error in
>                       XCTAssertEqual(error, 
> ProductError.unknownTransactionState)
>               }
>       }
> 
> An `@testable` value could not be passed to a non-`@testable` parameter or 
> into a non-`@testable` module, including the actual module the original type 
> came from, unless you had somehow excluded the possibility of an `#invalid` 
> value. You would need to design your code rather carefully to work around 
> this constraint, but I think it could be done.

I like this idea – it's basically sugar for Optional, but only used in one 
specific place. It gets a little tricky if layout matters—Optional<AnyObject> 
fits exactly in a single word-sized value, but Optional<Optional<AnyObject>> 
does not on Apple platforms—but that just means it should be opt-in or tied to 
-enable-testing in some way.

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

Reply via email to