cmd/vet: warn about unused struct or array, ignoring assignment to field or element

[gopherbot]

by RyanneDolan:

What steps will reproduce the problem?

v := new(struct{i int})
v.i = 1

What is the expected output? What do you see instead?

Should get "v declared and not used" error.  Instead, compiles fine.

What is your $GOOS?  $GOARCH?

linux/386

Which revision are you using?  (hg identify)

54392bd03d06 tip

Please provide any additional information below.

The dot operator should mark its left operand as "used" only when it 
appears on the RHS.

a = 1    // 'a' is not marked as used (CORRECT)
v.i = 1  // 'v' is marked as used (WRONG)
a = v.i  // 'v' is marked as used (CORRECT)

This bug is important to fix for consistency, and because it confuses new 
programmers who might expect reference semantics:

for _,v := range arr {v.i = 1} // doesn't do anything

Mentioned here:

http://groups.google.com/group/golang-
nuts/browse_thread/thread/342fe61c78916496/fbb57f693f1aca72

[rsc]

Comment 1:

Your first example is definitely too strict.
You suggest disallowing:
v := new(Point)
v.x = 1
but what about
v := NewPoint()
v.x = 1
or
v := OldPoint()  // returns a reference shared with others
v.x = 1
The used and not set rules do not treat "new" specially.
"v" is being used to get to the field v.x in all these
examples.  In particular, when v is a pointer,
    for _, v = range arr { v.i = 1 }
is a perfectly legitimate program.
It is only when v is a struct (not a pointer) that this
matters.  It might be reasonable to treat v.i = 1, when
v is not a pointer, only as a write to v, not a read.
I thought the specific rule was in the spec but I can't
find it at the moment.

Labels changed: added languagechange.

Owner changed to r...@golang.org.

Status changed to Thinking.

[gopherbot]

Comment 2 by RyanneDolan:

> v := OldPoint()  // returns a reference shared with others
> v.x = 1
Good point.  My example is still logically incorrect I think, but not a compile 
error.  Sorry.
A revised example (as per your comment) for the record:
var v struct{i int}
v.i = 1
Interestingly, I think
  for _, v = range arr { v.i = 1 }
illuminates a problem with adding generics to Go: the same code can have value or 
reference semantics depending on the underlying types.  I guess that is what confused 
me, and as I said above, confuses new Go programmers.
Thanks for considering the issue.

[rsc]

Comment 3:

Labels changed: added priority-later.

[rsc]

Comment 5:

Labels changed: added go2.

[rsc]

Comment 6:

Labels changed: added repo-main.

[rsc]

Comment 7:

Labels changed: added release-none.

[rsc]

To be clear, this is now only about:

var j int
j = 1

var v struct { i int }
v.i = 1

var x [2]int
x[0] = 1

The compiler reports that j is declared and not used, but it fails to report that both v and x are declared and not used.

The spec says "Implementation restriction: A compiler may make it illegal to declare a variable inside a function body if the variable is never used."

I think that spec wording can be read as applying to all of j, v, and x, but it could break existing programs, hence the LanguageChange and Go2 tags.

[ianlancetaylor]

In the current Go ecosystem, this makes sense as a vet change. Let's repurpose this issue to adding this check to vet, rather than worrying about changing the compiler or language.