Skip to content

Conversation

@joshlf
Copy link
Member

@joshlf joshlf commented Oct 9, 2022

The include_bytes! macro is like the macro from core of the same name, except that it transmutes the file's contents to an arbitrary type (which implements FromBytes).

TODO:

  • Change the example usage in the doc comment to use something more complex like the fixed-length header of a file format?
    • As discussed here, better to have a focused example that doesn't distract with other details.
  • Are there other variants we should introduce? Perhaps one for slice types? Can we design it in such a way that it would work with other (non-slice) DSTs in the future if we add support for those?

@joshlf joshlf force-pushed the include-file branch 4 times, most recently from 0f41a34 to 7dcafab Compare November 10, 2022 01:59
@joshlf joshlf force-pushed the include-file branch 2 times, most recently from c53040a to 7790cfd Compare October 24, 2023 14:48
@joshlf joshlf requested a review from jswrenn October 24, 2023 14:50
@joshlf joshlf force-pushed the include-file branch 2 times, most recently from f651691 to 5a1ab31 Compare October 24, 2023 15:02
Copy link
Collaborator

@jswrenn jswrenn left a comment

Choose a reason for hiding this comment

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

Nice! When I blogged about this a few years ago, I opted to match the standard library's behavior of returning a reference. I.e., this:

#[macro_export]
macro_rules! include_bytes {
    ($file:expr $(,)?) => {
        &$crate::transmute!(*::core::include_bytes!($file))
    };
}

I think it's fine to deviate from the standard library, but we probably should have some principled idea of why we're doing so.

@joshlf joshlf force-pushed the include-file branch 3 times, most recently from 3bae012 to 70a3819 Compare October 24, 2023 16:47
@joshlf
Copy link
Member Author

joshlf commented Oct 24, 2023

Nice! When I blogged about this a few years ago, I opted to match the standard library's behavior of returning a reference. I.e., this:

#[macro_export]
macro_rules! include_bytes {
    ($file:expr $(,)?) => {
        &$crate::transmute!(*::core::include_bytes!($file))
    };
}

I think it's fine to deviate from the standard library, but we probably should have some principled idea of why we're doing so.

tbh I think the stdlib is wrong here, but I agree that we should mirror it unless we have a good reason to diverge. Done.

@joshlf
Copy link
Member Author

joshlf commented Oct 24, 2023

Nice! When I blogged about this a few years ago, I opted to match the standard library's behavior of returning a reference. I.e., this:

#[macro_export]
macro_rules! include_bytes {
    ($file:expr $(,)?) => {
        &$crate::transmute!(*::core::include_bytes!($file))
    };
}

I think it's fine to deviate from the standard library, but we probably should have some principled idea of why we're doing so.

tbh I think the stdlib is wrong here, but I agree that we should mirror it unless we have a good reason to diverge. Done.

Actually wait: what about non-Copy types? In the stdlib's case, the type is always [u8; N], and thus always Copy. We don't have that luxury, and so returning a reference would presumably mean that users of non-Copy types could never dereference the return value.

@jswrenn
Copy link
Collaborator

jswrenn commented Oct 24, 2023

Agreed — I think that's a good reason for us to not return a reference.

@joshlf
Copy link
Member Author

joshlf commented Oct 24, 2023

Agreed — I think that's a good reason for us to not return a reference.

Done.

@joshlf joshlf force-pushed the include-file branch 3 times, most recently from b7e22ea to 9698b74 Compare October 24, 2023 20:17
jswrenn
jswrenn previously approved these changes Oct 24, 2023
The `include_bytes!` macro is like the macro from `core` of the same
name, except that it transmutes the file's contents to an arbitrary type
(which implements `FromBytes`).

Release 0.7.13.
@joshlf joshlf added this pull request to the merge queue Oct 24, 2023
Merged via the queue into main with commit db245f1 Oct 24, 2023
@joshlf joshlf deleted the include-file branch October 24, 2023 20:46
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