Skip to content

Commit 32a24b7

Browse files
Copilotminestarksidavis
authored
Fix cphase gate argument order and fix 4 QASM-to-Q# compiler bugs (#3158)
The `cphase` OpenQASM compatibility gate fails with type errors because its Q# signature has the control qubit before the angle parameter, while the compiler always emits classical args before qubit args (as with every other gate). ``` type error: expected Qubit, found Angle type error: expected Angle, found Qubit ``` - Reordered `cphase` parameters from `(ctrl : Qubit, lambda : Angle, qubit : Qubit)` to `(lambda : Angle, ctrl : Qubit, qubit : Qubit)` in `Std.OpenQASM.Intrinsic`, consistent with `cp`, `crx`, `cry`, `crz`, and `cu` - Body unchanged — it references params by name - Added comprehensive test `all_stdgates_inc_gates_can_be_called` that calls every gate from OpenQASM's `stdgates.inc` in a single test (25 main gates + 7 backwards compatibility gates: `CX`, `phase`, `cphase`, `id`, `u1`, `u2`, `u3`) - Added test `all_stdgates_inc_gates_adjoint_can_be_called` that calls the adjoint (`inv @`) of every stdgates.inc gate - Added test `all_stdgates_inc_gates_controlled_can_be_called` that calls the singly controlled (`ctrl @`) version of every stdgates.inc gate - Added Q# compilation verification to all QASM-to-Q# test helpers (`compile_qasm_to_qsharp`, `compile_qasm_stmt_to_qsharp_with_semantics`, `compile_qasm_to_qsharp_file`, `compile_qasm_to_qsharp_operation`, `qsharp_from_qasm_compilation`) so that generated Q# is verified through the Q# compiler via `compile_ast`. For File mode packages the AST is verified directly; for Fragments/Operation mode the QASM source is re-compiled as File mode to produce a complete Q# structure before verification. - Fixed `__AngleShl__` → `AngleShl` typo in compiler binary operator name mapping (the Q# library function is `AngleShl`, matching `AngleShr`, `AngleAndB`, etc.) - Fixed `negctrl` functor constraints: `ApplyControlledOnInt` requires `Adj + Ctl` (it uses a `within/apply` pattern internally), but the constraint solver only marked `Ctl` for `negctrl`. Split `Ctrl` and `NegCtrl` handling in `functor_constraints.rs` so that `NegCtrl` sets both `requires_adj` and `requires_ctl`. - Fixed `bit` ordered comparison operators (`>`, `>=`, `<`, `<=`): Q# `Result` type only supports `==` and `!=`. The compiler now wraps both operands in `ResultAsInt()` calls for ordered comparisons, matching how `bit[]` comparisons already use `ResultArrayAsIntBE`. - Fixed `input` variable mutation: OpenQASM `input` variables are mutable, but Q# operation parameters are immutable. Added a `collect_assigned_input_symbols` pre-analysis pass that identifies which input params are reassigned in the program, then generates `mutable a = a;` shadow copies at the start of the operation body for only those parameters. - Fixed `return_from_switch` test: the original QASM input was invalid (a function returning `bit` with a switch that didn't cover all cases). Added a `default { return 0; }` branch to make the test input valid. --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: minestarks <16928427+minestarks@users.noreply.github.com> Co-authored-by: idavis <63540+idavis@users.noreply.github.com>
1 parent 21ea7aa commit 32a24b7

9 files changed

Lines changed: 608 additions & 23 deletions

File tree

library/std/src/Std/OpenQASM/Intrinsic.qs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -222,7 +222,7 @@ operation phase(lambda : Angle, qubit : Qubit) : Unit is Adj + Ctl {
222222
U(ZERO_ANGLE(), ZERO_ANGLE(), lambda, qubit);
223223
}
224224

225-
operation cphase(ctrl : Qubit, lambda : Angle, qubit : Qubit) : Unit is Adj + Ctl {
225+
operation cphase(lambda : Angle, ctrl : Qubit, qubit : Qubit) : Unit is Adj + Ctl {
226226
Controlled phase([ctrl], (lambda, qubit));
227227
}
228228

source/compiler/qsc_openqasm_compiler/src/compiler.rs

Lines changed: 89 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ use std::{rc::Rc, str::FromStr, sync::Arc};
99
use error::CompilerErrorKind;
1010
use num_bigint::BigInt;
1111
use qsc_data_structures::{error::WithSource, source::SourceMap, span::Span, target::Profile};
12-
use rustc_hash::FxHashMap;
12+
use rustc_hash::{FxHashMap, FxHashSet};
1313

1414
use crate::{
1515
CompilerConfig, FunctorConstraintSolver, FunctorConstraints, OperationSignature,
@@ -52,6 +52,7 @@ use qsc_openqasm_parser::{
5252
},
5353
symbols::{IOKind, Symbol, SymbolId, SymbolTable},
5454
types::{Type, promote_types},
55+
visit::{Visitor, walk_stmt},
5556
},
5657
stdlib::complex::Complex,
5758
};
@@ -108,6 +109,7 @@ pub fn compile_to_qsharp_ast_with_config(
108109
errors,
109110
pragma_config: PragmaConfig::default(),
110111
functor_constraints: FxHashMap::default(),
112+
assigned_input_symbols: FxHashSet::default(),
111113
};
112114

113115
compiler.compile(&program)
@@ -182,6 +184,56 @@ pub struct QasmCompiler {
182184
/// Functor constraints for each gate, computed by the constraint solver pass.
183185
/// Maps gate symbol IDs to their required functor support (Adj, Ctl).
184186
pub functor_constraints: FxHashMap<SymbolId, FunctorConstraints>,
187+
/// Set of input symbol names that are targets of assignment statements.
188+
/// Used to create mutable shadow copies for these parameters in the operation body.
189+
pub assigned_input_symbols: FxHashSet<String>,
190+
}
191+
192+
/// Collects the names of input symbols that are assigned to in the program.
193+
/// This is used to determine which input parameters need mutable shadow copies
194+
/// in the generated Q# operation body.
195+
fn collect_assigned_input_symbols(
196+
program: &semast::Program,
197+
symbols: &SymbolTable,
198+
) -> FxHashSet<String> {
199+
struct AssignmentCollector<'a> {
200+
input_names: &'a FxHashSet<String>,
201+
symbols: &'a SymbolTable,
202+
assigned: FxHashSet<String>,
203+
}
204+
205+
impl Visitor for AssignmentCollector<'_> {
206+
fn visit_stmt(&mut self, stmt: &semast::Stmt) {
207+
if let semast::StmtKind::Assign(assign) = &*stmt.kind
208+
&& let semast::ExprKind::Ident(sym_id) = &*assign.lhs.kind
209+
{
210+
let sym = &self.symbols[*sym_id];
211+
if self.input_names.contains(&sym.name) {
212+
self.assigned.insert(sym.name.clone());
213+
}
214+
}
215+
walk_stmt(self, stmt);
216+
}
217+
}
218+
219+
let input_names: FxHashSet<String> = symbols
220+
.get_input()
221+
.unwrap_or_default()
222+
.iter()
223+
.map(|s| s.name.clone())
224+
.collect();
225+
226+
if input_names.is_empty() {
227+
return FxHashSet::default();
228+
}
229+
230+
let mut collector = AssignmentCollector {
231+
input_names: &input_names,
232+
symbols,
233+
assigned: FxHashSet::default(),
234+
};
235+
collector.visit_program(program);
236+
collector.assigned
185237
}
186238

187239
impl QasmCompiler {
@@ -194,6 +246,10 @@ impl QasmCompiler {
194246
// each gate definition needs to support based on how they're called.
195247
self.functor_constraints = FunctorConstraintSolver::solve(program);
196248

249+
// Collect input symbols that are targets of assignment statements.
250+
// These need mutable shadow copies when compiled as operation parameters.
251+
self.assigned_input_symbols = collect_assigned_input_symbols(program, &self.symbols);
252+
197253
// in non-file mode we need the runtime imports in the body
198254
let program_ty = self.config.program_ty.clone();
199255

@@ -385,6 +441,22 @@ impl QasmCompiler {
385441
})
386442
.collect::<Vec<_>>();
387443
let mut validation_stmts = Self::get_argument_validation_stmts(&args);
444+
445+
// In OpenQASM, input variables are mutable. In Q#, operation parameters
446+
// are immutable. Create mutable shadow copies for input params that
447+
// are reassigned in the program body so that `set` works correctly.
448+
for s in input {
449+
if self.assigned_input_symbols.contains(&s.name) {
450+
let qsharp_ty = self.map_semantic_type_to_qsharp_type(&s.ty, s.ty_span);
451+
let init_expr = build_path_ident_expr(&s.name, s.span, s.span);
452+
let shadow_stmt = build_classical_decl(
453+
&s.name, false, // mutable
454+
s.ty_span, s.span, s.span, &qsharp_ty, init_expr,
455+
);
456+
validation_stmts.push(shadow_stmt);
457+
}
458+
}
459+
388460
validation_stmts.extend(stmts);
389461
stmts = validation_stmts;
390462
}
@@ -1770,6 +1842,21 @@ impl QasmCompiler {
17701842
return Self::compile_complex_binary_op(op, lhs, rhs);
17711843
}
17721844

1845+
// Q# Result type only supports == and !=. For ordered comparisons
1846+
// (>, >=, <, <=) on bit values, convert to Int first.
1847+
if matches!(&binary.lhs.ty, Type::Bit(..))
1848+
&& matches!(&binary.rhs.ty, Type::Bit(..))
1849+
&& matches!(
1850+
op,
1851+
qsast::BinOp::Gt | qsast::BinOp::Gte | qsast::BinOp::Lt | qsast::BinOp::Lte
1852+
)
1853+
{
1854+
let span = binary.span();
1855+
let lhs = build_qasm_convert_call_with_one_param("ResultAsInt", lhs, span, span);
1856+
let rhs = build_qasm_convert_call_with_one_param("ResultAsInt", rhs, span, span);
1857+
return build_binary_expr(false, op, lhs, rhs, span);
1858+
}
1859+
17731860
let is_assignment = false;
17741861
build_binary_expr(is_assignment, op, lhs, rhs, binary.span())
17751862
}
@@ -1791,7 +1878,7 @@ impl QasmCompiler {
17911878

17921879
let fn_name: &str = match op {
17931880
// Bit shift
1794-
qsast::BinOp::Shl => "__AngleShl__",
1881+
qsast::BinOp::Shl => "AngleShl",
17951882
qsast::BinOp::Shr => "AngleShr",
17961883

17971884
// Bitwise

source/compiler/qsc_openqasm_compiler/src/functor_constraints.rs

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -194,7 +194,13 @@ impl Visitor for PropagationVisitor<'_> {
194194
GateModifierKind::Inv => {
195195
new_constraints.requires_adj = true;
196196
}
197-
GateModifierKind::Ctrl(_) | GateModifierKind::NegCtrl(_) => {
197+
GateModifierKind::Ctrl(_) => {
198+
new_constraints.requires_ctl = true;
199+
}
200+
GateModifierKind::NegCtrl(_) => {
201+
// The negctrl modifier uses ApplyControlledOnInt which requires
202+
// Adj + Ctl because it uses a within/apply pattern internally.
203+
new_constraints.requires_adj = true;
198204
new_constraints.requires_ctl = true;
199205
}
200206
GateModifierKind::Pow(_) => {
@@ -231,7 +237,13 @@ impl Visitor for FunctorConstraintSolver {
231237
GateModifierKind::Inv => {
232238
call_constraints.requires_adj = true;
233239
}
234-
GateModifierKind::Ctrl(_) | GateModifierKind::NegCtrl(_) => {
240+
GateModifierKind::Ctrl(_) => {
241+
call_constraints.requires_ctl = true;
242+
}
243+
GateModifierKind::NegCtrl(_) => {
244+
// The negctrl modifier uses ApplyControlledOnInt which requires
245+
// Adj + Ctl because it uses a within/apply pattern internally.
246+
call_constraints.requires_adj = true;
235247
call_constraints.requires_ctl = true;
236248
}
237249
GateModifierKind::Pow(_) => {

source/compiler/qsc_openqasm_compiler/src/tests.rs

Lines changed: 65 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ use qsc_hir::hir::PackageId;
2020
use qsc_openqasm_parser::io::{InMemorySourceResolver, SourceResolver};
2121
use qsc_openqasm_parser::semantic::{QasmSemanticParseResult, parse_source};
2222
use qsc_passes::PackageType;
23-
use rustc_hash::FxHashMap;
23+
use rustc_hash::{FxHashMap, FxHashSet};
2424
use std::sync::Arc;
2525

2626
pub(crate) mod assignment;
@@ -110,6 +110,7 @@ fn compile_with_config<S: Into<Arc<str>>>(
110110
errors,
111111
pragma_config: PragmaConfig::default(),
112112
functor_constraints: FxHashMap::default(),
113+
assigned_input_symbols: FxHashSet::default(),
113114
};
114115

115116
let unit = compiler.compile(&program);
@@ -176,6 +177,7 @@ pub fn compile_all_with_config<P: Into<Arc<str>>>(
176177
errors,
177178
pragma_config: PragmaConfig::default(),
178179
functor_constraints: FxHashMap::default(),
180+
assigned_input_symbols: FxHashSet::default(),
179181
};
180182

181183
let unit = compiler.compile(&program);
@@ -285,6 +287,7 @@ pub(crate) fn parse_all<P: Into<Arc<str>>>(
285287
pub fn compile_qasm_to_qsharp_file<S: Into<Arc<str>>>(
286288
source: S,
287289
) -> miette::Result<String, Vec<Report>> {
290+
let source: Arc<str> = source.into();
288291
let config = CompilerConfig::new(
289292
QubitSemantics::Qiskit,
290293
OutputSemantics::OpenQasm,
@@ -293,30 +296,27 @@ pub fn compile_qasm_to_qsharp_file<S: Into<Arc<str>>>(
293296
None,
294297
);
295298
let unit = compile_with_config(source, config)?;
296-
if unit.has_errors() {
297-
let errors = unit.errors.into_iter().map(Report::new).collect();
298-
return Err(errors);
299-
}
300-
let qsharp = gen_qsharp(&unit.package);
301-
Ok(qsharp)
299+
qsharp_from_qasm_compilation(unit)
302300
}
303301

304302
pub fn compile_qasm_to_qsharp_operation<S: Into<Arc<str>>>(
305303
source: S,
306304
) -> miette::Result<String, Vec<Report>> {
305+
let source: Arc<str> = source.into();
307306
let config = CompilerConfig::new(
308307
QubitSemantics::Qiskit,
309308
OutputSemantics::OpenQasm,
310309
ProgramType::Operation,
311310
Some("Test".into()),
312311
None,
313312
);
314-
let unit = compile_with_config(source, config)?;
313+
let unit = compile_with_config(source.clone(), config)?;
315314
if unit.has_errors() {
316315
let errors = unit.errors.into_iter().map(Report::new).collect();
317316
return Err(errors);
318317
}
319318
let qsharp = gen_qsharp(&unit.package);
319+
verify_qsharp_from_qasm_source(source, QubitSemantics::Qiskit)?;
320320
Ok(qsharp)
321321
}
322322

@@ -360,15 +360,22 @@ pub fn compile_qasm_to_qsharp_with_semantics<S: Into<Arc<str>>>(
360360
source: S,
361361
qubit_semantics: QubitSemantics,
362362
) -> miette::Result<String, Vec<Report>> {
363+
let source: Arc<str> = source.into();
363364
let config = CompilerConfig::new(
364365
qubit_semantics,
365366
OutputSemantics::Qiskit,
366367
ProgramType::Fragments,
367368
None,
368369
None,
369370
);
370-
let unit = compile_with_config(source, config)?;
371-
qsharp_from_qasm_compilation(unit)
371+
let unit = compile_with_config(source.clone(), config)?;
372+
if unit.has_errors() {
373+
let errors = unit.errors.into_iter().map(Report::new).collect();
374+
return Err(errors);
375+
}
376+
let qsharp = gen_qsharp(&unit.package);
377+
verify_qsharp_from_qasm_source(source, qubit_semantics)?;
378+
Ok(qsharp)
372379
}
373380

374381
pub fn qsharp_from_qasm_compilation(unit: QasmCompileUnit) -> miette::Result<String, Vec<Report>> {
@@ -377,9 +384,54 @@ pub fn qsharp_from_qasm_compilation(unit: QasmCompileUnit) -> miette::Result<Str
377384
return Err(errors);
378385
}
379386
let qsharp = gen_qsharp(&unit.package);
387+
verify_qsharp_ast(&unit)?;
380388
Ok(qsharp)
381389
}
382390

391+
/// Verifies that QASM source produces valid Q# by re-compiling it in File mode
392+
/// (which produces a complete Q# program) and passing the result through the Q#
393+
/// compiler. This is used for fragments mode where the Q# AST has top-level
394+
/// statements that the Q# compiler cannot process directly.
395+
fn verify_qsharp_from_qasm_source(
396+
source: Arc<str>,
397+
qubit_semantics: QubitSemantics,
398+
) -> miette::Result<(), Vec<Report>> {
399+
let config = CompilerConfig::new(
400+
qubit_semantics,
401+
OutputSemantics::OpenQasm,
402+
ProgramType::File,
403+
Some("Test".into()),
404+
None,
405+
);
406+
let file_unit = compile_with_config(source, config)?;
407+
if file_unit.has_errors() {
408+
let errors = file_unit.errors.into_iter().map(Report::new).collect();
409+
return Err(errors);
410+
}
411+
verify_qsharp_ast(&file_unit)
412+
}
413+
414+
/// Verifies a Q# AST package (with namespaces) compiles through the Q# compiler.
415+
fn verify_qsharp_ast(unit: &QasmCompileUnit) -> miette::Result<(), Vec<Report>> {
416+
let capabilities = unit.profile.into();
417+
let (stdid, store) = package_store_with_stdlib(capabilities);
418+
let dependencies = vec![(PackageId::CORE, None), (stdid, None)];
419+
let (_compiled, errors) = compile_ast(
420+
&store,
421+
&dependencies,
422+
unit.package.clone(),
423+
unit.source_map.clone(),
424+
PackageType::Lib,
425+
capabilities,
426+
);
427+
if errors.is_empty() {
428+
Ok(())
429+
} else {
430+
let reports = errors.into_iter().map(Report::new).collect();
431+
Err(reports)
432+
}
433+
}
434+
383435
pub fn compile_qasm_stmt_to_qsharp<S: Into<Arc<str>>>(
384436
source: S,
385437
) -> miette::Result<String, Vec<Report>> {
@@ -390,18 +442,20 @@ pub fn compile_qasm_stmt_to_qsharp_with_semantics<S: Into<Arc<str>>>(
390442
source: S,
391443
qubit_semantics: QubitSemantics,
392444
) -> miette::Result<String, Vec<Report>> {
445+
let source: Arc<str> = source.into();
393446
let config = CompilerConfig::new(
394447
qubit_semantics,
395448
OutputSemantics::Qiskit,
396449
ProgramType::Fragments,
397450
None,
398451
None,
399452
);
400-
let unit = compile_with_config(source, config)?;
453+
let unit = compile_with_config(source.clone(), config)?;
401454
if unit.has_errors() {
402455
let errors = unit.errors.into_iter().map(Report::new).collect();
403456
return Err(errors);
404457
}
458+
verify_qsharp_from_qasm_source(source, qubit_semantics)?;
405459
let qsharp = get_last_statement_as_qsharp(&unit.package);
406460
Ok(qsharp)
407461
}

source/compiler/qsc_openqasm_compiler/src/tests/assignment.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -187,7 +187,7 @@ fn angle_shl_assign() {
187187
Value = 0,
188188
Size = 53
189189
};
190-
set a = Std.OpenQASM.Angle.__AngleShl__(a, 1);
190+
set a = Std.OpenQASM.Angle.AngleShl(a, 1);
191191
"#]],
192192
);
193193
}

source/compiler/qsc_openqasm_compiler/src/tests/declaration/def.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -393,6 +393,7 @@ fn return_from_switch() {
393393
switch (a) {
394394
case 0 { return 1; }
395395
case 1 { return 0; }
396+
default { return 0; }
396397
}
397398
}
398399
"#;
@@ -406,6 +407,8 @@ fn return_from_switch() {
406407
return Std.OpenQASM.Convert.IntAsResult(1);
407408
} elif a == 1 {
408409
return Std.OpenQASM.Convert.IntAsResult(0);
410+
} else {
411+
return Std.OpenQASM.Convert.IntAsResult(0);
409412
};
410413
}
411414
"#]],

source/compiler/qsc_openqasm_compiler/src/tests/expression/binary/angle.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ fn shl() -> miette::Result<(), Vec<Report>> {
1717

1818
let qsharp = compile_qasm_stmt_to_qsharp(source)?;
1919
expect![[r#"
20-
mutable x = Std.OpenQASM.Angle.__AngleShl__(a, b);
20+
mutable x = Std.OpenQASM.Angle.AngleShl(a, b);
2121
"#]]
2222
.assert_eq(&qsharp);
2323
Ok(())

0 commit comments

Comments
 (0)