From 5e179d27afe924e476415300ef6bc6ad702a7c87 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 8 Aug 2026 11:17:19 +0000 Subject: [PATCH 1/2] `quoted!` covers all three recording vocabularies MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous arms handled a plain closure. That left the *interesting* case — a per-instrument parameter — needing the manual three-line form, which is exactly backwards: the shorthand worked for the easy case and not for the one the generator exists to serve. One rule now covers everything, marked in the argument list: map(|x| ..) records the closure map([fee] move |x| ..) the closure and the captured value ticker(cfg period) the data config fold(cfg 0u64, |a, v| ..) both join(&other, |x, y| ..) the closure; `&other` is an edge So a per-instrument leg reads without leaving the macro: let ticks = quoted!(g => ticker(cfg inst.tick)).count(); let px = quoted!(ticks => map(|n: &u64| *n as f64)); let net = quoted!(px => map([fee] move |p: &f64| p - fee)); `cfg` also removes a redundancy worth naming: `ticker(p).with_cfg(&p)` wrote the value twice, so it could be forgotten in one place or — worse — changed in one place. Two implementation notes: - **Arm order is load-bearing.** Each arm is discriminated by a *literal* token (`cfg`, `[`) before any fragment is parsed, and the arms are ordered most-specific first. `macro_rules!` falls through cleanly on a literal mismatch, but a failed `$x:expr` parse is a hard error rather than a fallthrough — so an arm that could swallow another's input has to come second. - A `cfg`-marked argument is **cloned** into the call so the original can be rendered. Every `EmitLiteral` type is `Clone`, and for the common `Copy` ones (`Duration`, numbers) the clone is free. Still not detected: an *undeclared* capture. `map(move |p| p - fee)` without `[fee]` records a body that will not resolve in an artifact. It surfaces as a generator refusal — or, if the name happens to resolve to something else at the splice site, as a pass-2 compile error. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01X4eojqRWBJ6uK5v5JDzmkd --- crates/wingfoil/src/quote.rs | 92 +++++++++++++++-- crates/wingfoil/tests/quoted_macro.rs | 137 ++++++++++++++++++++++++++ 2 files changed, 219 insertions(+), 10 deletions(-) diff --git a/crates/wingfoil/src/quote.rs b/crates/wingfoil/src/quote.rs index 719220ab4..14c711c39 100644 --- a/crates/wingfoil/src/quote.rs +++ b/crates/wingfoil/src/quote.rs @@ -271,25 +271,97 @@ macro_rules! func { /// trade. `Span::source_text` would settle it, but joining spans across an /// expression is nightly-only. /// +/// # One rule for all three vocabularies +/// +/// Everything a generator needs recorded is marked in the argument list: +/// +/// | Write | Records | +/// |---|---| +/// | `map(\|x\| ..)` | the closure | +/// | `map([fee] move \|x\| ..)` | the closure **and** the captured value | +/// | `ticker(cfg period)` | the data config | +/// | `fold(cfg 0u64, \|a, v\| ..)` | both | +/// | `join(&other, \|x, y\| ..)` | the closure; `&other` is an edge, left alone | +/// +/// So the whole per-instrument shape the generator exists for reads without +/// leaving the macro: +/// +/// ``` +/// use wingfoil::prelude::*; +/// use wingfoil::quoted; +/// # use std::time::Duration; +/// # struct Instrument { tick: Duration, fee: f64 } +/// # let cfg = [Instrument { tick: Duration::from_millis(1), fee: 2.5 }]; +/// # let g = GraphBuilder::new(); +/// for inst in &cfg { +/// let fee = inst.fee; +/// let ticks = quoted!(g => ticker(cfg inst.tick)).count(); +/// let px = quoted!(ticks => map(|n: &u64| *n as f64)); +/// let net = quoted!(px => map([fee] move |p: &f64| p - fee)); +/// assert!(net.src().is_some()); +/// } +/// ``` +/// +/// A `cfg`-marked argument is **cloned** into the call so the original can be +/// rendered — every [`EmitLiteral`](crate::emit::EmitLiteral) type is `Clone`, +/// and for the common `Copy` ones (`Duration`, numbers) the clone is free. +/// /// # What it does not do /// -/// Data configs still need [`with_cfg`](crate::fluent::Stream::with_cfg) -/// explicitly — recording them here would mean either evaluating the argument -/// twice or silently requiring `Clone` of it, and a bound that appears from -/// inside a macro is worse than a second method call. +/// A `&stream` edge is passed through untouched — there is nothing to record +/// about an edge, the graph already knows it. /// -/// Captures still need [`func!`] with an explicit list; a capturing closure -/// written inline here records a body that will not resolve in an artifact. +/// Nothing here detects an *undeclared* capture. `map(move |p| p - fee)` with +/// no `[fee]` records a body that will not resolve in an artifact; it surfaces +/// as a generator refusal, or a pass-2 compile error if the name happens to +/// resolve to something else. #[macro_export] macro_rules! quoted { - // `recv => method(closure)` - ($recv:expr => $m:ident($f:expr)) => {{ + // Arms are ordered most-specific first, and each is discriminated by a + // *literal* token (`cfg`, `[`) before any fragment is parsed. That matters: + // `macro_rules!` falls through cleanly on a literal mismatch, but a failed + // `$x:expr` parse is a hard error, not a fallthrough — so an arm that could + // swallow another's input must come second. + + // --- data config + captured closure ------------------------------------- + ($recv:expr => $m:ident(cfg $a:expr, [$($cap:ident),+ $(,)?] $f:expr)) => {{ + let __wf_c = $a; + let __wf_q = $crate::func!([$($cap),+] $f); + $recv.$m(__wf_c.clone(), __wf_q.f) + .with_cfg(&__wf_c) + .with_src(&__wf_q) + }}; + // --- data config + closure ---------------------------------------------- + ($recv:expr => $m:ident(cfg $a:expr, $f:expr)) => {{ + let __wf_c = $a; let __wf_q = $crate::func!($f); - $recv.$m(__wf_q.f).with_src(&__wf_q) + $recv.$m(__wf_c.clone(), __wf_q.f) + .with_cfg(&__wf_c) + .with_src(&__wf_q) + }}; + // --- data config only (a source: `ticker`, `limit`) --------------------- + ($recv:expr => $m:ident(cfg $a:expr)) => {{ + let __wf_c = $a; + $recv.$m(__wf_c.clone()).with_cfg(&__wf_c) + }}; + // --- leading argument + captured closure -------------------------------- + ($recv:expr => $m:ident($a:expr, [$($cap:ident),+ $(,)?] $f:expr)) => {{ + let __wf_q = $crate::func!([$($cap),+] $f); + $recv.$m($a, __wf_q.f).with_src(&__wf_q) }}; - // `recv => method(arg, closure)` — a stream edge or a seed, then the body. + // --- leading argument + closure (a stream edge for `join`, a seed) ------ ($recv:expr => $m:ident($a:expr, $f:expr)) => {{ let __wf_q = $crate::func!($f); $recv.$m($a, __wf_q.f).with_src(&__wf_q) }}; + // --- captured closure --------------------------------------------------- + ($recv:expr => $m:ident([$($cap:ident),+ $(,)?] $f:expr)) => {{ + let __wf_q = $crate::func!([$($cap),+] $f); + $recv.$m(__wf_q.f).with_src(&__wf_q) + }}; + // --- closure ------------------------------------------------------------ + ($recv:expr => $m:ident($f:expr)) => {{ + let __wf_q = $crate::func!($f); + $recv.$m(__wf_q.f).with_src(&__wf_q) + }}; } diff --git a/crates/wingfoil/tests/quoted_macro.rs b/crates/wingfoil/tests/quoted_macro.rs index 14af3fe95..218023bc2 100644 --- a/crates/wingfoil/tests/quoted_macro.rs +++ b/crates/wingfoil/tests/quoted_macro.rs @@ -123,3 +123,140 @@ fn it_is_not_coupled_to_any_particular_op() { let inspected = quoted!(ticks => inspect(|_v: &u64| ())); assert_eq!(Some("|_v: &u64| ()"), inspected.src().as_deref()); } + +// --------------------------------------------------------------------------- +// The full argument grammar: `cfg` marks a data config, `[..]` a capture list. +// --------------------------------------------------------------------------- + +/// `cfg` on a source's argument records the data config, so `ticker(cfg p)` +/// replaces the `ticker(p).with_cfg(&p)` repetition — where the value was +/// written twice and either could be forgotten or, worse, changed alone. +#[test] +fn cfg_marks_a_data_config() { + let g = GraphBuilder::new(); + let ticks = quoted!(g => ticker(cfg PERIOD)); + + assert_eq!( + Some("::core::time::Duration::new(0u64, 1000000u32)"), + ticks.cfg_src().as_deref() + ); + assert_eq!(None, ticks.src(), "a ticker has no closure"); +} + +/// A capture list inside `quoted!` forwards to `func!`'s tier-2 arm, so the +/// per-instrument parameter case needs no manual `func!` + `with_src`. +#[test] +fn a_capture_list_is_forwarded_to_func() { + let g = GraphBuilder::new(); + let fee = 2.5f64; + let px = g.ticker(PERIOD).count().map(|n: &u64| *n as f64); + let net = quoted!(px => map([fee] move |p: &f64| p - fee)); + + assert_eq!( + Some("{ let fee = 2.5f64; move |p: &f64| p - fee }"), + net.src().as_deref() + ); +} + +/// Multiple captures, and the manual form it replaces, recording identically. +#[test] +fn captures_match_the_manual_form() { + let lo = 1u64; + let hi = 9u64; + + let manual = { + let g = GraphBuilder::new(); + let q = func!([lo, hi] move |v: &u64| v.clamp(&lo, &hi).to_owned()); + g.ticker(PERIOD).count().map(q.f).with_src(&q).src() + }; + let shorthand = { + let g = GraphBuilder::new(); + let ticks = g.ticker(PERIOD).count(); + quoted!(ticks => map([lo, hi] move |v: &u64| v.clamp(&lo, &hi).to_owned())).src() + }; + + assert_eq!(manual, shorthand); + assert!( + shorthand + .unwrap() + .starts_with("{ let lo = 1u64; let hi = 9u64;") + ); +} + +/// `cfg` and a closure together — `fold`'s shape, both recorded from one call. +#[test] +fn cfg_and_a_closure_are_both_recorded() { + let g = GraphBuilder::new(); + let ticks = g.ticker(PERIOD).count(); + let total = quoted!(ticks => fold(cfg 0u64, |acc: &mut u64, v: &u64| *acc += v)); + + assert_eq!(Some("0u64"), total.cfg_src().as_deref()); + assert_eq!( + Some("|acc: &mut u64, v: &u64| *acc += v"), + total.src().as_deref() + ); +} + +/// A `&stream` edge stays an edge: nothing is recorded about it, because the +/// graph already knows its edges. +#[test] +fn a_stream_edge_is_passed_through_untouched() { + let g = GraphBuilder::new(); + let a = g.ticker(PERIOD).count(); + let b = quoted!(a => map(|i: &u64| i * 10)); + let joined = quoted!(a => join(&b, |x: &u64, y: &u64| x + y)); + + assert_eq!(Some("|x: &u64, y: &u64| x + y"), joined.src().as_deref()); + assert_eq!(None, joined.cfg_src(), "an edge is not a data config"); +} + +/// The whole per-instrument shape without leaving the macro — the case the +/// generator exists for, and the one that previously needed all three +/// vocabularies by hand. +#[test] +fn a_per_instrument_leg_needs_only_this_macro() { + struct Instrument { + tick: Duration, + fee: f64, + } + let cfg = [ + Instrument { + tick: Duration::from_millis(1), + fee: 2.5, + }, + Instrument { + tick: Duration::from_millis(5), + fee: 1.0, + }, + ]; + + let g = GraphBuilder::new(); + for inst in &cfg { + let fee = inst.fee; + let ticks = quoted!(g => ticker(cfg inst.tick)).count(); + let px = quoted!(ticks => map(|n: &u64| *n as f64 * 100.0)); + let _net = quoted!(px => map([fee] move |p: &f64| p - fee)); + } + + // Every node that needs recording has it: both tickers, both maps, both + // fee legs. `count` needs nothing. + let nodes = g.describe(); + let unrecorded: Vec<_> = nodes + .iter() + .filter(|n| n.takes_closure_cfg && n.src.is_none()) + .collect(); + assert!(unrecorded.is_empty(), "unrecorded closures: {unrecorded:?}"); + assert_eq!( + 2, + nodes.iter().filter(|n| n.cfg_src.is_some()).count(), + "both ticker periods recorded" + ); + assert_eq!( + 2, + nodes + .iter() + .filter(|n| n.src.as_deref().is_some_and(|s| s.contains("let fee ="))) + .count(), + "both fees frozen" + ); +} From 49bb0225b214e66424eb0e00d7562dfa92793d4d Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 8 Aug 2026 11:30:28 +0000 Subject: [PATCH 2/2] Rename the test fixture's `tick: Duration` to `period` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `ticker`'s parameter is `period: Duration` — an interval between synthetic ticks. In a trading context a "tick" is a price increment (tick size) or a single market-data update, and neither is a `Duration`, so an `Instrument` field called `tick` reads as the wrong thing entirely to the audience most likely to read these tests. The doc comment also records why the field exists at all, which is the more useful correction: it is there because `ticker` stands in for a market-data feed. A real instrument config carries a symbol and a subscription, and data arrives when it arrives. `external`/`channel` sources are still excluded from compiled graphs, so the placeholder is load-bearing for the test rather than representative of the domain — worth saying, so nobody reads these fixtures as a suggested config shape. Test-only rename plus a doc example; no behaviour change. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01X4eojqRWBJ6uK5v5JDzmkd --- crates/wingfoil/src/quote.rs | 6 +++--- crates/wingfoil/tests/codegen_emission.rs | 19 ++++++++++++++----- crates/wingfoil/tests/quoted_macro.rs | 17 +++++++++++++---- 3 files changed, 30 insertions(+), 12 deletions(-) diff --git a/crates/wingfoil/src/quote.rs b/crates/wingfoil/src/quote.rs index 14c711c39..e4681b23f 100644 --- a/crates/wingfoil/src/quote.rs +++ b/crates/wingfoil/src/quote.rs @@ -290,12 +290,12 @@ macro_rules! func { /// use wingfoil::prelude::*; /// use wingfoil::quoted; /// # use std::time::Duration; -/// # struct Instrument { tick: Duration, fee: f64 } -/// # let cfg = [Instrument { tick: Duration::from_millis(1), fee: 2.5 }]; +/// # struct Instrument { period: Duration, fee: f64 } +/// # let cfg = [Instrument { period: Duration::from_millis(1), fee: 2.5 }]; /// # let g = GraphBuilder::new(); /// for inst in &cfg { /// let fee = inst.fee; -/// let ticks = quoted!(g => ticker(cfg inst.tick)).count(); +/// let ticks = quoted!(g => ticker(cfg inst.period)).count(); /// let px = quoted!(ticks => map(|n: &u64| *n as f64)); /// let net = quoted!(px => map([fee] move |p: &f64| p - fee)); /// assert!(net.src().is_some()); diff --git a/crates/wingfoil/tests/codegen_emission.rs b/crates/wingfoil/tests/codegen_emission.rs index eb4d161e9..a9ea7de5b 100644 --- a/crates/wingfoil/tests/codegen_emission.rs +++ b/crates/wingfoil/tests/codegen_emission.rs @@ -522,16 +522,25 @@ wingfoil::nitro! { #[test] fn a_per_instrument_desk_generates_with_its_own_parameters() { struct Instrument { - tick: Duration, + /// The synthetic clock's period. Named `period`, not `tick`: in a + /// trading context a "tick" is a price increment or a single market + /// update, neither of which is a `Duration`. + /// + /// It exists only because `ticker` stands in for a market-data feed — + /// a real instrument config carries a symbol and a subscription, and + /// data arrives when it arrives. `external`/`channel` sources are still + /// excluded from compiled graphs, so the placeholder is load-bearing + /// for the test rather than representative of the domain. + period: Duration, fee: f64, } let cfg = [ Instrument { - tick: Duration::from_millis(1), + period: Duration::from_millis(1), fee: 2.5, }, Instrument { - tick: Duration::from_millis(5), + period: Duration::from_millis(5), fee: 1.0, }, ]; @@ -542,8 +551,8 @@ fn a_per_instrument_desk_generates_with_its_own_parameters() { .iter() .map(|inst| { let px = g - .ticker(inst.tick) - .with_cfg(&inst.tick) + .ticker(inst.period) + .with_cfg(&inst.period) .count() .map(to_px.f) .with_src(&to_px); diff --git a/crates/wingfoil/tests/quoted_macro.rs b/crates/wingfoil/tests/quoted_macro.rs index 218023bc2..83badfda3 100644 --- a/crates/wingfoil/tests/quoted_macro.rs +++ b/crates/wingfoil/tests/quoted_macro.rs @@ -216,16 +216,25 @@ fn a_stream_edge_is_passed_through_untouched() { #[test] fn a_per_instrument_leg_needs_only_this_macro() { struct Instrument { - tick: Duration, + /// The synthetic clock's period. Named `period`, not `tick`: in a + /// trading context a "tick" is a price increment or a single market + /// update, neither of which is a `Duration`. + /// + /// It exists only because `ticker` stands in for a market-data feed — + /// a real instrument config carries a symbol and a subscription, and + /// data arrives when it arrives. `external`/`channel` sources are still + /// excluded from compiled graphs, so the placeholder is load-bearing + /// for the test rather than representative of the domain. + period: Duration, fee: f64, } let cfg = [ Instrument { - tick: Duration::from_millis(1), + period: Duration::from_millis(1), fee: 2.5, }, Instrument { - tick: Duration::from_millis(5), + period: Duration::from_millis(5), fee: 1.0, }, ]; @@ -233,7 +242,7 @@ fn a_per_instrument_leg_needs_only_this_macro() { let g = GraphBuilder::new(); for inst in &cfg { let fee = inst.fee; - let ticks = quoted!(g => ticker(cfg inst.tick)).count(); + let ticks = quoted!(g => ticker(cfg inst.period)).count(); let px = quoted!(ticks => map(|n: &u64| *n as f64 * 100.0)); let _net = quoted!(px => map([fee] move |p: &f64| p - fee)); }