Join GitHub today
GitHub is home to over 28 million developers working together to host and review code, manage projects, and build software together.
Sign upproposal: Go 2: remove bare return #21291
Comments
gopherbot
added this to the Proposal milestone
Aug 3, 2017
gopherbot
added
the
Proposal
label
Aug 3, 2017
ALTree
added
the
Go2
label
Aug 3, 2017
dsnet
added
the
LanguageChange
label
Aug 3, 2017
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
awnumar
Aug 3, 2017
Contributor
I would be against this change, for a few reasons.
-
It's a relatively significant syntactic change. This could cause confusion with developers and contribute to leading Go into the same mess that Python went through.
-
I do not think that they're useless, why are they more trouble than they're worth?
return nils all over the place is a little verbose, don't you think?
|
I would be against this change, for a few reasons.
|
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
dominikh
Aug 3, 2017
Member
return nils all over the place is a little verbose, don't you think?
Simplicity beats verbosity. A bunch of return nil are a lot easier to understand than a bunch of return where one has to chase the latest value of the variable and make darn sure it hasn't been shadowed (intentionally or by mistake.)
Simplicity beats verbosity. A bunch of |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
josharian
Aug 3, 2017
Contributor
Note that naked returns allow you to do one thing you cannot otherwise accomplish: Change a return value in a defer. Quoting the code review comments wiki:
Finally, in some cases you need to name a result parameter in order to change it in a deferred closure. That is always OK.
A complete proposal to eliminate naked returns should explain how such uses should be written instead and why the alternative form is preferable (or at least acceptable). The obvious rewrites tend to involve lots of boilerplate and repetition, and while they usually involve error handling, the mechanism is general.
|
Note that naked returns allow you to do one thing you cannot otherwise accomplish: Change a return value in a defer. Quoting the code review comments wiki:
A complete proposal to eliminate naked returns should explain how such uses should be written instead and why the alternative form is preferable (or at least acceptable). The obvious rewrites tend to involve lots of boilerplate and repetition, and while they usually involve error handling, the mechanism is general. |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
tandr
Aug 3, 2017
I don't think this
func oops() (string, *int, string, error) {
...
return "", nil, "", &SomeError{err}
}is more readable than
func oops() (rs1 string, ri *int, rs2 string, e error) {
...
e = &SomeError{err}
return
},sorry.
tandr
commented
Aug 3, 2017
•
|
I don't think this func oops() (string, *int, string, error) {
...
return "", nil, "", &SomeError{err}
}is more readable than func oops() (rs1 string, ri *int, rs2 string, e error) {
...
e = &SomeError{err}
return
},sorry. |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
tandr
Aug 3, 2017
OTOH - yes, shadowing sucks. It would not make sense to outlaw it outright (generated code comes to mind), but I would gladly vote for at least prohibition of return values names shadowing. Oh, and make compiler generate some diagnostic in all other cases would be nice too :)
Edit: apparently there is #377 that talks about it
tandr
commented
Aug 3, 2017
•
|
OTOH - yes, shadowing sucks. It would not make sense to outlaw it outright (generated code comes to mind), but I would gladly vote for at least prohibition of return values names shadowing. Oh, and make compiler generate some diagnostic in all other cases would be nice too :) Edit: apparently there is #377 that talks about it |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
bcmills
Aug 4, 2017
Member
@josharian I was assuming that deferred functions could still modify the return variables. That semantic seems more or less orthogonal to the syntax of the return statement.
|
@josharian I was assuming that deferred functions could still modify the return variables. That semantic seems more or less orthogonal to the syntax of the return statement. |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
dominikh
Aug 4, 2017
Member
@josharian You need named return parameters to be able to modify them in a deferred function. You do not need to use a naked return for that. return with explicit values will copy the values into the named return values before the deferred functions run.
|
@josharian You need named return parameters to be able to modify them in a deferred function. You do not need to use a naked return for that. return with explicit values will copy the values into the named return values before the deferred functions run. |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
leonklingele
Aug 16, 2017
Contributor
A func without return values should still be able to "bare" return:
func noReturn() {
if !someCondition() {
return // bail out
}
// Happy path
}|
A func without return values should still be able to "bare" func noReturn() {
if !someCondition() {
return // bail out
}
// Happy path
} |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
dmitshur
Sep 12, 2017
Member
I spent 1.5 minutes looking at the following Go code in Go standard library (from 4 years ago, /cc @bradfitz), in disbelief, thinking there might be a bad bug:
Lines 3158 to 3166 in 2d69e9e
Specifically, this part:
tc, err := ln.AcceptTCP()
if err != nil {
return
}At first glance, it looked to me that on the first line there, new variables tc and err were being declared using short variable declaration syntax, distinct from the named err error return variable. Then, it looked to me that the bare return was effectively returning nil, nil rather than than nil, err as it should've been.
After about 90 seconds of thinking very hard about it, I realized that it's actually correct code. Since the named err error return value is in the same block as the function body, the short variable declaration tc, err := ... only declares tc as a new variable but does not declare a new err variable, so the named err error return variable is being set by the call to ln.AcceptTCP(), so the bare return actually returns a non-nil error as it should.
(Had it been in a new block, it would actually be a compile error "err is shadowed during return", see here.)
I think this would've been much more clear and readable code:
tc, err := ln.AcceptTCP()
if err != nil {
return nil, err
}I wanted to share this story because I think it's a good example of bare returns wasting programmer time. In my opinion, bare returns are marginally easier to write (just saving a few keystrokes), but often lead to code that's harder to read compared to equivalent code that uses full returns (non-bare). Go usually does the right thing of optimizing for reading, since that's done much more often (by more people) than writing. It seems to me that the bare returns feature tends to lower readability, so it might be the case that removing it in Go 2 would make the language better.
Disclaimer: In Go code I write and read most often, I tend to avoid having bare returns (because I think they're less readable). But that means I have less experience reading/understanding bare returns, which might negatively influence my ability to read/parse them. It's a bit of catch-22.
|
I spent 1.5 minutes looking at the following Go code in Go standard library (from 4 years ago, /cc @bradfitz), in disbelief, thinking there might be a bad bug: Lines 3158 to 3166 in 2d69e9e Specifically, this part: tc, err := ln.AcceptTCP()
if err != nil {
return
}At first glance, it looked to me that on the first line there, new variables After about 90 seconds of thinking very hard about it, I realized that it's actually correct code. Since the named (Had it been in a new block, it would actually be a compile error "err is shadowed during return", see here.) I think this would've been much more clear and readable code: tc, err := ln.AcceptTCP()
if err != nil {
return nil, err
}I wanted to share this story because I think it's a good example of bare returns wasting programmer time. In my opinion, bare returns are marginally easier to write (just saving a few keystrokes), but often lead to code that's harder to read compared to equivalent code that uses full returns (non-bare). Go usually does the right thing of optimizing for reading, since that's done much more often (by more people) than writing. It seems to me that the bare returns feature tends to lower readability, so it might be the case that removing it in Go 2 would make the language better. Disclaimer: In Go code I write and read most often, I tend to avoid having bare returns (because I think they're less readable). But that means I have less experience reading/understanding bare returns, which might negatively influence my ability to read/parse them. It's a bit of catch-22. |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
creker
Sep 12, 2017
@shurcooL, interesting example that threw me off as well. For me it was the fact that there're two connection variables tc and c. I thought you made a mistake when copy-pasting the code and instead of c there should be tc. It took me about the same time to realize what that code does.
creker
commented
Sep 12, 2017
•
|
@shurcooL, interesting example that threw me off as well. For me it was the fact that there're two connection variables |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
awnumar
Sep 12, 2017
Contributor
This is probably relevant:
https://blog.minio.io/golang-internals-part-2-nice-benefits-of-named-return-values-1e95305c8687
And the associated discussion:
|
This is probably relevant: https://blog.minio.io/golang-internals-part-2-nice-benefits-of-named-return-values-1e95305c8687 And the associated discussion: |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
carlmjohnson
Sep 12, 2017
Contributor
From the article:
Note that if you don’t like or prefer the naked return that Golang offers, you can use
return oiwhile still getting the same benefit, like so:
Named return values are great! Bare return of named return values are the problem. :-)
|
From the article:
Named return values are great! Bare return of named return values are the problem. :-) |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
bradfitz
Sep 12, 2017
Member
@awnumar, that is not relevant. See my comment on Hacker News: https://news.ycombinator.com/item?id=14668595
|
@awnumar, that is not relevant. See my comment on Hacker News: https://news.ycombinator.com/item?id=14668595 |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
pciet
Jan 31, 2018
Contributor
I think removing these would follow the approach of explicit error handling. I don’t use them.
Code I’ve reviewed on golang-nuts with bare returns isn’t difficult to understand, but parsing variable scope is an unnecessary added effort for readers.
|
I think removing these would follow the approach of explicit error handling. I don’t use them. Code I’ve reviewed on golang-nuts with bare returns isn’t difficult to understand, but parsing variable scope is an unnecessary added effort for readers. |
ianlancetaylor
changed the title from
proposal: Go 2: Remove bare return
to
proposal: Go 2: remove bare return
Mar 7, 2018
ianlancetaylor
added
the
NeedsDecision
label
Mar 7, 2018
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
kelwang
Apr 27, 2018
I really love bared return, especially when a function has multiple returns. It’s just making the code much cleaner. The biggest reason, I chose to work with go.
Go tool vet shadow can detect potential shadowed vars. If a func has less cyclomatic complexity, plus some test cases. I couldn’t see it will get any trouble. I could be wrong, but I wish to see some examples to demonstrate how bad bared return could be.
kelwang
commented
Apr 27, 2018
|
I really love bared return, especially when a function has multiple returns. It’s just making the code much cleaner. The biggest reason, I chose to work with go. Go tool vet shadow can detect potential shadowed vars. If a func has less cyclomatic complexity, plus some test cases. I couldn’t see it will get any trouble. I could be wrong, but I wish to see some examples to demonstrate how bad bared return could be. |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
dmitshur
Apr 27, 2018
Member
I wish to see some examples to demonstrate how bad bared return could be.
@kelwang Did you see my example about ln.AcceptTCP() from 7 comments above?
@kelwang Did you see my example about |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
kelwang
Apr 27, 2018
Hi, @shurcooL, thx
Yeah, I think you made a great point.
But like you said, it's been 4 years. I have a feeling maybe you already get used to it.
I think it's not really an issue for bared return. But a confusion in multiple vars initialization.
For me, the shadow vet tool usually works very well. I never really worry about that.
Maybe we should fill a ticket in go linter to avoid those kind of confusion. whether rename the err, or declare tc on top first. I feel a linter suggestion should be good enough.
kelwang
commented
Apr 27, 2018
|
Hi, @shurcooL, thx Yeah, I think you made a great point. I think it's not really an issue for bared return. But a confusion in multiple vars initialization. For me, the shadow vet tool usually works very well. I never really worry about that. |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
nhooyr
May 6, 2018
Contributor
@kelwang In my opinion, if a function has multiple returns to the point where the return statements are getting ugly, a struct/pointer to a struct should be returned instead over a bare return.
|
@kelwang In my opinion, if a function has multiple returns to the point where the return statements are getting ugly, a struct/pointer to a struct should be returned instead over a bare return. |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
pam4
May 7, 2018
Since #377 has been mentioned, I would argue that the source of confusion in the ln.AcceptTCP example is more about the magic behind := rather than the bare return itself.
I think the ln.AcceptTCP case wouldn't be so bad with a more explicit form of short declaration (proposed in the referenced issue):
func (ln tcpKeepAliveListener) Accept() (c net.Conn, err error) {
:tc, err = ln.AcceptTCP()
if err != nil {
return
}
// ...
}By just looking at a multi-variable := you can't tell which variables are being declared: you need to take into account all the preceding part of the block to know that.
An oversight about where the block boundary is, and you may end up with a hard to find bug.
Also you can't fix a multi-variable := to make it declare exactly what you want; you are forced to give up the short declaration or reorganize the code.
I've seen many proposal trying to address specific consequences of this, but I think the root of the problem is just the lack of explicitness of := (I would also argue that, whenever shadowing is considered a pitfall, it is really just multi-variable :='s fault).
I'm not saying that bare returns are necessarily worth it, I'm just saying that what we are seeing is a compound problem.
pam4
commented
May 7, 2018
•
|
Since #377 has been mentioned, I would argue that the source of confusion in the I think the func (ln tcpKeepAliveListener) Accept() (c net.Conn, err error) {
:tc, err = ln.AcceptTCP()
if err != nil {
return
}
// ...
}By just looking at a multi-variable I've seen many proposal trying to address specific consequences of this, but I think the root of the problem is just the lack of explicitness of I'm not saying that bare returns are necessarily worth it, I'm just saying that what we are seeing is a compound problem. |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
spencerpomme
Jun 6, 2018
Regardless of the readability arguments, I think the bare return is less logically consistent and elegant. It's somehow a shaky standpoint. Although, obviously I'm not the only one being subjective here.
spencerpomme
commented
Jun 6, 2018
|
Regardless of the readability arguments, I think the bare return is less logically consistent and elegant. It's somehow a shaky standpoint. Although, obviously I'm not the only one being subjective here. |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
ggicci
Aug 30, 2018
@tandr I think returning a small struct object is much more clear than having more than three return values.
ggicci
commented
Aug 30, 2018
|
@tandr I think returning a small struct object is much more clear than having more than three return values. |
mvdan
referenced this issue
Sep 25, 2018
Closed
Proposal: Go 2: make use of return values mandatory #27845
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
networkimprov
commented
Sep 25, 2018
•
deanveloper
referenced this issue
Oct 11, 2018
Closed
proposal: Go2: remove from spec implicitly returned parameters #28161
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
kf6nux
Oct 11, 2018
While I'm in favor of removing it in Go2, I made #28160 to (essentially) remove it in Go1 via gofmt. Folks comments there would be appreciated!
kf6nux
commented
Oct 11, 2018
|
While I'm in favor of removing it in Go2, I made #28160 to (essentially) remove it in Go1 via |
carlmjohnson
referenced this issue
Oct 11, 2018
Open
proposal: gofmt should add named return variables to empty return statements #28160
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
agnivade
Oct 12, 2018
Member
I have found naked returns to be surprisingly helpful in DB layer.
This is a real redacted production code -
func (p *PostgresStore) GetSt() (st St, err error) {
var tx *sql.Tx
var rows *sql.Rows
tx, err = p.handle.Begin()
if err != nil {
return
}
defer func() {
if err != nil {
tx.Rollback()
} else {
tx.Commit()
}
rows.Close()
}()
rows, err = tx.Query(`...`)
if err != nil {
return
}
// handle rows
for rows.Next() {
var all Call
err = rows.Scan(&all.Email, &all.Count)
if err != nil {
return
}
st.Today = append(st.Today, &all)
}
rows.Close()
rows, err = tx.Query(`...`)
if err != nil {
return
}
// handle rows
for rows.Next() {
var all Call
err = rows.Scan(&all.Email, &all.Count)
if err != nil {
return
}
st.Books = append(st.Books, &all)
}
return
}There are 6 return statements here. (Actually there are 2 more, I just shortened the code for brevity). But my point is when an error occurs, the caller will only inspect the err variable. If I just had to write return err, I would still be okay, but to match the return signature, I have to write return st, err over and over again. And this grows linearly if you have more variables to return; your return statement keeps growing bigger and bigger.
Whereas, if there are named return params, I just call return, knowing that it will also return any other variables in whatever state they were. This makes me happy and I consider it to be a great feature.
|
I have found naked returns to be surprisingly helpful in DB layer. This is a real redacted production code - func (p *PostgresStore) GetSt() (st St, err error) {
var tx *sql.Tx
var rows *sql.Rows
tx, err = p.handle.Begin()
if err != nil {
return
}
defer func() {
if err != nil {
tx.Rollback()
} else {
tx.Commit()
}
rows.Close()
}()
rows, err = tx.Query(`...`)
if err != nil {
return
}
// handle rows
for rows.Next() {
var all Call
err = rows.Scan(&all.Email, &all.Count)
if err != nil {
return
}
st.Today = append(st.Today, &all)
}
rows.Close()
rows, err = tx.Query(`...`)
if err != nil {
return
}
// handle rows
for rows.Next() {
var all Call
err = rows.Scan(&all.Email, &all.Count)
if err != nil {
return
}
st.Books = append(st.Books, &all)
}
return
}There are 6 return statements here. (Actually there are 2 more, I just shortened the code for brevity). But my point is when an error occurs, the caller will only inspect the Whereas, if there are named return params, I just call return, knowing that it will also return any other variables in whatever state they were. This makes me happy and I consider it to be a great feature. |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
nhooyr
Oct 12, 2018
Contributor
@agnivade not sure if this is because its redacted but it looks like your production code would panic in the defer if rows is nil which would occur if the query failed.
|
@agnivade not sure if this is because its redacted but it looks like your production code would panic in the defer if rows is nil which would occur if the query failed. |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
networkimprov
Oct 12, 2018
@agnivade five of your six returns should evaporate under the forthcoming Go2 error handling scheme :-)
More on the feedback wiki.
networkimprov
commented
Oct 12, 2018
|
@agnivade five of your six returns should evaporate under the forthcoming Go2 error handling scheme :-) More on the feedback wiki. |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
agnivade
Oct 12, 2018
Member
@nhooyr - Oh dear .. looks like we are doing a prod push today ..
@networkimprov - That's true. I hadn't thought of the new error handling. Personally, I would be okay with this when the new error handling lands. But even then I think this is a pretty major change which is bound to break a lot of codebases. Arguably, we can fix this with automated tooling. But I wonder how many of these major changes should be make for Go 2.
|
@nhooyr - Oh dear .. looks like we are doing a prod push today .. @networkimprov - That's true. I hadn't thought of the new error handling. Personally, I would be okay with this when the new error handling lands. But even then I think this is a pretty major change which is bound to break a lot of codebases. Arguably, we can fix this with automated tooling. But I wonder how many of these major changes should be make for Go 2. |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
kf6nux
Oct 12, 2018
Go2 is given license to not be backward compatible with Go1, is it not?
I agree that tooling could be made to upgrade existing Go1 code to be compatible with this change in Go2.
kf6nux
commented
Oct 12, 2018
•
|
Go2 is given license to not be backward compatible with Go1, is it not? I agree that tooling could be made to upgrade existing Go1 code to be compatible with this change in Go2. |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
bradfitz
Oct 12, 2018
Member
Go2 is given license to not be backward compatible with Go1, is it not?
It's almost certain we won't use said license. Breaking backwards compatibility is incredibly difficult and risky ecosystem-wise.
It's almost certain we won't use said license. Breaking backwards compatibility is incredibly difficult and risky ecosystem-wise. |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
kf6nux
Oct 12, 2018
If breaking changes aren't currently planned, I'd agree this proposal isn't worth causing a break. I hope we consider the proposal if/when breaking changes are planned. Updating go fix to accommodate this proposal should be trivial.
kf6nux
commented
Oct 12, 2018
|
If breaking changes aren't currently planned, I'd agree this proposal isn't worth causing a break. I hope we consider the proposal if/when breaking changes are planned. Updating |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
networkimprov
Oct 12, 2018
New keywords are on the table for Go2 error handling, and that would break some code.
networkimprov
commented
Oct 12, 2018
|
New keywords are on the table for Go2 error handling, and that would break some code. |
carlmjohnson commentedAug 3, 2017
(I couldn't find an existing issue for this, so please close if this is a duplicate and I missed it.)
I propose getting rid of bare returns. Named return values are great, keep those. Bare returns are more trouble than they are worth, eliminate them.