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

add basic derive macro for Pod, Zeroable and TransparentWrapper for structs #30

Merged
merged 10 commits into from
Aug 21, 2020

Conversation

icewind1991
Copy link
Contributor

Adds derive macros for Zeroable and Pod for structs.

The macro is exported behind the derive feature flag (same way as serde does)

  • The Zeroable macro checks that all fields are Zeroable.
  • The Pod macro checks that all fields are Pod, there is no padding bytes and that the struct is #[repr(C)] or #[repr(packed)]

This currently only supports deriving for structs, deriving Zeroable would be possible in the future.

Fixes #4

@Shnatsel
Copy link

Now that rgb crate requires these trait bounds, it'd be nice to have a safe way to derive them.

@danielhenrymantilla do I recall correctly that you've come up with a way to derive these via macros-by-example only?

@Lokathor
Copy link
Owner

I'm very interested in the idea of this PR, but I won't have the time to look until later today, and I'm also a little nervous about putting proc-macros near unsafe code since I don't really use them much, so I'm going to ask around for extra eyes to look at this to make sure it's all good.

@icewind1991
Copy link
Contributor Author

added a macro for deriving TransparentWrapper

it checks that the struct is #[repr(transparent)] and contains the a field with the type specified in the trait bounds.

If the struct only contains a single field it will automatically use the type of that field for the train bound, if there is more than one field (extra zero-sized fields) you'll need to specify the type using #[transparent(T)].

For example:

#[derive(TransparentWrapper)]
#[repr(transparent)]
struct TransparentSingle {
  a: u16,
}

#[derive(TransparentWrapper)]
#[repr(transparent)]
#[transparent(u16)]
struct TransparentWithZeroSized {
  a: u16,
  b: (),
}

note that this macro is more strict that the requirements of the trait, since the trait allows for "nested" transparent wrappers:

#[repr(transparent)]
struct A(u16);

#[repr(transparent)]
struct B(A);

unsafe impl TransparentWrapper<u16> for B{};

the macro currently rejects this since I can't think of a good way to verify the constraints while allowing this

@Lokathor
Copy link
Owner

it looks like some of the output code is calling std::mem::size_of ? That at least can be core::mem::size_of.

@Shnatsel
Copy link

https://crates.io/crates/zerocopy-derive solves largely the same problem, perhaps some techniques from it can be adopted here.

@icewind1991
Copy link
Contributor Author

it looks like some of the output code is calling std::mem::size_of ? That at least can be core::mem::size_of.

Fixed

@icewind1991 icewind1991 changed the title add basic derive macro for Pod and Zeroable for structs add basic derive macro for Pod, Zeroable and WrapperType for structs Aug 17, 2020
@icewind1991 icewind1991 changed the title add basic derive macro for Pod, Zeroable and WrapperType for structs add basic derive macro for Pod, Zeroable and TransparentWrapper for structs Aug 17, 2020
@icewind1991
Copy link
Contributor Author

running

cargo expand --test basic

from the derive folder is probably the easiest way to check what checks are being generated by the macro

@Lokathor Lokathor self-assigned this Aug 18, 2020
Copy link
Contributor

@LPGhatguy LPGhatguy left a comment

Choose a reason for hiding this comment

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

Looks great to me, I really want this! 😅

derive/src/lib.rs Outdated Show resolved Hide resolved
derive/src/traits.rs Outdated Show resolved Hide resolved
derive/src/traits.rs Outdated Show resolved Hide resolved
@Lokathor Lokathor merged commit cf94445 into Lokathor:main Aug 21, 2020
@Lokathor Lokathor mentioned this pull request Aug 21, 2020
@icewind1991 icewind1991 deleted the derive-macro branch August 21, 2020 09:37
leod pushed a commit to leod/bytemuck that referenced this pull request Jun 3, 2023
…tructs (Lokathor#30)

* add basic derive macro for Pod and Zeroable for structs

* add derive macro for TransparentWrapper

* use core::mem::size_of instead of std::mem::size_of in generated code

* cleanup error handling a bit

* remove unneeded iter logic

* remove unneeded clone and order impl

* fix generics

* fix doc typo

Co-authored-by: Lucien Greathouse <me@lpghatguy.com>

* remove unneeded lifetime anotation

* use unreachable for already rejected patch

Co-authored-by: Lucien Greathouse <me@lpghatguy.com>
@Lokathor Lokathor removed their assignment Aug 26, 2024
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.

Derives for traits?
4 participants