Make core::mem::needs_drop a const fn #54596
Conversation
|
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @cramertj (or someone else) soon. If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes. Please see the contribution instructions for more information. |
rust-highfive
added
the
S-waiting-on-review
| #[inline] | ||
| #[stable(feature = "needs_drop", since = "1.21.0")] | ||
| #[cfg(stage0)] | ||
| /// Ceci n'est pas la documentation |
There was a problem hiding this comment.
If #54601 merges first, you can drop this dummy non-const version.
There was a problem hiding this comment.
Hmm, though maybe not with the intrinsic safety change too.
There was a problem hiding this comment.
Thanks for the heads up. If #54601 merges first I'll be sure to sync and test with and without the dummy non-const version.
|
The stage0 version needs to stay.
…On Fri, Sep 28, 2018, 06:33 Michael Bradshaw ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In src/libcore/mem.rs
<#54596 (comment)>:
> @@ -483,6 +483,16 @@ pub fn align_of_val<T: ?Sized>(val: &T) -> usize {
/// ```
#[inline]
#[stable(feature = "needs_drop", since = "1.21.0")]
+#[rustc_const_unstable(feature = "const_needs_drop")]
+#[cfg(not(stage0))]
+pub const fn needs_drop<T>() -> bool {
+ intrinsics::needs_drop::<T>()
+}
+
+#[inline]
+#[stable(feature = "needs_drop", since = "1.21.0")]
+#[cfg(stage0)]
+/// Ceci n'est pas la documentation
Thanks for the heads up. If #54601
<#54601> merges first I'll be sure
to sync and test with and without the dummy non-const version.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#54596 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AApc0gjk-UOOYBNpNtTWAXKJJTB7bwbXks5ufZh7gaJpZM4W7Y3o>
.
|
|
r? @RalfJung you’re probably the most appropriate person to make an actual decision here. |
|
📌 Commit 2c3c882 has been approved by |
bors
added
S-waiting-on-bors
|
⌛ Testing commit 2c3c882 with merge 603d712859470adba1d6bf925214bf4c9d83da84... |
|
💔 Test failed - status-appveyor |
bors
added
S-waiting-on-review
|
I just pushed a new commit that should fix the broken test (by removing the unnecessary |
|
@bors r=RalfJung |
|
📌 Commit 43cc32f has been approved by |
bors
added
S-waiting-on-bors
Make core::mem::needs_drop a const fn This fixes #51929.
|
☀️ Test successful - status-appveyor, status-travis |
This fixes #51929.