-
Notifications
You must be signed in to change notification settings - Fork 17.8k
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: Go 2: use ? during variable assignment as implicit goto statement #64953
Comments
Did a bit more digging and testing after writing this up and came across this: https://go.dev/ref/spec#Goto_statements. Looks like goto can't skip over variable initializations, so looks like this proposal would have to create a separate construct anyway unless it's possible to eliminate the restriction on goto. Perhaps the switch/case-like version would work better after all, so it's clear the ? is something different than a standard goto, and that the cases are not standard labels:
|
If one were to accept this proposal, then the restriction should be eliminated (refer to #27165 and #26058, with the former closed in favor of the latter) rather than introducing a separate construct. |
this looks similar to (not a proposal, but we did see this): https://gist.github.com/dpremus/3b141157e7e47418ca6ccb1fc0210fc7 also #53074 where mixing label and variable was rejected |
Ah, great, so some thought is already being given to lifting the restriction, ty for the reference. |
There have been many error handling proposals with
|
Ah, I hadn't seen that, basically exactly the same as my proposal. If mixing label and variable is an issue, then seems easily solvable with a keyword in front of the label, that just adds a bit more work. |
Introducing a new keyword is usually a non-starter for proposals due to backward-compatibility considerations. |
Doesn't have to be a new keyword, could re-use an existing one. Maybe else? case? Or even re-use the ? in the label name. I personally like the brevity of just using a label, and they're rarely used anyway, but seems like we have plenty of options here if the concept is good:
|
As a veteran of the Try Wars, I like this more than I expected to. Reusing the variable name as a label is clever. It requires #26058, but that's probably a good idea anyway. What happens if the label is above the |
I don't see any reason to do anything different than a normal goto. Looping back would be a legit use case. Consider using it to implement error retry:
If you tried something like this with a for loop, you'd need checks after each function to either break or continue. The more functions in the retry chain the more boilerplate you'd save using the ? feature. It's interesting how I've circled wrt goto -- I started using goto/jmp in basic/assembly back when I started programming, moved on to higher level languages where I drank the kool-aid that it was evil and went to great lengths to avoid it, and now I'm back to a more balanced stance. Sometimes goto is just the right thing. |
I do broadly like this, to the extent that I like any proposals for shorthands for error handling. 😀 (I'm in the camp of "I don't actually have any problem with Go's current approach, but I'm not opposed to a good proposal to improve it.") The only small concern I have is that it seems likely that some functions will have multiple different error-handling labels, as shown in the proposal's current example: dstErr:
os.Remove(dst)
err = dstErr
err:
return fmt.Errorf("copy %s %s: %v", src, dst, err) ...but I suspect that falling through from one to another would more often be a bug rather than intention, even though I can see that in this specific example it was intentional. My reason for suspecting that is that I very often write functions that have multiple error return points that each have their own error message, which under this proposal suggests that they would each have their own label. As long as I remember to write The alternative in the proposal that uses a |
If each have their own message then there's no point in factoring them out. You'd just write the |
Yeah, definitely a good point here -- but go's normal labels fall through, even if select and switch do not. So it just depends on whether your mental model when you see the error goto labels is 'labels' or 'switch'. I suspect a large number of go developers rarely use labels, so I agree with your concern. For me, I think if we make them look like go labels, they should behave like go labels, and if we make them look like case statements, we should make them behave like case statements. |
I think there's a proposal floating out there to unpack function return arguments with three dots... if that gets implemented as well as my proposal, maybe something like this would be possible:
|
Agreed, although I think of it from the other perspective: we should choose which of the two it should look like based on how they are most likely to be used. If the most likely is that there's only one error label for the entire function then I think the plain label style is defensible, but if this proposal seems likely to cause functions to end with a series of different error handling labels that are mutually-exclusive then that suggests a Of course, I don't have any data either way, so I can't actually say whether my hunch is right. It would probably help to collect some more examples of code that would benefit from either shape of this improvement to see if one possibility stands out as more helpful than the other. |
That's a fair objection, but with the way I tend to write things I would tend not to add a prefix at all if the prefixes would all be the same, because the prefixes would not be adding any new information. The caller might potentially add a prefix to distinguish its call to my function from other functions, but then the caller's back in the mode of using a separate prefix for each return again, and so the advantage is diminished. I suppose now that you mention it I am essentially somewhat objecting to the motivating example from the proposal: func CopyFile(src, dst string) error {
// ....
return fmt.Errorf("copy %s %s: %v", src, dst, err)
} Personally, I would not have added the prefix here, because I'd consider it the caller's responsibility to report that the error occurred while it was copying a file, assuming that's interesting information. The caller might even choose to describe what it was doing in terms more appropriate for its own level of abstraction. It's implied that any return value from However, I don't really wish to derail this proposal by arguing about what error style is more appropriate. If there's evidence that I'm the outlier here, and that it's more common elsewhere to add a single prefix in each callee rather than multiple prefixes in different parts of the caller, then I concede this point. |
No matter which error style you prefer, I think there is still a lot of value in moving all the error handling logic out of the main body of the function -- it will make the logic easier to read and follow, as it won't have 3 lines of boilerplate after every function call that returns an error. |
My feedback is perhaps a bit subjective but I'm a bit annoyed by the addition of the? sigil. I don't find it very legible overall if we compare it to the try proposal that is linked. On the other hand (!!!), it is interesting that it allows to create named error handlers and apply them. I would perhaps do it differently so that it remains closer to common go code and work as a kind of macro expansion. handler1: (err error){
return err
}
handler2: (err error, f *os.File){
f.Close()
return err
}
v, err := fn()
check(handler1, err)
// happy path But even that I don't really like either, even disregarding the fact that it's difficult to find the correct way to declare error handlers out of the flow and difficult to find a correct syntax for their application. What I'm not sure about it is when the error handling needs to do more than simply returning an error. It means that the error handler needs to be aware of variables that look like they are out of its scope. Such is the case with handler2 above. I still think that there is not much better than the way we currently handle errors. |
I thought it would be interesting to try this proposal with a slightly more complex situation, with the following requirements:
Here's what I came up with. I'm not meaning to suggest that this is the only way or the best way to write this under the proposal, just trying to see what it might feel like to use in a more complex situation with lots of different error paths to handle. func CopyFile(src, dst string) (err error) {
r, openSrcErr? := os.Open(src)
defer func () {
if closeErr := r.Close(); closeErr != nil {
err = errors.Join(err, fmt.Errorf("closing source file: %w", closeErr))
}
}()
w, createDstErr? := os.Create(dst)
defer func () {
if err != nil {
// If anything fails after we've created the file
// then we'll remove the possibly-incomplete file
// before returning.
if removeErr := os.Remove(dst); removeErr != nil {
err = errors.Join(err, fmt.Errorf("removing failed %q: %w", dst, removeErr))
}
}
if closeErr := w.Close(); closeErr != nil {
err = errors.Join(err, fmt.Errorf("closing destination file: %w", closeErr))
}
}()
_, copyErr? := io.Copy(w, r)
return nil
openSrcErr:
return fmt.Errorf("opening source file %q: %w", src, openSrcErr)
createDstErr:
return fmt.Errorf("creating destination %q: %w", dst, createDstErr)
copyErr:
return fmt.Errorf("copying file contents: %w", copyErr)
} It's debatable whether handling the errors from One specific papercut I ran into while writing this is that I'm accustomed to typically just repeatedly assigning to a single Since I handled the |
I think your example shows exactly why the functions themselves should add context to their error returns, i.e. if close returned an error that reported 'closing file $filename', then copyfile could just do a single error wrap as @rsc's original example showed and everything would make sense. 'copy $sourcefile name to $destfilename: closing $filename' would be the final composed result, and it would be clear that the destination file was the one with the problem. |
I think we have different styles -- I definitely prefer conciseness, I find yours hard to read because it has so much extra noise. Though, I admittedly don't love the ? either, but for something as common as error checking I like being concise and for me ? means a test in most languages (consider C++'s ternary operator) so it maps nicely. |
I understand. It is also easy to forget to put a ? sigil and shadow an error because it's been reused a few lines below. Having apparent error checking statements gives at least more visibility. Or is this concern addressed somehow and I've overlooked it? |
As much as I'd like to see some changes in error handling, I'm finding it hard to get onboard this proposal. My main concern is whether this approach will bring back the horrors of
|
Can you elaborate here?
I think the 'more difficult to understand where the error originated from' is an issue with many of the error handling proposals though, even @rsc's original proposal I linked in my original message. That's just a tradeoff I think we're making to make error handling more concise -- for me at least, I find 90% (maybe even more) of the time I want it to go to the same place because I just need to wrap the error in some context and return. I'm having a hard time thinking of a way to alleviate your concern without making the 90% case less convenient. For instance, we could disallow more than one ? test per variable, forcing each to jump to a different label, but then the typical case of 'wrap and return' means we have to duplicate the code in each label. For instance:
when I would much rather write (and read, because it makes it very clear these are all the same case without having to manually scan and compare each case):
|
With this proposal, if you need to handle an error in a different manner, then you should simply use a different error variable name. This is illustrated in the original example. Unless I'm missing your point?
I don't understand this statement?
Yeah, I think this is maybe the fundamental style thing here for those in the 'error checking is fine' and 'error checking isn't fine' camps. I'm in the latter, because I think including the error handling in the main control flow is noisy and obscures the main logic. I still want it to be visible of course, but moving it the error handling to the end of the function using labels consolidates it in one place and makes both (the main logic, and the error handling logic) easier to read and understand for me. I wonder how people code too -- for me, first I write the main logic, not thinking too carefully about the error cases or handling them trivially because I'm focusing on making sure the function works. Then, once I'm done and happy with the logic, I do a second pass through the function to properly implement error handling. So this proposal maps nicely for me because I can just put a ? identifier after each err while I'm coding the main logic, implement a basic wrap-and-return at the end, and then during my second pass I can split out special error handling cases where needed by changing the variable name and adding a new label. |
But it is not enforced is it? What if someone employs the traditional go style and reuses the same error name mistakenly? Also re shadowing: v, err? := f() // handled
v, err = g() //not handled
// ...
w, err? := h() // handled
err: // handler definition It makes the lack of handling of the middle error a bit easier to go unnoticed. |
But being able to re-use the same error name is a feature, because then it's clear both these checks use the same error handling logic.
Ah I see what you mean. The ? token stands out to me personally since it's not used in any other context, so I don't think for me it's any less conspicuous than the normal if boilerplate. But I know everyone reads code differently. |
For this concern of forgetting the This question did also make me think about the fact that labels are function-scoped while variables are block-scoped, and so the following is a little challenging to describe the rules for: func example() error {
err? := Something()
if somethingElse {
// This declares a new "err" available only in
// this block.
err? := SomethingElse()
}
err:
// Which of the two "err" variables is live here?
} Normal scoping rules would mean that the nested The connection I see between this and the other point is that this situation seems like it might warrant another special rule constraining how the The existing rule that you can't jump over a variable declaration would already block this today, but this proposal seems to imply somehow loosening that rule, and so we'd presumably need to make sure we don't loosen it so far that this ambiguous situation becomes possible, or at least find a way to loosen it that seems useful. |
Yeah, we could vet that case for a warning or even make it outright illegal. I don't have a strong opinion here.
Very good point here about nested variable scoping -- though the loosening the 'goto-can't-skip-over-variable-declaration' rule is a separate proposal #26058, independent of this one. To keep this proposal straightforward which is one of the whole points, my initial thought is ideally we'd just do whatever goto does today (or, in the future, with #26058 implemented), and if one needs something more complicated, fall back to regular |
Borrowing from #53074 (comment), which discusses a similar proposal to overload variable names and label names:
Therefore, this is a likely decline. Leaving open for four weeks for final comments. |
We discussed this in the comment chain and I added an update that we could disambiguate labels by using a keyword or a prefix. (highlight added)
|
Thanks for pointing that out, @bsoudan, and for the thorough updates to the issue description. That does make it clearer, though it still mixes the semantics of variables and labels. In particular, as noted by @apparentlymart:
Consider this related example (apologies if this has already been brought up): func _() {
if cond {
err? := "hi"
}
err := 1
?err:
// What is the type of err here?
} Do we not allow the
I can relate to this, but I worry that this new form of error handling would encourage goto-like control flow in many places where it's not the right thing, and would impact readability. Sometimes |
Great, just don't want this proposal to be rejected for that reason because I think it is a very solvable problem. We could completely disambiguate with something like func CopyFile(src, dst string) error {
r, err? defaultError := os.Open(src)
defer r.Close()
w, err? defaultError := os.Create(dst)
defer w.Close()
_, err? closeError := io.Copy(w, r)
err ? closeError = w.Close()
return nil
closeError:
os.Remove(dst)
defaultError:
return fmt.Errorf("copy %s %s: %v", src, dst, err)
}
I noted in a response above I think we should just follow the normal goto rules (hopefully, eventually, with #26058 implemented), but I did promise to think about it more. I did spend more brainpower on it, and I still think the same thing. Just follow the normal goto rules, whatever they are. Today, for your example, the compiler already gives you a good error, as tested on the playground. Couldn't put a link for some reason, getting a server error when I click the share button. func main() {
if true {
// err? := "hi", translated to:
err := "hi"
goto err
}
err := 1
err:
fmt.Println("Hello,", err)
}
// compilation error: ./prog.go:10:8: goto err jumps over declaration of err at ./prog.go:13:6 Which is great, because you're writing confusing code. But it's a simple fix. a) don't do that and b) just declare err as a variable in the outer scope. func main() {
var err string
if true {
// err? = "hi", translated to:
err = "hi"
goto err
}
// can't do this anymore, but that's probably a good thing for code clarity
// err = 1
err = "everything's fine now"
err:
fmt.Println("Hello,", err)
}
// Prints 'Hello, hi'. And to directly answer your questions:
We should allow the err? form to jump out of a block if and when goto allows it. If the developer needs to use the scoped variable at the jump target but the jump target in is the outer scope, they can solve that problem easily by just declaring the error variable in the outer scope.
That is a separate proposal that seems to be getting traction, #26058, that would allow jumping across intervening declarations. If implemented alongside this proposal I think your concern would go away.
I thought about this but I don't share the same concern -- goto exists now and it's not abused. Yes, this proposal would mean it was used more, but I think we should trust developers to make good judgement calls and write good code. I think the positive impact on readability (getting rid of err != nil boilerplate and moving it to the end of a function, which I think would be the majority of uses for this) would be worth the small number of abuses we'd be likely to see. And don't forget code can just still do it the old-fashioned way for complicated error handling, which would be more clear than a rat's nest of goto statements. I think what I like so much about this proposal is the simplicity of it, which is very appealing to me. It's not inventing any new forms of flow control a new developer has to learn. After thinking through all these different cases, I keep coming back to 'it's just a normal goto'. |
func CopyFile(src) error {
r, err? := os.Open(src)
defer r.Close()
w, err? := os.Open(src)
defer w.Close()
return nil
err:
return fmt.Errorf("copy %s: %v", src, err)
} Is there any chance to figure out from which line comes the err? If no, I prefer inplace return rather than implicit goto. |
Is it important to have a clear understanding of which The first type func CopyFile(src) error {
r, err! when errors.Warp(err,"What is this") to err? := os.Open(src)
defer r.Close()
w, err? := os.Open(src)
defer w.Close()
return nil
err:
return fmt.Errorf("copy %s: %v", src, err)
} The second type func CopyFile(src) error {
r, err1? := os.Open(src)
defer r.Close()
w, err2? := os.Open(src)
defer w.Close()
return nil
err1:
return fmt.Errorf("copy %s: %v", src, err)
err2:
return fmt.Errorf("copy %s: %v", src, err)
} |
As described in #64953 (comment), the overloading of variable names and label names is confusing, in part because variables and labels have different scoping rules. No change in consensus, so declined. |
Proposal Details
Throwing my hat into the already crowded ring for Go error handling with an extremely simple proposal: suffixing ? to a variable name in an assignment statement adds an implicit
if (var != zeroval) { goto var-as-label; }
.Go proposal template attached at end.
Code Sample
@rsc's CopyFile example from https://go.googlesource.com/proposal/+/master/design/go2draft-error-handling-overview.md becomes:
Notes
Go has separate namespaces for labels and variables, so there's no conflict between the two already. Try it on the playground!
Allows arbitrary composition of error handling logic using normal golang control flow. Re-use error variable names to re-use error handling logic, use different variable names to support unique error handling logic, order them appropriately to chain handlers together, short circuit fallthrough with a return, use goto for more complex ordering.
No support for
if (var == zeroval) { goto var-as-label; }
. Could potentially allow ! prefix in labels to support this form, for instance, to support the common golang ok idiom:More than one ? test in a multiple assignment statement should be a compiler error. Use standard
if err
tests when multiple tests are needed in one assignment so control flow is clear.I considered a switch/case-like version:
But I don't see many advantages over the goto based version to justify the additional language spec/implementation complexity. (2024/01/08 -- broke this out into a separate proposal anyway: proposal: spec: support new form of switch statement during variable assignment which jumps to function-wide case blocks #65019)
Updates
2024/01/04: After I wrote this, it came to my attention that golang doesn't support goto jumping over variable declarations. I've used goto in several golang programs but somehow never stumbled across that issue, so I was unaware. However, commenters pointed out that there is an open proposal to address that issue: proposal: spec: permit goto over declaration if variable is not used after goto label #26058. This would need implemented in order to properly support this feature.
2024/01/04: @seankhliao pointed out there is a gist with an essentially identical proposal ( https://gist.github.com/dpremus/3b141157e7e47418ca6ccb1fc0210fc7) that was never submitted.
2024/01/04: @seankhliao also pointed out there was a proposal (proposal: Go 2: extend "goto" - statement #53074) that also re-used variable names as labels. @ianlancetaylor didn't like overloading variable names with labels, though golang currently permits it.
I do like the brevity of re-using the error variable as the label, less typing and cleaner looking code. Using typical go conventions, this would always be 'err' or '*Err' so it would be clear that this was an 'error goto' destination without having to use any special keywords. Could even enforce that with a 'go vet' check. It also means the error labels can be used for regular goto statements, which is nice for complex error handling chaining. If we need to distinguish the implicit error jump labels from standard goto labels, though, I propose this version for symmetry:
If this version is preferred, then I would also propose that we add the ability for the regular goto statement to jump to ? labels, as in 'goto ?err', to support complex error chaining.
2024/01/04: @carlmjohnson asked if the label can appear before the assignment. I don't see any reason to use different rules for the implicit ? goto labels than standard goto and labels. Consider using the feature to implement error retry:
If you tried something like this with a for loop, you'd need checks after each function to either break or continue. The more functions in the retry chain the more boilerplate you'd save using the ? feature.
2024/01/12 -- @apparentlymart and @findleyr brought up cases like this:
but if we apply the 'it's just a normal goto` test, you can see that even today, the compiler already gives you a good error, as tested on the playground. Couldn't put a link for some reason, getting a server error when I click the share button.
Which is great, because you're writing confusing code. But it's a simple fix. a) don't do that because you wouldn't be able to with a normal goto, and b) just declare err as a variable in the outer scope.
What I like so much about this proposal is the simplicity of it, which is very appealing to me. It's not inventing any new forms of flow control a new developer has to learn. After thinking through all these different cases, I keep coming back to 'it's just a normal goto'.
Go Proposal Template
Would you consider yourself a novice, intermediate, or experienced Go programmer?
Experienced.
What other languages do you have experience with?
Assembly, C, C++, Python, Erlang, several others
Would this change make Go easier or harder to learn, and why?
A bit more control flow to learn with new ? suffix, but by moving all the error handling boilerplate to separate blocks, the main logic would be easier to follow, so could argue either way.
Has this idea, or one like it, been proposed before?
I read through as many error handling proposals as I could and did my best with the github search feature but couldn't find anything as simple and straightforward.
Who does this proposal help, and why?
All gophers that would like more concise error handling.
Is this change backward compatible?
Yes, existing code does not need to be changed.
What is the cost of this proposal? (Every language change has a cost).
More complex grammar,
How many tools (such as vet, gopls, gofmt, goimports, etc.) would be affected?
Probably many, since this would be a change in language syntax.
What is the compile time cost?
Not familiar enough with the compiler internals to answer this.
What is the run time cost?
Probably negligible if the compiler can optimize out unnecessary case tests.
Do you have a prototype? (This is not required.)
No, but would be happy to put one together with a little guidance.
How would the language spec change?
Only adding support for a ? variable suffix.
Orthogonality: how does this change interact or overlap with existing features?
Re-uses existing golang control flow so minimal interactions.
Does this affect error handling?
This change is intended to provide a new control flow mechanism during variable assignment that can be used to make error handling less verbose, but is not exclusive to error handling.
If so, how does this differ from previous error handling proposals?
From spec: error handling meta issue #40432, seems similar to those with this classification: 'Special characters, often ! or ?, that insert an error check in a function call or assignment.', but none of the ones I could find are as dead simple.
The text was updated successfully, but these errors were encountered: