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 ptr::{str_from_raw_parts, str_from_raw_parts_mut} functions #91216

Closed

Conversation

WaffleLapkin
Copy link
Member

@WaffleLapkin WaffleLapkin commented Nov 25, 2021

TL;DR: this PR adds the following API:

// core::ptr
pub const fn str_from_raw_parts(data: *const u8, len: usize) -> *const str;
pub const fn str_from_raw_parts_mut(data: *mut u8, len: usize) -> *mut str;

impl NonNull<str> {
    pub const fn str_from_raw_parts(data: NonNull<u8>, len: usize) -> Self;
}

The functions are under feature gate str_from_raw_parts and are similar to
slice_from_raw_parts, slice_from_raw_parts_mut and NonNull::slice_from_raw_parts.


These methods were originally proposed as a part of #85816 and there were some concerns about the specific design we should use for str pointer construction.

Some alternative designs:

  • "Blessed cast"
    // core::ptr
    pub const fn str_from_raw_utf8(raw: *const [u8]) -> *const str;
    pub const fn str_from_raw_utf8_mut(raw: *mut [u8]) -> *mut str;
    // Also NonNull?
    Pros: similar to already existing &str construction (str::from_utf8)
    Cons: forces to use 2 functions (str_from_raw_utf8(slice_from_raw_parts(ptr, len))) to convert ptr, len into *const str
  • Add str::from_raw_parts too (addition to the current design of this PR)
    // core::str
    pub const unsafe fn from_raw_parts<'a>(ptr: *const u8, len: usize) -> &'a str;
    pub const unsafe fn from_raw_parts_mut<'a>(ptr: *mut u8, len: usize) -> &'a mut str;
    Pros: similar to slices
    Cons: adds a function to str which can be replaced with slice::from_raw_parts and str::from_utf8_unchecked (in this case having two functions is probably better, as it splits safety conditions into smaller parts)
  • Do nothing, use ptr::from_raw_parts
    Pros: doesn't add anything new, reuses existing API
    Cons: less clear/longer stabilization path

The functions are under feature gate `str_from_raw_parts` and are similar to
`slice_from_raw_parts`, `slice_from_raw_parts_mut`.
@rust-highfive
Copy link
Collaborator

r? @m-ou-se

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 25, 2021
@rust-log-analyzer

This comment has been minimized.

@apiraino apiraino added the T-libs Relevant to the library team, which will review and decide on the PR/issue. label Dec 1, 2021
@dtolnay dtolnay added T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. and removed T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Dec 10, 2021
@m-ou-se
Copy link
Member

m-ou-se commented Dec 16, 2021

r? @dtolnay

@rust-highfive rust-highfive assigned dtolnay and unassigned m-ou-se Dec 16, 2021
Copy link
Member

@dtolnay dtolnay left a comment

Choose a reason for hiding this comment

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

I would prefer not to add these. In the long term, ptr::from_raw_parts should be fine for this use case. In the short term, it sounded like the only immediate motivation for adding these APIs was:

#85816 (comment)
it seemed weird to have methods on *const str without a convenient way to create such pointers

This isn't compelling to me because people are successfully and reasonably conveniently working with *const str in real code today; they aren't the ones asking for this. A few examples:

as cast from *const [u8]:

rust/library/alloc/src/rc.rs

Lines 1818 to 1819 in 23f6923

let rc = Rc::<[u8]>::from(v.as_bytes());
unsafe { Rc::from_raw(Rc::into_raw(rc) as *const str) }

as cast from &str:

let string: &str =
unsafe { str::from_utf8_unchecked(inner.arena.alloc_slice(string.as_bytes())) };
// SAFETY: we can extend the arena allocation to `'static` because we
// only access these while the arena is still alive.
let string: &'static str = unsafe { &*(string as *const str) };

@WaffleLapkin
Copy link
Member Author

@dtolnay that makes sense. I didn't know the *const [u8] can be cast to *const str, I thought that as only worked for pointers to sized types...

Considering how easy it is to construct *const str and ptr::from_raw_parts, this proposal indeed doesn't seem to be useful enough. Closing as such.

@WaffleLapkin WaffleLapkin deleted the str_ptr_constraction branch December 20, 2021 23:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants