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: gc inconsistent about reporting "unused variable" errors #8560

Open
adonovan opened this Issue Aug 20, 2014 · 10 comments

Comments

Projects
None yet
@adonovan

adonovan commented Aug 20, 2014

For this program:
  http://play.golang.org/p/BzMO7Vt3uy
  switch x := interface{}(nil).(type) {}
go/types reports an error but gc does not.

I realize the spec does not say what should happen here, but the fact of this
implementation-specific behaviour is that we have two dialects of Go whose tools reject
each others' programs.

We should probably make the tools consistent one way or another.
I would also argue in favour of removing this hole in the spec.
@griesemer

This comment has been minimized.

Contributor

griesemer commented Sep 10, 2014

Comment 1:

gccgo also complains about this as an unused variable:
$ gccgo x.go
x.go:4:9: error: 'x' declared and not used
  switch x := interface{}(nil).(type) {
         ^
The spec makes this an implementation restriction, but go/types and gccgo agree with how
the implementation restriction should be applied in this case. Making this a cmd/gc bug.

Labels changed: added release-none, repo-main.

Status changed to Accepted.

@adg

This comment has been minimized.

Contributor

adg commented Nov 6, 2014

Comment 2:

This program should generate the error "p declared but not used":
package main
func main() {
        p := true
        func() {
                p = true
        }()
}
If you remove the closure, the compiler raises the error.
@gopherbot

This comment has been minimized.

gopherbot commented Jun 5, 2015

CL https://golang.org/cl/10757 mentions this issue.

alandonovan added a commit that referenced this issue Jun 5, 2015

runtime: add blank assignment to defeat "declared but not used" error…
… from go/types

gc should ideally consider this an error too; see #8560.

Change-Id: Ieee71c4ecaff493d7f83e15ba8c8a04ee90a4cf1
Reviewed-on: https://go-review.googlesource.com/10757
Reviewed-by: Robert Griesemer <gri@golang.org>

@rsc rsc changed the title from cmd/gc: gc inconsistent about reporting "unused variable" errors to cmd/compile: gc inconsistent about reporting "unused variable" errors Jun 8, 2015

@rsc

This comment has been minimized.

Contributor

rsc commented Mar 7, 2016

This is in milestone Unplanned. By my assignment should I infer you are requesting we reprioritize it? Normally Unplanned things have no assignee.

@alandonovan

This comment has been minimized.

Contributor

alandonovan commented Mar 7, 2016

Yes, that's what I meant. (Sorry if that was unclear or seemed passive-aggressive.)

@rsc rsc removed their assignment Mar 7, 2016

@rsc rsc modified the milestones: Go1.7, Unplanned Mar 7, 2016

@rsc

This comment has been minimized.

Contributor

rsc commented Mar 7, 2016

OK, reprioritized to Go 1.7, but that's still no guarantee.

@rsc rsc modified the milestones: Go1.8, Go1.7 May 17, 2016

@gopherbot

This comment has been minimized.

gopherbot commented May 28, 2016

CL https://golang.org/cl/23528 mentions this issue.

gopherbot pushed a commit that referenced this issue Jun 1, 2016

crypto/tls: remove unused variable in benchmark code
This fixes `go test go/types`.

https://golang.org/cl/23487/ introduced this code which contains
two unused variables (declared and assigned to, but never read).
cmd/compile doesn't report the error due open issue #8560 (the
variables are assigned to in a closure), but go/types does. The
build bot only runs go/types tests in -short mode (which doesn't
typecheck the std lib), hence this doesn't show up on the dashboard
either.

We cannot call b.Fatal and friends in the goroutine. Communicating
the error to the invoking function requires a channel or a mutex.
Unless the channel/sycnhronized variable is tested in each iteration
that follows, the iteration blocks if there's a failure. Testing in
each iteration may affect benchmark times.

One could use a time-out but that time depends on the underlying system.
Panicking seems good enough in this unlikely case; better than hanging
or affecting benchmark times.

Change-Id: Idce1172da8058e580fa3b3e398825b0eb4316325
Reviewed-on: https://go-review.googlesource.com/23528
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
@odeke-em

This comment has been minimized.

Member

odeke-em commented Aug 5, 2016

@adg your report in #8560 (comment) was a bug that was already reported at #3059, in case you are interested in tracking that case.

@jim-minter

This comment has been minimized.

jim-minter commented Sep 27, 2016

Out of interest, are there any known workarounds for the gorename issue #14596 (short of not using closures in this way across a codebase) that can be used to enable use of gorename in codebases such as that before this issue is resolved?

@bradfitz bradfitz modified the milestones: Go1.10Early, Go1.10 Jun 14, 2017

jaffee added a commit to travisturner/pilosa that referenced this issue Sep 18, 2017

travisturner added a commit to travisturner/pilosa that referenced this issue Sep 25, 2017

@mdempsky mdempsky modified the milestones: Go1.10, Go1.11 Nov 29, 2017

rmmh added a commit to rmmh/kubernetes that referenced this issue Feb 6, 2018

Remove unused variables (only assigned to) from test code.
This is revealed by the go/types package, which is stricter than
the Go compiler about unused variables. See also: golang/go#8560

rmmh added a commit to rmmh/kubernetes that referenced this issue Feb 6, 2018

Remove unused variables (only assigned to) from test code.
This is revealed by the go/types package, which is stricter than
the Go compiler about unused variables. See also: golang/go#8560

rmmh added a commit to rmmh/kubernetes that referenced this issue Feb 8, 2018

Remove unused variables (only assigned to) from test code.
This is revealed by the go/types package, which is stricter than
the Go compiler about unused variables. See also: golang/go#8560

rmmh added a commit to rmmh/kubernetes that referenced this issue Feb 9, 2018

Remove unused variables (only assigned to) from test code.
This is revealed by the go/types package, which is stricter than
the Go compiler about unused variables. See also: golang/go#8560

rmmh added a commit to rmmh/kubernetes that referenced this issue Feb 20, 2018

Remove unused variables (only assigned to) from test code.
This is revealed by the go/types package, which is stricter than
the Go compiler about unused variables. See also: golang/go#8560

rmmh added a commit to rmmh/kubernetes that referenced this issue Feb 21, 2018

Remove unused variables (only assigned to) from test code.
This is revealed by the go/types package, which is stricter than
the Go compiler about unused variables. See also: golang/go#8560

rmmh added a commit to rmmh/kubernetes that referenced this issue Feb 22, 2018

Remove unused variables (only assigned to) from test code.
This is revealed by the go/types package, which is stricter than
the Go compiler about unused variables. See also: golang/go#8560

rmmh added a commit to rmmh/kubernetes that referenced this issue Feb 27, 2018

Remove unused variables (only assigned to) from test code.
This is revealed by the go/types package, which is stricter than
the Go compiler about unused variables. See also: golang/go#8560

k8s-publishing-bot added a commit to kubernetes/apiserver that referenced this issue Feb 28, 2018

Remove unused variables (only assigned to) from test code.
This is revealed by the go/types package, which is stricter than
the Go compiler about unused variables. See also: golang/go#8560

Kubernetes-commit: e04b91facf180c17557a44e8e462858ea2936301

k8s-publishing-bot added a commit to kubernetes/apiserver that referenced this issue Feb 28, 2018

Remove unused variables (only assigned to) from test code.
This is revealed by the go/types package, which is stricter than
the Go compiler about unused variables. See also: golang/go#8560

Kubernetes-commit: e04b91facf180c17557a44e8e462858ea2936301

sttts pushed a commit to sttts/apiserver that referenced this issue Mar 1, 2018

Remove unused variables (only assigned to) from test code.
This is revealed by the go/types package, which is stricter than
the Go compiler about unused variables. See also: golang/go#8560

Kubernetes-commit: e04b91facf180c17557a44e8e462858ea2936301

sttts pushed a commit to sttts/apiserver that referenced this issue Mar 1, 2018

Remove unused variables (only assigned to) from test code.
This is revealed by the go/types package, which is stricter than
the Go compiler about unused variables. See also: golang/go#8560

Kubernetes-commit: e04b91facf180c17557a44e8e462858ea2936301

jingxu97 added a commit to jingxu97/kubernetes that referenced this issue Mar 13, 2018

Remove unused variables (only assigned to) from test code.
This is revealed by the go/types package, which is stricter than
the Go compiler about unused variables. See also: golang/go#8560

@bradfitz bradfitz modified the milestones: Go1.11, Go1.12 May 18, 2018

@gopherbot gopherbot modified the milestones: Go1.12, Unplanned May 23, 2018

FiloSottile pushed a commit to FiloSottile/go that referenced this issue Oct 12, 2018

crypto/tls: remove unused variable in benchmark code
This fixes `go test go/types`.

https://golang.org/cl/23487/ introduced this code which contains
two unused variables (declared and assigned to, but never read).
cmd/compile doesn't report the error due open issue golang#8560 (the
variables are assigned to in a closure), but go/types does. The
build bot only runs go/types tests in -short mode (which doesn't
typecheck the std lib), hence this doesn't show up on the dashboard
either.

We cannot call b.Fatal and friends in the goroutine. Communicating
the error to the invoking function requires a channel or a mutex.
Unless the channel/sycnhronized variable is tested in each iteration
that follows, the iteration blocks if there's a failure. Testing in
each iteration may affect benchmark times.

One could use a time-out but that time depends on the underlying system.
Panicking seems good enough in this unlikely case; better than hanging
or affecting benchmark times.

Change-Id: Idce1172da8058e580fa3b3e398825b0eb4316325
Reviewed-on: https://go-review.googlesource.com/23528
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>

FiloSottile pushed a commit to FiloSottile/go that referenced this issue Oct 12, 2018

crypto/tls: remove unused variable in benchmark code
This fixes `go test go/types`.

https://golang.org/cl/23487/ introduced this code which contains
two unused variables (declared and assigned to, but never read).
cmd/compile doesn't report the error due open issue golang#8560 (the
variables are assigned to in a closure), but go/types does. The
build bot only runs go/types tests in -short mode (which doesn't
typecheck the std lib), hence this doesn't show up on the dashboard
either.

We cannot call b.Fatal and friends in the goroutine. Communicating
the error to the invoking function requires a channel or a mutex.
Unless the channel/sycnhronized variable is tested in each iteration
that follows, the iteration blocks if there's a failure. Testing in
each iteration may affect benchmark times.

One could use a time-out but that time depends on the underlying system.
Panicking seems good enough in this unlikely case; better than hanging
or affecting benchmark times.

Change-Id: Idce1172da8058e580fa3b3e398825b0eb4316325
Reviewed-on: https://go-review.googlesource.com/23528
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment