Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 6 additions & 0 deletions crates/plotnik-lib/src/engine/checkpoint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
//! a checkpoint for each alternative. On failure, it restores the
//! most recent checkpoint and continues.

use super::cursor::SkipPolicy;

/// Checkpoint for backtracking.
#[derive(Clone, Copy, Debug)]
pub struct Checkpoint {
Expand All @@ -17,6 +19,10 @@ pub struct Checkpoint {
pub recursion_depth: u32,
/// Resume point (raw step index).
pub ip: u16,
/// If set, advance cursor before retrying (for Call instruction retry).
/// When a Call navigates and the callee fails, we need to try the next
/// sibling. This policy determines how to advance.
pub skip_policy: Option<SkipPolicy>,
}

/// Stack of checkpoints with O(1) max_frame_ref tracking.
Expand Down
22 changes: 18 additions & 4 deletions crates/plotnik-lib/src/engine/engine_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,10 +60,7 @@ fn build_trivia_types(module: &Module) -> Vec<u16> {
}

/// Resolve entrypoint by name or use the default.
fn resolve_entrypoint(
module: &Module,
name: Option<&str>,
) -> crate::bytecode::Entrypoint {
fn resolve_entrypoint(module: &Module, name: Option<&str>) -> crate::bytecode::Entrypoint {
let entrypoints = module.entrypoints();
let strings = module.strings();

Expand Down Expand Up @@ -367,3 +364,20 @@ fn regression_call_searches_for_field_constraint() {
entry: "Q"
);
}

/// BUG #5: Named definitions didn't search among siblings like inline patterns.
/// When using a named definition like `(GString)` inside a parent pattern, it
/// should search among siblings the same way an inline pattern like `(string)`
/// would. This is the "safe refactoring" promise: extracting a pattern to a
/// named definition should not change matching behavior.
#[test]
fn regression_call_searches_among_siblings() {
snap!(
indoc! {r#"
Num = (number) @n
Q = (program (expression_statement (array (Num) @num)))
"#},
"[1, 2, 3]",
entry: "Q"
);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
---
source: crates/plotnik-lib/src/engine/engine_tests.rs
---
Num = (number) @n
Q = (program (expression_statement (array (Num) @num)))
---
[1, 2, 3]
---
{
"num": {
"n": {
"kind": "number",
"text": "1",
"span": [
1,
2
]
}
}
}
77 changes: 66 additions & 11 deletions crates/plotnik-lib/src/engine/vm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,18 @@ fn continuation_nav(nav: Nav) -> Nav {

use super::checkpoint::{Checkpoint, CheckpointStack};
use super::cursor::{CursorWrapper, SkipPolicy};

/// Derive skip policy from navigation mode without navigating.
/// Used when retrying a Call to determine the policy for the next checkpoint.
fn skip_policy_for_nav(nav: Nav) -> Option<SkipPolicy> {
match nav {
Nav::Down | Nav::Next => Some(SkipPolicy::Any),
Nav::DownSkip | Nav::NextSkip => Some(SkipPolicy::Trivia),
Nav::DownExact | Nav::NextExact => Some(SkipPolicy::Exact),
// Stay doesn't navigate, Up doesn't retry among siblings
_ => None,
}
}
use super::effect::{EffectLog, RuntimeEffect};
use super::error::RuntimeError;
use super::frame::FrameArena;
Expand Down Expand Up @@ -56,6 +68,12 @@ pub struct VM<'t> {
exec_fuel: u32,
recursion_depth: u32,
limits: FuelLimits,

/// When true, the next Call instruction should skip navigation (use Stay).
/// This is set when backtracking to a Call retry checkpoint after advancing
/// the cursor to a new sibling. The Call's navigation was already done, and
/// we're now at the correct position for the callee to match.
skip_call_nav: bool,
}

impl<'t> VM<'t> {
Expand All @@ -71,6 +89,7 @@ impl<'t> VM<'t> {
exec_fuel: limits.exec_fuel,
recursion_depth: 0,
limits,
skip_call_nav: false,
}
}

Expand Down Expand Up @@ -216,6 +235,7 @@ impl<'t> VM<'t> {
frame_index: self.frames.current(),
recursion_depth: self.recursion_depth,
ip: m.successor(i).get(),
skip_policy: None,
});
tracer.trace_checkpoint_created(self.ip);
}
Expand All @@ -229,44 +249,69 @@ impl<'t> VM<'t> {
return Err(RuntimeError::RecursionLimitExceeded(self.recursion_depth));
}

self.navigate_to_field(c.nav, c.node_field, tracer)?;
// Get skip policy: from navigation (normal) or from nav mode (retry)
let skip_policy = if self.skip_call_nav {
// Retry: skip navigation, just check field, derive policy from nav mode
self.skip_call_nav = false;
self.check_field(c.node_field, tracer)?;
skip_policy_for_nav(c.nav)
} else {
// Normal: navigate and capture skip policy
self.navigate_to_field_with_policy(c.nav, c.node_field, tracer)?
};

// Push checkpoint for retry (both normal and retry paths need this)
if let Some(policy) = skip_policy
&& policy != SkipPolicy::Exact
{
self.checkpoints.push(Checkpoint {
descendant_index: self.cursor.descendant_index(),
effect_watermark: self.effects.len(),
frame_index: self.frames.current(),
recursion_depth: self.recursion_depth,
ip: self.ip,
skip_policy: Some(policy),
});
tracer.trace_checkpoint_created(self.ip);
}

// Save tree depth AFTER navigation. On Return, we go up to this depth.
// This allows continue_search advances at the same level to be preserved,
// while still restoring level when the callee descended into children.
let saved_depth = self.cursor.depth();

tracer.trace_call(c.target.get());
self.frames.push(c.next.get(), saved_depth);
self.recursion_depth += 1;
self.ip = c.target.get();
Ok(())
}

fn navigate_to_field<T: Tracer>(
/// Navigate to a field and return the skip policy for retry support.
///
/// Returns `Some(policy)` if navigation was performed, `None` if Stay nav was used.
fn navigate_to_field_with_policy<T: Tracer>(
&mut self,
nav: Nav,
field: Option<std::num::NonZeroU16>,
tracer: &mut T,
) -> Result<(), RuntimeError> {
if nav == Nav::Stay {
return self.check_field(field, tracer);
) -> Result<Option<SkipPolicy>, RuntimeError> {
if nav == Nav::Stay || nav == Nav::StayExact {
self.check_field(field, tracer)?;
return Ok(None);
}

let Some(policy) = self.cursor.navigate(nav) else {
return self.backtrack(tracer);
return Err(self.backtrack(tracer).unwrap_err());
};
tracer.trace_nav(nav, self.cursor.node());

let Some(field_id) = field else {
return Ok(());
return Ok(Some(policy));
};

let cont_nav = continuation_nav(nav);
loop {
if self.cursor.field_id() == Some(field_id) {
tracer.trace_field_success(field_id);
return Ok(());
return Ok(Some(policy));
}
tracer.trace_field_failure(self.cursor.node());
self.advance_or_backtrack(policy, cont_nav, tracer)?;
Expand Down Expand Up @@ -327,6 +372,16 @@ impl<'t> VM<'t> {
self.effects.truncate(cp.effect_watermark);
self.frames.restore(cp.frame_index);
self.recursion_depth = cp.recursion_depth;

// Call retry: advance cursor to next sibling before re-executing
if let Some(policy) = cp.skip_policy {
if !self.cursor.continue_search(policy) {
return self.backtrack(tracer);
}
tracer.trace_nav(continuation_nav(Nav::Down), self.cursor.node());
self.skip_call_nav = true;
}

self.ip = cp.ip;
Err(RuntimeError::Backtracked)
}
Expand Down