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 tests for compilation failure #17

Closed
63 tasks
joshlf opened this issue Sep 30, 2022 · 8 comments
Closed
63 tasks

Add tests for compilation failure #17

joshlf opened this issue Sep 30, 2022 · 8 comments
Labels
compatibility-nonbreaking Changes that are (likely to be) non-breaking experience-medium This issue is of medium difficulty, and requires some experience

Comments

@joshlf
Copy link
Member

joshlf commented Sep 30, 2022

We use the trybuild crate to test that zerocopy-derive correctly rejects code that it ought to. We have the same running for zerocopy itself, but no tests have been written to make use of it. zerocopy's API uses trait bounds to make certain illegal code impossible to express; we should write trybuild tests that make sure that each illegal code pattern actually fails to compile.

Contributing

If you'd like to contribute to this issue, just look through zerocopy's API, find unsound behavior that ought to be impossible to express, and write a test to confirm that it's inexpressible!

If you're not sure where to start, consider:

  • Does the API have any documented preconditions which ought to be enforced at compile time?
  • Does the API have any trait bounds?

Write a test which fails to uphold one of these requirements.

Here's a list of all of the APIs in zerocopy that have preconditions or trait bounds:

  • FromBytes (@IsaacCloos assigned)
    • read_from
    • read_from_prefix
    • read_from_suffix
  • AsBytes
    • as_bytes_mut
    • write_to
    • write_to_prefix
    • write_to_suffix
  • LayoutVerified
    • new
    • new_from_prefix
    • new_from_suffix
    • new_slice
    • new_slice_from_prefix
    • new_slice_from_suffix
    • new_zeroed
    • new_from_prefix_zeroed
    • new_from_suffix_zeroed
    • new_slice_zeroed
    • new_slice_from_prefix_zeroed
    • new_slice_from_suffix_zeroed
    • new_unaligned
    • new_unaligned_from_prefix
    • new_unaligned_from_suffix
    • new_slice_unaligned
    • new_slice_unaligned_from_prefix
    • new_slice_unaligned_from_suffix
    • new_unaligned_zeroed
    • new_unaligned_from_prefix_zeroed
    • new_unaligned_from_suffix_zeroed
    • new_slice_unaligned_zeroed
    • new_slice_unaligned_from_prefix_zeroed
    • new_slice_unaligned_from_suffix_zeroed
    • into_ref
    • into_mut
    • into_slice
    • into_mut_slice
    • bytes
    • bytes_mut
    • read
    • write
    • Debug for LayoutVerified<B, T>
    • Debug for LayoutVerified<B, [T]>
    • Deref for LayoutVerified<B, T>
    • Deref for LayoutVerified<B, [T]>
    • DerefMut for LayoutVerified<B, T>
    • DerefMut for LayoutVerified<B, [T]>
    • Display for LayoutVerified<B, T>
    • Display for LayoutVerified<B, [T]>
    • Ord for LayoutVerified<B, T>
    • Ord for LayoutVerified<B, [T]>
    • PartialOrd for LayoutVerified<B, T>
    • PartialOrd for LayoutVerified<B, [T]>
    • Eq for LayoutVerified<B, T>
    • Eq for LayoutVerified<B, [T]>
    • PartialEq for LayoutVerified<B, T>
    • PartialEq for LayoutVerified<B, [T]>
  • Unalign
    • get
    • AsBytes for Unalign<T>
    • FromBytes for Unalign<T>
    • Copy for Unalign<T>
    • Clone for Unalign<T>
  • extend_vec_zeroed
  • insert_vec_zeroed
  • transmute! macro
joshlf added a commit that referenced this issue Sep 30, 2022
This commit just adds the framework necessary to run these tests, but
does not add any compile-tests. Tests will need to be added to complete
 #17.
joshlf added a commit that referenced this issue Oct 1, 2022
This commit just adds the framework necessary to run these tests, but
does not add any compile-tests. Tests will need to be added to complete
 #17.
@joshlf joshlf changed the title Add compile-tests Add tests for compilation failure Oct 2, 2022
@IsaacCloos
Copy link

Hello, this issue wasn't referenced in This Week in Rust 464, but if you're open to assistance I'd be happy to contribute however might be valuable 😄

@joshlf joshlf added experience-medium This issue is of medium difficulty, and requires some experience Hacktoberfest and removed Hacktoberfest experience-medium This issue is of medium difficulty, and requires some experience labels Oct 16, 2022
@joshlf
Copy link
Member Author

joshlf commented Oct 16, 2022

Awesome, thanks @IsaacCloos! Welcome to zerocopy 😃

I've updated the description with more detailed instructions. Let me know which of the APIs you're planning on tackling so I can put your username next to them - we might have multiple people tackling different APIs. Let me know if you have any questions!

@IsaacCloos
Copy link

Wow! Documentation eruption 🌋 You can put me down for the FromBytes trait to start. I do have a few questions brewing, but I'll let them marinate before following up 👍🏻

Also, if at any point the timeline of completing this work isn't to your liking, just lmk and take me off 😄

@joshlf
Copy link
Member Author

joshlf commented Oct 17, 2022

Will do! And no rush - this is just "nice to have" stuff. There's no time pressure 😃

@Atixx
Copy link

Atixx commented Oct 21, 2022

Hey there! Seems like there's plenty to do here, if it seems like something more than one person can tackle, I'd like to take on another API

@joshlf
Copy link
Member Author

joshlf commented Oct 21, 2022

Definitely! Just lmk which you'd like to take and I'll put your name next to it.

@IsaacCloos
Copy link

I see since making my draft you have further defined the expected layout of the ui tests for zerocopy. I will adapt my tests to your format in a new PR after work today.

Feedback on the initial few I made would mean a lot, and help improve the quality of the rest to come 😄

@IsaacCloos IsaacCloos removed their assignment Jan 1, 2023
joshlf added a commit that referenced this issue Aug 3, 2023
This commit just adds the framework necessary to run these tests, but
does not add any compile-tests. Tests will need to be added to complete
 #17.
@joshlf joshlf added the compatibility-nonbreaking Changes that are (likely to be) non-breaking label Aug 12, 2023
@joshlf
Copy link
Member Author

joshlf commented Aug 14, 2023

At this point, we have the tests that we need. We could always add more, but it doesn't make sense to leave this as a standing TODO.

@joshlf joshlf closed this as completed Aug 14, 2023
@joshlf joshlf mentioned this issue Aug 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compatibility-nonbreaking Changes that are (likely to be) non-breaking experience-medium This issue is of medium difficulty, and requires some experience
Projects
None yet
Development

No branches or pull requests

3 participants