diff --git a/crates/engine/src/game/effects/change_zone.rs b/crates/engine/src/game/effects/change_zone.rs index f5af62ef0f..e142524dcb 100644 --- a/crates/engine/src/game/effects/change_zone.rs +++ b/crates/engine/src/game/effects/change_zone.rs @@ -1121,7 +1121,7 @@ pub fn resolve( per_obj_ctx.source_id, ); let delivery_start = events.len(); - let stack_depth_before_zone_move = state.resolution_stack.len(); + let stack_depth_before_zone_move = state.resolution_stack.capture_child_boundary(); match process_one_zone_move_with_terminal(state, &per_obj_ctx, *obj_id, events) { crate::game::zone_pipeline::ZoneMoveTerminalResult::Completed(completion) => { logical_zone_change_group @@ -1996,7 +1996,7 @@ pub fn resolve_all( let anticipated_pause = anticipated_zone_change_delivery(state, obj_id, dest_zone, ability.source_id); let delivery_start = events.len(); - let stack_depth_before_zone_move = state.resolution_stack.len(); + let stack_depth_before_zone_move = state.resolution_stack.capture_child_boundary(); match crate::game::zone_pipeline::execute_zone_move_with_terminal_and_controller( state, obj_id, diff --git a/crates/engine/src/game/effects/each_player_copy_chosen.rs b/crates/engine/src/game/effects/each_player_copy_chosen.rs index 84957f812e..293613bd02 100644 --- a/crates/engine/src/game/effects/each_player_copy_chosen.rs +++ b/crates/engine/src/game/effects/each_player_copy_chosen.rs @@ -34,6 +34,7 @@ use crate::types::game_state::{ }; use crate::types::identifiers::ObjectId; use crate::types::player::PlayerId; +use crate::types::resolution::ChildStackDepth; /// Effect parameters threaded through the whole APNAP walk. Bundled so the /// resolver, the `SelectTargets` continuation, and the replacement-resume drain @@ -263,7 +264,7 @@ pub(crate) fn drive_from_copy( ); // Depth 1: skip the depth-0 chain prelude so per-resolution ledgers // (`last_created_token_ids`) are not reset. - let stack_depth_before_copy = state.resolution_stack.len(); + let stack_depth_before_copy = state.resolution_stack.capture_child_boundary(); super::resolve_ability_chain(state, ©_ability, events, 1)?; // CR 616.1: The copy parked a replacement-ordering choice. Do NOT read @@ -326,7 +327,7 @@ pub(crate) fn perform_counter_step_then_advance( params.source_id, player, ); - let stack_depth_before_counter = state.resolution_stack.len(); + let stack_depth_before_counter = state.resolution_stack.capture_child_boundary(); super::resolve_ability_chain(state, &counter_ability, events, 1)?; // CR 616.1: the counter placement paused for a replacement // ordering — park an `AwaitingCounters` continuation. @@ -398,9 +399,13 @@ pub(crate) fn drain_pending(state: &mut GameState, events: &mut Vec) fn park_each_player_copy_chosen_after_current_step( state: &mut GameState, pending: PendingEachPlayerCopyChosen, - stack_depth_before_step: usize, + stack_depth_before_step: ChildStackDepth, ) { - match state.resolution_stack.len().cmp(&stack_depth_before_step) { + match state + .resolution_stack + .capture_child_boundary() + .cmp(&stack_depth_before_step) + { std::cmp::Ordering::Less => { panic!("each-player-copy-chosen step removed a parent before it could be re-parked") } diff --git a/crates/engine/src/game/effects/mod.rs b/crates/engine/src/game/effects/mod.rs index 97d7305dd3..39554af2d7 100644 --- a/crates/engine/src/game/effects/mod.rs +++ b/crates/engine/src/game/effects/mod.rs @@ -32,8 +32,8 @@ use crate::types::identifiers::{ObjectId, TrackedSetId}; use crate::types::mana::ManaCost; use crate::types::player::{Player, PlayerId}; use crate::types::resolution::{ - AbilityContinuationFrame, FrameGate, OptionalEffectFrame, PendingRepeatedOptionalPayment, - RepeatedOptionalPaymentFrame, ResolutionFrame, + AbilityContinuationFrame, ChildStackDepth, FrameGate, OptionalEffectFrame, + PendingRepeatedOptionalPayment, RepeatedOptionalPaymentFrame, ResolutionFrame, }; use crate::types::zones::Zone; @@ -1184,11 +1184,11 @@ fn drain_active_repeat_until(state: &mut GameState) { fn park_repeat_until_after_inner_pause( state: &mut GameState, pending: crate::types::game_state::PendingRepeatUntil, - stack_depth_before_iteration: usize, + stack_depth_before_iteration: ChildStackDepth, ) { match state .resolution_stack - .len() + .capture_child_boundary() .cmp(&stack_depth_before_iteration) { std::cmp::Ordering::Less => { @@ -1362,7 +1362,7 @@ fn drain_pending_change_zone_iteration(state: &mut GameState, events: &mut Vec) { } else { &ability }; - let stack_depth_before_iteration = state.resolution_stack.len(); + let stack_depth_before_iteration = state.resolution_stack.capture_child_boundary(); // CR 608.2c + CR 109.5: Drive the FULL chain (parent effect + // sub_ability + line-1660 continuation wiring) for each resumed // iteration, mirroring iteration 0's path. Calling `resolve_effect` @@ -1680,11 +1680,11 @@ fn drain_active_repeat_for(state: &mut GameState, events: &mut Vec) { fn park_repeat_for_after_current_iteration( state: &mut GameState, pending: crate::types::game_state::PendingRepeatIteration, - stack_depth_before_iteration: usize, + stack_depth_before_iteration: ChildStackDepth, ) { match state .resolution_stack - .len() + .capture_child_boundary() .cmp(&stack_depth_before_iteration) { std::cmp::Ordering::Less => { @@ -7422,7 +7422,7 @@ fn drive_repeat_for_outermost( while iteration < base_iterations { let mut iter_ability = effective.clone(); iter_ability.repeat_for = None; - let stack_depth_before_iteration = state.resolution_stack.len(); + let stack_depth_before_iteration = state.resolution_stack.capture_child_boundary(); resolve_chain_body(state, &iter_ability, events, depth)?; if state.waiting_for != initial_waiting_for || (!initial_continuation_present && state.active_ability_continuation().is_some()) @@ -9520,7 +9520,7 @@ pub fn resolve_ability_chain( None => resolve_chain_body(state, ability, events, depth), Some(RepeatContinuation::ControllerChoice) => { let initial_waiting_for = state.waiting_for.clone(); - let stack_depth_before_iteration = state.resolution_stack.len(); + let stack_depth_before_iteration = state.resolution_stack.capture_child_boundary(); resolve_chain_body(state, ability, events, depth)?; if state.waiting_for != initial_waiting_for { // Inner pause: stash so the drain re-sets the repeat prompt @@ -9547,7 +9547,7 @@ pub fn resolve_ability_chain( stop_on_duplicate_exiled_names, }) => loop { let initial_waiting_for = state.waiting_for.clone(); - let stack_depth_before_iteration = state.resolution_stack.len(); + let stack_depth_before_iteration = state.resolution_stack.capture_child_boundary(); resolve_chain_body(state, ability, events, depth)?; if state.waiting_for != initial_waiting_for { park_repeat_until_after_inner_pause( @@ -9597,7 +9597,7 @@ pub fn resolve_ability_chain( // iteration's stale result. state.resolution_coin_flip = None; let initial_waiting_for = state.waiting_for.clone(); - let stack_depth_before_iteration = state.resolution_stack.len(); + let stack_depth_before_iteration = state.resolution_stack.capture_child_boundary(); resolve_chain_body(state, ability, events, depth)?; if state.waiting_for != initial_waiting_for { // Inner pause: stash the loop ability with its remaining cap @@ -11193,7 +11193,7 @@ fn resolve_chain_body( } else { effective }; - let stack_depth_before_iteration = state.resolution_stack.len(); + let stack_depth_before_iteration = state.resolution_stack.capture_child_boundary(); // CR 608.2d: A kind-driven or member-driven iteration whose action // is optional fires its per-iteration "you may" gate through the // full chain. All other iterations resolve the effect directly — diff --git a/crates/engine/src/game/effects/token_copy.rs b/crates/engine/src/game/effects/token_copy.rs index 8afd4c4072..10a5464a9f 100644 --- a/crates/engine/src/game/effects/token_copy.rs +++ b/crates/engine/src/game/effects/token_copy.rs @@ -21,6 +21,7 @@ use crate::types::identifiers::{CardId, ObjectId, ObjectIncarnationRef}; use crate::types::proposed_event::{ CopyTokenSpec, EtbTapState, ProposedEvent, TokenCharacteristics, }; +use crate::types::resolution::ChildStackDepth; use crate::types::resolved_commands::{ ResolvedCopyBodyModifications, ResolvedTokenBody, ResolvedTokenCreationCommand, }; @@ -285,7 +286,7 @@ fn drain_copy_token_resolution( if batch.count == 0 { continue; } - let stack_depth_before_batch = state.resolution_stack.len(); + let stack_depth_before_batch = state.resolution_stack.capture_child_boundary(); let spec = super::token::copy_probe_spec_for( batch.copy.source_id, batch.copy.controller, @@ -357,9 +358,13 @@ fn drain_copy_token_resolution( fn park_copy_token_after_current_batch( state: &mut GameState, pending: PendingCopyTokenResolution, - stack_depth_before_batch: usize, + stack_depth_before_batch: ChildStackDepth, ) { - match state.resolution_stack.len().cmp(&stack_depth_before_batch) { + match state + .resolution_stack + .capture_child_boundary() + .cmp(&stack_depth_before_batch) + { std::cmp::Ordering::Less => { panic!("copy-token batch removed a parent frame before it could be re-parked") } diff --git a/crates/engine/src/game/effects/vote.rs b/crates/engine/src/game/effects/vote.rs index 23073aaccc..65fa3904d8 100644 --- a/crates/engine/src/game/effects/vote.rs +++ b/crates/engine/src/game/effects/vote.rs @@ -28,6 +28,7 @@ use crate::types::game_state::{ }; use crate::types::identifiers::ObjectId; use crate::types::player::PlayerId; +use crate::types::resolution::ChildStackDepth; use super::resolve_ability_chain; use crate::game::ability_utils::build_resolved_from_def; @@ -381,7 +382,7 @@ pub fn resolve_tally( // If a ballot parks an interactive choice (e.g. ChooseFromZoneChoice), // stash remaining voters and return early; the drain function resumes. let initial_waiting_for = state.waiting_for.clone(); - let stack_depth_before_ballot = state.resolution_stack.len(); + let stack_depth_before_ballot = state.resolution_stack.capture_child_boundary(); let mut remaining_voters: Vec = choice_ballots.clone(); while let Some(voter) = remaining_voters.first().copied() { @@ -654,7 +655,7 @@ pub(crate) fn drain_active_vote_ballot(state: &mut GameState, events: &mut Vec { panic!("vote ballot removed a parent frame before it could be re-parked") } diff --git a/crates/engine/src/game/engine_debug.rs b/crates/engine/src/game/engine_debug.rs index ec8885ca3f..cb7ed9bf29 100644 --- a/crates/engine/src/game/engine_debug.rs +++ b/crates/engine/src/game/engine_debug.rs @@ -971,7 +971,7 @@ fn drain_debug_card_entries( events: &mut Vec, ) { while pending.remaining > 0 && matches!(state.waiting_for, WaitingFor::Priority { .. }) { - let child_stack_start = state.resolution_stack.len(); + let child_stack_start = state.resolution_stack.capture_child_boundary(); let object_id = materialize_debug_card( state, &pending.source, @@ -986,9 +986,9 @@ fn drain_debug_card_entries( state.waiting_for = entry.waiting_for; if !matches!(state.waiting_for, WaitingFor::Priority { .. }) - || state.resolution_stack.len() > child_stack_start + || state.resolution_stack.capture_child_boundary() > child_stack_start { - if state.resolution_stack.len() > child_stack_start { + if state.resolution_stack.capture_child_boundary() > child_stack_start { state .insert_debug_card_entries_parent_at_child_boundary(pending, child_stack_start) .expect("debug-card parent must sit below the entry child stack"); diff --git a/crates/engine/src/types/game_state.rs b/crates/engine/src/types/game_state.rs index af563e46e3..bf07335011 100644 --- a/crates/engine/src/types/game_state.rs +++ b/crates/engine/src/types/game_state.rs @@ -48,9 +48,10 @@ use super::replacements::ReplacementEvent; #[cfg(debug_assertions)] use super::resolution::debug_assert_runtime_resolution_invariants; use super::resolution::{ - AbilityContinuationFrame, ChangeZoneFrame, FrameGate, MultiDrawFrame, OptionalEffectFrame, - PendingCoinFlip, PendingMutateMerge, PendingProliferateActions, RepeatedOptionalPaymentFrame, - ResolutionFrame, ResolutionStack, ResolutionStackError, ResolutionStateWire, + AbilityContinuationFrame, ChangeZoneFrame, ChildStackDepth, FrameGate, MultiDrawFrame, + OptionalEffectFrame, PendingCoinFlip, PendingMutateMerge, PendingProliferateActions, + RepeatedOptionalPaymentFrame, ResolutionFrame, ResolutionStack, ResolutionStackError, + ResolutionStateWire, }; use super::resolved_commands::{ ManaPaymentRecipient, ResolvedContinuousEffectCommand, @@ -18826,9 +18827,13 @@ impl GameState { pub fn push_change_zone_iteration_after_child( &mut self, pending: PendingChangeZoneIteration, - child_stack_start: usize, + child_stack_start: ChildStackDepth, ) { - match self.resolution_stack.len().cmp(&child_stack_start) { + match self + .resolution_stack + .capture_child_boundary() + .cmp(&child_stack_start) + { std::cmp::Ordering::Less => { panic!("ChangeZone move removed a parent before it could be parked") } @@ -18860,9 +18865,13 @@ impl GameState { pub fn replace_active_change_zone_iteration_after_child( &mut self, pending: PendingChangeZoneIteration, - child_stack_start: usize, + child_stack_start: ChildStackDepth, ) { - match self.resolution_stack.len().cmp(&child_stack_start) { + match self + .resolution_stack + .capture_child_boundary() + .cmp(&child_stack_start) + { std::cmp::Ordering::Less => { panic!("ChangeZone move removed its active owner before it could be re-parked") } @@ -19063,7 +19072,7 @@ impl GameState { pub fn insert_copy_token_parent_at_child_boundary( &mut self, pending: PendingCopyTokenResolution, - child_stack_start: usize, + child_stack_start: ChildStackDepth, ) -> Result<(), ResolutionStackError> { self.resolution_stack .insert_copy_token_parent_at_child_boundary(pending, child_stack_start) @@ -19093,7 +19102,7 @@ impl GameState { pub fn insert_debug_card_entries_parent_at_child_boundary( &mut self, pending: PendingDebugCardEntries, - child_stack_start: usize, + child_stack_start: ChildStackDepth, ) -> Result<(), ResolutionStackError> { self.resolution_stack .insert_debug_card_entries_parent_at_child_boundary(pending, child_stack_start) @@ -19141,7 +19150,7 @@ impl GameState { pub fn insert_each_player_copy_chosen_parent_at_child_boundary( &mut self, pending: PendingEachPlayerCopyChosen, - child_stack_start: usize, + child_stack_start: ChildStackDepth, ) -> Result<(), ResolutionStackError> { self.resolution_stack .insert_each_player_copy_chosen_parent_at_child_boundary(pending, child_stack_start) @@ -19184,7 +19193,7 @@ impl GameState { pub fn insert_repeat_for_parent_at_child_boundary( &mut self, pending: PendingRepeatIteration, - child_stack_start: usize, + child_stack_start: ChildStackDepth, ) -> Result<(), ResolutionStackError> { self.resolution_stack.insert_parent_at_child_boundary( super::resolution::ResolutionFrame::RepeatFor(pending), @@ -19227,7 +19236,7 @@ impl GameState { pub fn insert_repeat_until_parent_at_child_boundary( &mut self, pending: PendingRepeatUntil, - child_stack_start: usize, + child_stack_start: ChildStackDepth, ) -> Result<(), ResolutionStackError> { self.resolution_stack.insert_parent_at_child_boundary( super::resolution::ResolutionFrame::RepeatUntil(pending), @@ -19274,7 +19283,7 @@ impl GameState { pub fn insert_vote_ballot_parent_at_child_boundary( &mut self, pending: PendingVoteBallotIteration, - child_stack_start: usize, + child_stack_start: ChildStackDepth, ) -> Result<(), ResolutionStackError> { self.resolution_stack.insert_parent_at_child_boundary( super::resolution::ResolutionFrame::VoteBallot(pending), diff --git a/crates/engine/src/types/mod.rs b/crates/engine/src/types/mod.rs index d2edb0fe59..05665570d3 100644 --- a/crates/engine/src/types/mod.rs +++ b/crates/engine/src/types/mod.rs @@ -81,10 +81,10 @@ pub use proposed_event::{AppliedReplacementKey, ProposedEvent, ReplacementId}; pub use replacements::ReplacementEvent; pub use replay::{RecordedAction, ReplayHeader, ReplayLog, REPLAY_FORMAT_VERSION}; pub use resolution::{ - AbilityContinuationFrame, ChangeZoneFrame, DirectChoiceGate, FrameGate, FrameKind, - MultiDrawFrame, OptionalEffectFrame, PerCategoryZoneChoiceFrame, RepeatedOptionalPaymentFrame, - ResolutionFrame, ResolutionStack, ResolutionStackError, ResolutionStateWire, - RESOLUTION_STATE_WIRE_VERSION, + AbilityContinuationFrame, ChangeZoneFrame, ChildStackDepth, DirectChoiceGate, FrameGate, + FrameKind, MultiDrawFrame, OptionalEffectFrame, PerCategoryZoneChoiceFrame, + RepeatedOptionalPaymentFrame, ResolutionFrame, ResolutionStack, ResolutionStackError, + ResolutionStateWire, RESOLUTION_STATE_WIRE_VERSION, }; pub use resolved_commands::{ ManaPaymentRecipient, ProducedManaUnit, ResolvedCommandJournalEntry, ResolvedCommandOrdinal, diff --git a/crates/engine/src/types/resolution.rs b/crates/engine/src/types/resolution.rs index aefd7eb407..2dffd274d9 100644 --- a/crates/engine/src/types/resolution.rs +++ b/crates/engine/src/types/resolution.rs @@ -11,6 +11,7 @@ use std::collections::HashSet; use serde::{Deserialize, Deserializer, Serialize, Serializer}; use serde_json::{Map, Value}; +pub use frame_vec::ChildStackDepth; use frame_vec::{FrameSlot, FrameVec}; use crate::types::ability::{AbilityDefinition, DiscardedCardResult, ResolvedAbility, TargetRef}; @@ -469,19 +470,19 @@ pub enum ResolutionStackError { "child-stack boundary {child_stack_start} is not below the active child stack of length {stack_len}" )] InvalidChildBoundary { - child_stack_start: usize, + child_stack_start: ChildStackDepth, stack_len: usize, }, #[error( "child-stack boundary {child_stack_start} has {actual:?} immediately below it, expected {expected:?}" )] UnexpectedChildBoundaryParent { - child_stack_start: usize, + child_stack_start: ChildStackDepth, expected: FrameKind, actual: FrameKind, }, #[error("child-stack boundary {child_stack_start} does not retain the ChangeZone owner being re-parked")] - MismatchedChangeZoneBoundaryOwner { child_stack_start: usize }, + MismatchedChangeZoneBoundaryOwner { child_stack_start: ChildStackDepth }, #[error("top frame {frame:?} does not match waiting prompt {waiting_for}")] PromptMismatch { frame: FrameKind, @@ -529,8 +530,10 @@ pub enum ParkedFramePlacement { /// the type system rather than by convention: [`FrameVec`] hands out positions /// only as opaque [`FrameSlot`]s minted from the top, from an adjacent frame, or /// from a [`PostReplacementFrameId`]. A frame located any other way — by -/// scanning, by arithmetic on the length — yields a `usize` that no accessor -/// accepts, so a positional search cannot be spent even when it can be written. +/// scanning, by arithmetic on the length — yields a `usize`, and the only +/// thing that accepts one is [`FrameVec::frame_at_offset`], which hands back +/// a frame to read and never a position to address, so a positional search +/// still cannot be spent on a mutation even when it can be written. /// See [`frame_vec`] for why that replaced a grep-based guard. #[derive(Debug, Clone, Default, PartialEq, Serialize, Deserialize)] pub struct ResolutionStack { @@ -619,6 +622,15 @@ impl ResolutionStack { self.frames.len() } + /// Record the current stack depth, before running a child producer. + /// + /// This is the public face of the only [`ChildStackDepth`] constructor; + /// even this module cannot build one directly, because the field is private + /// to [`frame_vec`] and this module is its parent, not its descendant. + pub fn capture_child_boundary(&self) -> ChildStackDepth { + self.frames.capture_depth() + } + pub fn last(&self) -> Option<&ResolutionFrame> { self.frames.last() } @@ -1117,7 +1129,7 @@ impl ResolutionStack { pub fn insert_change_zone_parent_at_child_boundary( &mut self, pending: PendingChangeZoneIteration, - child_stack_start: usize, + child_stack_start: ChildStackDepth, ) -> Result<(), ResolutionStackError> { let Some(boundary) = self.frames.slot_at_captured_depth(child_stack_start) else { return Err(ResolutionStackError::InvalidChildBoundary { @@ -1173,7 +1185,7 @@ impl ResolutionStack { pub fn replace_change_zone_parent_at_child_boundary( &mut self, pending: PendingChangeZoneIteration, - child_stack_start: usize, + child_stack_start: ChildStackDepth, ) -> Result<(), ResolutionStackError> { let logical_group_id = pending.logical_zone_change_group.logical_group_id; let boundary = self.frames.slot_at_captured_depth(child_stack_start); @@ -1570,7 +1582,7 @@ impl ResolutionStack { pub fn insert_copy_token_parent_at_child_boundary( &mut self, pending: PendingCopyTokenResolution, - child_stack_start: usize, + child_stack_start: ChildStackDepth, ) -> Result<(), ResolutionStackError> { self.insert_parent_at_child_boundary(ResolutionFrame::CopyToken(pending), child_stack_start) } @@ -1614,7 +1626,7 @@ impl ResolutionStack { pub fn insert_debug_card_entries_parent_at_child_boundary( &mut self, pending: PendingDebugCardEntries, - child_stack_start: usize, + child_stack_start: ChildStackDepth, ) -> Result<(), ResolutionStackError> { self.insert_parent_at_child_boundary( ResolutionFrame::DebugCardEntries(Box::new(pending)), @@ -1692,7 +1704,7 @@ impl ResolutionStack { pub fn insert_each_player_copy_chosen_parent_at_child_boundary( &mut self, pending: PendingEachPlayerCopyChosen, - child_stack_start: usize, + child_stack_start: ChildStackDepth, ) -> Result<(), ResolutionStackError> { self.insert_parent_at_child_boundary( ResolutionFrame::EachPlayerCopyChosen(pending), @@ -3035,7 +3047,7 @@ impl ResolutionStack { pub fn insert_parent_at_child_boundary( &mut self, frame: ResolutionFrame, - child_stack_start: usize, + child_stack_start: ChildStackDepth, ) -> Result<(), ResolutionStackError> { let stack_len = self.frames.len(); if stack_len == 0 { @@ -5273,8 +5285,9 @@ mod tests { stack.insert_parent_of_active(continuation_frame(1)), Err(ResolutionStackError::NoActiveChild) ); + let empty_boundary = stack.capture_child_boundary(); assert_eq!( - stack.insert_parent_at_child_boundary(continuation_frame(1), 0), + stack.insert_parent_at_child_boundary(continuation_frame(1), empty_boundary), Err(ResolutionStackError::NoActiveChild) ); @@ -5295,10 +5308,11 @@ mod tests { FrameKind::AbilityContinuation, ] ); + let at_top = stack.capture_child_boundary(); assert_eq!( - stack.insert_parent_at_child_boundary(continuation_frame(3), stack.len()), + stack.insert_parent_at_child_boundary(continuation_frame(3), at_top), Err(ResolutionStackError::InvalidChildBoundary { - child_stack_start: stack.len(), + child_stack_start: at_top, stack_len: stack.len(), }) ); @@ -5731,6 +5745,21 @@ mod tests { ); } + #[test] + fn captured_child_boundaries_order_by_stack_growth() { + let mut stack = ResolutionStack::default(); + let empty = stack.capture_child_boundary(); + + stack.push_inner(continuation_frame(1)); + let after_push = stack.capture_child_boundary(); + assert!(after_push > empty); + + stack + .pop_expected(FrameKind::AbilityContinuation) + .expect("the pushed continuation is the top frame"); + assert_eq!(stack.capture_child_boundary(), empty); + } + #[test] fn change_zone_repark_keeps_a_distinct_nested_change_zone_child() { let ResolutionFrame::ChangeZone(outer) = change_zone_frame(160) else { @@ -5754,10 +5783,11 @@ mod tests { let mut stack = ResolutionStack::default(); stack.push_inner(ResolutionFrame::ChangeZone(outer)); + let boundary = stack.capture_child_boundary(); stack.push_inner(ResolutionFrame::ChangeZone(child)); stack - .replace_change_zone_parent_at_child_boundary(replacement, 1) + .replace_change_zone_parent_at_child_boundary(replacement, boundary) .expect("the outer ChangeZone owner remains immediately below its child"); let frames = stack.iter().collect::>(); diff --git a/crates/engine/src/types/resolution/frame_vec.rs b/crates/engine/src/types/resolution/frame_vec.rs index d3d16f9422..238c6ff732 100644 --- a/crates/engine/src/types/resolution/frame_vec.rs +++ b/crates/engine/src/types/resolution/frame_vec.rs @@ -18,19 +18,25 @@ //! [`FrameVec::above`] and [`FrameVec::by_id`], which are the three sanctioned //! access modes above, plus [`FrameVec::slot_at_captured_depth`]. //! -//! Reading or mutating a frame requires a slot. A `usize` obtained by scanning +//! [`ChildStackDepth`] is the module's second opaque value, minted by exactly +//! one method: [`FrameVec::capture_depth`], which takes no argument and reads +//! the stack's own length. +//! +//! Reading or mutating a frame requires a slot. The one depth-addressed door, +//! [`FrameVec::slot_at_captured_depth`], requires a [`ChildStackDepth`], and so +//! does [`FrameVec::insert_at_child_boundary`]. A `usize` obtained by scanning //! — `iter().position(..)`, arithmetic on `len()`, a literal — still compiles, -//! and the only thing that will accept it is `slot_at_captured_depth`, whose -//! argument is contractually a stack length recorded before a child producer -//! ran. So the guarantee is precisely this: positional addressing cannot be -//! reached by accident or by ordinary-looking code, and the single way to reach -//! it deliberately names itself at the call site. That is weaker than "no way -//! to spend it" and stronger than a lint, and the difference matters enough to -//! state exactly — see that method for why the door cannot close while the -//! depth arrives as a bare `usize`. +//! and it can no longer be spent on a mutation: it is neither a slot nor a +//! depth, and neither can be built from one. The one method that still accepts +//! one hands back a frame to read. //! -//! [`FrameVec::frame_at_offset`] takes a `usize` too, but returns a frame and -//! never a slot, so it cannot widen addressing. +//! [`FrameVec::frame_at_offset`] is the only method here that still takes a +//! bare `usize`. It returns a frame and never a slot, so it cannot widen +//! addressing, and `scripts/check-resolution-frame-boundaries.sh` fails if a +//! second bare-`usize` parameter appears in this module, wherever in the +//! parameter list it sits — behind a closure parameter or a generic list +//! included. That check is a text scan, so it does not see, for example, a +//! `usize` renamed by a type alias or a method a macro generated. //! //! Two operations are absent rather than restricted. `remove`, `swap_remove`, //! `retain`, `drain`, `truncate` and `clear` have no wrapper here because the @@ -63,6 +69,34 @@ use super::ResolutionFrame; #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub(super) struct FrameSlot(usize); +/// A resolution-stack DEPTH recorded before a child producer ran. +/// +/// The field is private to this module, so a `ChildStackDepth` can only come +/// from [`FrameVec::capture_depth`] — a real read of the current frame count. +/// A `usize` from `iter().position(..)`, from arithmetic on `len()`, or from a +/// literal cannot become one, which is what closes the last positional door +/// into frame addressing: [`FrameVec::slot_at_captured_depth`] and +/// [`FrameVec::insert_at_child_boundary`] are its only consumers, and neither +/// accepts a bare `usize` any more. +/// +/// Ordering compares depths, so a capture taken now can be compared against one +/// taken earlier to ask how far the stack has grown — the only arithmetic any +/// caller performs on it. +/// +/// A depth is a recorded length, not a handle. Frames can retire below it while +/// the child producer runs, so the consumers return [`Option`] and `bool` +/// rather than asserting. The type proves the number came from a real stack +/// read; it does not prove the boundary is still live, and — since two nested +/// producers each hold one — it does not prove you are spending the right one. +#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord)] +pub struct ChildStackDepth(usize); + +impl std::fmt::Display for ChildStackDepth { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "{}", self.0) + } +} + /// The backing storage for [`ResolutionStack`]'s frames. /// /// Serialized transparently, so the wire format is exactly the `Vec` this @@ -144,25 +178,31 @@ impl FrameVec { .map(FrameSlot) } + /// Record the current stack depth, before running a child producer. + /// + /// The sole constructor of [`ChildStackDepth`]. It takes no argument by + /// design: the value is the stack's own length, so there is no number a + /// caller could supply. + pub(super) fn capture_depth(&self) -> ChildStackDepth { + ChildStackDepth(self.frames.len()) + } + /// The slot at a stack DEPTH captured before a child producer ran. /// - /// This is the ONLY method that turns a `usize` into an addressable + /// This is the only method that turns a captured depth into an addressable /// position, and it exists because the depth originates far outside this - /// module: an effect records `resolution_stack.len()`, runs a child - /// producer, and hands the recorded length back so the owner can be parked - /// beneath the child stack that producer raised. `game/effects/`, - /// `game/casting_costs.rs` and their neighbours capture it in roughly - /// thirty-five places. + /// module: an effect calls `ResolutionStack::capture_child_boundary`, runs a + /// child producer, and hands the recorded depth back so the owner can be + /// parked beneath the child stack that producer raised. Fifteen origins + /// capture it that way — five files under `game/effects/`, plus + /// `game/engine_debug.rs`. /// - /// The argument must be such a captured length. Passing a scan result would - /// compile — the door cannot be closed entirely while the depth arrives as - /// a bare `usize` — but it would read as `slot_at_captured_depth(position)`, - /// which states the violation at the call site instead of hiding it behind - /// an ordinary-looking `get(i)`. Closing it completely means giving that - /// captured depth its own type at every one of those origins, which is a - /// separate change with a much wider blast radius than this one. - pub(super) fn slot_at_captured_depth(&self, depth: usize) -> Option { - (depth < self.frames.len()).then_some(FrameSlot(depth)) + /// The argument is a [`ChildStackDepth`], which only + /// [`FrameVec::capture_depth`] produces, so a scan result cannot be passed + /// here at all: the door that used to be open at the call site is now closed + /// at the type. Each origin holds its depth from capture to spend. + pub(super) fn slot_at_captured_depth(&self, depth: ChildStackDepth) -> Option { + (depth.0 < self.frames.len()).then_some(FrameSlot(depth.0)) } /// Read the frame at a raw offset during a full-stack walk. @@ -209,18 +249,20 @@ impl FrameVec { /// frames above it are exactly the child stack the producer created. It /// returns nothing addressable, so a depth cannot be laundered into a /// [`FrameSlot`] by inserting with it. + /// The depth itself is unforgeable now, so both directions are closed: a + /// scanned `usize` cannot become a depth, and a depth cannot become a slot. /// /// Returns `false` when `depth` does not name a boundary with at least one /// child frame above it; the caller reports that as a typed error. pub(super) fn insert_at_child_boundary( &mut self, - depth: usize, + depth: ChildStackDepth, frame: ResolutionFrame, ) -> bool { - if depth >= self.frames.len() { + if depth.0 >= self.frames.len() { return false; } - self.frames.insert(depth, frame); + self.frames.insert(depth.0, frame); true } } diff --git a/scripts/check-resolution-frame-boundaries.sh b/scripts/check-resolution-frame-boundaries.sh index 2833b6a689..4e77a81a6f 100755 --- a/scripts/check-resolution-frame-boundaries.sh +++ b/scripts/check-resolution-frame-boundaries.sh @@ -9,12 +9,14 @@ # identity-addressed access. Removing an arbitrary index, or searching the # vector to decide what to mutate, breaks that authority. # -# That rule is NO LONGER ENFORCED HERE. It is enforced by the type system: -# `ResolutionStack::frames` is a `FrameVec` whose backing `Vec` is private to -# `crates/engine/src/types/resolution/frame_vec.rs`, every accessor takes an -# opaque `FrameSlot`, and the removal operations have no wrapper. A positional -# scan still compiles and still cannot be spent, because it produces a `usize` -# and nothing accepts one. The distinction being drawn is unchanged — +# That rule is enforced by the type system, with one residual structural guard +# here. `ResolutionStack::frames` is a `FrameVec` whose backing `Vec` is +# private to `crates/engine/src/types/resolution/frame_vec.rs`, every operation +# that addresses a frame for mutation takes an opaque `FrameSlot` or an opaque +# `ChildStackDepth`, and the removal operations have no wrapper. A positional +# scan still compiles, and the one method that accepts the `usize` it yields is +# `frame_at_offset`, which hands back a frame to read and never a position to +# address. The distinction being drawn is unchanged — # positional/adjacency-inferred access GUESSES a structural relationship the # stack does not guarantee, while identity-addressed access asserts one, since # ids come from a monotonic allocator that never rewinds and a stale id matches @@ -25,10 +27,12 @@ # This script previously grepped for that rule because `frames` was private to # a 7,000-line module and Rust privacy is module-scoped, so "private" bought # nothing against the code beside it. Shrinking the module to ~230 lines is what -# made the privacy real. What remains below is a single structural check that -# the design itself is intact: `FrameSlot` must be mintable only by the -# documented methods, since a new one would reopen positional addressing -# without any compiler error to show for it. +# made the privacy real. This script carries five structural checks that the +# design itself is intact: `FrameSlot` and `ChildStackDepth` must each be +# mintable only by their documented methods, the two depth-addressed doors +# must each take a `ChildStackDepth`, and `frame_at_offset` must stay the +# module's only bare-`usize` parameter — any of those breaking would reopen +# positional addressing without any compiler error to show for it. set -euo pipefail @@ -267,6 +271,70 @@ def function_span(source: str, function_name: str) -> tuple[int, int]: return block_span(source, match) +def rust_fn_signatures(source: str) -> list[tuple[str, str, str]]: + """`(name, params, tail)` for every `fn` in `source`. + + `params` is the parameter list with the contents of every APPLIED group + elided -- a bracket whose opener directly follows an identifier, i.e. + `Fn(..)` / `fn(..)` / `Foo[..]`. A bare tuple type `(usize, u8)` is not + applied, so its contents are kept. `tail` is the text between the + parameter list and the body's `{` (or a `;`): the return type and any + `where` clause. + + This is a text scan, not a parser: it does not see, for example, a `usize` + behind a type alias, a method a macro generated, or a raw identifier. + """ + out: list[tuple[str, str, str]] = [] + n = len(source) + for match in re.finditer(r"\bfn\s+(\w+)", source): + i = match.end() + while i < n and source[i].isspace(): + i += 1 + if i < n and source[i] == "<": # skip a generic parameter list + angle = 0 + while i < n: + if source[i] == "<": + angle += 1 + elif source[i] == ">" and source[i - 1] != "-": # not the `>` of `->` + angle -= 1 + if angle == 0: + i += 1 + break + i += 1 + while i < n and source[i].isspace(): + i += 1 + if i >= n or source[i] != "(": + continue + depth = 0 + elide_from = 0 + params: list[str] = [] + while i < n: + char = source[i] + if char in "([{": + depth += 1 + if depth > 1 and not elide_from and re.match(r"\w", source[i - 1]): + elide_from = depth + i += 1 + continue + if char in ")]}": + if elide_from == depth: + elide_from = 0 + depth -= 1 + i += 1 + if depth == 0: + break + continue + if not elide_from: + params.append(char) + i += 1 + tail: list[str] = [] + while i < n and source[i] not in "{;": + tail.append(source[i]) + i += 1 + out.append((match.group(1), "".join(params), "".join(tail))) + return out + + def fail(failures: list[str], path: Path, source: str, offset: int, message: str) -> None: failures.append(f" {path}:{line_number(source, offset)}: {message}") @@ -361,24 +429,29 @@ for file_name in files: # The frame-search and frame-removal scans that used to run here are gone, # because the type system now enforces what they policed. # `ResolutionStack::frames` is a `FrameVec` whose backing `Vec` is private - # to `types/resolution/frame_vec.rs`; every accessor takes an opaque - # `FrameSlot`, and the removal operations have no wrapper at all. A - # positional scan still compiles, and still cannot be spent: it yields a - # `usize`, and nothing accepts one. + # to `types/resolution/frame_vec.rs`; every operation that addresses a + # frame for mutation takes an opaque `FrameSlot` or an opaque + # `ChildStackDepth`, and the removal operations have no wrapper at all. A + # positional scan still compiles, and the one method that accepts the + # `usize` it yields is `frame_at_offset`, which hands back a frame to read + # and never a position to address. # - # That argument holds only while `FrameSlot` values come from the minting - # methods below. A new `fn ... -> Option` in that module would - # reopen positional addressing with no compiler error to show for it, so - # that -- and only that -- is what a grep still has to protect. The rule is - # now one structural check on a ~230-line module rather than a search-shape - # scan over 7,000 lines. + # That argument holds only while `FrameSlot` and `ChildStackDepth` values + # come from their minting methods, and while `frame_at_offset` stays the + # only bare-`usize` parameter in that module. A new + # `fn ... -> Option`, a second mint of a depth, or a new + # bare-`usize` parameter would each reopen positional addressing with no + # compiler error to show for it, so that -- and only that -- is what a grep + # still has to protect: five structural checks on a ~270-line module rather + # than a search-shape scan over 7,000 lines. # - # `slot_at_captured_depth` is deliberately on this list: it is the single - # `usize` door, and it exists because an effect records - # `resolution_stack.len()` before running a child producer and hands that - # length back afterwards. Giving that captured depth its own type at all of - # its origins would remove the door entirely; until then it is named so - # that misuse reads as misuse at the call site. + # `slot_at_captured_depth` stays on this list, and its argument is no + # longer a bare `usize`: the captured depth has its own opaque type, + # `ChildStackDepth`, minted only by `FrameVec::capture_depth`. The deferral + # this comment used to record -- giving that captured depth its own type at + # all of its origins -- has been taken, so the sanctioned `FrameSlot` + # minting set is unchanged while the one door it names is closed at the + # type. frame_vec_source = (root / "crates/engine/src/types/resolution/frame_vec.rs").read_text() minting = set(re.findall(r"fn\s+(\w+)\s*\([^)]*\)\s*->[^{;]*\bFrameSlot\b", frame_vec_source)) sanctioned_minting = {"top", "below", "above", "by_id", "slot_at_captured_depth"} @@ -391,6 +464,58 @@ for file_name in files: f"unexpected: {added}; missing: {missing}" ) + # Four more structural checks, on the second opaque value this module + # mints. (1) `ChildStackDepth` may be minted only by `capture_depth`. + # (2) `slot_at_captured_depth` and (3) `insert_at_child_boundary` must each + # take one. (4) `frame_at_offset` must remain the module's only + # bare-`usize` parameter, since a new one would reopen positional mutation + # with no compiler error to show for it -- the same hazard the `FrameSlot` + # minting check exists for, on the parameter axis instead of the return + # axis. + # + # All four read `rust_fn_signatures`, which splits a signature into its + # top-level parameter list and its return text, so a `usize` sitting after + # a nested `)` -- `pick: impl Fn(&ResolutionFrame) -> bool, depth: usize` + # -- is still seen. It is a text scan, not a compiler: it does not see, for + # example, a `usize` renamed by a type alias or a method a macro generated. + signatures = rust_fn_signatures(frame_vec_source) + + depth_minting = { + name for name, _params, tail in signatures + if re.search(r"\bChildStackDepth\b", tail) + } + if depth_minting != {"capture_depth"}: + added = ", ".join(sorted(depth_minting - {"capture_depth"})) or "none" + missing = "none" if "capture_depth" in depth_minting else "capture_depth" + failures.append( + " crates/engine/src/types/resolution/frame_vec.rs: ChildStackDepth may " + f"be minted only by capture_depth; unexpected: {added}; missing: {missing}" + ) + + depth_typed = { + name for name, params, _tail in signatures + if re.search(r"\bChildStackDepth\b", params) + } + for door in ("slot_at_captured_depth", "insert_at_child_boundary"): + if door not in depth_typed: + failures.append( + " crates/engine/src/types/resolution/frame_vec.rs: " + f"{door} must take a ChildStackDepth, not a bare usize" + ) + + usize_params = { + name for name, params, _tail in signatures + if re.search(r"\busize\b", params) + } + if usize_params != {"frame_at_offset"}: + added = ", ".join(sorted(usize_params - {"frame_at_offset"})) or "none" + missing = "none" if "frame_at_offset" in usize_params else "frame_at_offset" + failures.append( + " crates/engine/src/types/resolution/frame_vec.rs: frame_at_offset must " + "be the only method taking a bare usize (it returns a frame, never a " + f"slot); unexpected: {added}; missing: {missing}" + ) + if failures: print("Resolution-frame boundary guard failed:", file=sys.stderr) print("\n".join(failures), file=sys.stderr)