From 0bb30b41bccf75561431556cdac97c3f26f1bd13 Mon Sep 17 00:00:00 2001 From: shindakioku Date: Thu, 9 Jul 2026 18:57:10 +0200 Subject: [PATCH 1/3] Allow field use after a conditional field move When a field is conditionally moved out of `self` (e.g. `if x { drop(@a) }`), using a different field afterwards was wrongly rejected as "self has been moved". Joining the branches leaves `self` as `MaybeMoved`, which was treated the same as a whole move. We now report this only when `self` itself is moved on one of the paths. This fixes #962. Changelog: fixed --- compiler/src/mir/passes.rs | 54 +++++++++++++++++++ ...eld_used_after_conditional_field_move.inko | 35 ++++++++++++ ...ield_used_after_conditional_self_move.inko | 40 ++++++++++++++ 3 files changed, 129 insertions(+) create mode 100644 std/fixtures/diagnostics/field_used_after_conditional_field_move.inko create mode 100644 std/fixtures/diagnostics/field_used_after_conditional_self_move.inko diff --git a/compiler/src/mir/passes.rs b/compiler/src/mir/passes.rs index 61a016f03..e51eb99fc 100644 --- a/compiler/src/mir/passes.rs +++ b/compiler/src/mir/passes.rs @@ -3782,6 +3782,37 @@ impl<'a> LowerMethod<'a> { RegisterState::Available | RegisterState::PartiallyMoved => { self.check_if_moved(reg, name, check_loc); } + // Issue #962: when a field is conditionally moved, the remaining + // fields could no longer be used. + // + // A conditional such as: + // + // if x { drop(@some_field) } + // @other_field + // + // splits the control flow into separate blocks that are later + // joined back together. At the join, the state of `self` is the + // merge of its state in each predecessor: on the branch that moved + // the field `self` is `PartiallyMoved`, on the other it's still + // `Available`, and merging the two yields `MaybeMoved`. + // + // `MaybeMoved` is ambiguous: it's produced both when `self` is + // moved as a whole on some path (e.g. `drop(self)`) and when only a + // field is moved. In the latter case `self` itself is still valid, + // so reading a *different* field is fine. To tell the two apart we + // walk the predecessor blocks and only report an error if `self` + // was actually moved as a whole on one of them. + RegisterState::MaybeMoved => { + if self.self_fully_moved_on_any_path() { + self.state.diagnostics.implicit_receiver_moved( + name, + self.file(), + node.location, + ); + } else { + self.check_if_moved(reg, name, check_loc); + } + } _ => { self.state.diagnostics.implicit_receiver_moved( name, @@ -3800,6 +3831,29 @@ impl<'a> LowerMethod<'a> { reg } + fn self_fully_moved_on_any_path(&mut self) -> bool { + let mut stack = self.method.body.predecessors(self.current_block); + let mut visited = HashSet::new(); + + while let Some(block) = stack.pop() { + if let Some(v) = self.register_states.get(block, self.self_register) + && let RegisterState::Moved = v + { + return true; + } + + visited.insert(block); + + for block in self.method.body.predecessors(block) { + if !visited.contains(&block) { + stack.push(block); + } + } + } + + return false; + } + fn constant(&mut self, node: hir::ConstantRef) -> RegisterId { let loc = node.location; let ins_loc = InstructionLocation::new(loc); diff --git a/std/fixtures/diagnostics/field_used_after_conditional_field_move.inko b/std/fixtures/diagnostics/field_used_after_conditional_field_move.inko new file mode 100644 index 000000000..da3c3dc6f --- /dev/null +++ b/std/fixtures/diagnostics/field_used_after_conditional_field_move.inko @@ -0,0 +1,35 @@ +import std.drop (Drop, drop) +import std.stdio (Stdout) + +type Inner {} + +impl Drop for Inner { + fn mut drop { + Stdout.new.print('Inner.drop') + } +} + +type Foo {} + +impl Drop for Foo { + fn mut drop { + Stdout.new.print('Foo.drop') + } +} + +type inline Outer { + let @inner: Inner + let @foo: Foo + + fn move submit { + if true { drop(@inner) } + + drop(@foo) + } +} + +type async Main { + fn async main { + Outer(inner: Inner(), foo: Foo()).submit + } +} diff --git a/std/fixtures/diagnostics/field_used_after_conditional_self_move.inko b/std/fixtures/diagnostics/field_used_after_conditional_self_move.inko new file mode 100644 index 000000000..f3fbaa60d --- /dev/null +++ b/std/fixtures/diagnostics/field_used_after_conditional_self_move.inko @@ -0,0 +1,40 @@ +import std.drop (Drop, drop) +import std.stdio (Stdout) + +type Inner {} + +impl Drop for Inner { + fn mut drop { + Stdout.new.print('Inner.drop') + } +} + +type Foo {} + +impl Drop for Foo { + fn mut drop { + Stdout.new.print('Foo.drop') + } +} + +type inline Outer { + let @inner: Inner + let @foo: Foo + + fn move submit { + if true { + drop(self) + } + + drop(@foo) + } +} + +type async Main { + fn async main { + Outer(inner: Inner(), foo: Foo()).submit + } +} + + +# field_used_after_conditional_self_move.inko:29:10 error(moved): 'foo' can't be used, as 'self' has been moved \ No newline at end of file From cd949d0e5ca0a001433d33e66c73ebff882c80db Mon Sep 17 00:00:00 2001 From: shindakioku Date: Sun, 12 Jul 2026 16:17:26 +0200 Subject: [PATCH 2/3] Track move state per field, not on self Instead of marking `self` as partially moved and walking predecessor blocks to distinguish a whole-`self` move from a field move, the move state is now tracked per field register. Reading a field checks only that field's register; moving `self` marks all of its fields as moved; and whether `self` is partially moved is derived from its fields where needed (e.g. when using `self` directly or as a method receiver). --- compiler/src/mir/passes.rs | 184 +++++------------- ...eld_used_after_conditional_field_move.inko | 15 +- ...ield_used_after_conditional_self_move.inko | 20 +- 3 files changed, 48 insertions(+), 171 deletions(-) diff --git a/compiler/src/mir/passes.rs b/compiler/src/mir/passes.rs index e51eb99fc..e005964b9 100644 --- a/compiler/src/mir/passes.rs +++ b/compiler/src/mir/passes.rs @@ -1428,9 +1428,6 @@ pub(crate) struct LowerMethod<'a> { /// Variables to remap to field reads, and the types to expose the fields /// as. variable_fields: HashMap, - - /// The number of fields that are moved. - moved_fields: usize, } impl<'a> LowerMethod<'a> { @@ -1458,7 +1455,6 @@ impl<'a> LowerMethod<'a> { self_register: RegisterId(SELF_ID), variable_fields: HashMap::new(), used_variables: HashSet::new(), - moved_fields: 0, } } @@ -1553,7 +1549,6 @@ impl<'a> LowerMethod<'a> { location: InstructionLocation, ) { self.mark_register_as_moved(register); - self.partially_move_self_if_field(register); self.drop_all_registers(location); self.check_for_unused_variables(); self.return_register(register, location); @@ -2099,7 +2094,9 @@ impl<'a> LowerMethod<'a> { types::Receiver::Implicit => { let reg = self.self_register; - if !self.register_is_available(self.self_register) { + if !self.register_is_available(reg) + || self.self_has_moved_field() + { let name = info.id.name(self.db()).clone(); self.state.diagnostics.implicit_receiver_moved( @@ -2474,16 +2471,6 @@ impl<'a> LowerMethod<'a> { self.drop_register(old, loc); } - // We allow the use of `self` again once all moved fields are - // assigned a new value. - if is_moved { - self.moved_fields -= 1; - - if self.moved_fields == 0 { - self.mark_register_as_available(self.self_register); - } - } - self.update_register_state(reg, RegisterState::Available); self.current_block_mut().set_field(rec, tid, id, new_val, loc); } else { @@ -3778,49 +3765,7 @@ impl<'a> LowerMethod<'a> { let name = &node.name; let check_loc = node.location; - match self.register_state(rec) { - RegisterState::Available | RegisterState::PartiallyMoved => { - self.check_if_moved(reg, name, check_loc); - } - // Issue #962: when a field is conditionally moved, the remaining - // fields could no longer be used. - // - // A conditional such as: - // - // if x { drop(@some_field) } - // @other_field - // - // splits the control flow into separate blocks that are later - // joined back together. At the join, the state of `self` is the - // merge of its state in each predecessor: on the branch that moved - // the field `self` is `PartiallyMoved`, on the other it's still - // `Available`, and merging the two yields `MaybeMoved`. - // - // `MaybeMoved` is ambiguous: it's produced both when `self` is - // moved as a whole on some path (e.g. `drop(self)`) and when only a - // field is moved. In the latter case `self` itself is still valid, - // so reading a *different* field is fine. To tell the two apart we - // walk the predecessor blocks and only report an error if `self` - // was actually moved as a whole on one of them. - RegisterState::MaybeMoved => { - if self.self_fully_moved_on_any_path() { - self.state.diagnostics.implicit_receiver_moved( - name, - self.file(), - node.location, - ); - } else { - self.check_if_moved(reg, name, check_loc); - } - } - _ => { - self.state.diagnostics.implicit_receiver_moved( - name, - self.file(), - node.location, - ); - } - } + self.check_if_moved(reg, name, check_loc); if info.as_pointer { self.current_block_mut().field_pointer(reg, rec, tid, id, loc); @@ -3831,29 +3776,6 @@ impl<'a> LowerMethod<'a> { reg } - fn self_fully_moved_on_any_path(&mut self) -> bool { - let mut stack = self.method.body.predecessors(self.current_block); - let mut visited = HashSet::new(); - - while let Some(block) = stack.pop() { - if let Some(v) = self.register_states.get(block, self.self_register) - && let RegisterState::Moved = v - { - return true; - } - - visited.insert(block); - - for block in self.method.body.predecessors(block) { - if !visited.contains(&block) { - stack.push(block); - } - } - } - - return false; - } - fn constant(&mut self, node: hir::ConstantRef) -> RegisterId { let loc = node.location; let ins_loc = InstructionLocation::new(loc); @@ -3882,7 +3804,18 @@ impl<'a> LowerMethod<'a> { let reg = self.self_register; - self.check_if_moved(reg, SELF_NAME, node.location); + if self.register_is_available(reg) { + if self.self_has_moved_field() { + self.state.diagnostics.moved_variable( + SELF_NAME, + self.file(), + node.location, + ); + } + } else { + self.check_if_moved(reg, SELF_NAME, node.location); + } + reg } @@ -4178,7 +4111,6 @@ impl<'a> LowerMethod<'a> { if typ.is_owned_or_uni(self.db()) { self.mark_register_as_moved(reg); - self.partially_move_self_if_field(reg); if let Some(flag) = self.drop_flags.get(®).cloned() { self.current_block_mut().bool_literal(flag, false, ins_loc); @@ -4283,13 +4215,8 @@ impl<'a> LowerMethod<'a> { self.check_field_move(register, location); self.mark_register_as_moved(register); - self.partially_move_self_if_field(register); self.record_loop_move(register, location); - if self.register_kind(register).is_field() { - self.mark_register_as_partially_moved(self.self_register); - } - if let Some(flag) = self.drop_flags.get(®ister).cloned() { self.current_block_mut().bool_literal(flag, false, ins_loc); } @@ -4354,7 +4281,6 @@ impl<'a> LowerMethod<'a> { } self.record_loop_move(register, location); - self.partially_move_self_if_field(register); self.mark_register_as_moved(register); if let Some(flag) = self.drop_flags.get(®ister).cloned() { @@ -4382,15 +4308,6 @@ impl<'a> LowerMethod<'a> { register } - fn partially_move_self_if_field(&mut self, register: RegisterId) { - if !self.register_kind(register).is_field() { - return; - } - - self.moved_fields += 1; - self.mark_register_as_partially_moved(self.self_register); - } - fn clone_value_type( &mut self, source: RegisterId, @@ -4546,22 +4463,22 @@ impl<'a> LowerMethod<'a> { }); match self.register_state(self_reg) { - RegisterState::PartiallyMoved => { + // Fully moved: its fields are moved too, so there's nothing to drop. + RegisterState::Moved => {} + // Maybe moved as a whole: `drop_register` drops it conditionally. + RegisterState::MaybeMoved => { + self.drop_register(self_reg, location); + } + // Available but with some fields moved out: drop the remaining + // fields and free self's memory without running its dropper. + RegisterState::Available if partially_moved => { self.drop_remaining_fields(&fields, location); self.current_block_mut() .drop_without_dropper(self_reg, location); } - // Parts of self may be moved conditionally, such as for code like - // `if foo { drop(field) }`. In this case we need to determine at - // runtime if `self` is to be dropped as a whole or only its memory - // should be deallocated. - RegisterState::MaybeMoved if partially_moved => { - self.drop_partial_self(&fields, location); - } - RegisterState::Available | RegisterState::MaybeMoved => { + RegisterState::Available | RegisterState::PartiallyMoved => { self.drop_register(self_reg, location); } - RegisterState::Moved => {} } } @@ -4582,37 +4499,6 @@ impl<'a> LowerMethod<'a> { } } - fn drop_partial_self( - &mut self, - fields: &[(FieldId, TypeRef)], - location: InstructionLocation, - ) { - let self_reg = self.surrounding_type_register; - let before_block = self.current_block; - let drop_block = self.add_block(); - let after_block = self.add_block(); - let drop_flag = self.drop_flags.get(&self_reg).cloned().unwrap(); - - self.current_block_mut().branch( - drop_flag, - drop_block, - after_block, - location, - ); - - self.add_edge(before_block, drop_block); - self.add_edge(before_block, after_block); - self.add_edge(drop_block, after_block); - - self.current_block = drop_block; - self.drop_remaining_fields(fields, location); - self.current_block_mut().drop_without_dropper(self_reg, location); - self.current_block_mut().goto(after_block, location); - - self.current_block = after_block; - self.mark_register_as_moved(self_reg); - } - fn drop_loop_registers(&mut self, location: InstructionLocation) { let mut registers = Vec::new(); let mut scope = Some(&self.scope); @@ -4991,6 +4877,24 @@ impl<'a> LowerMethod<'a> { fn mark_register_as_moved(&mut self, register: RegisterId) { self.update_register_state(register, RegisterState::Moved); + + // Moving `self` as a whole also moves all of its fields. + if register == self.self_register { + let fields: Vec = + self.field_mapping.values().cloned().collect(); + + for field in fields { + self.update_register_state(field, RegisterState::Moved); + } + } + } + + /// Returns `true` if any field of `self` has been (possibly) moved. + fn self_has_moved_field(&mut self) -> bool { + let fields: Vec = + self.field_mapping.values().cloned().collect(); + + fields.into_iter().any(|reg| !self.register_is_available(reg)) } fn mark_register_as_available(&mut self, register: RegisterId) { diff --git a/std/fixtures/diagnostics/field_used_after_conditional_field_move.inko b/std/fixtures/diagnostics/field_used_after_conditional_field_move.inko index da3c3dc6f..573078ed7 100644 --- a/std/fixtures/diagnostics/field_used_after_conditional_field_move.inko +++ b/std/fixtures/diagnostics/field_used_after_conditional_field_move.inko @@ -1,22 +1,9 @@ -import std.drop (Drop, drop) -import std.stdio (Stdout) +import std.drop (drop) type Inner {} -impl Drop for Inner { - fn mut drop { - Stdout.new.print('Inner.drop') - } -} - type Foo {} -impl Drop for Foo { - fn mut drop { - Stdout.new.print('Foo.drop') - } -} - type inline Outer { let @inner: Inner let @foo: Foo diff --git a/std/fixtures/diagnostics/field_used_after_conditional_self_move.inko b/std/fixtures/diagnostics/field_used_after_conditional_self_move.inko index f3fbaa60d..b6d6dd498 100644 --- a/std/fixtures/diagnostics/field_used_after_conditional_self_move.inko +++ b/std/fixtures/diagnostics/field_used_after_conditional_self_move.inko @@ -1,28 +1,15 @@ -import std.drop (Drop, drop) -import std.stdio (Stdout) +import std.drop (drop) type Inner {} -impl Drop for Inner { - fn mut drop { - Stdout.new.print('Inner.drop') - } -} - type Foo {} -impl Drop for Foo { - fn mut drop { - Stdout.new.print('Foo.drop') - } -} - type inline Outer { let @inner: Inner let @foo: Foo fn move submit { - if true { + if true { drop(self) } @@ -36,5 +23,4 @@ type async Main { } } - -# field_used_after_conditional_self_move.inko:29:10 error(moved): 'foo' can't be used, as 'self' has been moved \ No newline at end of file +# field_used_after_conditional_self_move.inko:16:10 error(moved): 'foo' can't be used as it has been moved From 8f27ec890f330f5cd2ee95aee82432c072414d10 Mon Sep 17 00:00:00 2001 From: shindakioku Date: Sat, 18 Jul 2026 07:23:17 +0200 Subject: [PATCH 3/3] Report self usage after a partial field move Detect when `self` (or one of its fields) is used after a field has been moved out, emitting a `moved` diagnostic. Handle the `PartiallyMoved` state explicitly when dropping `self` so remaining fields are dropped and `self`'s memory is freed without running its dropper. Switch `field_mapping` to an `IndexMap` for stable positional iteration. Adds diagnostics fixtures covering self use, implicit method calls, and closure captures after a field move. --- compiler/src/mir/passes.rs | 49 +++++++++---------- ...licitly_captures_partially_moved_self.inko | 21 ++++++++ ...licitly_captures_partially_moved_self.inko | 21 ++++++++ ...implicit_method_call_after_field_move.inko | 23 +++++++++ .../self_used_after_field_move.inko | 21 ++++++++ 5 files changed, 108 insertions(+), 27 deletions(-) create mode 100644 std/fixtures/diagnostics/closure_explicitly_captures_partially_moved_self.inko create mode 100644 std/fixtures/diagnostics/closure_implicitly_captures_partially_moved_self.inko create mode 100644 std/fixtures/diagnostics/implicit_method_call_after_field_move.inko create mode 100644 std/fixtures/diagnostics/self_used_after_field_move.inko diff --git a/compiler/src/mir/passes.rs b/compiler/src/mir/passes.rs index e005964b9..c6e5d8391 100644 --- a/compiler/src/mir/passes.rs +++ b/compiler/src/mir/passes.rs @@ -1420,7 +1420,7 @@ pub(crate) struct LowerMethod<'a> { /// a register. We map fields to registers here so that for field A we /// always write to register R, removing the need for tracking field states /// separately. - field_mapping: HashMap, + field_mapping: IndexMap, /// The registers to write to when a register is moved. drop_flags: HashMap, @@ -1447,7 +1447,7 @@ impl<'a> LowerMethod<'a> { current_block, scope: Scope::root_scope(), variable_mapping: IndexMap::new(), - field_mapping: HashMap::new(), + field_mapping: IndexMap::new(), drop_flags: HashMap::new(), register_states: RegisterStates::new(), register_kinds: Vec::new(), @@ -3803,17 +3803,12 @@ impl<'a> LowerMethod<'a> { self.verify_type(node.resolved_type, node.location); let reg = self.self_register; - - if self.register_is_available(reg) { - if self.self_has_moved_field() { - self.state.diagnostics.moved_variable( - SELF_NAME, - self.file(), - node.location, - ); - } - } else { - self.check_if_moved(reg, SELF_NAME, node.location); + if !self.register_is_available(reg) || self.self_has_moved_field() { + self.state.diagnostics.moved_variable( + SELF_NAME, + self.file(), + node.location, + ); } reg @@ -4463,20 +4458,21 @@ impl<'a> LowerMethod<'a> { }); match self.register_state(self_reg) { - // Fully moved: its fields are moved too, so there's nothing to drop. RegisterState::Moved => {} - // Maybe moved as a whole: `drop_register` drops it conditionally. RegisterState::MaybeMoved => { self.drop_register(self_reg, location); } - // Available but with some fields moved out: drop the remaining - // fields and free self's memory without running its dropper. + RegisterState::PartiallyMoved => { + self.drop_remaining_fields(&fields, location); + self.current_block_mut() + .drop_without_dropper(self_reg, location); + } RegisterState::Available if partially_moved => { self.drop_remaining_fields(&fields, location); self.current_block_mut() .drop_without_dropper(self_reg, location); } - RegisterState::Available | RegisterState::PartiallyMoved => { + RegisterState::Available => { self.drop_register(self_reg, location); } } @@ -4880,21 +4876,20 @@ impl<'a> LowerMethod<'a> { // Moving `self` as a whole also moves all of its fields. if register == self.self_register { - let fields: Vec = - self.field_mapping.values().cloned().collect(); - - for field in fields { - self.update_register_state(field, RegisterState::Moved); + for &field in self.field_mapping.values() { + self.register_states.set( + self.current_block, + field, + RegisterState::Moved, + ); } } } /// Returns `true` if any field of `self` has been (possibly) moved. fn self_has_moved_field(&mut self) -> bool { - let fields: Vec = - self.field_mapping.values().cloned().collect(); - - fields.into_iter().any(|reg| !self.register_is_available(reg)) + (0..self.field_mapping.len()) + .any(|i| !self.register_is_available(self.field_mapping[i])) } fn mark_register_as_available(&mut self, register: RegisterId) { diff --git a/std/fixtures/diagnostics/closure_explicitly_captures_partially_moved_self.inko b/std/fixtures/diagnostics/closure_explicitly_captures_partially_moved_self.inko new file mode 100644 index 000000000..8ec62425a --- /dev/null +++ b/std/fixtures/diagnostics/closure_explicitly_captures_partially_moved_self.inko @@ -0,0 +1,21 @@ +import std.drop (drop) + +type Inner {} + +type inline Outer { + let @a: Inner + let @b: Inner + + fn move submit { + drop(@a) + fn { self.helper }.call + } + + fn helper {} +} + +type async Main { + fn async main { + Outer(a: Inner(), b: Inner()).submit + } +} diff --git a/std/fixtures/diagnostics/closure_implicitly_captures_partially_moved_self.inko b/std/fixtures/diagnostics/closure_implicitly_captures_partially_moved_self.inko new file mode 100644 index 000000000..ade1d95bc --- /dev/null +++ b/std/fixtures/diagnostics/closure_implicitly_captures_partially_moved_self.inko @@ -0,0 +1,21 @@ +import std.drop (drop) + +type Inner {} + +type inline Outer { + let @a: Inner + let @b: Inner + + fn move submit { + drop(@a) + fn { helper }.call + } + + fn helper {} +} + +type async Main { + fn async main { + Outer(a: Inner(), b: Inner()).submit + } +} diff --git a/std/fixtures/diagnostics/implicit_method_call_after_field_move.inko b/std/fixtures/diagnostics/implicit_method_call_after_field_move.inko new file mode 100644 index 000000000..e42d2fe14 --- /dev/null +++ b/std/fixtures/diagnostics/implicit_method_call_after_field_move.inko @@ -0,0 +1,23 @@ +import std.drop (drop) + +type Inner {} + +type inline Outer { + let @a: Inner + let @b: Inner + + fn move submit { + drop(@a) + helper + } + + fn move helper {} +} + +type async Main { + fn async main { + Outer(a: Inner(), b: Inner()).submit + } +} + +# implicit_method_call_after_field_move.inko:11:5 error(moved): 'helper' can't be used, as 'self' has been moved diff --git a/std/fixtures/diagnostics/self_used_after_field_move.inko b/std/fixtures/diagnostics/self_used_after_field_move.inko new file mode 100644 index 000000000..72f332518 --- /dev/null +++ b/std/fixtures/diagnostics/self_used_after_field_move.inko @@ -0,0 +1,21 @@ +import std.drop (drop) + +type Inner {} + +type inline Outer { + let @a: Inner + let @b: Inner + + fn move submit { + drop(@a) + drop(self) + } +} + +type async Main { + fn async main { + Outer(a: Inner(), b: Inner()).submit + } +} + +# self_used_after_field_move.inko:11:10 error(moved): 'self' can't be used as it has been moved