Stabilize const_cstr_from_ptr (CStr::from_ptr, CStr::count_bytes)#127433
Conversation
|
rustbot has assigned @workingjubilee. Use |
| /// The pointer must point to a valid buffer that contains a NUL terminator. The NUL must be | ||
| /// located within `isize::MAX` from `ptr`. | ||
| #[inline] | ||
| #[rustc_const_stable(feature = "const_cstr_from_ptr", since = "CURRENT_RUSTC_VERSION")] |
There was a problem hiding this comment.
Assuming the attribute on a nonpublic function allows using const_strlen everywhere without adding const_eval_select each time?
There was a problem hiding this comment.
I would prefer not to add such misuses of stability attributes. It makes it harder to review internal changes, because an important part of T-libs reviews when you aren't reviewing an addition API is making sure that you are not, in fact, adding API. This weakens such a defense and makes it easier for a reviewer to get confused.
There was a problem hiding this comment.
In what way is this a misuse? The lang and libs teams FCPd having a const_strlen available in some form for internal use, currently via const_eval_select but that is an implementation detail. Marking it as "stable" (but of course nonpublic) seems to accurately reflect that status, as opposed to leaking the const_eval_select implementation detail around everywhere it is used.
There was a problem hiding this comment.
Right @tgross35, I believe this is const stability working as intended. The attribute enforces that future changes to this function do not introduce any unapproved implementation, because this private function now participates in the implementation of some public stable const function and the use of any new weird const internals here would become a stable commitment. Flagging the presence of a stable commitment is the point of a stability attribute.
@RalfJung endorsed the same thing recently in #124941 (comment).
There was a problem hiding this comment.
It would probably be okay to rename const_strlen to just strlen at some point to make it more clear that's just our implementation of strlen now (avoiding confusion like #127444).
There was a problem hiding this comment.
I believe it should have a comment disclaiming its status, then, rather than passing unremarked, because this strikes me as a nonintuitive combination of attributes when placed in a public module.
Or it could be marked #[unstable], as the other function cited is.
There was a problem hiding this comment.
Thank you. "Fix this one attribute" or "oops this stabilization missed a spot" or "make this thing public that obviously was intended to be public... right?" are all fairly common PRs.
A `rustc_const_stable` attribute by itself has nonintuitive purpose when placed in a public module. Separately, it would probably be okay to rename `const_strlen` to just `strlen` to make it more clear this is our general-purpose implementation of strlen now, not something specifically for const (avoiding confusion like in PR 127444).
|
@bors r+ |
Stabilize const_cstr_from_ptr (CStr::from_ptr, CStr::count_bytes) Completed the pair of FCPs rust-lang#113219 (comment) + rust-lang#114441 (comment). `CStr::from_ptr` is covered by just the first FCP on its own. `CStr::count_bytes` requires the approval of both FCPs. The second paragraph of the first link and the last paragraph of the second link explain the relationship between the two FCPs. As both have been approved, we can proceed with stabilizing `const` on both of these already-stable functions.
…iaskrgr Rollup of 7 pull requests Successful merges: - rust-lang#124980 (Generalize `fn allocator` for Rc/Arc.) - rust-lang#126639 (Add AMX target-features and `x86_amx_intrinsics` feature flag) - rust-lang#126827 (Use pidfd_spawn for faster process spawning when a PidFd is requested) - rust-lang#127397 (fix interleaved output in the default panic hook when multiple threads panic simultaneously) - rust-lang#127433 (Stabilize const_cstr_from_ptr (CStr::from_ptr, CStr::count_bytes)) - rust-lang#127613 (Update dist-riscv64-linux to binutils 2.40) - rust-lang#127632 (Implement `precise_capturing` support for rustdoc) r? `@ghost` `@rustbot` modify labels: rollup
Stabilize const_cstr_from_ptr (CStr::from_ptr, CStr::count_bytes) Completed the pair of FCPs rust-lang#113219 (comment) + rust-lang#114441 (comment). `CStr::from_ptr` is covered by just the first FCP on its own. `CStr::count_bytes` requires the approval of both FCPs. The second paragraph of the first link and the last paragraph of the second link explain the relationship between the two FCPs. As both have been approved, we can proceed with stabilizing `const` on both of these already-stable functions.
Stabilize const_cstr_from_ptr (CStr::from_ptr, CStr::count_bytes) Completed the pair of FCPs rust-lang#113219 (comment) + rust-lang#114441 (comment). `CStr::from_ptr` is covered by just the first FCP on its own. `CStr::count_bytes` requires the approval of both FCPs. The second paragraph of the first link and the last paragraph of the second link explain the relationship between the two FCPs. As both have been approved, we can proceed with stabilizing `const` on both of these already-stable functions.
…iaskrgr Rollup of 6 pull requests Successful merges: - rust-lang#124944 (On trait bound mismatch, detect multiple crate versions in dep tree) - rust-lang#124980 (Generalize `fn allocator` for Rc/Arc.) - rust-lang#126639 (Add AMX target-features and `x86_amx_intrinsics` feature flag) - rust-lang#126827 (Use pidfd_spawn for faster process spawning when a PidFd is requested) - rust-lang#127433 (Stabilize const_cstr_from_ptr (CStr::from_ptr, CStr::count_bytes)) - rust-lang#127613 (Update dist-riscv64-linux to binutils 2.40) r? `@ghost` `@rustbot` modify labels: rollup
Stabilize const_cstr_from_ptr (CStr::from_ptr, CStr::count_bytes) Completed the pair of FCPs rust-lang#113219 (comment) + rust-lang#114441 (comment). `CStr::from_ptr` is covered by just the first FCP on its own. `CStr::count_bytes` requires the approval of both FCPs. The second paragraph of the first link and the last paragraph of the second link explain the relationship between the two FCPs. As both have been approved, we can proceed with stabilizing `const` on both of these already-stable functions.
…iaskrgr Rollup of 9 pull requests Successful merges: - rust-lang#124980 (Generalize `fn allocator` for Rc/Arc.) - rust-lang#126639 (Add AMX target-features and `x86_amx_intrinsics` feature flag) - rust-lang#126827 (Use pidfd_spawn for faster process spawning when a PidFd is requested) - rust-lang#127153 (Initial implementation of anonymous_pipe API) - rust-lang#127433 (Stabilize const_cstr_from_ptr (CStr::from_ptr, CStr::count_bytes)) - rust-lang#127552 (remove unnecessary `git` usages) - rust-lang#127613 (Update dist-riscv64-linux to binutils 2.40) - rust-lang#127627 (generalize search graph to enable fuzzing) - rust-lang#127648 (Lower timeout of CI jobs to 4 hours) r? `@ghost` `@rustbot` modify labels: rollup
…iaskrgr Rollup of 8 pull requests Successful merges: - rust-lang#124980 (Generalize `fn allocator` for Rc/Arc.) - rust-lang#126639 (Add AMX target-features and `x86_amx_intrinsics` feature flag) - rust-lang#126827 (Use pidfd_spawn for faster process spawning when a PidFd is requested) - rust-lang#127433 (Stabilize const_cstr_from_ptr (CStr::from_ptr, CStr::count_bytes)) - rust-lang#127552 (remove unnecessary `git` usages) - rust-lang#127613 (Update dist-riscv64-linux to binutils 2.40) - rust-lang#127627 (generalize search graph to enable fuzzing) - rust-lang#127648 (Lower timeout of CI jobs to 4 hours) r? `@ghost` `@rustbot` modify labels: rollup
Completed the pair of FCPs #113219 (comment) + #114441 (comment).
CStr::from_ptris covered by just the first FCP on its own.CStr::count_bytesrequires the approval of both FCPs. The second paragraph of the first link and the last paragraph of the second link explain the relationship between the two FCPs. As both have been approved, we can proceed with stabilizingconston both of these already-stable functions.