yeast: Make rule guard context read-only

Top-level rule guards currently get a mutable copy of the `ctx` user
context. If the guard succeeds, this copy is used to replace the
existing context and the rule body is executed.

However, this design is somewhat problematic: first of all, it's
questionable to do such mutation in the first place, and secondly if we
ever want to embed guards deeper inside of a rule, then for consistency
we would expect these to be mutable as well. This would entail a lot of
unnecessary context cloning and bookkeeping for a "feature" that will
likely never be used.

Thus, I opted to simply make the access to the user context read-only
inside of the guard body (as in: we only pass it an immutable
reference).

Author: tausbn

shared/yeast-macros/src/lib.rs

@@ -158,10 +158,9 @@ pub fn trees(input: TokenStream) -> TokenStream {
 /// evaluated after the query matches and before any `@` captures are
 /// translated. Returning false makes the driver try the next rule. All
 /// captures are therefore raw in the guard; `@` versus `@@` controls only
-/// whether the capture is translated for the transform. `ctx` provides
-/// mutable user-context access, and `ast` provides read-only AST access.
-/// Mutations to `ctx` are visible to the transform when the guard succeeds.
-/// Omitting the guard is equivalent to writing `where true`.
+/// whether the capture is translated for the transform. Both `ctx` and `ast`
+/// provide read-only access. Omitting the guard is equivalent to writing
+/// `where true`.
 ///
 /// `tree!` and `trees!` use the rule transform's `ctx` binding inside `{...}`.
 #[proc_macro]

shared/yeast-macros/src/lower.rs

@@ -59,7 +59,7 @@ impl Rule {
                 let __query = #query;
                 yeast::Rule::guarded(
                     __query,
-                    Box::new(|__ast: &yeast::Ast, __captures: &yeast::captures::Captures, __user_ctx: &mut _| {
+                    Box::new(|__ast: &yeast::Ast, __captures: &yeast::captures::Captures, __user_ctx: &_| {
                         #(#guard_bindings)*
                         let ast = __ast;
                         let #ctx = __user_ctx;

shared/yeast/doc/yeast.md

@@ -513,22 +513,17 @@ rule, while a capture marked `@@` would remain raw in the transform as well.
 In this example `label` and `rest` are empty whenever the guard succeeds, so
 there is nothing to translate for those captures.
 
-Guards receive the mutable user context as `ctx` and the raw AST as `ast`.
-The framework clones the user context before evaluating each guard. If the
-guard succeeds, its context mutations are visible to the rule transform and
-recursive translation; if it fails, the clone is discarded before the next
-rule is tried:
+Guards receive the user context as read-only `ctx` and the raw AST as `ast`.
+The transform receives a private clone of the context after the guard succeeds:
 
 ```rust
 rule!(
     (tupleExpr elements: _* @elements)
-    where {
-        ctx.in_pattern = true;
-        elements
+    where ctx.in_pattern
+        && elements
             .first()
             .and_then(|element| ast.get_node(*element))
             .is_some()
-    }
     =>
     (tuple_pattern element: {elements})
 )

shared/yeast/src/lib.rs

@@ -1179,10 +1179,9 @@ pub type Transform<C = ()> = Box<
 
 /// Predicate evaluated after a rule's query matches and before its transform
 /// runs. Returning `false` makes the rule behave as if its query did not
-/// match, so the driver continues to the next rule. The user context is a
-/// private clone for this rule attempt; mutations are retained for the
-/// transform when the guard succeeds and discarded when it fails.
-pub type Guard<C = ()> = Box<dyn Fn(&Ast, &Captures, &mut C) -> Result<bool, String> + Send + Sync>;
+/// match, so the driver continues to the next rule. Guards have read-only
+/// access to the shared user context.
+pub type Guard<C = ()> = Box<dyn Fn(&Ast, &Captures, &C) -> Result<bool, String> + Send + Sync>;
 
 pub struct Rule<C = ()> {
     query: QueryNode,
@@ -1202,8 +1201,8 @@ impl<C> Rule<C> {
         }
     }
 
-    /// Construct a guarded rule. The guard sees raw captures and shares its
-    /// private user-context clone with the transform when it succeeds.
+    /// Construct a guarded rule. The guard sees raw captures and has read-only
+    /// access to the user context.
     pub fn guarded(query: QueryNode, guard: Guard<C>, transform: Transform<C>) -> Self {
         Self {
             query,
@@ -1230,12 +1229,7 @@ impl<C> Rule<C> {
 
     /// Evaluate this rule's guard against a successful query match. An
     /// unguarded rule always succeeds.
-    fn guard_matches(
-        &self,
-        ast: &Ast,
-        captures: &Captures,
-        user_ctx: &mut C,
-    ) -> Result<bool, String> {
+    fn guard_matches(&self, ast: &Ast, captures: &Captures, user_ctx: &C) -> Result<bool, String> {
         if let Some(guard) = &self.guard {
             guard(ast, captures, user_ctx)
         } else {
@@ -1327,15 +1321,14 @@ fn apply_rules_inner<C: Clone>(
             continue;
         };
 
-        // Clone only after the query matches, but before the guard runs.
-        // Guard mutations are visible to the transform and transitively-
-        // translated captures when the guard succeeds; a failed guard drops
-        // the clone before trying the next rule.
-        let mut local = user_ctx.clone();
-        if !rule.guard_matches(ast, &captures, &mut local)? {
+        if !rule.guard_matches(ast, &captures, user_ctx)? {
             continue;
         }
 
+        // The transform gets a private context clone. Its mutations are visible
+        // to transitively translated captures, but not to sibling rule attempts.
+        let mut local = user_ctx.clone();
+
         // Build the translator handle the transform will use to recursively
         // translate captured input nodes when they are consumed.
         let translator = TranslatorHandle {

shared/yeast/tests/test.rs

@@ -933,20 +933,16 @@ fn test_desugar_for_loop() {
 #[derive(Clone, Default)]
 struct GuardTestContext {
     enabled: bool,
-    selected: bool,
 }
 
 fn guarded_integer_rules() -> Vec<Rule<GuardTestContext>> {
     vec![
         yeast::rule!(
             (integer) @@raw
-            where {
-                ctx.selected = ctx.enabled && ast.source_text(raw) == "1";
-                ctx.selected
-            }
+            where ctx.enabled && ast.source_text(raw) == "1"
             =>
             identifier {
-                assert!(ctx.selected);
+                assert!(ctx.enabled);
                 tree!((identifier "guarded"))
             }
         ),
@@ -966,10 +962,7 @@ fn test_rule_guard_reads_raw_capture_and_user_context() {
             with_passthrough_rules(guarded_integer_rules()),
         )];
         let runner = Runner::with_schema(lang, &schema, &phases);
-        let mut user_ctx = GuardTestContext {
-            enabled,
-            selected: false,
-        };
+        let mut user_ctx = GuardTestContext { enabled };
         let ast = runner.run_with_ctx(input, &mut user_ctx).unwrap();
         dump_ast(&ast, ast.get_root(), input)
     }
@@ -1191,7 +1184,7 @@ fn test_guard_runs_before_capture_translation() {
 }
 
 #[test]
-fn test_guard_context_mutation_is_visible_to_transform() {
+fn test_guard_context_is_visible_to_transform() {
     let lang: tree_sitter::Language = tree_sitter_ruby::LANGUAGE.into();
     let schema =
         yeast::node_types_yaml::schema_from_yaml_with_language(OUTPUT_SCHEMA_YAML, &lang).unwrap();
@@ -1203,20 +1196,17 @@ fn test_guard_context_mutation_is_visible_to_transform() {
         ),
         yeast::rule!(
             (integer)
-            where {
-                ctx.selected = true;
-                true
-            }
+            where ctx.enabled
             =>
             identifier {
-                assert!(ctx.selected);
+                assert!(ctx.enabled);
                 tree!((identifier "guarded"))
             }
         ),
     ];
     let phases = vec![Phase::new("translate", rules)];
     let runner = Runner::with_schema(lang, &schema, &phases);
-    let mut user_ctx = GuardTestContext::default();
+    let mut user_ctx = GuardTestContext { enabled: true };
 
     let input = "1";
     let ast = runner.run_with_ctx(input, &mut user_ctx).unwrap();