[gccrs COMMIT] gccrs: Fix unused_assignment check logic

[email protected]
Newsgroups gmane.comp.gcc.rust,gmane.comp.gcc.patches
Message-ID <[email protected]>
From: Aiman Najjar <[email protected]>

Fixes Rust-GCC/gccrs#4747

Update logic so unused collector tracks the usage of only the last
assignment for a given variable. When a new assignment is encountered
before last one is used, move the prior assignment to a new
unused_assigns set.

gcc/rust/ChangeLog:

	* checks/lints/unused/rust-unused-checker.cc
	(UnusedChecker::visit): Update logic for checking unused
	assignments.
	* checks/lints/unused/rust-unused-context.cc
	(UnusedContext::add_assign): Use new set to track unused
	assignments.
	(UnusedContext::remove_assign): Update to track only last
	assignment.
	(UnusedContext::is_variable_assigned): Likewise.
	(UnusedContext::is_assign_unused): New helper function to
	check if given id was marked as unused by collector.
	* checks/lints/unused/rust-unused-context.h:
	(class UnusedContext): Add new set to track unused assignments and
	changed current structure to track only last assignment per
	variable (one to one map).

gcc/testsuite/ChangeLog:

	* rust/compile/issue-4747.rs: New test.

Signed-off-by: Aiman Najjar <[email protected]>
---
This change was merged into the gccrs repository and is posted here for
upstream visibility and potential drive-by review, as requested by GCC
release managers.
Each commit email contains a link to its details on github from where you can
find the Pull-Request and associated discussions.


Commit on github: https://github.com/Rust-GCC/gccrs/commit/7a0c8beaa0c2cdd5607e561edeeebee196277ad9

The commit has been mentioned in the following issue(s):
 - Rust-GCC/gccrs#4747: https://github.com/Rust-GCC/gccrs/issues/4747

The commit has been mentioned in the following pull-request(s):
 - https://github.com/Rust-GCC/gccrs/pull/4756

 .../lints/unused/rust-unused-checker.cc       | 17 +++------
 .../lints/unused/rust-unused-context.cc       | 21 +++++------
 .../checks/lints/unused/rust-unused-context.h |  6 ++--
 gcc/testsuite/rust/compile/issue-4747.rs      | 36 +++++++++++++++++++
 4 files changed, 55 insertions(+), 25 deletions(-)
 create mode 100644 gcc/testsuite/rust/compile/issue-4747.rs

diff --git a/gcc/rust/checks/lints/unused/rust-unused-checker.cc b/gcc/rust/checks/lints/unused/rust-unused-checker.cc
index 1c128f519..c9c906338 100644
--- a/gcc/rust/checks/lints/unused/rust-unused-checker.cc
+++ b/gcc/rust/checks/lints/unused/rust-unused-checker.cc
@@ -105,19 +105,10 @@ UnusedChecker::visit (HIR::AssignmentExpr &expr)
 {
   const auto &lhs = expr.get_lhs ();
   auto var_name = lhs.to_string ();
-  NodeId ast_node_id = lhs.get_mappings ().get_nodeid ();
-  if (auto def_id
-      = nr_context.lookup (ast_node_id, Resolver2_0::Namespace::Values))
-    {
-      if (auto id = mappings.lookup_node_to_hir (*def_id))
-	{
-	  if (unused_context.is_variable_assigned (
-		*id, lhs.get_mappings ().get_hirid ())
-	      && var_name[0] != '_')
-	    rust_warning_at (lhs.get_locus (), OPT_Wunused_variable,
-			     "unused assignment %qs", var_name.c_str ());
-	}
-    }
+  if (var_name[0] != '_'
+      && unused_context.is_assign_unused (lhs.get_mappings ().get_hirid ()))
+    rust_warning_at (lhs.get_locus (), OPT_Wunused_variable,
+		     "unused assignment %qs", var_name.c_str ());
 }
 
 void
diff --git a/gcc/rust/checks/lints/unused/rust-unused-context.cc b/gcc/rust/checks/lints/unused/rust-unused-context.cc
index 8a68a601a..ff7f25ac9 100644
--- a/gcc/rust/checks/lints/unused/rust-unused-context.cc
+++ b/gcc/rust/checks/lints/unused/rust-unused-context.cc
@@ -37,27 +37,28 @@ UnusedContext::is_variable_used (HirId id) const
 void
 UnusedContext::add_assign (HirId id_def, HirId id)
 {
-  assigned_vars[id_def].push_back (id);
+  if (is_variable_assigned (id_def))
+    unused_assigns.emplace (assigned_vars[id_def]);
+  assigned_vars[id_def] = id;
 }
 
 void
 UnusedContext::remove_assign (HirId id_def)
 {
   if (assigned_vars.find (id_def) != assigned_vars.end ())
-    {
-      assigned_vars[id_def].pop_back ();
+    assigned_vars.erase (id_def);
+}
 
-      if (assigned_vars[id_def].empty ())
-	assigned_vars.erase (id_def);
-    }
+bool
+UnusedContext::is_variable_assigned (HirId id_def)
+{
+  return assigned_vars.find (id_def) != assigned_vars.end ();
 }
 
 bool
-UnusedContext::is_variable_assigned (HirId id_def, HirId id)
+UnusedContext::is_assign_unused (HirId id)
 {
-  auto assigned_vec = assigned_vars[id_def];
-  return std::find (assigned_vec.begin (), assigned_vec.end (), id)
-	 != assigned_vec.end ();
+  return unused_assigns.find (id) != unused_assigns.end ();
 }
 
 void
diff --git a/gcc/rust/checks/lints/unused/rust-unused-context.h b/gcc/rust/checks/lints/unused/rust-unused-context.h
index 1405ebb64..9dd589759 100644
--- a/gcc/rust/checks/lints/unused/rust-unused-context.h
+++ b/gcc/rust/checks/lints/unused/rust-unused-context.h
@@ -31,7 +31,8 @@ public:
   // Assigned var
   void add_assign (HirId id_def, HirId id);
   void remove_assign (HirId id_def);
-  bool is_variable_assigned (HirId id_def, HirId id);
+  bool is_variable_assigned (HirId id_ref);
+  bool is_assign_unused (HirId id);
 
   // Mutable var
   void add_mut (HirId id);
@@ -47,7 +48,8 @@ public:
 private:
   std::unordered_set<HirId> used_vars;
   std::unordered_set<HirId> mutable_vars;
-  std::map<HirId, std::vector<HirId>> assigned_vars;
+  std::map<HirId, HirId> assigned_vars;
+  std::unordered_set<HirId> unused_assigns;
   std::unordered_set<HirId> used_labels;
 };
 
diff --git a/gcc/testsuite/rust/compile/issue-4747.rs b/gcc/testsuite/rust/compile/issue-4747.rs
new file mode 100644
index 000000000..7b1f28e81
--- /dev/null
+++ b/gcc/testsuite/rust/compile/issue-4747.rs
@@ -0,0 +1,36 @@
+// { dg-additional-options "-frust-unused-check-2.0" }
+#![feature(no_core)]
+#![no_core]
+fn foo(mut n: i32) {
+    if false {
+        n = 1i32;
+        // { dg-warning "unused assignment .n." "" { target *-*-* } .-1 }
+    }
+
+    n = 1i32;
+    // { dg-warning "unused assignment .n." "" { target *-*-* } .-1 }
+
+    n = 2i32;
+    bar(n);
+
+    if n > 0i32 {
+        let _ = 1i32 / n;
+    }
+    let _ = 1i32 / n;
+
+    let mut n;
+
+    n = 10;
+    // { dg-warning "unused assignment .n." "" { target *-*-* } .-1 }
+
+    n = 5;
+    if n > 0 {
+        let _ = 1i32 / n;
+    }
+}
+
+fn bar(_n: i32) {}
+
+fn main() {
+    foo(1);
+}

base-commit: 016d4b958629cf944231fc56d6a4699d30f06467
-- 
2.54.0
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.