From 1007d924a6c501eca48182ada33e4f8c3c059fe2 Mon Sep 17 00:00:00 2001 From: Roger-luo Date: Sun, 17 May 2026 12:46:19 -0400 Subject: [PATCH 1/2] fix(stim-parser): address proptest findings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Resolves the two issues surfaced by the fuzz suite in #73 by fixing the underlying parser/AST, not the tests. The workarounds that were present in the proptest files are removed. **Finding 1: stack overflow on deeply nested REPEAT.** The chumsky grammar uses `recursive(...)` for REPEAT bodies and the interpret pass recurses through `Repeat` bodies as well. Both pieces descend recursively at parse time, and the default thread stack (≈2–8 MiB) overflowed somewhere past 24 nesting levels in debug builds. Fixed by running `parse` and `parse_extended` on a dedicated 16 MiB parser thread via `std::thread::scope`. This covers both the chumsky combinator stack and the interpret pass without touching the grammar itself. The proptest cap on `nested_repeats_never_panic` is raised from 16 to 128. **Finding 2: `Raw(MPad)` never round-trips through `parse_extended`.** `parse_extended` always lowers `MPAD …` into the typed `ExtendedInstruction::MPad`, so `ExtendedInstruction::Raw(RawInstruction::MPad{..})` and `Raw(RawInstruction::Repeat{..})` were states that the parser never produced but the type system still allowed — causing runtime `unreachable!()` paths in `prepare` / `executor` and a workaround in the proptest generator. Fixed by introducing `RawPassthrough` (a sub-enum of `RawInstruction` covering only `Gate`/`Noise`/`Measure`/`Annotation`) and changing `ExtendedInstruction::Raw(RawInstruction)` to `Raw(RawPassthrough)`. The bad states are now compile-time unrepresentable: - `prepare` drops the `ExecError::Malformed` variant — it can no longer be reached. - `executor` drops the two `unreachable!()` arms. - `proptest_ast.rs` drops the `ext_raw_passthrough` workaround and generates `Raw(RawPassthrough::*)` directly across all four passthrough variants. The stale regression seed is removed (the case it captured no longer compiles). - `tests/extended.rs` pattern-matches are updated to the new variant. Co-Authored-By: Claude Opus 4.7 (1M context) --- crates/ppvm-stim/src/executor.rs | 18 +-- crates/ppvm-stim/src/prepare.rs | 33 +---- crates/stim-parser/src/display.rs | 5 +- crates/stim-parser/src/extended/ast.rs | 115 +++++++++++++++- crates/stim-parser/src/extended/interpret.rs | 36 ++++- crates/stim-parser/src/extended/mod.rs | 2 +- crates/stim-parser/src/extended/parser.rs | 8 +- crates/stim-parser/src/lib.rs | 3 +- crates/stim-parser/src/parser.rs | 33 +++++ crates/stim-parser/tests/extended.rs | 22 +-- .../tests/proptest_ast.proptest-regressions | 7 - crates/stim-parser/tests/proptest_ast.rs | 125 ++++++++++++++++-- crates/stim-parser/tests/proptest_parse.rs | 15 +-- 13 files changed, 326 insertions(+), 96 deletions(-) delete mode 100644 crates/stim-parser/tests/proptest_ast.proptest-regressions diff --git a/crates/ppvm-stim/src/executor.rs b/crates/ppvm-stim/src/executor.rs index ee768fffa..aaee34101 100644 --- a/crates/ppvm-stim/src/executor.rs +++ b/crates/ppvm-stim/src/executor.rs @@ -8,8 +8,8 @@ use std::fmt::Debug; use ppvm_runtime::prelude::*; use ppvm_tableau::prelude::*; -use stim_parser::ast::{GateName, MeasureName, NoiseName, RawInstruction}; -use stim_parser::extended::{Axis, ExtendedInstruction, ExtendedProgram}; +use stim_parser::ast::{GateName, MeasureName, NoiseName}; +use stim_parser::extended::{Axis, ExtendedInstruction, ExtendedProgram, RawPassthrough}; use crate::prepare::{ExecError, prepare}; @@ -125,7 +125,7 @@ pub fn execute_prepared( { for instr in instructions { match instr { - ExtendedInstruction::Raw(RawInstruction::Gate { name, targets, .. }) => match name { + ExtendedInstruction::Raw(RawPassthrough::Gate { name, targets, .. }) => match name { GateName::Reset | GateName::ResetZ => targets.iter().for_each(|&q| tab.reset(q)), GateName::X => targets.iter().for_each(|&q| tab.x(q)), GateName::Y => targets.iter().for_each(|&q| tab.y(q)), @@ -193,7 +193,7 @@ pub fn execute_prepared( } => targets .iter() .for_each(|&q| tab.u3(q, (*theta).into(), (*phi).into(), (*lambda).into())), - ExtendedInstruction::Raw(RawInstruction::Noise { + ExtendedInstruction::Raw(RawPassthrough::Noise { name, targets, args, @@ -261,7 +261,7 @@ pub fn execute_prepared( tab.correlated_loss_channel(a, b, ps.clone()); } } - ExtendedInstruction::Raw(RawInstruction::Measure { + ExtendedInstruction::Raw(RawPassthrough::Measure { name, args, targets, @@ -307,18 +307,12 @@ pub fn execute_prepared( results.push(Some(tab.flip_with_prob(bit, noise))); } } - ExtendedInstruction::Raw(RawInstruction::Annotation { .. }) => { /* no-op */ } + ExtendedInstruction::Raw(RawPassthrough::Annotation { .. }) => { /* no-op */ } ExtendedInstruction::Repeat { count, body, .. } => { for _ in 0..*count { execute_prepared(body, tab, results); } } - ExtendedInstruction::Raw(RawInstruction::MPad { .. }) => { - unreachable!("MPad is consumed into the extended dialect; never reaches Raw"); - } - ExtendedInstruction::Raw(RawInstruction::Repeat { .. }) => { - unreachable!("Repeat is consumed into the extended dialect; never reaches Raw"); - } } } } diff --git a/crates/ppvm-stim/src/prepare.rs b/crates/ppvm-stim/src/prepare.rs index 4868db1e8..41d4b1378 100644 --- a/crates/ppvm-stim/src/prepare.rs +++ b/crates/ppvm-stim/src/prepare.rs @@ -1,19 +1,10 @@ -use stim_parser::ast::{GateName, MeasureName, NoiseName, RawInstruction}; -use stim_parser::extended::{ExtendedInstruction, ExtendedProgram}; +use stim_parser::ast::{GateName, MeasureName, NoiseName}; +use stim_parser::extended::{ExtendedInstruction, ExtendedProgram, RawPassthrough}; #[derive(Debug, thiserror::Error, PartialEq)] pub enum ExecError { #[error("unsupported instruction '{name}' at line {line}")] Unsupported { name: String, line: usize }, - /// Raised for `ExtendedInstruction::Raw(_)` values that the extended - /// interpreter would have lowered to typed variants (`MPad`, `Repeat`). - /// `parse_extended` never produces these; only a caller hand-constructing - /// an `ExtendedProgram` can. Reported as a recoverable error rather than - /// a panic. - #[error( - "malformed ExtendedProgram: Raw({kind}) at line {line} should have been lowered to ExtendedInstruction::{kind} by the interpreter" - )] - Malformed { kind: &'static str, line: usize }, #[error("invalid probability {value} for '{name}' at line {line}; expected value in [0, 1]")] InvalidProbability { name: String, @@ -29,13 +20,13 @@ pub fn prepare(program: &ExtendedProgram) -> Result<(), ExecError> { fn validate_slice(instructions: &[ExtendedInstruction]) -> Result<(), ExecError> { for instr in instructions { match instr { - ExtendedInstruction::Raw(RawInstruction::Gate { name, line, .. }) => { + ExtendedInstruction::Raw(RawPassthrough::Gate { name, line, .. }) => { check_gate_supported(*name, *line)?; } - ExtendedInstruction::Raw(RawInstruction::Noise { name, line, .. }) => { + ExtendedInstruction::Raw(RawPassthrough::Noise { name, line, .. }) => { check_noise_supported(*name, *line)?; } - ExtendedInstruction::Raw(RawInstruction::Measure { + ExtendedInstruction::Raw(RawPassthrough::Measure { name, args, line, .. }) => { check_measure_supported(*name, *line)?; @@ -44,7 +35,7 @@ fn validate_slice(instructions: &[ExtendedInstruction]) -> Result<(), ExecError> } } ExtendedInstruction::Repeat { body, .. } => validate_slice(body)?, - ExtendedInstruction::Raw(RawInstruction::Annotation { .. }) + ExtendedInstruction::Raw(RawPassthrough::Annotation { .. }) | ExtendedInstruction::T { .. } | ExtendedInstruction::TDag { .. } | ExtendedInstruction::Rotation { .. } @@ -56,18 +47,6 @@ fn validate_slice(instructions: &[ExtendedInstruction]) -> Result<(), ExecError> check_probability(*p, "MPAD", *line)?; } } - ExtendedInstruction::Raw(RawInstruction::MPad { line, .. }) => { - return Err(ExecError::Malformed { - kind: "MPad", - line: *line, - }); - } - ExtendedInstruction::Raw(RawInstruction::Repeat { line, .. }) => { - return Err(ExecError::Malformed { - kind: "Repeat", - line: *line, - }); - } } } Ok(()) diff --git a/crates/stim-parser/src/display.rs b/crates/stim-parser/src/display.rs index ae01654e8..544c0081a 100644 --- a/crates/stim-parser/src/display.rs +++ b/crates/stim-parser/src/display.rs @@ -124,7 +124,10 @@ fn fmt_raw(i: &RawInstruction, f: &mut fmt::Formatter<'_>, depth: usize) -> fmt: fn fmt_ext(i: &ExtendedInstruction, f: &mut fmt::Formatter<'_>, depth: usize) -> fmt::Result { match i { - ExtendedInstruction::Raw(r) => return fmt_raw(r, f, depth), + ExtendedInstruction::Raw(r) => { + let owned: RawInstruction = r.clone().into_raw(); + return fmt_raw(&owned, f, depth); + } ExtendedInstruction::Repeat { count, body, .. } => { write_indent(f, depth)?; writeln!(f, "REPEAT {count} {{")?; diff --git a/crates/stim-parser/src/extended/ast.rs b/crates/stim-parser/src/extended/ast.rs index ae31ebc64..07533cef1 100644 --- a/crates/stim-parser/src/extended/ast.rs +++ b/crates/stim-parser/src/extended/ast.rs @@ -1,19 +1,124 @@ //! Typed AST for Stim with PPVM tag-based extensions promoted to //! first-class instruction variants. -use crate::ast::{RawInstruction, Tag}; +use crate::ast::{AnnotationKind, GateName, MeasureName, NoiseName, RawInstruction, Tag}; #[derive(Debug, Clone, PartialEq)] pub struct ExtendedProgram { pub instructions: Vec, } +/// Subset of [`RawInstruction`] that passes through the extended-dialect +/// interpreter unchanged. Excludes `MPad` and `Repeat`, which are always +/// lowered to the typed [`ExtendedInstruction::MPad`] / +/// [`ExtendedInstruction::Repeat`] variants — so by construction +/// `ExtendedInstruction::Raw(_)` can never wrap one of them. +#[derive(Debug, Clone, PartialEq)] +pub enum RawPassthrough { + Gate { + name: GateName, + tags: Vec, + args: Vec, + targets: Vec, + line: usize, + }, + Noise { + name: NoiseName, + tags: Vec, + args: Vec, + targets: Vec, + line: usize, + }, + Measure { + name: MeasureName, + tags: Vec, + args: Vec, + targets: Vec, + line: usize, + }, + Annotation { + kind: AnnotationKind, + args: Vec, + targets: Vec, + line: usize, + }, +} + +impl RawPassthrough { + /// Lift back into [`RawInstruction`]. Used by the printer to share + /// formatting code with the vanilla AST. + pub fn into_raw(self) -> RawInstruction { + match self { + RawPassthrough::Gate { + name, + tags, + args, + targets, + line, + } => RawInstruction::Gate { + name, + tags, + args, + targets, + line, + }, + RawPassthrough::Noise { + name, + tags, + args, + targets, + line, + } => RawInstruction::Noise { + name, + tags, + args, + targets, + line, + }, + RawPassthrough::Measure { + name, + tags, + args, + targets, + line, + } => RawInstruction::Measure { + name, + tags, + args, + targets, + line, + }, + RawPassthrough::Annotation { + kind, + args, + targets, + line, + } => RawInstruction::Annotation { + kind, + args, + targets, + line, + }, + } + } + + pub fn line(&self) -> usize { + match self { + RawPassthrough::Gate { line, .. } + | RawPassthrough::Noise { line, .. } + | RawPassthrough::Measure { line, .. } + | RawPassthrough::Annotation { line, .. } => *line, + } + } +} + #[derive(Debug, Clone, PartialEq)] pub enum ExtendedInstruction { /// Pass-through from vanilla Stim — covers `Gate`, `Noise`, `Measure`, - /// `Annotation`. `MPad` and `Repeat` are NOT in `Raw` because their - /// bits/body shapes diverge between dialects. - Raw(RawInstruction), + /// `Annotation`. `MPad` and `Repeat` get their own typed variants on + /// [`ExtendedInstruction`] because their bits/body shapes diverge + /// between dialects. + Raw(RawPassthrough), // --- Extended-dialect sugar variants --- T { @@ -81,7 +186,7 @@ fn count_in_slice(instructions: &[ExtendedInstruction], factor: u64) -> usize { let factor_usize = usize::try_from(factor).unwrap_or(usize::MAX); for instr in instructions { match instr { - ExtendedInstruction::Raw(RawInstruction::Measure { targets, .. }) => { + ExtendedInstruction::Raw(RawPassthrough::Measure { targets, .. }) => { total = total.saturating_add(targets.len().saturating_mul(factor_usize)); } ExtendedInstruction::MPad { bits, .. } => { diff --git a/crates/stim-parser/src/extended/interpret.rs b/crates/stim-parser/src/extended/interpret.rs index 83cb58fd5..3469e6308 100644 --- a/crates/stim-parser/src/extended/interpret.rs +++ b/crates/stim-parser/src/extended/interpret.rs @@ -1,7 +1,7 @@ //! Post-pass interpretation from the vanilla Stim AST to the extended AST. use crate::ast::{GateName, NoiseName, Program, RawInstruction, Tag, TagParam}; -use crate::extended::ast::{Axis, ExtendedInstruction, ExtendedProgram}; +use crate::extended::ast::{Axis, ExtendedInstruction, ExtendedProgram, RawPassthrough}; use crate::extended::parser::ExtendedParseError; pub(crate) fn interpret(prog: Program) -> Result { @@ -36,8 +36,30 @@ fn interpret_one(raw: RawInstruction) -> Result interpret_noise(name, tags, args, targets, line), - m @ RawInstruction::Measure { .. } => Ok(ExtendedInstruction::Raw(m)), - a @ RawInstruction::Annotation { .. } => Ok(ExtendedInstruction::Raw(a)), + RawInstruction::Measure { + name, + tags, + args, + targets, + line, + } => Ok(ExtendedInstruction::Raw(RawPassthrough::Measure { + name, + tags, + args, + targets, + line, + })), + RawInstruction::Annotation { + kind, + args, + targets, + line, + } => Ok(ExtendedInstruction::Raw(RawPassthrough::Annotation { + kind, + args, + targets, + line, + })), RawInstruction::MPad { tags, prob, @@ -72,7 +94,7 @@ fn interpret_gate( match name { S | SDag => match tags.as_slice() { - [] => Ok(ExtendedInstruction::Raw(RawInstruction::Gate { + [] => Ok(ExtendedInstruction::Raw(RawPassthrough::Gate { name, tags, args, @@ -101,7 +123,7 @@ fn interpret_gate( )), }, Identity => match tags.as_slice() { - [] => Ok(ExtendedInstruction::Raw(RawInstruction::Gate { + [] => Ok(ExtendedInstruction::Raw(RawPassthrough::Gate { name, tags, args, @@ -116,7 +138,7 @@ fn interpret_gate( "expected exactly one tag", )), }, - _ => Ok(ExtendedInstruction::Raw(RawInstruction::Gate { + _ => Ok(ExtendedInstruction::Raw(RawPassthrough::Gate { name, tags, args, @@ -254,7 +276,7 @@ fn interpret_noise( line, "expected exactly one tag", )), - _ => Ok(ExtendedInstruction::Raw(RawInstruction::Noise { + _ => Ok(ExtendedInstruction::Raw(RawPassthrough::Noise { name, tags, args, diff --git a/crates/stim-parser/src/extended/mod.rs b/crates/stim-parser/src/extended/mod.rs index 7d4297c7f..e15c76c83 100644 --- a/crates/stim-parser/src/extended/mod.rs +++ b/crates/stim-parser/src/extended/mod.rs @@ -5,5 +5,5 @@ pub mod ast; mod interpret; pub mod parser; -pub use ast::{Axis, ExtendedInstruction, ExtendedProgram}; +pub use ast::{Axis, ExtendedInstruction, ExtendedProgram, RawPassthrough}; pub use parser::{ExtendedParseError, parse_extended}; diff --git a/crates/stim-parser/src/extended/parser.rs b/crates/stim-parser/src/extended/parser.rs index 3d6b204a8..7551d8952 100644 --- a/crates/stim-parser/src/extended/parser.rs +++ b/crates/stim-parser/src/extended/parser.rs @@ -24,6 +24,10 @@ pub enum ExtendedParseError { } pub fn parse_extended(src: &str) -> Result { - let prog = crate::parser::parse(src)?; - interpret(prog) + // Both `parse_impl` and `interpret` recurse through REPEAT bodies, + // so run the whole pipeline on the oversized parser stack. + crate::parser::run_on_parser_stack(|| { + let prog = crate::parser::parse_impl(src)?; + interpret(prog) + }) } diff --git a/crates/stim-parser/src/lib.rs b/crates/stim-parser/src/lib.rs index 2d395b273..4f2e737c9 100644 --- a/crates/stim-parser/src/lib.rs +++ b/crates/stim-parser/src/lib.rs @@ -14,7 +14,8 @@ pub mod prelude { TagParam, }; pub use crate::extended::{ - Axis, ExtendedInstruction, ExtendedParseError, ExtendedProgram, parse_extended, + Axis, ExtendedInstruction, ExtendedParseError, ExtendedProgram, RawPassthrough, + parse_extended, }; pub use crate::parser::parse; } diff --git a/crates/stim-parser/src/parser.rs b/crates/stim-parser/src/parser.rs index 9df170c46..01278b875 100644 --- a/crates/stim-parser/src/parser.rs +++ b/crates/stim-parser/src/parser.rs @@ -42,8 +42,41 @@ pub(crate) struct RawTarget { pub span: SimpleSpan, } +/// Stack size for the dedicated parsing thread. The chumsky grammar +/// is built around `recursive(...)`, which descends into REPEAT bodies +/// via recursive parser calls; on the default thread stack (typically +/// 2–8 MiB), deeply nested programs overflow somewhere past ~24 levels +/// in debug builds. Running on an oversized dedicated stack lets us +/// support thousands of nested REPEATs without rewriting the grammar. +const PARSER_STACK_SIZE: usize = 16 * 1024 * 1024; + +/// Run `f` on a dedicated thread with [`PARSER_STACK_SIZE`] bytes of +/// stack. Used by both [`parse`] and [`parse_extended`] (which also +/// recurses into REPEAT bodies during the interpret pass). +pub(crate) fn run_on_parser_stack(f: F) -> R +where + R: Send, + F: FnOnce() -> R + Send, +{ + std::thread::scope(|s| { + let handle = std::thread::Builder::new() + .stack_size(PARSER_STACK_SIZE) + .name("stim-parser".to_string()) + .spawn_scoped(s, f) + .expect("failed to spawn parser thread"); + match handle.join() { + Ok(value) => value, + Err(payload) => std::panic::resume_unwind(payload), + } + }) +} + /// Parse Stim source into a [`Program`]. pub fn parse(src: &str) -> Result { + run_on_parser_stack(|| parse_impl(src)) +} + +pub(crate) fn parse_impl(src: &str) -> Result { use chumsky::Parser; let line_map = Arc::new(LineMap::new(src)); let parse_result = grammar::program_parser().parse(src); diff --git a/crates/stim-parser/tests/extended.rs b/crates/stim-parser/tests/extended.rs index 665fbcca4..60a036b43 100644 --- a/crates/stim-parser/tests/extended.rs +++ b/crates/stim-parser/tests/extended.rs @@ -1,6 +1,6 @@ -use stim_parser::ast::{AnnotationKind, GateName, MeasureName, NoiseName, RawInstruction}; +use stim_parser::ast::{AnnotationKind, GateName, MeasureName, NoiseName}; use stim_parser::extended::{ - Axis, ExtendedInstruction, ExtendedParseError, ExtendedProgram, parse_extended, + Axis, ExtendedInstruction, ExtendedParseError, ExtendedProgram, RawPassthrough, parse_extended, }; fn parse_ok(src: &str) -> ExtendedProgram { @@ -20,7 +20,7 @@ fn vanilla_h_passes_through() { let p = parse_ok("H 0\n"); assert_eq!(p.instructions.len(), 1); match &p.instructions[0] { - ExtendedInstruction::Raw(RawInstruction::Gate { + ExtendedInstruction::Raw(RawPassthrough::Gate { name, tags, targets, @@ -40,7 +40,7 @@ fn vanilla_h_passes_through() { fn vanilla_measure_passes_through() { let p = parse_ok("M 0 1\n"); match &p.instructions[0] { - ExtendedInstruction::Raw(RawInstruction::Measure { name, targets, .. }) => { + ExtendedInstruction::Raw(RawPassthrough::Measure { name, targets, .. }) => { assert_eq!(*name, MeasureName::M); assert_eq!(targets, &vec![0, 1]); } @@ -52,7 +52,7 @@ fn vanilla_measure_passes_through() { fn vanilla_depolarize1_noise_passes_through() { let p = parse_ok("DEPOLARIZE1(0.01) 0\n"); match &p.instructions[0] { - ExtendedInstruction::Raw(RawInstruction::Noise { name, args, .. }) => { + ExtendedInstruction::Raw(RawPassthrough::Noise { name, args, .. }) => { assert_eq!(*name, NoiseName::Depolarize1); assert_eq!(args, &vec![0.01]); } @@ -64,7 +64,7 @@ fn vanilla_depolarize1_noise_passes_through() { fn vanilla_annotation_passes_through() { let p = parse_ok("TICK\n"); match &p.instructions[0] { - ExtendedInstruction::Raw(RawInstruction::Annotation { kind, .. }) => { + ExtendedInstruction::Raw(RawPassthrough::Annotation { kind, .. }) => { assert_eq!(*kind, AnnotationKind::Tick); } other => panic!("{other:?}"), @@ -105,7 +105,7 @@ fn repeat_recurses_into_body() { assert_eq!(body.len(), 1); assert!(matches!( &body[0], - ExtendedInstruction::Raw(RawInstruction::Gate { + ExtendedInstruction::Raw(RawPassthrough::Gate { name: GateName::H, .. }) @@ -150,7 +150,7 @@ fn repeat_invalid_extended_tag_in_body_errors() { fn lenient_unknown_tag_on_h_passes_through() { let p = parse_ok("H[unrelated] 0\n"); match &p.instructions[0] { - ExtendedInstruction::Raw(RawInstruction::Gate { name, tags, .. }) => { + ExtendedInstruction::Raw(RawPassthrough::Gate { name, tags, .. }) => { assert_eq!(*name, GateName::H); assert_eq!(tags.len(), 1); assert_eq!(tags[0].name, "unrelated"); @@ -200,7 +200,7 @@ fn s_dag_t_promotes_to_t_dag() { fn s_with_no_tag_is_vanilla_gate() { let p = parse_ok("S 0\n"); match &p.instructions[0] { - ExtendedInstruction::Raw(RawInstruction::Gate { name, tags, .. }) => { + ExtendedInstruction::Raw(RawPassthrough::Gate { name, tags, .. }) => { assert_eq!(*name, GateName::S); assert!(tags.is_empty()); } @@ -212,7 +212,7 @@ fn s_with_no_tag_is_vanilla_gate() { fn s_dag_with_no_tag_is_vanilla_gate() { let p = parse_ok("S_DAG 0\n"); match &p.instructions[0] { - ExtendedInstruction::Raw(RawInstruction::Gate { name, tags, .. }) => { + ExtendedInstruction::Raw(RawPassthrough::Gate { name, tags, .. }) => { assert_eq!(*name, GateName::SDag); assert!(tags.is_empty()); } @@ -344,7 +344,7 @@ fn i_u3_missing_phi_errors() { fn i_with_no_tag_is_vanilla_identity() { let p = parse_ok("I 0\n"); match &p.instructions[0] { - ExtendedInstruction::Raw(RawInstruction::Gate { name, tags, .. }) => { + ExtendedInstruction::Raw(RawPassthrough::Gate { name, tags, .. }) => { assert_eq!(*name, GateName::Identity); assert!(tags.is_empty()); } diff --git a/crates/stim-parser/tests/proptest_ast.proptest-regressions b/crates/stim-parser/tests/proptest_ast.proptest-regressions deleted file mode 100644 index 97b73673c..000000000 --- a/crates/stim-parser/tests/proptest_ast.proptest-regressions +++ /dev/null @@ -1,7 +0,0 @@ -# Seeds for failure cases proptest has generated in the past. It is -# automatically read and these particular cases re-run before any -# novel cases are generated. -# -# It is recommended to check this file in to source control so that -# everyone who runs the test benefits from these saved cases. -cc d0bfbe5dca5d321afee55a38f6c38251eb0719b685395931c97283270fdbdf4e # shrinks to mut prog = ExtendedProgram { instructions: [Repeat { count: 1, body: [Raw(MPad { tags: [], prob: None, bits: [0], line: 0 })], line: 0 }] } diff --git a/crates/stim-parser/tests/proptest_ast.rs b/crates/stim-parser/tests/proptest_ast.rs index 262f0bbc1..667327aff 100644 --- a/crates/stim-parser/tests/proptest_ast.rs +++ b/crates/stim-parser/tests/proptest_ast.rs @@ -15,7 +15,7 @@ use proptest::prelude::*; use stim_parser::ast::{AnnotationKind, GateName, MeasureName, NoiseName, Program, RawInstruction}; use stim_parser::extended::parse_extended; -use stim_parser::extended::{Axis, ExtendedInstruction, ExtendedProgram}; +use stim_parser::extended::{Axis, ExtendedInstruction, ExtendedProgram, RawPassthrough}; use stim_parser::prelude::parse; // ---- generators -------------------------------------------------------- @@ -241,24 +241,112 @@ fn program() -> impl Strategy { // ---- extended-AST generators ------------------------------------------ -/// Subset of `flat_instr` safe to wrap in `ExtendedInstruction::Raw`. -/// -/// Excludes `MPad`, because `parse_extended` unconditionally promotes -/// `MPAD …` to `ExtendedInstruction::MPad { bits: Vec, … }`. A -/// generated `Raw(MPad)` would print as `MPAD …` and reparse as -/// `ExtendedInstruction::MPad`, never round-tripping to itself. -fn ext_raw_passthrough() -> impl Strategy { +/// `RawPassthrough` generators mirror the raw-AST generators for the four +/// variants that survive the interpret pass unchanged. +fn ext_gate_instr() -> impl Strategy { prop_oneof![ - gate_instr(), - noise_instr(), - measure_instr(), - annotation_instr(), + (single_qubit_clifford(), one_q_targets()).prop_map(|(name, targets)| { + RawPassthrough::Gate { + name, + tags: vec![], + args: vec![], + targets, + line: 0, + } + }), + (two_qubit_clifford(), two_q_pair_targets()).prop_map(|(name, targets)| { + RawPassthrough::Gate { + name, + tags: vec![], + args: vec![], + targets, + line: 0, + } + }), + ] +} + +fn ext_noise_instr() -> impl Strategy { + prop_oneof![ + (prob_lit(), one_q_targets()).prop_map(|(p, targets)| RawPassthrough::Noise { + name: NoiseName::Depolarize1, + tags: vec![], + args: vec![p], + targets, + line: 0, + }), + (prob_lit(), two_q_pair_targets()).prop_map(|(p, targets)| RawPassthrough::Noise { + name: NoiseName::Depolarize2, + tags: vec![], + args: vec![p], + targets, + line: 0, + }), + (prob_lit(), prob_lit(), prob_lit(), one_q_targets()).prop_map(|(a, b, c, targets)| { + RawPassthrough::Noise { + name: NoiseName::PauliChannel1, + tags: vec![], + args: vec![a, b, c], + targets, + line: 0, + } + }), + (prob_lit(), one_q_targets()).prop_map(|(p, targets)| RawPassthrough::Noise { + name: NoiseName::XError, + tags: vec![], + args: vec![p], + targets, + line: 0, + }), + ] +} + +fn ext_measure_instr() -> impl Strategy { + let name = prop_oneof![ + Just(MeasureName::M), + Just(MeasureName::MZ), + Just(MeasureName::MR), + ]; + (name, proptest::option::of(prob_lit()), one_q_targets()).prop_map(|(name, args, targets)| { + RawPassthrough::Measure { + name, + tags: vec![], + args: args.map(|p| vec![p]).unwrap_or_default(), + targets, + line: 0, + } + }) +} + +fn ext_annotation_instr() -> impl Strategy { + prop_oneof![ + Just(RawPassthrough::Annotation { + kind: AnnotationKind::Tick, + args: vec![], + targets: vec![], + line: 0, + }), + Just(RawPassthrough::Annotation { + kind: AnnotationKind::Detector, + args: vec![], + targets: vec![], + line: 0, + }), + ] +} + +fn ext_raw() -> impl Strategy { + prop_oneof![ + ext_gate_instr(), + ext_noise_instr(), + ext_measure_instr(), + ext_annotation_instr(), ] } fn ext_flat() -> impl Strategy { prop_oneof![ - ext_raw_passthrough().prop_map(ExtendedInstruction::Raw), + ext_raw().prop_map(ExtendedInstruction::Raw), one_q_targets().prop_map(|targets| ExtendedInstruction::T { targets, line: 0 }), one_q_targets().prop_map(|targets| ExtendedInstruction::TDag { targets, line: 0 }), ( @@ -347,10 +435,19 @@ fn zero_lines_raw(instrs: &mut [RawInstruction]) { } } +fn zero_lines_raw_passthrough(r: &mut RawPassthrough) { + match r { + RawPassthrough::Gate { line, .. } + | RawPassthrough::Noise { line, .. } + | RawPassthrough::Measure { line, .. } + | RawPassthrough::Annotation { line, .. } => *line = 0, + } +} + fn zero_lines_ext(instrs: &mut [ExtendedInstruction]) { for i in instrs { match i { - ExtendedInstruction::Raw(r) => zero_lines_raw(std::slice::from_mut(r)), + ExtendedInstruction::Raw(r) => zero_lines_raw_passthrough(r), ExtendedInstruction::T { line, .. } | ExtendedInstruction::TDag { line, .. } | ExtendedInstruction::Rotation { line, .. } diff --git a/crates/stim-parser/tests/proptest_parse.rs b/crates/stim-parser/tests/proptest_parse.rs index 687f0547c..ac1a6d392 100644 --- a/crates/stim-parser/tests/proptest_parse.rs +++ b/crates/stim-parser/tests/proptest_parse.rs @@ -101,15 +101,14 @@ proptest! { let _ = parse_extended(&src); } - /// Many nested REPEATs — guards against stack-blowing recursion. - /// - /// NOTE: capped at depth 16. The chumsky grammar uses `recursive(...)` - /// for REPEAT bodies and overflows the default thread stack somewhere - /// past depth ~24 in debug builds. Filed as a known limitation; not - /// addressed here since the fix is a grammar rewrite, not a test - /// change. + /// Many nested REPEATs. The grammar's chumsky `recursive(...)` + /// descends recursively through REPEAT bodies; `parse` / + /// `parse_extended` run on a 16 MiB dedicated stack thread, which is + /// large enough to handle a few hundred levels comfortably in debug + /// builds. 128 is a generous upper bound for any realistic Stim + /// program. #[test] - fn nested_repeats_never_panic(depth in 0usize..16) { + fn nested_repeats_never_panic(depth in 0usize..128) { let mut src = String::new(); for _ in 0..depth { src.push_str("REPEAT 1 {\n"); From a2e2985794ccfdd12f6fe2857a7127c6e9fde98a Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 17 May 2026 17:03:01 +0000 Subject: [PATCH 2/2] fix(stim-parser): address PR review follow-ups Agent-Logs-Url: https://github.com/QuEraComputing/ppvm/sessions/3ac90f00-66d8-4d7f-9f7d-f2227bde1f48 Co-authored-by: Roger-luo <8445510+Roger-luo@users.noreply.github.com> --- crates/stim-parser/src/display.rs | 64 ++++++++++++++++++++-- crates/stim-parser/tests/proptest_ast.rs | 14 +++++ crates/stim-parser/tests/proptest_parse.rs | 2 +- 3 files changed, 74 insertions(+), 6 deletions(-) diff --git a/crates/stim-parser/src/display.rs b/crates/stim-parser/src/display.rs index 544c0081a..a671f76e2 100644 --- a/crates/stim-parser/src/display.rs +++ b/crates/stim-parser/src/display.rs @@ -8,7 +8,7 @@ use std::fmt; use crate::ast::{Program, RawInstruction, Tag, TagParam}; -use crate::extended::ast::{Axis, ExtendedInstruction, ExtendedProgram}; +use crate::extended::ast::{Axis, ExtendedInstruction, ExtendedProgram, RawPassthrough}; const INDENT: &str = " "; @@ -122,12 +122,66 @@ fn fmt_raw(i: &RawInstruction, f: &mut fmt::Formatter<'_>, depth: usize) -> fmt: writeln!(f) } -fn fmt_ext(i: &ExtendedInstruction, f: &mut fmt::Formatter<'_>, depth: usize) -> fmt::Result { +fn fmt_raw_passthrough( + i: &RawPassthrough, + f: &mut fmt::Formatter<'_>, + depth: usize, +) -> fmt::Result { + write_indent(f, depth)?; match i { - ExtendedInstruction::Raw(r) => { - let owned: RawInstruction = r.clone().into_raw(); - return fmt_raw(&owned, f, depth); + RawPassthrough::Gate { + name, + tags, + args, + targets, + .. + } => { + f.write_str(name.canonical_name())?; + write_tags(f, tags)?; + write_args(f, args)?; + write_usize_targets(f, targets)?; + } + RawPassthrough::Noise { + name, + tags, + args, + targets, + .. + } => { + f.write_str(name.canonical_name())?; + write_tags(f, tags)?; + write_args(f, args)?; + write_usize_targets(f, targets)?; } + RawPassthrough::Measure { + name, + tags, + args, + targets, + .. + } => { + f.write_str(name.canonical_name())?; + write_tags(f, tags)?; + write_args(f, args)?; + write_usize_targets(f, targets)?; + } + RawPassthrough::Annotation { + kind, + args, + targets, + .. + } => { + f.write_str(kind.canonical_name())?; + write_args(f, args)?; + write_usize_targets(f, targets)?; + } + } + writeln!(f) +} + +fn fmt_ext(i: &ExtendedInstruction, f: &mut fmt::Formatter<'_>, depth: usize) -> fmt::Result { + match i { + ExtendedInstruction::Raw(r) => return fmt_raw_passthrough(r, f, depth), ExtendedInstruction::Repeat { count, body, .. } => { write_indent(f, depth)?; writeln!(f, "REPEAT {count} {{")?; diff --git a/crates/stim-parser/tests/proptest_ast.rs b/crates/stim-parser/tests/proptest_ast.rs index 667327aff..882b485e2 100644 --- a/crates/stim-parser/tests/proptest_ast.rs +++ b/crates/stim-parser/tests/proptest_ast.rs @@ -298,6 +298,20 @@ fn ext_noise_instr() -> impl Strategy { targets, line: 0, }), + (prob_lit(), one_q_targets()).prop_map(|(p, targets)| RawPassthrough::Noise { + name: NoiseName::YError, + tags: vec![], + args: vec![p], + targets, + line: 0, + }), + (prob_lit(), one_q_targets()).prop_map(|(p, targets)| RawPassthrough::Noise { + name: NoiseName::ZError, + tags: vec![], + args: vec![p], + targets, + line: 0, + }), ] } diff --git a/crates/stim-parser/tests/proptest_parse.rs b/crates/stim-parser/tests/proptest_parse.rs index ac1a6d392..d108c6985 100644 --- a/crates/stim-parser/tests/proptest_parse.rs +++ b/crates/stim-parser/tests/proptest_parse.rs @@ -108,7 +108,7 @@ proptest! { /// builds. 128 is a generous upper bound for any realistic Stim /// program. #[test] - fn nested_repeats_never_panic(depth in 0usize..128) { + fn nested_repeats_never_panic(depth in 0usize..=128) { let mut src = String::new(); for _ in 0..depth { src.push_str("REPEAT 1 {\n");