Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 36 additions & 0 deletions .claude/commands/new-op.md
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ shape decides what you *write in the op*, not how much wiring you hand-code:
| **Source** (no input) | `()` | `#[op(build = name)]` + `start` that `schedule`s | `name(cfg)` | `Ticker`, `Const` |
| **Signature ≠ shape** | any | `#[op(build = name, no_builder)]` + hand `Builder` method | — | `WithTime` |
| **Phantom type parameter** (a stage, a unit, a marker) | any | `#[op(build = name, explicit = S)]` | `.name::<S>()` — the type crosses as a `PhantomData` argument | `Stamp`, `StampPrecise` |
| **Side-effect sink** (the point is the effect, not the handle) | any | `#[op(build = name, fluent, sink)]` | same — the flag only drops the generated `#[must_use]` (step 4c) | `Print`, `ForEach`, `Inspect`, `Timed`, `Finally` |
| **Variadic** (any number of same-type edges) | `&'a [(&'a T, bool)]` | no attribute — hand `Builder` method *and* hand forwarders | `name(&[Handle<T>])` | `MergeN` |

**Declare a tick flag only on edges whose `cycle` actually reads it.** Every
Expand Down Expand Up @@ -450,6 +451,41 @@ may legitimately be stricter than the op needs (`StreamOps::accumulate`
requires `T: Default` where the op, whose `Out` is `Vec<T>`, does not), and
going through it would import that bound into a signature nobody wrote.

## 4c. `#[must_use]` — on by default, off for sinks

Dropping a combinator's result is **not** a no-op here: `Stream::wire` has
already registered the node with the shared `Builder`, nothing prunes
unreachable nodes, so a discarded stream stays wired and cycles every tick for
the whole run producing a value nobody reads (#830). Transforms and sources
therefore carry, verbatim:

```rust
#[must_use = "a dropped stream stays wired and cycles every tick, producing an unread value"]
```

Where it goes depends on who writes the method, and there is one trap:

- **Fluent trait method → on the hand-written *declaration*.** Never inside the
`__wf_fluent_*` expansion. That lands in a trait `impl`, where `#[must_use]`
is inert (rustc resolves a method call to the *trait's* item) and rustc warns
`unused_attributes` — a future hard error — for good measure.
- **`Signal` method → generated for you**, because those are inherent. Nothing
to write.
- **Sinks get neither.** `#[op(build = name, fluent, sink)]` suppresses the
generated one, and you leave it off the fluent declaration. `for_each` /
`for_each_mut` / `print` / `logged` / `inspect` / `timed` / `finally` — and
`StreamOps::feedback`, which sends to its sink whether or not the
pass-through is kept — are called as bare statements throughout this tree
(`examples/adapters/fix/main.rs`), and their side effect happens regardless
of the handle. A false positive here teaches users to ignore the warning, so
**when a new op is genuinely ambiguous — the handle is useful *and* the node
earns its keep unread — leave the attribute off.**

Pin any addition in `tests/trybuild/must_use_combinators.rs`: it `#![deny]`s
`unused_must_use` so a warning becomes compiler output trybuild can compare,
and it exercises the sinks under the same `deny` so a mis-scoped attribute
breaks the fixture. Regenerate with `TRYBUILD=overwrite`.

## 5. `nitro!` / compiled coverage

For any `#[op]` op this is **zero-touch**: the attribute emits the
Expand Down
47 changes: 44 additions & 3 deletions crates/wingfoil-derive/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1415,12 +1415,17 @@ pub fn op(attr: TokenStream, item: TokenStream) -> TokenStream {
/// state and the value slot (so passive reads before the first tick see
/// the seed), and the op's `Cfg` is a literal closure passed by value. The
/// generated `Builder` method takes that seed as an `init` parameter ahead
/// of the config — `fold(src, init, f)`.
/// of the config — `fold(src, init, f)`;
/// - `sink` — this op exists for its **side effect** (`print`, `for_each`,
/// `inspect`, `timed`, `finally`), so discarding what it returns is
/// idiomatic rather than a mistake. It suppresses the `#[must_use]` the
/// generated methods otherwise carry; see [`expand_signal`].
struct OpArgs {
build: Ident,
no_builder: bool,
init_arg: bool,
fluent: bool,
sink: bool,
passive: u32,
/// Impl type parameters the **call site** supplies by turbofish, because
/// nothing in the op's `Cfg` / `In` / `Out` mentions them and inference
Expand Down Expand Up @@ -1451,6 +1456,7 @@ impl Parse for OpArgs {
let mut no_builder = false;
let mut init_arg = false;
let mut fluent = false;
let mut sink = false;
let mut passive: u32 = 0;
let mut explicit: Vec<Ident> = Vec::new();
while input.peek(Token![,]) {
Expand All @@ -1460,6 +1466,7 @@ impl Parse for OpArgs {
"no_builder" => no_builder = true,
"init_arg" => init_arg = true,
"fluent" => fluent = true,
"sink" => sink = true,
// `explicit = S` / `explicit = [S, T]` — impl type params the
// call site supplies by turbofish. See `OpArgs::explicit`.
"explicit" => {
Expand Down Expand Up @@ -1494,8 +1501,8 @@ impl Parse for OpArgs {
flag.span(),
format!(
"unknown #[op] flag `{other}`; expected `no_builder`, \
`init_arg`, `fluent`, `explicit = [..]`, or \
`passive = [..]`"
`init_arg`, `fluent`, `sink`, `explicit = [..]`, \
or `passive = [..]`"
),
));
}
Expand All @@ -1506,6 +1513,7 @@ impl Parse for OpArgs {
no_builder,
init_arg,
fluent,
sink,
passive,
explicit,
})
Expand Down Expand Up @@ -2558,6 +2566,15 @@ fn subst_ident(ts: TokenStream2, from: &Ident, to: &TokenStream2) -> TokenStream
/// signature that drifts from the op's shape is a compile error rather than a
/// silent mismatch.
///
/// That split is also why the combinators' `#[must_use]` (#830) lives on those
/// hand-written declarations and **not** in this quote. What this macro emits
/// lands inside a trait `impl`, and `#[must_use]` on a trait-impl method is
/// inert — rustc resolves a method call to the *trait's* item, so the attribute
/// there would never fire, and it warns `unused_attributes` ("cannot be used on
/// trait methods in impl blocks", a future hard error) into the bargain. The
/// generated `Signal` twin is inherent, so it carries the attribute itself; see
/// [`expand_signal`].
///
/// One shape stays hand-written by construction rather than oversight: an op
/// whose fluent signature orders its parameters differently from the generated
/// `(edges.., init, cfg)` — `delay_with_reset(delay, trigger)` — cannot be
Expand Down Expand Up @@ -2736,6 +2753,7 @@ fn expand_fluent(args: &OpArgs, b: &BuilderShape<'_>) -> syn::Result<TokenStream
);
let signal = expand_signal(SignalShape {
name,
sink: args.sink,
receiver: &receiver,
matcher: &matcher,
generics: &generics,
Expand Down Expand Up @@ -2774,6 +2792,9 @@ fn expand_fluent(args: &OpArgs, b: &BuilderShape<'_>) -> syn::Result<TokenStream
/// the `Signal` twin differs only in the types it wraps and the body it emits.
struct SignalShape<'a> {
name: &'a Ident,
/// `#[op(sink)]`: the op is wired for its side effect, so the generated
/// method is *not* `#[must_use]`.
sink: bool,
receiver: &'a FluentReceiver,
matcher: &'a TokenStream2,
generics: &'a TokenStream2,
Expand Down Expand Up @@ -2821,6 +2842,16 @@ struct SignalShape<'a> {
/// makes the graph (`signal::ticker`), which is a different shape and stays
/// hand-written.
///
/// These being inherent is also what lets the generator carry the combinators'
/// `#[must_use]` (#830) — the attribute is inert on the trait-impl methods
/// [`expand_fluent`] emits, so there it stays on the hand-written declaration.
/// Dropping a `Signal` combinator's result has the same consequence as dropping
/// a `Stream`'s: the node is registered with the shared builder, nothing prunes
/// it, and it cycles for the whole run producing a value nobody reads. The one
/// exception is `#[op(sink)]` — an op wired *for* its side effect (`print`,
/// `for_each`, `inspect`, `timed`, `finally`), where discarding the handle is
/// idiomatic and a warning would be a false positive.
///
/// The expansion targets a `pub(crate)` seam, so it only compiles inside
/// `wingfoil`. That is not a limitation to design around — these are
/// inherent methods on a type this crate owns, so no other crate could invoke
Expand All @@ -2839,6 +2870,15 @@ fn expand_signal(s: SignalShape<'_>) -> TokenStream2 {
.zip(s.edge_tys)
.map(|(id, ty)| quote! { #id: &$crate::signal::Signal<#ty> });

// A transform's or source's result is the only thing it produces; a sink's
// side effect has already happened by the time the handle comes back.
let must_use = if s.sink {
quote! {}
} else {
quote! {
#[must_use = "a dropped stream stays wired and cycles every tick, producing an unread value"]
}
};
let mac = format_ident!("__wf_signal_{name}");
let method_doc = format!(
"The [`Signal`](crate::signal::Signal) form of [`{name}`]\
Expand All @@ -2857,6 +2897,7 @@ fn expand_signal(s: SignalShape<'_>) -> TokenStream2 {
macro_rules! #mac {
#matcher => {
#[doc = #method_doc]
#must_use
pub fn #name #generics (
&self,
#(#edge_params,)*
Expand Down
Loading
Loading