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

Procedural Macro for implementing SharedMemCast #25

Closed
wants to merge 12 commits into from

Conversation

sunjay
Copy link
Contributor

@sunjay sunjay commented Aug 2, 2019

Fixes #17.

Hello! Thanks for the great crate. I want to use this for a project and I noticed that you needed a custom derive for the SharedMemCast trait. This is really useful for using this crate without having to write the unsafe impl, so I thought I would help out and contribute the macro. :)

The size of this PR is probably quite daunting, so I'll give you an overview of what I had to do so you understand each step:

  1. All procedural macros need to go in a separate crate that is compiled before shared_memory is compiled. I called this crate shared_memory_derive and added it to the root directory of the project.
  2. In order to re-export the SharedMemCast proc macro from src/lib.rs and make using the macro convenient, I needed to upgrade the crate to the 2018 edition. I didn't want my changes to be too obtrusive, so I only added the edition = 2018 line in the Cargo.toml file and made the most minimal changes to fix any errors that cropped up. You can always use cargo fix to finish the upgrade later if you feel the need to do that.
  3. I copied all the metadata from the root Cargo.toml file into the shared_memory_derive/Cargo.toml file. I made sure that the versions of both crates are in sync. This can be useful to avoid confusion. Please make sure you publish shared_memory_derive before publishing shared_memory to avoid any problems.
  4. I then went and created a comprehensive set of tests to make sure we never accidentally let through anything that should not be casted from shared memory. I'm using the trybuild crate for that. The tests in tests/ui are all meant to produce compiler errors and the tests in tests/run-pass are all meant to compile with no errors. See the CONTRIBUTING.md file I added for more details.
    • You should go through my tests and make sure all the cases I've allowed to pass are valid
    • Can you confirm whether or not zero-sized types are allowed to be cast from shared memory? (e.g. PhantomData<T>, (), [T; 0], unit structs, etc.) I've disabled them (by producing a compiler error) because it made the most sense to me that they should not ever be casted from shared memory. The same goes for empty enums which should never be initialized.
  5. In order for these tests to pass, we need the SharedMemCast trait to be implemented for various sizes of fixed-size array. Rust has no type-level integer support just yet, so the solution that every crate uses is to have a bunch of impls for different sizes. I think we can cover a large number of use-cases by having impls for all sizes between 1 and 32, then the powers of 2 up to about 2 GB. See cast.rs for what I mean by that. I think these impls should be more than enough for most peoples' needs and we can always add more later.
  6. Finally, after all these tests and impls were added, I implemented the actual custom derive code. This code actually isn't terribly complicated. I have attempted to comment it quite well so you understand the approach and the implementation. Please let me know if you have any questions.
  7. After implementing that and getting the tests to pass, I updated the example in the examples/ directory to use #[derive] instead of unsafe impl. I also updated the docs to let people know that this feature exists. See below for a screenshot of that.
  8. I modified the Travis CI script so that it uses the --all flag. This should ensure that the custom derive tests also run on CI.

These steps can be roughly seen in my commits as well if you prefer to go through things that way. Please don't hesitate to ask me any questions about all of this and do let me know if I've missed describing any part of what I did.

Thanks again for the great library! Looking forward to using it once these changes land and everything is published. 😄

Documentation Screenshot

Only the docs for SharedMemCast were updated:

updated docs

@elast0ny
Copy link
Owner

elast0ny commented Aug 6, 2019

Thanks a lot for your work and the good documentation 👍

I should have time in the next few days to take a quick glance and merge it into master

@sunjay
Copy link
Contributor Author

sunjay commented Aug 6, 2019

Thank you! Appreciate you taking the time to look at it. :)

@sunjay
Copy link
Contributor Author

sunjay commented Aug 11, 2019

@elast0ny I realized that we were missing some impls for a few primitive types, Option<T>, Result<T>, as well as for tuples. I added the relevant impls in my most recent commit.

After some thought, I think it might also be okay to add impls of SharedMemCast for zero-sized types? I realize that it doesn't make sense to cast to something with zero size, but I believe the compiler would optimize that stuff out anyway. Not having the impl may limit people who want to cast to structs that store useful things like PhantomData. What do you think?

Here's the updated documentation screenshot for the impls I added:

image

@asajeffrey
Copy link

Please pretty please can we have this? I got hit by there being no SharedMemCast implementation for (T, U), and ended up writing a custom struct as a work-around.

@elast0ny elast0ny mentioned this pull request Sep 20, 2019
@elast0ny
Copy link
Owner

This PR was merged into a bigger PR #30

Thanks again for your work and sorry for the delays !

@elast0ny elast0ny closed this Sep 20, 2019
elast0ny added a commit that referenced this pull request Sep 20, 2019
Version 0.10.0 : 
    Implements PR #25 : You can now use a custom derive to make arbitrary struct castable to the shared memory
    Fixes PR #23 : Link file check&create is now atomic
    Implements #27 : All functions now return a custom error code instead of Box'ed strings
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.

Provide a procedural macro to cast structs onto the shmem
3 participants