diff --git a/crates/engine/src/types/resolution.rs b/crates/engine/src/types/resolution.rs index 4f5c5d3a42..11b17db419 100644 --- a/crates/engine/src/types/resolution.rs +++ b/crates/engine/src/types/resolution.rs @@ -4,11 +4,15 @@ //! families use [`ResolutionStack`] as their runtime authority; unmigrated //! families remain in their legacy `GameState` slots until their Phase-3 turn. +mod frame_vec; + use std::collections::HashSet; use serde::{Deserialize, Deserializer, Serialize, Serializer}; use serde_json::{Map, Value}; +use frame_vec::{FrameSlot, FrameVec}; + use crate::types::ability::{AbilityDefinition, DiscardedCardResult, ResolvedAbility, TargetRef}; use crate::types::events::GameEvent; use crate::types::game_state::{ @@ -435,12 +439,16 @@ pub enum ResolutionStackError { /// An ordered, LIFO stack of suspended resolution work. /// -/// Its backing storage is intentionally private: all future mutations must -/// pass through the checked structural APIs rather than searching for or -/// removing a non-top parent. +/// Its backing storage is intentionally private, and the privacy is enforced by +/// 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. +/// See [`frame_vec`] for why that replaced a grep-based guard. #[derive(Debug, Clone, Default, PartialEq, Serialize, Deserialize)] pub struct ResolutionStack { - frames: Vec, + frames: FrameVec, /// The monotonic draw-frame allocator survives an abandoned MultiDraw /// frame, so a stale captured ID cannot alias a later instruction. #[serde(default)] @@ -708,19 +716,16 @@ impl ResolutionStack { &mut self, discard_id: DiscardFrameId, ) -> Option<&mut AbilityContinuationFrame> { - let continuation_index = self.frames.len().checked_sub(1)?; - let discard_index = continuation_index.checked_sub(1)?; - match ( - self.frames.get(discard_index), - self.frames.get(continuation_index), - ) { + let continuation = self.frames.top()?; + let discard = self.frames.below(continuation)?; + match (self.frames.get(discard), self.frames.get(continuation)) { ( Some(ResolutionFrame::Discard(discard)), Some(ResolutionFrame::AbilityContinuation(_)), ) if discard.id == discard_id => {} _ => return None, } - match self.frames.get_mut(continuation_index) { + match self.frames.get_mut(continuation) { Some(ResolutionFrame::AbilityContinuation(continuation)) => Some(continuation), Some(_) | None => { unreachable!("checked direct continuation must retain its frame kind") @@ -736,19 +741,16 @@ impl ResolutionStack { &mut self, discard_id: DiscardFrameId, ) -> Option<&mut DiscardFrame> { - let continuation_index = self.frames.len().checked_sub(1)?; - let discard_index = continuation_index.checked_sub(1)?; - match ( - self.frames.get(discard_index), - self.frames.get(continuation_index), - ) { + let continuation = self.frames.top()?; + let discard = self.frames.below(continuation)?; + match (self.frames.get(discard), self.frames.get(continuation)) { ( Some(ResolutionFrame::Discard(discard)), Some(ResolutionFrame::AbilityContinuation(_)), ) if discard.id == discard_id => {} _ => return None, } - match self.frames.get_mut(discard_index) { + match self.frames.get_mut(discard) { Some(ResolutionFrame::Discard(discard)) => Some(discard), Some(_) | None => unreachable!("checked direct discard must retain its frame kind"), } @@ -762,21 +764,19 @@ impl ResolutionStack { &mut self, discard_id: DiscardFrameId, ) -> Option<&mut DiscardFrame> { - let active_index = self.frames.len().checked_sub(1)?; - let discard_index = match self.frames.get(active_index) { - Some(ResolutionFrame::Discard(discard)) if discard.id == discard_id => active_index, + let active = self.frames.top()?; + let discard = match self.frames.get(active) { + Some(ResolutionFrame::Discard(discard)) if discard.id == discard_id => active, Some(ResolutionFrame::AbilityContinuation(_)) => { - let discard_index = active_index.checked_sub(1)?; - match self.frames.get(discard_index) { - Some(ResolutionFrame::Discard(discard)) if discard.id == discard_id => { - discard_index - } + let below = self.frames.below(active)?; + match self.frames.get(below) { + Some(ResolutionFrame::Discard(discard)) if discard.id == discard_id => below, Some(_) | None => return None, } } Some(_) | None => return None, }; - match self.frames.get_mut(discard_index) { + match self.frames.get_mut(discard) { Some(ResolutionFrame::Discard(discard)) => Some(discard), Some(_) | None => unreachable!("checked discard frame must retain its frame kind"), } @@ -785,12 +785,9 @@ impl ResolutionStack { /// Identifies the exact discard parent of the active continuation without /// exposing any non-adjacent frame. pub fn active_ability_continuation_discard_parent_id(&self) -> Option { - let continuation_index = self.frames.len().checked_sub(1)?; - let discard_index = continuation_index.checked_sub(1)?; - match ( - self.frames.get(discard_index), - self.frames.get(continuation_index), - ) { + let continuation = self.frames.top()?; + let discard = self.frames.below(continuation)?; + match (self.frames.get(discard), self.frames.get(continuation)) { ( Some(ResolutionFrame::Discard(discard)), Some(ResolutionFrame::AbilityContinuation(_)), @@ -803,12 +800,9 @@ impl ResolutionStack { &self, discard_id: DiscardFrameId, ) -> Option { - let continuation_index = self.frames.len().checked_sub(1)?; - let discard_index = continuation_index.checked_sub(1)?; - match ( - self.frames.get(discard_index), - self.frames.get(continuation_index), - ) { + let continuation = self.frames.top()?; + let discard = self.frames.below(continuation)?; + match (self.frames.get(discard), self.frames.get(continuation)) { ( Some(ResolutionFrame::Discard(frame)), Some(ResolutionFrame::AbilityContinuation(_)), @@ -850,17 +844,19 @@ impl ResolutionStack { self.take_active_ability_continuation() } Some(ResolutionFrame::BatchDelivery(_)) => { - let Some(parent_index) = self.frames.len().checked_sub(2) else { + let Some(child) = self.frames.top() else { + return Ok(None); + }; + let Some(parent) = self.frames.below(child) else { return Ok(None); }; if !matches!( - self.frames.get(parent_index), + self.frames.get(parent), Some(ResolutionFrame::AbilityContinuation(_)) ) { return Ok(None); } - let child_index = self.frames.len() - 1; - self.frames.swap(parent_index, child_index); + self.frames.swap(parent, child); let ResolutionFrame::AbilityContinuation(frame) = self.pop_expected(FrameKind::AbilityContinuation)? else { @@ -1037,34 +1033,38 @@ impl ResolutionStack { pending: PendingChangeZoneIteration, child_stack_start: usize, ) -> Result<(), ResolutionStackError> { - if child_stack_start >= self.frames.len() { + let Some(boundary) = self.frames.slot_at_captured_depth(child_stack_start) else { return Err(ResolutionStackError::InvalidChildBoundary { child_stack_start, stack_len: self.frames.len(), }); - } + }; - if let Some(ResolutionFrame::ChangeZone(frame)) = self.frames.get(child_stack_start) { + if let Some(ResolutionFrame::ChangeZone(frame)) = self.frames.get(boundary) { if frame.pending.is_none() && frame.devour_eligible_snapshot.is_some() { let devour_eligible_snapshot = frame.devour_eligible_snapshot.clone(); - self.frames[child_stack_start] = + self.frames.replace( + boundary, ResolutionFrame::ChangeZone(Box::new(ChangeZoneFrame { pending: Some(pending), devour_eligible_snapshot, - })); + })), + ); return Ok(()); } } - if let Some(parent_index) = child_stack_start.checked_sub(1) { - if let Some(ResolutionFrame::ChangeZone(frame)) = self.frames.get(parent_index) { + if let Some(parent) = self.frames.below(boundary) { + if let Some(ResolutionFrame::ChangeZone(frame)) = self.frames.get(parent) { if frame.pending.is_none() && frame.devour_eligible_snapshot.is_some() { let devour_eligible_snapshot = frame.devour_eligible_snapshot.clone(); - self.frames[parent_index] = + self.frames.replace( + parent, ResolutionFrame::ChangeZone(Box::new(ChangeZoneFrame { pending: Some(pending), devour_eligible_snapshot, - })); + })), + ); return Ok(()); } } @@ -1090,28 +1090,32 @@ impl ResolutionStack { child_stack_start: usize, ) -> Result<(), ResolutionStackError> { let logical_group_id = pending.logical_zone_change_group.logical_group_id; - if let Some(ResolutionFrame::ChangeZone(frame)) = self.frames.get(child_stack_start) { - if frame.pending.as_ref().is_some_and(|current| { - current.logical_zone_change_group.logical_group_id == logical_group_id - }) { - let devour_eligible_snapshot = frame.devour_eligible_snapshot.clone(); - self.frames[child_stack_start] = - ResolutionFrame::ChangeZone(Box::new(ChangeZoneFrame { - pending: Some(pending), - devour_eligible_snapshot, - })); - return Ok(()); + let boundary = self.frames.slot_at_captured_depth(child_stack_start); + if let Some(boundary) = boundary { + if let Some(ResolutionFrame::ChangeZone(frame)) = self.frames.get(boundary) { + if frame.pending.as_ref().is_some_and(|current| { + current.logical_zone_change_group.logical_group_id == logical_group_id + }) { + let devour_eligible_snapshot = frame.devour_eligible_snapshot.clone(); + self.frames.replace( + boundary, + ResolutionFrame::ChangeZone(Box::new(ChangeZoneFrame { + pending: Some(pending), + devour_eligible_snapshot, + })), + ); + return Ok(()); + } } } - let parent_index = - child_stack_start - .checked_sub(1) - .ok_or(ResolutionStackError::InvalidChildBoundary { - child_stack_start, - stack_len: self.frames.len(), - })?; - let Some(parent) = self.frames.get(parent_index) else { + let parent_slot = boundary.and_then(|slot| self.frames.below(slot)).ok_or( + ResolutionStackError::InvalidChildBoundary { + child_stack_start, + stack_len: self.frames.len(), + }, + )?; + let Some(parent) = self.frames.get(parent_slot) else { return Err(ResolutionStackError::InvalidChildBoundary { child_stack_start, stack_len: self.frames.len(), @@ -1132,10 +1136,13 @@ impl ResolutionStack { }); } let devour_eligible_snapshot = frame.devour_eligible_snapshot.clone(); - self.frames[parent_index] = ResolutionFrame::ChangeZone(Box::new(ChangeZoneFrame { - pending: Some(pending), - devour_eligible_snapshot, - })); + self.frames.replace( + parent_slot, + ResolutionFrame::ChangeZone(Box::new(ChangeZoneFrame { + pending: Some(pending), + devour_eligible_snapshot, + })), + ); Ok(()) } @@ -1165,14 +1172,14 @@ impl ResolutionStack { pub fn active_batch_delivery_or_post_replacement_child_mut( &mut self, ) -> Option<&mut PendingBatchDeliveries> { - let parent_index = self.frames.len().checked_sub(2); + let parent = self.frames.top().and_then(|top| self.frames.below(top)); match self.frames.last() { Some(ResolutionFrame::BatchDelivery(_)) => match self.frames.last_mut() { Some(ResolutionFrame::BatchDelivery(frame)) => Some(frame), Some(_) | None => unreachable!("checked active batch frame must match"), }, - Some(ResolutionFrame::PostReplacement(_)) => match parent_index { - Some(index) => match self.frames.get_mut(index) { + Some(ResolutionFrame::PostReplacement(_)) => match parent { + Some(parent) => match self.frames.get_mut(parent) { Some(ResolutionFrame::BatchDelivery(frame)) => Some(frame), Some(_) | None => None, }, @@ -2339,8 +2346,8 @@ impl ResolutionStack { /// immediate parent of the active child raised while its continuation /// dispatches; there is intentionally no general frame search. pub fn active_post_replacement_or_paired_parent(&self) -> Option<&PostReplacementDrainStack> { - let index = self.active_post_replacement_parent_index()?; - match self.frames.get(index) { + let parent = self.active_post_replacement_parent_slot()?; + match self.frames.get(parent) { Some(ResolutionFrame::PostReplacement(drains)) => Some(drains), Some(_) | None => unreachable!("checked post-replacement parent must match"), } @@ -2350,8 +2357,8 @@ impl ResolutionStack { pub fn active_post_replacement_or_paired_parent_mut( &mut self, ) -> Option<&mut PostReplacementDrainStack> { - let index = self.active_post_replacement_parent_index()?; - match self.frames.get_mut(index) { + let parent = self.active_post_replacement_parent_slot()?; + match self.frames.get_mut(parent) { Some(ResolutionFrame::PostReplacement(drains)) => Some(drains), Some(_) | None => unreachable!("checked post-replacement parent must match"), } @@ -2362,9 +2369,9 @@ impl ResolutionStack { /// dispatch; it never reaches through a child or searches for a buried /// parent. pub fn take_active_post_replacement_child(&mut self) -> Option { - let parent_index = self.active_post_replacement_parent_index()?; - let child_index = self.frames.len().checked_sub(1)?; - if parent_index.checked_add(1) != Some(child_index) { + let parent = self.active_post_replacement_parent_slot()?; + let child = self.frames.top()?; + if self.frames.above(parent) != Some(child) { return None; } self.frames.pop() @@ -2374,21 +2381,21 @@ impl ResolutionStack { /// The two legal shapes are `[... PostReplacement]` and /// `[... PostReplacement, child]`; any deeper relationship is deliberately /// invisible here so callers cannot turn this into a generic frame search. - fn active_post_replacement_parent_index(&self) -> Option { - let parent_index = self.frames.len().checked_sub(1)?; + fn active_post_replacement_parent_slot(&self) -> Option { + let top = self.frames.top()?; if matches!( - self.frames.get(parent_index), + self.frames.get(top), Some(ResolutionFrame::PostReplacement(_)) ) { - return Some(parent_index); + return Some(top); } - let parent_index = parent_index.checked_sub(1)?; + let below = self.frames.below(top)?; matches!( - self.frames.get(parent_index), + self.frames.get(below), Some(ResolutionFrame::PostReplacement(_)) ) - .then_some(parent_index) + .then_some(below) } /// CR 614.12a + CR 616.1g: take the active frame's resident continuation and @@ -2397,7 +2404,7 @@ impl ResolutionStack { /// Ordering here is load-bearing: /// /// * the frame index is resolved through the EXISTING private - /// [`Self::active_post_replacement_parent_index`], which keeps its + /// [`Self::active_post_replacement_parent_slot`], which keeps its /// documented "no general frame search" contract and remains the /// mint-time authority — identity addressing is added for the CLEANUP, /// which is the only side that can be reached after the frame has moved; @@ -2418,10 +2425,10 @@ impl ResolutionStack { crate::types::ability::PostReplacementContinuation, IdentifiedPostReplacementDispatch, )> { - let index = self.active_post_replacement_parent_index()?; + let parent = self.active_post_replacement_parent_slot()?; let candidate = PostReplacementFrameId(self.last_post_replacement_frame_id.saturating_add(1)); - let Some(ResolutionFrame::PostReplacement(drains)) = self.frames.get_mut(index) else { + let Some(ResolutionFrame::PostReplacement(drains)) = self.frames.get_mut(parent) else { unreachable!("checked post-replacement parent must match") }; let (continuation, dispatch) = drains.begin_dispatch()?; @@ -2442,59 +2449,41 @@ impl ResolutionStack { )) } - /// The SINGLE identity-addressed search over `frames`, and the only place in - /// this module licensed to search the frame vector at all. + /// The drain stack of the frame `id` names, if that frame is still resident. /// - /// `scripts/check-resolution-frame-boundaries.sh` anchors its exemption to - /// this function BY NAME. The rule the guard enforces is "one search, here", - /// not "any search that looks identity-shaped": a second search anywhere in - /// this file — including a verbatim copy of the expression below, and - /// including one added INSIDE this body — still fails the guard, which - /// counts the searches in this span and requires exactly one. The guard - /// also requires that search to select on `frame_id() == Some(id)`, so the - /// exemption cannot be inherited by a positional probe that merely takes - /// this function's name. Move this function and the guard fails loudly - /// rather than silently widening. + /// The identity search itself lives in [`FrameVec::by_id`], which is the + /// only place in the crate that can turn a match into an addressable + /// position. This used to be a hand-written `frames.iter().position(..)` + /// exempted by name from a grep guard; the exemption is gone because the + /// search is now the only one the type system permits. /// - /// The distinction the guard draws is between POSITIONAL or - /// adjacency-inferred access, which guesses a structural relationship the - /// stack does not guarantee, and identity-addressed access, which asserts - /// one. The latter is an established access mode in this codebase, not a - /// carve-out invented here: [`DrawSequenceStack`]'s `frame_mut` / `active_if` - /// / `pop` address their frames the same way, resting on the same - /// monotonic-allocator property — an id is never reissued, so a stale id - /// matches nothing rather than aliasing a later frame. Unstamped frames - /// carry `None` and can never match. + /// The distinction being drawn is between POSITIONAL or adjacency-inferred + /// access, which guesses a structural relationship the stack does not + /// guarantee, and identity-addressed access, which asserts one. The latter + /// is an established access mode in this codebase, not a carve-out invented + /// here: [`DrawSequenceStack`]'s `frame_mut` / `active_if` / `pop` address + /// their frames the same way, resting on the same monotonic-allocator + /// property — an id is never reissued, so a stale id matches nothing rather + /// than aliasing a later frame. Unstamped frames carry `None` and can never + /// match. /// /// Both halves of that soundness argument are pinned by existing rows rather /// than asserted here: `h6a_legacy_id_less_post_replacement_frames_restore_unstamped` /// for the unstamped case, and /// `v2_reader_recovers_discard_allocator_and_rejects_duplicate_frame_ids` - /// for the no-reissue case. If either is deleted, this exemption loses its - /// basis. + /// for the no-reissue case. If either is deleted, identity addressing loses + /// its basis. /// /// [`DrawSequenceStack`]: crate::types::game_state::DrawSequenceStack - fn post_replacement_frame_index(&self, id: PostReplacementFrameId) -> Option { - self.frames.iter().position(|frame| { - matches!(frame, ResolutionFrame::PostReplacement(drains) if drains.frame_id() == Some(id)) - }) - } - - /// The drain stack of the frame `id` names, if that frame is still resident. - /// - /// The index does NOT escape this accessor pair. Handing a position to - /// callers would reintroduce exactly the positional coupling this change - /// exists to remove, which is why the three operations below take the - /// payload rather than an index. fn post_replacement_frame( &self, id: PostReplacementFrameId, ) -> Option<&PostReplacementDrainStack> { - let index = self.post_replacement_frame_index(id)?; - match self.frames.get(index) { + let slot = self.frames.by_id(id)?; + match self.frames.get(slot) { Some(ResolutionFrame::PostReplacement(drains)) => Some(drains), Some(_) | None => { - unreachable!("the index came from a matched post-replacement frame") + unreachable!("the slot came from a matched post-replacement frame") } } } @@ -2504,11 +2493,11 @@ impl ResolutionStack { &mut self, id: PostReplacementFrameId, ) -> Option<&mut PostReplacementDrainStack> { - let index = self.post_replacement_frame_index(id)?; - match self.frames.get_mut(index) { + let slot = self.frames.by_id(id)?; + match self.frames.get_mut(slot) { Some(ResolutionFrame::PostReplacement(drains)) => Some(drains), Some(_) | None => { - unreachable!("the index came from a matched post-replacement frame") + unreachable!("the slot came from a matched post-replacement frame") } } } @@ -2665,7 +2654,8 @@ impl ResolutionStack { /// This is intentionally narrower than a frame search: it serves only the /// paused-drain/draw adjacency. pub fn active_predecessor(&self) -> Option<&ResolutionFrame> { - self.frames.get(self.frames.len().checked_sub(2)?) + let top = self.frames.top()?; + self.frames.get(self.frames.below(top)?) } /// True only for the live general-drain/draw pair at the active stack @@ -2693,11 +2683,12 @@ impl ResolutionStack { pub fn outer_ability_continuation_of_active_post_replacement_draw_pair( &self, ) -> Option<&AbilityContinuationFrame> { - let post_replacement_index = self.frames.len().checked_sub(2)?; - let continuation_index = post_replacement_index.checked_sub(1)?; + let draw_child = self.frames.top()?; + let post_replacement = self.frames.below(draw_child)?; + let continuation = self.frames.below(post_replacement)?; match ( - self.frames.get(continuation_index), - self.frames.get(post_replacement_index), + self.frames.get(continuation), + self.frames.get(post_replacement), self.last(), ) { ( @@ -2721,9 +2712,13 @@ impl ResolutionStack { pub fn outer_ability_continuation_of_active_post_replacement_draw_pair_mut( &mut self, ) -> Option<&mut AbilityContinuationFrame> { - let continuation_index = self.frames.len().checked_sub(3)?; + // The pair is {draw child, post-replacement parent}; the outer + // continuation sits immediately beneath both. + let draw_child = self.frames.top()?; + let post_replacement = self.frames.below(draw_child)?; + let continuation = self.frames.below(post_replacement)?; self.outer_ability_continuation_of_active_post_replacement_draw_pair()?; - match self.frames.get_mut(continuation_index) { + match self.frames.get_mut(continuation) { Some(ResolutionFrame::AbilityContinuation(continuation)) => Some(continuation), Some(_) | None => { unreachable!("checked paired continuation must retain its frame kind") @@ -2751,13 +2746,16 @@ impl ResolutionStack { }), }; } - let post_replacement_index = self + let draw_child = self + .frames + .top() + .expect("the checked adjacent pair has a child"); + let post_replacement = self .frames - .len() - .checked_sub(2) + .below(draw_child) .expect("the checked adjacent pair has a parent"); - self.frames.insert( - post_replacement_index, + self.frames.insert_below( + post_replacement, ResolutionFrame::AbilityContinuation(frame), ); Ok(()) @@ -2770,17 +2768,13 @@ impl ResolutionStack { pub fn promote_ability_continuation_after_post_replacement_draw( &mut self, ) -> Result { - let post_replacement_index = self - .frames - .len() - .checked_sub(1) - .ok_or(ResolutionStackError::Empty)?; - let Some(continuation_index) = post_replacement_index.checked_sub(1) else { + let post_replacement = self.frames.top().ok_or(ResolutionStackError::Empty)?; + let Some(continuation) = self.frames.below(post_replacement) else { return Ok(false); }; match ( - self.frames.get(continuation_index), - self.frames.get(post_replacement_index), + self.frames.get(continuation), + self.frames.get(post_replacement), ) { ( Some(ResolutionFrame::AbilityContinuation(_)), @@ -2790,7 +2784,7 @@ impl ResolutionStack { Some(DrainStatus::Paused) ) => { - self.frames.swap(continuation_index, post_replacement_index); + self.frames.swap(continuation, post_replacement); Ok(true) } (Some(_), Some(ResolutionFrame::PostReplacement(_))) => Ok(false), @@ -2798,7 +2792,7 @@ impl ResolutionStack { expected: FrameKind::PostReplacement, actual: actual.kind(), }), - (_, None) => unreachable!("checked active post-replacement index exists"), + (_, None) => unreachable!("checked active post-replacement slot exists"), } } @@ -2822,12 +2816,11 @@ impl ResolutionStack { &mut self, frame: ResolutionFrame, ) -> Result<(), ResolutionStackError> { - let active_index = self + let active = self .frames - .len() - .checked_sub(1) + .top() .ok_or(ResolutionStackError::NoActiveChild)?; - self.frames.insert(active_index, frame); + self.frames.insert_below(active, frame); Ok(()) } @@ -2847,13 +2840,15 @@ impl ResolutionStack { if stack_len == 0 { return Err(ResolutionStackError::NoActiveChild); } - if child_stack_start >= stack_len { + if !self + .frames + .insert_at_child_boundary(child_stack_start, frame) + { return Err(ResolutionStackError::InvalidChildBoundary { child_stack_start, stack_len, }); } - self.frames.insert(child_stack_start, frame); Ok(()) } @@ -2916,21 +2911,19 @@ impl ResolutionStack { pub fn complete_adjacent_post_replacement_draw( &mut self, ) -> Result { - let child_index = self + let child = self.frames.top().ok_or(ResolutionStackError::Empty)?; + let parent = self .frames - .len() - .checked_sub(1) - .ok_or(ResolutionStackError::Empty)?; - let parent_index = - child_index - .checked_sub(1) - .ok_or(ResolutionStackError::InvalidAdjacentPair( - "a multi-draw child has no immediate post-replacement predecessor", - ))?; - validate_shipped_post_replacement_draw_pair( - &self.frames[parent_index], - &self.frames[child_index], - )?; + .below(child) + .ok_or(ResolutionStackError::InvalidAdjacentPair( + "a multi-draw child has no immediate post-replacement predecessor", + ))?; + let (Some(parent_frame), Some(child_frame)) = + (self.frames.get(parent), self.frames.get(child)) + else { + unreachable!("both slots were just located in this stack") + }; + validate_shipped_post_replacement_draw_pair(parent_frame, child_frame)?; Ok(self .frames .pop() @@ -3029,12 +3022,11 @@ impl ResolutionStack { ) ) { - let child = - self.frames - .get(index + 1) - .ok_or(ResolutionStackError::InvalidAdjacentPair( - "a paused post-replacement drain has no immediate multi-draw child", - ))?; + let child = self.frames.frame_at_offset(index + 1).ok_or( + ResolutionStackError::InvalidAdjacentPair( + "a paused post-replacement drain has no immediate multi-draw child", + ), + )?; validate_shipped_post_replacement_draw_pair(frame, child)?; // CR 614.11a + CR 121.6b: when a replacement replaces a draw // inside a draw sequence, ALL actions the replacement requires @@ -3073,7 +3065,7 @@ impl ResolutionStack { // ABOVE the draw instead of outside the pair, where // `insert_ability_continuation_outside_active_post_replacement_draw` // puts it (CR 608.2c: instructions run in the order written). - let paired_child_is_reachable = match self.frames.get(index + 2) { + let paired_child_is_reachable = match self.frames.frame_at_offset(index + 2) { None => true, Some(above) => { matches!(above.gate(), FrameGate::DirectChoice(_)) @@ -4519,7 +4511,11 @@ mod tests { .expect("the direct discard parent is mutable") .source_id = Some(ObjectId(1)); assert_eq!( - match &direct.frames[0] { + match direct + .iter() + .next() + .expect("the discard parent is resident") + { ResolutionFrame::Discard(frame) => frame.source_id, other => panic!("expected discard parent, got {other:?}"), }, @@ -6360,9 +6356,8 @@ mod tests { // Positive reach-guard: the POSITIONAL accessor still resolves here, so // the row is not passing merely because positional addressing was // already broken before the insert. - assert_eq!( - stack.active_post_replacement_parent_index(), - Some(0), + assert!( + stack.active_post_replacement_parent_slot().is_some(), "reach-guard: distance from the top is 1 before the insert" ); @@ -6379,9 +6374,8 @@ mod tests { Some(PostReplacementFrameId(1)), "the PostReplacement frame's ABSOLUTE index is still 0" ); - assert_eq!( - stack.active_post_replacement_parent_index(), - None, + assert!( + stack.active_post_replacement_parent_slot().is_none(), "reach-guard: the frame has left the two-deep positional window" ); @@ -6434,9 +6428,8 @@ mod tests { stack.push_inner(continuation_frame(1)); stack.push_inner(continuation_frame(2)); - assert_eq!( - stack.active_post_replacement_parent_index(), - None, + assert!( + stack.active_post_replacement_parent_slot().is_none(), "reach-guard: F1 is buried, which is the precondition under which \ install_post_replacement_drain mints a sibling in production" ); diff --git a/crates/engine/src/types/resolution/frame_vec.rs b/crates/engine/src/types/resolution/frame_vec.rs new file mode 100644 index 0000000000..d3d16f9422 --- /dev/null +++ b/crates/engine/src/types/resolution/frame_vec.rs @@ -0,0 +1,226 @@ +//! The frame vector, and the only positions that can address it. +//! +//! [`ResolutionStack`] permits three ways to reach a frame: the top, a frame +//! adjacent to one you already hold, and the frame a +//! [`PostReplacementFrameId`] names. Everything else — scanning for the first +//! frame of some kind, remembering an index across a mutation, removing from +//! the middle — is a positional GUESS about a structural relationship the stack +//! does not guarantee. +//! +//! That rule used to be enforced by `scripts/check-resolution-frame-boundaries.sh` +//! grepping for `frames.iter().position(..)`, because `frames` was private to a +//! 7,000-line module and Rust privacy is module-scoped: "private" bought +//! nothing against the code sitting next to it. This module is the type-level +//! version of the same rule. +//! +//! [`FrameSlot`] is an opaque position with a private field, so it can be minted +//! by exactly five methods: [`FrameVec::top`], [`FrameVec::below`], +//! [`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 +//! — `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`. +//! +//! [`FrameVec::frame_at_offset`] takes a `usize` too, but returns a frame and +//! never a slot, so it cannot widen addressing. +//! +//! Two operations are absent rather than restricted. `remove`, `swap_remove`, +//! `retain`, `drain`, `truncate` and `clear` have no wrapper here because the +//! stack has no legitimate use for them: frames leave through [`FrameVec::pop`] +//! at the top, or not at all. Absence costs nothing while the use count is zero, +//! and a future caller must add the method — and justify it — rather than +//! quietly reach for one that already exists. +//! +//! [`ResolutionStack`]: super::ResolutionStack + +use serde::{Deserialize, Serialize}; + +use crate::types::game_state::PostReplacementFrameId; + +use super::ResolutionFrame; + +/// A position in a [`FrameVec`]. +/// +/// The field is private to this module, so a `FrameSlot` can only come from one +/// of the five minting methods listed in the module docs. That is the whole +/// mechanism: it makes "which frames may I address?" a question the compiler +/// answers. +/// +/// A slot is a position, not a handle. Pushing, popping or inserting can move +/// the frame a slot refers to, so a slot held across a mutation may name a +/// different frame or none at all. [`FrameVec::get`] and [`FrameVec::get_mut`] +/// therefore return [`Option`] rather than asserting — the type stops you +/// addressing a frame you never located, and does not pretend to stop you +/// addressing one that has since moved. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(super) struct FrameSlot(usize); + +/// The backing storage for [`ResolutionStack`]'s frames. +/// +/// Serialized transparently, so the wire format is exactly the `Vec` this +/// wraps and no save migration is involved. +/// +/// [`ResolutionStack`]: super::ResolutionStack +#[derive(Debug, Clone, Default, PartialEq, Serialize, Deserialize)] +#[serde(transparent)] +pub(super) struct FrameVec { + frames: Vec, +} + +impl FrameVec { + pub(super) fn len(&self) -> usize { + self.frames.len() + } + + pub(super) fn is_empty(&self) -> bool { + self.frames.is_empty() + } + + /// Read-only iteration, for validation, comparison and projection. + /// + /// This does expose `position`/`find` through [`Iterator`] — but their + /// `usize` cannot be turned into a [`FrameSlot`], so a scan can be written + /// and its result cannot be used. That is the intended shape: iteration + /// over frames is legitimate, addressing a frame you found by scanning is + /// not. + pub(super) fn iter(&self) -> std::slice::Iter<'_, ResolutionFrame> { + self.frames.iter() + } + + pub(super) fn last(&self) -> Option<&ResolutionFrame> { + self.frames.last() + } + + pub(super) fn last_mut(&mut self) -> Option<&mut ResolutionFrame> { + self.frames.last_mut() + } + + pub(super) fn push(&mut self, frame: ResolutionFrame) { + self.frames.push(frame); + } + + pub(super) fn pop(&mut self) -> Option { + self.frames.pop() + } + + /// The top of the stack, if any. + pub(super) fn top(&self) -> Option { + self.frames.len().checked_sub(1).map(FrameSlot) + } + + /// The frame immediately beneath `slot` — the adjacent-pair boundary. + pub(super) fn below(&self, slot: FrameSlot) -> Option { + slot.0.checked_sub(1).map(FrameSlot) + } + + /// The frame immediately above `slot`, if the stack reaches that far. + pub(super) fn above(&self, slot: FrameSlot) -> Option { + let above = slot.0.checked_add(1)?; + (above < self.frames.len()).then_some(FrameSlot(above)) + } + + /// The frame `id` names — the single identity-addressed lookup. + /// + /// Sound because ids come from a monotonic allocator that never rewinds + /// within an action, so a stale id matches nothing rather than aliasing a + /// later frame, and unstamped frames carry `None` and can never match. + /// Those two properties are pinned by + /// `v2_reader_recovers_discard_allocator_and_rejects_duplicate_frame_ids` + /// and `h6a_legacy_id_less_post_replacement_frames_restore_unstamped`. + pub(super) fn by_id(&self, id: PostReplacementFrameId) -> Option { + self.frames + .iter() + .position(|frame| { + matches!(frame, ResolutionFrame::PostReplacement(drains) if drains.frame_id() == Some(id)) + }) + .map(FrameSlot) + } + + /// The slot at a stack DEPTH captured before a child producer ran. + /// + /// This is the ONLY method that turns a `usize` 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. + /// + /// 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)) + } + + /// Read the frame at a raw offset during a full-stack walk. + /// + /// `validate` traverses every frame and asks questions about a frame's + /// neighbours by offset; it must, since checking a whole-stack invariant is + /// precisely a whole-stack operation. This returns a frame and never a + /// [`FrameSlot`], so an offset — including one produced by a scan — cannot + /// be laundered into something addressable. That is the line this module + /// draws: reading is not the hazard, addressing for mutation is. + pub(super) fn frame_at_offset(&self, offset: usize) -> Option<&ResolutionFrame> { + self.frames.get(offset) + } + + pub(super) fn get(&self, slot: FrameSlot) -> Option<&ResolutionFrame> { + self.frames.get(slot.0) + } + + /// Overwrite the frame at `slot`, leaving stack length unchanged. + pub(super) fn replace(&mut self, slot: FrameSlot, frame: ResolutionFrame) { + self.frames[slot.0] = frame; + } + + pub(super) fn get_mut(&mut self, slot: FrameSlot) -> Option<&mut ResolutionFrame> { + self.frames.get_mut(slot.0) + } + + /// Exchange two located frames, preserving stack length. + pub(super) fn swap(&mut self, a: FrameSlot, b: FrameSlot) { + self.frames.swap(a.0, b.0); + } + + /// Insert `frame` so that it sits immediately beneath the frame currently + /// at `slot`, lifting `slot` and everything above it by one. + pub(super) fn insert_below(&mut self, slot: FrameSlot, frame: ResolutionFrame) { + self.frames.insert(slot.0, frame); + } + + /// Insert `frame` at a stack DEPTH captured before a child producer ran. + /// + /// This is the one depth-addressed entry point, and it is deliberately not + /// a slot operation: `depth` is not a position someone located in the + /// current stack, it is a length recorded earlier and handed back, so the + /// 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. + /// + /// 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, + frame: ResolutionFrame, + ) -> bool { + if depth >= self.frames.len() { + return false; + } + self.frames.insert(depth, frame); + true + } +} diff --git a/scripts/check-resolution-frame-boundaries.sh b/scripts/check-resolution-frame-boundaries.sh index 0bc20dc2e8..2833b6a689 100755 --- a/scripts/check-resolution-frame-boundaries.sh +++ b/scripts/check-resolution-frame-boundaries.sh @@ -6,26 +6,29 @@ # fixtures. Runtime resolution work is represented by typed ResolutionFrame # payloads; identically named typed payload members are not wire keys. The # frame stack permits top access, a captured adjacent-pair boundary, or -# identity-addressed access through its SINGLE named accessor. Removing an -# arbitrary index, or searching the vector anywhere else, breaks that authority. +# identity-addressed access. Removing an arbitrary index, or searching the +# vector to decide what to mutate, breaks that authority. # -# On the identity-addressed exemption: the rule is "one search, in -# `post_replacement_frame_index`", not "any search that looks identity-shaped". -# Anchoring to the function name rather than to a closure pattern is what keeps -# this from becoming a loophole — a second call site cannot acquire its own -# search by mimicking the expression, and moving or renaming the accessor fails -# the guard loudly instead of silently widening it. Each half of that sentence -# is CHECKED below rather than left to the reader: the accessor must be defined -# exactly once, must contain exactly one search, and that search must select on -# `frame_id() == Some(id)`. A span-only exemption would enforce the weaker -# "searches only there" while this comment claimed the stronger "one search, -# there" — the gap being closed here. The distinction being drawn -# is between positional/adjacency-inferred access, which GUESSES a structural -# relationship the stack does not guarantee, and identity-addressed access, -# which asserts one: ids come from a monotonic allocator that never rewinds, so -# a stale id matches nothing rather than aliasing a later frame. This mirrors -# `DrawSequenceStack::frame_mut` / `active_if` / `pop`, which is the same access -# mode on a sibling frame stack. +# 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 — +# 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 +# nothing rather than aliasing a later frame. It mirrors +# `DrawSequenceStack::frame_mut` / `active_if` / `pop`, the same access mode on +# a sibling frame stack. +# +# 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. set -euo pipefail @@ -355,73 +358,38 @@ for file_name in files: if path != resolution_path: continue - remove_pattern = re.compile( - r"\b(?:self\s*\.\s*)?frames\s*\.\s*" - r"(?:remove|swap_remove|retain|drain|truncate|clear)\s*\(" - ) - search_pattern = re.compile( - r"\b(?:self\s*\.\s*)?frames\s*\.\s*iter(?:_mut)?\s*\(\s*\)" - r"(?:\s*\.\s*\w+\s*\([^;{}]*\))*?" - r"\s*\.\s*(?:position|rposition|find|find_map|any|next|nth)\s*\(", - re.DOTALL, - ) - - # `frames` may be searched in EXACTLY ONE production site: the module's - # single identity-addressed accessor. A span-only exemption would enforce - # "searches only there" while this script's header promises "one search, - # there" -- so the three properties that make the header true are checked - # rather than assumed: + # 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. # - # 1. exactly one `fn post_replacement_frame_index` exists. `function_span` - # takes the first textual match, so a second definition would silently - # decide which one is exempt; - # 2. its span holds exactly one search, so the exemption cannot be widened - # from the inside by adding a second search beside the first; - # 3. that search matches on `frame_id() == Some(id)`, so what is exempted - # is an identity lookup and not a positional or first-match probe - # wearing the accessor's name. + # 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. # - # The removal patterns are NOT exempted here: the accessor reads `frames` - # and never restructures it. `function_span` raises when its target is - # missing, so deleting or renaming the accessor fails the guard rather than - # quietly removing the anchor. - accessor = "post_replacement_frame_index" - definitions = len(re.findall(rf"\bfn\s+{re.escape(accessor)}\s*\(", resolution_source)) - if definitions != 1: + # `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. + 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"} + if minting != sanctioned_minting: + added = ", ".join(sorted(minting - sanctioned_minting)) or "none" + missing = ", ".join(sorted(sanctioned_minting - minting)) or "none" failures.append( - f" {resolution_path}: expected exactly one `fn {accessor}` " - f"definition, found {definitions}" + " crates/engine/src/types/resolution/frame_vec.rs: FrameSlot may be " + f"minted only by {', '.join(sorted(sanctioned_minting))}; " + f"unexpected: {added}; missing: {missing}" ) - accessor_start, accessor_end = function_span(resolution_source, accessor) - accessor_body = resolution_source[accessor_start:accessor_end] - searches = len(search_pattern.findall(accessor_body)) - if searches != 1: - failures.append( - f" {resolution_path}: `{accessor}` is the single sanctioned " - f"`frames` search; found {searches}" - ) - if not re.search(r"\bframe_id\s*\(\s*\)\s*==\s*Some\s*\(\s*id\s*\)", accessor_body): - failures.append( - f" {resolution_path}: `{accessor}` must select frames by identity " - "(`frame_id() == Some(id)`), not by position" - ) - - for pattern, message, spans in [ - ( - remove_pattern, - "arbitrary ResolutionStack frame removal is forbidden; use a checked top-only API", - test_spans, - ), - ( - search_pattern, - "generic ResolutionStack frame search is forbidden; use top access, " - "adjacent-pair access, or the single identity accessor", - test_spans + [(accessor_start, accessor_end)], - ), - ]: - for match in pattern.finditer(source): - if not in_any_span(match.start(), spans): - fail(failures, path, source, match.start(), message) if failures: print("Resolution-frame boundary guard failed:", file=sys.stderr)