remove RawRegId + support non-breaking RegId impls

Issue #29 has been updated to reflect this new approach.

src/arch/arm/reg/id.rs

@@ -2,6 +2,7 @@ use crate::arch::RegId;
 
 /// 32-bit ARM core register identifier.
 #[derive(Debug, Clone, Copy)]
+#[non_exhaustive]
 pub enum ArmCoreRegId {
     /// General purpose registers (R0-R12)
     Gpr(u8),
@@ -21,14 +22,15 @@ pub enum ArmCoreRegId {
 
 impl RegId for ArmCoreRegId {
     fn from_raw_id(id: usize) -> Option<(Self, usize)> {
-        match id {
-            0..=12 => Some((Self::Gpr(id as u8), 4)),
-            13 => Some((Self::Sp, 4)),
-            14 => Some((Self::Lr, 4)),
-            15 => Some((Self::Pc, 4)),
-            16..=23 => Some((Self::Fpr(id as u8), 4)),
-            25 => Some((Self::Cpsr, 4)),
-            _ => None,
-        }
+        let reg = match id {
+            0..=12 => Self::Gpr(id as u8),
+            13 => Self::Sp,
+            14 => Self::Lr,
+            15 => Self::Pc,
+            16..=23 => Self::Fpr((id as u8) - 16),
+            25 => Self::Cpsr,
+            _ => return None,
+        };
+        Some((reg, 4))
     }
 }

src/arch/mips/mod.rs

@@ -1,29 +1,42 @@
 //! Implementations for the MIPS architecture.
 
 use crate::arch::Arch;
+use crate::arch::RegId;
 
 pub mod reg;
 
 /// Implements `Arch` for 32-bit MIPS.
-pub enum Mips {}
+///
+/// Check out the [module level docs](../index.html#whats-with-regidimpl) for
+/// more info about the `RegIdImpl` type parameter.
+pub enum Mips<RegIdImpl: RegId> {
+    #[doc(hidden)]
+    _Marker(core::marker::PhantomData<RegIdImpl>),
+}
 
 /// Implements `Arch` for 64-bit MIPS.
-pub enum Mips64 {}
+///
+/// Check out the [module level docs](../index.html#whats-with-regidimpl) for
+/// more info about the `RegIdImpl` type parameter.
+pub enum Mips64<RegIdImpl: RegId> {
+    #[doc(hidden)]
+    _Marker(core::marker::PhantomData<RegIdImpl>),
+}
 
-impl Arch for Mips {
+impl<RegIdImpl: RegId> Arch for Mips<RegIdImpl> {
     type Usize = u32;
     type Registers = reg::MipsCoreRegs<u32>;
-    type RegId = reg::id::MipsRegId;
+    type RegId = RegIdImpl;
 
     fn target_description_xml() -> Option<&'static str> {
         Some(r#"<target version="1.0"><architecture>mips</architecture></target>"#)
     }
 }
 
-impl Arch for Mips64 {
+impl<RegIdImpl: RegId> Arch for Mips64<RegIdImpl> {
     type Usize = u64;
     type Registers = reg::MipsCoreRegs<u64>;
-    type RegId = reg::id::MipsRegId;
+    type RegId = RegIdImpl;
 
     fn target_description_xml() -> Option<&'static str> {
         Some(r#"<target version="1.0"><architecture>mips64</architecture></target>"#)

src/arch/mips/reg/id.rs

@@ -1,4 +1,2 @@
-use crate::arch::RawRegId;
-
-/// TODO: Add proper `RegId` implementation. See [issue #29](https://github.com/daniel5151/gdbstub/issues/29)
-pub type MipsRegId = RawRegId;
+// TODO: Add proper `RegId` implementation. See [issue #29](https://github.com/daniel5151/gdbstub/issues/29)
+// pub enum MipsRegId {}

src/arch/mod.rs

@@ -15,13 +15,48 @@
 //! while they are tested (by the PR's author) and code-reviewed, it's not
 //! particularly feasible to write detailed tests for each architecture! If you
 //! spot a bug in any of the implementations, please file an issue / open a PR!
+//!
+//! # What's with `RegIdImpl`?
+//!
+//! Supporting the `Target::read/write_register` API required introducing a new
+//! [`RegId`](trait.RegId.html) trait + [`Arch` associated
+//! type](trait.Arch.html#associatedtype.RegId). `RegId` is used by `gdbstub`
+//! to translate raw GDB register ids (a protocol level arch-dependent `usize`)
+//! into human-readable enum variants.
+//!
+//! Unfortunately, this API was added after several contributors had already
+//! upstreamed their `Arch` implementations, and as a result, there are several
+//! built-in arch implementations which are missing proper `RegId` enums
+//! (tracked under [issue #29](https://github.com/daniel5151/gdbstub/issues/29)).
+//!
+//! As a stop-gap measure, affected `Arch` implementations have been modified to
+//! accept a `RegIdImpl` type parameter, which requires users to manually
+//! specify a `RegId` implementation.
+//!
+//! If you're not interested in implementing the `Target::read/write_register`
+//! methods and just want to get up-and-running with `gdbstub`, it's fine to
+//! set `RegIdImpl` to `()` and use the built-in stubbed `impl RegId for ()`.
+//!
+//! A better approach would be to implement (and hopefully upstream!) a proper
+//! `RegId` enum. While this will require doing a bit of digging through the GDB
+//! docs + [architecture XML definitions](https://github.com/bminor/binutils-gdb/tree/master/gdb/features/),
+//! it's not too tricky to get a working implementation up and running, and
+//! makes it possible to safely and efficiently implement the
+//! `Target::read/write_register` API. As an example, check out
+//! [`ArmCoreRegId`](arm/reg/id/enum.ArmCoreRegId.html#impl-RegId).
+//!
+//! Whenever a `RegId` enum is upstreamed, the associated `Arch`'s `RegIdImpl`
+//! parameter will be defaulted to the newly added enum. This will simplify the
+//! API without requiring an explicit breaking API change. Once all `RegIdImpl`
+//! have a default implementation, only a single breaking API change will be
+//! required to remove `RegIdImpl` entirely (along with this documentation).
 
 pub mod arm;
 pub mod mips;
 pub mod msp430;
 pub mod ppc;
 pub mod riscv;
-mod traits;
 pub mod x86;
 
+mod traits;
 pub use traits::*;

src/arch/msp430/mod.rs

@@ -1,16 +1,23 @@
 //! Implementations for the TI-MSP430 family of MCUs.
 
 use crate::arch::Arch;
+use crate::arch::RegId;
 
 pub mod reg;
 
 /// Implements `Arch` for standard 16-bit TI-MSP430 MCUs.
-pub enum Msp430 {}
+///
+/// Check out the [module level docs](../index.html#whats-with-regidimpl) for
+/// more info about the `RegIdImpl` type parameter.
+pub enum Msp430<RegIdImpl: RegId> {
+    #[doc(hidden)]
+    _Marker(core::marker::PhantomData<RegIdImpl>),
+}
 
-impl Arch for Msp430 {
+impl<RegIdImpl: RegId> Arch for Msp430<RegIdImpl> {
     type Usize = u32;
     type Registers = reg::Msp430Regs;
-    type RegId = reg::id::Msp430RegId;
+    type RegId = RegIdImpl;
 
     fn target_description_xml() -> Option<&'static str> {
         Some(r#"<target version="1.0"><architecture>msp430</architecture></target>"#)

src/arch/msp430/reg/id.rs

@@ -1,4 +1,2 @@
-use crate::arch::RawRegId;
-
-/// TODO: Add proper `RegId` implementation. See [issue #29](https://github.com/daniel5151/gdbstub/issues/29)
-pub type Msp430RegId = RawRegId;
+// TODO: Add proper `RegId` implementation. See [issue #29](https://github.com/daniel5151/gdbstub/issues/29)
+// pub enum Msp430RegId {}

src/arch/ppc/mod.rs

@@ -1,16 +1,23 @@
 //! Implementations for various PowerPC architectures.
 
 use crate::arch::Arch;
+use crate::arch::RegId;
 
 pub mod reg;
 
-/// Implements `Arch` for 32-bit PowerPC + AltiVec SIMD
-pub enum PowerPcAltivec32 {}
+/// Implements `Arch` for 32-bit PowerPC + AltiVec SIMD.
+///
+/// Check out the [module level docs](../index.html#whats-with-regidimpl) for
+/// more info about the `RegIdImpl` type parameter.
+pub enum PowerPcAltivec32<RegIdImpl: RegId> {
+    #[doc(hidden)]
+    _Marker(core::marker::PhantomData<RegIdImpl>),
+}
 
-impl Arch for PowerPcAltivec32 {
+impl<RegIdImpl: RegId> Arch for PowerPcAltivec32<RegIdImpl> {
     type Usize = u32;
     type Registers = reg::PowerPcCommonRegs;
-    type RegId = reg::id::PowerPc32RegId;
+    type RegId = RegIdImpl;
 
     fn target_description_xml() -> Option<&'static str> {
         Some(

src/arch/ppc/reg/id.rs

@@ -1,4 +1,2 @@
-use crate::arch::RawRegId;
-
-/// TODO: Add proper `RegId` implementation. See [issue #29](https://github.com/daniel5151/gdbstub/issues/29)
-pub type PowerPc32RegId = RawRegId;
+// TODO: Add proper `RegId` implementation. See [issue #29](https://github.com/daniel5151/gdbstub/issues/29)
+// pub enum PowerPc32RegId {}

src/arch/riscv/reg/id.rs

@@ -2,6 +2,7 @@ use crate::arch::RegId;
 
 /// RISC-V Register identifier.
 #[derive(Debug, Clone, Copy)]
+#[non_exhaustive]
 pub enum RiscvRegId {
     /// General Purpose Register (x0-x31).
     Gpr(u8),
@@ -17,13 +18,14 @@ pub enum RiscvRegId {
 
 impl RegId for RiscvRegId {
     fn from_raw_id(id: usize) -> Option<(Self, usize)> {
-        match id {
-            0..=31 => Some((Self::Gpr(id as u8), 4)),
-            32 => Some((Self::Pc, 4)),
-            33..=64 => Some((Self::Fpr((id - 33) as u8), 4)),
-            65..=4160 => Some((Self::Csr((id - 65) as u16), 4)),
-            4161 => Some((Self::Priv, 1)),
-            _ => None,
-        }
+        let reg_size = match id {
+            0..=31 => (Self::Gpr(id as u8), 4),
+            32 => (Self::Pc, 4),
+            33..=64 => (Self::Fpr((id - 33) as u8), 4),
+            65..=4160 => (Self::Csr((id - 65) as u16), 4),
+            4161 => (Self::Priv, 1),
+            _ => return None,
+        };
+        Some(reg_size)
     }
 }

src/arch/traits.rs

@@ -14,35 +14,10 @@ pub trait RegId: Sized + Debug {
     fn from_raw_id(id: usize) -> Option<(Self, usize)>;
 }
 
-/// A "stop-gap" `RegId` which contains the raw register number used by GDB.
-///
-/// If you come across this `RegId` while working with a built-in `arch`, please
-/// consider opening a PR to add a proper enum-based `RegId` instead!
-///
-/// Several of the built-in `arch` implementations predate the addition of the
-/// `read/write_register` methods to `gdbstub`. As such, until someone opens a
-/// PR to update them with a proper enum-based RegId, they are stuck using this
-/// temporary `RawRegId` instead.
-///
-/// While it is possible to implement the `read/write_register` methods using
-/// `RawRegId`, it does require looking up the architecture's corresponding
-/// feature.xml files in the [GDB source code](https://github.com/bminor/binutils-gdb/tree/master/gdb/features/).
-///
-/// When using `RawRegId`, the `dst` and `val` buffers are conservatively sized
-/// to be at least 256 bits, which should be large enough to store any register
-/// size required by GDB. Please note that these buffer will be de/serialized as
-/// **big-endian** values!
-///
-/// It bears repeating: if you end up implementing the `read/write_register`
-/// methods using `RawRegId`, please consider implementing a proper `RegId` enum
-/// instead and upstreaming it!
-#[derive(Debug)]
-pub struct RawRegId(pub usize);
-
-impl RegId for RawRegId {
-    // simply pass-through the raw register ID +
-    fn from_raw_id(id: usize) -> Option<(Self, usize)> {
-        Some((RawRegId(id), 256 / 8))
+/// Stub implementation -- Returns `None` for all raw IDs.
+impl RegId for () {
+    fn from_raw_id(_id: usize) -> Option<(Self, usize)> {
+        None
     }
 }
 

src/arch/x86/mod.rs

@@ -1,17 +1,24 @@
 //! Implementations for various x86 architectures.
 
 use crate::arch::Arch;
+use crate::arch::RegId;
 
 pub mod reg;
 
-/// Implements `Arch` for 64-bit x86 + SSE Extensions
+/// Implements `Arch` for 64-bit x86 + SSE Extensions.
+///
+/// Check out the [module level docs](../index.html#whats-with-regidimpl) for
+/// more info about the `RegIdImpl` type parameter.
 #[allow(non_camel_case_types)]
-pub enum X86_64_SSE {}
+pub enum X86_64_SSE<RegIdImpl: RegId> {
+    #[doc(hidden)]
+    _Marker(core::marker::PhantomData<RegIdImpl>),
+}
 
-impl Arch for X86_64_SSE {
+impl<RegIdImpl: RegId> Arch for X86_64_SSE<RegIdImpl> {
     type Usize = u64;
     type Registers = reg::X86_64CoreRegs;
-    type RegId = reg::id::X86_64RegId;
+    type RegId = RegIdImpl;
 
     fn target_description_xml() -> Option<&'static str> {
         Some(
@@ -20,14 +27,20 @@ impl Arch for X86_64_SSE {
     }
 }
 
-/// Implements `Arch` for 32-bit x86 + SSE Extensions
+/// Implements `Arch` for 32-bit x86 + SSE Extensions.
+///
+/// Check out the [module level docs](../index.html#whats-with-regidimpl) for
+/// more info about the `RegIdImpl` type parameter.
 #[allow(non_camel_case_types)]
-pub enum X86_SSE {}
+pub enum X86_SSE<RegIdImpl: RegId> {
+    #[doc(hidden)]
+    _Marker(core::marker::PhantomData<RegIdImpl>),
+}
 
-impl Arch for X86_SSE {
+impl<RegIdImpl: RegId> Arch for X86_SSE<RegIdImpl> {
     type Usize = u32;
     type Registers = reg::X86CoreRegs;
-    type RegId = reg::id::X86RegId;
+    type RegId = RegIdImpl;
 
     fn target_description_xml() -> Option<&'static str> {
         Some(

src/arch/x86/reg/id.rs

@@ -1,7 +1,5 @@
-use crate::arch::RawRegId;
+// TODO: Add proper `RegId` implementation. See [issue #29](https://github.com/daniel5151/gdbstub/issues/29)
+// pub enum X86RegId {}
 
-/// TODO: Add proper `RegId` implementation. See [issue #29](https://github.com/daniel5151/gdbstub/issues/29)
-pub type X86RegId = RawRegId;
-
-/// TODO: Add proper `RegId` implementation. See [issue #29](https://github.com/daniel5151/gdbstub/issues/29)
-pub type X86_64RegId = RawRegId;
+// TODO: Add proper `RegId` implementation. See [issue #29](https://github.com/daniel5151/gdbstub/issues/29)
+// pub enum X86_64RegId {}

src/target_ext/base/multithread.rs

@@ -111,9 +111,8 @@ pub trait MultiThreadOps: Target {
     ///
     /// _Note:_ This method includes a stubbed default implementation which
     /// simply returns `Ok(())`. This is due to the fact that several built-in
-    /// `arch` implementations still use the generic, albeit highly un-ergonomic
-    /// [`RawRegId`](../../../arch/struct.RawRegId.html) type. See the docs
-    /// for `RawRegId` for more info.
+    /// `arch` implementations haven't been updated with proper `RegId`
+    /// implementations.
     fn read_register(
         &mut self,
         reg_id: <Self::Arch as Arch>::RegId,
@@ -135,9 +134,8 @@ pub trait MultiThreadOps: Target {
     ///
     /// _Note:_ This method includes a stubbed default implementation which
     /// simply returns `Ok(())`. This is due to the fact that several built-in
-    /// `arch` implementations still use the generic, albeit highly un-ergonomic
-    /// [`RawRegId`](../../../arch/struct.RawRegId.html) type. See the docs
-    /// for `RawRegId` for more info.
+    /// `arch` implementations haven't been updated with proper `RegId`
+    /// implementations.
     fn write_register(
         &mut self,
         reg_id: <Self::Arch as Arch>::RegId,

src/target_ext/base/singlethread.rs

@@ -65,9 +65,8 @@ pub trait SingleThreadOps: Target {
     ///
     /// _Note:_ This method includes a stubbed default implementation which
     /// simply returns `Ok(())`. This is due to the fact that several built-in
-    /// `arch` implementations still use the generic, albeit highly un-ergonomic
-    /// [`RawRegId`](../../../arch/struct.RawRegId.html) type. See the docs
-    /// for `RawRegId` for more info.
+    /// `arch` implementations haven't been updated with proper `RegId`
+    /// implementations.
     fn read_register(
         &mut self,
         reg_id: <Self::Arch as Arch>::RegId,
@@ -88,9 +87,8 @@ pub trait SingleThreadOps: Target {
     ///
     /// _Note:_ This method includes a stubbed default implementation which
     /// simply returns `Ok(())`. This is due to the fact that several built-in
-    /// `arch` implementations still use the generic, albeit highly un-ergonomic
-    /// [`RawRegId`](../../../arch/struct.RawRegId.html) type. See the docs
-    /// for `RawRegId` for more info.
+    /// `arch` implementations haven't been updated with proper `RegId`
+    /// implementations.
     fn write_register(
         &mut self,
         reg_id: <Self::Arch as Arch>::RegId,