From 82728486202062b9f95bbfd4d679fde7ccf233b3 Mon Sep 17 00:00:00 2001 From: Taus Date: Sat, 4 Jul 2026 15:00:24 +0000 Subject: [PATCH 1/4] yeast: Implement IntoIterator for Id as a singleton There's an awkward divide in yeast between returning a list of nodes or optional node (both of which are iterable), and returning a single node (which is not). In practice, we would like all of these cases to be handled transparently: if a single node is returned, it behaves as if it were a singleton list containing that node. This gives us a uniform interface during translation -- no matter what is returned, it will be an iterable of nodes. To facilitate this, we make the slightly unorthodox choice of implementing IntoIterator for Id, with the behaviour detailed above. --- shared/yeast/src/lib.rs | 27 ++++++++++++++++++--------- 1 file changed, 18 insertions(+), 9 deletions(-) diff --git a/shared/yeast/src/lib.rs b/shared/yeast/src/lib.rs index fdfe4dd0fb01..c50304e623ef 100644 --- a/shared/yeast/src/lib.rs +++ b/shared/yeast/src/lib.rs @@ -26,6 +26,12 @@ use query::QueryNode; /// without colliding with the impls for plain integers. /// /// Use `id.0` (or `id.into()`) to obtain the raw arena index. +/// +/// Implements [`IntoIterator`] as a singleton (`iter::once(self)`) +/// so that a bare `Id` can be used interchangeably with `Option` +/// / `Vec` in places that expect an iterable of ids (e.g. +/// [`crate::build::BuildCtx::translate`] and the field-splice +/// interpolation via [`IntoFieldIds`]). #[repr(transparent)] #[derive(Copy, Clone, Eq, PartialEq, Ord, PartialOrd, Debug, Hash, Serialize)] pub struct Id(pub usize); @@ -42,6 +48,14 @@ impl From for usize { } } +impl IntoIterator for Id { + type Item = Id; + type IntoIter = std::iter::Once; + fn into_iter(self) -> Self::IntoIter { + std::iter::once(self) + } +} + /// Field and Kind ids are provided by tree-sitter type FieldId = u16; type KindId = u16; @@ -49,9 +63,10 @@ type KindId = u16; /// Trait for values that can be appended to a field's id list inside a /// `tree!`/`trees!`/`rule!` template (in `{expr}` placeholders). /// -/// `Id` pushes a single id; the blanket impl for -/// `IntoIterator>` handles `Vec`, `Option`, -/// arbitrary iterators yielding `Id`, etc. +/// The blanket impl for `IntoIterator>` handles all +/// current shapes: `Vec`, `Option`, arbitrary iterators +/// yielding `Id`, and a bare `Id` itself (which is `IntoIterator` +/// via a singleton). /// /// This lets `{expr}` interpolate any of these shapes without a /// dedicated splice syntax — the macro emits the same trait-dispatched @@ -60,12 +75,6 @@ pub trait IntoFieldIds { fn extend_into(self, out: &mut Vec); } -impl IntoFieldIds for Id { - fn extend_into(self, out: &mut Vec) { - out.push(self); - } -} - impl IntoFieldIds for I where I: IntoIterator, From 13510b1dddf6d1cdb7709aacfde6f81d3dd7da1a Mon Sep 17 00:00:00 2001 From: Taus Date: Sat, 4 Jul 2026 15:00:48 +0000 Subject: [PATCH 2/4] yeast: Add BuildCtx::scoped for isolated context modification A previous commit added a translate_reset method on BuildCtx, which had the effect of performing a translation in a completely empty context. One issue with this is that this is an all-or-nothing proposition -- If you want to preserve _some_ parts of the context, you have to do something more complicated. Moreover, if you introduce a contextual value that _should_ be preserved, all of the existing uses of translate_reset now silently do the wrong thing. There are two patterns that we want to address. The first one is "modify the context in some way, then do a translation". If the translation is the last step of a Rust block, then we don't actually need translate_reset -- we could just reset the context and then call `translate`. The fact that the outer context is restored afterwards means it's okay to make destructive changes to `ctx.user_ctx` -- none of these changes will persist. The second pattern is the same, but where we want to do more translations using the original context, after having performed a translation with a modified context. In this case, we cannot just overwrite the context, since that would invalidate the subsequent translations. Instead, we introduce a new method `ctx.scoped` which takes a closure as an argument. With this we can now write ``` ctx.scoped(|ctx| ctx.reset(); ctx.translate(...)); ``` and the closure is run with a copy of `ctx` that has a clone of `user_ctx` on the inside, so no changes will persist. (You may wonder: why not just clone `ctx` and use the clone? The answer is that `ctx` owns mutable pointers to the AST etc., and this makes it awkward to just "clone" it. The closure circumvents this issue nicely, since it can borrow these pointers internally.) For now, this rewrite has the same behaviour as the version that used translate_reset -- we clear the entire `user_ctx`. However, we could imagine being more fine-grained in this approach, by implementing, say, SwiftContext::reset_modifiers (which would only affect what modifiers are currently in the context, leaving everything else as-is). --- shared/yeast/src/build.rs | 94 ++++++++++++------- shared/yeast/src/lib.rs | 20 ++++ .../extractor/src/languages/swift/swift.rs | 59 ++++++++---- 3 files changed, 120 insertions(+), 53 deletions(-) diff --git a/shared/yeast/src/build.rs b/shared/yeast/src/build.rs index 6c2a9d4181d7..f550947ffd4a 100644 --- a/shared/yeast/src/build.rs +++ b/shared/yeast/src/build.rs @@ -161,52 +161,74 @@ impl<'a, C> BuildCtx<'a, C> { } impl BuildCtx<'_, C> { - /// Recursively translate a node via the framework's rule machinery. - /// In a OneShot phase, applies OneShot rules to the given node and - /// returns the resulting node ids. In a Repeating phase, errors - /// (translation is not meaningful when input and output share a - /// schema). + /// Recursively translate every id in the given iterable via the + /// framework's rule machinery. In a OneShot phase, applies OneShot + /// rules to each id and returns the accumulated resulting node ids + /// in order. In a Repeating phase, errors (translation is not + /// meaningful when input and output share a schema). + /// + /// The single-`Id` case works too, because `Id: IntoIterator` as a singleton iterator — so `ctx.translate(some_id)?` + /// returns a `Vec` containing whatever `some_id` translated to. /// /// Errors if this `BuildCtx` was constructed by hand (without a /// translator handle) — for example, in unit tests that don't go /// through the rule driver. - pub fn translate>(&mut self, id: I) -> Result, String> { - let id = id.into(); - match &self.translator { - Some(t) => t.translate(self.ast, self.user_ctx, id), - None => Err("translate() called on a BuildCtx without a translator handle".into()), + pub fn translate>( + &mut self, + ids: impl IntoIterator, + ) -> Result, String> { + let translator = self + .translator + .as_ref() + .ok_or("translate() called on a BuildCtx without a translator handle")?; + let mut out = Vec::new(); + for id in ids { + let translated = translator.translate(self.ast, self.user_ctx, id.into())?; + out.extend(translated); } + Ok(out) } - /// Translate every node in an iterator with a **fresh** user context - /// (reset to `C::default()`), restoring the previous context afterwards. + /// Run `f` with a temporary child [`BuildCtx`] whose `user_ctx` is + /// a fresh clone of the current one, sharing everything else + /// (`ast`, `captures`, `fresh`, `source_range`, `translator`) by + /// re-borrow. Any mutations `f` makes to the child's `user_ctx` + /// are discarded when it returns — no restore needed, because the + /// mutations only ever happened on a local clone. /// - /// Use when descending into a subtree — a body, expression, or statement - /// list — that must not inherit any of the surrounding translation - /// context (for example an enclosing binding modifier). Accepts optional - /// (`Option`) and repeated (`Vec`) captures (both `IntoIterator`); - /// for a single `Id`, wrap it in `std::iter::once(id)`. - pub fn translate_reset>( - &mut self, - ids: impl IntoIterator, - ) -> Result, String> + /// Use for the rare rule that needs to translate a subtree under a + /// modified context *and then continue using its own (unmodified) + /// context afterwards*. For rules where the modified translation + /// is the last use of `ctx`, mutate `ctx` in place — the + /// framework's rule-boundary save/restore cleans up on rule exit. + /// + /// Example: an outer rule that translates one child subtree with a + /// reset context, then continues with the outer context intact: + /// + /// ```ignore + /// let val = ctx.scoped(|ctx| { + /// ctx.reset(); + /// ctx.translate(val) + /// })?; + /// // `ctx` here is untouched by the reset inside the closure. + /// let other = ctx.translate(other_id)?; + /// ``` + pub fn scoped(&mut self, f: F) -> R where - C: Default, + F: for<'b> FnOnce(&mut BuildCtx<'b, C>) -> R, { - let saved = std::mem::take(&mut *self.user_ctx); - let mut out = Vec::new(); - let mut result = Ok(()); - for id in ids { - match self.translate(id) { - Ok(v) => out.extend(v), - Err(e) => { - result = Err(e); - break; - } - } - } - *self.user_ctx = saved; - result.map(|()| out) + let mut child_user_ctx = self.user_ctx.clone(); + let mut child = BuildCtx { + ast: &mut *self.ast, + captures: self.captures, + fresh: self.fresh, + source_range: self.source_range, + user_ctx: &mut child_user_ctx, + translator: self.translator, + }; + f(&mut child) + // child_user_ctx dropped; the outer `self` is unaffected. } } diff --git a/shared/yeast/src/lib.rs b/shared/yeast/src/lib.rs index c50304e623ef..516f99fc1ceb 100644 --- a/shared/yeast/src/lib.rs +++ b/shared/yeast/src/lib.rs @@ -741,6 +741,16 @@ pub struct TranslatorHandle<'a, C> { inner: TranslatorImpl<'a, C>, } +// Manual `Copy` / `Clone` so `TranslatorHandle<'_, C>: Copy` holds +// regardless of whether `C: Copy`. `TranslatorImpl` contains only +// shared references, which are `Copy` unconditionally. +impl Copy for TranslatorHandle<'_, C> {} +impl Clone for TranslatorHandle<'_, C> { + fn clone(&self) -> Self { + *self + } +} + /// Internal phase-specific translation state. Kept private — callers /// interact with [`TranslatorHandle`] only. enum TranslatorImpl<'a, C> { @@ -761,6 +771,16 @@ enum TranslatorImpl<'a, C> { Repeating, } +// Manual `Copy` / `Clone` so `TranslatorImpl<'_, C>: Copy` holds +// regardless of whether `C: Copy`. All variants hold only shared +// references and small `Copy` scalars. +impl Copy for TranslatorImpl<'_, C> {} +impl Clone for TranslatorImpl<'_, C> { + fn clone(&self) -> Self { + *self + } +} + impl<'a, C: Clone> TranslatorHandle<'a, C> { /// Recursively apply OneShot rules to `id` and return the resulting /// node ids. Errors in a Repeating phase (where translation is not diff --git a/unified/extractor/src/languages/swift/swift.rs b/unified/extractor/src/languages/swift/swift.rs index 0f4f80a8f019..04537f55c09e 100644 --- a/unified/extractor/src/languages/swift/swift.rs +++ b/unified/extractor/src/languages/swift/swift.rs @@ -45,12 +45,29 @@ impl SwiftContext { /// /// True exactly when an enclosing binding has published its modifier into /// `outer_modifiers`. This is reliable because non-binding subtrees - /// (bodies, initializer values, ...) are translated with a reset context - /// (see `BuildCtx::translate_reset`), so a bare identifier only sees a + /// (bodies, initializer values, ...) are translated after resetting the + /// context (see `reset`), so a bare identifier only sees a /// non-empty `outer_modifiers` when it really is a binding. fn in_binding_pattern(&self) -> bool { !self.outer_modifiers.is_empty() } + + /// Clear the context fields that must not propagate into an + /// expression / statement / body subtree. + /// + /// Mirrors `Default::default()` for `SwiftContext` today, but is a + /// named method so future context fields can opt in or out of + /// clearing here per-field. + /// + /// Called before recursively translating a body / initializer + /// slot. Most rules mutate `ctx` in place — the framework's + /// rule-boundary snapshot/restore cleans up on exit. Rules that + /// need the outer context intact *after* the reset-and-translate + /// (see e.g. the `property_binding` willSet/didSet rule) wrap the + /// mutation in `ctx.scoped(...)` instead. + fn reset(&mut self) { + *self = SwiftContext::default(); + } } /// Build a freshly-created `chained_declaration` modifier node if @@ -239,7 +256,7 @@ fn translation_rules() -> Vec> { name: (identifier #{name}) type: {ty} accessor_kind: (accessor_kind "get") - body: (block stmt: {ctx.translate_reset(body)?})) + body: (block stmt: {ctx.reset(); ctx.translate(body)?})) ), // Stored property with willSet/didSet observers (initializer // optional) → a `variable_declaration` followed by one @@ -260,12 +277,20 @@ fn translation_rules() -> Vec> { observers: (willset_didset_block willset: _? @@ws didset: _? @@ds)) => {{ - // The initializer value must not inherit the binding context - // (it may contain patterns, e.g. a switch expression), so - // translate it with a reset context. The observers keep the - // context: each willSet/didSet accessor emits the binding - // modifier and resets its own body. - let val = ctx.translate_reset(val)?; + // The initializer value must not inherit the binding + // context (it may contain patterns, e.g. a switch + // expression), so translate it inside a `ctx.scoped` + // block — the block receives a temporary `ctx` whose + // `user_ctx` is a clone; mutations to it are discarded + // when the block returns, so the outer `ctx` is intact + // for the observer loop below. The observers keep the + // outer context: each willSet/didSet accessor emits + // the binding modifier and, in turn, resets the + // context for its own body. + let val = ctx.scoped(|ctx| { + ctx.reset(); + ctx.translate(val) + })?; let var_decl = tree!( (variable_declaration @@ -295,7 +320,7 @@ fn translation_rules() -> Vec> { // The enclosing `property_declaration` leads `ctx.outer_modifiers` // with the `let`/`var` binding modifier, so the auto-translated name // pattern (the LHS) becomes a binding, while the initializer value is - // translated with a reset context (see `translate_reset`). + // translated with a reset context (see `SwiftContext::reset`). rule!( (property_binding name: @pattern @@ -307,7 +332,7 @@ fn translation_rules() -> Vec> { modifier: {chained_modifier(&mut ctx)} pattern: {pattern} type: {ty} - value: {ctx.translate_reset(val)?}) // reset context: the initializer must not see the binding + value: {ctx.reset(); ctx.translate(val)?}) ), // property_declaration: flatten declarators (each may translate // to multiple nodes — variable_declaration and/or @@ -1118,7 +1143,7 @@ fn translation_rules() -> Vec> { name: {ctx.property_name.ok_or("computed_getter outside property_binding context")?} type: {ctx.property_type} accessor_kind: (accessor_kind "get") - body: (block stmt: {ctx.translate_reset(body)?})) + body: (block stmt: {ctx.reset(); ctx.translate(body)?})) ), // Computed setter with explicit parameter name. rule!( @@ -1131,7 +1156,7 @@ fn translation_rules() -> Vec> { type: {ctx.property_type} accessor_kind: (accessor_kind "set") parameter: (parameter pattern: (name_pattern identifier: (identifier #{param}))) - body: (block stmt: {ctx.translate_reset(body)?})) + body: (block stmt: {ctx.reset(); ctx.translate(body)?})) ), // Computed setter without explicit parameter name; body optional. rule!( @@ -1143,7 +1168,7 @@ fn translation_rules() -> Vec> { name: {ctx.property_name.ok_or("computed_setter outside property_binding context")?} type: {ctx.property_type} accessor_kind: (accessor_kind "set") - body: (block stmt: {ctx.translate_reset(body)?})) + body: (block stmt: {ctx.reset(); ctx.translate(body)?})) ), // Computed modify → accessor_declaration rule!( @@ -1155,7 +1180,7 @@ fn translation_rules() -> Vec> { name: {ctx.property_name.ok_or("computed_modify outside property_binding context")?} type: {ctx.property_type} accessor_kind: (accessor_kind "modify") - body: (block stmt: {ctx.translate_reset(body)?})) + body: (block stmt: {ctx.reset(); ctx.translate(body)?})) ), // willset/didset block — spread to children (only reachable as a // fallback; the outer property_binding manual rule normally @@ -1173,7 +1198,7 @@ fn translation_rules() -> Vec> { modifier: {chained_modifier(&mut ctx)} name: {ctx.property_name.ok_or("willset_clause outside property_binding context")?} accessor_kind: (accessor_kind "willSet") - body: (block stmt: {ctx.translate_reset(body)?})) + body: (block stmt: {ctx.reset(); ctx.translate(body)?})) ), // didset clause → accessor_declaration (body optional). rule!( @@ -1184,7 +1209,7 @@ fn translation_rules() -> Vec> { modifier: {chained_modifier(&mut ctx)} name: {ctx.property_name.ok_or("didset_clause outside property_binding context")?} accessor_kind: (accessor_kind "didSet") - body: (block stmt: {ctx.translate_reset(body)?})) + body: (block stmt: {ctx.reset(); ctx.translate(body)?})) ), // Preprocessor conditionals — unsupported rule!((diagnostic) => (unsupported_node)), From 9b7a9c3851e49a68fa32b4ce9c20554851551af3 Mon Sep 17 00:00:00 2001 From: Taus Date: Mon, 6 Jul 2026 12:04:44 +0000 Subject: [PATCH 3/4] yeast: Use clone-and-drop for per-rule context isolation In order to avoid having context changes bubble up through the tree (or from one sibling to another) the current framework makes a copy of the context before calling `translate` recursively, and then restores it afterwards. However, this is a bit silly -- after we're done with all of the translations, there's really no need to restore the context (as it doesn't get accessed again). So instead we change it from "save and then restore" to "clone and then drop". Each rule invocation gets its own copy of the context, and simply drops it when it's done. --- shared/yeast/src/build.rs | 5 +-- shared/yeast/src/lib.rs | 36 ++++++++++--------- .../extractor/src/languages/swift/swift.rs | 9 ++--- 3 files changed, 27 insertions(+), 23 deletions(-) diff --git a/shared/yeast/src/build.rs b/shared/yeast/src/build.rs index f550947ffd4a..0877da22f86d 100644 --- a/shared/yeast/src/build.rs +++ b/shared/yeast/src/build.rs @@ -200,8 +200,9 @@ impl BuildCtx<'_, C> { /// Use for the rare rule that needs to translate a subtree under a /// modified context *and then continue using its own (unmodified) /// context afterwards*. For rules where the modified translation - /// is the last use of `ctx`, mutate `ctx` in place — the - /// framework's rule-boundary save/restore cleans up on rule exit. + /// is the last use of `ctx`, mutate `ctx` in place — the framework + /// invokes each rule with a private clone of the user context, so + /// mutations are discarded on rule exit anyway. /// /// Example: an outer rule that translates one child subtree with a /// reset context, then continues with the outer context intact: diff --git a/shared/yeast/src/lib.rs b/shared/yeast/src/lib.rs index 516f99fc1ceb..2f6da2039d12 100644 --- a/shared/yeast/src/lib.rs +++ b/shared/yeast/src/lib.rs @@ -999,10 +999,13 @@ fn apply_repeating_rules_inner( if Some(rule_ptr) == skip_rule { continue; } - // Snapshot the user context before invoking the rule so that any - // mutations the rule makes are visible during recursive translation - // of its result, but not leaked to the parent's siblings. - let snapshot = user_ctx.clone(); + // Give each rule attempt a private clone of the user context. + // Any mutations the rule makes are visible to its transform and + // to the recursive translation of its result, but never leak + // back to the parent — the clone is simply dropped when we + // return. This is also `?`-safe: an error return drops `local` + // without touching the caller's `user_ctx`. + let mut local = user_ctx.clone(); // Repeating rules don't need a real translator: their captures // aren't auto-translated (Repeating preserves the input schema), // and `ctx.translate(id)` errors if invoked from a Repeating @@ -1010,7 +1013,7 @@ fn apply_repeating_rules_inner( let translator = TranslatorHandle { inner: TranslatorImpl::Repeating, }; - let try_result = rule.try_rule(ast, id, fresh, user_ctx, translator)?; + let try_result = rule.try_rule(ast, id, fresh, &mut local, translator)?; if let Some(result_node) = try_result { // For non-repeated rules, suppress further application of *this* // rule on the result root, so a rule whose output matches its own @@ -1022,19 +1025,17 @@ fn apply_repeating_rules_inner( results.extend(apply_repeating_rules_inner( index, ast, - user_ctx, + &mut local, node, fresh, rewrite_depth + 1, next_skip, )?); } - *user_ctx = snapshot; return Ok(results); } - // Rule didn't match; restore any speculative changes (none expected - // since try_rule only mutates on match, but be defensive). - *user_ctx = snapshot; + // Rule didn't match; `local` is dropped as we loop to the next + // rule. } // Take the parent's fields by ownership: the recursion will rewrite @@ -1116,11 +1117,13 @@ fn apply_one_shot_rules_inner( for rule in index.rules_for_kind(node_kind) { if let Some(captures) = rule.try_match(ast, id)? { - // Snapshot the user context before invoking the rule so that any - // mutations the rule (or its transitively-translated captures) - // make are visible during this rule's transform, but not leaked - // to the parent's siblings. - let snapshot = user_ctx.clone(); + // Give the rule a private clone of the user context. Any + // mutations the rule (or its transitively-translated + // captures) make are visible during this rule's transform, + // but never leak back — the clone is dropped when we + // return. `?`-safe: an error return drops `local` without + // touching the caller's `user_ctx`. + let mut local = user_ctx.clone(); // Build the translator handle the transform will use to // recursively translate captures (or, for macro-generated // rules, the auto-translate prefix uses it to translate every @@ -1133,8 +1136,7 @@ fn apply_one_shot_rules_inner( matched_root: id, }, }; - let result = rule.run_transform(ast, captures, id, fresh, user_ctx, translator)?; - *user_ctx = snapshot; + let result = rule.run_transform(ast, captures, id, fresh, &mut local, translator)?; return Ok(result); } } diff --git a/unified/extractor/src/languages/swift/swift.rs b/unified/extractor/src/languages/swift/swift.rs index 04537f55c09e..54cce1a7fc4c 100644 --- a/unified/extractor/src/languages/swift/swift.rs +++ b/unified/extractor/src/languages/swift/swift.rs @@ -60,10 +60,11 @@ impl SwiftContext { /// clearing here per-field. /// /// Called before recursively translating a body / initializer - /// slot. Most rules mutate `ctx` in place — the framework's - /// rule-boundary snapshot/restore cleans up on exit. Rules that - /// need the outer context intact *after* the reset-and-translate - /// (see e.g. the `property_binding` willSet/didSet rule) wrap the + /// slot. Most rules mutate `ctx` in place — the framework invokes + /// each rule with a private clone of the user context, so + /// mutations are discarded on rule exit anyway. Rules that need + /// the outer context intact *after* the reset-and-translate (see + /// e.g. the `property_binding` willSet/didSet rule) wrap the /// mutation in `ctx.scoped(...)` instead. fn reset(&mut self) { *self = SwiftContext::default(); From 33da3ef74e01c8b559b47ea22dd7cfac40b500b4 Mon Sep 17 00:00:00 2001 From: Taus Date: Wed, 8 Jul 2026 16:35:01 +0200 Subject: [PATCH 4/4] yeast: Fix typo Co-authored-by: Jeroen Ketema <93738568+jketema@users.noreply.github.com> --- shared/yeast/src/build.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/shared/yeast/src/build.rs b/shared/yeast/src/build.rs index 0877da22f86d..68a4c883242a 100644 --- a/shared/yeast/src/build.rs +++ b/shared/yeast/src/build.rs @@ -168,7 +168,7 @@ impl BuildCtx<'_, C> { /// meaningful when input and output share a schema). /// /// The single-`Id` case works too, because `Id: IntoIterator` as a singleton iterator — so `ctx.translate(some_id)?` + /// = Id>` is a singleton iterator — so `ctx.translate(some_id)?` /// returns a `Vec` containing whatever `some_id` translated to. /// /// Errors if this `BuildCtx` was constructed by hand (without a