Skip to content

Implement unused assignments lints on HIR #4285

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
Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Implement unused assignments lints on HIR #4285

wants to merge 4 commits into from

Conversation

@lucasly-ba
Copy link
Contributor

@lucasly-ba lucasly-ba commented Nov 13, 2025 •
edited
Loading

Fixes #4260
on top of PR #4055
Used the HIR default visitor for AssignExpr, added a map, to have all the unused assignments and attached some methods to it.
Since

a = 5

is an HIR::PathInExpression we cannot visit the left expr of an assignment, because it would be considerered used. So I also added a walk_right to only visit the right expr.
( if we have a = b, we want to visit b)

@lucasly-ba lucasly-ba changed the title Implement unused variable checker on HIR Implement unused assignments checker on HIR Nov 13, 2025
@lucasly-ba lucasly-ba changed the title Implement unused assignments checker on HIR Implement unused assignments lints on HIR Nov 13, 2025
@lucasly-ba lucasly-ba force-pushed the 4260-unused-assignments branch from 4712032 to 344b6eb Compare November 13, 2025 18:06
CohenArthur
CohenArthur reviewed Nov 14, 2025
View reviewed changes
Copy link
Member

@CohenArthur CohenArthur left a comment

Choose a reason for hiding this comment

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

This is really good work, well done!!!

@lucasly-ba lucasly-ba force-pushed the 4260-unused-assignments branch 15 times, most recently from 189fcd2 to 95fc4c2 Compare November 18, 2025 12:41
sakupan102 and others added 4 commits November 18, 2025 12:42
@sakupan102 @lucasly-ba
gccrs: implement unused variable checker on HIR.
ab88635 
This change moves the unused variable checker from the type resolver
to HIR. We can now use the HIR Default Visitor, and it will be much more
easier to implement other unused lints with this change.

gcc/rust/ChangeLog:

	* Make-lang.in: Add new files rules in Makefile.
	* lang.opt: Add new flag.
	* rust-session-manager.cc (Session::compile_crate): Execute new variable checker.
	* checks/lints/unused-var/rust-unused-var-checker.cc (UnusedVarChecker): Implement unused
	variable checker.
	* checks/lints/unused-var/rust-unused-var-checker.h (UnusedVarChecker): Implement unused
	variable checker.
	* checks/lints/unused-var/rust-unused-var-collector.cc (UnusedVarCollector): Implement
	unused variable collector.
	* checks/lints/unused-var/rust-unused-var-collector.h (UnusedVarCollector): Implement
	unused variable collector.
	* checks/lints/unused-var/rust-unused-var-context.cc (UnusedVarContext): Implement
	unused variable context.
	* checks/lints/unused-var/rust-unused-var-context.h (UnusedVarContext): Implement unused
	variable context.

gcc/testsuite/ChangeLog:

	* rust/compile/static_item_0.rs: New test.
	* rust/compile/template_function_0.rs: New test.

Signed-off-by: Lucas Ly Ba <[email protected]>
@lucasly-ba
gccrs: add unused-assignments lint
4cf082b 
gcc/rust/ChangeLog:

	* checks/lints/unused-var/rust-unused-var-checker.cc (UnusedVarChecker):
	Implement unused assignments warning.
	(UnusedVarChecker::go): Remove unique pointer unused var context.
	(UnusedVarChecker::visit): Visit AssignExpr in HIR default visitor.
	* checks/lints/unused-var/rust-unused-var-checker.h: Add visit method.
	* checks/lints/unused-var/rust-unused-var-collector.cc (UnusedVarCollector):
	Collect warnings for assignments.
	(UnusedVarCollector::visit): Visit AssignExpr in HIR default visitor.
	* checks/lints/unused-var/rust-unused-var-collector.h: Add visit method.
	* checks/lints/unused-var/rust-unused-var-context.cc (UnusedVarContext::add_assign):
	Add assignment in map.
	(UnusedVarContext::remove_assign): Remove assignment in map.
	(UnusedVarContext::is_variable_assigned): Check if a variable is assigned.
	* checks/lints/unused-var/rust-unused-var-context.h: Add a map to stock assignments.

gcc/testsuite/ChangeLog:

	* rust/compile/issue-4260_0.rs: New test.

Signed-off-by: Lucas Ly Ba <[email protected]>
@lucasly-ba
gccrs: refactor unused var lint
80ba1df 
gcc/rust/ChangeLog:

	* checks/lints/unused-var/rust-unused-var-checker.cc (UnusedVarChecker::visit):
	Change unused name warning to unused variable warning.
	* checks/lints/unused-var/rust-unused-var-collector.cc (UnusedVarCollector::visit):
	Remove useless methods.
	* checks/lints/unused-var/rust-unused-var-collector.h: Same here.
	* checks/lints/unused-var/rust-unused-var-context.cc (UnusedVarContext::add_variable):
	Add used variables to set.
	(UnusedVarContext::mark_used): Remove method.
	(UnusedVarContext::is_variable_used):
	Check if the set contains the hir id linked to a variable.
	(UnusedVarContext::as_string): Refactor method for new set.
	* checks/lints/unused-var/rust-unused-var-context.h: Refactor methods.
	* lang.opt: Change description for unused check flag.

gcc/testsuite/ChangeLog:

	* rust/compile/static_item_0.rs: Modify warning output.
	* rust/compile/template_function_0.rs: Modify warning output.

Signed-off-by: Lucas Ly Ba <[email protected]>
@lucasly-ba
gccrs: change unused lint files name to unused
9353bdf 
gcc/rust/ChangeLog:

	* Make-lang.in: Compile the right files.
	* checks/lints/unused-var/rust-unused-var-checker.cc: Move to...
	* checks/lints/unused/rust-unused-checker.cc: ...here.
	* checks/lints/unused-var/rust-unused-var-checker.h: Move to...
	* checks/lints/unused/rust-unused-checker.h: ...here.
	* checks/lints/unused-var/rust-unused-var-collector.cc: Move to...
	* checks/lints/unused/rust-unused-collector.cc: ...here.
	* checks/lints/unused-var/rust-unused-var-collector.h: Move to...
	* checks/lints/unused/rust-unused-collector.h: ...here.
	* checks/lints/unused-var/rust-unused-var-context.cc: Move to...
	* checks/lints/unused/rust-unused-context.cc: ...here.
	* checks/lints/unused-var/rust-unused-var-context.h: Move to...
	* checks/lints/unused/rust-unused-context.h: ...here.
	* rust-session-manager.cc (Session::compile_crate): Call the right method.

Signed-off-by: Lucas Ly Ba <[email protected]>
@lucasly-ba lucasly-ba force-pushed the 4260-unused-assignments branch from 95fc4c2 to 9353bdf Compare November 18, 2025 12:42
P-E-P
P-E-P requested changes Nov 24, 2025
View reviewed changes
gcc/rust/checks/lints/unused/rust-unused-checker.cc
UnusedChecker::visit (HIR::ConstantItem &item)
{
std::string var_name = item.get_identifier ().as_string ();
bool starts_with_under_score = var_name.compare (0, 1, "_") == 0;
Copy link
Member

@P-E-P P-E-P Nov 24, 2025

Choose a reason for hiding this comment

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

This compare usage looks convoluted and could be simplified. It would even be worth introducing a function that check that prefix.

gcc/rust/checks/lints/unused/rust-unused-checker.cc
UnusedChecker::visit (HIR::StaticItem &item)
{
std::string var_name = item.get_identifier ().as_string ();
bool starts_with_under_score = var_name.compare (0, 1, "_") == 0;
Copy link
Member

@P-E-P P-E-P Nov 24, 2025

Choose a reason for hiding this comment

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

Likewise.

gcc/rust/checks/lints/unused/rust-unused-checker.cc
UnusedChecker::visit (HIR::IdentifierPattern &pattern)
{
std::string var_name = pattern.get_identifier ().as_string ();
bool starts_with_under_score = var_name.compare (0, 1, "_") == 0;
Copy link
Member

@P-E-P P-E-P Nov 24, 2025

Choose a reason for hiding this comment

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

Likewise.

gcc/rust/checks/lints/unused/rust-unused-checker.cc
const auto &lhs = expr.get_lhs ();
auto s = lhs.as_string ();
std::string var_name = s.substr (0, s.find (':'));
bool starts_with_under_score = var_name.compare (0, 1, "_") == 0;
Copy link
Member

@P-E-P P-E-P Nov 24, 2025

Choose a reason for hiding this comment

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

Likewise.

gcc/rust/checks/lints/unused/rust-unused-checker.cc
{
const auto &lhs = expr.get_lhs ();
auto s = lhs.as_string ();
std::string var_name = s.substr (0, s.find (':'));
Copy link
Member

@P-E-P P-E-P Nov 24, 2025

Choose a reason for hiding this comment

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

You're trying to parse the string representation from your AST ? Something looks wrong over here.

gcc/rust/checks/lints/unused/rust-unused-checker.cc
std::string var_name = pattern.get_identifier ().as_string ();
bool starts_with_under_score = var_name.compare (0, 1, "_") == 0;
auto id = pattern.get_mappings ().get_hirid ();
if (!unused_context.is_variable_used (id) && var_name != "self"
Copy link
Member

@P-E-P P-E-P Nov 24, 2025

Choose a reason for hiding this comment

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

Prefer Values::Keywords::SELF instead

@P-E-P P-E-P mentioned this pull request Nov 24, 2025
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@CohenArthur CohenArthur CohenArthur left review comments

@P-E-P P-E-P P-E-P requested changes

Requested changes must be addressed to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

unused assignments lint not implemented

4 participants

@lucasly-ba @P-E-P @CohenArthur @sakupan102