fix: accept subscript and attribute targets in unpacking, for-loop, and comprehension contexts

.gitignore

@@ -7,6 +7,7 @@
 /TODO.md
 /monty
 /.claude/settings.local.json
+/CLAUDE.local.md
 /scratch/
 /worktrees/
 /flame/

crates/monty/src/bytecode/compiler.rs

@@ -118,12 +118,15 @@ fn check_comp_generators(count: usize, position: CodeRange) -> Result<(), Compil
 /// Returns a position that locates `target` in source for error reporting.
 ///
 /// `Name` / `Starred` carry the identifier's position; `Tuple` carries its
-/// own. Used by comp-target unpacking when the per-leaf position isn't
-/// available at the error point.
+/// own; `Subscript`/`Attribute` carry `target_position`. Used by comp-target
+/// unpacking when the per-leaf position isn't available at the error point.
 fn target_position(target: &UnpackTarget) -> CodeRange {
     match target {
         UnpackTarget::Name(ident) | UnpackTarget::Starred(ident) => ident.position,
         UnpackTarget::Tuple { position, .. } => *position,
+        UnpackTarget::Subscript { target_position, .. } | UnpackTarget::Attribute { target_position, .. } => {
+            *target_position
+        }
     }
 }
 
@@ -2880,6 +2883,22 @@ impl<'a> Compiler<'a> {
             UnpackTarget::Name(ident) | UnpackTarget::Starred(ident) => {
                 sim.push(SimItem::Leaf(ident.namespace_id().as_u16()));
             }
+            UnpackTarget::Subscript {
+                target,
+                index,
+                target_position,
+            } => {
+                // Consumes value off TOS; net −1 matches `Name`/`Starred`, so the
+                // sim ↔ operand-stack 1:1 invariant survives without pushing back.
+                self.emit_subscript_store(target, index, *target_position)?;
+            }
+            UnpackTarget::Attribute {
+                object,
+                attr,
+                target_position,
+            } => {
+                self.emit_attr_store(object, attr, *target_position)?;
+            }
             UnpackTarget::Tuple { targets, position } => {
                 // Pick UNPACK_EX vs UNPACK_SEQUENCE based on whether a starred
                 // sub-target is present (same logic as the regular assignment
@@ -2928,11 +2947,12 @@ impl<'a> Compiler<'a> {
         Ok(())
     }
 
-    /// Compiles storage of an unpack target - either a single identifier, nested tuple, or starred.
+    /// Compiles storage of an unpack target, assuming the value is on TOS.
     ///
-    /// For single identifiers: emits a simple store.
-    /// For nested tuples: emits `UnpackSequence` (or `UnpackEx` with starred) and recursively
-    /// handles each sub-target.
+    /// Each variant's net stack effect is −1, so the per-target loops in
+    /// `Tuple` and `emit_unpack_store` work uniformly. Subscript/attribute
+    /// delegate to `emit_subscript_store`/`emit_attr_store` (sub-expressions
+    /// evaluated at store time, matching CPython).
     fn compile_unpack_target(&mut self, target: &UnpackTarget) -> Result<(), CompileError> {
         match target {
             UnpackTarget::Name(ident) => {
@@ -2944,6 +2964,20 @@ impl<'a> Compiler<'a> {
                 // Just store as if it were a name
                 self.compile_store(ident)?;
             }
+            UnpackTarget::Subscript {
+                target,
+                index,
+                target_position,
+            } => {
+                self.emit_subscript_store(target, index, *target_position)?;
+            }
+            UnpackTarget::Attribute {
+                object,
+                attr,
+                target_position,
+            } => {
+                self.emit_attr_store(object, attr, *target_position)?;
+            }
             UnpackTarget::Tuple { targets, position } => {
                 // Check if there's a starred target
                 let star_idx = targets.iter().position(|t| matches!(t, UnpackTarget::Starred(_)));

crates/monty/src/expressions.rs

@@ -357,26 +357,35 @@ pub enum Expr {
     },
 }
 
-/// Target for tuple unpacking - can be a single name, nested tuple, or starred target.
+/// Target for tuple unpacking. Used in assignment LHS, `for`-loop, and
+/// comprehension `for` clauses.
 ///
-/// Supports recursive structures like `(a, b), c` or `a, (b, c)`.
-/// Also supports starred targets like `first, *rest = [1, 2, 3, 4]`.
-/// Used in assignment statements, for loop targets, and comprehension targets.
+/// `Name`/`Starred` introduce bindings; `Subscript`/`Attribute` mutate existing
+/// state and bind nothing (scope analysis must skip them as binding sites but
+/// still walk their sub-expressions for walrus targets). The matching leaves
+/// on `AssignTarget` carry identical shapes.
 #[derive(Debug, Clone, serde::Serialize, serde::Deserialize)]
 pub enum UnpackTarget {
-    /// Single identifier: `a`
     Name(Identifier),
-    /// Nested tuple: `(a, b)` - can contain further nested tuples
+    /// Nested tuple — can recurse via `targets`.
     Tuple {
-        /// The targets to unpack into (can be names or nested tuples)
         targets: Vec<Self>,
-        /// Source position covering all targets (for error caret placement)
         position: CodeRange,
     },
-    /// Starred target: `*rest` - captures remaining values into a list.
-    ///
-    /// Only one starred target is allowed per unpacking level.
+    /// `*rest`. At most one per unpacking level.
     Starred(Identifier),
+    /// `container[index]`. Sub-expressions evaluated at store time.
+    Subscript {
+        target: ExprLoc,
+        index: ExprLoc,
+        target_position: CodeRange,
+    },
+    /// `obj.attr`. Sub-expression evaluated at store time.
+    Attribute {
+        object: ExprLoc,
+        attr: EitherStr,
+        target_position: CodeRange,
+    },
 }
 
 /// Target of a single assignment step within a chained assignment.

crates/monty/src/parse.rs

@@ -1454,6 +1454,18 @@ impl<'a> Parser<'a> {
                 }
                 Ok(UnpackTarget::Tuple { targets, position })
             }
+            AstExpr::Subscript(ast::ExprSubscript {
+                value, slice, range, ..
+            }) => Ok(UnpackTarget::Subscript {
+                target: self.parse_expression(*value)?,
+                index: self.parse_expression(*slice)?,
+                target_position: self.convert_range(range),
+            }),
+            AstExpr::Attribute(ast::ExprAttribute { value, attr, range, .. }) => Ok(UnpackTarget::Attribute {
+                object: self.parse_expression(*value)?,
+                attr: EitherStr::Interned(self.interner.intern(attr.id())),
+                target_position: self.convert_range(range),
+            }),
             other => Err(ParseError::syntax(
                 format!("invalid unpacking target: {}", describe_expr_kind(&other)),
                 self.convert_range(other.range()),

crates/monty/src/prepare.rs

@@ -1210,6 +1210,24 @@ impl<'i> Prepare<'i> {
                     position,
                 })
             }
+            UnpackTarget::Subscript {
+                target: container,
+                index,
+                target_position,
+            } => Ok(UnpackTarget::Subscript {
+                target: self.prepare_expression(container)?,
+                index: self.prepare_expression(index)?,
+                target_position,
+            }),
+            UnpackTarget::Attribute {
+                object,
+                attr,
+                target_position,
+            } => Ok(UnpackTarget::Attribute {
+                object: self.prepare_expression(object)?,
+                attr,
+                target_position,
+            }),
         }
     }
 
@@ -1260,6 +1278,26 @@ impl<'i> Prepare<'i> {
                     position,
                 })
             }
+            // Subscript/attribute leaves mutate, they don't bind, so no comp-var
+            // slot. Their sub-expressions resolve against the surrounding scope.
+            UnpackTarget::Subscript {
+                target: container,
+                index,
+                target_position,
+            } => Ok(UnpackTarget::Subscript {
+                target: self.prepare_expression(container)?,
+                index: self.prepare_expression(index)?,
+                target_position,
+            }),
+            UnpackTarget::Attribute {
+                object,
+                attr,
+                target_position,
+            } => Ok(UnpackTarget::Attribute {
+                object: self.prepare_expression(object)?,
+                attr,
+                target_position,
+            }),
         }
     }
 
@@ -2480,8 +2518,14 @@ fn collect_cell_vars_from_node(
         }
         // Recurse into control flow structures
         Node::For {
-            iter, body, or_else, ..
+            target,
+            iter,
+            body,
+            or_else,
+            ..
         } => {
+            // Subscript/attribute targets carry sub-expressions that may capture.
+            collect_cell_vars_from_unpack_target(target, our_locals, cell_vars, interner);
             collect_cell_vars_from_expr(iter, our_locals, cell_vars, interner);
             for n in body {
                 collect_cell_vars_from_node(n, our_locals, cell_vars, interner);
@@ -2529,8 +2573,14 @@ fn collect_cell_vars_from_node(
                 collect_cell_vars_from_node(n, our_locals, cell_vars, interner);
             }
         }
-        Node::With { context, body, .. } => {
+        Node::With {
+            context, target, body, ..
+        } => {
             collect_cell_vars_from_expr(context, our_locals, cell_vars, interner);
+            // `with f() as x[i]:` — same target-walk rationale as `For` above.
+            if let Some(target) = target {
+                collect_cell_vars_from_unpack_target(target, our_locals, cell_vars, interner);
+            }
             for n in body {
                 collect_cell_vars_from_node(n, our_locals, cell_vars, interner);
             }
@@ -2854,8 +2904,14 @@ fn collect_referenced_names_from_node(node: &ParseNode, referenced: &mut AHashSe
             collect_referenced_names_from_expr(object, referenced, interner);
         }
         Node::For {
-            iter, body, or_else, ..
+            target,
+            iter,
+            body,
+            or_else,
+            ..
         } => {
+            // Subscript/attribute targets read from surrounding state at store time.
+            collect_referenced_names_from_unpack_target(target, referenced, interner);
             collect_referenced_names_from_expr(iter, referenced, interner);
             for n in body {
                 collect_referenced_names_from_node(n, referenced, interner);
@@ -2910,8 +2966,13 @@ fn collect_referenced_names_from_node(node: &ParseNode, referenced: &mut AHashSe
                 collect_referenced_names_from_node(n, referenced, interner);
             }
         }
-        Node::With { context, body, .. } => {
+        Node::With {
+            context, target, body, ..
+        } => {
             collect_referenced_names_from_expr(context, referenced, interner);
+            if let Some(target) = target {
+                collect_referenced_names_from_unpack_target(target, referenced, interner);
+            }
             for n in body {
                 collect_referenced_names_from_node(n, referenced, interner);
             }
@@ -3199,9 +3260,9 @@ fn collect_referenced_names_from_fstring_parts(
     }
 }
 
-/// Collects all names from an unpack target into the given set.
-///
-/// Recursively traverses nested tuples to find all identifier names.
+/// Collects names bound by an unpack target, plus walrus targets found inside
+/// subscript/attribute sub-expressions (those leaves bind nothing themselves).
+/// Mirrors `collect_assigned_names_from_assign_target`.
 fn collect_names_from_unpack_target(target: &UnpackTarget, names: &mut AHashSet<String>, interner: &InternerBuilder) {
     match target {
         UnpackTarget::Name(ident) | UnpackTarget::Starred(ident) => {
@@ -3212,6 +3273,64 @@ fn collect_names_from_unpack_target(target: &UnpackTarget, names: &mut AHashSet<
                 collect_names_from_unpack_target(t, names, interner);
             }
         }
+        UnpackTarget::Subscript { target, index, .. } => {
+            collect_assigned_names_from_expr(target, names, interner);
+            collect_assigned_names_from_expr(index, names, interner);
+        }
+        UnpackTarget::Attribute { object, .. } => {
+            collect_assigned_names_from_expr(object, names, interner);
+        }
+    }
+}
+
+/// Cell-var walk for the sub-expressions inside `Subscript`/`Attribute` leaves.
+/// `Name`/`Starred` contribute nothing. Mirrors
+/// `collect_cell_vars_from_assign_target`.
+fn collect_cell_vars_from_unpack_target(
+    target: &UnpackTarget,
+    our_locals: &AHashSet<String>,
+    cell_vars: &mut AHashSet<String>,
+    interner: &InternerBuilder,
+) {
+    match target {
+        UnpackTarget::Name(_) | UnpackTarget::Starred(_) => {}
+        UnpackTarget::Tuple { targets, .. } => {
+            for t in targets {
+                collect_cell_vars_from_unpack_target(t, our_locals, cell_vars, interner);
+            }
+        }
+        UnpackTarget::Subscript { target, index, .. } => {
+            collect_cell_vars_from_expr(target, our_locals, cell_vars, interner);
+            collect_cell_vars_from_expr(index, our_locals, cell_vars, interner);
+        }
+        UnpackTarget::Attribute { object, .. } => {
+            collect_cell_vars_from_expr(object, our_locals, cell_vars, interner);
+        }
+    }
+}
+
+/// Read-side name walk for the sub-expressions inside `Subscript`/`Attribute`
+/// leaves. `Name`/`Starred` contribute nothing. Mirrors
+/// `collect_referenced_names_from_assign_target`.
+fn collect_referenced_names_from_unpack_target(
+    target: &UnpackTarget,
+    referenced: &mut AHashSet<String>,
+    interner: &InternerBuilder,
+) {
+    match target {
+        UnpackTarget::Name(_) | UnpackTarget::Starred(_) => {}
+        UnpackTarget::Tuple { targets, .. } => {
+            for t in targets {
+                collect_referenced_names_from_unpack_target(t, referenced, interner);
+            }
+        }
+        UnpackTarget::Subscript { target, index, .. } => {
+            collect_referenced_names_from_expr(target, referenced, interner);
+            collect_referenced_names_from_expr(index, referenced, interner);
+        }
+        UnpackTarget::Attribute { object, .. } => {
+            collect_referenced_names_from_expr(object, referenced, interner);
+        }
     }
 }
 

crates/monty/test_cases/comprehension__all.py

@@ -206,3 +206,25 @@ def inner():
         [int(c) for c in s]
     except ValueError:
         pass
+
+# === Subscript target as comprehension `for` variable (issue #408) ===
+# CPython allows `for x[i] in iter:` inside comprehensions. The store leaks
+# out of the comprehension's transient scope into the surrounding container
+# (subscript targets mutate, they don't allocate a fresh comp-var).
+box = [None]
+result = [box[0] for box[0] in (1, 2, 3)]
+assert result == [1, 2, 3], 'comp subscript target visible inside body'
+assert box[0] == 3, 'comp subscript target final write persists'
+
+# Generator with subscript target — each emitted value passes through the
+# subscript store; the slot holds the last write after exhaustion.
+a = [0, 0]
+i = 0
+list(a[i] for a[i] in (5, 6, 7))
+assert a == [7, 0], 'generator subscript target stores final value'
+
+# Subscript target inside nested tuple in a comprehension
+a = [0]
+out = [(a[0], y) for a[0], y in [(1, 'x'), (2, 'y')]]
+assert out == [(1, 'x'), (2, 'y')], 'comp nested subscript target reads-through'
+assert a == [2], 'comp nested subscript target final write persists'