-
Notifications
You must be signed in to change notification settings - Fork 13.2k
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
std: Remove deprecated ptr functions #23503
Conversation
r? @brson (rust_highfive has picked a reviewer for you, use r? to override) |
#[stable(feature = "rust1", since = "1.0.0")] | ||
pub fn copy_memory<T>(dst: *mut T, src: *const T, count: usize); | ||
|
||
/// Invokes memset on the specified pointer, setting `count * size_of::<T>()` | ||
/// bytes of memory starting at `dst` to `c`. | ||
#[cfg(not(stage0))] | ||
#[stable(feature = "rust1", since = "1.0.0")] | ||
pub fn write_bytes<T>(dst: *mut T, val: u8, count: usize); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe set_all
is a better name? Closer to memset.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR isn't changing the names, though. It's just bringing the intrinsics in line with the already-stable names to improve the rustdoc output.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(FWIW, I do like this suggestion. But it's probably too late to change at this point.)
63ec635
to
581757f
Compare
@@ -44,6 +44,10 @@ | |||
|
|||
use marker::Sized; | |||
|
|||
#[cfg(stage0)] pub use self::copy_memory as copy; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
// SNAP
comment please
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I personally very much prefer to not use // SNAP
as in the past it has ended up causing lots of pain when registering a new snapshot. It simultaneously forces the snapshot-maker to do the work necessary to remove it (which can be quite large and isn't always necessary) and also causes extra pain if a // SNAP
annotation was added after the new snapshot was made.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we have some way of marking that lines should be removed with the new snapshot? The amount of permanently-#[cfg(stage0)]
d code is probably going to keep increasing, and the snapshot maker has to go through this each time.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The string stage0
is enough, I regularly clean out via git grep stage0
whenever a new snapshot is registered. We have very little permanent #[cfg(stage0)]
☔ The latest upstream changes (presumably #23470) made this pull request unmergeable. Please resolve the merge conflicts. |
The method with which backwards compatibility was retained ended up leading to documentation that rustdoc didn't handle well and largely ended up confusing.
581757f
to
e24fe5b
Compare
r? @aturon |
The method with which backwards compatibility was retained ended up leading to documentation that rustdoc didn't handle well and largely ended up confusing.
The method with which backwards compatibility was retained ended up leading to
documentation that rustdoc didn't handle well and largely ended up confusing.