review #2: traverse assignment-target reads uniformly in semantic passes

Haofei · claude · Haofei · commit e18539c8b315 · 2026-06-13T13:09:20.000-07:00
Controlled-assignment targets evaluate subexpressions (a field base, an index
expression) that are genuine reads of bindings. Several use/ident/effect/capture
passes only visited the assigned value, silently dropping those target reads.
Route them all through crate::hir::assign_target_reads / its AST analogue:

  local.rs:  collect_hir_stmt_idents, collect_hir_stmt_effect_events,
             collect_hir_stmt_inline_capture_uses,
             collect_closure_local_moved_uses_from_stmt,
             collect_retained_closure_captures_from_stmt
  body.rs:   collect_stmt_uses, collect_spawn_capture_idents_from_stmt,
             check_explicit_closure_captures_stmt
  analyzer:  check_match_exhaustiveness, check_unknown_bindings

This makes use-after-move, unknown-binding, capture, and effect analysis see
reads like obj/idx in 'obj.field[idx] = value' that were previously invisible.

Co-Authored-By: Claude Opus 4.8 &lt;noreply@anthropic.com&gt;
diff --git a/crates/rsscript/src/analyzer.rs b/crates/rsscript/src/analyzer.rs
@@ -1996,8 +1996,12 @@ impl Analyzer<'_> {
                     self.check_match_exhaustiveness_block(&arm.body);
                 }
             }
-            HirStmt::Expr(expr) | HirStmt::Assign { value: expr, .. } => {
-                self.check_match_exhaustiveness_expr(expr)
+            HirStmt::Expr(expr) => self.check_match_exhaustiveness_expr(expr),
+            HirStmt::Assign { target, value, .. } => {
+                for read in crate::hir::assign_target_reads(target) {
+                    self.check_match_exhaustiveness_expr(read);
+                }
+                self.check_match_exhaustiveness_expr(value);
             }
             HirStmt::Break(_) | HirStmt::Continue(_) | HirStmt::Unknown(_) => {}
         }
@@ -3299,8 +3303,12 @@ impl Analyzer<'_> {
                     self.check_unknown_bindings_in_block(&arm.body, &mut arm_visible);
                 }
             }
-            HirStmt::Expr(value) | HirStmt::Assign { value, .. } => {
-                self.check_unknown_bindings_in_expr(value, visible)
+            HirStmt::Expr(value) => self.check_unknown_bindings_in_expr(value, visible),
+            HirStmt::Assign { target, value, .. } => {
+                for read in crate::hir::assign_target_reads(target) {
+                    self.check_unknown_bindings_in_expr(read, visible);
+                }
+                self.check_unknown_bindings_in_expr(value, visible);
             }
             HirStmt::Break(_) | HirStmt::Continue(_) | HirStmt::Unknown(_) => {}
         }
diff --git a/crates/rsscript/src/checks/body.rs b/crates/rsscript/src/checks/body.rs
@@ -150,8 +150,12 @@ fn check_explicit_closure_captures_stmt(
                 check_explicit_closure_captures_block(analyzer, &arm.body, binding_names);
             }
         }
-        HirStmt::Expr(expr) | HirStmt::Assign { value: expr, .. } => {
-            check_explicit_closure_captures_expr(analyzer, expr, binding_names)
+        HirStmt::Expr(expr) => check_explicit_closure_captures_expr(analyzer, expr, binding_names),
+        HirStmt::Assign { target, value, .. } => {
+            for read in crate::hir::assign_target_reads(target) {
+                check_explicit_closure_captures_expr(analyzer, read, binding_names);
+            }
+            check_explicit_closure_captures_expr(analyzer, value, binding_names);
         }
         HirStmt::Break(_) | HirStmt::Continue(_) | HirStmt::Unknown(_) => {}
     }
@@ -1068,7 +1072,13 @@ fn collect_stmt_uses(statement: &HirStmt, uses: &mut HashSet<String>) {
                 collect_block_uses(&arm.body, uses);
             }
         }
-        HirStmt::Expr(expr) | HirStmt::Assign { value: expr, .. } => collect_expr_uses(expr, uses),
+        HirStmt::Expr(expr) => collect_expr_uses(expr, uses),
+        HirStmt::Assign { target, value, .. } => {
+            for read in crate::hir::assign_target_reads(target) {
+                collect_expr_uses(read, uses);
+            }
+            collect_expr_uses(value, uses);
+        }
         HirStmt::Break(_) | HirStmt::Continue(_) | HirStmt::Unknown(_) => {}
     }
 }
@@ -3645,8 +3655,13 @@ fn collect_spawn_capture_idents_from_stmt(statement: &HirStmt, captures: &mut Ve
         | HirStmt::Return {
             value: Some(value), ..
         }
-        | HirStmt::Expr(value)
-        | HirStmt::Assign { value, .. } => collect_spawn_capture_idents(value, captures),
+        | HirStmt::Expr(value) => collect_spawn_capture_idents(value, captures),
+        HirStmt::Assign { target, value, .. } => {
+            for read in crate::hir::assign_target_reads(target) {
+                collect_spawn_capture_idents(read, captures);
+            }
+            collect_spawn_capture_idents(value, captures);
+        }
         HirStmt::With { resource, body, .. } => {
             collect_spawn_capture_idents(resource, captures);
             for statement in &body.statements {
diff --git a/crates/rsscript/src/checks/local.rs b/crates/rsscript/src/checks/local.rs
@@ -1041,9 +1041,12 @@ fn collect_closure_local_moved_uses_from_stmt(statement: &HirStmt, moved_uses: &
         | HirStmt::Return {
             value: Some(value), ..
         }
-        | HirStmt::Expr(value)
-        | HirStmt::Assign { value, .. } => {
-            collect_closure_local_moved_uses_from_expr(value, moved_uses)
+        | HirStmt::Expr(value) => collect_closure_local_moved_uses_from_expr(value, moved_uses),
+        HirStmt::Assign { target, value, .. } => {
+            for read in crate::hir::assign_target_reads(target) {
+                collect_closure_local_moved_uses_from_expr(read, moved_uses);
+            }
+            collect_closure_local_moved_uses_from_expr(value, moved_uses);
         }
         HirStmt::With { resource, body, .. } => {
             collect_closure_local_moved_uses_from_expr(resource, moved_uses);
@@ -1200,12 +1203,19 @@ fn collect_retained_closure_captures_from_stmt(
         | HirStmt::Return {
             value: Some(value), ..
         }
-        | HirStmt::Expr(value)
-        | HirStmt::Assign { value, .. } => {
+        | HirStmt::Expr(value) => {
             if let Some(state) = entry_state {
                 collect_retained_closure_captures_from_expr(value, state, captures);
             }
         }
+        HirStmt::Assign { target, value, .. } => {
+            if let Some(state) = entry_state {
+                for read in crate::hir::assign_target_reads(target) {
+                    collect_retained_closure_captures_from_expr(read, state, captures);
+                }
+                collect_retained_closure_captures_from_expr(value, state, captures);
+            }
+        }
         HirStmt::With { resource, body, .. } => {
             if let Some(state) = entry_state {
                 collect_retained_closure_captures_from_expr(resource, state, captures);
@@ -2050,8 +2060,13 @@ fn collect_hir_stmt_idents(statement: &HirStmt, uses: &mut Vec<(String, Span)>)
         | HirStmt::Return {
             value: Some(value), ..
         }
-        | HirStmt::Expr(value)
-        | HirStmt::Assign { value, .. } => collect_hir_expr_idents(value, uses),
+        | HirStmt::Expr(value) => collect_hir_expr_idents(value, uses),
+        HirStmt::Assign { target, value, .. } => {
+            for read in crate::hir::assign_target_reads(target) {
+                collect_hir_expr_idents(read, uses);
+            }
+            collect_hir_expr_idents(value, uses);
+        }
         HirStmt::With { resource, .. } => collect_hir_expr_idents(resource, uses),
         HirStmt::If { condition, .. } => collect_hir_expr_idents(condition, uses),
         HirStmt::Loop {
@@ -2084,8 +2099,13 @@ fn collect_hir_stmt_effect_events(statement: &HirStmt, events: &mut Vec<HirEffec
         | HirStmt::Return {
             value: Some(value), ..
         }
-        | HirStmt::Expr(value)
-        | HirStmt::Assign { value, .. } => collect_hir_expr_effect_events(value, events),
+        | HirStmt::Expr(value) => collect_hir_expr_effect_events(value, events),
+        HirStmt::Assign { target, value, .. } => {
+            for read in crate::hir::assign_target_reads(target) {
+                collect_hir_expr_effect_events(read, events);
+            }
+            collect_hir_expr_effect_events(value, events);
+        }
         HirStmt::With { resource, .. } => collect_hir_expr_effect_events(resource, events),
         HirStmt::If { condition, .. } => collect_hir_expr_effect_events(condition, events),
         HirStmt::Loop {
@@ -2310,8 +2330,13 @@ fn collect_hir_stmt_inline_capture_uses(statement: &HirStmt, uses: &mut Vec<(Str
         | HirStmt::Return {
             value: Some(value), ..
         }
-        | HirStmt::Expr(value)
-        | HirStmt::Assign { value, .. } => collect_hir_expr_inline_capture_uses(value, uses),
+        | HirStmt::Expr(value) => collect_hir_expr_inline_capture_uses(value, uses),
+        HirStmt::Assign { target, value, .. } => {
+            for read in crate::hir::assign_target_reads(target) {
+                collect_hir_expr_inline_capture_uses(read, uses);
+            }
+            collect_hir_expr_inline_capture_uses(value, uses);
+        }
         HirStmt::With { resource, .. } => collect_hir_expr_inline_capture_uses(resource, uses),
         HirStmt::If { condition, .. } => collect_hir_expr_inline_capture_uses(condition, uses),
         HirStmt::Loop {

Original file line number	Diff line number	Diff line change
`@@ -1996,8 +1996,12 @@ impl Analyzer<'_> {`
`1996`	`1996`	`self.check_match_exhaustiveness_block(&arm.body);`
`1997`	`1997`	`}`
`1998`	`1998`	`}`
`1999`		`- HirStmt::Expr(expr) \| HirStmt::Assign { value: expr, .. } => {`
`2000`		`- self.check_match_exhaustiveness_expr(expr)`
	`1999`	`+ HirStmt::Expr(expr) => self.check_match_exhaustiveness_expr(expr),`
	`2000`	`+ HirStmt::Assign { target, value, .. } => {`
	`2001`	`+ for read in crate::hir::assign_target_reads(target) {`
	`2002`	`+ self.check_match_exhaustiveness_expr(read);`
	`2003`	`+ }`
	`2004`	`+ self.check_match_exhaustiveness_expr(value);`
`2001`	`2005`	`}`
`2002`	`2006`	`HirStmt::Break(_) \| HirStmt::Continue(_) \| HirStmt::Unknown(_) => {}`
`2003`	`2007`	`}`
`@@ -3299,8 +3303,12 @@ impl Analyzer<'_> {`
`3299`	`3303`	`self.check_unknown_bindings_in_block(&arm.body, &mut arm_visible);`
`3300`	`3304`	`}`
`3301`	`3305`	`}`
`3302`		`- HirStmt::Expr(value) \| HirStmt::Assign { value, .. } => {`
`3303`		`- self.check_unknown_bindings_in_expr(value, visible)`
	`3306`	`+ HirStmt::Expr(value) => self.check_unknown_bindings_in_expr(value, visible),`
	`3307`	`+ HirStmt::Assign { target, value, .. } => {`
	`3308`	`+ for read in crate::hir::assign_target_reads(target) {`
	`3309`	`+ self.check_unknown_bindings_in_expr(read, visible);`
	`3310`	`+ }`
	`3311`	`+ self.check_unknown_bindings_in_expr(value, visible);`
`3304`	`3312`	`}`
`3305`	`3313`	`HirStmt::Break(_) \| HirStmt::Continue(_) \| HirStmt::Unknown(_) => {}`
`3306`	`3314`	`}`
Original file line number	Diff line number	Diff line change
`@@ -150,8 +150,12 @@ fn check_explicit_closure_captures_stmt(`
`150`	`150`	`check_explicit_closure_captures_block(analyzer, &arm.body, binding_names);`
`151`	`151`	`}`
`152`	`152`	`}`
`153`		`- HirStmt::Expr(expr) \| HirStmt::Assign { value: expr, .. } => {`
`154`		`- check_explicit_closure_captures_expr(analyzer, expr, binding_names)`
	`153`	`+ HirStmt::Expr(expr) => check_explicit_closure_captures_expr(analyzer, expr, binding_names),`
	`154`	`+ HirStmt::Assign { target, value, .. } => {`
	`155`	`+ for read in crate::hir::assign_target_reads(target) {`
	`156`	`+ check_explicit_closure_captures_expr(analyzer, read, binding_names);`
	`157`	`+ }`
	`158`	`+ check_explicit_closure_captures_expr(analyzer, value, binding_names);`
`155`	`159`	`}`
`156`	`160`	`HirStmt::Break(_) \| HirStmt::Continue(_) \| HirStmt::Unknown(_) => {}`
`157`	`161`	`}`
`@@ -1068,7 +1072,13 @@ fn collect_stmt_uses(statement: &HirStmt, uses: &mut HashSet<String>) {`
`1068`	`1072`	`collect_block_uses(&arm.body, uses);`
`1069`	`1073`	`}`
`1070`	`1074`	`}`
`1071`		`- HirStmt::Expr(expr) \| HirStmt::Assign { value: expr, .. } => collect_expr_uses(expr, uses),`
	`1075`	`+ HirStmt::Expr(expr) => collect_expr_uses(expr, uses),`
	`1076`	`+ HirStmt::Assign { target, value, .. } => {`
	`1077`	`+ for read in crate::hir::assign_target_reads(target) {`
	`1078`	`+ collect_expr_uses(read, uses);`
	`1079`	`+ }`
	`1080`	`+ collect_expr_uses(value, uses);`
	`1081`	`+ }`
`1072`	`1082`	`HirStmt::Break(_) \| HirStmt::Continue(_) \| HirStmt::Unknown(_) => {}`
`1073`	`1083`	`}`
`1074`	`1084`	`}`
`@@ -3645,8 +3655,13 @@ fn collect_spawn_capture_idents_from_stmt(statement: &HirStmt, captures: &mut Ve`
`3645`	`3655`	`\| HirStmt::Return {`
`3646`	`3656`	`value: Some(value), ..`
`3647`	`3657`	`}`
`3648`		`- \| HirStmt::Expr(value)`
`3649`		`- \| HirStmt::Assign { value, .. } => collect_spawn_capture_idents(value, captures),`
	`3658`	`+ \| HirStmt::Expr(value) => collect_spawn_capture_idents(value, captures),`
	`3659`	`+ HirStmt::Assign { target, value, .. } => {`
	`3660`	`+ for read in crate::hir::assign_target_reads(target) {`
	`3661`	`+ collect_spawn_capture_idents(read, captures);`
	`3662`	`+ }`
	`3663`	`+ collect_spawn_capture_idents(value, captures);`
	`3664`	`+ }`
`3650`	`3665`	`HirStmt::With { resource, body, .. } => {`
`3651`	`3666`	`collect_spawn_capture_idents(resource, captures);`
`3652`	`3667`	`for statement in &body.statements {`