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

x/tools/cmd/gorename: analysis of variable usage differs from go build #14596

Closed
iand opened this issue Mar 2, 2016 · 6 comments
Closed

x/tools/cmd/gorename: analysis of variable usage differs from go build #14596

iand opened this issue Mar 2, 2016 · 6 comments
Assignees
Milestone

Comments

@iand
Copy link
Contributor

@iand iand commented Mar 2, 2016

gorename reports a variable as being unused when the variable is assigned in a closure. go build will compile the code without complaining.

Error reported by gorename:

11:50 $ gorename -offset rotator.go:#831 -to TimeFormat
/home/iand/src/code.avct.io/services/log/rotator.go:398:6: s declared but not used
gorename: couldn't load packages due to errors: code.avct.io/services/log

Can reproduce with code like this:

func Scan() {
    var s string

    tester(func() {
        s = "done"
    })
}

func tester(fn func()) {
    fn()
}

go version go1.6 linux/amd64
golang.org/x/tools commit 6f233b9

This renders gorename unusable if any of the GOPATH contains this pattern of code, even though it compiles fine with the go command.

@alandonovan
Copy link
Contributor

@alandonovan alandonovan commented Mar 7, 2016

This is really a duplicate of #8560, the more general problem that the two frontends accept slightly different dialects.

@iand
Copy link
Contributor Author

@iand iand commented Mar 7, 2016

Yes it's the same as the example given by adg on 6 Nov 2014 so I'm happy to close this as a dupe

@iand iand closed this Mar 7, 2016
@jim-minter
Copy link

@jim-minter jim-minter commented Sep 27, 2016

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

@gopherbot
Copy link

@gopherbot gopherbot commented Sep 27, 2016

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

gopherbot pushed a commit to golang/tools that referenced this issue Sep 27, 2016
Because go/types is slightly more strict than gc about certain "soft"
errors (ones that aren't necessary to interpret a Go program), gorename
rejects programs that compile under gc.  This change relaxes gorename's
error checks so that they are weaker than gc's.

This is a workaround for issue golang/go#14596 in gorename,
whose underlying problem is issue golang/go#8560 in gc.

Fixes golang/go#14596

Change-Id: Ica5006c2376c0564a575224269093c1497348ee6
Reviewed-on: https://go-review.googlesource.com/29853
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
@alandonovan
Copy link
Contributor

@alandonovan alandonovan commented Sep 27, 2016

I've added a workaround to gorename to ignore "soft" errors like "unused variable".

@jim-minter
Copy link

@jim-minter jim-minter commented Sep 28, 2016

Thank-you @alandonovan , that workaround has really helped!

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

Successfully merging a pull request may close this issue.

None yet
5 participants
You can’t perform that action at this time.