Fail gracefully on bad SabreDAG construction (Qiskit#10724)

* Fail gracefully on bad `SabreDAG` construction

This is a private, internal Rust type, but it doesn't cost us anything
(meaningful) to bounds-check the accessors and ensure we fail gracefully
rather than panicking if we ever make a mistake in Python space. This
more faithfully fulfills the return value of `SabreDAG::new`, which
previously was an effectively infallible `Result`.

* Run `cargo fmt`

.azure/test-linux.yml

@@ -34,6 +34,7 @@ jobs:
       - task: UsePythonVersion@0
         inputs:
           versionSpec: '${{ parameters.pythonVersion }}'
+        name: 'usePython'
         displayName: 'Use Python ${{ parameters.pythonVersion }}'
 
       - task: Cache@2
@@ -47,9 +48,14 @@ jobs:
         displayName: "Cache stestr"
 
       - ${{ if eq(parameters.testRust, true) }}:
-        - bash: |
-            set -e
-            cargo test
+        # We need to avoid linking our crates into full Python extension libraries during Rust-only
+        # testing because Rust/PyO3 can't handle finding a static CPython interpreter.
+        - bash: cargo test --no-default-features
+          env:
+            # On Linux we link against `libpython` dynamically, but it isn't written into the rpath
+            # of the test executable (I'm not 100% sure why ---Jake).  It's easiest just to forcibly
+            # include the correct place in the `dlopen` search path.
+            LD_LIBRARY_PATH: '$(usePython.pythonLocation)/lib:$LD_LIBRARY_PATH'
           displayName: "Run Rust tests"
 
       - bash: |

Cargo.toml

@@ -8,7 +8,9 @@ rust-version = "1.64"  # Keep in sync with README.md and rust-toolchain.toml.
 license = "Apache-2.0"
 
 [workspace.dependencies]
-pyo3 = { version = "0.19.2", features = ["extension-module", "abi3-py38"] }
+# This doesn't set `extension-module` as a shared feature because we need to be able to disable it
+# during Rust-only testing (see # https://github.com/PyO3/pyo3/issues/340).
+pyo3 = { version = "0.19.2", features = ["abi3-py38"] }
 
 [profile.release]
 lto = 'fat'

crates/accelerate/Cargo.toml

@@ -9,6 +9,11 @@ license.workspace = true
 name = "qiskit_accelerate"
 crate-type = ["cdylib"]
 
+[features]
+# This is a test-only shim removable feature.  See the root `Cargo.toml`.
+default = ["extension-module"]
+extension-module = ["pyo3/extension-module"]
+
 [dependencies]
 rayon = "1.7"
 numpy = "0.19.0"

crates/accelerate/src/sabre_layout.rs

@@ -144,7 +144,8 @@ fn layout_trial(
         dag_no_control_forward.num_clbits,
         dag_no_control_forward.nodes.iter().rev().cloned().collect(),
         dag_no_control_forward.node_blocks.clone(),
-    );
+    )
+    .unwrap();
 
     for _iter in 0..max_iterations {
         for dag in [&dag_no_control_forward, &dag_no_control_reverse] {

crates/accelerate/src/sabre_swap/sabre_dag.rs

@@ -12,6 +12,7 @@
 
 use hashbrown::HashMap;
 use hashbrown::HashSet;
+use pyo3::exceptions::PyIndexError;
 use pyo3::prelude::*;
 use rustworkx_core::petgraph::prelude::*;
 
@@ -51,7 +52,7 @@ impl SabreDAG {
         num_clbits: usize,
         nodes: Vec<(usize, Vec<usize>, HashSet<usize>)>,
         node_blocks: HashMap<usize, Vec<SabreDAG>>,
-    ) -> Self {
+    ) -> PyResult<Self> {
         let mut qubit_pos: Vec<Option<NodeIndex>> = vec![None; num_qubits];
         let mut clbit_pos: Vec<Option<NodeIndex>> = vec![None; num_clbits];
         let mut dag = DiGraph::with_capacity(nodes.len(), 2 * nodes.len());
@@ -65,30 +66,58 @@ impl SabreDAG {
             });
             let mut is_front = true;
             for x in qargs {
-                if let Some(predecessor) = qubit_pos[*x] {
+                let pos = qubit_pos.get_mut(*x).ok_or_else(|| {
+                    PyIndexError::new_err(format!(
+                        "qubit index {} is out of range for {} qubits",
+                        *x, num_qubits
+                    ))
+                })?;
+                if let Some(predecessor) = *pos {
                     is_front = false;
                     dag.add_edge(predecessor, gate_index, ());
                 }
-                qubit_pos[*x] = Some(gate_index);
+                *pos = Some(gate_index);
             }
             for x in cargs {
-                if let Some(predecessor) = clbit_pos[*x] {
+                let pos = clbit_pos.get_mut(*x).ok_or_else(|| {
+                    PyIndexError::new_err(format!(
+                        "clbit index {} is out of range for {} clbits",
+                        *x, num_qubits
+                    ))
+                })?;
+                if let Some(predecessor) = *pos {
                     is_front = false;
                     dag.add_edge(predecessor, gate_index, ());
                 }
-                clbit_pos[*x] = Some(gate_index);
+                *pos = Some(gate_index);
             }
             if is_front {
                 first_layer.push(gate_index);
             }
         }
-        SabreDAG {
+        Ok(SabreDAG {
             num_qubits,
             num_clbits,
             dag,
             first_layer,
             nodes,
             node_blocks,
-        }
+        })
+    }
+}
+
+#[cfg(test)]
+mod test {
+    use super::SabreDAG;
+    use hashbrown::{HashMap, HashSet};
+
+    #[test]
+    fn no_panic_on_bad_qubits() {
+        assert!(SabreDAG::new(2, 0, vec![(0, vec![0, 2], HashSet::new())], HashMap::new()).is_err())
+    }
+
+    #[test]
+    fn no_panic_on_bad_clbits() {
+        assert!(SabreDAG::new(2, 1, vec![(0, vec![0, 1], [0, 1].into())], HashMap::new()).is_err())
     }
 }

crates/qasm2/Cargo.toml

@@ -9,6 +9,11 @@ license.workspace = true
 name = "qiskit_qasm2"
 crate-type = ["cdylib"]
 
+[features]
+# This is a test-only shim removable feature.  See the root `Cargo.toml`.
+default = ["extension-module"]
+extension-module = ["pyo3/extension-module"]
+
 [dependencies]
 hashbrown = "0.14.0"
 pyo3.workspace = true