fix: emit distinct x gates with fresh operands in negctrl expansion

CHANGELOG.md

@@ -26,6 +26,7 @@ Types of changes:
 ### Removed
 
 ### Fixed
+- 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))
 - Fixed `unroll(consolidate_qubits=True)` raising `AttributeError: 'str' object has no attribute 'name'` for any gate applied to a physical qubit, e.g. `h $1;`. Consolidation assumed every gate operand was an `IndexedIdentifier`, but a physical qubit survives unrolling as `Identifier("$1")`. Physical qubits are absolute hardware indices belonging to no declared register, so they are now left as written — matching how `measure`, `reset` and `barrier` already treat them. ([#344](https://github.com/qBraid/pyqasm/pull/344))

src/pyqasm/visitor.py

@@ -62,6 +62,7 @@
     MAX_ARRAY_DIMENSIONS,
 )
 from pyqasm.maps.gates import (
+    fresh_qubits,
     map_qasm_ctrl_op_to_callable,
     map_qasm_inv_op_to_callable,
     map_qasm_op_num_params,
@@ -1608,11 +1609,16 @@ def _visit_generic_gate_operation(  # pylint: disable=too-many-branches, too-man
             else:
                 result.extend(self._visit_basic_gate_operation(operation, inverse_value, ctrls))
 
-        # negctrl -> ctrl conversion
-        negs = [
-            qasm3_ast.QuantumGate([], qasm3_ast.Identifier("x"), [], [ctrl]) for ctrl in negctrls
-        ]
-        result = negs + result + negs  # type: ignore
+        # negctrl -> ctrl conversion; build each x gate with fresh operand nodes so
+        # the leading and trailing statements share nothing (issue #350)
+        def _neg_x_gates() -> list[qasm3_ast.QuantumGate]:
+            """Build an x gate with fresh operand nodes for each negative control."""
+            return [
+                qasm3_ast.QuantumGate([], qasm3_ast.Identifier("x"), [], fresh_qubits(ctrl))
+                for ctrl in negctrls
+            ]
+
+        result = _neg_x_gates() + result + _neg_x_gates()  # type: ignore
         self._in_generic_gate_op_scope -= 1
         if self._consolidate_qubits and not self._in_generic_gate_op_scope:
             result = cast(

tests/qasm3/test_transformations.py

@@ -228,6 +228,13 @@ def _assert_no_shared_operand_nodes(module):
         "c4x q[0], q[1], q[2], q[3], q[4];",
         "ecr q[0], q[1];",
         "inv @ crz(0.5) q[1], q[2];",
+        "negctrl @ x q[0], q[1];",
+        "negctrl(2) @ x q[0], q[1], q[2];",
+        # each of these reaches the negctrl expansion through a different
+        # operand shape or repeats it, so each is a distinct way to regress
+        "negctrl @ x q[{0, 1}], q[2];",
+        "pow(2) @ negctrl @ x q[0], q[1];",
+        "ctrl @ negctrl @ x q[0], q[1], q[2];",
     ],
 )
 def test_unroll_emits_fresh_operand_nodes(operation):
@@ -244,6 +251,89 @@ def test_unroll_emits_fresh_operand_nodes(operation):
     _assert_no_shared_operand_nodes(module)
 
 
+def test_reverse_qubit_order_negctrl():
+    """Test reverse_qubit_order on a negctrl gate whose leading and trailing x
+    statements previously were the same object (issue #350)"""
+    qasm3_str = """
+    OPENQASM 3.0;
+    include "stdgates.inc";
+    qubit[3] q;
+    negctrl @ x q[0], q[1];
+    """
+
+    expected_qasm3_str = """
+    OPENQASM 3.0;
+    include "stdgates.inc";
+    qubit[3] q;
+    x q[2];
+    cx q[2], q[1];
+    x q[2];
+    """
+
+    module = loads(qasm3_str)
+    module.unroll()
+    module.reverse_qubit_order()
+    check_unrolled_qasm(dumps(module), expected_qasm3_str)
+
+
+def test_remove_idle_qubits_negctrl():
+    """Test remove_idle_qubits on a negctrl gate whose leading and trailing x
+    statements previously were the same object (issue #350).
+
+    This path survived the aliasing only via the ``visited_node_ids`` guard in
+    ``_remap_qubits``, so it is pinned here to keep that incidental rescue from
+    being the only thing standing between a refactor and the crash.
+    """
+    qasm3_str = """
+    OPENQASM 3.0;
+    include "stdgates.inc";
+    qubit[3] q;
+    negctrl @ x q[0], q[1];
+    """
+
+    expected_qasm3_str = """
+    OPENQASM 3.0;
+    include "stdgates.inc";
+    qubit[2] q;
+    x q[0];
+    cx q[0], q[1];
+    x q[0];
+    """
+
+    module = loads(qasm3_str)
+    module.unroll()
+    module.remove_idle_qubits()
+    check_unrolled_qasm(dumps(module), expected_qasm3_str)
+
+
+def test_consolidate_qubits_negctrl():
+    """Test unroll(consolidate_qubits=True) on a negctrl gate whose leading and
+    trailing x statements previously were the same object (issue #350).
+
+    Consolidation rewrote the shared operand once, then met the already-renamed
+    node again and raised ``KeyError: '__PYQASM_QUBITS__'``.
+    """
+    qasm3_str = """
+    OPENQASM 3.0;
+    include "stdgates.inc";
+    qubit[3] q;
+    negctrl @ x q[0], q[1];
+    """
+
+    expected_qasm3_str = """
+    OPENQASM 3.0;
+    qubit[3] __PYQASM_QUBITS__;
+    include "stdgates.inc";
+    x __PYQASM_QUBITS__[0];
+    cx __PYQASM_QUBITS__[0], __PYQASM_QUBITS__[1];
+    x __PYQASM_QUBITS__[0];
+    """
+
+    module = loads(qasm3_str)
+    module.unroll(consolidate_qubits=True)
+    check_unrolled_qasm(dumps(module), expected_qasm3_str)
+
+
 @pytest.mark.parametrize(
     "operation", ["crz(0.5) q[1], q[2];", "swap q[0], q[2];", "cz q[1], q[2];"]
 )