Add and modify safety descriptions in some of intrinsics APIs #135334
Conversation
rustbot
added
S-waiting-on-review
|
Some changes occurred to the intrinsics. Make sure the CTFE / Miri interpreter cc @rust-lang/miri, @rust-lang/wg-const-eval |
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
These intrinsic docs are internal, they will never be stabilized and are not meant for users. So I don't quite understand what your goal here is. Why did you pick those 3 out of the dozens of intrinsics that don't have an explicit # Safety section? This already huge file will double in size if we do this systematically. So if you want to pursue the goal of improving our internal intrinsic safety docs, there should be some discussion with the relevant team (t-libs) first about how to best organize this.
Personally I think that yes we should document our intrinsics properly, but we don't need to make it as verbose as we do for user-facing docs. We can assume the reader of these docs to be quite knowledgeable and use advanced terminology without having to spell out everything in extensive detail.
| @@ -3965,8 +3981,16 @@ pub const fn is_val_statically_known<T: Copy>(_arg: T) -> bool { | |||
| /// The stabilized form of this intrinsic is [`crate::mem::swap`]. | |||
| /// | |||
| /// # Safety | |||
| /// Behavior is undefined if any of the following conditions are violated: | |||
There was a problem hiding this comment.
This is incomplete. The operation is typed, so the pointers must point to data that is valid at the given type.
| /// | ||
| /// * `_dst` must be properly aligned. | ||
| /// | ||
| /// Note that even if `T` has size `0`, the pointer must be properly aligned. |
There was a problem hiding this comment.
You can just point at write_bytes here, the safety requirements are the same.
Except of course they aren't really -- these are volatile operations which can be used in ways that would be invalid for write_bytes. The same applies to copy_nonoverlapping vs volatile_copy_nonoverlapping_memory.
| @@ -2042,6 +2046,18 @@ pub unsafe fn volatile_copy_memory<T>(_dst: *mut T, _src: *const T, _count: usiz | |||
| /// The volatile parameter is set to `true`, so it will not be optimized out | |||
| /// unless size is equal to zero. | |||
| /// | |||
| /// # Safety | |||
There was a problem hiding this comment.
What would be much more important do clarify here is whether both the read and the write are volatile, or only one of them. Also, "the volatile parameter is set to true" makes no sense, that's talking about a parameter inside the codegen backend that shouldn't show up in such docs. Same for llvm.memset.p0i8; maybe it's useful to mention it but the first summary should not refer to LLVM-specific concepts.
|
r? @rust-lang/opsem |
|
@rustbot author |
rustbot
added
S-waiting-on-author
|
@DiuDiu777 |
Add missing doc for intrinsic (Fix PR135334) The previous [PR135334](rust-lang#135334) mentioned that some of the intrinsic APIs were missing safety descriptions. Among intrinsic APIs that miss safety specifications, most are related to numerical operations. They might need to be discussed and then seen how to organize. Apart from them, only a few intrinsics lack safety. So this PR deals with the APIs with non-numerical operations in priority.
Rollup merge of rust-lang#138309 - DiuDiu777:intrinsic-doc-fix, r=thomcc Add missing doc for intrinsic (Fix PR135334) The previous [PR135334](rust-lang#135334) mentioned that some of the intrinsic APIs were missing safety descriptions. Among intrinsic APIs that miss safety specifications, most are related to numerical operations. They might need to be discussed and then seen how to organize. Apart from them, only a few intrinsics lack safety. So this PR deals with the APIs with non-numerical operations in priority.
Add missing doc for intrinsic (Fix PR135334) The previous [PR135334](rust-lang#135334) mentioned that some of the intrinsic APIs were missing safety descriptions. Among intrinsic APIs that miss safety specifications, most are related to numerical operations. They might need to be discussed and then seen how to organize. Apart from them, only a few intrinsics lack safety. So this PR deals with the APIs with non-numerical operations in priority.
PR Description
Many public
intrinsicsunsafe APIs lack clear or sufficiently detailed# Safetydescriptions. These updates aim to provide clearer and more accurate safety documentation for parts of these unsafe APIs.This PR includes the following changes:
volatile_copy_nonoverlapping_memory: Added a safety description aligning with that of
copy_nonoverlapping.volatile_set_memory: Added a detailed safety description specifying the validity and alignment requirements for the
_dstpointer.typed_swap_nonoverlapping: Removed the vague safety description and replaced it with the precise one, referencing
swap_nonoverlapping.