Skip to content

Commit 0217465

Browse files
committed
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).
1 parent 72ab57e commit 0217465

3 files changed

Lines changed: 120 additions & 53 deletions

File tree

shared/yeast/src/build.rs

Lines changed: 58 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -161,52 +161,74 @@ impl<'a, C> BuildCtx<'a, C> {
161161
}
162162

163163
impl<C: Clone> BuildCtx<'_, C> {
164-
/// Recursively translate a node via the framework's rule machinery.
165-
/// In a OneShot phase, applies OneShot rules to the given node and
166-
/// returns the resulting node ids. In a Repeating phase, errors
167-
/// (translation is not meaningful when input and output share a
168-
/// schema).
164+
/// Recursively translate every id in the given iterable via the
165+
/// framework's rule machinery. In a OneShot phase, applies OneShot
166+
/// rules to each id and returns the accumulated resulting node ids
167+
/// in order. In a Repeating phase, errors (translation is not
168+
/// meaningful when input and output share a schema).
169+
///
170+
/// The single-`Id` case works too, because `Id: IntoIterator<Item
171+
/// = Id>` as a singleton iterator — so `ctx.translate(some_id)?`
172+
/// returns a `Vec<Id>` containing whatever `some_id` translated to.
169173
///
170174
/// Errors if this `BuildCtx` was constructed by hand (without a
171175
/// translator handle) — for example, in unit tests that don't go
172176
/// through the rule driver.
173-
pub fn translate<I: Into<Id>>(&mut self, id: I) -> Result<Vec<Id>, String> {
174-
let id = id.into();
175-
match &self.translator {
176-
Some(t) => t.translate(self.ast, self.user_ctx, id),
177-
None => Err("translate() called on a BuildCtx without a translator handle".into()),
177+
pub fn translate<I: Into<Id>>(
178+
&mut self,
179+
ids: impl IntoIterator<Item = I>,
180+
) -> Result<Vec<Id>, String> {
181+
let translator = self
182+
.translator
183+
.as_ref()
184+
.ok_or("translate() called on a BuildCtx without a translator handle")?;
185+
let mut out = Vec::new();
186+
for id in ids {
187+
let translated = translator.translate(self.ast, self.user_ctx, id.into())?;
188+
out.extend(translated);
178189
}
190+
Ok(out)
179191
}
180192

181-
/// Translate every node in an iterator with a **fresh** user context
182-
/// (reset to `C::default()`), restoring the previous context afterwards.
193+
/// Run `f` with a temporary child [`BuildCtx`] whose `user_ctx` is
194+
/// a fresh clone of the current one, sharing everything else
195+
/// (`ast`, `captures`, `fresh`, `source_range`, `translator`) by
196+
/// re-borrow. Any mutations `f` makes to the child's `user_ctx`
197+
/// are discarded when it returns — no restore needed, because the
198+
/// mutations only ever happened on a local clone.
183199
///
184-
/// Use when descending into a subtree — a body, expression, or statement
185-
/// list — that must not inherit any of the surrounding translation
186-
/// context (for example an enclosing binding modifier). Accepts optional
187-
/// (`Option<Id>`) and repeated (`Vec<Id>`) captures (both `IntoIterator`);
188-
/// for a single `Id`, wrap it in `std::iter::once(id)`.
189-
pub fn translate_reset<I: Into<Id>>(
190-
&mut self,
191-
ids: impl IntoIterator<Item = I>,
192-
) -> Result<Vec<Id>, String>
200+
/// Use for the rare rule that needs to translate a subtree under a
201+
/// modified context *and then continue using its own (unmodified)
202+
/// context afterwards*. For rules where the modified translation
203+
/// is the last use of `ctx`, mutate `ctx` in place — the
204+
/// framework's rule-boundary save/restore cleans up on rule exit.
205+
///
206+
/// Example: an outer rule that translates one child subtree with a
207+
/// reset context, then continues with the outer context intact:
208+
///
209+
/// ```ignore
210+
/// let val = ctx.scoped(|ctx| {
211+
/// ctx.reset();
212+
/// ctx.translate(val)
213+
/// })?;
214+
/// // `ctx` here is untouched by the reset inside the closure.
215+
/// let other = ctx.translate(other_id)?;
216+
/// ```
217+
pub fn scoped<F, R>(&mut self, f: F) -> R
193218
where
194-
C: Default,
219+
F: for<'b> FnOnce(&mut BuildCtx<'b, C>) -> R,
195220
{
196-
let saved = std::mem::take(&mut *self.user_ctx);
197-
let mut out = Vec::new();
198-
let mut result = Ok(());
199-
for id in ids {
200-
match self.translate(id) {
201-
Ok(v) => out.extend(v),
202-
Err(e) => {
203-
result = Err(e);
204-
break;
205-
}
206-
}
207-
}
208-
*self.user_ctx = saved;
209-
result.map(|()| out)
221+
let mut child_user_ctx = self.user_ctx.clone();
222+
let mut child = BuildCtx {
223+
ast: &mut *self.ast,
224+
captures: self.captures,
225+
fresh: self.fresh,
226+
source_range: self.source_range,
227+
user_ctx: &mut child_user_ctx,
228+
translator: self.translator,
229+
};
230+
f(&mut child)
231+
// child_user_ctx dropped; the outer `self` is unaffected.
210232
}
211233
}
212234

shared/yeast/src/lib.rs

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -741,6 +741,16 @@ pub struct TranslatorHandle<'a, C> {
741741
inner: TranslatorImpl<'a, C>,
742742
}
743743

744+
// Manual `Copy` / `Clone` so `TranslatorHandle<'_, C>: Copy` holds
745+
// regardless of whether `C: Copy`. `TranslatorImpl` contains only
746+
// shared references, which are `Copy` unconditionally.
747+
impl<C> Copy for TranslatorHandle<'_, C> {}
748+
impl<C> Clone for TranslatorHandle<'_, C> {
749+
fn clone(&self) -> Self {
750+
*self
751+
}
752+
}
753+
744754
/// Internal phase-specific translation state. Kept private — callers
745755
/// interact with [`TranslatorHandle`] only.
746756
enum TranslatorImpl<'a, C> {
@@ -761,6 +771,16 @@ enum TranslatorImpl<'a, C> {
761771
Repeating,
762772
}
763773

774+
// Manual `Copy` / `Clone` so `TranslatorImpl<'_, C>: Copy` holds
775+
// regardless of whether `C: Copy`. All variants hold only shared
776+
// references and small `Copy` scalars.
777+
impl<C> Copy for TranslatorImpl<'_, C> {}
778+
impl<C> Clone for TranslatorImpl<'_, C> {
779+
fn clone(&self) -> Self {
780+
*self
781+
}
782+
}
783+
764784
impl<'a, C: Clone> TranslatorHandle<'a, C> {
765785
/// Recursively apply OneShot rules to `id` and return the resulting
766786
/// node ids. Errors in a Repeating phase (where translation is not

unified/extractor/src/languages/swift/swift.rs

Lines changed: 42 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -45,12 +45,29 @@ impl SwiftContext {
4545
///
4646
/// True exactly when an enclosing binding has published its modifier into
4747
/// `outer_modifiers`. This is reliable because non-binding subtrees
48-
/// (bodies, initializer values, ...) are translated with a reset context
49-
/// (see `BuildCtx::translate_reset`), so a bare identifier only sees a
48+
/// (bodies, initializer values, ...) are translated after resetting the
49+
/// context (see `reset`), so a bare identifier only sees a
5050
/// non-empty `outer_modifiers` when it really is a binding.
5151
fn in_binding_pattern(&self) -> bool {
5252
!self.outer_modifiers.is_empty()
5353
}
54+
55+
/// Clear the context fields that must not propagate into an
56+
/// expression / statement / body subtree.
57+
///
58+
/// Mirrors `Default::default()` for `SwiftContext` today, but is a
59+
/// named method so future context fields can opt in or out of
60+
/// clearing here per-field.
61+
///
62+
/// Called before recursively translating a body / initializer
63+
/// slot. Most rules mutate `ctx` in place — the framework's
64+
/// rule-boundary snapshot/restore cleans up on exit. Rules that
65+
/// need the outer context intact *after* the reset-and-translate
66+
/// (see e.g. the `property_binding` willSet/didSet rule) wrap the
67+
/// mutation in `ctx.scoped(...)` instead.
68+
fn reset(&mut self) {
69+
*self = SwiftContext::default();
70+
}
5471
}
5572

5673
/// Build a freshly-created `chained_declaration` modifier node if
@@ -239,7 +256,7 @@ fn translation_rules() -> Vec<Rule<SwiftContext>> {
239256
name: (identifier #{name})
240257
type: {ty}
241258
accessor_kind: (accessor_kind "get")
242-
body: (block stmt: {ctx.translate_reset(body)?}))
259+
body: (block stmt: {ctx.reset(); ctx.translate(body)?}))
243260
),
244261
// Stored property with willSet/didSet observers (initializer
245262
// optional) → a `variable_declaration` followed by one
@@ -260,12 +277,20 @@ fn translation_rules() -> Vec<Rule<SwiftContext>> {
260277
observers: (willset_didset_block willset: _? @@ws didset: _? @@ds))
261278
=>
262279
{{
263-
// The initializer value must not inherit the binding context
264-
// (it may contain patterns, e.g. a switch expression), so
265-
// translate it with a reset context. The observers keep the
266-
// context: each willSet/didSet accessor emits the binding
267-
// modifier and resets its own body.
268-
let val = ctx.translate_reset(val)?;
280+
// The initializer value must not inherit the binding
281+
// context (it may contain patterns, e.g. a switch
282+
// expression), so translate it inside a `ctx.scoped`
283+
// block — the block receives a temporary `ctx` whose
284+
// `user_ctx` is a clone; mutations to it are discarded
285+
// when the block returns, so the outer `ctx` is intact
286+
// for the observer loop below. The observers keep the
287+
// outer context: each willSet/didSet accessor emits
288+
// the binding modifier and, in turn, resets the
289+
// context for its own body.
290+
let val = ctx.scoped(|ctx| {
291+
ctx.reset();
292+
ctx.translate(val)
293+
})?;
269294

270295
let var_decl = tree!(
271296
(variable_declaration
@@ -295,7 +320,7 @@ fn translation_rules() -> Vec<Rule<SwiftContext>> {
295320
// The enclosing `property_declaration` leads `ctx.outer_modifiers`
296321
// with the `let`/`var` binding modifier, so the auto-translated name
297322
// pattern (the LHS) becomes a binding, while the initializer value is
298-
// translated with a reset context (see `translate_reset`).
323+
// translated with a reset context (see `SwiftContext::reset`).
299324
rule!(
300325
(property_binding
301326
name: @pattern
@@ -307,7 +332,7 @@ fn translation_rules() -> Vec<Rule<SwiftContext>> {
307332
modifier: {chained_modifier(&mut ctx)}
308333
pattern: {pattern}
309334
type: {ty}
310-
value: {ctx.translate_reset(val)?}) // reset context: the initializer must not see the binding
335+
value: {ctx.reset(); ctx.translate(val)?})
311336
),
312337
// property_declaration: flatten declarators (each may translate
313338
// to multiple nodes — variable_declaration and/or
@@ -1118,7 +1143,7 @@ fn translation_rules() -> Vec<Rule<SwiftContext>> {
11181143
name: {ctx.property_name.ok_or("computed_getter outside property_binding context")?}
11191144
type: {ctx.property_type}
11201145
accessor_kind: (accessor_kind "get")
1121-
body: (block stmt: {ctx.translate_reset(body)?}))
1146+
body: (block stmt: {ctx.reset(); ctx.translate(body)?}))
11221147
),
11231148
// Computed setter with explicit parameter name.
11241149
rule!(
@@ -1131,7 +1156,7 @@ fn translation_rules() -> Vec<Rule<SwiftContext>> {
11311156
type: {ctx.property_type}
11321157
accessor_kind: (accessor_kind "set")
11331158
parameter: (parameter pattern: (name_pattern identifier: (identifier #{param})))
1134-
body: (block stmt: {ctx.translate_reset(body)?}))
1159+
body: (block stmt: {ctx.reset(); ctx.translate(body)?}))
11351160
),
11361161
// Computed setter without explicit parameter name; body optional.
11371162
rule!(
@@ -1143,7 +1168,7 @@ fn translation_rules() -> Vec<Rule<SwiftContext>> {
11431168
name: {ctx.property_name.ok_or("computed_setter outside property_binding context")?}
11441169
type: {ctx.property_type}
11451170
accessor_kind: (accessor_kind "set")
1146-
body: (block stmt: {ctx.translate_reset(body)?}))
1171+
body: (block stmt: {ctx.reset(); ctx.translate(body)?}))
11471172
),
11481173
// Computed modify → accessor_declaration
11491174
rule!(
@@ -1155,7 +1180,7 @@ fn translation_rules() -> Vec<Rule<SwiftContext>> {
11551180
name: {ctx.property_name.ok_or("computed_modify outside property_binding context")?}
11561181
type: {ctx.property_type}
11571182
accessor_kind: (accessor_kind "modify")
1158-
body: (block stmt: {ctx.translate_reset(body)?}))
1183+
body: (block stmt: {ctx.reset(); ctx.translate(body)?}))
11591184
),
11601185
// willset/didset block — spread to children (only reachable as a
11611186
// fallback; the outer property_binding manual rule normally
@@ -1173,7 +1198,7 @@ fn translation_rules() -> Vec<Rule<SwiftContext>> {
11731198
modifier: {chained_modifier(&mut ctx)}
11741199
name: {ctx.property_name.ok_or("willset_clause outside property_binding context")?}
11751200
accessor_kind: (accessor_kind "willSet")
1176-
body: (block stmt: {ctx.translate_reset(body)?}))
1201+
body: (block stmt: {ctx.reset(); ctx.translate(body)?}))
11771202
),
11781203
// didset clause → accessor_declaration (body optional).
11791204
rule!(
@@ -1184,7 +1209,7 @@ fn translation_rules() -> Vec<Rule<SwiftContext>> {
11841209
modifier: {chained_modifier(&mut ctx)}
11851210
name: {ctx.property_name.ok_or("didset_clause outside property_binding context")?}
11861211
accessor_kind: (accessor_kind "didSet")
1187-
body: (block stmt: {ctx.translate_reset(body)?}))
1212+
body: (block stmt: {ctx.reset(); ctx.translate(body)?}))
11881213
),
11891214
// Preprocessor conditionals — unsupported
11901215
rule!((diagnostic) => (unsupported_node)),

0 commit comments

Comments
 (0)