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: ++ counts as using a variable, shouldn't #23416

Closed
heschi opened this issue Jan 11, 2018 · 4 comments
Closed

cmd/compile: ++ counts as using a variable, shouldn't #23416

heschi opened this issue Jan 11, 2018 · 4 comments

Comments

@heschi
Copy link
Contributor

heschi commented Jan 11, 2018

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

1.9.2, as seen on the playground.

Does this issue reproduce with the latest release?

Yes.

What did you do?

https://play.golang.org/p/2RuYMrSLjt3:

package main

func main() {
	x := 0
	x++
}

What did you expect to see?

A compiler error saying that x is unused. For the purposes of removing useless code, ++ isn't any more of a reason to have a variable than the initial assignment. += behaves the same way.

What did you see instead?

Nothing, apparently because x++ counts as using the variable. go vet also seems content.

It's hard to believe this has never been reported before, but the closest bug Alan and I could find is https://golang.org/issue/10989, which is rather different.

I don't know if it's worth catching this as a compile error, but it ought to at least be reported by vet?

@as
Copy link
Contributor

as commented Jan 11, 2018

Working as designed, because x++ is equivalent to x=x+1 and that reduces to x=x, which is coherent with the current definition of usage.

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

@mvdan
Copy link
Member

mvdan commented Jan 11, 2018

One error that can be given is that the last assigned value is unused. For example, when one does x := 0; x = x + 1, @dominikh's staticcheck reports this value of x is never used. That is pointed at the second statement, not at the first, as it concerns the last assigned value and not the declaration.

It is a different error, but perhaps one that would be easier to integrate into vet and other static analysis tools.

@heschi
Copy link
Contributor Author

heschi commented Jan 11, 2018

That's #6072, which I personally feel is a weaker case than this. I suppose if the best way to implement this check is an SSA analysis then both really are the same.

@golang golang locked and limited conversation to collaborators Jan 11, 2019
@adonovan
Copy link
Member

FWIW, the github.com/gordonklaus/ineffassign analyzer reports this mistake, and it doesn't use SSA. (It constructs its own control-flow graph similar to ctrlflow.)

$ cat a.go

package main

func main() {
	x := 0
	x++
}

$ go run github.com/gordonklaus/ineffassign@latest ./a.go 
a.go:6:2: ineffectual assignment to x

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

6 participants