-
Notifications
You must be signed in to change notification settings - Fork 13.3k
Make Unique::as_ptr
, NonNull::dangling
and NonNull::cast
const
#58750
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
Conversation
r? @Kimundi (rust_highfive has picked a reviewer for you, use r? to override) |
Please add some tests using these in a const context. Also consider making them |
Should r? @oli-obk |
Since it's unstable, there's no need |
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
@@ -2790,7 +2790,7 @@ impl<T: ?Sized> Unique<T> { | |||
} | |||
|
|||
/// Acquires the underlying `*mut` pointer. | |||
pub fn as_ptr(self) -> *mut T { | |||
pub const fn as_ptr(self) -> *mut T { |
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.
cc @Centril is it too dangerous that just adding a const
doesn't require anything like a #[const_stable(since = "1.2.3")]
attribute?
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.
Dangerous from what perspective? If it's used in a min-const-fn it will also need to be min-const-fn... or do you mean from the POV of accidental stabilization?
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.
yes, accidental stabilization. Like making something stable that is already const fn
stabilizes the constness along with it.
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.
Do you mean any stabilized const fn should have something like a #[const_stable]
attribute?
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.
Ah... idk if I find it risky rising to the level of dangerous but there's some risk, no doubt. @varkor wanted it from a different perspective (#57562 (comment)) but I thought it too much of an annotation burden. However, from the POV of accidental stabilization, #[const_stable(...)]
makes sense to me.
Can you open up an issue and cc me and varkor?
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.
@TimDiekmann Yeah, that's the idea.
@bors r+ |
📌 Commit c5f3830 has been approved by |
Make `Unique::as_ptr`, `NonNull::dangling` and `NonNull::cast` const
Make `Unique::as_ptr`, `NonNull::dangling` and `NonNull::cast` const
Make `Unique::as_ptr`, `NonNull::dangling` and `NonNull::cast` const
Failed in #58766 (comment), @bors r- |
Make `Unique::as_ptr` const without feature attribute as it's unstable Make `NonNull::dangling` and `NonNull::cast` const with `feature = "const_ptr_nonnull"`
Yes, indeed, very helpful! |
⌛ Testing commit 60a649e with merge 28660c5782de2bb06e1571d5eb9491a7316c4647... |
💔 Test failed - status-appveyor |
Seems like a spurious failure as mentioned in #58160. |
@bors retry |
⌛ Testing commit 60a649e with merge 053b2117f7e4b3bbbf1130289882f6772813bea4... |
💔 Test failed - status-appveyor |
@bors retry |
Make `Unique::as_ptr`, `NonNull::dangling` and `NonNull::cast` const
Make `Unique::as_ptr`, `NonNull::dangling` and `NonNull::cast` const
Make `Unique::as_ptr`, `NonNull::dangling` and `NonNull::cast` const
Rollup of 13 pull requests Successful merges: - #58518 (Use early unwraps instead of bubbling up errors just to unwrap in the end) - #58626 (rustdoc: add option to calculate "documentation coverage") - #58629 (rust-lldb: fix crash when printing empty string) - #58660 (MaybeUninit: add read_initialized, add examples) - #58670 (fixes #52482) - #58676 (look for python2 symlinks before bootstrap python) - #58679 (Refactor passes and pass execution to be more parallel) - #58750 (Make `Unique::as_ptr`, `NonNull::dangling` and `NonNull::cast` const) - #58762 (Mention `unwind(aborts)` in diagnostics for `#[unwind]`) - #58924 (Add as_slice() to slice::IterMut and vec::Drain) - #58990 (Actually publish miri in the manifest) - #59018 (std: Delete a by-definition spuriously failing test) - #59045 (Expose new_sub_parser_from_file) Failed merges: r? @ghost
No description provided.