Skip to content

std: Align raw modules with unsafe conventions #19152

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

Merged
merged 1 commit into from
Nov 23, 2014

Conversation

alexcrichton
Copy link
Member

This commit is an implementation of RFC 240 when applied to the standard
library. It primarily deprecates the entirety of string::raw, vec::raw,
slice::raw, and str::raw in favor of associated functions, methods, and
other free functions. The detailed renaming is:

  • slice::raw::buf_as_slice => slice::from_raw_buf
  • slice::raw::mut_buf_as_slice => slice::from_raw_mut_buf
  • slice::shift_ptr => deprecated with no replacement
  • slice::pop_ptr => deprecated with no replacement
  • str::raw::from_utf8 => str::from_utf8_unchecked
  • str::raw::c_str_to_static_slice => str::from_c_str
  • str::raw::slice_bytes => deprecated for slice_unchecked (slight semantic diff)
  • str::raw::slice_unchecked => str.slice_unchecked
  • string::raw::from_parts => String::from_raw_parts
  • string::raw::from_buf_len => String::from_raw_buf_len
  • string::raw::from_buf => String::from_raw_buf
  • string::raw::from_utf8 => String::from_utf8_unchecked
  • vec::raw::from_buf => Vec::from_raw_buf

All previous functions exist in their #[deprecated] form, and the deprecation
messages indicate how to migrate to the newer variants.

[breaking-change]

Closes #17863

@rust-highfive
Copy link
Contributor

warning Warning warning

  • These commits modify unsafe code. Please review it carefully!

@alexcrichton
Copy link
Member Author

Note that I took a few liberties with functions like c_str_to_static_slice and buf_as_slice, and I'm curious what others think in terms of the final APIs

@Gankra
Copy link
Contributor

Gankra commented Nov 20, 2014

🙊 🎉 (reading now)

/// Creates a `String` from a `*const u8` buffer of the given length.
///
/// This function is unsafe because of two reasons:
/// * A raw pointer is dereferenced and transmuted to `&[u8]`;
Copy link
Contributor

Choose a reason for hiding this comment

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

Some of these you say what's actually done, others you just say "we call an unsafe thing on Vec". Might want to pick one (also no transmute to a &[u8] every actually occurs).

Copy link
Member Author

Choose a reason for hiding this comment

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

Hm yeah I tweaked the implementation in copy/pasting, so I'll clarify these unsafety points.

@alexcrichton alexcrichton force-pushed the issue-17863 branch 2 times, most recently from 8299549 to 591fd66 Compare November 21, 2014 03:08
@alexcrichton
Copy link
Member Author

Ok @gankro, updated with from_raw_buf as we were discussing on IRC, plus I tried to get all your other comments as well. Thanks for the review!

@Gankra
Copy link
Contributor

Gankra commented Nov 21, 2014

@_@ rebased. Just skimming but I think everything's been addressed.

r=me but I dunno if you want a second opinion or anything.

@alexcrichton
Copy link
Member Author

r? @aturon, just want to make sure me and @gankro aren't bonkers

}
result.push_str(unsafe{raw::slice_bytes(me, last_end, me.len())});
result
replace(self.as_slice(), from, to)
Copy link
Member

Choose a reason for hiding this comment

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

Hm, I would've thought we'd deprecate the free replace fn in favor of this method.

@aturon
Copy link
Member

aturon commented Nov 21, 2014

I or github must be missing something: where is with_raw_buf?

@alexcrichton
Copy link
Member Author

Oops sorry forgot to update the description, it's not called from_raw_buf

@@ -160,14 +159,14 @@ pub fn render(w: &mut fmt::Formatter, s: &str, print_toc: bool) -> fmt::Result {

let opaque = opaque as *mut hoedown_html_renderer_state;
let my_opaque: &MyOpaque = &*((*opaque).opaque as *const MyOpaque);
slice::raw::buf_as_slice((*orig_text).data, (*orig_text).size as uint,
slice::with_raw_buf((*orig_text).data, (*orig_text).size as uint,
Copy link
Member

Choose a reason for hiding this comment

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

I can't find the definition of this function.

@aturon
Copy link
Member

aturon commented Nov 21, 2014

@alexcrichton But it seems to be referenced in the code, still?

/// ```
#[inline]
#[unstable = "just renamed from `mod raw`"]
pub unsafe fn from_raw_buf<'a, T>(p: &'a *const T, len: uint) -> &'a [T] {
Copy link
Member

Choose a reason for hiding this comment

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

I'm reasonably happy with this design; it's pragmatic.

alexcrichton added a commit to alexcrichton/rust that referenced this pull request Nov 22, 2014
This commit is an implementation of [RFC 240][rfc] when applied to the standard
library. It primarily deprecates the entirety of `string::raw`, `vec::raw`,
`slice::raw`, and `str::raw` in favor of associated functions, methods, and
other free functions. The detailed renaming is:

* slice::raw::buf_as_slice => slice::with_raw_buf
* slice::raw::mut_buf_as_slice => slice::with_raw_mut_buf
* slice::shift_ptr => deprecated with no replacement
* slice::pop_ptr => deprecated with no replacement
* str::raw::from_utf8 => str::from_utf8_unchecked
* str::raw::c_str_to_static_slice => str::from_c_str
* str::raw::slice_bytes => deprecated for slice_unchecked (slight semantic diff)
* str::raw::slice_unchecked => str.slice_unchecked
* string::raw::from_parts => String::from_raw_parts
* string::raw::from_buf_len => String::from_raw_buf_len
* string::raw::from_buf => String::from_raw_buf
* string::raw::from_utf8 => String::from_utf8_unchecked
* vec::raw::from_buf => Vec::from_raw_buf

All previous functions exist in their `#[deprecated]` form, and the deprecation
messages indicate how to migrate to the newer variants.

[rfc]: https://github.com/rust-lang/rfcs/blob/master/text/0240-unsafe-api-location.md
[breaking-change]

Closes rust-lang#17863
bors added a commit that referenced this pull request Nov 23, 2014
This commit is an implementation of [RFC 240][rfc] when applied to the standard
library. It primarily deprecates the entirety of `string::raw`, `vec::raw`,
`slice::raw`, and `str::raw` in favor of associated functions, methods, and
other free functions. The detailed renaming is:

* slice::raw::buf_as_slice => slice::from_raw_buf
* slice::raw::mut_buf_as_slice => slice::from_raw_mut_buf
* slice::shift_ptr => deprecated with no replacement
* slice::pop_ptr => deprecated with no replacement
* str::raw::from_utf8 => str::from_utf8_unchecked
* str::raw::c_str_to_static_slice => str::from_c_str
* str::raw::slice_bytes => deprecated for slice_unchecked (slight semantic diff)
* str::raw::slice_unchecked => str.slice_unchecked
* string::raw::from_parts => String::from_raw_parts
* string::raw::from_buf_len => String::from_raw_buf_len
* string::raw::from_buf => String::from_raw_buf
* string::raw::from_utf8 => String::from_utf8_unchecked
* vec::raw::from_buf => Vec::from_raw_buf

All previous functions exist in their `#[deprecated]` form, and the deprecation
messages indicate how to migrate to the newer variants.

[rfc]: https://github.com/rust-lang/rfcs/blob/master/text/0240-unsafe-api-location.md
[breaking-change]

Closes #17863
@bors bors closed this Nov 23, 2014
@bors bors merged commit 8ca27a6 into rust-lang:master Nov 23, 2014
@alexcrichton alexcrichton deleted the issue-17863 branch December 28, 2014 06:43
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.

Tighten up conventions with unsafe apis
6 participants