Correctly error on incorrect clbits in QuantumCircuit.append (Qiski…

…t#9386)

* Correctly error on incorrect clbits in `QuantumCircuit.append`

Previously, the clbits were unchecked, which could allow incorrect use
of `append` that might later explode in an unrelated place.  Making this
change has turned up a few places in the Terra tests that also build
invalid assumptions.

* Fix incorrect tests

* Fix lint

* Apply suggestions from code review

Co-authored-by: Julien Gacon <gaconju@gmail.com>

---------

Co-authored-by: Julien Gacon <gaconju@gmail.com>

qiskit/circuit/instruction.py

@@ -481,6 +481,11 @@ def broadcast_arguments(self, qargs, cargs):
                 f"The amount of qubit arguments {len(qargs)} does not match"
                 f" the instruction expectation ({self.num_qubits})."
             )
+        if len(cargs) != self.num_clbits:
+            raise CircuitError(
+                f"The amount of clbit arguments {len(cargs)} does not match"
+                f" the instruction expectation ({self.num_clbits})."
+            )
 
         #  [[q[0], q[1]], [c[0], c[1]]] -> [q[0], c[0]], [q[1], c[1]]
         flat_qargs = [qarg for sublist in qargs for qarg in sublist]

releasenotes/notes/append-bad-argument-error-cc9eafe94cc39033.yaml

@@ -0,0 +1,5 @@
+---
+fixes:
+  - |
+    :meth:`.QuantumCircuit.append` will now correctly raise an error if given an incorrect number of
+    classical bits to apply to an operation.  Fix `#9385 <https://github.com/Qiskit/qiskit-terra/issues/9385>`__.

test/python/circuit/test_circuit_operations.py

@@ -147,6 +147,16 @@ def test_append_rejects_wrong_types(self, specifier):
         with self.subTest("c list"), self.assertRaisesRegex(CircuitError, "Invalid bit index"):
             test.append(opaque, [[0]], [[specifier]])
 
+    @data([], [0], [0, 1, 2])
+    def test_append_rejects_bad_arguments_opaque(self, bad_arg):
+        """Test that a suitable exception is raised when there is an argument mismatch."""
+        inst = QuantumCircuit(2, 2).to_instruction()
+        qc = QuantumCircuit(3, 3)
+        with self.assertRaisesRegex(CircuitError, "The amount of qubit arguments"):
+            qc.append(inst, bad_arg, [0, 1])
+        with self.assertRaisesRegex(CircuitError, "The amount of clbit arguments"):
+            qc.append(inst, [0, 1], bad_arg)
+
     def test_anding_self(self):
         """Test that qc &= qc finishes, which can be prone to infinite while-loops.
 
@@ -1123,9 +1133,9 @@ def test_from_instructions(self):
         a, b, c, d = qreg
         x, y, z = creg
 
-        circuit_1 = QuantumCircuit(2)
+        circuit_1 = QuantumCircuit(2, 1)
         circuit_1.x(0)
-        circuit_2 = QuantumCircuit(2)
+        circuit_2 = QuantumCircuit(2, 1)
         circuit_2.y(0)
 
         def instructions():

test/python/circuit/test_control_flow_builders.py

@@ -91,7 +91,7 @@ def test_if_register(self):
 
         expected = QuantumCircuit(qr, cr)
         expected.measure(qr, cr)
-        expected.if_test((cr, 0), if_true0, [qr[0]], [cr[:]])
+        expected.if_test((cr, 0), if_true0, [qr[0]], cr)
 
         self.assertEqual(canonicalize_control_flow(test), canonicalize_control_flow(expected))
 

test/python/transpiler/test_check_map.py

@@ -132,7 +132,7 @@ def test_swap_mapped_cf_true(self):
         qr = QuantumRegister(3)
         cr = ClassicalRegister(3)
         circuit = QuantumCircuit(qr, cr)
-        true_body = QuantumCircuit(qr)
+        true_body = QuantumCircuit(qr, cr)
         true_body.swap(0, 1)
         true_body.cx(2, 1)
         circuit.if_else((cr[0], 0), true_body, None, qr, cr)
@@ -150,7 +150,7 @@ def test_swap_mapped_cf_false(self):
         circuit = QuantumCircuit(qr, cr)
         true_body = QuantumCircuit(qr)
         true_body.cx(0, 2)
-        circuit.if_else((cr[0], 0), true_body, None, qr, cr)
+        circuit.if_else((cr[0], 0), true_body, None, qr, [])
         dag = circuit_to_dag(circuit)
         pass_ = CheckMap(coupling)
         pass_.run(dag)
@@ -163,7 +163,7 @@ def test_swap_mapped_cf_layout_change_false(self):
         qr = QuantumRegister(3)
         cr = ClassicalRegister(3)
         circuit = QuantumCircuit(qr, cr)
-        true_body = QuantumCircuit(qr)
+        true_body = QuantumCircuit(qr, cr)
         true_body.cx(1, 2)
         circuit.if_else((cr[0], 0), true_body, None, qr[[1, 0, 2]], cr)
         dag = circuit_to_dag(circuit)
@@ -180,7 +180,7 @@ def test_swap_mapped_cf_layout_change_true(self):
         circuit = QuantumCircuit(qr, cr)
         true_body = QuantumCircuit(qr)
         true_body.cx(0, 2)
-        circuit.if_else((cr[0], 0), true_body, None, qr[[1, 0, 2]], cr)
+        circuit.if_else((cr[0], 0), true_body, None, qr[[1, 0, 2]], [])
         dag = circuit_to_dag(circuit)
         pass_ = CheckMap(coupling)
         pass_.run(dag)
@@ -193,9 +193,9 @@ def test_swap_mapped_cf_different_bits(self):
         qr = QuantumRegister(3)
         cr = ClassicalRegister(3)
         circuit = QuantumCircuit(qr, cr)
-        true_body = QuantumCircuit(3)
+        true_body = QuantumCircuit(3, 1)
         true_body.cx(0, 2)
-        circuit.if_else((cr[0], 0), true_body, None, qr[[1, 0, 2]], cr)
+        circuit.if_else((cr[0], 0), true_body, None, qr[[1, 0, 2]], [cr[0]])
         dag = circuit_to_dag(circuit)
         pass_ = CheckMap(coupling)
         pass_.run(dag)
@@ -209,9 +209,9 @@ def test_disjoint_controlflow_bits(self):
         qr2 = QuantumRegister(3, "qrif")
         cr = ClassicalRegister(3)
         circuit = QuantumCircuit(qr1, cr)
-        true_body = QuantumCircuit(qr2)
+        true_body = QuantumCircuit(qr2, [cr[0]])
         true_body.cx(0, 2)
-        circuit.if_else((cr[0], 0), true_body, None, qr1[[1, 0, 2]], cr)
+        circuit.if_else((cr[0], 0), true_body, None, qr1[[1, 0, 2]], [cr[0]])
         dag = circuit_to_dag(circuit)
         pass_ = CheckMap(coupling)
         pass_.run(dag)
@@ -229,7 +229,7 @@ def test_nested_controlflow_true(self):
         true_body = QuantumCircuit(qr2, cr2)
         for_body = QuantumCircuit(3)
         for_body.cx(0, 2)
-        true_body.for_loop(range(5), body=for_body, qubits=qr2, clbits=cr2)
+        true_body.for_loop(range(5), body=for_body, qubits=qr2, clbits=[])
         circuit.if_else((cr1[0], 0), true_body, None, qr1[[1, 0, 2]], cr1)
         dag = circuit_to_dag(circuit)
         pass_ = CheckMap(coupling)
@@ -248,7 +248,7 @@ def test_nested_controlflow_false(self):
         true_body = QuantumCircuit(qr2, cr2)
         for_body = QuantumCircuit(3)
         for_body.cx(0, 2)
-        true_body.for_loop(range(5), body=for_body, qubits=qr2, clbits=cr2)
+        true_body.for_loop(range(5), body=for_body, qubits=qr2, clbits=[])
         circuit.if_else((cr1[0], 0), true_body, None, qr1[[0, 1, 2]], cr1)
         dag = circuit_to_dag(circuit)
         pass_ = CheckMap(coupling)

test/python/transpiler/test_clifford_passes.py

@@ -244,18 +244,12 @@ def test_if_else(self):
 
         test = QuantumCircuit(cliff1.num_qubits, 1)
         test.measure(0, 0)
-        test.if_else(
-            (test.clbits[0], True), inner_test.copy(), inner_test.copy(), test.qubits, test.clbits
-        )
+        test.if_else((test.clbits[0], True), inner_test.copy(), inner_test.copy(), test.qubits, [])
 
         expected = QuantumCircuit(combined.num_qubits, 1)
         expected.measure(0, 0)
         expected.if_else(
-            (expected.clbits[0], True),
-            inner_expected,
-            inner_expected,
-            expected.qubits,
-            expected.clbits,
+            (expected.clbits[0], True), inner_expected, inner_expected, expected.qubits, []
         )
 
         self.assertEqual(OptimizeCliffords()(test), expected)

test/python/transpiler/test_linear_functions_passes.py

@@ -417,7 +417,7 @@ def test_if_else(self, do_commutative_analysis):
         """Test that collection recurses into a simple if-else."""
         pass_ = CollectLinearFunctions(do_commutative_analysis=do_commutative_analysis)
 
-        circuit = QuantumCircuit(4)
+        circuit = QuantumCircuit(4, 1)
         circuit.cx(0, 1)
         circuit.cx(1, 0)
         circuit.cx(2, 3)

test/python/transpiler/test_optimize_1q_decomposition.py

@@ -684,7 +684,7 @@ def test_if_else(self):
         test = QuantumCircuit(qr, cr)
         test.h(0)
         test.measure(0, 0)
-        test_true = QuantumCircuit(qr)
+        test_true = QuantumCircuit(qr, cr)
         test_true.h(qr[0])
         test_true.h(qr[0])
         test_true.h(qr[0])
@@ -693,7 +693,7 @@ def test_if_else(self):
         expected = QuantumCircuit(qr, cr)
         expected.u(np.pi / 2, 0, -np.pi, 0)
         expected.measure(0, 0)
-        expected_true = QuantumCircuit(qr)
+        expected_true = QuantumCircuit(qr, cr)
         expected_true.u(np.pi / 2, 0, -np.pi, qr[0])
         expected.if_else((0, True), expected_true, None, range(num_qubits), [0])
 

test/python/transpiler/test_optimize_swap_before_measure.py

@@ -199,10 +199,12 @@ def test_nested_control_flow(self):
         base_expected.measure(1, 0)
 
         body_test = QuantumCircuit(2, 1)
-        body_test.for_loop((0,), None, base_expected.copy(), body_test.qubits, [])
+        body_test.for_loop((0,), None, base_expected.copy(), body_test.qubits, body_test.clbits)
 
         body_expected = QuantumCircuit(2, 1)
-        body_expected.for_loop((0,), None, base_expected.copy(), body_expected.qubits, [])
+        body_expected.for_loop(
+            (0,), None, base_expected.copy(), body_expected.qubits, body_expected.clbits
+        )
 
         test = QuantumCircuit(2, 1)
         test.while_loop((test.clbits[0], True), body_test, test.qubits, test.clbits)

test/python/transpiler/test_remove_barriers.py

@@ -91,10 +91,12 @@ def test_nested_control_flow(self):
         base_expected.measure(0, 0)
 
         body_test = QuantumCircuit(1, 1)
-        body_test.for_loop((0,), None, base_expected.copy(), body_test.qubits, [])
+        body_test.for_loop((0,), None, base_expected.copy(), body_test.qubits, body_test.clbits)
 
         body_expected = QuantumCircuit(1, 1)
-        body_expected.for_loop((0,), None, base_expected.copy(), body_expected.qubits, [])
+        body_expected.for_loop(
+            (0,), None, base_expected.copy(), body_expected.qubits, body_expected.clbits
+        )
 
         test = QuantumCircuit(1, 1)
         test.while_loop((test.clbits[0], True), body_test, test.qubits, test.clbits)

test/python/transpiler/test_remove_diagonal_gates_before_measure.py

@@ -251,10 +251,12 @@ def test_nested_control_flow(self):
         base_expected.measure(1, 0)
 
         body_test = QuantumCircuit(2, 1)
-        body_test.for_loop((0,), None, base_expected.copy(), body_test.qubits, [])
+        body_test.for_loop((0,), None, base_expected.copy(), body_test.qubits, body_test.clbits)
 
         body_expected = QuantumCircuit(2, 1)
-        body_expected.for_loop((0,), None, base_expected.copy(), body_expected.qubits, [])
+        body_expected.for_loop(
+            (0,), None, base_expected.copy(), body_expected.qubits, body_expected.clbits
+        )
 
         test = QuantumCircuit(2, 1)
         test.while_loop((test.clbits[0], True), body_test, test.qubits, test.clbits)

test/python/transpiler/test_reset_after_measure_simplification.py

@@ -180,10 +180,12 @@ def test_nested_control_flow(self):
         base_expected.x(0).c_if(0, True)
 
         body_test = QuantumCircuit(1, 1)
-        body_test.for_loop((0,), None, base_expected.copy(), body_test.qubits, [])
+        body_test.for_loop((0,), None, base_expected.copy(), body_test.qubits, body_test.clbits)
 
         body_expected = QuantumCircuit(1, 1)
-        body_expected.for_loop((0,), None, base_expected.copy(), body_expected.qubits, [])
+        body_expected.for_loop(
+            (0,), None, base_expected.copy(), body_expected.qubits, body_expected.clbits
+        )
 
         test = QuantumCircuit(1, 1)
         test.while_loop((test.clbits[0], True), body_test, test.qubits, test.clbits)

test/python/transpiler/test_unitary_synthesis.py

@@ -775,7 +775,7 @@ def test_if_simple(self):
         qc_true_body.unitary(qc_uni_mat, [0, 1])
 
         qc = QuantumCircuit(qr, cr)
-        qc.if_test((cr, 1), qc_true_body, [0, 1], [0, 1])
+        qc.if_test((cr, 1), qc_true_body, [0, 1], [])
         dag = circuit_to_dag(qc)
         cdag = UnitarySynthesis(basis_gates=basis_gates).run(dag)
         cqc = dag_to_circuit(cdag)

test/python/transpiler/test_unroll_custom_definitions.py

@@ -194,16 +194,16 @@ def test_nested_control_flow(self):
         while_body.append(TestCompositeGate(), [0], [])
 
         true_body = QuantumCircuit([qubit, clbit])
-        true_body.while_loop((clbit, True), while_body, [0], [0])
+        true_body.while_loop((clbit, True), while_body, [0], [])
 
         test = QuantumCircuit([qubit, clbit])
-        test.for_loop(range(2), None, for_body, [0], [0])
+        test.for_loop(range(2), None, for_body, [0], [])
         test.if_else((clbit, True), true_body, None, [0], [0])
 
         expected_if_body = QuantumCircuit([qubit, clbit])
-        expected_if_body.while_loop((clbit, True), pass_(while_body), [0], [0])
+        expected_if_body.while_loop((clbit, True), pass_(while_body), [0], [])
         expected = QuantumCircuit([qubit, clbit])
-        expected.for_loop(range(2), None, pass_(for_body), [0], [0])
+        expected.for_loop(range(2), None, pass_(for_body), [0], [])
         expected.if_else(range(2), pass_(expected_if_body), None, [0], [0])
 
         self.assertEqual(pass_(test), expected)