[Pitch] Remove "Default will never be executed" Warning?


(Robert Widmann) #1

Hello all,

I’ve seen and been a part of a number of conversations recently where talk of planning for “resilient enums”, or even just authors that ship frameworks that will eventually offer a binary option that export enum-based APIs. The consensus seems to be that the only safe way to deal with this situation is to always cover a `switch` statement with a default, regardless of whether the totality checker thinks you’ve covered all cases. Currently, the compiler warns when this is the case, as in stdlib

extension ProcessTerminationStatus {
  var isSwiftTrap: Bool {
    switch self {
    case .exit(_):
      return false
    case .signal(let signal):
      return CInt(signal) == SIGILL || CInt(signal) == SIGTRAP
    default:
      // This default case is needed for standard library builds where
      // resilience is enabled
      return false
    }
  }
}

I think this warning doesn’t actually serve a purpose and I’d like to remove it, or at the very least curb it. Currently, I see three paths forward:

1) Do nothing.
2) Completely remove the diagnostic
3) Only emit the diagnostic when the case-tree is switching over enum(s) declared inside the current module.

I’ve filed SR-3278 <https://bugs.swift.org/browse/SR-3278> to track this as well.

Thanks,

~Robert Widmann


(David Sweeris) #2

Has the “resilient enum” thing been finalized, or is this kind of code just a preventative measure?

Also, if “resilience” is a compiler flag, there's a fourth option: Disable the warning on builds that have “resilience” enabled.

- Dave Sweeris

···

On Nov 26, 2016, at 5:25 PM, Robert Widmann via swift-dev <swift-dev@swift.org> wrote:

Hello all,

I’ve seen and been a part of a number of conversations recently where talk of planning for “resilient enums”, or even just authors that ship frameworks that will eventually offer a binary option that export enum-based APIs. The consensus seems to be that the only safe way to deal with this situation is to always cover a `switch` statement with a default, regardless of whether the totality checker thinks you’ve covered all cases. Currently, the compiler warns when this is the case, as in stdlib

extension ProcessTerminationStatus {
  var isSwiftTrap: Bool {
    switch self {
    case .exit(_):
      return false
    case .signal(let signal):
      return CInt(signal) == SIGILL || CInt(signal) == SIGTRAP
    default:
      // This default case is needed for standard library builds where
      // resilience is enabled
      return false
    }
  }
}

I think this warning doesn’t actually serve a purpose and I’d like to remove it, or at the very least curb it. Currently, I see three paths forward:

1) Do nothing.
2) Completely remove the diagnostic
3) Only emit the diagnostic when the case-tree is switching over enum(s) declared inside the current module.


(Slava Pestov) #3

Hello all,

I’ve seen and been a part of a number of conversations recently where talk of planning for “resilient enums”, or even just authors that ship frameworks that will eventually offer a binary option that export enum-based APIs. The consensus seems to be that the only safe way to deal with this situation is to always cover a `switch` statement with a default, regardless of whether the totality checker thinks you’ve covered all cases.

This is not quite right. The totality checker only kicks in if the module containing the enum is not built with resilient interfaces. When built with resilience enabled (you can pass —swift-stdlib-enable-resilience=1 to build-script), the warning is gone, and the default case is required; without it, you get an error about a non-exhaustive switch.

Currently, the compiler warns when this is the case, as in stdlib

extension ProcessTerminationStatus {
  var isSwiftTrap: Bool {
    switch self {
    case .exit(_):
      return false
    case .signal(let signal):
      return CInt(signal) == SIGILL || CInt(signal) == SIGTRAP
    default:
      // This default case is needed for standard library builds where
      // resilience is enabled
      return false
    }
  }
}

I think this warning doesn’t actually serve a purpose and I’d like to remove it, or at the very least curb it. Currently, I see three paths forward:

1) Do nothing.
2) Completely remove the diagnostic
3) Only emit the diagnostic when the case-tree is switching over enum(s) declared inside the current module.

One workaround would be to define ProcessTerminationStatus as @_fixed_layout, but that attribute is not documented right now. The eventual goal is that the attribute will be renamed to @closed for enums, and the warning will be emitted even if the module was built non-resiliently.

However there’s a chance that at some point we will also be building the stdlib with resilience enabled by default, but I can’t comment on what the plans are in this area yet.

···

On Nov 26, 2016, at 3:25 PM, Robert Widmann via swift-dev <swift-dev@swift.org> wrote:

I’ve filed SR-3278 <https://bugs.swift.org/browse/SR-3278> to track this as well.

Thanks,

~Robert Widmann
_______________________________________________
swift-dev mailing list
swift-dev@swift.org
https://lists.swift.org/mailman/listinfo/swift-dev


(Slava Pestov) #4

Hello all,

I’ve seen and been a part of a number of conversations recently where talk of planning for “resilient enums”, or even just authors that ship frameworks that will eventually offer a binary option that export enum-based APIs. The consensus seems to be that the only safe way to deal with this situation is to always cover a `switch` statement with a default, regardless of whether the totality checker thinks you’ve covered all cases. Currently, the compiler warns when this is the case, as in stdlib

extension ProcessTerminationStatus {
  var isSwiftTrap: Bool {
    switch self {
    case .exit(_):
      return false
    case .signal(let signal):
      return CInt(signal) == SIGILL || CInt(signal) == SIGTRAP
    default:
      // This default case is needed for standard library builds where
      // resilience is enabled
      return false
    }
  }
}

I think this warning doesn’t actually serve a purpose and I’d like to remove it, or at the very least curb it. Currently, I see three paths forward:

1) Do nothing.
2) Completely remove the diagnostic
3) Only emit the diagnostic when the case-tree is switching over enum(s) declared inside the current module.

Has the “resilient enum” thing been finalized, or is this kind of code just a preventative measure?

Also, if “resilience” is a compiler flag, there's a fourth option: Disable the warning on builds that have “resilience” enabled.

The warning is already gone (and the default cause is required) when the standard library and overlays are built with resilience enabled. The issue is that resilience is off by default.

I agree the warning is annoying; we should figure out something.

···

On Nov 27, 2016, at 4:32 PM, David Sweeris via swift-dev <swift-dev@swift.org> wrote:

On Nov 26, 2016, at 5:25 PM, Robert Widmann via swift-dev <swift-dev@swift.org <mailto:swift-dev@swift.org>> wrote:

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


(Jordan Rose) #5

This issue isn't really specific to resilience. If you depend on some third-party library through the Swift package manager, that library should be free to add new cases to enums in new releases without that being a source-breaking change. On the other hand, maybe you do want to know when new cases are added, so you can go update your switch statements as the author of the client.

There hasn't been any formal proposal for open vs. closed enums yet. Today all Swift enums are treated as closed, while imported C enums are sometimes treated as closed (switch) and sometimes as open (init(rawValue:) always succeeding). The Library Evolution document <http://jrose-apple.github.io/swift-library-evolution/#enums> describes how we want enums to behave in a resilient library, but doesn't discuss non-resilient source packages.

I'd lean towards (3) as the right answer for now, but it would also be great to start the ball rolling on designing a "closed" annotation for enums that makes sense for both source and binary distribution.

Jordan

···

On Nov 27, 2016, at 16:32, David Sweeris via swift-dev <swift-dev@swift.org> wrote:

On Nov 26, 2016, at 5:25 PM, Robert Widmann via swift-dev <swift-dev@swift.org <mailto:swift-dev@swift.org>> wrote:

Hello all,

I’ve seen and been a part of a number of conversations recently where talk of planning for “resilient enums”, or even just authors that ship frameworks that will eventually offer a binary option that export enum-based APIs. The consensus seems to be that the only safe way to deal with this situation is to always cover a `switch` statement with a default, regardless of whether the totality checker thinks you’ve covered all cases. Currently, the compiler warns when this is the case, as in stdlib

extension ProcessTerminationStatus {
  var isSwiftTrap: Bool {
    switch self {
    case .exit(_):
      return false
    case .signal(let signal):
      return CInt(signal) == SIGILL || CInt(signal) == SIGTRAP
    default:
      // This default case is needed for standard library builds where
      // resilience is enabled
      return false
    }
  }
}

I think this warning doesn’t actually serve a purpose and I’d like to remove it, or at the very least curb it. Currently, I see three paths forward:

1) Do nothing.
2) Completely remove the diagnostic
3) Only emit the diagnostic when the case-tree is switching over enum(s) declared inside the current module.

Has the “resilient enum” thing been finalized, or is this kind of code just a preventative measure?

Also, if “resilience” is a compiler flag, there's a fourth option: Disable the warning on builds that have “resilience” enabled.


(Dave Abrahams) #6

There should also be a way to write code that will compile without
warnings whether or not you have resilience enabled. :wink:

···

on Sun Nov 27 2016, Slava Pestov <swift-dev-AT-swift.org> wrote:

On Nov 27, 2016, at 4:32 PM, David Sweeris via swift-dev <swift-dev@swift.org> wrote:

On Nov 26, 2016, at 5:25 PM, Robert Widmann via swift-dev <swift-dev@swift.org > <mailto:swift-dev@swift.org>> wrote:

Hello all,

I’ve seen and been a part of a number of conversations recently where talk of planning for “resilient enums”, or even just authors that ship frameworks that will eventually offer a binary option that export enum-based APIs. The consensus seems to be that the only safe way to deal with this situation is to always cover a `switch` statement with a default, regardless of whether the totality checker thinks you’ve covered all cases. Currently, the compiler warns when this is the case, as in stdlib

extension ProcessTerminationStatus {
  var isSwiftTrap: Bool {
    switch self {
    case .exit(_):
      return false
    case .signal(let signal):
      return CInt(signal) == SIGILL || CInt(signal) == SIGTRAP
    default:
      // This default case is needed for standard library builds where
      // resilience is enabled
      return false
    }
  }
}

I think this warning doesn’t actually serve a purpose and I’d like to remove it, or at the very

least curb it. Currently, I see three paths forward:

1) Do nothing.
2) Completely remove the diagnostic
3) Only emit the diagnostic when the case-tree is switching over enum(s) declared inside the

current module.

Has the “resilient enum” thing been finalized, or is this kind of code just a preventative

measure?

Also, if “resilience” is a compiler flag, there's a fourth option: Disable the warning on builds

that have “resilience” enabled.

The warning is already gone (and the default cause is required) when
the standard library and overlays are built with resilience
enabled. The issue is that resilience is off by default.

I agree the warning is annoying; we should figure out something.

--
-Dave


(Alexis) #7

Hello all,

I’ve seen and been a part of a number of conversations recently where talk of planning for “resilient enums”, or even just authors that ship frameworks that will eventually offer a binary option that export enum-based APIs. The consensus seems to be that the only safe way to deal with this situation is to always cover a `switch` statement with a default, regardless of whether the totality checker thinks you’ve covered all cases. Currently, the compiler warns when this is the case, as in stdlib

extension ProcessTerminationStatus {
  var isSwiftTrap: Bool {
    switch self {
    case .exit(_):
      return false
    case .signal(let signal):
      return CInt(signal) == SIGILL || CInt(signal) == SIGTRAP
    default:
      // This default case is needed for standard library builds where
      // resilience is enabled
      return false
    }
  }
}

I think this warning doesn’t actually serve a purpose and I’d like to remove it, or at the very least curb it. Currently, I see three paths forward:

1) Do nothing.
2) Completely remove the diagnostic
3) Only emit the diagnostic when the case-tree is switching over enum(s) declared inside the current module.

Has the “resilient enum” thing been finalized, or is this kind of code just a preventative measure?

Also, if “resilience” is a compiler flag, there's a fourth option: Disable the warning on builds that have “resilience” enabled.

This issue isn't really specific to resilience. If you depend on some third-party library through the Swift package manager, that library should be free to add new cases to enums in new releases without that being a source-breaking change. On the other hand, maybe you do want to know when new cases are added, so you can go update your switch statements as the author of the client.

There hasn't been any formal proposal for open vs. closed enums yet. Today all Swift enums are treated as closed, while imported C enums are sometimes treated as closed (switch) and sometimes as open (init(rawValue:) always succeeding). The Library Evolution document <http://jrose-apple.github.io/swift-library-evolution/#enums> describes how we want enums to behave in a resilient library, but doesn't discuss non-resilient source packages.

I'd lean towards (3) as the right answer for now, but it would also be great to start the ball rolling on designing a "closed" annotation for enums that makes sense for both source and binary distribution.

Jordan

Idle thought: this discussion makes me think we might want a tristate for the different behaviours inside and outside the resilience domain:

* closed: inside and outside are exhaustive; always warn on dead defaults
* open: inside and outside are inexhaustive; never warn on dead defaults
* “default”: inside is exhaustive, outside is inexhaustive; warn inside, but not outside
(the hypothetical 4th state seems incoherent to me)

Basically the default behaviour is that I know what my enum is and would like the safety benefits of exhaustive matching, but I don’t want the evolution hazard of letting others exhaustively match. However I may also want the ability to be able to make certain enums act in-exhaustive even in my own code. This may be too niche to care about, though. Certainly this mode can be added later in a backwards compatible way.

···

On Nov 30, 2016, at 5:02 PM, Jordan Rose via swift-dev <swift-dev@swift.org> wrote:

On Nov 27, 2016, at 16:32, David Sweeris via swift-dev <swift-dev@swift.org <mailto:swift-dev@swift.org>> wrote:

On Nov 26, 2016, at 5:25 PM, Robert Widmann via swift-dev <swift-dev@swift.org <mailto:swift-dev@swift.org>> wrote:

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


(Zhao Xin) #8

​It will be better to use `fatalError()` in `default` part that you think
never be needed.​

default:
fatalError("This should never happen!")

Zhaoxin

···

On Tue, Nov 29, 2016 at 11:25 AM, Dave Abrahams via swift-dev < swift-dev@swift.org> wrote:

on Sun Nov 27 2016, Slava Pestov <swift-dev-AT-swift.org> wrote:

>> On Nov 27, 2016, at 4:32 PM, David Sweeris via swift-dev < > swift-dev@swift.org> wrote:
>>
>>>
>>> On Nov 26, 2016, at 5:25 PM, Robert Widmann via swift-dev < > swift-dev@swift.org > > <mailto:swift-dev@swift.org>> wrote:
>>>
>
>>> Hello all,
>>>
>>> I’ve seen and been a part of a number of conversations recently where
talk of planning for “resilient enums”, or even just authors that ship
frameworks that will eventually offer a binary option that export
enum-based APIs. The consensus seems to be that the only safe way to deal
with this situation is to always cover a `switch` statement with a default,
regardless of whether the totality checker thinks you’ve covered all
cases. Currently, the compiler warns when this is the case, as in stdlib
>>>
>>> extension ProcessTerminationStatus {
>>> var isSwiftTrap: Bool {
>>> switch self {
>>> case .exit(_):
>>> return false
>>> case .signal(let signal):
>>> return CInt(signal) == SIGILL || CInt(signal) == SIGTRAP
>>> default:
>>> // This default case is needed for standard library builds where
>>> // resilience is enabled
>>> return false
>>> }
>>> }
>>> }
>>>
>>> I think this warning doesn’t actually serve a purpose and I’d like to
remove it, or at the very
> least curb it. Currently, I see three paths forward:
>>>
>>> 1) Do nothing.
>>> 2) Completely remove the diagnostic
>>> 3) Only emit the diagnostic when the case-tree is switching over
enum(s) declared inside the
> current module.
>>
>> Has the “resilient enum” thing been finalized, or is this kind of code
just a preventative
> measure?
>>
>> Also, if “resilience” is a compiler flag, there's a fourth option:
Disable the warning on builds
> that have “resilience” enabled.
>
> The warning is already gone (and the default cause is required) when
> the standard library and overlays are built with resilience
> enabled. The issue is that resilience is off by default.
>
> I agree the warning is annoying; we should figure out something.

There should also be a way to write code that will compile without
warnings whether or not you have resilience enabled. :wink:

--
-Dave

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


(Alexis) #9

I wouldn’t recommend this in general. This will just make resiliently linked programs incredibly unstable. There’s often something better that can be done, but it requires thinking about your particular case and the particular enum.

* ignore what you don’t understand (or care about)
* map it to a less good but ultimately correct generic handler
* throw
* forward it
* call a method on the enum itself to handle it

Ultimately it’s much the same as the default case for a throw.

···

On Nov 29, 2016, at 2:25 AM, Zhao Xin via swift-dev <swift-dev@swift.org> wrote:

​It will be better to use `fatalError()` in `default` part that you think never be needed.​

default:
fatalError("This should never happen!")

Zhaoxin

On Tue, Nov 29, 2016 at 11:25 AM, Dave Abrahams via swift-dev <swift-dev@swift.org <mailto:swift-dev@swift.org>> wrote:

on Sun Nov 27 2016, Slava Pestov <swift-dev-AT-swift.org> wrote:

>> On Nov 27, 2016, at 4:32 PM, David Sweeris via swift-dev <swift-dev@swift.org <mailto:swift-dev@swift.org>> wrote:
>>
>>>
>>> On Nov 26, 2016, at 5:25 PM, Robert Widmann via swift-dev <swift-dev@swift.org <mailto:swift-dev@swift.org> > > <mailto:swift-dev@swift.org <mailto:swift-dev@swift.org>>> wrote:
>>>
>
>>> Hello all,
>>>
>>> I’ve seen and been a part of a number of conversations recently where talk of planning for “resilient enums”, or even just authors that ship frameworks that will eventually offer a binary option that export enum-based APIs. The consensus seems to be that the only safe way to deal with this situation is to always cover a `switch` statement with a default, regardless of whether the totality checker thinks you’ve covered all cases. Currently, the compiler warns when this is the case, as in stdlib
>>>
>>> extension ProcessTerminationStatus {
>>> var isSwiftTrap: Bool {
>>> switch self {
>>> case .exit(_):
>>> return false
>>> case .signal(let signal):
>>> return CInt(signal) == SIGILL || CInt(signal) == SIGTRAP
>>> default:
>>> // This default case is needed for standard library builds where
>>> // resilience is enabled
>>> return false
>>> }
>>> }
>>> }
>>>
>>> I think this warning doesn’t actually serve a purpose and I’d like to remove it, or at the very
> least curb it. Currently, I see three paths forward:
>>>
>>> 1) Do nothing.
>>> 2) Completely remove the diagnostic
>>> 3) Only emit the diagnostic when the case-tree is switching over enum(s) declared inside the
> current module.
>>
>> Has the “resilient enum” thing been finalized, or is this kind of code just a preventative
> measure?
>>
>> Also, if “resilience” is a compiler flag, there's a fourth option: Disable the warning on builds
> that have “resilience” enabled.
>
> The warning is already gone (and the default cause is required) when
> the standard library and overlays are built with resilience
> enabled. The issue is that resilience is off by default.
>
> I agree the warning is annoying; we should figure out something.

There should also be a way to write code that will compile without
warnings whether or not you have resilience enabled. :wink:

--
-Dave

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

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