From e6bbfe2ea1725e269ad6c870e0dcee9327c2ddf5 Mon Sep 17 00:00:00 2001 From: Dennis Kobert Date: Mon, 14 Sep 2026 17:20:26 +0200 Subject: [PATCH] Read a node's eval tail off its record work instead of a class label --- node-graph/node-macro/src/codegen.rs | 6 +- node-graph/node-macro/src/codegen/ir.rs | 108 +++++++++++++++++++++++- node-graph/node-macro/src/validation.rs | 16 ++++ 3 files changed, 124 insertions(+), 6 deletions(-) diff --git a/node-graph/node-macro/src/codegen.rs b/node-graph/node-macro/src/codegen.rs index 84fb354ac7..0316ead3df 100644 --- a/node-graph/node-macro/src/codegen.rs +++ b/node-graph/node-macro/src/codegen.rs @@ -1861,11 +1861,7 @@ pub(crate) fn generate_node_impl(crate_ident: &CrateIdent, parsed: &ParsedNodeFn } else if future_kernel { Tail::SpawnFuture } else { - match ir::node_kind(&node) { - ir::NodeKind::RecordIo => Tail::Record, - ir::NodeKind::Flip => Tail::Flip, - ir::NodeKind::Routing | ir::NodeKind::Opaque => Tail::Forward, - } + ir::record_tail(&node) }; // A carried tail claims the node's frame first, evaluates the carrier // beyond it, and carries its fields; every exit closes the frame through diff --git a/node-graph/node-macro/src/codegen/ir.rs b/node-graph/node-macro/src/codegen/ir.rs index 342f2392d5..32dab8a93c 100644 --- a/node-graph/node-macro/src/codegen/ir.rs +++ b/node-graph/node-macro/src/codegen/ir.rs @@ -2,7 +2,9 @@ // Fields below the `Node` root are read by the IR's own tests only. #![allow(dead_code)] -use crate::codegen::classify::{Dialect, RoutingIo, bare_ident, context_param, dialect, flip_carrier, generic_assignment, generic_extractable, is_served, record_shape, routing_io, slot_value_type}; +use crate::codegen::classify::{ + Dialect, RoutingIo, Tail, bare_ident, context_param, dialect, flip_carrier, generic_assignment, generic_extractable, is_served, record_shape, routing_io, slot_value_type, +}; use crate::codegen::entries::implementation_rows; use crate::parsing::{AttributeRead, NodeParsedField, ParsedField, ParsedFieldType, ParsedNodeFn, RecordWrites, RegularParsedField, record_writes}; use proc_macro2::TokenStream as TokenStream2; @@ -546,6 +548,33 @@ pub(crate) fn node_kind(node: &Node) -> NodeKind { } } +/// The tail that closes a node's `eval` body, read off what the node does to the +/// record. The branch order is a priority, not a partition: an opaque kernel owns +/// the frame and has already served it, so it shadows every question below; a node +/// touching columns needs the record tail to write them, which shadows the carried +/// forward a routing output would otherwise take. Only a node writing a fresh +/// element and touching no columns flips. +/// +/// Opaque shadowing column work would silently drop it, so +/// [`crate::validation`] refuses that combination rather than letting it lower. +pub(crate) fn record_tail(node: &Node) -> Tail { + if matches!(node.output.shape.element, Element::Opaque) { + Tail::Forward + } else if has_attr_io(node) { + Tail::Record + } else if is_routing(node) { + Tail::Forward + } else { + Tail::Flip + } +} + +/// The column work an opaque kernel would swallow: it serves the frame itself, so +/// writes, removes, and gathers the macro would otherwise emit never run. +pub(crate) fn opaque_swallows_columns(node: &Node) -> bool { + matches!(node.output.shape.element, Element::Opaque) && (!node.output.shape.attrs.is_empty() || !node.output.removes.is_empty() || node.output.gathers) +} + /// Routing forwards an unbounded generic from a source whole; a bounded generic /// or one transformed into a different output type works on the element and flips. fn is_routing(node: &Node) -> bool { @@ -814,12 +843,37 @@ mod tests { } } + fn tail_label(tail: Tail) -> &'static str { + match tail { + Tail::Forward => "forward", + Tail::Record => "record", + Tail::Flip => "flip", + Tail::SpawnAsyncFn => "spawn-async-fn", + Tail::SpawnFuture => "spawn-future", + } + } + + /// The tail the kind mapping used to pick, frozen as the oracle + /// [`record_tail`] must reproduce now that it reads the node's record work. + fn tail_from_kind(node: &Node) -> Tail { + match node_kind(node) { + NodeKind::RecordIo => Tail::Record, + NodeKind::Flip => Tail::Flip, + NodeKind::Routing | NodeKind::Opaque => Tail::Forward, + } + } + + fn assert_tail_matches_kind(node: &Node) { + assert_eq!(tail_label(record_tail(node)), tail_label(tail_from_kind(node)), "the derived tail must agree with the kind mapping"); + } + fn assert_bridge(attr: TokenStream2, item: TokenStream2) -> Node { let mut parsed = parse_node_fn(attr, item).unwrap(); parsed.replace_impl_trait_in_input(); analyze(&parsed).expect("representative resolves to a supported node"); let node = build(&parsed); assert_eq!(facts_from_ir(&node), facts_from_signature(&parsed)); + assert_tail_matches_kind(&node); node } @@ -1362,6 +1416,58 @@ mod tests { assert_eq!(node.output.shape.depth as i8 - subject.shape.depth as i8, -1, "the reducer collapses one level"); } + fn node_of(item: TokenStream2) -> Node { + let mut parsed = parse_node_fn(quote!(category("")), item).unwrap(); + parsed.replace_impl_trait_in_input(); + build(&parsed) + } + + #[test] + fn every_tail_is_read_off_the_node_the_kind_mapping_agreed_on() { + // One signature per branch of the priority, so a reordering shows up here + // rather than as a silently different tail. + let flip = node_of(quote!( + fn negate(_: impl Ctx, x: f64) -> f64 { + -x + } + )); + let record = node_of(quote!( + fn set_opacity(_: impl Ctx, val: f64) -> (f64, Attr) { + (val, Attr(1.)) + } + )); + let routing = node_of(quote!( + fn pick(ctx: impl Ctx + Copy, condition: bool, on: impl Node, Output = T>, off: impl Node, Output = T>) -> Result { + if condition { on.eval(ctx) } else { off.eval(ctx) } + } + )); + let opaque = node_of(quote!( + fn hold<'e, 'l>(ctx: impl Ctx + Copy, content: impl Node>, slot: FrameClaim<'e, 'l>) -> GPoll> { + content.serve(ctx, slot) + } + )); + + for (name, node, expected) in [("flip", &flip, "flip"), ("record", &record, "record"), ("routing", &routing, "forward"), ("opaque", &opaque, "forward")] { + assert_eq!(tail_label(record_tail(node)), expected, "{name} takes the {expected} tail"); + assert_tail_matches_kind(node); + } + } + + #[test] + fn an_opaque_kernel_that_declares_a_write_is_caught() { + // The kernel serves its own frame, so the forward tail it takes emits no + // write: without the refusal the declared attribute silently vanishes. + let node = node_of(quote!( + fn hold<'e, 'l>(_: impl Ctx, content: impl Node>, slot: FrameClaim<'e, 'l>) -> (Served<'e>, Attr<'e, Opacity>) { + todo!() + } + )); + assert!(matches!(node.output.shape.element, Element::Opaque), "the served return is the opaque element"); + assert_eq!(node.output.shape.attrs.len(), 1, "the write is recorded on the output"); + assert_eq!(tail_label(record_tail(&node)), "forward", "the opaque branch still shadows the column work"); + assert!(opaque_swallows_columns(&node), "so the shape is refused rather than lowered"); + } + #[test] fn the_deepest_layout_source_is_the_delta_base() { let mut parsed = parse_node_fn( diff --git a/node-graph/node-macro/src/validation.rs b/node-graph/node-macro/src/validation.rs index 618f4797cb..8d54b18688 100644 --- a/node-graph/node-macro/src/validation.rs +++ b/node-graph/node-macro/src/validation.rs @@ -16,6 +16,7 @@ pub fn validate_node_fn(parsed: &ParsedNodeFn) -> syn::Result<()> { validate_record_io, validate_lazy_reads, validate_lowering_supported, + validate_opaque_frame_ownership, ]; for validator in validators { @@ -25,6 +26,21 @@ pub fn validate_node_fn(parsed: &ParsedNodeFn) -> syn::Result<()> { Ok(()) } +/// A kernel returning `Served` has already closed the frame, so the forward tail +/// it takes emits none of the column work the signature declares. Left to lower, +/// the writes simply never happen and the node reports no error. +fn validate_opaque_frame_ownership(parsed: &ParsedNodeFn) { + let node = crate::codegen::ir::build(parsed); + if crate::codegen::ir::opaque_swallows_columns(&node) { + emit_error!( + parsed.fn_name.span(), + "`{}` serves its own frame, so the attributes it declares would never be written", + parsed.fn_name; + help = "drop the attributes from the return type, or return the element instead of `Served` so the macro owns the frame" + ); + } +} + /// A signature no lowering claims generates no `Node` impl at all, so the node /// compiles and is simply missing at runtime. Naming the gap is the only thing /// that ends it, since nothing downstream can tell "declined" from "absent".