Skip to content

Conversation

nvzqz
Copy link
Contributor

@nvzqz nvzqz commented Jun 3, 2022

This change attempts to make needs_drop work with types like [u8] and str.

This enables code in types like Arc<T> that was not possible before, such as #97676.

@rustbot rustbot added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Jun 3, 2022
@rust-highfive
Copy link
Contributor

Some changes occured to rustc_codegen_cranelift

cc @bjorn3

Some changes occured to rustc_codegen_gcc

cc @antoyo

Hey! It looks like you've submitted a new PR for the library teams!

If this PR contains changes to any rust-lang/rust public library APIs then please comment with r? rust-lang/libs-api @rustbot label +T-libs-api -T-libs to request review from a libs-api team reviewer. If you're unsure where your change falls no worries, just leave it as is and the reviewer will take a look and make a decision to forward on if necessary.

Examples of T-libs-api changes:

  • Stabilizing library features
  • Introducing insta-stable changes such as new implementations of existing stable traits on existing stable types
  • Introducing new or changing existing unstable library APIs (excluding permanently unstable features / features without a tracking issue)
  • Changing public documentation in ways that create new stability guarantees
  • Changing observable runtime behavior of library APIs
@rust-highfive
Copy link
Contributor

r? @oli-obk

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 3, 2022
@oli-obk
Copy link
Contributor

oli-obk commented Jun 3, 2022

The compiler's implementation already supported this. This is solely a libs-api decision on whether to expose it to users, as the change is insta-stable

r? rust-lang/libs-api

@rust-highfive rust-highfive assigned dtolnay and unassigned oli-obk Jun 3, 2022
@oli-obk
Copy link
Contributor

oli-obk commented Jun 3, 2022

@rustbot label +T-libs-api -T-libs

@rustbot rustbot added T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. and removed T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Jun 3, 2022
@dtolnay dtolnay added needs-fcp This change is insta-stable, or significant enough to need a team FCP to proceed. and removed T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jun 3, 2022
@dtolnay
Copy link
Member

dtolnay commented Jun 3, 2022

@rust-lang/libs-api:
@rfcbot fcp merge

I don't foresee this causing problems. needs_drop is defined as being allowed to be implemented conservatively, so in the worst case making it return true for "weird" unsized types would be okay.

/// This is purely an optimization hint, and may be implemented conservatively:
/// it may return `true` for types that don't actually need to be dropped.
/// As such always returning `true` would be a valid implementation of
/// this function. However if this function actually returns `false`, then you
/// can be certain dropping `T` has no side effect.

@rfcbot
Copy link

rfcbot commented Jun 3, 2022

Team member @dtolnay has proposed to merge this. The next step is review by the rest of the tagged team members:

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

@rfcbot rfcbot added proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. labels Jun 3, 2022
@bjorn3
Copy link
Member

bjorn3 commented Jun 3, 2022

I think this change can be feature gated by introducing a new unstable function with the relaxed bound and merging it back in again once it is stabilized.

@clarfonthey
Copy link
Contributor

I think this change can be feature gated by introducing a new unstable function with the relaxed bound and merging it back in again once it is stabilized.

I believe this path would be changing it on the intrinsic without changing it on the mem export, then later also changing the mem export after confidence has built up.

@rfcbot rfcbot added final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. and removed proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. labels Jun 6, 2022
@rfcbot
Copy link

rfcbot commented Jun 6, 2022

🔔 This is now entering its final comment period, as per the review above. 🔔

@rfcbot rfcbot added finished-final-comment-period The final comment period is finished for this PR / Issue. to-announce Announce this issue on triage meeting and removed final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. labels Jun 16, 2022
@rfcbot
Copy link

rfcbot commented Jun 16, 2022

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

This will be merged soon.

@apiraino apiraino removed the to-announce Announce this issue on triage meeting label Jun 16, 2022
@dtolnay
Copy link
Member

dtolnay commented Jun 16, 2022

@bors r+

@bors
Copy link
Collaborator

bors commented Jun 16, 2022

📌 Commit 24b1b7c has been approved by dtolnay

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 16, 2022
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 16, 2022
Rollup of 5 pull requests Successful merges: - rust-lang#97377 (Do not suggest adding semicolon/changing delimiters for macros in item position that originates in macros) - rust-lang#97675 (Make `std::mem::needs_drop` accept `?Sized`) - rust-lang#98118 (Test NLL fix of bad lifetime inference for reference captured in closure.) - rust-lang#98166 (Add rustdoc-json regression test for rust-lang#98009) - rust-lang#98169 (Keyword docs: Link to wikipedia article for dynamic dispatch) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
@bors bors merged commit cf68fd7 into rust-lang:master Jun 17, 2022
@rustbot rustbot added this to the 1.63.0 milestone Jun 17, 2022
@nvzqz nvzqz deleted the unsized-needs-drop branch June 17, 2022 02:33
JohnTitor added a commit to JohnTitor/rust that referenced this pull request Jul 26, 2022
Make `std::mem::needs_drop` accept `?Sized` This change attempts to make `needs_drop` work with types like `[u8]` and `str`. This enables code in types like `Arc<T>` that was not possible before, such as rust-lang#97676.
netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this pull request Oct 10, 2022
Pkgsrc changes: * Adjust patches as needed & checksum updates. Upstream changes: Version 1.63.0 (2022-08-11) ========================== Language -------- - [Remove migrate borrowck mode for pre-NLL errors.][95565] - [Modify MIR building to drop repeat expressions with length zero.][95953] - [Remove label/lifetime shadowing warnings.][96296] - [Allow explicit generic arguments in the presence of `impl Trait` args.] [96868] - [Make `cenum_impl_drop_cast` warnings deny-by-default.][97652] - [Prevent unwinding when `-C panic=abort` is used regardless of declared ABI.][96959] - [lub: don't bail out due to empty binders.][97867] Compiler -------- - [Stabilize the `bundle` native library modifier,][95818] also removing the deprecated `static-nobundle` linking kind. - [Add Apple WatchOS compile targets\*.][95243] - [Add a Windows application manifest to rustc-main.][96737] \* Refer to Rust's [platform support page][platform-support-doc] for more information on Rust's tiered platform support. Libraries --------- - [Implement `Copy`, `Clone`, `PartialEq` and `Eq` for `core::fmt::Alignment`.][94530] - [Extend `ptr::null` and `null_mut` to all thin (including extern) types.][94954] - [`impl Read and Write for VecDeque<u8>`.][95632] - [STD support for the Nintendo 3DS.][95897] - [Make write/print macros eagerly drop temporaries.][96455] - [Implement internal traits that enable `[OsStr]::join`.][96881] - [Implement `Hash` for `core::alloc::Layout`.][97034] - [Add capacity documentation for `OsString`.][97202] - [Put a bound on collection misbehavior.][97316] - [Make `std::mem::needs_drop` accept `?Sized`.][97675] - [`impl Termination for Infallible` and then make the `Result` impls of `Termination` more generic.][97803] - [Document Rust's stance on `/proc/self/mem`.][97837] Stabilized APIs --------------- - [`array::from_fn`] - [`Box::into_pin`] - [`BinaryHeap::try_reserve`] - [`BinaryHeap::try_reserve_exact`] - [`OsString::try_reserve`] - [`OsString::try_reserve_exact`] - [`PathBuf::try_reserve`] - [`PathBuf::try_reserve_exact`] - [`Path::try_exists`] - [`Ref::filter_map`] - [`RefMut::filter_map`] - [`NonNull::<[T]>::len`][`NonNull::<slice>::len`] - [`ToOwned::clone_into`] - [`Ipv6Addr::to_ipv4_mapped`] - [`unix::io::AsFd`] - [`unix::io::BorrowedFd<'fd>`] - [`unix::io::OwnedFd`] - [`windows::io::AsHandle`] - [`windows::io::BorrowedHandle<'handle>`] - [`windows::io::OwnedHandle`] - [`windows::io::HandleOrInvalid`] - [`windows::io::HandleOrNull`] - [`windows::io::InvalidHandleError`] - [`windows::io::NullHandleError`] - [`windows::io::AsSocket`] - [`windows::io::BorrowedSocket<'handle>`] - [`windows::io::OwnedSocket`] - [`thread::scope`] - [`thread::Scope`] - [`thread::ScopedJoinHandle`] These APIs are now usable in const contexts: - [`array::from_ref`] - [`slice::from_ref`] - [`intrinsics::copy`] - [`intrinsics::copy_nonoverlapping`] - [`<*const T>::copy_to`] - [`<*const T>::copy_to_nonoverlapping`] - [`<*mut T>::copy_to`] - [`<*mut T>::copy_to_nonoverlapping`] - [`<*mut T>::copy_from`] - [`<*mut T>::copy_from_nonoverlapping`] - [`str::from_utf8`] - [`Utf8Error::error_len`] - [`Utf8Error::valid_up_to`] - [`Condvar::new`] - [`Mutex::new`] - [`RwLock::new`] Cargo ----- - [Stabilize the `--config path` command-line argument.][cargo/10755] - [Expose rust-version in the environment as `CARGO_PKG_RUST_VERSION`.][cargo/10713] Compatibility Notes ------------------- - [`#[link]` attributes are now checked more strictly,][96885] which may introduce errors for invalid attribute arguments that were previously ignored. Internal Changes ---------------- These changes provide no direct user facing benefits, but represent significant improvements to the internals and overall performance of rustc and related tools. - [Prepare Rust for LLVM opaque pointers.][94214] [94214]: rust-lang/rust#94214 [94530]: rust-lang/rust#94530 [94954]: rust-lang/rust#94954 [95243]: rust-lang/rust#95243 [95565]: rust-lang/rust#95565 [95632]: rust-lang/rust#95632 [95818]: rust-lang/rust#95818 [95897]: rust-lang/rust#95897 [95953]: rust-lang/rust#95953 [96296]: rust-lang/rust#96296 [96455]: rust-lang/rust#96455 [96737]: rust-lang/rust#96737 [96868]: rust-lang/rust#96868 [96881]: rust-lang/rust#96881 [96885]: rust-lang/rust#96885 [96959]: rust-lang/rust#96959 [97034]: rust-lang/rust#97034 [97202]: rust-lang/rust#97202 [97316]: rust-lang/rust#97316 [97652]: rust-lang/rust#97652 [97675]: rust-lang/rust#97675 [97803]: rust-lang/rust#97803 [97837]: rust-lang/rust#97837 [97867]: rust-lang/rust#97867 [cargo/10713]: rust-lang/cargo#10713 [cargo/10755]: rust-lang/cargo#10755 [`array::from_fn`]: https://doc.rust-lang.org/stable/std/array/fn.from_fn.html [`Box::into_pin`]: https://doc.rust-lang.org/stable/std/boxed/struct.Box.html#method.into_pin [`BinaryHeap::try_reserve_exact`]: https://doc.rust-lang.org/stable/alloc/collections/binary_heap/struct.BinaryHeap.html#method.try_reserve_exact [`BinaryHeap::try_reserve`]: https://doc.rust-lang.org/stable/std/collections/struct.BinaryHeap.html#method.try_reserve [`OsString::try_reserve`]: https://doc.rust-lang.org/stable/std/ffi/struct.OsString.html#method.try_reserve [`OsString::try_reserve_exact`]: https://doc.rust-lang.org/stable/std/ffi/struct.OsString.html#method.try_reserve_exact [`PathBuf::try_reserve`]: https://doc.rust-lang.org/stable/std/path/struct.PathBuf.html#method.try_reserve [`PathBuf::try_reserve_exact`]: https://doc.rust-lang.org/stable/std/path/struct.PathBuf.html#method.try_reserve_exact [`Path::try_exists`]: https://doc.rust-lang.org/stable/std/path/struct.Path.html#method.try_exists [`Ref::filter_map`]: https://doc.rust-lang.org/stable/std/cell/struct.Ref.html#method.filter_map [`RefMut::filter_map`]: https://doc.rust-lang.org/stable/std/cell/struct.RefMut.html#method.filter_map [`NonNull::<slice>::len`]: https://doc.rust-lang.org/stable/std/ptr/struct.NonNull.html#method.len [`ToOwned::clone_into`]: https://doc.rust-lang.org/stable/std/borrow/trait.ToOwned.html#method.clone_into [`Ipv6Addr::to_ipv4_mapped`]: https://doc.rust-lang.org/stable/std/net/struct.Ipv6Addr.html#method.to_ipv4_mapped [`unix::io::AsFd`]: https://doc.rust-lang.org/stable/std/os/unix/io/trait.AsFd.html [`unix::io::BorrowedFd<'fd>`]: https://doc.rust-lang.org/stable/std/os/unix/io/struct.BorrowedFd.html [`unix::io::OwnedFd`]: https://doc.rust-lang.org/stable/std/os/unix/io/struct.OwnedFd.html [`windows::io::AsHandle`]: https://doc.rust-lang.org/stable/std/os/windows/io/trait.AsHandle.html [`windows::io::BorrowedHandle<'handle>`]: https://doc.rust-lang.org/stable/std/os/windows/io/struct.BorrowedHandle.html [`windows::io::OwnedHandle`]: https://doc.rust-lang.org/stable/std/os/windows/io/struct.OwnedHandle.html [`windows::io::HandleOrInvalid`]: https://doc.rust-lang.org/stable/std/os/windows/io/struct.HandleOrInvalid.html [`windows::io::HandleOrNull`]: https://doc.rust-lang.org/stable/std/os/windows/io/struct.HandleOrNull.html [`windows::io::InvalidHandleError`]: https://doc.rust-lang.org/stable/std/os/windows/io/struct.InvalidHandleError.html [`windows::io::NullHandleError`]: https://doc.rust-lang.org/stable/std/os/windows/io/struct.NullHandleError.html [`windows::io::AsSocket`]: https://doc.rust-lang.org/stable/std/os/windows/io/trait.AsSocket.html [`windows::io::BorrowedSocket<'handle>`]: https://doc.rust-lang.org/stable/std/os/windows/io/struct.BorrowedSocket.html [`windows::io::OwnedSocket`]: https://doc.rust-lang.org/stable/std/os/windows/io/struct.OwnedSocket.html [`thread::scope`]: https://doc.rust-lang.org/stable/std/thread/fn.scope.html [`thread::Scope`]: https://doc.rust-lang.org/stable/std/thread/struct.Scope.html [`thread::ScopedJoinHandle`]: https://doc.rust-lang.org/stable/std/thread/struct.ScopedJoinHandle.html [`array::from_ref`]: https://doc.rust-lang.org/stable/std/array/fn.from_ref.html [`slice::from_ref`]: https://doc.rust-lang.org/stable/std/slice/fn.from_ref.html [`intrinsics::copy`]: https://doc.rust-lang.org/stable/std/intrinsics/fn.copy.html [`intrinsics::copy_nonoverlapping`]: https://doc.rust-lang.org/stable/std/intrinsics/fn.copy_nonoverlapping.html [`<*const T>::copy_to`]: https://doc.rust-lang.org/stable/std/primitive.pointer.html#method.copy_to [`<*const T>::copy_to_nonoverlapping`]: https://doc.rust-lang.org/stable/std/primitive.pointer.html#method.copy_to_nonoverlapping [`<*mut T>::copy_to`]: https://doc.rust-lang.org/stable/std/primitive.pointer.html#method.copy_to-1 [`<*mut T>::copy_to_nonoverlapping`]: https://doc.rust-lang.org/stable/std/primitive.pointer.html#method.copy_to_nonoverlapping-1 [`<*mut T>::copy_from`]: https://doc.rust-lang.org/stable/std/primitive.pointer.html#method.copy_from [`<*mut T>::copy_from_nonoverlapping`]: https://doc.rust-lang.org/stable/std/primitive.pointer.html#method.copy_from_nonoverlapping [`str::from_utf8`]: https://doc.rust-lang.org/stable/std/str/fn.from_utf8.html [`Utf8Error::error_len`]: https://doc.rust-lang.org/stable/std/str/struct.Utf8Error.html#method.error_len [`Utf8Error::valid_up_to`]: https://doc.rust-lang.org/stable/std/str/struct.Utf8Error.html#method.valid_up_to [`Condvar::new`]: https://doc.rust-lang.org/stable/std/sync/struct.Condvar.html#method.new [`Mutex::new`]: https://doc.rust-lang.org/stable/std/sync/struct.Mutex.html#method.new [`RwLock::new`]: https://doc.rust-lang.org/stable/std/sync/struct.RwLock.html#method.new
matthiaskrgr pushed a commit to matthiaskrgr/rust that referenced this pull request Mar 7, 2023
Make `std::mem::needs_drop` accept `?Sized` This change attempts to make `needs_drop` work with types like `[u8]` and `str`. This enables code in types like `Arc<T>` that was not possible before, such as rust-lang#97676.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. finished-final-comment-period The final comment period is finished for this PR / Issue. needs-fcp This change is insta-stable, or significant enough to need a team FCP to proceed. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
10 participants