Fix internal transpiler error in CommutativeCancellation with classic…

…al conditions (#8556)

* Bug fix related to issue 8553

* Shrink regression test to minimal reproducer

* Correct comments after condition change

Co-authored-by: Jake Lishman <jake.lishman@ibm.com>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

qiskit/circuit/commutation_checker.py

@@ -80,6 +80,11 @@ def commute(
         Returns:
             bool: whether two operations commute.
         """
+        # We don't support commutation of conditional gates for now due to bugs in
+        # CommutativeCancellation.  See gh-8553.
+        if getattr(op1, "condition") is not None or getattr(op2, "condition") is not None:
+            return False
+
         # These lines are adapted from dag_dependency and say that two gates over
         # different quantum and classical bits necessarily commute. This is more
         # permissive that the check from commutation_analysis, as for example it
@@ -91,15 +96,13 @@ def commute(
         if not (intersection_q or intersection_c):
             return True
 
-        # These lines are adapted from commutation_analysis, which is more restrictive
-        # than the check from dag_dependency when considering nodes with "_directive"
-        # or "condition". It would be nice to think which optimizations
-        # from dag_dependency can indeed be used.
+        # These lines are adapted from commutation_analysis, which is more restrictive than the
+        # check from dag_dependency when considering nodes with "_directive".  It would be nice to
+        # think which optimizations from dag_dependency can indeed be used.
         for op in [op1, op2]:
             if (
                 getattr(op, "_directive", False)
                 or op.name in {"measure", "reset", "delay"}
-                or getattr(op, "condition", None)
                 or op.is_parameterized()
             ):
                 return False

test/python/circuit/test_commutation_checker.py

@@ -311,15 +311,13 @@ def test_conditional_gates(self):
         qr = QuantumRegister(3)
         cr = ClassicalRegister(2)
 
-        # Different quantum bits (and empty classical bits).
-        # We should be able to swap these.
+        # Currently, in all cases commutativity checker should returns False.
+        # This is definitely suboptimal.
         res = comm_checker.commute(
             CXGate().c_if(cr[0], 0), [qr[0], qr[1]], [], XGate(), [qr[2]], []
         )
-        self.assertTrue(res)
+        self.assertFalse(res)
 
-        # In all other cases, commutativity checker currently returns False.
-        # This is definitely suboptimal.
         res = comm_checker.commute(
             CXGate().c_if(cr[0], 0), [qr[0], qr[1]], [], XGate(), [qr[1]], []
         )

test/python/transpiler/test_commutative_cancellation.py

@@ -621,6 +621,17 @@ def test_basis_global_phase_03(self):
         ccirc = passmanager.run(circ)
         self.assertEqual(Operator(circ), Operator(ccirc))
 
+    def test_basic_classical_wires(self):
+        """Test that transpile runs without internal errors when dealing with commutable operations
+        with classical controls. Regression test for gh-8553."""
+        original = QuantumCircuit(2, 1)
+        original.x(0).c_if(original.cregs[0], 0)
+        original.x(1).c_if(original.cregs[0], 0)
+        # This transpilation shouldn't change anything, but it should succeed.  At one point it was
+        # triggering an internal logic error and crashing.
+        transpiled = PassManager([CommutativeCancellation()]).run(original)
+        self.assertEqual(original, transpiled)
+
 
 if __name__ == "__main__":
     unittest.main()