Skip to content
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

cmd/compile: the compiler should probably recognize runtime.Goexit #37193

Open
perillo opened this issue Feb 12, 2020 · 4 comments
Open

cmd/compile: the compiler should probably recognize runtime.Goexit #37193

perillo opened this issue Feb 12, 2020 · 4 comments
Labels
Milestone

Comments

@perillo
Copy link

@perillo perillo commented Feb 12, 2020

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

$ go version
go version go1.14rc1 linux/amd64

Does this issue reproduce with the latest release?

Yes.

What operating system and processor architecture are you using (go env)?

go env Output
$ go env
GO111MODULE="on"
GOARCH="amd64"
GOBIN="/home/manlio/.local/bin"
GOCACHE="/home/manlio/.cache/go-build"
GOENV="/home/manlio/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/manlio/.local/lib/go:/home/manlio/src/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/home/manlio/sdk/go1.14rc1"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/home/manlio/sdk/go1.14rc1/pkg/tool/linux_amd64"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/dev/null"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build423783759=/tmp/go-build -gno-record-gcc-switches"
GOROOT/bin/go version: go version go1.14rc1 linux/amd64
GOROOT/bin/go tool compile -V: compile version go1.14rc1
uname -sr: Linux 5.5.2-arch1-1
/usr/lib/libc.so.6: GNU C Library (GNU libc) stable release version 2.31.
gdb --version: GNU gdb (GDB) 8.3.1

What did you do?

https://play.golang.org/p/SZZ4gDss0DK

What did you expect to see?

The program to be compiled.

What did you see instead?

/prog.go:13:1: missing return at end of function

The Go compiler does not know that runtime.Goexit causes the function to exit, so the return statement is unreachable and not required.

Of course there are similar functions like os.Exit, but Goexit is part of the runtime, so it should probably be recognized by the compiler.

On the other hand, vet should know about runtime.Goexit and should report that the statements after Goexit() are not reachable.

The compile or vet commands should also report an error if a function used in the go statement has a non empty result in the signature, but this is a different issue and probably there is a reason why it does not report an error.

@josharian
Copy link
Contributor

@josharian josharian commented Feb 13, 2020

The relevant bit of the spec is https://golang.org/ref/spec#Terminating_statements. I’d personally be reluctant to add runtime.Goexit; everything else in that list is lower level.

It is important for vet to use the same set of terminating statements as the compiler. Otherwise you’ll be forced to add a terminating statement in some cases (like this) to make your code compile but vet will then complain about it.

@go101
Copy link

@go101 go101 commented Feb 14, 2020

The Goexit function really acts as a low level API: #29226

@dmitshur dmitshur added this to the Backlog milestone Feb 15, 2020
@dmitshur
Copy link
Member

@dmitshur dmitshur commented Feb 15, 2020

@griesemer
Copy link
Contributor

@griesemer griesemer commented Feb 18, 2020

The definition of "terminating statements" is based strictly on properties of the language spec (which are unlikely to change), not APIs (which may change).

Also, even if we added runtime.Goexit it wouldn't be satisfactory anyway: It's not unlikely that runtime.Goexit is factored out into a function that does cleanups or perhaps prints additional error information; thus it is that function that will be called (possibly from multiple places) in a program. And unless we propagate the "exit" information to the caller, knowing that runtime.Goexit doesn't return won't be very helpful - it'll save a single return statement. That doesn't seem worth it.

More precisely, runtime.Goexit doesn't terminate the function, it terminates the goroutine that calls it. If we wanted to capture that correctly, we'd need to propagate that up the call stack, with extra rules. I am not convinced this is worth the extra complexity or the mixing of language spec and library APIs.

Not really a proposal, but leaving open for a decision by the proposal review commitee.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
5 participants
You can’t perform that action at this time.