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

Assigning an unused parameter to a blank identifier doesn't make it used. #32

Open
dmitshur opened this Issue Oct 7, 2018 · 2 comments

Comments

2 participants
@dmitshur
Contributor

dmitshur commented Oct 7, 2018

This seems like an unintentional behavior, but please correct me otherwise.

In most other contests, assigning something to the blank identifier makes it used. It's often used to "force" a use to avoid compilation errors, when there's no other normal use because the code is in development.

This doesn't seem to work for unparam. Consider the program:

package p

import "fmt"

func foo(v string) {
	fmt.Println("this does some stuff")
	fmt.Println("not a stub, etc.")
	if true {
		fmt.Println("really, there's complex logic!")
	}

	_ = v // TODO: Actually use v.
}

I would expect the _ = v line to cause v to be marked as "used" and not get printed as an unused parameter, but that's not what happens with latest unparam:

$ unparam
main.go:5:10: foo - v is unused

Or are there good reasons why assigning to a blank identifier should not be considered a "use"? /cc @dominikh

@mvdan

This comment has been minimized.

Show comment
Hide comment
@mvdan

mvdan Oct 8, 2018

Owner

You're right - this should make the tool quiet. Might not be easy to do though, since the tool works almost entirely on SSA.

Owner

mvdan commented Oct 8, 2018

You're right - this should make the tool quiet. Might not be easy to do though, since the tool works almost entirely on SSA.

@mvdan

This comment has been minimized.

Show comment
Hide comment
@mvdan

mvdan Oct 11, 2018

Owner

I had a quick crack at it, but _ = v is indeed gone from the final SSA. I'm not sure what the best approach here is.

The simplest solution is to go through the function's syntax tree and detect all statements of the form _ = some-parameter. However, this won't detect other syntactic variants, such as var _ = some-parameter or _, _ = some-parameter, some-parameter-2.

Another solution would be to check if a parameter is used zero times in the SSA body, but non-zero times in the syntax tree body. This, in theory, should cover cases like if false { use(v) } on top of the ones described above.

I'm leaning towards the second solution, which should be the most generic while still being simple to maintain.

Owner

mvdan commented Oct 11, 2018

I had a quick crack at it, but _ = v is indeed gone from the final SSA. I'm not sure what the best approach here is.

The simplest solution is to go through the function's syntax tree and detect all statements of the form _ = some-parameter. However, this won't detect other syntactic variants, such as var _ = some-parameter or _, _ = some-parameter, some-parameter-2.

Another solution would be to check if a parameter is used zero times in the SSA body, but non-zero times in the syntax tree body. This, in theory, should cover cases like if false { use(v) } on top of the ones described above.

I'm leaning towards the second solution, which should be the most generic while still being simple to maintain.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment