Build a gathered subject's copy plan on collapse and reject an empty carry

This commit is contained in:
Dennis Kobert
2026-09-11 14:11:11 +00:00
parent d4dc2e9e6b
commit d218e408fd
6 changed files with 167 additions and 2 deletions

View File

@@ -112,6 +112,7 @@ impl<'e> Frames<'e> {
frame,
free: self.reborrow(),
filled_fields: false,
carried_empty: false,
}
}
@@ -182,4 +183,48 @@ mod tests {
}
assert!(addresses.windows(2).all(|pair| pair[0] == pair[1]), "each claim reuses the same region");
}
#[test]
#[should_panic(expected = "carried an empty plan and filled nothing")]
fn carrying_an_empty_plan_and_filling_nothing_refuses_to_close() {
// The shape that served uninitialized bytes before this guard existed: a
// layout that DECLARES a field, a carry whose plan turned out empty, and
// nothing else to fill it. The field would otherwise be the prior frame's
// bytes, which for a reference field is an uninitialized read.
let layout = Layout::default().with_writes(0, element_write::<f64>(), &[f64_field("opacity")]);
let mut frame_arena = FrameArena::new();
frame_arena.reserve(1 << 10);
let frames = frame_arena.frames();
let scope = frames.scope();
let mut claim = scope.claim(&layout);
let value = crate::record::access::RecordValue::zeroed();
let src = layout.rec(&value);
// SAFETY: an empty plan reads nothing from `src`; closing afterwards is
// what the guard refuses.
unsafe { claim.carry(src, &[]) };
// SAFETY: the assertion fires before anything reads the unfilled field.
let _ = unsafe { claim.finish() };
}
#[test]
fn an_empty_carry_whose_fields_are_written_closes_normally() {
// The control that pins what the guard actually tracks: the same empty
// carry, but the declared field is then written. Closing must succeed, so
// the guard is about a field left unfilled rather than about the carry.
let layout = Layout::default().with_writes(0, element_write::<f64>(), &[f64_field("opacity")]);
let offset = layout.fields.first().expect("the fixture declares one field").offset;
let mut frame_arena = FrameArena::new();
frame_arena.reserve(1 << 10);
let frames = frame_arena.frames();
let scope = frames.scope();
let mut claim = scope.claim(&layout);
let value = crate::record::access::RecordValue::zeroed();
let src = layout.rec(&value);
// SAFETY: an empty plan reads nothing from `src`.
unsafe { claim.carry(src, &[]) };
// SAFETY: `offset` is this layout's own resolved offset for an f64 field.
unsafe { claim.attr_at(offset, 0.5_f64) };
// SAFETY: the write above filled the declared field.
let _ = unsafe { claim.finish() };
}
}

View File

@@ -478,6 +478,18 @@ pub struct LayoutMeta {
/// The materialized subject a reducer folds, as `(input, levels)`. The fold
/// consumes the whole subject input, so only the node's own levels remain.
pub folded: Option<(u8, u8)>,
/// Whether [`sources`](Self::sources)`[0]` is a GATHERED subject, whose
/// per-lane layout is this output's base.
///
/// Carried explicitly because it cannot be inferred from `sources` and
/// `level_delta`. A gathered subject that also collapses the level
/// (`level_delta < 0`) still has a well-defined copy plan: the gathered
/// lane's layout IS the base being copied into. A LAZY folding subject looks
/// identical from the outside - un-materialized, so it also sits in
/// `sources` with a negative delta - but copying its fields down is exactly
/// what the plan must not do. Keying on "sources is non-empty" would
/// conflate the two and reintroduce the defect in the other direction.
pub gathered: bool,
}
/// The attributes a node reads from one input, recorded on [`LayoutMeta`] for
@@ -513,6 +525,8 @@ impl LayoutMeta {
removes: Vec::new(),
level_delta: 0,
folded: None,
// A retype keeps input 0's level, so its plan is already unconditional.
gathered: false,
}
}
@@ -545,7 +559,9 @@ impl LayoutMeta {
let layout = self.fold(inputs);
let frame_bytes = layout.frame_bytes();
let plan = match self.sources.first() {
Some(&source) if self.level_delta >= 0 => {
// A gathered subject copies from the lane it gathered, whose layout is
// this output's base, so its plan holds however the level moves.
Some(&source) if self.level_delta >= 0 || self.gathered => {
let from = inputs[source as usize].expect("layout resolve source input has no layout");
let carry_element = matches!(self.element, ElementSpec::Carried);
let removes: Vec<(&str, u8)> = self.removes.clone();

View File

@@ -47,6 +47,7 @@ impl<'a> SlotRun<'a> {
frame: (self.layout.frame_bytes() != 0).then_some(frame),
free: frames.reborrow(),
filled_fields: false,
carried_empty: false,
}
}
@@ -86,6 +87,11 @@ pub struct FrameClaim<'e, 'l> {
/// Set by the writes that fill the declared fields, so the safe closers can
/// refuse a field-bearing frame that was never filled.
pub(in crate::record) filled_fields: bool,
/// Set where a carry ran against an EMPTY plan, which fills nothing. Kept
/// apart from `filled_fields` because "carried nothing" and "never carried"
/// are different mistakes: the first is a wiring defect the closers can
/// prove, the second is the ordinary shape of a fresh record.
pub(in crate::record) carried_empty: bool,
}
impl<'e, 'l> FrameClaim<'e, 'l> {
@@ -124,7 +130,16 @@ impl<'e, 'l> FrameClaim<'e, 'l> {
/// serving the source through [`Self::frames`] establishes it.
pub unsafe fn carry(&mut self, src: Rec<'_>, plan: &[(usize, usize, usize)]) {
unsafe { apply_plan(src, self.dst(), plan) };
self.filled_fields = true;
// An empty plan copied nothing, so it must not satisfy the closers'
// "carried or wrote its fields" guard: the frame still holds whatever
// bytes the previous claim left, and a declared field read out of them
// is an uninitialized read rather than a missing value. Recorded rather
// than ignored so the closers can tell "carried nothing" apart from
// "never carried", which are not the same mistake.
match plan.is_empty() {
true => self.carried_empty = true,
false => self.filled_fields = true,
}
}
/// Copies the source record's element bytes into the frame, for a gathered
@@ -207,6 +222,17 @@ impl<'e, 'l> FrameClaim<'e, 'l> {
/// The frame must hold a complete record of the layout, written through
/// the carry, element, and field writes.
pub unsafe fn finish(mut self) -> RecordValue<'e> {
// A frame that CARRIED, but through an empty plan, and then filled
// nothing else: its declared fields hold the previous claim's bytes, and
// a reference field read out of them is an uninitialized read. This is
// narrower than [`Self::lift`]'s guard on purpose - a frame that never
// carried at all may legitimately serve fields the census staged as
// declared defaults, so only an empty carry is evidence of the defect.
assert!(
!(self.carried_empty && !self.filled_fields && !self.layout.fields.is_empty()),
"a layout with {} fields carried an empty plan and filled nothing: its fields would be the prior frame's bytes",
self.layout.fields.len()
);
match self.frame {
Some(frame) => RecordValue::spilled(unsafe { Rec::new(frame.cast_const()) }),
// SAFETY: the inline record is the value's own bytes.