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.

Sign up for GitHub

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

Jump to bottom

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
Labels
FrozenDueToAge
Milestone
Unreleased

Comments

@iand
Copy link
Contributor

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.
@ianlancetaylor ianlancetaylor added this to the Unreleased milestone Mar 2, 2016
@alandonovan
Copy link
Contributor

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 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 as completed Mar 7, 2016
@jim-minter
Copy link

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?

@jim-minter jim-minter mentioned this issue Sep 27, 2016
Closed
@gopherbot
Copy link
Contributor

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

gopherbot pushed a commit to golang/tools that referenced this issue Sep 27, 2016
@adonovan
refactor/rename: don't stop because of "soft" errors
fc2b74b 
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

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

@jim-minter
Copy link

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

@alandonovan alandonovan mentioned this issue Oct 5, 2016
Closed
@golang golang locked and limited conversation to collaborators Sep 28, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge
Projects
None yet
Milestone
Unreleased
Development

No branches or pull requests
5 participants
@iand @jim-minter @ianlancetaylor @alandonovan @gopherbot