Skip to content

Commit a97599c

Browse files
Roger-luoclaude
andcommitted
fix(stim-parser): reject *pi in bare R_X/R_Y/R_Z/U3 args (half-turn double-scale)
Bare rotation/U3 args are in half-turns and get multiplied by pi when lowering (R_Z(0.5) == I[R_Z(theta=0.5*pi)]). But args flow through the generic pi_expr parser, so R_Z(0.5*pi) / R_Z(pi) were silently scaled by pi twice (0.5*pi*pi instead of 0.5*pi) with no diagnostic — the inverse of the ambiguity the *pi tag requirement was added to prevent. args_block now carries a had_pi flag (via the existing pi_expr_flagged); the validator rejects a *pi argument on the bare R_X/R_Y/R_Z/U3 mnemonics with a clear half-turn-arg error. pi-expressions remain valid in args for every other instruction (stim.rs compatibility, e.g. X_ERROR(0.5*pi)). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent e5ccc7a commit a97599c

4 files changed

Lines changed: 143 additions & 21 deletions

File tree

crates/stim-parser/src/pipeline/lower.rs

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -767,6 +767,42 @@ mod tests {
767767
assert_eq!(err.last().unwrap().code, Some("invalid-tag"));
768768
}
769769

770+
#[test]
771+
fn bare_rotation_with_pi_arg_is_rejected() {
772+
// The bare arg is in half-turns and gets multiplied by pi when
773+
// lowering, so `R_Z(0.5*pi)` would scale by pi twice. The `*pi` form
774+
// is only valid in the tag (I[R_Z(theta=0.5*pi)]); reject it here.
775+
for src in ["R_X(0.5*pi) 0", "R_Y(1*pi) 0", "R_Z(0.5*pi) 0", "R_Z(pi) 0"] {
776+
let err = lower_extended(src).unwrap_err();
777+
assert_eq!(err.last().unwrap().code, Some("half-turn-arg"), "src={src}");
778+
}
779+
}
780+
781+
#[test]
782+
fn bare_u3_with_pi_arg_is_rejected() {
783+
// Every U3 angle is in half-turns; a `*pi` (or bare `pi`) in any slot
784+
// would double-scale.
785+
for src in [
786+
"U3(0.5*pi, 0.0, 0.0) 0",
787+
"U3(0.0, 0.5*pi, 0.0) 0",
788+
"U3(0.0, 0.0, pi) 0",
789+
] {
790+
let err = lower_extended(src).unwrap_err();
791+
assert_eq!(err.last().unwrap().code, Some("half-turn-arg"), "src={src}");
792+
}
793+
}
794+
795+
#[test]
796+
fn bare_rotation_plain_arg_is_accepted() {
797+
// The half-turn plain form is the canonical bare spelling and lowers
798+
// to the same radians as the tagged `*pi` form.
799+
let prog = lower_extended("R_Z(0.5) 0").expect("lower");
800+
assert!(matches!(
801+
&prog.instructions[0],
802+
ExtendedInstruction::Rotation { axis: Axis::Z, .. }
803+
));
804+
}
805+
770806
#[test]
771807
fn bare_u3_with_tag_is_rejected() {
772808
let err = lower_extended("U3[foo](0.5, 1.0, 1.5) 0").unwrap_err();

crates/stim-parser/src/pipeline/validate.rs

Lines changed: 71 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,9 @@ use crate::ast::shared::{
1414
};
1515
use crate::ast::vanilla::{Instruction, Program};
1616
use crate::diagnostics::{Aborted, DiagnosticSink, LineMap, Span};
17-
use crate::instructions::{ArgCount, EntryKind, MeasureName, TableEntry, TargetArity, lookup};
17+
use crate::instructions::{
18+
ArgCount, EntryKind, GateName, MeasureName, TableEntry, TargetArity, lookup,
19+
};
1820
use crate::syntax::{RawSyntaxNode, RawSyntaxTree, RawTarget};
1921

2022
use super::emit_skip;
@@ -64,6 +66,7 @@ fn validate_node(
6466
name,
6567
tags,
6668
args,
69+
args_had_pi,
6770
targets,
6871
span,
6972
} => {
@@ -148,6 +151,31 @@ fn validate_node(
148151
);
149152
}
150153

154+
// Bare rotation/U3 mnemonics take their angle in half-turns and
155+
// multiply by pi when lowering, so a `*pi` argument would be scaled
156+
// by pi twice. The `*pi` form is a tag convention
157+
// (`I[R_Z(theta=0.5*pi)]`); reject it on the bare gate with a clear
158+
// message rather than silently double-scaling.
159+
if args_had_pi
160+
&& matches!(
161+
entry.kind,
162+
EntryKind::Gate(
163+
GateName::RotX | GateName::RotY | GateName::RotZ | GateName::U3
164+
)
165+
)
166+
{
167+
return emit_skip(
168+
sink,
169+
span,
170+
"half-turn-arg",
171+
format!(
172+
"'{canonical}' takes angle arguments in half-turns; remove the '*pi' \
173+
suffix (the '*pi' form is only valid in tags, e.g. \
174+
I[R_Z(theta=0.5*pi)])"
175+
),
176+
);
177+
}
178+
151179
let divisor = match target_rule {
152180
TargetArity::Any => None,
153181
TargetArity::AtLeastOne => Some(1),
@@ -339,6 +367,7 @@ mod tests {
339367
name: name.to_string(),
340368
tags: vec![],
341369
args,
370+
args_had_pi: false,
342371
targets: targets
343372
.into_iter()
344373
.map(|t| RawTarget {
@@ -360,6 +389,7 @@ mod tests {
360389
name: name.to_string(),
361390
tags: vec![],
362391
args,
392+
args_had_pi: false,
363393
targets: targets
364394
.into_iter()
365395
.map(|(text, span)| RawTarget {
@@ -376,6 +406,7 @@ mod tests {
376406
name: name.to_string(),
377407
tags,
378408
args: vec![],
409+
args_had_pi: false,
379410
targets: vec![RawTarget {
380411
text: "0".to_string(),
381412
span: SimpleSpan::from(2..3),
@@ -454,6 +485,45 @@ mod tests {
454485
assert_eq!(items[0].code, Some("arg-count"));
455486
}
456487

488+
#[test]
489+
fn rotation_gate_with_pi_arg_is_half_turn_arg_error() {
490+
// A bare rotation/U3 mnemonic whose arg used the `*pi` form is
491+
// rejected: the half-turn arg already gets multiplied by pi when
492+
// lowering, so accepting `*pi` would double-scale.
493+
let node = RawSyntaxNode::Instruction {
494+
name: "R_Z".to_string(),
495+
tags: vec![],
496+
args: vec![std::f64::consts::PI],
497+
args_had_pi: true,
498+
targets: vec![RawTarget {
499+
text: "0".to_string(),
500+
span: SimpleSpan::from(0..1),
501+
}],
502+
span: SimpleSpan::from(0..3),
503+
};
504+
let items = collect_errors(vec![node], &lm());
505+
assert_eq!(items[0].code, Some("half-turn-arg"));
506+
}
507+
508+
#[test]
509+
fn non_rotation_gate_keeps_pi_arg() {
510+
// Only the bare rotation/U3 mnemonics reject `*pi` args; for everything
511+
// else the pi-expression stays valid (stim.rs compatibility).
512+
let node = RawSyntaxNode::Instruction {
513+
name: "X_ERROR".to_string(),
514+
tags: vec![],
515+
args: vec![0.5 * std::f64::consts::PI],
516+
args_had_pi: true,
517+
targets: vec![RawTarget {
518+
text: "0".to_string(),
519+
span: SimpleSpan::from(0..1),
520+
}],
521+
span: SimpleSpan::from(0..7),
522+
};
523+
let prog = ok_program(vec![node], &lm());
524+
assert!(matches!(&prog.instructions[0], Instruction::Noise(_)));
525+
}
526+
457527
#[test]
458528
fn target_pair_errors() {
459529
let items = collect_errors(

crates/stim-parser/src/syntax/grammar.rs

Lines changed: 32 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -141,9 +141,14 @@ pub(crate) fn tags_block<'src>() -> impl Parser<'src, &'src str, Vec<Tag>, Extra
141141
.delimited_by(just('[').then(inline_pad()), inline_pad().then(just(']')))
142142
}
143143

144-
/// `(pi_expr, pi_expr, ...)`.
145-
pub(crate) fn args_block<'src>() -> impl Parser<'src, &'src str, Vec<f64>, Extra<'src>> + Clone {
146-
pi_expr()
144+
/// `(pi_expr, pi_expr, ...)`. Each argument is paired with whether it was
145+
/// written with the `*pi` (half-turn) form. Most callers ignore the flag,
146+
/// but the bare rotation mnemonics (`R_X`/`R_Y`/`R_Z`/`U3`) take their angle
147+
/// in half-turns and multiply by pi when lowering, so they reject a `*pi`
148+
/// argument — which would otherwise be multiplied by pi twice.
149+
pub(crate) fn args_block<'src>()
150+
-> impl Parser<'src, &'src str, Vec<(f64, bool)>, Extra<'src>> + Clone {
151+
pi_expr_flagged()
147152
.separated_by(inline_pad().then(just(',')).then(inline_pad()))
148153
.allow_trailing()
149154
.collect::<Vec<_>>()
@@ -168,22 +173,21 @@ pub(crate) fn target_lexeme<'src>() -> impl Parser<'src, &'src str, RawTarget, E
168173
})
169174
}
170175

171-
/// `<ident> [<tags>]? (<args>)?`. Returns name, tags, args, and the
172-
/// span of the identifier (used for line-number reporting).
176+
/// `<ident> [<tags>]? (<args>)?`. Returns name, tags, args, whether any
177+
/// argument used the `*pi` (half-turn) form, and the span of the identifier
178+
/// (used for line-number reporting).
173179
pub(crate) fn instruction_head<'src>()
174-
-> impl Parser<'src, &'src str, (String, Vec<Tag>, Vec<f64>, SimpleSpan<usize>), Extra<'src>> + Clone
175-
{
180+
-> impl Parser<'src, &'src str, (String, Vec<Tag>, Vec<f64>, bool, SimpleSpan<usize>), Extra<'src>>
181+
+ Clone {
176182
ident()
177183
.map_with(|name, e| (name, e.span()))
178184
.then(tags_block().or_not())
179185
.then(args_block().or_not())
180186
.map(|(((name, span), tags), args)| {
181-
(
182-
name,
183-
tags.unwrap_or_default(),
184-
args.unwrap_or_default(),
185-
span,
186-
)
187+
let args = args.unwrap_or_default();
188+
let args_had_pi = args.iter().any(|&(_, had_pi)| had_pi);
189+
let values = args.into_iter().map(|(value, _)| value).collect();
190+
(name, tags.unwrap_or_default(), values, args_had_pi, span)
187191
})
188192
}
189193

@@ -212,10 +216,11 @@ pub(crate) fn instruction_line<'src>()
212216
.collect::<Vec<RawTarget>>(),
213217
)
214218
.map(
215-
|((name, tags, args, span), targets)| RawSyntaxNode::Instruction {
219+
|((name, tags, args, args_had_pi, span), targets)| RawSyntaxNode::Instruction {
216220
name,
217221
tags,
218222
args,
223+
args_had_pi,
219224
targets,
220225
span,
221226
},
@@ -371,14 +376,21 @@ mod tests {
371376
#[test]
372377
fn args_block_parses_csv_floats() {
373378
let a = run(args_block(), "(0.1, 0.2, 0.3)");
374-
assert_eq!(a, vec![0.1, 0.2, 0.3]);
379+
// Each arg is paired with whether it used the `*pi` form (none here).
380+
assert_eq!(a, vec![(0.1, false), (0.2, false), (0.3, false)]);
375381
}
376382

377383
#[test]
378384
fn args_block_with_pi_exprs() {
379-
let a = run(args_block(), "(pi, 0.5*pi)");
380-
assert!((a[0] - std::f64::consts::PI).abs() < 1e-12);
381-
assert!((a[1] - 0.5 * std::f64::consts::PI).abs() < 1e-12);
385+
// Args still accept pi-expressions (for stim.rs compatibility); the
386+
// `had_pi` flag records that they did so callers can reject it where
387+
// the half-turn convention applies.
388+
let a = run(args_block(), "(pi, 0.5*pi, 2.0)");
389+
assert!((a[0].0 - std::f64::consts::PI).abs() < 1e-12);
390+
assert!(a[0].1);
391+
assert!((a[1].0 - 0.5 * std::f64::consts::PI).abs() < 1e-12);
392+
assert!(a[1].1);
393+
assert_eq!(a[2], (2.0, false));
382394
}
383395

384396
#[test]
@@ -399,7 +411,7 @@ mod tests {
399411

400412
#[test]
401413
fn instruction_head_with_tags_and_args() {
402-
let (name, tags, args, _span) = run(instruction_head(), "S[T](0.5)");
414+
let (name, tags, args, _had_pi, _span) = run(instruction_head(), "S[T](0.5)");
403415
assert_eq!(name, "S");
404416
assert_eq!(tags.len(), 1);
405417
assert_eq!(tags[0].name, "T");
@@ -408,7 +420,7 @@ mod tests {
408420

409421
#[test]
410422
fn instruction_head_no_tags_no_args() {
411-
let (name, tags, args, _span) = run(instruction_head(), "H");
423+
let (name, tags, args, _had_pi, _span) = run(instruction_head(), "H");
412424
assert_eq!(name, "H");
413425
assert!(tags.is_empty());
414426
assert!(args.is_empty());

crates/stim-parser/src/syntax/raw.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,10 @@ pub(crate) enum RawSyntaxNode {
1717
name: String,
1818
tags: Vec<Tag>,
1919
args: Vec<f64>,
20+
/// Whether any argument was written with the `*pi` (half-turn) form.
21+
/// The bare rotation mnemonics (`R_X`/`R_Y`/`R_Z`/`U3`) reject it,
22+
/// since they already scale their half-turn argument by pi.
23+
args_had_pi: bool,
2024
targets: Vec<RawTarget>,
2125
span: SimpleSpan<usize>,
2226
},

0 commit comments

Comments
 (0)