Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

reimplement codegen using quote #40

Merged
merged 33 commits into from
Mar 5, 2024
Merged

reimplement codegen using quote #40

merged 33 commits into from
Mar 5, 2024

Conversation

hawkw
Copy link
Member

@hawkw hawkw commented Feb 29, 2024

Currently, all of idols code generation is implemented by "manually"
writing out Rust code as strings. The quote crate provides a macro for
quasi-quoting Rust code inside of Rust code, as a
proc-_macro2::TokenStream. This is generally the preferred way to do
code generation in much of the Rust ecosystem.

Some reasons we might want to use quote include:

  • Integrating with prettyplease, a crate that does rustfmt-like
    formatting as a library dep, so we no longer need to invoke an external
    rustfmt as part of the build process in Hubris
  • When using quote, we can interpolate things like types and
    identifiers as their parsed representation from the syn crate. This
    allows us to actually parse these fragments from the interface
    definition and validate that they're correct --- a Name can be
    represented by a syn::Ident, rather than a String, so we know that
    it's a valid Rust identifier, and parsing the RON will fail if it
    isn't. Similarly, we can parse types this way, allowing us to detect
    !Sized types much more accurately.
  • Code quoted using quote! looks like Rust code, rather than a big
    string literal. Therefore, it can benefit from an editor's brace
    matching, syntax highlighting, indentation, etc.

This branch rewrites idol's code generation using quote.

Testing

I've manually tested that the code generated is still correct by
comparing the output between this branch and main, using the example
interface definition from the README:

Interface(
    name: "Spi",
    ops: {
        "exchange": (
            args: {
                "device_index": (type: "u8"),
            },
            leases: {
                "source": (type: "[u8]", read: true),
                "sink": (type: "[u8]", write: true),
            },
            reply: Result(
                ok: "()",
                err: CLike("SpiError"),
            ),
        ),
    },
)
Generated client stubs:

main:

#[allow(non_camel_case_types)]
#[derive(Copy, Clone, Debug, Eq, PartialEq, userlib::FromPrimitive)]
pub enum SpiOperation {
    exchange = 1,
}

#[allow(unused_imports)]
use userlib::UnwrapLite;
#[derive(Clone, Debug)]
pub struct Spi {
    current_id: core::cell::Cell<userlib::TaskId>,
}

impl From<userlib::TaskId> for Spi {
    fn from(x: userlib::TaskId) -> Self {
        Self {
            current_id: core::cell::Cell::new(x),
        }
    }
}

#[allow(
    clippy::let_unit_value,
    clippy::collapsible_else_if,
    clippy::needless_return,
    clippy::unused_unit
)]
impl Spi {
    // operation: exchange (0)
    // not idempotent, error type must represent death
    pub fn exchange(
        &self,
        device_index: u8,
        source: &[u8],
        sink: &mut [u8],
    ) -> Result<(), SpiError> {
        let (arg_device_index, arg_source, arg_sink) =
            (device_index, source, sink);
        #[allow(non_camel_case_types)]
        #[derive(zerocopy::AsBytes)]
        #[repr(C, packed)]
        struct Spi_exchange_ARGS {
            device_index: u8,
        }

        const REPLY_SIZE: usize = {
            let oksize = core::mem::size_of::<()>();
            let errsize = 0;
            if oksize > errsize {
                oksize
            } else {
                errsize
            }
        };

        let args = Spi_exchange_ARGS {
            device_index: arg_device_index,
        };

        let mut reply = [0u8; REPLY_SIZE];

        let task = self.current_id.get();

        let (rc, len) = sys_send(
            task,
            SpiOperation::exchange as u16,
            zerocopy::AsBytes::as_bytes(&args),
            &mut reply,
            &[
                userlib::Lease::read_only(zerocopy::AsBytes::as_bytes(
                    arg_source,
                )),
                userlib::Lease::write_only(zerocopy::AsBytes::as_bytes_mut(
                    arg_sink,
                )),
            ],
        );
        if rc == 0 {
            let _len = len;
            #[derive(zerocopy::FromBytes, zerocopy::Unaligned)]
            #[repr(C, packed)]
            struct Spi_exchange_REPLY {
                value: (),
            }
            let lv = zerocopy::LayoutVerified::<_, Spi_exchange_REPLY>::new_unaligned(&reply[..])
                .unwrap_lite();
            let v: () = lv.value;
            return Ok(v);
        } else {
            if let Some(g) = userlib::extract_new_generation(rc) {
                self.current_id
                    .set(userlib::TaskId::for_index_and_gen(task.index(), g));
            }
            return Err(<SpiError as core::convert::TryFrom<u32>>::try_from(
                rc,
            )
            .unwrap_lite());
        }
    }
}

This branch:

#[allow(non_camel_case_types)]
#[derive(Copy, Clone, Debug, Eq, PartialEq, userlib::FromPrimitive)]
pub enum SpiOperation {
    exchange = 1usize,
}
#[allow(unused_imports)]
use userlib::UnwrapLite;
#[derive(Clone, Debug)]
pub struct Spi {
    current_id: core::cell::Cell<userlib::TaskId>,
}
#[automatically_derived]
impl From<userlib::TaskId> for Spi {
    fn from(x: userlib::TaskId) -> Self {
        Self {
            current_id: core::cell::Cell::new(x),
        }
    }
}
#[allow(
    clippy::let_unit_value,
    clippy::collapsible_else_if,
    clippy::needless_returnm
    clippy::unused_unit
)]
#[automatically_derived]
impl Spi {
    /// operation: exchange (0)
    /// not idempotent, error type must represent death
    pub fn exchange(
        &self,
        device_index: u8,
        source: &[u8],
        sink: &mut [u8],
    ) -> Result<(), SpiError> {
        let (arg_device_index, arg_source, arg_sink) =
            (device_index, source, sink);
        #[allow(non_camel_case_types)]
        #[derive(zerocopy::AsBytes)]
        #[repr(C, packed)]
        struct Spi_exchange_ARGS {
            device_index: u8,
        }
        const REPLY_SIZE: usize = {
            {
                let oksize = core::mem::size_of::<()>();
                let errsize = 0;
                if oksize > errsize {
                    oksize
                } else {
                    errsize
                }
            }
        };
        let mut reply = [0u8; REPLY_SIZE];
        let args = Spi_exchange_ARGS {
            device_index: arg_device_index,
        };
        let task = self.current_id.get();
        let (rc, len) = sys_send(
            task,
            SpiOperation::exchange as u16,
            zerocopy::AsBytes::as_bytes(&args),
            &mut reply,
            &[
                userlib::Lease::read_only(zerocopy::AsBytes::as_bytes(
                    arg_source,
                )),
                userlib::Lease::write_only(zerocopy::AsBytes::as_bytes_mut(
                    arg_sink,
                )),
            ],
        )?;
        if rc == 0 {
            let _len = len;
            #[derive(zerocopy::FromBytes, zerocopy::Unaligned)]
            #[repr(C, packed)]
            struct Spi_exchange_REPLY {
                value: (),
            }
            let lv = zerocopy::LayoutVerified::<
                _,
                Spi_exchange_REPLY,
            >::new_unaligned(&reply[..])
                .unwrap_lite();
            let v: () = lv.value;
            return Ok(v);
        } else {
            if let Some(g) = userlib::extract_new_generation(rc) {
                self.current_id
                    .set(userlib::TaskId::for_index_and_gen(task.index(), g));
            }
            return Err(<SpiError as core::convert::TryFrom<u32>>::try_from(
                rc,
            )
            .unwrap_lite());
        }
    }
}
Generated server stubs:

main:

pub const EXCHANGE_MSG_SIZE: usize = 0
    + core::mem::size_of::<u8>()
    ;
pub const EXCHANGE_REPLY_SIZE: usize =core::mem::size_of::<()>();
#[allow(clippy::absurd_extreme_comparisons)]
const fn max_incoming_size() -> usize {
    let mut max = 0;
    if max < EXCHANGE_MSG_SIZE {
        max = EXCHANGE_MSG_SIZE;
    }
    max
}
pub const INCOMING_SIZE: usize = max_incoming_size();
#[allow(non_camel_case_types)]
#[repr(C, packed)]
#[derive(Copy, Clone, zerocopy::FromBytes, zerocopy::Unaligned)]
pub struct Spi_exchange_ARGS {
    pub device_index: u8,
}

pub fn read_exchange_msg(bytes: &[u8])
    -> Option<&Spi_exchange_ARGS>
{
    Some(zerocopy::LayoutVerified::<_, Spi_exchange_ARGS>::new_unaligned(bytes)?
        .into_ref())
}
#[repr(C, packed)]
struct Spi_exchange_REPLY {
    value: (),
}

#[allow(dead_code)]
static SPI_EXCHANGE_REPLY: Option<&Spi_exchange_REPLY> = None;
#[allow(non_camel_case_types)]
#[derive(Copy, Clone, Debug, Eq, PartialEq, userlib::FromPrimitive)]
pub enum SpiOperation {
    exchange = 1,
}

impl idol_runtime::ServerOp for SpiOperation {
    fn max_reply_size(self) -> usize {
        match self {
            Self::exchange => EXCHANGE_REPLY_SIZE,
        }
    }

    fn required_lease_count(self) -> usize {
        match self {
            Self::exchange => 2,
        }
    }

}
pub trait InOrderSpiImpl {
    fn recv_source(&self) -> Option<userlib::TaskId> {
        None
    }

    fn closed_recv_fail(&mut self) {
        panic!()
    }

    fn exchange(
        &mut self,
        msg: &userlib::RecvMessage,
        device_index: u8,
        source: idol_runtime::Leased<idol_runtime::R, [u8]>,
        sink: idol_runtime::Leased<idol_runtime::W, [u8]>,
) -> Result<(), idol_runtime::RequestError<SpiError>> where SpiError: idol_runtime::IHaveConsideredServerDeathWithThisErrorType;

}

impl<S: InOrderSpiImpl> idol_runtime::Server<SpiOperation> for (core::marker::PhantomData<SpiOperation>, &'_ mut S) {
    fn recv_source(&self) -> Option<userlib::TaskId> {
        <S as InOrderSpiImpl>::recv_source(self.1)
    }

    fn closed_recv_fail(&mut self) {
        <S as InOrderSpiImpl>::closed_recv_fail(self.1)
    }

    fn handle(
        &mut self,
        op: SpiOperation,
        incoming: &[u8],
        rm: &userlib::RecvMessage,
    ) -> Result<(), idol_runtime::RequestError<u16>> {
        #[allow(unused_imports)]
        use core::convert::TryInto;
        use idol_runtime::ClientError;
        match op {
            SpiOperation::exchange => {
                let args = read_exchange_msg(incoming).ok_or_else(|| ClientError::BadMessageContents.fail())?;
                let r = self.1.exchange(
                    rm,
                    args.device_index,
                    idol_runtime::Leased::read_only_slice(rm.sender, 0, None).ok_or_else(|| ClientError::BadLease.fail())?,
                    idol_runtime::Leased::write_only_slice(rm.sender, 1, None).ok_or_else(|| ClientError::BadLease.fail())?,
                );
                match r {
                    Ok(val) => {
                        userlib::sys_reply(rm.sender, 0, zerocopy::AsBytes::as_bytes(&val));
                        Ok(())
                    }
                    Err(val) => {
                        Err(val.map_runtime(u16::from))
                    }
                }
            }
        }
    }
}

this branch:

#[allow(unused_imports)]
use userlib::UnwrapLite;
pub const EXCHANGE_MSG_SIZE: usize = 0 + core::mem::size_of::<u8>();
pub const EXCHANGE_REPLY_SIZE: usize = core::mem::size_of::<()>();
#[allow(clippy::absurd_extreme_comparisons)]
const fn max_incoming_size() -> usize {
    let mut max = 0;
    if max < EXCHANGE_MSG_SIZE {
        max = EXCHANGE_MSG_SIZE;
    }
    max
}
pub const INCOMING_SIZE: usize = max_incoming_size();
#[repr(C, packed)]
#[derive(Copy, Clone, zerocopy::FromBytes, zerocopy::Unaligned)]
#[allow(non_camel_case_types)]
pub struct Spi_exchange_ARGS {
    pub device_index: u8,
}
pub fn read_exchange_msg(bytes: &[u8]) -> Option<&Spi_exchange_ARGS> {
    zerocopy::LayoutVerified::<_, Spi_exchange_ARGS>::new_unaligned(bytes)
        .ok()
        .into_ref()
}
#[repr(C, packed)]
struct Spi_exchange_REPLY {
    value: (),
}
#[allow(dead_code)]
static SPI_EXCHANGE_REPLY: Option<&Spi_exchange_REPLY> = None;
#[allow(non_camel_case_types)]
#[derive(Copy, Clone, Debug, Eq, PartialEq, userlib::FromPrimitive)]
pub enum SpiOperation {
    exchange = 1usize,
}
impl idol_runtime::ServerOp for SpiOperation {
    fn max_reply_size(self) -> usize {
        match self {
            Self::exchange => EXCHANGE_REPLY_SIZE,
        }
    }
    fn required_lease_count(self) -> usize {
        match self {
            Self::exchange => 2usize,
        }
    }
}
pub trait InOrderSpiImpl {
    fn recv_source(&self) -> Option<userlib::TaskId> {
        None
    }
    fn closed_recv_fail(&mut self) {
        panic!()
    }
    fn exchange(
        &mut self,
        msg: &userlib::RecvMessage,
        device_index: u8,
        source: idol_runtime::Leased<idol_runtime::R, [u8]>,
        sink: idol_runtime::Leased<idol_runtime::W, [u8]>,
    ) -> Result<(), idol_runtime::RequestError<SpiError>>
    where
        SpiError: idol_runtime::IHaveConsideredServerDeathWithThisErrorType;
}
impl<S: InOrderSpiImpl> idol_runtime::Server<SpiOperation>
for (core::marker::PhantomData<SpiOperation>, &'_ mut S) {
    fn recv_source(&self) -> Option<userlib::TaskId> {
        <S as InOrderSpiImpl>::recv_source(self.1)
    }
    fn closed_recv_fail(&mut self) {
        <S as InOrderSpiImpl>::closed_recv_fail(self.1)
    }
    fn handle(
        &mut self,
        op: SpiOperation,
        incoming: &[u8],
        rm: &userlib::RecvMessage,
    ) -> Result<(), idol_runtime::RequestError<u16>> {
        #[allow(unused_imports)]
        use core::convert::TryInto;
        use idol_runtime::ClientError;
        match op {
            SpiOperation::exchange => {
                let args = read_exchange_msg(incoming)
                    .ok_or_else(|| {
                        idol_runtime::ClientError::BadMessageContents.fail()
                    })?;
                let r = self
                    .1
                    .exchange(
                        rm,
                        args.device_index,
                        idol_runtime::Leased::read_only_slice(rm.sender, 0usize, None)
                            .ok_or_else(|| ClientError::BadLease.fail())?,
                        idol_runtime::Leased::write_only_slice(rm.sender, 1usize, None)
                            .ok_or_else(|| ClientError::BadLease.fail())?,
                    );
                match r {
                    Ok(val) => {
                        userlib::sys_reply(
                            rm.sender,
                            0,
                            zerocopy::AsBytes::as_bytes(&val),
                        );
                        Ok(())
                    }
                    Err(val) => Err(val.map_runtime(u16::from)),
                }
            }
        }
    }
}

As you can (hopefully) tell, the generated code is the same, with the
exception of different formatting.

@hawkw hawkw force-pushed the eliza/highly-quotable branch from bde9fc5 to a9dc0d8 Compare March 1, 2024 19:31
@hawkw hawkw marked this pull request as ready for review March 1, 2024 20:09
@hawkw hawkw requested a review from cbiffle March 1, 2024 20:20
@hawkw hawkw changed the title [WIP] reimplement codegen using quote reimplement codegen using quote Mar 1, 2024
@hawkw hawkw requested review from mkeeter and jgallagher March 1, 2024 20:20
Copy link
Collaborator

@cbiffle cbiffle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Other than rustfmt and prettyplease appearing to have very different opinions about certain formatting situations, the code before and after this looks equivalent to me -- except for the new automatically_generated attribute and the nit below.

src/server.rs Outdated
) -> Result<(), idol_runtime::RequestError<u16>> {
#[allow(unused_imports)]
use core::convert::TryInto;
#[allow(unused_imports)]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: this unused_imports was not in the original and should not be required.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

...alternatively... it kind of looks like you've newly qualified all references to ClientError? In which case this use might be removed.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, whoops, I didn't notice that; I can either remove the use or change them back --- have you a preference?

@hawkw hawkw merged commit c5222fe into main Mar 5, 2024
hawkw added a commit to oxidecomputer/hubris that referenced this pull request Mar 5, 2024
PR oxidecomputer/idolatry#40 updates `idol`'s code generation to use the
`quote` crate for generating Rust code, rather than representing it as a
string. This commit updates Hubris' dependency on `idol` to
oxidecomputer/idolatry@c5222fe.
cbiffle pushed a commit to oxidecomputer/hubris that referenced this pull request Mar 5, 2024
PR oxidecomputer/idolatry#40 updates `idol`'s code generation to use the
`quote` crate for generating Rust code, rather than representing it as a
string. This commit updates Hubris' dependency on `idol` to
oxidecomputer/idolatry@c5222fe.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants