compiler/core/src/opcode.rs (3)

7-8: Consider adding accessor and additional trait implementations.

The OpcodeId newtype lacks a public accessor for the inner u16 value. Additionally, consider implementing Hash, Display, and potentially FromStr for better ergonomics.

Apply this diff to add an accessor:

 #[derive(Clone, Copy, Debug, Eq, PartialEq)]
 pub struct OpcodeId(u16);

 impl OpcodeId {
+    /// Returns the raw opcode ID value.
+    #[must_use]
+    pub const fn get(self) -> u16 {
+        self.0
+    }
+

For Hash, update the derives:

-#[derive(Clone, Copy, Debug, Eq, PartialEq)]
+#[derive(Clone, Copy, Debug, Eq, PartialEq, Hash)]
 pub struct OpcodeId(u16);

---

235-243: Enhance safety documentation.

The safety documentation for new_unchecked should reference the is_valid method to clarify what constitutes a valid opcode ID.

Apply this diff:

     /// Creates a new opcode ID without checking the value is a valid opcode ID.
     ///
     /// # Safety
     ///
-    /// The value should be a valid opcode ID.
+    /// The caller must ensure that `id` satisfies `Self::is_valid(id)`.
     #[must_use]
     pub const unsafe fn new_unchecked(id: u16) -> Self {

---

545-560: Consider a more descriptive error type for TryFrom.

The TryFrom implementations use () as the error type, providing no information about why a conversion failed. While this is acceptable, a custom error type could improve debuggability.

Consider defining an error type:

/// Error type for invalid opcode ID conversions.
#[derive(Clone, Copy, Debug, Eq, PartialEq)]
pub struct InvalidOpcodeId;

impl std::fmt::Display for InvalidOpcodeId {
    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
        write!(f, "invalid opcode ID")
    }
}

impl std::error::Error for InvalidOpcodeId {}

Then update the macro:

 macro_rules! opcode_id_try_from_impl {
     ($t:ty) => {
         impl TryFrom<$t> for OpcodeId {
-            type Error = ();
+            type Error = InvalidOpcodeId;

             fn try_from(value: $t) -> Result<Self, Self::Error> {
-                let id = value.try_into().map_err(|_| ())?;
+                let id = value.try_into().map_err(|_| InvalidOpcodeId)?;
                 if Self::is_valid(id) {
                     Ok(Self(id))
                 } else {
-                    Err(())
+                    Err(InvalidOpcodeId)
                 }
             }
         }

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

stdlib/src/opcode.rs (1)

26-26: Track the TODO for auto-generated Instruction enum.

This TODO references future work to leverage the auto-generated Instruction enum. Consider tracking this as a follow-up task.

Do you want me to open a new issue to track this task?

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro