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

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

TheOpcodeId newtype lacks a public accessor for the inneru16 value. Additionally, consider implementingHash,Display, and potentiallyFromStr 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+    }+

ForHash, 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 fornew_unchecked should reference theis_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.

TheTryFrom 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)]pubstructInvalidOpcodeId;impl std::fmt::DisplayforInvalidOpcodeId{fnfmt(&self,f:&mut std::fmt::Formatter<'_>) -> std::fmt::Result{write!(f,"invalid opcode ID")}}impl std::error::ErrorforInvalidOpcodeId{}

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-generatedInstruction 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