Go: cmd/compile: unreachable "break" after return in switch statement causes erroneous compiler static analysis

[fsufitch]

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

$ go version
1.13.7

(from the Playground)

Does this issue reproduce with the latest release?

Yes.

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

The Go playground at https://play.golang.org

What did you do?

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

The break after a return in a switch/case causes the compiler to think that the wrapping function is missing a return, when in fact it is impossible for the function to break out of the switch.

What did you expect to see?

1

(or 2 or 3 depending on the variable)

What did you see instead?

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

[therealfakemoot]

A reproducing code sample that's even more aggressively minimal: https://play.golang.org/p/MGido5G7mc7

[fsufitch]

This may be a philosophical argument about what constitutes a "terminating statement" in the context of switch. The language spec says a terminating statement is:

A "switch" statement in which:

• there are no "break" statements referring to the "switch" statement,
• there is a default case, and
• the statement lists in each case, including the default, end in a terminating statement, or a possibly labeled "fallthrough" statement.

(https://golang.org/ref/spec#Terminating_statements)

In the code examples above, the switch statement was not a terminating statement because it had a break statement in it referring to it. However, that break statement is itself unreachable, so the switch is unaffected by it, and is functionally a terminating statement (even if it does not fit the spec's strict definition).

In other words, the strict wording of the spec does not support the behavior in this issue being a "bug" per se.

[josharian]

cc @griesemer

[ianlancetaylor]

I think the most important point in this area is that we keep the rules for terminating statements simple, so that all compilers can simply and easily agree as to which statements terminate and which do not. I think that is much more important than handling cases like return; break. Admittedly humans can easily see that the break is not relevant, but writing down simple rules for that is less easy.

Also, return; break is simply not common Go code.

So my preference here would be to do nothing.

[josharian]

Also, vet should catch this.

[fsufitch]

$ go build
./test.go:17:1: missing return at end of function

$ go vet
vet: ./test.go:17:1: missing return

Vet has the same confusion about the missing return that build does. Is that what you meant by vet "catching" it, or could vet give a more helpful error like "line 11 is unreachable"?

[josharian]

Oh right, because vet needs the code to typecheck before it runs.

[griesemer]

I agree with @ianlancetaylor here: Making the rules more complicated just to catch a few extra, possibly pathological, cases does not seem worth it. It is also a slippery slope: What about

func f() int {
   if true {
      return 0
   }
}

(https://play.golang.org/p/_bZ7FxnpNbt)

It's obvious that this function returns, yet it is not covered by the current rules and the compiler will complain. It's not obvious where to stop, once we make the existing rules even just slightly more complicated.

That all said, the current behavior is correct according to the spec and I am closing this as "working as intended".

If you disagree, I encourage you to open a new issue with a concrete proposal regarding the rule change you're envisioning. Thanks.