Restrict unsafe blocks to single statements where possible.

crates/cext/src/circuit.rs

@@ -16,6 +16,7 @@ use crate::circuit_library::pbc::{CPauliProductMeasurement, CPauliProductRotatio
 use crate::dag::COperationKind;
 use crate::exit_codes::ExitCode;
 use crate::pointers::{const_ptr_as_ref, mut_ptr_as_ref};
+use crate::transpiler::target::parse_params;
 
 use nalgebra::{Matrix2, Matrix4};
 use ndarray::{Array2, ArrayView2};
@@ -449,53 +450,14 @@ pub unsafe extern "C" fn qk_circuit_gate(
 ) -> ExitCode {
     // SAFETY: Per documentation, the pointer is non-null and aligned.
     let circuit = unsafe { mut_ptr_as_ref(circuit) };
-    // SAFETY: Per the documentation the qubits and params pointers are arrays of num_qubits()
-    // and num_params() elements respectively.
-    unsafe {
-        let qargs: &[Qubit] = match gate.num_qubits() {
-            0 => &[],
-            1 => &[Qubit(*qubits.wrapping_add(0))],
-            2 => &[
-                Qubit(*qubits.wrapping_add(0)),
-                Qubit(*qubits.wrapping_add(1)),
-            ],
-            3 => &[
-                Qubit(*qubits.wrapping_add(0)),
-                Qubit(*qubits.wrapping_add(1)),
-                Qubit(*qubits.wrapping_add(2)),
-            ],
-            4 => &[
-                Qubit(*qubits.wrapping_add(0)),
-                Qubit(*qubits.wrapping_add(1)),
-                Qubit(*qubits.wrapping_add(2)),
-                Qubit(*qubits.wrapping_add(3)),
-            ],
-            // There are no ``QkGate``s > 4 qubits
-            _ => unreachable!(),
-        };
-        let params: &[Param] = match gate.num_params() {
-            0 => &[],
-            1 => &[(*params.wrapping_add(0)).into()],
-            2 => &[
-                (*params.wrapping_add(0)).into(),
-                (*params.wrapping_add(1)).into(),
-            ],
-            3 => &[
-                (*params.wrapping_add(0)).into(),
-                (*params.wrapping_add(1)).into(),
-                (*params.wrapping_add(2)).into(),
-            ],
-            4 => &[
-                (*params.wrapping_add(0)).into(),
-                (*params.wrapping_add(1)).into(),
-                (*params.wrapping_add(2)).into(),
-                (*params.wrapping_add(3)).into(),
-            ],
-            // There are no ``QkGate``s that take > 4 params
-            _ => unreachable!(),
-        };
-        circuit.push_standard_gate(gate, params, qargs).unwrap()
-    }
+    // SAFETY: Per documentation, qubits is readable for num_qubits elements of type u32
+    let qargs: &[Qubit] =
+        unsafe { ::std::slice::from_raw_parts(qubits as *const Qubit, gate.num_qubits() as usize) };
+
+    // SAFETY: Per documentation, the params points is compatible with the gate and safe to read.
+    let params = unsafe { parse_params(gate, params) };
+
+    circuit.push_standard_gate(gate, &params, qargs).unwrap();
     ExitCode::Success
 }
 
@@ -1442,35 +1404,34 @@ pub unsafe extern "C" fn qk_circuit_inst_pauli_product_measurement(
 #[unsafe(no_mangle)]
 pub unsafe extern "C" fn qk_circuit_instruction_clear(inst: *mut CInstruction) {
     // SAFETY: Loading the data from pointers contained in a CInstruction. These should only be
-    // created by rust code and are constructed from Vecs internally or CStrings.
-    unsafe {
-        let inst = mut_ptr_as_ref(inst);
-        if inst.num_qubits > 0 && !inst.qubits.is_null() {
-            let qubits = std::slice::from_raw_parts_mut(inst.qubits, inst.num_qubits as usize);
-            let _: Box<[u32]> = Box::from_raw(qubits as *mut [u32]);
-            inst.qubits = std::ptr::null_mut();
-        }
-        inst.num_qubits = 0;
-        if inst.num_clbits > 0 && !inst.clbits.is_null() {
-            let clbits = std::slice::from_raw_parts_mut(inst.clbits, inst.num_clbits as usize);
-            let _: Box<[u32]> = Box::from_raw(clbits as *mut [u32]);
-            inst.clbits = std::ptr::null_mut();
-        }
-        inst.num_clbits = 0;
-        if inst.num_params > 0 && !inst.params.is_null() {
-            let params = std::slice::from_raw_parts_mut(inst.params, inst.num_params as usize);
-            for param in params.iter() {
-                let _ = Box::from_raw(*param);
-            }
-            let _ = Box::from_raw(params);
-            inst.params = std::ptr::null_mut();
-        }
-        inst.num_params = 0;
-        if !inst.name.is_null() {
-            let _ = CString::from_raw(inst.name);
-            inst.name = std::ptr::null_mut();
+    // created by Rust code and are constructed from Vecs internally or CStrings.
+    // This safety comment holds for all unsafe instructions in this function.
+    let inst = unsafe { mut_ptr_as_ref(inst) };
+    if inst.num_qubits > 0 && !inst.qubits.is_null() {
+        let qubits =
+            unsafe { std::slice::from_raw_parts_mut(inst.qubits, inst.num_qubits as usize) };
+        let _: Box<[u32]> = unsafe { Box::from_raw(qubits as *mut [u32]) };
+        inst.qubits = std::ptr::null_mut();
+    }
+    inst.num_qubits = 0;
+    if inst.num_clbits > 0 && !inst.clbits.is_null() {
+        let clbits =
+            unsafe { std::slice::from_raw_parts_mut(inst.clbits, inst.num_clbits as usize) };
+        let _: Box<[u32]> = unsafe { Box::from_raw(clbits as *mut [u32]) };
+        inst.clbits = std::ptr::null_mut();
+    }
+    inst.num_clbits = 0;
+    if inst.num_params > 0 && !inst.params.is_null() {
+        let params =
+            unsafe { std::slice::from_raw_parts_mut(inst.params, inst.num_params as usize) };
+        for param in params.iter() {
+            let _ = unsafe { Box::from_raw(*param) };
         }
+        let _ = unsafe { Box::from_raw(params) };
+        inst.params = std::ptr::null_mut();
     }
+    inst.num_params = 0;
+    inst.name = std::ptr::null_mut();
 }
 
 /// @ingroup QkCircuit
@@ -1489,19 +1450,20 @@ pub unsafe extern "C" fn qk_opcounts_clear(op_counts: *mut OpCounts) {
     if op_counts.len > 0 && !op_counts.data.is_null() {
         // SAFETY: We load the box from a slice pointer created from
         // the raw parts from the OpCounts::data attribute.
-        unsafe {
-            let slice: Box<[OpCount]> = Box::from_raw(std::ptr::slice_from_raw_parts_mut(
+        let slice: Box<[OpCount]> = unsafe {
+            Box::from_raw(std::ptr::slice_from_raw_parts_mut(
                 op_counts.data,
                 op_counts.len,
-            ));
-            // free the allocated strings in each OpCount
-            for count in slice.iter() {
-                if !count.name.is_null() {
-                    let _ = CString::from_raw(count.name as *mut c_char);
-                }
+            ))
+        };
+        // free the allocated strings in each OpCount
+        for count in slice.iter() {
+            if !count.name.is_null() {
+                // SAFETY: Rust constructed this string, so we are fine to free it here
+                let _ = unsafe { CString::from_raw(count.name as *mut c_char) };
             }
-            // the variable vec goes out of bounds and is freed too
         }
+        // the variable vec goes out of bounds and is freed too
     }
     op_counts.len = 0;
     op_counts.data = std::ptr::null_mut();

crates/cext/src/circuit_library/pbc.rs

@@ -74,6 +74,7 @@ pub unsafe extern "C" fn qk_pauli_product_rotation_clear(inst: *mut CPauliProduc
 
     // SAFETY: The user guarantees the instruction is coherent, i.e. the Z and X arrays are
     // readable for the correct length and `angle` is a valid, non-null Param pointer.
+    // Note that every single line here is unsafe, hence we wrap it in a single block.
     unsafe {
         let x = ::std::slice::from_raw_parts_mut(inst.x, inst.len);
         let _: Box<[bool]> = Box::from_raw(x);
@@ -152,6 +153,7 @@ pub unsafe extern "C" fn qk_pauli_product_measurement_clear(inst: *mut CPauliPro
 
     // SAFETY: The user guarantees the instruction is coherent, i.e. the Z and X arrays are
     // readable for the correct length and `angle` is a valid, non-null Param pointer.
+    // Note that every single line here is unsafe, hence we wrap it in a single block.
     unsafe {
         let x = ::std::slice::from_raw_parts_mut(inst.x, inst.len);
         let _: Box<[bool]> = Box::from_raw(x);

crates/cext/src/dag.rs

@@ -13,9 +13,10 @@
 use anyhow::Error;
 use hashbrown::HashMap;
 use num_complex::Complex64;
-use smallvec::smallvec;
+use smallvec::SmallVec;
 
 use crate::exit_codes::ExitCode;
+use crate::transpiler::target::parse_params;
 use qiskit_circuit::bit::{ClassicalRegister, QuantumRegister};
 use qiskit_circuit::circuit_data::CircuitData;
 use qiskit_circuit::dag_circuit::{DAGCircuit, NodeIndex, NodeType};
@@ -503,76 +504,39 @@ pub unsafe extern "C" fn qk_dag_apply_gate(
 ) -> u32 {
     // SAFETY: Per documentation, the pointer is to valid data.
     let dag = unsafe { mut_ptr_as_ref(dag) };
-    // SAFETY: Per the documentation the qubits and params pointers are arrays of num_qubits()
-    // and num_params() elements respectively.
-    unsafe {
-        let qargs: &[Qubit] = match gate.num_qubits() {
-            0 => &[],
-            1 => &[Qubit(*qubits.wrapping_add(0))],
-            2 => &[
-                Qubit(*qubits.wrapping_add(0)),
-                Qubit(*qubits.wrapping_add(1)),
-            ],
-            3 => &[
-                Qubit(*qubits.wrapping_add(0)),
-                Qubit(*qubits.wrapping_add(1)),
-                Qubit(*qubits.wrapping_add(2)),
-            ],
-            4 => &[
-                Qubit(*qubits.wrapping_add(0)),
-                Qubit(*qubits.wrapping_add(1)),
-                Qubit(*qubits.wrapping_add(2)),
-                Qubit(*qubits.wrapping_add(3)),
-            ],
-            // There are no ``QkGate``s > 4 qubits
-            _ => panic!(),
-        };
-        let params = match gate.num_params() {
-            0 => None,
-            1 => Some(smallvec![(*params.wrapping_add(0)).into()]),
-            2 => Some(smallvec![
-                (*params.wrapping_add(0)).into(),
-                (*params.wrapping_add(1)).into(),
-            ]),
-            3 => Some(smallvec![
-                (*params.wrapping_add(0)).into(),
-                (*params.wrapping_add(1)).into(),
-                (*params.wrapping_add(2)).into(),
-            ]),
-            4 => Some(smallvec![
-                (*params.wrapping_add(0)).into(),
-                (*params.wrapping_add(1)).into(),
-                (*params.wrapping_add(2)).into(),
-                (*params.wrapping_add(3)).into(),
-            ]),
-            // There are no ``QkGate``s that take > 4 params
-            _ => panic!(),
-        };
-        let new_node = if front {
-            dag.apply_operation_front(
-                gate.into(),
-                qargs,
-                &[],
-                params.map(Parameters::Params),
-                None,
-                #[cfg(feature = "cache_pygates")]
-                None,
-            )
-            .unwrap()
-        } else {
-            dag.apply_operation_back(
-                gate.into(),
-                qargs,
-                &[],
-                params.map(Parameters::Params),
-                None,
-                #[cfg(feature = "cache_pygates")]
-                None,
-            )
-            .unwrap()
-        };
-        new_node.index() as u32
-    }
+    // SAFETY: Per the documentation the qubits pointer is an arrays of num_qubits() elements
+    let qargs: &[Qubit] =
+        unsafe { ::std::slice::from_raw_parts(qubits as *const Qubit, gate.num_qubits() as usize) };
+    let params: Option<SmallVec<[Param; 3]>> = if gate.num_params() == 0 {
+        None
+    } else {
+        // SAFETY: Per the documentation, params is readable for num_params elements of f64
+        Some(unsafe { parse_params(gate, params) })
+    };
+    let new_node = if front {
+        dag.apply_operation_front(
+            gate.into(),
+            qargs,
+            &[],
+            params.map(Parameters::Params),
+            None,
+            #[cfg(feature = "cache_pygates")]
+            None,
+        )
+        .unwrap()
+    } else {
+        dag.apply_operation_back(
+            gate.into(),
+            qargs,
+            &[],
+            params.map(Parameters::Params),
+            None,
+            #[cfg(feature = "cache_pygates")]
+            None,
+        )
+        .unwrap()
+    };
+    new_node.index() as u32
 }
 
 /// @ingroup QkDag

crates/cext/src/transpiler/neighbors.rs

@@ -69,7 +69,8 @@ pub struct CNeighbors {
 #[unsafe(no_mangle)]
 pub unsafe extern "C" fn qk_neighbors_is_all_to_all(neighbors: *const CNeighbors) -> bool {
     // SAFETY: per documentation, `neighbors` points to a valid initialized `CNeighbors`.
-    unsafe { (*neighbors).neighbors.is_null() && (*neighbors).partition.is_null() }
+    let neighbors = unsafe { const_ptr_as_ref(neighbors) };
+    neighbors.neighbors.is_null() && neighbors.partition.is_null()
 }
 
 /// @ingroup QkNeighbors

crates/cext/src/transpiler/passes/consolidate_blocks.rs

@@ -39,13 +39,13 @@ pub unsafe extern "C" fn qk_transpiler_pass_standalone_consolidate_blocks(
     approximation_degree: f64,
     force_consolidate: bool,
 ) {
+    // SAFETY: Per documentation, the pointer is not null and aligned.
     let circuit = unsafe { mut_ptr_as_ref(circuit) };
-    let target = unsafe {
-        if target.is_null() {
-            None
-        } else {
-            Some(const_ptr_as_ref(target))
-        }
+    let target = if target.is_null() {
+        None
+    } else {
+        // SAFETY: Per documentation, the pointer is not null and aligned.
+        Some(unsafe { const_ptr_as_ref(target) })
     };
     let approximation_degree = if approximation_degree.is_nan() {
         1.0

crates/cext/src/transpiler/passes/optimize_1q_sequences.rs

@@ -40,6 +40,7 @@ pub unsafe extern "C" fn qk_transpiler_standalone_optimize_1q_sequences(
     circuit: *mut CircuitData,
     target: *const Target,
 ) {
+    // SAFETY: Per documentation, the pointers are valid and non-null.
     unsafe { qk_transpiler_pass_standalone_optimize_1q_sequences(circuit, target) };
 }
 
@@ -63,13 +64,11 @@ pub unsafe extern "C" fn qk_transpiler_pass_standalone_optimize_1q_sequences(
     circuit: *mut CircuitData,
     target: *const Target,
 ) {
-    // SAFETY: Per documentation, the pointer is non-null and aligned.
-    let target = unsafe {
-        if target.is_null() {
-            None
-        } else {
-            Some(const_ptr_as_ref(target))
-        }
+    let target = if target.is_null() {
+        None
+    } else {
+        // SAFETY: Per documentation, the pointer is non-null and aligned.
+        Some(unsafe { const_ptr_as_ref(target) })
     };
     // SAFETY: Per documentation, the pointer is non-null and aligned.
     let circuit = unsafe { mut_ptr_as_ref(circuit) };
@@ -146,12 +145,10 @@ pub unsafe extern "C" fn qk_transpiler_pass_optimize_1q_sequences(
     target: *const Target,
 ) {
     // SAFETY: Per documentation, the pointer is non-null and aligned.
-    let target = unsafe {
-        if target.is_null() {
-            None
-        } else {
-            Some(const_ptr_as_ref(target))
-        }
+    let target = if target.is_null() {
+        None
+    } else {
+        Some(unsafe { const_ptr_as_ref(target) })
     };
     // SAFETY: Per documentation, the pointer is non-null and aligned.
     let dag = unsafe { mut_ptr_as_ref(dag) };