cmd/compile: compiler does vet things

[zigo101]

What version of Go are you using (go version)?

$ go version
go version go1.19rc2 linux/amd64

Also for many previous versions.

Does this issue reproduce with the latest release?

Yes

What did you do?

package main

func main() {}

func foo() (err error) {
	{
		err := bar()
		if err != nil {
			return
		}
	}
	
	return
}

func bar() error {
	return nil
}

What did you expect to see?

Complies okay.

What did you see instead?

Fails to compile.

$ go run main.go
# command-line-arguments
./main.go:9:4: result parameter err not in scope at return
	./main.go:7:3: inner declaration of var err error

[seankhliao]

Duplicate of #5425

https://go.dev/ref/spec#:~:text=functions%20are%20executed.-,Implementation%20restriction%3A,-A%20compiler%20may

Implementation restriction: A compiler may disallow an empty expression list in a "return" statement if a different entity (constant, type, or variable) with the same name as a result parameter is in scope at the place of the return.

[zigo101]

This breaks expectations and does prevent some valid code from compiling.

[zigo101]

I plead to reopen this issue, for three reasons:

1. go vet is also able to catch this.
2. gc doesn't catch all similar such cases.
3. I recently wrote several such (expected) valid code, but which is rejected by gc. The following is a simplified example (there are more expected-valid variants):

package main

import "fmt"

func g() (int, error) {
	return 0, fmt.Errorf("not implemented")
}

func f() (err error) {
	if n, err := g(); err == nil {
		return
	} else {
		return fmt.Errorf("%w: %d", err, n)
	}
}

func main() {
	fmt.Println(f())
}

Just let compilers do compiler things is a good philosophy.

@griesemer @ianlancetaylor

[ianlancetaylor]

@go101 As @seankhliao said, the language spec explicitly calls this out as an implementation restriction. The implementation restriction was added to the tools in https://go.dev/cl/5245056. That predates the proposal process; it was part of the set of changes made for the release of Go 1. You can see it mentioned at https://docs.google.com/document/pub?id=1ny8uI-_BHrDCZv_zNBSthNKAMX_fR_0dc6epA6lztRE which is linked from the blog entry https://go.dev/blog/go1-preview. I don't think there is any new information since then, so I don't think we are going to change that decision.

[zigo101]

The problem is the restriction might do false positive things, which is okay for govet, but is weird for go compilers.

Personally, I don't get why this warning is any more harmful than the other warnings reported by govet. So I don't understand why it is promoted to the compiler level.

there is any new information since then

If this breaks user expectations and causes unhappy user experiences is not new information ...

[zigo101]

BTW, there is a poll: https://twitter.com/go100and1/status/1578997940487876608, which shows 2/3 voters think it should compile.

[ianlancetaylor]

The compiler gives an error on this code because we observed that people thought they could use a naked return to return an error, and didn't realize that the named err result parameter was being shadowed by a locally declared err variable. People were writing that kind of code in practice, but it was never what they wanted.

It's true that we might make a different decision today, but I think that given that the compiler has acted this way for 10 years, we need a really strong reason to change it. And "weird for Go compilers" is a valid reason, but it's not a strong reason.

[zigo101]

we observed that people thought they could us .... but it was never what they wanted.

This is also true for many cases reported by vet.

And this decision was made before 1.0, so I doubt the validity of the reason. At least for me, it is not true.

Personally, I think this is a “draw legs on a snake” thing, because it breaks user expectations, which is a strong enough reason.