Make Qubit and Clbit inner type private.

Also removes From<u32> and Into<u32> implementations for bit types
(and DAGCircuit's Var type) to improve data encapsulation
outside of the circuit crate. In its place, we now have WireIndex
which is private to the circuit crate, which we implement for our
wire types and use as a bound for types used with BitData going
forward.

crates/accelerate/src/check_map.rs

@@ -24,16 +24,16 @@ use qiskit_circuit::Qubit;
 fn recurse<'py>(
     py: Python<'py>,
     dag: &'py DAGCircuit,
-    edge_set: &'py HashSet<[u32; 2]>,
+    edge_set: &'py HashSet<[usize; 2]>,
     wire_map: Option<&'py [Qubit]>,
-) -> PyResult<Option<(String, [u32; 2])>> {
+) -> PyResult<Option<(String, [usize; 2])>> {
     let check_qubits = |qubits: &[Qubit]| -> bool {
         match wire_map {
             Some(wire_map) => {
                 let mapped_bits = [wire_map[qubits[0].index()], wire_map[qubits[1].index()]];
-                edge_set.contains(&[mapped_bits[0].into(), mapped_bits[1].into()])
+                edge_set.contains(&[mapped_bits[0].index(), mapped_bits[1].index()])
             }
-            None => edge_set.contains(&[qubits[0].into(), qubits[1].into()]),
+            None => edge_set.contains(&[qubits[0].index(), qubits[1].index()]),
         }
     };
     for node in dag.op_nodes(false) {
@@ -72,10 +72,7 @@ fn recurse<'py>(
             {
                 return Ok(Some((
                     inst.op.name().to_string(),
-                    [
-                        qubits[0].index().try_into().unwrap(),
-                        qubits[1].index().try_into().unwrap(),
-                    ],
+                    [qubits[0].index(), qubits[1].index()],
                 )));
             }
         }
@@ -87,8 +84,8 @@ fn recurse<'py>(
 pub fn check_map(
     py: Python,
     dag: &DAGCircuit,
-    edge_set: HashSet<[u32; 2]>,
-) -> PyResult<Option<(String, [u32; 2])>> {
+    edge_set: HashSet<[usize; 2]>,
+) -> PyResult<Option<(String, [usize; 2])>> {
     recurse(py, dag, &edge_set, None)
 }
 

crates/circuit/src/bit_data.rs

@@ -10,7 +10,6 @@
 // copyright notice, and modified files need to carry a notice indicating
 // that they have been altered from the originals.
 
-use crate::BitType;
 use hashbrown::HashMap;
 use pyo3::exceptions::{PyKeyError, PyRuntimeError, PyValueError};
 use pyo3::prelude::*;
@@ -70,6 +69,26 @@ impl PartialEq for BitAsKey {
 
 impl Eq for BitAsKey {}
 
+/// An intentionally crate-private trait that must be implemented
+/// for types used with [BitData].
+///
+/// Because this is private to the crate, implementing it does
+/// not automatically make the implementing type convertible to
+/// and from a [usize]. This is handy for types like
+/// [crate::dagcircuit::Var] which are intended to serve as opaque
+/// keys to access variable data managed by a
+/// [crate::dagcircuit::DAGCircuit].
+pub(crate) trait WireIndex {
+    fn new(index: usize) -> Self;
+    fn index(&self) -> usize;
+}
+
+/// An internal structure used to track the mapping between
+/// wire indices and Python bits.
+///
+/// This is very much only intended as a short-term method of
+/// tracking Python bits, and should not be exposed beyond
+/// this crate.
 #[derive(Clone, Debug)]
 pub(crate) struct BitData<T> {
     /// The public field name (i.e. `qubits` or `clbits`).
@@ -84,8 +103,7 @@ pub(crate) struct BitData<T> {
 
 impl<T> BitData<T>
 where
-    T: From<BitType> + Copy,
-    BitType: From<T>,
+    T: WireIndex + Copy,
 {
     pub fn new(py: Python<'_>, description: String) -> Self {
         BitData {
@@ -96,7 +114,7 @@ where
         }
     }
 
-    pub fn with_capacity(py: Python<'_>, description: String, capacity: usize) -> Self {
+    pub fn with_capacity(py: Python, description: String, capacity: usize) -> Self {
         BitData {
             description,
             bits: Vec::with_capacity(capacity),
@@ -168,7 +186,7 @@ where
     /// bit index.
     #[inline]
     pub fn get(&self, index: T) -> Option<&PyObject> {
-        self.bits.get(<BitType as From<T>>::from(index) as usize)
+        self.bits.get(index.index())
     }
 
     /// Adds a new Python bit.
@@ -178,17 +196,8 @@ where
             format!("This circuit's {} list has become out of sync with the circuit data. Did something modify it?", self.description)
             ));
         }
-        let idx: BitType = self.bits.len().try_into().map_err(|_| {
-            PyRuntimeError::new_err(format!(
-                "The number of {} in the circuit has exceeded the maximum capacity",
-                self.description
-            ))
-        })?;
-        if self
-            .indices
-            .try_insert(BitAsKey::new(bit), idx.into())
-            .is_ok()
-        {
+        let idx = T::new(self.bits.len());
+        if self.indices.try_insert(BitAsKey::new(bit), idx).is_ok() {
             self.bits.push(bit.into_py(py));
             self.cached.bind(py).append(bit)?;
         } else if strict {
@@ -198,17 +207,14 @@ where
             )));
         }
         // TODO: looks like a bug where `idx` is wrong if not strict and already exists
-        Ok(idx.into())
+        Ok(idx)
     }
 
     pub fn remove_indices<I>(&mut self, py: Python, indices: I) -> PyResult<()>
     where
         I: IntoIterator<Item = T>,
     {
-        let mut indices_sorted: Vec<usize> = indices
-            .into_iter()
-            .map(|i| <BitType as From<T>>::from(i) as usize)
-            .collect();
+        let mut indices_sorted: Vec<usize> = indices.into_iter().map(|i| i.index()).collect();
         indices_sorted.sort();
 
         for index in indices_sorted.into_iter().rev() {
@@ -218,8 +224,7 @@ where
         }
         // Update indices.
         for (i, bit) in self.bits.iter().enumerate() {
-            self.indices
-                .insert(BitAsKey::new(bit.bind(py)), (i as BitType).into());
+            self.indices.insert(BitAsKey::new(bit.bind(py)), T::new(i));
         }
         Ok(())
     }

crates/circuit/src/dag_circuit.rs

@@ -15,7 +15,7 @@ use std::hash::Hash;
 use ahash::RandomState;
 use smallvec::SmallVec;
 
-use crate::bit_data::BitData;
+use crate::bit_data::{BitData, WireIndex};
 use crate::circuit_data::CircuitData;
 use crate::circuit_instruction::{
     CircuitInstruction, ExtraInstructionAttributes, OperationFromPython,
@@ -29,7 +29,7 @@ use crate::interner::{Interned, Interner};
 use crate::operations::{Operation, OperationRef, Param, PyInstruction, StandardGate};
 use crate::packed_instruction::{PackedInstruction, PackedOperation};
 use crate::rustworkx_core_vnext::isomorphism;
-use crate::{BitType, Clbit, Qubit, TupleLikeArg};
+use crate::{Clbit, Qubit, TupleLikeArg};
 
 use hashbrown::{HashMap, HashSet};
 use indexmap::IndexMap;
@@ -82,38 +82,22 @@ static SEMANTIC_EQ_SYMMETRIC: [&str; 4] = ["barrier", "swap", "break_loop", "con
 /// These keys are [Eq], but this is semantically valid only for keys
 /// from the same [DAGCircuit] instance.
 #[derive(Copy, Clone, Debug, Eq, PartialEq, Hash)]
-pub struct Var(BitType);
+pub struct Var(u32);
 
-impl Var {
-    /// Construct a new [Var] object from a usize. if you have a u32 you can
-    /// create a [Var] object directly with `Var(0u32)`. This will panic
-    /// if the `usize` index exceeds `u32::MAX`.
-    #[inline(always)]
+// Note that the implementation is private to this crate
+// because the visibility of WireIndex is only pub(crate).
+// This is intentional, since it prevents users from creating
+// a Var, which should only be done by DAGCircuit.
+impl WireIndex for Var {
     fn new(index: usize) -> Self {
-        Var(index
-            .try_into()
-            .unwrap_or_else(|_| panic!("Index value '{}' exceeds the maximum bit width!", index)))
+        Var(index.try_into().expect("Variable storage exceeded."))
     }
 
-    /// Get the index of the [Var]
-    #[inline(always)]
     fn index(&self) -> usize {
         self.0 as usize
     }
 }
 
-impl From<BitType> for Var {
-    fn from(value: BitType) -> Self {
-        Var(value)
-    }
-}
-
-impl From<Var> for BitType {
-    fn from(value: Var) -> Self {
-        value.0
-    }
-}
-
 #[derive(Clone, Debug)]
 pub enum NodeType {
     QubitIn(Qubit),
@@ -942,10 +926,10 @@ def _format(operand):
             }
             params.push(p.to_object(py));
         }
-        let qubits: Vec<BitType> = self
+        let qubits: Vec<usize> = self
             .qubits
             .map_bits(node.instruction.qubits.bind(py).iter())?
-            .map(|bit| bit.0)
+            .map(|bit| bit.index())
             .collect();
         let qubits = PyTuple::new_bound(py, qubits);
         let params = PyTuple::new_bound(py, params);

crates/circuit/src/lib.rs

@@ -29,17 +29,17 @@ pub mod util;
 
 mod rustworkx_core_vnext;
 
+use crate::bit_data::WireIndex;
 use pyo3::prelude::*;
 use pyo3::types::{PySequence, PyTuple};
 
-pub(crate) type BitType = u32;
 #[derive(Copy, Clone, Debug, Hash, Ord, PartialOrd, Eq, PartialEq, FromPyObject)]
-pub struct Qubit(pub(crate) BitType);
+pub struct Qubit(u32);
 
 impl Qubit {
-    /// Construct a new Qubit object from a usize, if you have a u32 you can
-    /// create a `Qubit` object directly with `Qubit(0u32)`. This will panic
-    /// if the `usize` index exceeds `u32::MAX`.
+    /// Construct a new [Qubit] from a usize.
+    ///
+    /// This will panic if the `usize` index exceeds `u32::MAX`.
     #[inline(always)]
     pub fn new(index: usize) -> Self {
         Qubit(
@@ -49,20 +49,30 @@ impl Qubit {
         )
     }
 
-    /// Convert a Qubit to a usize
+    /// Convert a [Qubit] to a usize.
     #[inline(always)]
     pub fn index(&self) -> usize {
         self.0 as usize
     }
 }
 
+impl WireIndex for Qubit {
+    fn new(index: usize) -> Self {
+        Qubit::new(index)
+    }
+
+    fn index(&self) -> usize {
+        self.index()
+    }
+}
+
 #[derive(Copy, Clone, Debug, Hash, Ord, PartialOrd, Eq, PartialEq)]
-pub struct Clbit(pub BitType);
+pub struct Clbit(u32);
 
 impl Clbit {
-    /// Construct a new Clbit object from a usize. if you have a u32 you can
-    /// create a `Clbit` object directly with `Clbit(0u32)`. This will panic
-    /// if the `usize` index exceeds `u32::MAX`.
+    /// Construct a new [Clbit] from a usize.
+    ///
+    /// This will panic if the `usize` index exceeds `u32::MAX`.
     #[inline(always)]
     pub fn new(index: usize) -> Self {
         Clbit(
@@ -72,13 +82,23 @@ impl Clbit {
         )
     }
 
-    /// Convert a Clbit to a usize
+    /// Convert a [Clbit] to a usize.
     #[inline(always)]
     pub fn index(&self) -> usize {
         self.0 as usize
     }
 }
 
+impl WireIndex for Clbit {
+    fn new(index: usize) -> Self {
+        Clbit::new(index)
+    }
+
+    fn index(&self) -> usize {
+        self.index()
+    }
+}
+
 pub struct TupleLikeArg<'py> {
     value: Bound<'py, PyTuple>,
 }
@@ -98,30 +118,6 @@ impl<'py> FromPyObject<'py> for TupleLikeArg<'py> {
     }
 }
 
-impl From<BitType> for Qubit {
-    fn from(value: BitType) -> Self {
-        Qubit(value)
-    }
-}
-
-impl From<Qubit> for BitType {
-    fn from(value: Qubit) -> Self {
-        value.0
-    }
-}
-
-impl From<BitType> for Clbit {
-    fn from(value: BitType) -> Self {
-        Clbit(value)
-    }
-}
-
-impl From<Clbit> for BitType {
-    fn from(value: Clbit) -> Self {
-        value.0
-    }
-}
-
 pub fn circuit(m: &Bound<PyModule>) -> PyResult<()> {
     m.add_class::<circuit_data::CircuitData>()?;
     m.add_class::<circuit_instruction::CircuitInstruction>()?;