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

staticcheck: detect unused values inside closures #287

Open
ainar-g opened this issue May 29, 2018 · 2 comments
Open

staticcheck: detect unused values inside closures #287

ainar-g opened this issue May 29, 2018 · 2 comments
Labels
enhancement pta Issues that require points-to analysis

Comments

@ainar-g
Copy link
Contributor

ainar-g commented May 29, 2018

Found this while refactoring something. Consider this code:

func returnErr() error { return nil }

func main() {
	err := returnErr()
	if err != nil {
		panic(err)
	}

	err = errors.New("ignored")
}

Staticcheck will correctly warn that the second value of err is never used:

scbug.go:15:2: this value of err is never used (SA4006)

But if we use closures, staticcheck fails to detect the bug:

func f(g func()) { g() }

func returnErr() error { return nil }

func main() {
	err := returnErr()
	if err != nil {
		panic(err)
	}

	f(func() {
		err = errors.New("ignored")
	})
}

This code generates no warnings, despite the fact that err is unchecked.

@dominikh
Copy link
Owner

This is essentially because the check, as written, only works on local, non-escaping variables. It bails out as soon as a variable's address is taken. We cannot trivially determine if the variable is being read at some point or not once its address has been taken. We'll have to do whole program analysis and take pointer aliasing into consideration.

@ainar-g
Copy link
Contributor Author

ainar-g commented May 30, 2018

FWIW, LLVM's clang-tidy doesn't find the dead load in similar C++ code either:

void f(const std::function<void()> &g) { g(); }

int returnInt() { return 0; }

int main() {
  int i = returnInt();
  assert(i == 0);

  // i = 42;
  f([&i]() { i = 42; });

  return 0;
}

So I guess this is really a tough feature to get right.

@dominikh dominikh added the pta Issues that require points-to analysis label Aug 10, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement pta Issues that require points-to analysis
Projects
None yet
Development

No branches or pull requests

2 participants