Skip to content

Commit 1c035f5

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

3 files changed

Lines changed: 27 additions & 23 deletions

File tree

shared/yeast/src/build.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -200,8 +200,9 @@ impl<C: Clone> BuildCtx<'_, C> {
200200
/// Use for the rare rule that needs to translate a subtree under a
201201
/// modified context *and then continue using its own (unmodified)
202202
/// 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.
203+
/// is the last use of `ctx`, mutate `ctx` in place — the framework
204+
/// invokes each rule with a private clone of the user context, so
205+
/// mutations are discarded on rule exit anyway.
205206
///
206207
/// Example: an outer rule that translates one child subtree with a
207208
/// reset context, then continues with the outer context intact:

shared/yeast/src/lib.rs

Lines changed: 19 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -999,18 +999,21 @@ fn apply_repeating_rules_inner<C: Clone>(
999999
if Some(rule_ptr) == skip_rule {
10001000
continue;
10011001
}
1002-
// Snapshot the user context before invoking the rule so that any
1003-
// mutations the rule makes are visible during recursive translation
1004-
// of its result, but not leaked to the parent's siblings.
1005-
let snapshot = user_ctx.clone();
1002+
// Give each rule attempt a private clone of the user context.
1003+
// Any mutations the rule makes are visible to its transform and
1004+
// to the recursive translation of its result, but never leak
1005+
// back to the parent — the clone is simply dropped when we
1006+
// return. This is also `?`-safe: an error return drops `local`
1007+
// without touching the caller's `user_ctx`.
1008+
let mut local = user_ctx.clone();
10061009
// Repeating rules don't need a real translator: their captures
10071010
// aren't auto-translated (Repeating preserves the input schema),
10081011
// and `ctx.translate(id)` errors if invoked from a Repeating
10091012
// transform.
10101013
let translator = TranslatorHandle {
10111014
inner: TranslatorImpl::Repeating,
10121015
};
1013-
let try_result = rule.try_rule(ast, id, fresh, user_ctx, translator)?;
1016+
let try_result = rule.try_rule(ast, id, fresh, &mut local, translator)?;
10141017
if let Some(result_node) = try_result {
10151018
// For non-repeated rules, suppress further application of *this*
10161019
// rule on the result root, so a rule whose output matches its own
@@ -1022,19 +1025,17 @@ fn apply_repeating_rules_inner<C: Clone>(
10221025
results.extend(apply_repeating_rules_inner(
10231026
index,
10241027
ast,
1025-
user_ctx,
1028+
&mut local,
10261029
node,
10271030
fresh,
10281031
rewrite_depth + 1,
10291032
next_skip,
10301033
)?);
10311034
}
1032-
*user_ctx = snapshot;
10331035
return Ok(results);
10341036
}
1035-
// Rule didn't match; restore any speculative changes (none expected
1036-
// since try_rule only mutates on match, but be defensive).
1037-
*user_ctx = snapshot;
1037+
// Rule didn't match; `local` is dropped as we loop to the next
1038+
// rule.
10381039
}
10391040

10401041
// Take the parent's fields by ownership: the recursion will rewrite
@@ -1116,11 +1117,13 @@ fn apply_one_shot_rules_inner<C: Clone>(
11161117

11171118
for rule in index.rules_for_kind(node_kind) {
11181119
if let Some(captures) = rule.try_match(ast, id)? {
1119-
// Snapshot the user context before invoking the rule so that any
1120-
// mutations the rule (or its transitively-translated captures)
1121-
// make are visible during this rule's transform, but not leaked
1122-
// to the parent's siblings.
1123-
let snapshot = user_ctx.clone();
1120+
// Give the rule a private clone of the user context. Any
1121+
// mutations the rule (or its transitively-translated
1122+
// captures) make are visible during this rule's transform,
1123+
// but never leak back — the clone is dropped when we
1124+
// return. `?`-safe: an error return drops `local` without
1125+
// touching the caller's `user_ctx`.
1126+
let mut local = user_ctx.clone();
11241127
// Build the translator handle the transform will use to
11251128
// recursively translate captures (or, for macro-generated
11261129
// rules, the auto-translate prefix uses it to translate every
@@ -1133,8 +1136,7 @@ fn apply_one_shot_rules_inner<C: Clone>(
11331136
matched_root: id,
11341137
},
11351138
};
1136-
let result = rule.run_transform(ast, captures, id, fresh, user_ctx, translator)?;
1137-
*user_ctx = snapshot;
1139+
let result = rule.run_transform(ast, captures, id, fresh, &mut local, translator)?;
11381140
return Ok(result);
11391141
}
11401142
}

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

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -60,10 +60,11 @@ impl SwiftContext {
6060
/// clearing here per-field.
6161
///
6262
/// 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
63+
/// slot. Most rules mutate `ctx` in place — the framework invokes
64+
/// each rule with a private clone of the user context, so
65+
/// mutations are discarded on rule exit anyway. Rules that need
66+
/// the outer context intact *after* the reset-and-translate (see
67+
/// e.g. the `property_binding` willSet/didSet rule) wrap the
6768
/// mutation in `ctx.scoped(...)` instead.
6869
fn reset(&mut self) {
6970
*self = SwiftContext::default();

0 commit comments

Comments
 (0)