From 760c2723896a69f0163fc27eaadfa97de715a909 Mon Sep 17 00:00:00 2001 From: Sergei Zharinov Date: Wed, 7 Jan 2026 12:07:10 -0300 Subject: [PATCH] refactor: Add semantic builder API to `CaptureEffects` --- crates/plotnik-lib/src/compile/capture.rs | 74 +++++++++++++++++++ .../plotnik-lib/src/compile/capture_tests.rs | 61 +++++++++++++++ crates/plotnik-lib/src/compile/expressions.rs | 13 +--- crates/plotnik-lib/src/compile/mod.rs | 2 + crates/plotnik-lib/src/compile/quantifier.rs | 14 +--- crates/plotnik-lib/src/compile/scope.rs | 18 ++--- crates/plotnik-lib/src/compile/sequences.rs | 18 +---- 7 files changed, 151 insertions(+), 49 deletions(-) create mode 100644 crates/plotnik-lib/src/compile/capture_tests.rs diff --git a/crates/plotnik-lib/src/compile/capture.rs b/crates/plotnik-lib/src/compile/capture.rs index 45fd34c3..35dbf1ed 100644 --- a/crates/plotnik-lib/src/compile/capture.rs +++ b/crates/plotnik-lib/src/compile/capture.rs @@ -36,6 +36,80 @@ pub struct CaptureEffects { pub post: Vec, } +impl CaptureEffects { + /// Create with explicit pre and post effects. + pub fn new(pre: Vec, post: Vec) -> Self { + Self { pre, post } + } + + /// Create with only pre effects. + pub fn new_pre(pre: Vec) -> Self { + Self { pre, post: vec![] } + } + + /// Create with only post effects. + pub fn new_post(post: Vec) -> Self { + Self { pre: vec![], post } + } + + /// Add an inner scope (opens after existing scopes, closes before them). + /// + /// Use for: Obj/EndObj, Enum/EndEnum, Arr/EndArr, SuppressBegin/SuppressEnd + /// + /// Given existing `pre=[A_Open]`, `post=[A_Close]`, adding inner scope B: + /// - Result: `pre=[A_Open, B_Open]`, `post=[B_Close, A_Close]` + /// - Execution: A opens -> B opens -> match -> B closes -> A closes + pub fn nest_scope(mut self, open: EffectIR, close: EffectIR) -> Self { + assert!( + matches!( + open.opcode, + EffectOpcode::Obj + | EffectOpcode::Enum + | EffectOpcode::Arr + | EffectOpcode::SuppressBegin + ), + "nest_scope expects scope-opening effect, got {:?}", + open.opcode + ); + assert!( + matches!( + close.opcode, + EffectOpcode::EndObj + | EffectOpcode::EndEnum + | EffectOpcode::EndArr + | EffectOpcode::SuppressEnd + ), + "nest_scope expects scope-closing effect, got {:?}", + close.opcode + ); + self.pre.push(open); + self.post.insert(0, close); + self + } + + /// Add pre-match value effects (run after all scopes open). + /// + /// Use for: Null+Set injection in untagged alternations + /// + /// Given `pre=[Scope_Open]`, adding value effects: + /// - Result: `pre=[Scope_Open, Value1, Value2]` + pub fn with_pre_values(mut self, effects: Vec) -> Self { + self.pre.extend(effects); + self + } + + /// Add post-match value effects (run before any scope closes). + /// + /// Use for: Node/Text+Set capture effects, Push for arrays + /// + /// Given `post=[Scope_Close]`, adding value effects: + /// - Result: `post=[Value1, Value2, Scope_Close]` + pub fn with_post_values(mut self, effects: Vec) -> Self { + self.post.splice(0..0, effects); + self + } +} + impl Compiler<'_> { /// Build capture effects (Node/Text + Set) based on capture type. pub(super) fn build_capture_effects( diff --git a/crates/plotnik-lib/src/compile/capture_tests.rs b/crates/plotnik-lib/src/compile/capture_tests.rs new file mode 100644 index 00000000..b2e3ac31 --- /dev/null +++ b/crates/plotnik-lib/src/compile/capture_tests.rs @@ -0,0 +1,61 @@ +use super::capture::CaptureEffects; +use crate::bytecode::{EffectIR, EffectOpcode, MemberRef}; + +#[test] +fn nest_scope_preserves_outer_and_nests_inner() { + let outer = CaptureEffects::new(vec![EffectIR::start_obj()], vec![EffectIR::end_obj()]); + + let result = outer.nest_scope(EffectIR::start_enum(), EffectIR::end_enum()); + + assert_eq!(result.pre.len(), 2); + assert_eq!(result.pre[0].opcode, EffectOpcode::Obj); + assert_eq!(result.pre[1].opcode, EffectOpcode::Enum); + + assert_eq!(result.post.len(), 2); + assert_eq!(result.post[0].opcode, EffectOpcode::EndEnum); + assert_eq!(result.post[1].opcode, EffectOpcode::EndObj); +} + +#[test] +fn with_pre_values_appends_after_scope_opens() { + let outer = CaptureEffects::new_pre(vec![EffectIR::start_obj()]); + + let result = outer.with_pre_values(vec![ + EffectIR::null(), + EffectIR::with_member(EffectOpcode::Set, MemberRef::absolute(0)), + ]); + + assert_eq!(result.pre.len(), 3); + assert_eq!(result.pre[0].opcode, EffectOpcode::Obj); + assert_eq!(result.pre[1].opcode, EffectOpcode::Null); + assert_eq!(result.pre[2].opcode, EffectOpcode::Set); +} + +#[test] +fn with_post_values_prepends_before_scope_closes() { + let outer = CaptureEffects::new_post(vec![EffectIR::end_obj()]); + + let result = outer.with_post_values(vec![ + EffectIR::node(), + EffectIR::with_member(EffectOpcode::Set, MemberRef::absolute(0)), + ]); + + assert_eq!(result.post.len(), 3); + assert_eq!(result.post[0].opcode, EffectOpcode::Node); + assert_eq!(result.post[1].opcode, EffectOpcode::Set); + assert_eq!(result.post[2].opcode, EffectOpcode::EndObj); +} + +#[test] +#[should_panic(expected = "nest_scope expects scope-opening effect")] +fn nest_scope_rejects_non_scope_open() { + let outer = CaptureEffects::default(); + outer.nest_scope(EffectIR::node(), EffectIR::end_obj()); +} + +#[test] +#[should_panic(expected = "nest_scope expects scope-closing effect")] +fn nest_scope_rejects_non_scope_close() { + let outer = CaptureEffects::default(); + outer.nest_scope(EffectIR::start_obj(), EffectIR::node()); +} diff --git a/crates/plotnik-lib/src/compile/expressions.rs b/crates/plotnik-lib/src/compile/expressions.rs index c4ad19a6..844f5562 100644 --- a/crates/plotnik-lib/src/compile/expressions.rs +++ b/crates/plotnik-lib/src/compile/expressions.rs @@ -487,17 +487,8 @@ impl Compiler<'_> { } // Scalar: capture effects go directly on the match instruction - let mut combined = capture_effects; - combined.extend(outer_capture.post); - self.compile_expr_inner( - &inner, - exit, - nav_override, - CaptureEffects { - pre: outer_capture.pre, - post: combined, - }, - ) + let combined = outer_capture.with_post_values(capture_effects); + self.compile_expr_inner(&inner, exit, nav_override, combined) } /// Compile a suppressive capture (@_ or @_name). diff --git a/crates/plotnik-lib/src/compile/mod.rs b/crates/plotnik-lib/src/compile/mod.rs index a023322c..5dc8633d 100644 --- a/crates/plotnik-lib/src/compile/mod.rs +++ b/crates/plotnik-lib/src/compile/mod.rs @@ -23,6 +23,8 @@ mod quantifier; mod scope; mod sequences; +#[cfg(test)] +mod capture_tests; #[cfg(test)] mod compile_tests; diff --git a/crates/plotnik-lib/src/compile/quantifier.rs b/crates/plotnik-lib/src/compile/quantifier.rs index 6316afee..6ef3de7a 100644 --- a/crates/plotnik-lib/src/compile/quantifier.rs +++ b/crates/plotnik-lib/src/compile/quantifier.rs @@ -192,11 +192,7 @@ impl Compiler<'_> { // Non-array capture: build capture effects and recurse let capture_effects = self.build_capture_effects(cap, Some(&inner)); - let mut combined = CaptureEffects { - pre: capture.pre.clone(), - post: capture_effects, - }; - combined.post.extend(capture.post); + let combined = capture.clone().with_post_values(capture_effects); return self.compile_skippable_with_exits( &inner, @@ -259,9 +255,8 @@ impl Compiler<'_> { let match_endarr = self.emit_endarr_step(&capture_effects, &outer_capture.post, match_exit); let skip_endarr = self.emit_endarr_step(&capture_effects, &outer_capture.post, skip_exit); - let push_effects = CaptureEffects { - pre: vec![], - post: if self.quantifier_needs_node_for_push(inner) { + let push_effects = + CaptureEffects::new_post(if self.quantifier_needs_node_for_push(inner) { let node_eff = if cap.has_string_annotation() { EffectIR::text() } else { @@ -270,8 +265,7 @@ impl Compiler<'_> { vec![node_eff, EffectIR::push()] } else { vec![EffectIR::push()] - }, - }; + }); let inner_entry = self.compile_star_for_array_with_exits( inner, match_endarr, diff --git a/crates/plotnik-lib/src/compile/scope.rs b/crates/plotnik-lib/src/compile/scope.rs index 6184eda5..fbf78ddf 100644 --- a/crates/plotnik-lib/src/compile/scope.rs +++ b/crates/plotnik-lib/src/compile/scope.rs @@ -127,10 +127,7 @@ impl Compiler<'_> { }; // Compile inner with capture_effects on the match instruction - let inner_capture = CaptureEffects { - pre: outer_capture.pre, - post: capture_effects, - }; + let inner_capture = CaptureEffects::new(outer_capture.pre, capture_effects); return self.compile_expr_inner(inner, actual_exit, nav_override, inner_capture); } @@ -146,10 +143,7 @@ impl Compiler<'_> { // Compile inner WITH capture_effects on the match instruction // Note: pre effects don't propagate through Obj/EndObj scope wrapper - let inner_capture = CaptureEffects { - pre: vec![], - post: capture_effects, - }; + let inner_capture = CaptureEffects::new_post(capture_effects); let inner_entry = self.with_scope(scope_type_id.unwrap(), |this| { this.compile_expr_inner(inner, endobj_step, nav_override, inner_capture) }); @@ -186,9 +180,8 @@ impl Compiler<'_> { .into(), ); - let push_effects = CaptureEffects { - pre: vec![], - post: if self.quantifier_needs_node_for_push(inner) { + let push_effects = + CaptureEffects::new_post(if self.quantifier_needs_node_for_push(inner) { // Use Text if the capture has `:: string` annotation, else Node let node_eff = if use_text_for_elements { EffectIR::text() @@ -198,8 +191,7 @@ impl Compiler<'_> { vec![node_eff, EffectIR::push()] } else { vec![EffectIR::push()] - }, - }; + }); let inner_entry = if let Expr::QuantifiedExpr(quant) = inner { self.compile_quantified_for_array(quant, endarr_step, nav_override, push_effects) } else { diff --git a/crates/plotnik-lib/src/compile/sequences.rs b/crates/plotnik-lib/src/compile/sequences.rs index 739c7af8..26f3f553 100644 --- a/crates/plotnik-lib/src/compile/sequences.rs +++ b/crates/plotnik-lib/src/compile/sequences.rs @@ -259,14 +259,8 @@ impl Compiler<'_> { EffectIR::start_enum() }; - // Build capture effects: Enum as pre, EndEnum + outer as post - let mut post_effects = vec![EffectIR::end_enum()]; - post_effects.extend(capture.post.iter().cloned()); - - let branch_capture = CaptureEffects { - pre: vec![e_effect], - post: post_effects, - }; + // Build capture effects: nest Enum/EndEnum inside outer effects + let branch_capture = capture.clone().nest_scope(e_effect, EffectIR::end_enum()); // Compile body with merged effects - no separate epsilon wrappers needed let body_entry = self.with_scope(payload_type_id, |this| { @@ -300,13 +294,7 @@ impl Compiler<'_> { }; // Merge null injection with outer capture effects - let mut pre = null_effects; - pre.extend(capture.pre.iter().cloned()); - - let branch_capture = CaptureEffects { - pre, - post: capture.post.clone(), - }; + let branch_capture = capture.clone().with_pre_values(null_effects); let branch_entry = self.compile_expr_inner(&body, exit, branch_nav, branch_capture); successors.push(branch_entry);