Introducing Namespacing for Common Swift Error Scenarios

Hey @davedelong, what do you think about just using Never as the core type and providing a public typealias of Fatal = Never. Sure people will then be free to use Never.unreachable but then again people are also free to lick Tide pods. Neither is a good idea.

This could let you build off an existing type, with the proper semantics, but avoid the unfortunate Never core name that seems to rebut its static methods. This way you still get Fatal.unreachable("reason") , etc., with a minimum of stdlib turbulence.

This seems to be going down the route that some would call DBC (Design By Contract) coined by Eiffel language. I previously worked at a company that had its own implementation of a DBC framework in Swift to define those contracts by using similar fatalError messages in Debug builds. But some would not crash when built in release. There seems like there are too many cases where th behavior of functions like these would need to be specific to a teams needs and would be a better fit as a dependent library instead. Examples of how we defined them were require, check, and ensure Require being the only one mentioned in this pitch

If a required method was never implemented, shouldn't it still crash in a release build? Each of these Fatal conditions refers to a specific situation that should never be reached. Or am I misunderstanding you?

I'd like to hear more about the ways Swift could adopt design-by-contract features though.

I think that’s exactly what I’m trying to get at is that the semantics of what these should look like is up to the team deciding to adopt them. They might want them to crash in any build type and another team might want some to crash in Debug but not release.

We've finally gotten around to putting together a proposal for this, which can be seen here (PR): [Proposal] Introducing Namespacing for Common Swift Error Scenarios by erica · Pull Request #861 · apple/swift-evolution · GitHub

The main difference is that we no longer have a top-level Fatal type, but instead have added an overload of fatalError() that allows you to pass in some pre-defined reasons.

4 Likes

do we really want the mustOverride case though? I understand a lot of existing code still follows the pure virtual function/abstract base class paradigm but I thought this was a pattern that Swift discourages. It’s fine and good for the language to support such styles but it shouldn’t endorse them in the standard library.

1 Like

It's not difficult to eliminate that member in the standard library and include it in a more suitable extension, for example in Swift Foundation or anywhere else that touches on Cocoa.

1 Like

I had the same thought but realised dynamic dispatch could allow people to invoke placeholder implementations by mistake.

.notImplemented doesn't seem like good practice. Especially when one has #warning?


Does fatalError(because: FatalReason("abc")) not overlap with fatalError("abc"), and have y'all tried possibly implementing your solution in a way that's an extension of the current fatalError function? I haven't put much thought into this, but possibly just add a namespace with the error strings?

enum FatalReason {
  static let mustOverride = "must override"
  ...
}

Then calls could be:

fatalError(FatalReason.mustOverride)

That doesn't work for class hierarchies, only for protocols and direct implementations of protocols.

I like the minimalistic new design but I don't like the because label. To me as a non-native English speaker it reads really strange and somehow inconsistent to the other labels:

fatalError(because:function:file:line:)

I would rather see something like this:

fatalError(reason:function:file:line:)
1 Like

Yes, we want this. It's a common enough pattern when building apps that making everyone who wants this implement it themselves is silly.

Or as @Soroush_Khanlou would say, "we deserve nice things".

We address this in the document:

In other words, sometimes a build warning about unimplemented code is insufficient or inappropriate for the purpose. For definite examples, look no further than NSUnimplemented() scattered all over swift-corelibs-foundation.

As for the suggestion about making FatalReason an enum instead of a struct... functionally an enum backed by String would be equivalent to a struct with a single String field. However, the semantics are different. It is far easier to add new "cases" to a struct than to an enum. An enum implies exhaustiveness, which would incorrectly lead people to believe that you can't add new cases. A struct has no such implication.

1 Like

Let me push this even further. The design specifically allows in-house extensions of the type with new static members, allowing you to create custom fatal scenarios beyond the four we have enumerated as "universal". There is an example of this in the proposal.

2 Likes

Hi! This is a nice proposal and a cool idea.

Whilst looking on the proposal on Github, I thought that the current syntax is a little awkward grammatically:

fatalError(because: .unimplemented)

because it is missing a verb and an object to make a full sentence. I think that adding these would also be pretty awkward:

fatalError(becauseThisIs: .unimplemented)

as "this" is pretty ambiguous. So perhaps another syntax could be:

fatalError(withReason: .unimplemented)

or

fatalError(reason: .unimplemented)

What do you think?

1 Like

Quick impression: I think this reworking is significantly improved in terms of design, and I like that it is extensible, but some concerns--

First, I don't know that the four "pre-defined" reasons are as clearly distinguished as presented. Indeed, uncallable and mustOverride seem like flavors of "not implemented"; meanwhile, notImplemented is kind of a misnomer, as it really means not yet implemented, a particular kind of "not implemented" just like the other two above. This really leaves us with "not implemented" in three flavors and "unreachable"--and I wonder if the latter deserves its own function, hooked up to the LLVM primitive, so as to help the compiler reason about it too.

Second, I continue to think that "pre-defined" reasons--unless they enable some other functionality--work against the stated goal of making errors more expressive. It seems actively to steer users to try to fit their errors into pre-conceived categories instead of making sure that each one is well explained. Yes, the extensible design is a great improvement in that direction, but still the overall picture is that of a design that actively encourages categorization of errors to emit pre-written messages which happens to accommodate custom extensions.

Now, a good justification for some "pre-defined" reasons in the standard library might be that it helps the Swift compiler reason about your code. For example, if Swift has special knowledge that a method is meant to be overridden in a subclass, it may be able to provide the right fix-its for the user. But that seems to me that these deserve special treatment in the form of global functions or even syntax (for instance, true abstract classes, if there is really a need), rather than simply being a static member with a pre-defined message, which feels like a thin veneer over having the Swift compiler exhibit diagnostic behavior based on stringily-typed values.

Finally, in bikeshedding, I agree with others that because: seems awkward. The existing syntax fatalError("reason") already sets the precedent that "because' is implied, and I see no reason to deviate from that.

1 Like

I firmly disagree that these three distinct concepts refer to one kind of fatal outcome. They are never interchangeable and their use means a particular situation is being managed. I can refine the names and the descriptions to punch their different use-cases.

  • notImplemented can be expanded to "notYetImplemented"
  • uncallable can be expanded to "shouldNeverBeCalled"
  • mustOverride I think stands on its own, although it could be expanded to "subtypeMustOverride"

I mildly prefer a label over using fatalError(aString) interchangeably with fatalError(aReason).

2 Likes

Hello all,

Do you think it would it be possible to add support for assertionFailure and preconditionFailure to the pitch?

// three levels of failure:
assertionFailure(reason: .uncallable)
preconditionFailure(reason: .uncallable)
fatalError(reason: .uncallable)

Those variants would help those built-in errors fit nicely in the existing -Ounchecked, -O and -Onone compilation variants.

3 Likes

I don't think it would be hard to do but can you give me some justification examples for why? Generally preconditions validate calling conditions and assertions validate things known to be true. I'm not really seeing how these four scenarios (or others) fall under those two umbrellas. Help me understand and I'll be happy to expand and incorporate.

Sure!

The assertionFailure and preconditionFailure functions that accept a string already exist:

switch value {
    case ...
    default: preconditionFailure("uncallable") // or assertionFailure, or fatalError
}

It is already up to the developer to choose between fatalError, preconditionFailure, or assertionFailure today. If we extend fatalError with namespaced errors, then we should also extend preconditionFailure and assertionFailure, so that the developer's choice is preserved:

switch value {
    case ...
    default: preconditionFailure(reason: .uncallable)
}
3 Likes