fix: drop unroll-emitted global phase for QASM 2 targets

CHANGELOG.md

@@ -28,6 +28,7 @@ Types of changes:
 ### Removed
 
 ### Fixed
+- Fixed unrolling of `rzz`/`rxx` in an OpenQASM 2 program emitting an invalid `gphase(...)` statement — syntax QASM 2 does not have — so the output was not a loadable QASM 2 program. Global phase is unobservable, so unroll-emitted phases are now dropped for a QASM 2 target; a user-written `gphase` is still rejected. A conditional left with no body by the drop is removed too, since QASM 2 has no form for an `if` without a `qop`. ([#351](https://github.com/qBraid/pyqasm/issues/351))
 - Fixed the `negctrl @` expansion emitting the same `x` `QuantumGate` object at both the leading and trailing position, so in-place transformations mutated its operands twice — crashing `reverse_qubit_order()` (`KeyError`) and `unroll(consolidate_qubits=True)` (`KeyError: '__PYQASM_QUBITS__'`) on any unrolled `negctrl` gate. The two `x` gates are now distinct statements with fresh operand nodes. ([#350](https://github.com/qBraid/pyqasm/issues/350))
 - Fixed inaccurate `device_qubits` entry in `QasmModule.unroll()` docstring ([#349](https://github.com/qBraid/pyqasm/pull/349))
 - Fixed `remove_idle_qubits()` and `reverse_qubit_order()` ignoring statements nested inside `box` and `if` blocks. Top-level operands were rewritten while nested ones kept their old indices, so the result silently addressed the wrong qubits — and when a nested index fell outside the shrunken register, the output was not a loadable program at all. Both passes now walk nested bodies, as do `has_measurements()` / `remove_measurements()` and `has_barriers()` / `remove_barriers()`; a box left empty by a removal is dropped, since pyqasm rejects a box with no statements. Two consequences of the same blind spot are fixed alongside: a qubit operated on only inside an `if` block no longer counts as idle, and `remove_idle_qubits()` no longer raises `AssertionError` on a program that mixes physical qubits with declared registers. ([#345](https://github.com/qBraid/pyqasm/pull/345))

src/pyqasm/modules/base.py

@@ -813,6 +813,22 @@ def copy(self) -> QasmModule:
         """Return a deep copy of the module."""
         return deepcopy(self)
 
+    def finalize(self, statements: list[qasm3_ast.Statement]) -> list[qasm3_ast.Statement]:
+        """Apply dialect-specific transformations to the finalized statement list.
+
+        Runs after the visitor has unrolled and finalized the program, on the
+        statements about to become the unrolled AST. The base implementation
+        returns them unchanged; a subclass overrides this when its dialect
+        cannot express something the unroller emits.
+
+        Args:
+            statements (list[Statement]): The finalized statements.
+
+        Returns:
+            list[Statement]: The statements to store as the unrolled AST.
+        """
+        return statements
+
     @abstractmethod
     def _qasm_ast_to_str(self, qasm_ast: Program) -> str:
         """Convert the qasm AST to a string."""

src/pyqasm/modules/qasm2.py

@@ -96,12 +96,11 @@ def _filter_branch_body(self, statement: qasm3_ast.BranchingStatement) -> None:
                 self._filter_branch_body(inner_stmt)
                 continue
             if isinstance(inner_stmt, qasm3_ast.QuantumPhase):
-                # not something the user wrote: rzz/rxx decompose to a global phase, so this
-                # is only reachable by re-filtering an already-unrolled body (see issue #351)
+                # unroll-emitted phases are dropped in accept() (issue #351), so only a
+                # user-written gphase reaches this
                 raise_qasm3_error(
                     "Global phase is not representable in QASM 2.0, so it cannot appear in "
-                    "a conditional body; it is introduced by unrolling gates such as 'rzz' "
-                    "and 'rxx'",
+                    "a conditional body",
                     error_node=inner_stmt,
                     span=inner_stmt.span,
                 )
@@ -148,6 +147,46 @@ def to_qasm3(self, as_str: bool = False) -> str | Qasm3Module:
         qasm_program.version = "3.0"
         return dumps(qasm_program) if as_str else Qasm3Module(self._name, qasm_program)
 
+    def finalize(self, statements: list[qasm3_ast.Statement]) -> list[qasm3_ast.Statement]:
+        """Apply the QASM 2 transformations the finalized statement list needs.
+
+        Args:
+            statements (list[Statement]): The finalized statements.
+
+        Returns:
+            list[Statement]: The statements to store as the unrolled AST.
+        """
+        return self._drop_global_phase(statements)
+
+    def _drop_global_phase(
+        self, statements: list[qasm3_ast.Statement]
+    ) -> list[qasm3_ast.Statement]:
+        """Remove QuantumPhase statements the unroller emitted (e.g. from the rzz/rxx
+        decompositions), descending into conditional bodies. OpenQASM 2 has no
+        global-phase syntax, and a global phase is unobservable, so dropping it is
+        semantically safe (issue #351)."""
+        filtered = []
+        for stmt in statements:
+            if isinstance(stmt, qasm3_ast.QuantumPhase):
+                # a controlled phase is relative, not global, and is observable; the
+                # visitor rewrites those to 'p' gates, so none should reach here
+                if stmt.modifiers:
+                    raise_qasm3_error(
+                        "Modified global phase cannot be dropped for a QASM 2 target",
+                        error_node=stmt,
+                        span=stmt.span,
+                    )
+                continue
+            if isinstance(stmt, qasm3_ast.BranchingStatement):
+                stmt.if_block = self._drop_global_phase(stmt.if_block)
+                stmt.else_block = self._drop_global_phase(stmt.else_block)
+                if not stmt.if_block and not stmt.else_block:
+                    # the body was nothing but global phase, and QASM 2 has no
+                    # form for a conditional without a qop
+                    continue
+            filtered.append(stmt)
+        return filtered
+
     def accept(self, visitor: QasmVisitor) -> None:
         """Accept a visitor for the module.
 
@@ -158,4 +197,4 @@ def accept(self, visitor: QasmVisitor) -> None:
         unrolled_stmt_list = visitor.visit_basic_block(self._statements)
         final_stmt_list = visitor.finalize(unrolled_stmt_list)
 
-        self.unrolled_ast.statements = final_stmt_list  # type: ignore[assignment]
+        self.unrolled_ast.statements = self.finalize(final_stmt_list)  # type: ignore[assignment]

src/pyqasm/modules/qasm3.py

@@ -89,4 +89,4 @@ def accept(self, visitor: QasmVisitor) -> None:
         unrolled_stmt_list = visitor.visit_basic_block(self._statements)
         final_stmt_list = visitor.finalize(unrolled_stmt_list)
 
-        self._unrolled_ast.statements = final_stmt_list  # type: ignore[assignment]
+        self._unrolled_ast.statements = self.finalize(final_stmt_list)  # type: ignore[assignment]

tests/qasm2/test_conditional_body.py

@@ -19,7 +19,7 @@
 
 import pytest
 
-from pyqasm.entrypoint import loads
+from pyqasm.entrypoint import dumps, loads
 from pyqasm.exceptions import ValidationError
 
 QASM2_PREAMBLE = """OPENQASM 2.0;
@@ -64,15 +64,30 @@ def test_conditional_non_qop_rejected(operation, keyword):
         module.validate()
 
 
-def test_conditional_global_phase_reports_global_phase():
-    """Test that the QuantumPhase unrolling introduces for rzz/rxx is reported as global
-    phase rather than as an AST class name. Reachable only by re-filtering an already
-    unrolled body, which remove_idle_qubits/reverse_qubit_order do (issue #351)."""
+def test_conditional_rzz_survives_refiltering():
+    """Test that transformations which re-filter an already unrolled body no longer
+    trip over the rzz global phase: it is dropped for a QASM 2 target (issue #351)"""
     module = loads(QASM2_PREAMBLE + "if(m==1) rzz(0.3) q[0], q[1];\n")
     module.unroll()
     module.reverse_qubit_order()
+    module.remove_idle_qubits()
+
+
+def test_conditional_user_gphase_rejected():
+    """Test that a gphase the user wrote in a conditional body is still rejected. Only
+    unroll-emitted phases are dropped; this branch keeps its own diagnostic (issue #351)"""
+    module = loads(QASM2_PREAMBLE + "if(m==1) gphase(0.3);\n")
     with pytest.raises(ValidationError, match="Global phase is not representable in QASM 2.0"):
-        module.remove_idle_qubits()
+        module.validate()
+
+
+def test_conditional_emptied_by_phase_drop_is_removed():
+    """Test that a conditional whose body was nothing but global phase is dropped rather
+    than emitted bodiless: QASM 2 has no form for an 'if' without a qop (issue #351)"""
+    module = loads(QASM2_PREAMBLE + "gate ph(t) a { gphase(t); }\nif(m==1) ph(0.3) q[1];\n")
+    module.unroll()
+    assert "if" not in dumps(module)
+    loads(dumps(module)).validate()
 
 
 @pytest.mark.parametrize(

tests/qasm2/test_operations.py

@@ -67,6 +67,115 @@ def test_whitelisted_ops():
     check_unrolled_qasm(dumps(result), expected_qasm)
 
 
+def test_rzz_unrolls_without_gphase():
+    """Test that the global phase from the rzz decomposition is dropped for a QASM 2
+    target, which has no global-phase syntax (issue #351)"""
+    qasm2_string = """
+    OPENQASM 2.0;
+    include 'qelib1.inc';
+    qreg q[2];
+    rzz(0.3) q[0], q[1];
+    """
+
+    expected_qasm = """
+    OPENQASM 2.0;
+    include 'qelib1.inc';
+    qreg q[2];
+    cx q[0], q[1];
+    rz(0.3) q[1];
+    rx(1.5707963267948966) q[1];
+    rz(3.141592653589793) q[1];
+    rx(1.5707963267948966) q[1];
+    rz(3.141592653589793) q[1];
+    cx q[0], q[1];
+    """
+
+    result = loads(qasm2_string)
+    result.unroll()
+    check_unrolled_qasm(dumps(result), expected_qasm)
+
+
+def test_rxx_unrolls_without_gphase():
+    """Test that the global phase from the rxx decomposition is dropped for a QASM 2
+    target (issue #351)"""
+    qasm2_string = """
+    OPENQASM 2.0;
+    include 'qelib1.inc';
+    qreg q[2];
+    rxx(0.3) q[0], q[1];
+    """
+
+    expected_qasm = """
+    OPENQASM 2.0;
+    include 'qelib1.inc';
+    qreg q[2];
+    h q[0];
+    h q[1];
+    cx q[0], q[1];
+    rz(0.3) q[1];
+    cx q[0], q[1];
+    h q[1];
+    h q[0];
+    """
+
+    result = loads(qasm2_string)
+    result.unroll()
+    check_unrolled_qasm(dumps(result), expected_qasm)
+
+
+def test_conditional_rzz_unrolls_without_gphase():
+    """Test that a conditional rzz body carries no gphase statement either (issue #351)"""
+    qasm2_string = """
+    OPENQASM 2.0;
+    include 'qelib1.inc';
+    qreg q[2];
+    creg m[1];
+    measure q[0] -> m[0];
+    if(m==1) rzz(0.3) q[0], q[1];
+    """
+
+    result = loads(qasm2_string)
+    result.unroll()
+    unrolled = dumps(result)
+    assert "gphase" not in unrolled
+
+    # the unrolled output must still re-load in pyqasm. It is not yet accepted by a
+    # strict QASM 2 parser: the conditional still prints as `if (m[0] == true) { ... }`,
+    # which is QASM 3 syntax. That half is #338's territory, not this PR's.
+    loads(unrolled).validate()
+
+
+def test_unrolled_qasm2_round_trips():
+    """Test that unrolled rzz output loads and re-unrolls cleanly: no gphase means the
+    second filtering pass has nothing to reject (issue #351)"""
+    qasm2_string = """
+    OPENQASM 2.0;
+    include 'qelib1.inc';
+    qreg q[2];
+    rzz(0.3) q[0], q[1];
+    """
+
+    result = loads(qasm2_string)
+    result.unroll()
+    round_tripped = loads(dumps(result))
+    round_tripped.unroll()
+    check_unrolled_qasm(dumps(round_tripped), dumps(result))
+
+
+def test_user_written_gphase_rejected():
+    """Test that a gphase statement written in QASM 2 source is still rejected --
+    OpenQASM 2 has no global-phase syntax, so only unroller-introduced phases are dropped"""
+    qasm2_string = """
+    OPENQASM 2.0;
+    include 'qelib1.inc';
+    qreg q[2];
+    gphase(0.3);
+    """
+
+    with pytest.raises(ValidationError):
+        loads(qasm2_string).validate()
+
+
 def test_subroutine_blacklist():
 
     # subroutines