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 safety documentation #2344

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Add safety documentation #2344

wants to merge 3 commits into from

Conversation

aumetra
Copy link
Member

@aumetra aumetra commented Jan 21, 2025

Closes #2305

I hope this documentation makes it a little clearer why the usage here is sound.
Best proof-read by someone that isn't me to make sure what I wrote is actually coherent for others.

@aumetra
Copy link
Member Author

aumetra commented Jan 21, 2025

I guess the part where we say "pointer is required to be non-null can be covered by simply utilizing the std::ptr::NonNull type. Also opens up some fun Option optimizations (not that we really need them) where Option<Region> == Region since they can encode the Option tag as ptr == ptr::null()

packages/std/src/memory.rs Outdated Show resolved Hide resolved
///
/// This function requires the following invariants to be upheld:
/// - `capacity` is smaller or equal to `length`
/// - The number of bytes allocated by the pointer must be equal to `capacity`
Copy link
Collaborator

Choose a reason for hiding this comment

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

This one is only the case for Region<Owned>, right? Because a slice does not uphold this.

/// - `capacity` is smaller or equal to `length`
/// - The number of bytes allocated by the pointer must be equal to `capacity`
/// - The byte range covered by `length` must be initialized
/// - `ptr` is a non-dangling and non-null pointer
Copy link
Collaborator

Choose a reason for hiding this comment

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

What do we need the non-dangling part for? I'm pretty sure vec![].as_ptr() creates a dangling pointer

packages/std/src/memory.rs Outdated Show resolved Hide resolved
Co-authored-by: Christoph Otter <[email protected]>
@chipshort
Copy link
Collaborator

simply utilizing the std::ptr::NonNull type

That sounds nice. I'm always a fan of enforcing correct usage through the type system where possible.

Co-authored-by: Christoph Otter <[email protected]>
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.

Maybe unsound in as_bytes
2 participants