Skip to content

yeast: Isolate interpolation contexts from each other #22964

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
tausbn wants to merge 8 commits into
base: main
Choose a base branch
from
Draft

yeast: Isolate interpolation contexts from each other #22964

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
2a90757
yeast: Make translation failures panic
tausbn Nov 5, 2026
07877db
yeast: Test panicking translation
tausbn Nov 5, 2026
0af4824
Unified: Use infallible yeast translation
tausbn Nov 5, 2026
b29609e
yeast: Catch translation panics before they get printed
tausbn Nov 6, 2026
e561640
yeast: Isolate interpolation contexts
tausbn Nov 6, 2026
84e8e81
yeast: Test interpolation context isolation
tausbn Nov 6, 2026
81057bd
Unified: Use isolated interpolation contexts
tausbn Nov 6, 2026
3d4f761
yeast: Only clone the user context
tausbn Nov 6, 2026
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
136 changes: 93 additions & 43 deletions shared/yeast-macros/src/lower.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use proc_macro2::{Ident, Span, TokenStream};
use quote::quote;
use proc_macro2::{Ident, Span, TokenStream, TokenTree};
use quote::{quote, ToTokens};
use std::cell::RefCell;
use std::sync::atomic::{AtomicUsize, Ordering};

Expand Down Expand Up @@ -85,18 +85,18 @@ impl Rule {
#(#guard_bindings)*
let ast = __ast;
let #ctx = __user_ctx;
Ok(#guard)
let __guard: bool = #guard;
__guard
}),
Box::new(|__ast: &yeast::Ast, __output: &mut yeast::OutputAst, __captures: yeast::captures::Captures, __source_range: Option<yeast::Range>, __user_ctx: &mut _, __translator: yeast::TranslatorHandle<'_, _>| {
let __skip: &[&str] = &[#(#raw_capture_names),*];
let __translated_captures =
__translator.translate_captures(&__captures, __ast, __output, __user_ctx, __skip)?;
__translator.translate_captures(&__captures, __ast, __output, __user_ctx, __skip);
#(#raw_bindings)*
#(#translated_bindings)*
let mut #ctx = yeast::build::BuildCtx::with_translator(__ast, __output, __source_range, __user_ctx, __translator);
let __result: Vec<yeast::OutputId> = { #replacement };
let __result = #ctx.finish_rule(__result)?;
Ok(__result)
#ctx.finish_rule(__result)
}),
)
}
Expand Down Expand Up @@ -324,12 +324,16 @@ impl Template {
quote! { __nodes.push(#node); }
}
Template::Splice(block) => {
let uses_ctx = block_uses_ctx(block);
let block = rust_block(block);
quote! {
let __value = ::std::iter::IntoIterator::into_iter(#block)
.collect::<Vec<_>>();
ctx.extend_field_ids(__value, &mut __nodes);
}
isolated_interpolation(
uses_ctx,
quote! {
let __value = ::std::iter::IntoIterator::into_iter(#block)
.collect::<Vec<_>>();
ctx.extend_field_ids(__value, &mut __nodes);
},
)
}
Template::Choice(alternatives) => {
let pick = Self::lower_choice(alternatives);
Expand All @@ -355,8 +359,14 @@ impl Template {
match self {
Template::Node(node) => node.lower(context),
Template::Splice(block) => {
let uses_ctx = block_uses_ctx(block);
let block = rust_block(block);
quote! { ::std::convert::Into::<yeast::OutputId>::into(#block) }
isolated_interpolation(
uses_ctx,
quote! {
::std::convert::Into::<yeast::OutputId>::into(#block)
},
)
}
Template::Choice(_) => {
unreachable!("choices are lowered through Template::lower_choice")
Expand Down Expand Up @@ -453,35 +463,38 @@ impl Node {
let node = match content {
Content::Static(lit) => quote! { #ctx.literal(#kind_str, #lit) },
Content::Computed(block) => {
let uses_ctx = block_uses_ctx(block);
let block = rust_block(block);
if let Some(scope) = context.choice_alternative_scope {
let value = fresh_prepared_ident("value");
let source_range = fresh_prepared_ident("source_range");
let expression = isolated_interpolation(
uses_ctx,
quote! {
let __expr = #block;
yeast::MaybeYeastValue::maybe_yeast_value(&__expr).map(
|__value_ref| {
(
yeast::YeastDisplay::yeast_to_string(
__value_ref,
&#ctx,
),
yeast::YeastSourceRange::yeast_source_range(
__value_ref,
&#ctx,
),
)
},
)
},
);
scope
.interpolations
.borrow_mut()
.push(GuardingInterpolation {
value: value.clone(),
source_range: source_range.clone(),
expression: quote! {
{
let __expr = #block;
yeast::MaybeYeastValue::maybe_yeast_value(&__expr).map(
|__value_ref| {
(
yeast::YeastDisplay::yeast_to_string(
__value_ref,
&#ctx,
),
yeast::YeastSourceRange::yeast_source_range(
__value_ref,
&#ctx,
),
)
},
)
}
},
expression,
});
quote! {
#ctx.literal_with_source_range(
Expand All @@ -491,8 +504,9 @@ impl Node {
)
}
} else {
quote! {
{
isolated_interpolation(
uses_ctx,
quote! {
let __expr = #block;
let __value =
yeast::YeastDisplay::yeast_to_string(&__expr, &#ctx);
Expand All @@ -503,8 +517,8 @@ impl Node {
&__value,
__source_range,
)
}
}
},
)
}
}
Content::Inherited => quote! { #ctx.inherited_literal(#kind_str) },
Expand All @@ -524,12 +538,16 @@ impl Node {
for (field_counter, field) in self.fields.iter().enumerate() {
match field {
Field::Location { block } => {
let uses_ctx = block_uses_ctx(block);
let block = rust_block(block);
stmts.push(quote! {
let __value = ::std::iter::IntoIterator::into_iter(#block)
.collect::<Vec<_>>();
#ctx.extend_location_ranges(__value, &mut __location_ranges);
});
stmts.push(isolated_interpolation(
uses_ctx,
quote! {
let __value = ::std::iter::IntoIterator::into_iter(#block)
.collect::<Vec<_>>();
#ctx.extend_location_ranges(__value, &mut __location_ranges);
},
));
}
Field::LocationNone => {}
Field::Named { name, value } => {
Expand All @@ -544,12 +562,19 @@ impl Node {
);
match value {
Template::Splice(block) => {
let uses_ctx = block_uses_ctx(block);
let block = rust_block(block);
let interpolation = isolated_interpolation(
uses_ctx,
quote! {
let __value = ::std::iter::IntoIterator::into_iter(#block)
.collect::<Vec<_>>();
#ctx.extend_field_ids(__value, &mut #temp);
},
);
stmts.push(quote! {
let mut #temp: Vec<yeast::OutputId> = Vec::new();
let __value = ::std::iter::IntoIterator::into_iter(#block)
.collect::<Vec<_>>();
#ctx.extend_field_ids(__value, &mut #temp);
#interpolation
});
field_args.push(quote! {
if !#temp.is_empty() {
Expand Down Expand Up @@ -612,3 +637,28 @@ fn rust_block(block: &syn::Block) -> TokenStream {
let statements = &block.stmts;
quote!({ #(#statements)* })
}

fn isolated_interpolation(uses_ctx: bool, body: TokenStream) -> TokenStream {
if uses_ctx {
quote! {{
let __saved_user_ctx = (*ctx.user_ctx).clone();
let __result = { #body };
*ctx.user_ctx = __saved_user_ctx;
__result
}}
} else {
quote! {{ #body }}
}
}

fn block_uses_ctx(block: &syn::Block) -> bool {
fn uses_ctx(tokens: TokenStream) -> bool {
tokens.into_iter().any(|token| match token {
TokenTree::Ident(ident) => ident == "ctx",
TokenTree::Group(group) => uses_ctx(group.stream()),
Comment thread
tausbn marked this conversation as resolved.
Resolved
TokenTree::Punct(_) | TokenTree::Literal(_) => false,
})
}

uses_ctx(block.to_token_stream())
}
47 changes: 12 additions & 35 deletions shared/yeast/src/build.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ use crate::{
/// When constructed by the framework (via the rule! macro), `BuildCtx` also
/// carries a [`TranslatorHandle`] that the [`translate`] method delegates
/// to. When constructed by hand (e.g. in tests), the translator is `None`
/// and [`translate`] returns an error.
/// and [`translate`] panics.
pub struct BuildCtx<'a, C: 'a = ()> {
ast: &'a Ast,
pub output: &'a mut OutputAst,
Expand All @@ -44,10 +44,6 @@ pub struct BuildCtx<'a, C: 'a = ()> {
/// Optional translator handle, populated when the context is built by
/// the framework's rule driver. None when the context is built by hand.
pub(crate) translator: Option<TranslatorHandle<'a, C>>,
/// First failure encountered while auto-translating an input `Id` during
/// template construction. Templates remain infallible expressions, so
/// [`finish_rule`](Self::finish_rule) returns this error at the rule boundary.
deferred_translation_error: Option<String>,
/// Nodes built directly through this context without an explicit source
/// range in either their content or constructor argument. Recursive
/// translations use their own context and therefore do not contribute to
Expand All @@ -64,7 +60,6 @@ impl<'a, C> BuildCtx<'a, C> {
source_range: None,
user_ctx,
translator: None,
deferred_translation_error: None,
created_nodes_without_source_range: BTreeSet::new(),
}
}
Expand All @@ -82,7 +77,6 @@ impl<'a, C> BuildCtx<'a, C> {
source_range,
user_ctx,
translator: None,
deferred_translation_error: None,
created_nodes_without_source_range: BTreeSet::new(),
}
}
Expand All @@ -102,7 +96,6 @@ impl<'a, C> BuildCtx<'a, C> {
source_range,
user_ctx,
translator: Some(translator),
deferred_translation_error: None,
created_nodes_without_source_range: BTreeSet::new(),
}
}
Expand Down Expand Up @@ -159,18 +152,15 @@ impl<'a, C> BuildCtx<'a, C> {
/// Finish the current rule invocation by applying the matched source range
/// to locally-created result roots.
#[doc(hidden)]
pub fn finish_rule(self, results: Vec<OutputId>) -> Result<Vec<OutputId>, String> {
if let Some(error) = self.deferred_translation_error {
return Err(error);
}
pub fn finish_rule(self, results: Vec<OutputId>) -> Vec<OutputId> {
if let Some(source_range) = self.source_range {
for &id in &results {
if self.created_nodes_without_source_range.contains(&id) {
self.output.extend_source_range(id, source_range);
}
}
}
Ok(results)
results
}

/// Assign an explicit source range to a newly-built result root.
Expand Down Expand Up @@ -300,40 +290,31 @@ impl<C: Clone> BuildCtx<'_, C> {

#[doc(hidden)]
pub fn translate_into(&mut self, id: Id, out: &mut Vec<OutputId>) {
if self.deferred_translation_error.is_some() {
return;
}
match self.translate(id) {
Ok(ids) => out.extend(ids),
Err(error) => self.deferred_translation_error = Some(error),
}
out.extend(self.translate(id));
}

/// Recursively translate every id in the given iterable via the
/// framework's rule machinery and return the accumulated resulting node
/// ids in order.
///
/// The single-`Id` case works too, because `Id: IntoIterator<Item
/// = Id>` is a singleton iterator — so `ctx.translate(some_id)?`
/// = Id>` is a singleton iterator — so `ctx.translate(some_id)`
/// returns a `Vec<OutputId>` containing whatever `some_id` translated to.
///
/// Errors if this `BuildCtx` was constructed by hand (without a
/// Panics 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,
ids: impl IntoIterator<Item = Id>,
) -> Result<Vec<OutputId>, String> {
pub fn translate(&mut self, ids: impl IntoIterator<Item = Id>) -> Vec<OutputId> {
let translator = self
.translator
.as_ref()
.ok_or("translate() called on a BuildCtx without a translator handle")?;
.expect("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.output, self.user_ctx, id)?;
let translated = translator.translate(self.ast, self.output, self.user_ctx, id);
out.extend(translated);
}
Ok(out)
out
}

/// Run `f` with a temporary child [`BuildCtx`] whose `user_ctx` is
Expand All @@ -358,9 +339,9 @@ impl<C: Clone> BuildCtx<'_, C> {
/// 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)?;
/// let other = ctx.translate(other_id);
/// ```
pub fn scoped<F, R>(&mut self, f: F) -> R
where
Expand All @@ -373,15 +354,11 @@ impl<C: Clone> BuildCtx<'_, C> {
source_range: self.source_range,
user_ctx: &mut child_user_ctx,
translator: self.translator,
deferred_translation_error: None,
created_nodes_without_source_range: BTreeSet::new(),
};
let result = f(&mut child);
let created_nodes_without_source_range =
std::mem::take(&mut child.created_nodes_without_source_range);
if self.deferred_translation_error.is_none() {
self.deferred_translation_error = child.deferred_translation_error.take();
}
drop(child);
self.created_nodes_without_source_range
.extend(created_nodes_without_source_range);
Expand Down
10 changes: 5 additions & 5 deletions shared/yeast/src/captures.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,23 +54,23 @@ impl Captures {
self.captures.entry(key).or_default().push(id);
}

pub(crate) fn try_translate_except<E>(
pub(crate) fn translate_except(
&self,
skip: &[&str],
mut f: impl FnMut(Id) -> Result<Vec<OutputId>, E>,
) -> Result<TranslatedCaptures, E> {
mut f: impl FnMut(Id) -> Vec<OutputId>,
) -> TranslatedCaptures {
let mut translated = TranslatedCaptures::new();
for (name, ids) in &self.captures {
if skip.contains(name) {
continue;
}
for &id in ids {
for translated_id in f(id)? {
for translated_id in f(id) {
translated.insert(name, translated_id);
}
}
}
Ok(translated)
translated
}

pub fn merge(&mut self, other: &Captures) {
Expand Down
Loading
Loading