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

Rust: Placeholder queries for unused variable, unused value #17497

Merged
merged 10 commits into from
Sep 20, 2024

Conversation

geoffw0
Copy link
Contributor

@geoffw0 geoffw0 commented Sep 17, 2024

Placeholder queries for unused variable + unused value in Rust.

They are placeholder because we don't have everything we need to write the queries yet, but I want to get the tests and docs reviewed (and merged) ahead of time so that we are ready to move quickly. The implementations will follow when we have a working SSA library.

As for why there are two queries, it turns out that CPP has three "unused variable" type queries:

  • cpp/unused-local-variable, which looks for local variables that are never used.
  • cpp/unused-static-variable, which looks for static variables that are never used.
  • cpp/unused-variable, which looks for assignments to variables that are never accessed.

The unused variable query corresponds with the first two, unused value (probably the more interesting from the perspective of demonstrating accurate AST, CFG and SSA information) corresponds with the last one.

@geoffw0 geoffw0 added no-change-note-required This PR does not need a change note Rust Pull requests that update Rust code labels Sep 17, 2024
Copy link
Contributor

github-actions bot commented Sep 17, 2024

QHelp previews:

rust/ql/src/queries/unusedentities/UnusedValue.qhelp

Unused value

This rule finds values that are assigned to variables but never used. Unused values should be removed to increase readability and avoid confusion.

Recommendation

Remove any unused values. Also remove any variables that only hold unused values.

Example

In the following example, there is a variable average that is initialized to 0, but that value is never used:

fn get_average(values:&[i32]) -> f64 {
	let mut sum = 0;
	let mut average = 0.0; // BAD: unused value

	for v in values {
		sum += v;
	}

	average = sum as f64 / values.len() as f64;
	return average;
}

The problem can be fixed by removing the unused value:

fn get_average(values:&[i32]) -> f64 {
	let mut sum = 0;
	let average;

	for v in values {
		sum += v;
	}

	average = sum as f64 / values.len() as f64;
	return average;
}

References

rust/ql/src/queries/unusedentities/UnusedVariable.qhelp

Unused variable

This rule finds variables that are never accessed. Unused variables should be removed to increase readability and avoid confusion.

Recommendation

Remove any unused variables.

Example

In the following example, there is an unused variable average that is never used:

fn get_sum(values:&[i32]) -> i32 {
	let mut sum = 0;
	let mut average; // BAD: unused variable

	for v in values {
		sum += v;
	}

	return sum;
}

The problem can be fixed simply by removing the variable:

fn get_sum(values:&[i32]) -> i32 {
	let mut sum = 0;

	for v in values {
		sum += v;
	}

	return sum;
}

References

@geoffw0 geoffw0 added the ready-for-doc-review This PR requires and is ready for review from the GitHub docs team. label Sep 17, 2024
Copy link
Contributor

@subatoi subatoi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Geoff—LGTM 👍

@geoffw0 geoffw0 merged commit d7aa5f1 into github:main Sep 20, 2024
15 checks passed
@geoffw0 geoffw0 deleted the unusedvar branch October 29, 2024 16:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation no-change-note-required This PR does not need a change note ready-for-doc-review This PR requires and is ready for review from the GitHub docs team. Rust Pull requests that update Rust code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants