From d9e32041f051f9a2b5fef07c78dc97dcd5c2d159 Mon Sep 17 00:00:00 2001 From: Taus Date: Fri, 30 Oct 2026 15:52:33 +0000 Subject: [PATCH 1/2] yeast: Clone `ctx` for each interpolation This prevents mutations in one output template interpolation from affecting sibling output. --- shared/yeast-macros/src/lower.rs | 88 +++++++++++++++++--------------- shared/yeast/src/build.rs | 45 +++++++++++++++- shared/yeast/tests/test.rs | 55 ++++++++++++++++++++ 3 files changed, 147 insertions(+), 41 deletions(-) diff --git a/shared/yeast-macros/src/lower.rs b/shared/yeast-macros/src/lower.rs index e127423910d5..253072d4f0d2 100644 --- a/shared/yeast-macros/src/lower.rs +++ b/shared/yeast-macros/src/lower.rs @@ -304,11 +304,11 @@ impl Template { } Template::Splice(block) => { let block = rust_block(block); - quote! { + isolated_interpolation(quote! { let __value = ::std::iter::IntoIterator::into_iter(#block) .collect::>(); ctx.extend_field_ids(__value, &mut __nodes); - } + }) } }) .collect::>(); @@ -327,7 +327,9 @@ impl Template { Template::Node(node) => node.lower(context), Template::Splice(block) => { let block = rust_block(block); - quote! { ::std::convert::Into::::into(#block) } + isolated_interpolation(quote! { + ::std::convert::Into::::into(#block) + }) } } } @@ -344,40 +346,36 @@ impl Node { Content::Computed(block) => { let block = rust_block(block); if let Some(label) = context.fallible_scope { - quote! { - { - let __expr = #block; - let ::std::option::Option::Some(__value_ref) = - yeast::MaybeYeastValue::maybe_yeast_value(&__expr) - else { - break #label ::std::option::Option::None; - }; - let __value = - yeast::YeastDisplay::yeast_to_string(__value_ref, &#ctx); - let __source_range = - yeast::YeastSourceRange::yeast_source_range(__value_ref, &#ctx); - #ctx.literal_with_source_range( - #kind_str, - &__value, - __source_range, - ) - } - } + isolated_interpolation(quote! { + let __expr = #block; + let ::std::option::Option::Some(__value_ref) = + yeast::MaybeYeastValue::maybe_yeast_value(&__expr) + else { + break #label ::std::option::Option::None; + }; + let __value = + yeast::YeastDisplay::yeast_to_string(__value_ref, &#ctx); + let __source_range = + yeast::YeastSourceRange::yeast_source_range(__value_ref, &#ctx); + #ctx.literal_with_source_range( + #kind_str, + &__value, + __source_range, + ) + }) } else { - quote! { - { - let __expr = #block; - let __value = - yeast::YeastDisplay::yeast_to_string(&__expr, &#ctx); - let __source_range = - yeast::YeastSourceRange::yeast_source_range(&__expr, &#ctx); - #ctx.literal_with_source_range( - #kind_str, - &__value, - __source_range, - ) - } - } + isolated_interpolation(quote! { + let __expr = #block; + let __value = + yeast::YeastDisplay::yeast_to_string(&__expr, &#ctx); + let __source_range = + yeast::YeastSourceRange::yeast_source_range(&__expr, &#ctx); + #ctx.literal_with_source_range( + #kind_str, + &__value, + __source_range, + ) + }) } } }; @@ -393,11 +391,11 @@ impl Node { match field { Field::Location { block } => { let block = rust_block(block); - stmts.push(quote! { + stmts.push(isolated_interpolation(quote! { let __value = ::std::iter::IntoIterator::into_iter(#block) .collect::>(); #ctx.extend_location_ranges(__value, &mut __location_ranges); - }); + })); } Field::Named { name, @@ -418,12 +416,15 @@ impl Node { Template::Splice(block) => { debug_assert!(!optional); let block = rust_block(block); - stmts.push(quote! { - let mut #temp: Vec = Vec::new(); + let interpolation = isolated_interpolation(quote! { let __value = ::std::iter::IntoIterator::into_iter(#block) .collect::>(); #ctx.extend_field_ids(__value, &mut #temp); }); + stmts.push(quote! { + let mut #temp: Vec = Vec::new(); + #interpolation + }); field_args.push(quote! { if !#temp.is_empty() { __fields.push((#field_str, #temp)); @@ -486,3 +487,10 @@ fn rust_block(block: &syn::Block) -> TokenStream { let statements = &block.stmts; quote!({ #(#statements)* }) } + +fn isolated_interpolation(body: TokenStream) -> TokenStream { + quote! {{ + let mut ctx = ctx.cloned_user_ctx(); + #body + }} +} diff --git a/shared/yeast/src/build.rs b/shared/yeast/src/build.rs index f3075973be41..05407d9f9969 100644 --- a/shared/yeast/src/build.rs +++ b/shared/yeast/src/build.rs @@ -56,6 +56,39 @@ pub struct BuildCtx<'a, C: 'a = ()> { created_nodes_without_source_range: BTreeSet, } +#[doc(hidden)] +/// RAII guard that gives an output-template interpolation its own user context +/// while preserving the surrounding lexical scope for control flow. +/// +/// It temporarily swaps a clone into the existing [`BuildCtx`] and restores +/// the original context on drop. +pub struct ClonedUserCtx<'ctx, 'a, C: 'a> { + ctx: &'ctx mut BuildCtx<'a, C>, + original_user_ctx: Option, +} + +impl<'a, C> std::ops::Deref for ClonedUserCtx<'_, 'a, C> { + type Target = BuildCtx<'a, C>; + + fn deref(&self) -> &Self::Target { + self.ctx + } +} + +impl std::ops::DerefMut for ClonedUserCtx<'_, '_, C> { + fn deref_mut(&mut self) -> &mut Self::Target { + self.ctx + } +} + +impl Drop for ClonedUserCtx<'_, '_, C> { + fn drop(&mut self) { + if let Some(original_user_ctx) = self.original_user_ctx.take() { + *self.ctx.user_ctx = original_user_ctx; + } + } +} + impl<'a, C> BuildCtx<'a, C> { pub fn new(ast: &'a Ast, output: &'a mut OutputAst, user_ctx: &'a mut C) -> Self { Self { @@ -263,7 +296,17 @@ impl<'a, C> BuildCtx<'a, C> { } } -impl BuildCtx<'_, C> { +impl<'a, C: Clone> BuildCtx<'a, C> { + #[doc(hidden)] + pub fn cloned_user_ctx(&mut self) -> ClonedUserCtx<'_, 'a, C> { + let cloned_user_ctx = (*self.user_ctx).clone(); + let original_user_ctx = std::mem::replace(self.user_ctx, cloned_user_ctx); + ClonedUserCtx { + ctx: self, + original_user_ctx: Some(original_user_ctx), + } + } + #[doc(hidden)] pub fn extend_field_ids>(&mut self, value: V, out: &mut Vec) { value.extend_into(self, out); diff --git a/shared/yeast/tests/test.rs b/shared/yeast/tests/test.rs index f53b3e22615a..04f8ce3ab76d 100644 --- a/shared/yeast/tests/test.rs +++ b/shared/yeast/tests/test.rs @@ -713,6 +713,61 @@ fn test_tree_builder() { ); } +#[derive(Clone, Default)] +struct InterpolationContext { + counter: usize, +} + +#[test] +fn test_output_template_interpolations_clone_user_context() { + let input = "x = 1"; + let input_ast = parse_input(input); + let mut ast = empty_output_ast(input); + let mut user_ctx = InterpolationContext::default(); + let mut ctx = yeast::build::BuildCtx::new(&input_ast, &mut ast, &mut user_ctx); + let source = yeast::tree!((identifier "source")); + + let nodes = yeast::trees!( + (call + location: { + ctx.counter += 1; + source + } + method: { + ctx.counter += 1; + let value = ctx.counter.to_string(); + ctx.literal("identifier", &value) + } + receiver: (identifier #{ + ctx.counter += 1; + ctx.counter + }) + ) + { + ctx.counter += 1; + let value = ctx.counter.to_string(); + [ctx.literal("identifier", &value)] + } + ); + + assert_eq!(ctx.counter, 0); + assert_eq!(nodes.len(), 2); + assert_dump_eq( + &dump_ast(ctx.output, nodes[0], input), + r#" + call + method: identifier "1" + receiver: identifier "1" + "#, + ); + assert_dump_eq( + &dump_ast(ctx.output, nodes[1], input), + r#" + identifier "1" + "#, + ); +} + /// Builds `(assignment left: … right: (integer #{value})?)`, where a `None` /// `value` leaves `right` unset rather than producing an `integer` with no /// content. From 3c13da38edd62d92d61886f87ddeaf482bad8ae1 Mon Sep 17 00:00:00 2001 From: Taus Date: Fri, 30 Oct 2026 16:29:47 +0000 Subject: [PATCH 2/2] unified: Remove redundant Swift context scopes With the new context isolation, we don't need to explicitly introduce a new scope. We could probably get rid of `ctx.scoped` now, but there's no harm in leaving it around. --- .../extractor/src/languages/swift/swift.rs | 39 +++++++------------ 1 file changed, 13 insertions(+), 26 deletions(-) diff --git a/unified/extractor/src/languages/swift/swift.rs b/unified/extractor/src/languages/swift/swift.rs index a1dab86b626e..72516d7645f8 100644 --- a/unified/extractor/src/languages/swift/swift.rs +++ b/unified/extractor/src/languages/swift/swift.rs @@ -48,13 +48,11 @@ impl SwiftContext { /// 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 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. + /// Called before recursively translating a body / initializer slot. + /// Output-template interpolations each receive a private clone of the user + /// context, so resetting one interpolation cannot affect sibling fields. + /// Rules can also mutate `ctx` in place when the translation is their last + /// use of it, since mutations are discarded on rule exit. fn reset(&mut self) { *self = SwiftContext::default(); } @@ -380,17 +378,13 @@ fn translation_rules() -> Vec> { let mut result = Vec::new(); if let Some(val) = val { // Stored property with observers: the initializer is not part - // of the binding, so translate it in a reset scope. - let val = ctx.scoped(|ctx| { - ctx.reset(); - ctx.translate(val) - })?; + // of the binding, so translate it with a reset context. result.push(tree!( (variable_declaration modifier: {ctx.outer_modifiers.clone()} pattern: (identifier #{name}) type: {ty} - value: {val}) + value: {ctx.reset(); val}) )); } for acc in accessors.into_iter() { @@ -750,19 +744,12 @@ fn translation_rules() -> Vec> { trailingClosure: _? @trailing additionalTrailingClosures: _* @additional) => - expr { - // Always translate the callee in non-pattern context. - let callee = ctx.scoped(|ctx| { - ctx.in_pattern = false; - ctx.translate(rawCallee) - })?; - tree!((call_expr - callee: {callee} - argument: {args} - argument: {trailing.map(|trailing| tree!((argument value: {trailing})))} - argument: {additional} - )) - } + // Always translate the callee in non-pattern context. + (call_expr + callee: {ctx.in_pattern = false; rawCallee} + argument: {args} + argument: {trailing.map(|trailing| tree!((argument value: {trailing})))} + argument: {additional}) ), // A call or enum-case pattern argument. rule!(