-
Notifications
You must be signed in to change notification settings - Fork 17.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
proposal: allow implicit conversion of error to bool in if statements, fmt on one line #38151
Comments
Only after
Would this also work with annotated (wrapped) errors, e.g. |
This actually makes a lot of sense to do in my opinion. Both saves time writing and makes error handling look a lot cleaner. Sounds good to me 👍 |
This comment has been minimized.
This comment has been minimized.
As a C programmer, I often miss Would you want this to work also for I have actually been thinking about this same thing, because I frequently end up with, roughly:
and what I really want for a lot of these is basically "return these values if [this condition involving these values]". But in 99% of cases, the condition is "the last one, which is an error, is non-nil". the ugly answer: |
I like the idea of being able to test for errors this way, but I really feel like if you were going to make this change, that you would just make the bigger change for truthy testing any value. I get that error is a special case interface but I don't think users are likely to think of that when they find they can only truthy test an error interface and not any type. It will feel like a quirk where the language is unbalanced.
|
It sound like a good compromise when we can still choose to keep It should be easy to test and revert if needed. That could be pretty fun at the moment ! |
@peterbourgon would be very strange that this
is allowed but this
is not. @justinfx the problem is that empty slice and nil slice are both useful and practically identical in most of the cases ( |
@creker , honestly I would have just thought of an empty and nil slice to both be |
@swdee, there are plenty of proposals already open for new keywords like that. This is specifically what this issue is trying to avoid. |
@peterbourgon, returning wrapped errors seems reasonable, yes. |
@creker, writing: if err := foo(); err {} ... separates the word "if" from the word "err" (error). Since it no longer reads "if error", I think it's totally reasonable to require that be written the normal |
@justinfx, many downsides. Like others said: it's ambiguous whether that means len > 0 or != nil. |
@bradfitz it's fine if gofmt doesn't format it as a one liner but it doesn't make sense disallowing
That just means language spec would have some strange rule about completely identical code (apart from err scope which is irrelevant here) being treated differently by the compiler. I would expect Go to be consistent in its syntax. |
Yes, it does. |
@peterbourgon what is the objective reasoning? I understand if you personally don't like how it looks but that shouldn't affect language spec if it introduces inconsistencies in it. Either allow both or don't implement the proposal at all. |
That I don't know what it means. You might mean "if s is non-nil", you might mean "if s has a length other than zero". Heck, you might mean "if s contains at least one string which isn't an empty string". I think it's pretty reasonable to only accept the exact form
makes it a bit too easy for me to miss the nature of the condition.
is clearer. The argument here is just that, for the very specific case of I don't think this breaks the syntax much. The proposal here isn't to allow single |
That's not decided per the proposal
It does introduce inconsistencies in the language if we start disallowing syntactic constructs just because we don't like them. If we make |
There's nothing wrong with this. |
@peterbourgon there is and it's called bad language design. Language should be consistent and intuitive. If someone sees
The last one is unfortunate consequence of the proposal but it must work, otherwise the whole thing becomes a mess of special case syntactic sugar that doesn't mean anything (in other words, it no longer means implicit conversion to boolean. It now means "we like how this looks, let's allow it"). |
Overall, I like how refreshingly simple this proposal is, compared to most other error handling proposals. It deals with the verbosity without adding too much magic.
I think this rule could be simplified to: this kind of if statement is allowed to fit in a single line if its body is exactly one statement, and its length is short enough. This rule is already in place in
I think another point in favor of the
Note how we repeat the error wrapping/decoration. This is good if we want the call site to decorate an error with the function that was called, but I thought the point of the go2 draft was also to encourage functions to decorate errors with their own information when returning. |
I think it's good to be consistent and logical up to a point, but syntactic sugar is frequently special-case exceptions. We already have unusual rules on the behavior of if statements; the "if a := expr; a != 23" construct isn't a way you can spell an expression in any context other than an if statement, and it's not quite equivalent to having the first expression simply before the if statement, because it has different scoping. I would prefer "an if statement can take an expression which is either any expression of type bool, or a single identifier referring to an object of type error" to "automatic conversion of some things to bool". I don't think the proposed change here would have much impact on my daily life, but I'd probably use it. A change allowing error in general to be used as a conditional expression, or pointers or interfaces in general, would be a major change, and I'd guess that I'd end up being hurt by it more than I'd be helped by it. (Although I'm not sure; I got very used to "if (p)" in C, and sort of miss it sometimes.) I would be pretty strongly opposed to a general "implicit conversion to boolean". I would not object to conditional operations, like for or if loops, being able to take interfaces, or pointers, or possibly just error in particular. As it gets more general, it gets more confusing. Consider:
I would be thrown off by this even given the assumption that "err" could be used as a control expression for an if statement. It wouldn't confuse me in C, but in C, we're already used to pointers being usable as truth values. That said... What about cases where we'd return early on success? At the risk that it will make this proposal seem dangerous, consider:
This, coupled with the above, makes me think that this absolutely should not imply a general conversion of |
I'm fine with 2. I'd prefer if it's further limited only to if's that contain a return/break/continue statement. That's not especially important to me. 1 makes me uneasy. If it's just limited to Other boolean contexts seems like it could get convoluted. I could see I don't have any problem with |
Limiting the if to only contain a return/break/continue makes it not apply for testing, like if err != nil { t.Fatal("unexpected error:", err) }
if a != b { t.Fatal(a, "does not equal", b) } which that style I think would help remove the desire and proliferation of incompatible assertion style testing libraries. For those situations, one could even consider going further and adding brace alignment to aid visual scanning: if err != nil { t.Fatal("unexpected error:", err) }
if a != b { t.Fatal(a, "does not equal", b) } But, there's no existing precedent to do that alignment with function or struct literals like there is for allowing the whole thing to exist on a single line. During the
In my estimation, the amount of win you can get by allowing the discerning author to place if statements on a single line easily pays for the complexity and implementation cost. It has no negative interactions on existing or future code, greatly reducing the risk. And since there have already been changes to As far as worries about abuse, there are already a multitude of places where I think it's a very nice gradual step forward to reducing the page weight of error handling, even if it doesn't reduce the amount of ink on the page. |
@zeebo that's a fair point about testing |
This comment has been minimized.
This comment has been minimized.
I had actually explored this same idea myself, around the time when the other error proposal was being circulated. My conclusions/worries in respect to this specific proposal:
|
Although I promised myself that I wouldn't bother commenting on any more error handling proposals, this one may be unique in that (as I post this) it hasn't yet been heavily voted down! So here FWIW are my two cents. Firstly, several people (including myself) have long advocated that I realize, of course, that there are many people who won't agree with this but as it would presumably be an optional part of the proposal, they could carry on writing the As far as the |
I'm not in favor of implicit error-to-bool, but I'd be disappointed to see another error handling discussion engulf a lot of Go team time. We were told the topic had been shelved after the community voted overwhelmingly against making any change. That was a well-considered decision. |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
I think both of these look even worse than |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
C++ defines explicit conversions to bool as being special in the language, as they can occur implicitly in certain contexts. For example, if x is explicitly convertible to bool, then I believe a similar approach would work in Go. Allow the error type to be explicitly converted to bool using the conversion operator as in This creates a small set of rules that to me seem very Go-like, as they are simple and can't be leveraged and overloaded by the user; it's just a special case. On the other hand, if we were to simply allow the specific syntax It is similar to PHP's |
Based on the discussion above, this does not seem to be a clear win. And the emoji voting is not strongly in favor. Therefore, this is a likely decline. Leaving open for four weeks for final comments. |
Neat idea, but IMO it reduces clarity. Something like |
I am new to the Golang party and aware that there is a ton of past proposals that I have not read, but might as well wade into the arena: This proposal is nice in that it 1) accepts the reality that errors are special and unavoidable and 2) makes a minimal change for ergonomics that while are admittedly special-cased, are of such commonality as to fade into the background once adopted. I think of the semantics for how I think when combined with an affordance that enables the low ceremony return of underlying errors, we can address 80% of the pain without introducing new keywords, flow control, etc. To highlight what I mean, consider the introduction of a new error specific sentinel value (and maybe this should be a parallel proposal but this is where I am right now) of Consider something like: func readSomething() (string, error) {
file, ^ := os.OpenFile("some_file", os.O_TRUNC|os.O_CREATE|os.O_WRONLY, 0666)
defer file.Close()
result, ^ := doSomethingWithFile(file)
output, err := processResult(result)
if err {
// Try to recover from the error and return
}
return output, nil
} IMO boilerplate check and return of errors and coercion to boolean is the essence of the error handling ergonomic issues |
@blakewatters i like this idea for simple apps. But most of the time i prefer to wrap the error to add context. I think this idea was already discussed in the |
No change in consensus. |
@blakewatters I really like your idea for its simplicity and resemblance to
which is a bit hard to read with a later-called function on the left hand side.
or, to make the
The last two variants could be combined with non-zero values to the non-error returns, too:
Different from this proposal this would allow to simplify a lot of error return statements to concise one-liners. Would it make sense to open yet another error proposal for this? |
The guard let foo = bar() else {
return “error”
}
// Use foo here If Go adopted it: guard foo, err := bar() else {
return err
}
// Use foo here |
@andig @blakewatters. |
Will wait some more to not raise this twice. A connection of @ydnar‘s suggestion of try/guard statement that is able to elevate variables out of block scope (which was my biggest pain) allows for using syntax similar to the if statement plus introduce a short form that entirely avoids the multi-line if err checking. I wouldn‘t want to raise it and waste everybody‘s time though if this has been seen before- I couldn‘t find it in the error handling-tagged proposals. |
The main benefit of guard in Swift is not saving a few lines but optional binding. Other than that I always considered it as a pretty strange construct. It's an if statement and Go doesn't need two types of if statements. I'm sure the proposal would be quickly downvoted and declined. As for extending an if statement, there were multiple such proposals and all of them were declined either because people don't want another special magic symbol to do all kinds of things (like ^ in the above example) or it doesn't address all the requirements (like allowing for errors to be augmented with some contextual information). The example above have the same problem many other such proposals had - it makes it difficult to see the error path. You have to scan whole line to get to it. It also makes lines much longer and will require you to split them in multiple lines possibly making code even harder to read. The benefit of current error handling is it's completely linear - you can scan the function from top to bottom only checking a few starting symbols on each line and get a glimpse on every return path. |
I don't support special-casing the However, if the proposal were to special-case all interface values in this way, then I think it's more acceptable. That is: any interface variable can be used in a boolean context; a nil interface is treated as false, and any non-nil interface is treated as true (even if it contains a nil value, e.g. a nil pointer) It could be used as a guard for a possibly-nil interface:
However, this may lead to requests for similar treatment of non-interface types. Aside: I do sometimes miss Python's truthiness values, which each type can define using |
As this proposal is declined, could folks pls continue the discussion re error handling in a new proposal, or golang-nuts? Thanks! |
A couple weeks back during a proposal review, reviewing the fiftieth or so error handling proposal, I made a somewhat flippant remark that if we made
if err != nil {\n return nil, err \n}\n
shorter, most of the error handling proposals would go away. (At least the more incomplete ones)I then mentioned two things we could do pretty easily:
if
statements only, just:... as syntactic sugar instead of:
That is, if the static type of a variable is exactly and only
error
, allow conversion to a boolean."But that's not orthogonal, why would you make error special?" Yes. But errors are already special. They're the only interface predefined in the language. And they're incredibly common.
Even if somebody is momentarily confused why it works, it's at least not ambiguous.
IIRC, we discussed a bit whether to allow it in all boolean contexts, just in boolean expressions inside an
if
condition (soif !err {
would work), or only when it's the entire expression in anif
(onlyif err {
).Later, if we do #21182, that extends to:
But if you have any non-return statement in the if body, then it formats as multiple lines.
Taken together, this makes error handling much lighter looking (the common motivation for new error handling proposals), while keeping flow control unchanged and explicit (a common argument against most error handling proposals).
I made these comments mostly as a joke at first, but the more we talked about, the more we didn't outright hate the idea. I reluctantly agreed to file an issue, so here it is.
The text was updated successfully, but these errors were encountered: