Go: cmd/compile: mention shadowing of predeclared identifiers in errors they cause

[seebs]

(Related: #31064, #14494)

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

1.12, but N/A

Does this issue reproduce with the latest release?

I believe so?

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

N/A

What did you do?

Horrible monstrous things. Examples:

const iota = 0
type int struct{}
var nil = "haha"

Okay, fine, those are stupid. But you know what I've done unintentionally?

...
len := a -b
...
for i := 0; i < len(x); i++ {
...

and then get confused by the weird error message about len not being a function.

What did you expect to see?

I think redeclaring predeclared identifiers should be an error. Or, since that's probably impractical, it should be a thing in go vet.

What did you see instead?

Really weird and misleading error messages when I forget that something is a predeclared identifier. I've hit this most often with len, because that's a natural variable name and when I'm naming something that I'm usually not thinking about the function.

[bcmills]

See also #31064 and #21606.

[robpike]

This particular case of shadowing is no different in kind from the general issue. We should be able to close this in deference to the ones linked above.

[rsc]

cmd/vet/README lists the requirements for a vet check.

The other two issues seem like clear declines to me on correctness/precision. They reject far too much good Go code. This proposal is different. It is unlikely to reject good Go code but seems unlikely to catch many bugs either, for the simple reason that hardly any Go programmers reuse predeclared identifiers. So it seems like this one fails on frequency.

There's still some correctness/precision argument too. Go has some obscure identifiers that are fine to reuse in your program, especially as a local variable. It's easy to see writing identifiers like imag or close or cap as a local variable in a short function and that's completely fine. It would be unfortunate for vet to reject that code.

Predeclared identifiers are not reserved words in Go precisely so that the set can expand without invalidating old programs and so that it can have some obscure entries without forcing developers to learn a list of "bad words" to avoid. Adding a vet check would undo much of that flexibility.

[seebs]

Empirically, usually if I unintentiontally shadow len, I find out very quickly, but the errors can be very confusing, because the real problem is "you shadowed a predeclared identifier and then tried to use the predeclared identifier without noticing this", and the diagnostic is always unrelated.

I wonder if it'd be sane/practical to track whether a given symbol shadows another symbol, and if so, and it's used in a diagnostic, point that out. That might be a saner approach that won't break real code, and would almost certainly be of use to developers who have been awake a bit too long.

[rsc]

The other problem with trying to use vet to catch the error in the example at the top of this issue is that vet only works on code that compiles, and that code does not.

For this specific case it seems like cmd/compile and go/types should give a more helpful error message when the problem seems to be caused by shadowing. Will retitle.

[rsc]

As a compiler error this no longer needs to be a proposal.

[rsc]

(Probably should happens in go/types too so it shows up in gopls etc.)