I would be against this change, for a few reasons.

1) 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.

2) 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?

awnumar on 4 Aug 2017

👎59 👍30

@awnumar for 2 this would pair nicely with #21182

jimmyfrasche on 4 Aug 2017

👍2

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.)

dominikh on 4 Aug 2017

👍127 👎10 😕1

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.

josharian on 4 Aug 2017

👍12 👎5 😕4

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 on 4 Aug 2017

👎66 👍26

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 on 4 Aug 2017

❤1

@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.

bcmills on 4 Aug 2017

@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.

dominikh on 4 Aug 2017

👍25

@bcmills @dominikh right, silly me. Mental lapse; thanks for setting me straight.

josharian on 4 Aug 2017

😄1

A func without return values should still be able to "bare" return:

func noReturn() {
    if !someCondition() {
        return // bail out
    }
    // Happy path
}

leonklingele on 16 Aug 2017

👍18

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:

https://github.com/golang/go/blob/2d69e9e259ec0f5d5fbeb3498fbd9fed135fe869/src/net/http/server.go#L3158-L3166

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.

dmitshur on 12 Sep 2017

👍97 👎2

@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 on 12 Sep 2017

👍3

This is probably relevant:

https://blog.minio.io/golang-internals-part-2-nice-benefits-of-named-return-values-1e95305c8687

And the associated discussion:

https://news.ycombinator.com/item?id=14668323

awnumar on 12 Sep 2017

👎4

From the article:

Note that if you don’t like or prefer the naked return that Golang offers, you can use return oi while still getting the same benefit, like so:

Named return values are great! Bare return of named return values are the problem. :-)

carlmjohnson on 12 Sep 2017

👍8

@awnumar, that is not relevant. See my comment on Hacker News: https://news.ycombinator.com/item?id=14668595

bradfitz on 12 Sep 2017

👍3

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.

pciet on 31 Jan 2018

👍2

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 on 27 Apr 2018

👍3 👎2

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?

dmitshur on 27 Apr 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 on 27 Apr 2018

👎1

@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.

nhooyr on 7 May 2018

👍7 👎1

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 on 7 May 2018

👍9 👎1

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 on 6 Jun 2018

@tandr I think returning a small struct object is much more clear than having more than three return values.

ggicci on 30 Aug 2018

👎1 👍1

Alternative to a purely bare return, although debated and abandoned in #21182 in favor of #19642:

func f() (e error) {
   return ... // ellipsis trigram
}

networkimprov on 25 Sep 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 on 11 Oct 2018

👍1

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.

agnivade on 12 Oct 2018

👍3

@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.

nhooyr on 12 Oct 2018

@agnivade five of your six returns should evaporate under the forthcoming Go2 error handling scheme :-)

28160 has been closed, which seems to me to close the door on fixing this in Go1.

kf6nux on 17 Oct 2018

Well, this kind of change was never going to happen in Go 1 anyhow, no matter how one approached it.

ianlancetaylor on 17 Oct 2018

👍2

Maybe, maybe, the problem is in :=, and we should have proposal to remove it.

shuLhan on 28 Nov 2018

👎6 😄2

This isn't just about shadowing. It's about making the reader carry more context than necessary.

Using the code example here: https://github.com/golang/go/issues/21291#issuecomment-320120637

In the first example, the reader knows exactly what's returned with only having to trace the value of err.

In the second example, the reader would need to trace the values of three additional variables.

Put another way

if err != nil {
    return err
}
...
return err

vs

if err != nil {
    return err
}
...
return nil

(returning a variable with a value nil vs returning nil)

Which do you prefer?

I prefer the latter because it's explicit and easier to understand at first glance. Removing bare returns would encourage explicit return values instead of encouraging return values that must be deduced.

kf6nux on 28 Nov 2018

👍7

Another example of the cognitive burden is that you can't tell a void function (func(arg T)) from a non-void function (func(arg T) ret R) when you look at a bare return. It could be that the function has no return values or it could be that it has named values that are being returned. You have to remember the function signature to know what the return actually means. Not a huge issue, to be sure, but it's one more thing to keep in your head.

carlmjohnson on 3 Dec 2018

👍5

Bare return statements can be confusing in some cases. But there are also specific cases where they are useful. And, of course, they work today, which is a strong argument for keeping them.

The best solution here is likely to keep bare returns in the language, but consider adding a lint check that warns about using them in complex cases in which shadowing occurs.

ianlancetaylor on 29 Jan 2019

👍2

This is my exact thought when start reading a Go code base.
The short version should be nice for hackers who want to code fast, do trick with the syntax here and there and poison the victim computer.
But in term of enterprise application, few keystrokes saving may cost you millions to detect

Edit: I want to clarify that my suggestion is disable bare return for functions that return value and leave bare return for void function as it is.

manhquyet-nguyen on 19 Mar 2019

😕1

So I encountered some sample code with no return at all from 2017. With go 1.12.5 (windows/amd64), this is flagged on go build as a missing return. Is this the same issue? Were implicit returns allowed in the past and now invalid? I couldn't fix it with a bare return. The type was uintptr so I returned 0 (nill would not lint).

datatribe on 31 May 2019

@datatribe, I can't think of any change that would change that. It has ways been an error to miss a return. File a new bug with the details?

bradfitz on 31 May 2019

@datatribe, I can't think of any change that would change that. It has ways been an error to miss a return. File a new bug with the details?

It's entirely possible the example code was no good. Thanks @bradfitz

datatribe on 31 May 2019

I'm not sure if it causes production bugs, but for me, when I see naked return, it usually makes me pause to try to figure out what's happening. I get paranoid about shadowed err working in an unexpected way because I'm not 100% confident in my understanding of how they interact. It makes reading the code much slower.

I agree with you on that. It makes me pause to try to figure out what values are assigned to the named parameter variables. My discomfort reading (especially long) functions with naked returns is that I can't follow easily what are actually returned. Each naked return actually means return p1, p2, ..., pn for named parameters, but sometimes the values of some parameters are obviously literal like return nil, nil, return nil, err, return "", err and return p, nil rather than return p, err. If I write return nil, err, err still might be nil value, but it's clear that the first return value is nil. If naked return is disallowed, the writer would more likely to write literal values in return statement if that's possible. Actually they all did use literal values, when I asked them to avoid naked returns during the code reviews.

When writers write in naked returns, they tend not to think about what are returned. When I asked them to avoid naked returns, now they think about the actual value they want to return and notice that they were returning wrong values like partially constructed values. Isn't being explicit better than implicit?

I know it's recommended to indent error flows but not everyone follows this pattern in reality, especially folks who like to write long functions with lots of naked returns. Sometimes, it's hard to know if it's in an error flow or not by looking many naked returns. I read the code carefully decode it and convert naked returns to non-naked with some literal values, then it's much easier to read. (sometimes it's not possible to figure out the literal values if the code is complex)

I know the documentation comment block mentions what it returns for some meaningful conditions, but not everyone does that. With return statement with some literal values, I can follow that easily by looking at the code.

I know we can use named parameters and non-naked returns together, but not everyone knows that. I often see something like the following, and I think return io.EOF is better.

err = io.EOF
return

That said, I think #28160 is probably a better fix since it's backward compatible and gofmt has already had several minor changes.

I disagree on the other proposal that gofmt should put them back, even if that's gofmt's job. That's because just putting bunch of return p1, ..., pn mechanically won't make the code more understandable.

It's possible that lint tools can check it (nakedret does that), but IMO, naked return has more harm than benefits. I also agree that the benefit of removing naked return may be smaller than the harm breaking the compatibility. But I guess it's easy to fix with go fix even if mechanical go fix won't make the code readable.

Go is very clear and readable language in my opinion. And naked return is a small part of the language that makes me hard to understand the code.

jaeyeom on 25 Jun 2019

👍3

Change https://golang.org/cl/189778 mentions this issue: cmd/go/internal/get: propagate parse errors in parseMetaGoImports

gopherbot on 10 Aug 2019

I may be in the minority, but I'd actually like to propose the opposite. Functions with named returns should always use bare return. Otherwise, it just looks like the code is lying. You assign values and then potentially silently overwrite them upon return. Here is a bad piece of code to illustrate what I mean,

func hello() (n, m int) {
    n = 2
    m = 3
    return m, n
}

If you'd just use a bare return there, the code makes sense again.

millerlogic on 9 Dec 2019

👍3 👎2 😕1

@millerlogic I think that would be far too agressive. For example, there's nothing wrong with code like:

func documentedReturns() (foo, bar string, _ error) {
    [...]
    if err != nil {
        return "", "", fmt.Errorf(...)
    }
}

I also think your point might fit best as a separate counter-proposal, since you're suggesting the opposite of what the current proposal brings forward.

mvdan on 15 Dec 2019

👍2

@mvdan Good point, I think there is a middle ground which doesn't involve demonizing them. I think I read an idea similar to: not explicitly returning a value if you previously assigned to the return variable. It might even be implemented somewhere, but I never hit it. So that would mean my hello example would error without a naked return, and your example would succeed because foo or bar weren't assigned to.

millerlogic on 16 Dec 2019

I think these should be removed from the tour of go, that way newer go developers aren't encouraged to use these. In practice they're just a cognitive tax. The few I've seen always required my brain to say "wait what?!...oh yeah, naked returns and named return values" I understand maybe leaving them in the tour so everyone knows, but there's an entire page dedicated to them.

dustinevan on 23 May 2020

👍2

Go: proposal: Go 2: remove bare return

Most helpful comment

All 52 comments

28160 has been closed, which seems to me to close the door on fixing this in Go1.

Related issues