Skip to content

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
Contributor

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