Reviews are an important part of the Swift evolution process. All review feedback should be either on this forum thread or, if you would like to keep your feedback private, directly to the review manager @jckarter (via email or direct message in the Swift forums).
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?
Much of this is a restatement of my opinion from March 31 on the pull request.
I am -1 on the proposal. The motivation isn't strong, the proposed solution isn't complete, and it doesn't seem to fit with the design of Swift.
In my opinion, no.
The "The Naive User / Fixit Problem" and "Moral Hazards" motivations have a better solution. Instead of the compiler producing a fixit hint recommending the use of x!, we should provide a fixit recommending x ?? <#default value#>. This defines away the first problem, and directly addresses the second problem, all without introducing new complexity to the Swift language. The proposal requires a change to this behavior in any case, it isn't clear why rewriting this to x !! <#error message#> is better than x ?? <#default value#>.
The "Runtime Diagnostics" is interesting, but this proposal is not a complete solution to the problem. While it addresses the issue of x!, it doesn't generalize or address the other failing forms in Swift, such as like try!, as!, array subscripting, integer overflows, etc. If we wanted to address the general solution, I'd recommend investigating scoped failure handlers and tying all these mechanisms into that.
This doesn't seem to fit with the design of Swift because the existing doubled up operators like &&, ||, ?? all exist specifically because they provide important "short circuiting" behavior. While the proposed !! operator could be defined as taking an autoclosure, it doesn't have the same nature of these operators and therefore IMO doesn't make sense to be short circuiting.
Furthermore, if this were important functionality to include, it isn't clear why we'd sugar it with such concise syntax. The point of the x! syntax is to be concise, but the proposed !! operator has a verbose message string attached, so the weight of syntax is significantly less.
Also, it is worth mentioning that since the first proposal was drafted, Xcode's behavior handling these sorts of failures has been greatly improved.
N/A
I participated and contributed to the pitch phases, contributed this opinion to the pull request, and read the final proposal in detail.
I'm against the overall design of this proposal but I wouldn't say it's completely useless. In fact this proposal depends on the acceptance of SE-0215. If the mentioned proposal will be accepted then we know that we can add temporary solutions that can be removed later when Never becomes the bottom type. In that sense the current proposal already mentioned an alternative solution which with Never as the bottom type will eventually become valid.
I wouldn't want to see a new !! operator but I can accept an overload of the current ?? such as:
func ?? <T>(optional: T?, noreturn: @autoclosure () -> Never) -> T {
switch optional {
case .some(let value):
return value
case .none:
noreturn()
}
}
That said, I think we should reconsider the proposal and try to simplify it instead of bloating the standard library with new operators that would eventually fade with Never as the bottom type.
Correct, but my point was that we have to agree on temporary additional changes that can be removed later. If for instance SE-0215 won't be accepted then I don't see why we should accept the current proposal. Furthermore I don't see the justification of !! as sugar for ??, it just adds another operator that from my point of view will fade completely as soon as Never will become the bottom type.
In fact I can see myself favoriting something like this instead of fatalError (with runtime warning/error directives support):
I am opposed to this proposal. While the I agree with the motivation, it is not strong enough to introduce another operator which is very redundant (! already throws a fatalError()).
No, part of the Swift design should be to avoid process trapping/crashing, which this proposal encourages. A simpler solution would be to use a better default fix-it (like one based on guard let ... else).
This is naïve, IMHO.
Using a statically typed language with a powerful type system means that you can use the compiler more often to turn run-time assertions into compile-time assertions. In that sense, the compiler is like a proof-assistant that can automatically verify certain properties about your program.
However, I have yet to see a language in which you can actually prove everything of interest to the compiler. Swift, with its lack of HKT and no support for dependent types, certainly can't prove every interesting property at compile time yet (e.g. about the length of an array). Therefore, the need for runtime assertions arises.
Runtime assertions enable you to say "I strongly believe that this condition holds, however I can't formally verify it (via the compiler). If this condition should, contrary to my belief, not hold anymore, or if surrounding code ever changes in such a way that the condition is invalidated, I want this to fail as fast as possible, so I am immediately made aware of the problem, instead of passing on nonsensical values and getting wrong results in the end, long after the initial error actually occured".
This in fact, is the same situation where you would use unchecked exceptions in Java or RuntimeErrors in Ruby: for unrecoverable errors, where the only thing you can do is throw your hands up in the air and say "well, this is bad, we should fix it".
Think about what the alternative would be: You have a situation where you know that your array must be non-empty. Certainly, if it were empty for some reason that would indicate a bug that should be fixed. Now image you want to call first on your array - what kind of options do you have now?
You could:
pass on the optional value. At some point, you'll have to unwrap it though, so you're only deferring the problem to a point where, when the condition is violated, it's very hard to pin-point what went wrong.
use force unwrap. But that also goes against your "avoid crashing" philosophy, and it also gives you worse diagnostics than the proposal here
use guard/let. But then, what's your alternative? You know the empty array / whatever other case can never (or at least, should never) occur, so what kind of alternative implementation would you provide? Some random default value? How does that help?
throw an error and have some kind of error handling at the outermost level. this works, but unfortunately forces you to annotate every intermediate function with throws, which for conditions you know to be wrong, seems like an overkill. Worse, this is the same kind of "checked exceptions" fiasco that went horribly wrong in Java (IMHO, not because checked exceptions are per se wrong, but because they were overused for unrecoverable errors and therefore caused lots of pain in client code. Ask any Java programmer how many empty catch blocks he has seen in enterprise code.)
or you actually use guard/let but with a fatalError. But this is exactly what this proposal wants to have a shorthand for.
In practical terms, what you want - avoid crashing - doesn't even work. Every time you do an array access you're risking a crash (array access returning optionals was ruled out for performance reasons, afaik). Every time you add or multiply integers, you risk a crash, unless you use the special functions that check for overflow (but these are very cumbersome, so they're not often used). So in any program of sufficient complexity, crashes will occur. The question is how to deal with them, and crashes with good diagnostics are much more helpful than "SIGILL".
Now there is a problem, unfortunately, in that Swift doesn't allow you to recover from crashes (unlike every interpreted language, where every error is recoverable, because the runtime can deal with it, including JVM languages). This, IMHO, is a problem that should be solved at some other level. In an ideal future, I would love to have a concept similar to Erlang in which you can have supervisor trees where parts of the program can restart other parts of the program... but this is probably a pipe dream for now.
In sever-side land, where I am, currently the only solution I see is to deal with this on a container/load balancer basis, where crashed containers can be respawned and requests rerouted etc.
All that said, the motivation for the proposal makes 100% sense for me. I write guard/let + fatalError a lot, and having a shorthand for this would be a) incredibly convenient and b) encourage other (more junior) developers to actually use it to assert certain conditions instead of either using the bang notation or including some nonsensical default value in the "else" case (that is often just drawn out of thin air).
I don't have an opinion on whether it needs to be this explicit proposal, or whether some alternative with ?? and a Never type would be preferrable, but I do believe that there is a gap here that should be filled somehow.
IMHO using force unwrapping / try ! / fatalError() should be left for extreme case were it is impossible for the unwrapping to crash or because the process is in an unrecoverable state.
All other cases should be solved with throwing functions, optionals and default values. For me, that's part of the safe language paradigm. If we don't follow this terms then any third-party library/code could decide to crash a process for recoverable cases like "your array must be non-empty".
This isn't actually a thing. "Safe" means whatever you want it to mean.
If by "safe" you mean that you can move more assertions from compile-time assertions to runtime assertions, then Swift enables you to do that - but only up to a point.
If by "safe" you mean something like "doesn't randomly point to unallocated memory" then Swift does that (as opposed to e.g. C), but it does so by crashing your program instead of propagating nonsense values. So in that sense, Swift consciously chooses to crash in the interest of safety in some cases.
If by "safe" you mean "doesn't allow you to write bugs", then well... we haven't discovered that language yet.
Just compare Erlang with Haskell (or even Idris), for example. Both have radically different approaches to resiliency. Which one is superior?
For library code, as is usual, design guidelines must apply that can be a bit different than for in-house code. In particular, you must assume more consumers and less direct feedback cycles.
Yes. In addition to the fixit problem outlined in the proposal, the introduction of !! encourages thoughtfulness in code, because it wants you to consider why, and come up with an explanation and rationale supporting that position. Postfix-! does not; Postfix-! just blindly goes in with no explanation why and no deeper consideration or evidence for future coders as to why it's an appropriate thing to do.
The idea that you could always instead supply a default value if you get a nil value instead is ultimately insufficient. There are cases where you must force-unwrap:
let url = URL(string: "https://forums.swift.org")
The type of url is URL?.There is no good way to supply a default value here, because all of the other non-file-path initializers for URL are failable. You must force-unwrap. However, if you must do so, it would be a wise idea to document why you must do so, and document it in a way that future generations can easily understand why that decision was made without having to dig through git history.
I am firmly of the opinion that Swift should be the nicest and most-full featured language we can possibly make it. As @Soroush_Khanlou would say, "We deserve nice things". An operator like this is a Nice Thing™. It's used commonly enough that I believe it merits inclusion into the standard library. The objectors to the proposal are free to not use it if they don't want.
N/A
I'm one of the proposal authors and use this operator everywhere I need to force-unwrap. I do not use Postfix-! at all in my code, and always favor using !! instead because it forces me to explain to myself why I think this is an acceptable choice to make in code.
I'm indifferent towards this proposal. On one hand I see the use cases for this, and it's a helpful little operator if your code style warrants such a thing. But on the other hand, I don't see the benefit vs the alternatives:
Writing the operator yourself in your own projects.
Using some alternative way of achieving the same thing. e.g. guard, assert, precondition, or just ! with a comment somewhere nearby explaining why you're committing a Swift sin.
Just waiting for optional ?? fatalError("why this shouldn't happen"),
However, I don't think the inclusion of this operator is going to degrade the "safety" of Swift. It's the same as ! but with the added "benefit" of explaining at runtime that some particular value was nil when it shouldn't have been.
I believe this would be a useful addition and am a +1.
I frequently find myself doing two things:
helping developers new to Swift understand what the ! that they inserted via the fixit actually means
debugging code that trapped on nil in a way the user thought was legit, but didn't elect to document
I think the ! fixit, while useful in the early days of adoption and migration, is now actively harmful and needs addressing. A ?? fixit would be better, but on its own would still be harmful as it would encourage silent toleration of nils that haven't been thought through.
Adopting both !! and ?? fixits, and having the developer choose between the two, would have considerable educational benefit IMO, and use of !! as an idiom when you have good reason to believe nil is impossible would be a significant readability and debugability win.
Encouraging the documenting of reasons for legitimate force unwraps as an everyday practice would be beneficial, and introducing sugar to encourage this with a simple form is a good idea.
Counters to alternatives being proposed
As mentioned above, replacing the ! fixit with a ?? one would be better, but still harmful, and pairing it with a !! alternative is necessary and IMO the right complete solution to the problem.
The main downside to offering two options is that it doesn't work well with Xcode's fix-all-in-scope. But if you start requiring <#something> anyway, that becomes less of an issue.
This seems unsubstantiated. Given an error may involve string interpolation, short circuiting is legit (and is consistent with !! being a shorthand for guard + fatalError).
I believe the someOptional ?? Never (or fatalError) form is extremely beginner-hostile – a cute trick that is actually an abuse of the nil-coalescing operator and harder to read than a simple dedicated operator, so a poor idiom to encourage. It is also dissonant, to my eyes, as I read ?? as very much a "tolerant of nils" operation, and coupling it with a trap causes me to do a double-take.
The key difference is that !'s fatalError produces no diagnostic. I often wish, when debugging someone else's code that trapped on nil, that there was a reason given for why the nil wasn't legit. Sometimes there's a comment above the unwrap, but it'd be delightful if that comment appeared right there in the debug output. I'd like to say I always write guard let else fatalError myself but I'd be lying, as it's a real drag to do so and clutters up often otherwise-simple code.
A "we shouldn't fix one thing until we fix everything" argument. If there is a neat solution to try! and as! that'd be great, but the lack of one shouldn't be a blocker to progress in this area. What's more, it is already the case in Swift that we have more than one way to handle things, especially with optionals – both shorthands for common cases and more general solutions with slightly more ceremony. So this seems to fit well with Swift's current design principles IMO.
What's more, those cases are different to ! in that they mostly already produce useful diagnostics when they trap. The problem with ! is that the error message you get when it traps is so unhelpful.
In the original pitch discussions months ago, I was fairly strongly against this. Upon reading the official proposal, and perhaps some personal evolution, I'm mildly for it (+0.5?).
I still have some reluctance about an operator that takes an arbitrary value T on one side and a String used purely for diagnostic purposes on the other. The coupling with fatalError makes me uncomfortable; it means that users can only get the convenience of this operator if they want that specific failure path, as opposed to something else (i.e., a custom logging solution that may do extra work before trapping).
For that reason, I'd prefer something more along the lines of a Never-based approach, like x ?? fatalError("unwrap failed because..."). But that is admittedly more verbose.
The main reason I've warmed up to this operator is the same as something @Ben_Cohen stated above. Google's Swift style guide states that any force-unwrap that is guaranteed to be safe due to other invariants should be annotated with a comment describing why it is safe. The string argument to !! would serve the same purpose of that comment but is also strictly superior to that because if the invariant does fail to hold, the string is emitted as a diagnostic whereas the comment is not.
Maybe. There are certain scenarios where trapping on nil is absolutely the correct choice, if there is no clear path to recovery or avoiding data corruption. We don't want people to take an arbitrary "Force Unwrap Considered Harmful" view. In those cases, neither (1) using ! (even with a comment) and getting no meaningful feedback or (2) writing a guard statement that adds much more noise to the usage site than a shorter expression are appealing options.
So I guess the question is, do these situations come up with such frequency that this proposal is a readability win?
The operator-based syntax is a bit of a stretch in terms of ergonomics compared to existing Swift constructs and feels too much like a Perl-ism for me. Something like x ?? fatalError("...") feels like it would be a better fit, albeit slightly more verbose.
Read the proposal and participated in some of the earlier discussions.
No. The Never part is just an implementation detail. What I'm against is encouraging this "second use" of ??. I fear that the ingeniousness of the solution involving Never is skewing people in favor of a solution that strikes me as an abuse of that operator for an entirely different purpose, and one that will take a lot of explaining to people as to how it works. The explanation is really cool – but confusing people until they get that explanation* is very uncool.
*and maybe after they get it, depending on how good the explanation is
I don't think that the Never on the RHS of ?? is actually clever enough to be dangerous or hard to teach. It's the kind of thing that looks like it should work anyway, and the explanation of why it works isn't very difficult.