From 7f2cf191917e4e562ee49ab51a324714bbb36412 Mon Sep 17 00:00:00 2001 From: joboet Date: Tue, 17 Jan 2023 14:08:35 +0100 Subject: [PATCH 01/17] refactor[std]: do not use box syntax --- library/std/src/lib.rs | 1 - library/std/src/sys/hermit/thread.rs | 4 +-- .../std/src/sys/hermit/thread_local_dtor.rs | 23 ++++--------- .../std/src/sys/solid/thread_local_dtor.rs | 32 +++++++------------ library/std/src/sys/unix/thread.rs | 2 +- library/std/src/sys/unix/thread_local_dtor.rs | 25 ++++++--------- library/std/src/sys/windows/thread.rs | 2 +- .../std/src/sys_common/thread_local_dtor.rs | 2 +- library/std/src/thread/local.rs | 3 +- 9 files changed, 35 insertions(+), 59 deletions(-) diff --git a/library/std/src/lib.rs b/library/std/src/lib.rs index a7e13f5b866b..99cc01863104 100644 --- a/library/std/src/lib.rs +++ b/library/std/src/lib.rs @@ -238,7 +238,6 @@ #![feature(allocator_internals)] #![feature(allow_internal_unsafe)] #![feature(allow_internal_unstable)] -#![feature(box_syntax)] #![feature(c_unwind)] #![feature(cfg_target_thread_local)] #![feature(concat_idents)] diff --git a/library/std/src/sys/hermit/thread.rs b/library/std/src/sys/hermit/thread.rs index 8f65544a9e89..2507f7069517 100644 --- a/library/std/src/sys/hermit/thread.rs +++ b/library/std/src/sys/hermit/thread.rs @@ -27,10 +27,10 @@ impl Thread { p: Box, core_id: isize, ) -> io::Result { - let p = Box::into_raw(box p); + let p = Box::into_raw(Box::new(p)); let tid = abi::spawn2( thread_start, - p as usize, + p.expose_addr(), abi::Priority::into(abi::NORMAL_PRIO), stack, core_id, diff --git a/library/std/src/sys/hermit/thread_local_dtor.rs b/library/std/src/sys/hermit/thread_local_dtor.rs index 9b683fce1574..613266b9530a 100644 --- a/library/std/src/sys/hermit/thread_local_dtor.rs +++ b/library/std/src/sys/hermit/thread_local_dtor.rs @@ -5,32 +5,23 @@ // The this solution works like the implementation of macOS and // doesn't additional OS support -use crate::cell::Cell; -use crate::ptr; +use crate::mem; #[thread_local] -static DTORS: Cell<*mut List> = Cell::new(ptr::null_mut()); - -type List = Vec<(*mut u8, unsafe extern "C" fn(*mut u8))>; +static mut DTORS: Vec<(*mut u8, unsafe extern "C" fn(*mut u8))> = Vec::new(); pub unsafe fn register_dtor(t: *mut u8, dtor: unsafe extern "C" fn(*mut u8)) { - if DTORS.get().is_null() { - let v: Box = box Vec::new(); - DTORS.set(Box::into_raw(v)); - } - - let list: &mut List = &mut *DTORS.get(); + let list = &mut DTORS; list.push((t, dtor)); } // every thread call this function to run through all possible destructors pub unsafe fn run_dtors() { - let mut ptr = DTORS.replace(ptr::null_mut()); - while !ptr.is_null() { - let list = Box::from_raw(ptr); - for (ptr, dtor) in list.into_iter() { + let mut list = mem::take(&mut DTORS); + while !list.is_empty() { + for (ptr, dtor) in list { dtor(ptr); } - ptr = DTORS.replace(ptr::null_mut()); + list = mem::take(&mut DTORS); } } diff --git a/library/std/src/sys/solid/thread_local_dtor.rs b/library/std/src/sys/solid/thread_local_dtor.rs index 973564570577..bad14bb37f72 100644 --- a/library/std/src/sys/solid/thread_local_dtor.rs +++ b/library/std/src/sys/solid/thread_local_dtor.rs @@ -5,43 +5,35 @@ use super::{abi, itron::task}; use crate::cell::Cell; -use crate::ptr; +use crate::mem; #[thread_local] -static DTORS: Cell<*mut List> = Cell::new(ptr::null_mut()); +static REGISTERED: Cell = Cell::new(false); -type List = Vec<(*mut u8, unsafe extern "C" fn(*mut u8))>; +#[thread_local] +static mut DTORS: Vec<(*mut u8, unsafe extern "C" fn(*mut u8))> = Vec::new(); pub unsafe fn register_dtor(t: *mut u8, dtor: unsafe extern "C" fn(*mut u8)) { - if DTORS.get().is_null() { + if !REGISTERED.get() { let tid = task::current_task_id_aborting(); - let v: Box = box Vec::new(); - DTORS.set(Box::into_raw(v)); - // Register `tls_dtor` to make sure the TLS destructors are called // for tasks created by other means than `std::thread` unsafe { abi::SOLID_TLS_AddDestructor(tid as i32, tls_dtor) }; + REGISTERED.set(true); } - let list: &mut List = unsafe { &mut *DTORS.get() }; + let list = unsafe { &mut DTORS }; list.push((t, dtor)); } pub unsafe fn run_dtors() { - let ptr = DTORS.get(); - if !ptr.is_null() { - // Swap the destructor list, call all registered destructors, - // and repeat this until the list becomes permanently empty. - while let Some(list) = Some(crate::mem::replace(unsafe { &mut *ptr }, Vec::new())) - .filter(|list| !list.is_empty()) - { - for (ptr, dtor) in list.into_iter() { - unsafe { dtor(ptr) }; - } + let mut list = mem::take(unsafe { &mut DTORS }); + while !list.is_empty() { + for (ptr, dtor) in list { + unsafe { dtor(ptr) }; } - // Drop the destructor list - unsafe { Box::from_raw(DTORS.replace(ptr::null_mut())) }; + list = mem::take(unsafe { &mut DTORS }); } } diff --git a/library/std/src/sys/unix/thread.rs b/library/std/src/sys/unix/thread.rs index 2a1830d060ed..cc0e59295697 100644 --- a/library/std/src/sys/unix/thread.rs +++ b/library/std/src/sys/unix/thread.rs @@ -49,7 +49,7 @@ unsafe impl Sync for Thread {} impl Thread { // unsafe: see thread::Builder::spawn_unchecked for safety requirements pub unsafe fn new(stack: usize, p: Box) -> io::Result { - let p = Box::into_raw(box p); + let p = Box::into_raw(Box::new(p)); let mut native: libc::pthread_t = mem::zeroed(); let mut attr: libc::pthread_attr_t = mem::zeroed(); assert_eq!(libc::pthread_attr_init(&mut attr), 0); diff --git a/library/std/src/sys/unix/thread_local_dtor.rs b/library/std/src/sys/unix/thread_local_dtor.rs index d7fd2130f7cc..c31fb3a48dab 100644 --- a/library/std/src/sys/unix/thread_local_dtor.rs +++ b/library/std/src/sys/unix/thread_local_dtor.rs @@ -57,39 +57,34 @@ pub unsafe fn register_dtor(t: *mut u8, dtor: unsafe extern "C" fn(*mut u8)) { #[cfg(target_os = "macos")] pub unsafe fn register_dtor(t: *mut u8, dtor: unsafe extern "C" fn(*mut u8)) { use crate::cell::Cell; + use crate::mem; use crate::ptr; #[thread_local] static REGISTERED: Cell = Cell::new(false); + + #[thread_local] + static mut DTORS: Vec<(*mut u8, unsafe extern "C" fn(*mut u8))> = Vec::new(); + if !REGISTERED.get() { _tlv_atexit(run_dtors, ptr::null_mut()); REGISTERED.set(true); } - type List = Vec<(*mut u8, unsafe extern "C" fn(*mut u8))>; - - #[thread_local] - static DTORS: Cell<*mut List> = Cell::new(ptr::null_mut()); - if DTORS.get().is_null() { - let v: Box = box Vec::new(); - DTORS.set(Box::into_raw(v)); - } - extern "C" { fn _tlv_atexit(dtor: unsafe extern "C" fn(*mut u8), arg: *mut u8); } - let list: &mut List = &mut *DTORS.get(); + let list = &mut DTORS; list.push((t, dtor)); unsafe extern "C" fn run_dtors(_: *mut u8) { - let mut ptr = DTORS.replace(ptr::null_mut()); - while !ptr.is_null() { - let list = Box::from_raw(ptr); - for (ptr, dtor) in list.into_iter() { + let mut list = mem::take(&mut DTORS); + while !list.is_empty() { + for (ptr, dtor) in list { dtor(ptr); } - ptr = DTORS.replace(ptr::null_mut()); + list = mem::take(&mut DTORS); } } } diff --git a/library/std/src/sys/windows/thread.rs b/library/std/src/sys/windows/thread.rs index 1cb576c95947..ed58c47e0907 100644 --- a/library/std/src/sys/windows/thread.rs +++ b/library/std/src/sys/windows/thread.rs @@ -22,7 +22,7 @@ pub struct Thread { impl Thread { // unsafe: see thread::Builder::spawn_unchecked for safety requirements pub unsafe fn new(stack: usize, p: Box) -> io::Result { - let p = Box::into_raw(box p); + let p = Box::into_raw(Box::new(p)); // FIXME On UNIX, we guard against stack sizes that are too small but // that's because pthreads enforces that stacks are at least diff --git a/library/std/src/sys_common/thread_local_dtor.rs b/library/std/src/sys_common/thread_local_dtor.rs index 1d13a7171b03..844946eda031 100644 --- a/library/std/src/sys_common/thread_local_dtor.rs +++ b/library/std/src/sys_common/thread_local_dtor.rs @@ -30,7 +30,7 @@ pub unsafe fn register_dtor_fallback(t: *mut u8, dtor: unsafe extern "C" fn(*mut static DTORS: StaticKey = StaticKey::new(Some(run_dtors)); type List = Vec<(*mut u8, unsafe extern "C" fn(*mut u8))>; if DTORS.get().is_null() { - let v: Box = box Vec::new(); + let v: Box = Box::new(Vec::new()); DTORS.set(Box::into_raw(v) as *mut u8); } let list: &mut List = &mut *(DTORS.get() as *mut List); diff --git a/library/std/src/thread/local.rs b/library/std/src/thread/local.rs index b30bb7b77efb..cf7c2e05a2e9 100644 --- a/library/std/src/thread/local.rs +++ b/library/std/src/thread/local.rs @@ -1110,8 +1110,7 @@ pub mod os { let ptr = if ptr.is_null() { // If the lookup returned null, we haven't initialized our own // local copy, so do that now. - let ptr: Box> = box Value { inner: LazyKeyInner::new(), key: self }; - let ptr = Box::into_raw(ptr); + let ptr = Box::into_raw(Box::new(Value { inner: LazyKeyInner::new(), key: self })); // SAFETY: At this point we are sure there is no value inside // ptr so setting it will not affect anyone else. unsafe { From 318d77ddfd87d034d54c4e73a3793fbc743b0476 Mon Sep 17 00:00:00 2001 From: Josh Stone Date: Wed, 18 Jan 2023 11:14:40 -0800 Subject: [PATCH 02/17] ci: add runners for vanilla LLVM 14 and 15 --- .github/workflows/ci.yml | 8 +++ .../host-x86_64/x86_64-gnu-llvm-14/Dockerfile | 67 +++++++++++++++++++ .../host-x86_64/x86_64-gnu-llvm-15/Dockerfile | 67 +++++++++++++++++++ src/ci/github-actions/ci.yml | 10 +++ 4 files changed, 152 insertions(+) create mode 100644 src/ci/docker/host-x86_64/x86_64-gnu-llvm-14/Dockerfile create mode 100644 src/ci/docker/host-x86_64/x86_64-gnu-llvm-15/Dockerfile diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 82048f800d0e..5f77656e5c18 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -291,6 +291,14 @@ jobs: - name: x86_64-gnu-distcheck os: ubuntu-20.04-xl env: {} + - name: x86_64-gnu-llvm-15 + env: + RUST_BACKTRACE: 1 + os: ubuntu-20.04-xl + - name: x86_64-gnu-llvm-14 + env: + RUST_BACKTRACE: 1 + os: ubuntu-20.04-xl - name: x86_64-gnu-llvm-13 env: RUST_BACKTRACE: 1 diff --git a/src/ci/docker/host-x86_64/x86_64-gnu-llvm-14/Dockerfile b/src/ci/docker/host-x86_64/x86_64-gnu-llvm-14/Dockerfile new file mode 100644 index 000000000000..db6032f87521 --- /dev/null +++ b/src/ci/docker/host-x86_64/x86_64-gnu-llvm-14/Dockerfile @@ -0,0 +1,67 @@ +FROM ubuntu:22.04 + +ARG DEBIAN_FRONTEND=noninteractive + +# NOTE: intentionally installs both python2 and python3 so we can test support for both. +RUN apt-get update && apt-get install -y --no-install-recommends \ + g++ \ + gcc-multilib \ + make \ + ninja-build \ + file \ + curl \ + ca-certificates \ + python2.7 \ + python3 \ + git \ + cmake \ + sudo \ + gdb \ + llvm-14-tools \ + llvm-14-dev \ + libedit-dev \ + libssl-dev \ + pkg-config \ + zlib1g-dev \ + xz-utils \ + nodejs \ + && rm -rf /var/lib/apt/lists/* + +# Install powershell (universal package) so we can test x.ps1 on Linux +RUN curl -sL "https://github.com/PowerShell/PowerShell/releases/download/v7.3.1/powershell_7.3.1-1.deb_amd64.deb" > powershell.deb && \ + dpkg -i powershell.deb && \ + rm -f powershell.deb + +COPY scripts/sccache.sh /scripts/ +RUN sh /scripts/sccache.sh + +# We are disabling CI LLVM since this builder is intentionally using a host +# LLVM, rather than the typical src/llvm-project LLVM. +ENV NO_DOWNLOAD_CI_LLVM 1 + +# Using llvm-link-shared due to libffi issues -- see #34486 +ENV RUST_CONFIGURE_ARGS \ + --build=x86_64-unknown-linux-gnu \ + --llvm-root=/usr/lib/llvm-14 \ + --enable-llvm-link-shared \ + --set rust.thin-lto-import-instr-limit=10 + +# NOTE: intentionally uses all of `x.py`, `x`, and `x.ps1` to make sure they all work on Linux. +ENV SCRIPT ../x.py --stage 2 test --exclude src/tools/tidy && \ + # Run the `mir-opt` tests again but this time for a 32-bit target. + # This enforces that tests using `// EMIT_MIR_FOR_EACH_BIT_WIDTH` have + # both 32-bit and 64-bit outputs updated by the PR author, before + # the PR is approved and tested for merging. + # It will also detect tests lacking `// EMIT_MIR_FOR_EACH_BIT_WIDTH`, + # despite having different output on 32-bit vs 64-bit targets. + ../x --stage 2 test tests/mir-opt \ + --host='' --target=i686-unknown-linux-gnu && \ + # Run the UI test suite again, but in `--pass=check` mode + # + # This is intended to make sure that both `--pass=check` continues to + # work. + # + ../x.ps1 --stage 2 test tests/ui --pass=check \ + --host='' --target=i686-unknown-linux-gnu && \ + # Run tidy at the very end, after all the other tests. + python2.7 ../x.py --stage 2 test src/tools/tidy diff --git a/src/ci/docker/host-x86_64/x86_64-gnu-llvm-15/Dockerfile b/src/ci/docker/host-x86_64/x86_64-gnu-llvm-15/Dockerfile new file mode 100644 index 000000000000..5219247cc6f6 --- /dev/null +++ b/src/ci/docker/host-x86_64/x86_64-gnu-llvm-15/Dockerfile @@ -0,0 +1,67 @@ +FROM ubuntu:22.10 + +ARG DEBIAN_FRONTEND=noninteractive + +# NOTE: intentionally installs both python2 and python3 so we can test support for both. +RUN apt-get update && apt-get install -y --no-install-recommends \ + g++ \ + gcc-multilib \ + make \ + ninja-build \ + file \ + curl \ + ca-certificates \ + python2.7 \ + python3 \ + git \ + cmake \ + sudo \ + gdb \ + llvm-15-tools \ + llvm-15-dev \ + libedit-dev \ + libssl-dev \ + pkg-config \ + zlib1g-dev \ + xz-utils \ + nodejs \ + && rm -rf /var/lib/apt/lists/* + +# Install powershell (universal package) so we can test x.ps1 on Linux +RUN curl -sL "https://github.com/PowerShell/PowerShell/releases/download/v7.3.1/powershell_7.3.1-1.deb_amd64.deb" > powershell.deb && \ + dpkg -i powershell.deb && \ + rm -f powershell.deb + +COPY scripts/sccache.sh /scripts/ +RUN sh /scripts/sccache.sh + +# We are disabling CI LLVM since this builder is intentionally using a host +# LLVM, rather than the typical src/llvm-project LLVM. +ENV NO_DOWNLOAD_CI_LLVM 1 + +# Using llvm-link-shared due to libffi issues -- see #34486 +ENV RUST_CONFIGURE_ARGS \ + --build=x86_64-unknown-linux-gnu \ + --llvm-root=/usr/lib/llvm-15 \ + --enable-llvm-link-shared \ + --set rust.thin-lto-import-instr-limit=10 + +# NOTE: intentionally uses all of `x.py`, `x`, and `x.ps1` to make sure they all work on Linux. +ENV SCRIPT ../x.py --stage 2 test --exclude src/tools/tidy && \ + # Run the `mir-opt` tests again but this time for a 32-bit target. + # This enforces that tests using `// EMIT_MIR_FOR_EACH_BIT_WIDTH` have + # both 32-bit and 64-bit outputs updated by the PR author, before + # the PR is approved and tested for merging. + # It will also detect tests lacking `// EMIT_MIR_FOR_EACH_BIT_WIDTH`, + # despite having different output on 32-bit vs 64-bit targets. + ../x --stage 2 test tests/mir-opt \ + --host='' --target=i686-unknown-linux-gnu && \ + # Run the UI test suite again, but in `--pass=check` mode + # + # This is intended to make sure that both `--pass=check` continues to + # work. + # + ../x.ps1 --stage 2 test tests/ui --pass=check \ + --host='' --target=i686-unknown-linux-gnu && \ + # Run tidy at the very end, after all the other tests. + python2.7 ../x.py --stage 2 test src/tools/tidy diff --git a/src/ci/github-actions/ci.yml b/src/ci/github-actions/ci.yml index d2a9264c84a1..a466777dd46f 100644 --- a/src/ci/github-actions/ci.yml +++ b/src/ci/github-actions/ci.yml @@ -450,6 +450,16 @@ jobs: - name: x86_64-gnu-distcheck <<: *job-linux-xl + - name: x86_64-gnu-llvm-15 + env: + RUST_BACKTRACE: 1 + <<: *job-linux-xl + + - name: x86_64-gnu-llvm-14 + env: + RUST_BACKTRACE: 1 + <<: *job-linux-xl + - name: x86_64-gnu-llvm-13 env: RUST_BACKTRACE: 1 From 33696fa9cab5bec947147d65833233b957b5edd5 Mon Sep 17 00:00:00 2001 From: Frank Steffahn Date: Tue, 25 Aug 2020 17:30:46 +0200 Subject: [PATCH 03/17] Add Arc::into_inner for safely discarding Arcs without calling the destructor on the inner type. Mainly rebased and squashed from PR rust-lang/rust#79665, furthermore includes minor changes in comments. --- library/alloc/src/sync.rs | 145 ++++++++++++++++++++++++++++++++ library/alloc/src/sync/tests.rs | 32 +++++++ 2 files changed, 177 insertions(+) diff --git a/library/alloc/src/sync.rs b/library/alloc/src/sync.rs index d833d4d1dfbd..a31b597930c8 100644 --- a/library/alloc/src/sync.rs +++ b/library/alloc/src/sync.rs @@ -654,6 +654,20 @@ impl Arc { /// /// This will succeed even if there are outstanding weak references. /// + // FIXME: when `Arc::into_inner` is stabilized, add this paragraph: + /* + /// It is strongly recommended to use [`Arc::into_inner`] instead if you don't + /// want to keep the `Arc` in the [`Err`] case. + /// Immediately dropping the [`Err`] payload, like in the expression + /// `Arc::try_unwrap(this).ok()`, can still cause the strong count to + /// drop to zero and the inner value of the `Arc` to be dropped: + /// For instance if two threads execute this expression in parallel, then + /// there is a race condition. The threads could first both check whether they + /// have the last clone of their `Arc` via `Arc::try_unwrap`, and then + /// both drop their `Arc` in the call to [`ok`][`Result::ok`], + /// taking the strong count from two down to zero. + /// + */ /// # Examples /// /// ``` @@ -685,6 +699,137 @@ impl Arc { Ok(elem) } } + + /// Returns the inner value, if the `Arc` has exactly one strong reference. + /// + /// Otherwise, [`None`] is returned and the `Arc` is dropped. + /// + /// This will succeed even if there are outstanding weak references. + /// + /// If `Arc::into_inner` is called on every clone of this `Arc`, + /// it is guaranteed that exactly one of the calls returns the inner value. + /// This means in particular that the inner value is not dropped. + /// + /// The similar expression `Arc::try_unwrap(this).ok()` does not + /// offer such a guarantee. See the last example below. + // + // FIXME: when `Arc::into_inner` is stabilized, add this to end + // of the previous sentence: + /* + /// and the documentation of [`Arc::try_unwrap`]. + */ + /// + /// # Examples + /// + /// Minimal example demonstrating the guarantee that `Arc::into_inner` gives. + /// ``` + /// #![feature(arc_into_inner)] + /// + /// use std::sync::Arc; + /// + /// let x = Arc::new(3); + /// let y = Arc::clone(&x); + /// + /// // Two threads calling `Arc::into_inner` on both clones of an `Arc`: + /// let x_thread = std::thread::spawn(|| Arc::into_inner(x)); + /// let y_thread = std::thread::spawn(|| Arc::into_inner(y)); + /// + /// let x_inner_value = x_thread.join().unwrap(); + /// let y_inner_value = y_thread.join().unwrap(); + /// + /// // One of the threads is guaranteed to receive the inner value: + /// assert!(matches!( + /// (x_inner_value, y_inner_value), + /// (None, Some(3)) | (Some(3), None) + /// )); + /// // The result could also be `(None, None)` if the threads called + /// // `Arc::try_unwrap(x).ok()` and `Arc::try_unwrap(y).ok()` instead. + /// ``` + /// + /// A more practical example demonstrating the need for `Arc::into_inner`: + /// ``` + /// #![feature(arc_into_inner)] + /// + /// use std::sync::Arc; + /// + /// // Definition of a simple singly linked list using `Arc`: + /// #[derive(Clone)] + /// struct LinkedList(Option>>); + /// struct Node(T, Option>>); + /// + /// // Dropping a long `LinkedList` relying on the destructor of `Arc` + /// // can cause a stack overflow. To prevent this, we can provide a + /// // manual `Drop` implementation that does the destruction in a loop: + /// impl Drop for LinkedList { + /// fn drop(&mut self) { + /// let mut link = self.0.take(); + /// while let Some(arc_node) = link.take() { + /// if let Some(Node(_value, next)) = Arc::into_inner(arc_node) { + /// link = next; + /// } + /// } + /// } + /// } + /// + /// // Implementation of `new` and `push` omitted + /// impl LinkedList { + /// /* ... */ + /// # fn new() -> Self { + /// # LinkedList(None) + /// # } + /// # fn push(&mut self, x: T) { + /// # self.0 = Some(Arc::new(Node(x, self.0.take()))); + /// # } + /// } + /// + /// // The following code could have still caused a stack overflow + /// // despite the manual `Drop` impl if that `Drop` impl had used + /// // `Arc::try_unwrap(arc).ok()` instead of `Arc::into_inner(arc)`. + /// + /// // Create a long list and clone it + /// let mut x = LinkedList::new(); + /// for i in 0..100000 { + /// x.push(i); // Adds i to the front of x + /// } + /// let y = x.clone(); + /// + /// // Drop the clones in parallel + /// let x_thread = std::thread::spawn(|| drop(x)); + /// let y_thread = std::thread::spawn(|| drop(y)); + /// x_thread.join().unwrap(); + /// y_thread.join().unwrap(); + /// ``` + + // FIXME: when `Arc::into_inner` is stabilized, adjust above documentation + // and the documentation of `Arc::try_unwrap` according to the `FIXME`s. Also + // open an issue on rust-lang/rust-clippy, asking for a lint against + // `Arc::try_unwrap(...).ok()`. + #[inline] + #[unstable(feature = "arc_into_inner", issue = "106894")] + pub fn into_inner(this: Self) -> Option { + // Make sure that the ordinary `Drop` implementation isn’t called as well + let mut this = mem::ManuallyDrop::new(this); + + // Following the implementation of `drop` and `drop_slow` + if this.inner().strong.fetch_sub(1, Release) != 1 { + return None; + } + + acquire!(this.inner().strong); + + // SAFETY: This mirrors the line + // + // unsafe { ptr::drop_in_place(Self::get_mut_unchecked(self)) }; + // + // in `drop_slow`. Instead of dropping the value behind the pointer, + // it is read and eventually returned; `ptr::read` has the same + // safety conditions as `ptr::drop_in_place`. + let inner = unsafe { ptr::read(Self::get_mut_unchecked(&mut this)) }; + + drop(Weak { ptr: this.ptr }); + + Some(inner) + } } impl Arc<[T]> { diff --git a/library/alloc/src/sync/tests.rs b/library/alloc/src/sync/tests.rs index 0fae8953aa2c..863d58bdf4d9 100644 --- a/library/alloc/src/sync/tests.rs +++ b/library/alloc/src/sync/tests.rs @@ -101,6 +101,38 @@ fn try_unwrap() { assert_eq!(Arc::try_unwrap(x), Ok(5)); } +#[test] +fn into_inner() { + for _ in 0..100 + // ^ Increase chances of hitting potential race conditions + { + let x = Arc::new(3); + let y = Arc::clone(&x); + let r_thread = std::thread::spawn(|| Arc::into_inner(x)); + let s_thread = std::thread::spawn(|| Arc::into_inner(y)); + let r = r_thread.join().expect("r_thread panicked"); + let s = s_thread.join().expect("s_thread panicked"); + assert!( + matches!((r, s), (None, Some(3)) | (Some(3), None)), + "assertion failed: unexpected result `{:?}`\ + \n expected `(None, Some(3))` or `(Some(3), None)`", + (r, s), + ); + } + + let x = Arc::new(3); + assert_eq!(Arc::into_inner(x), Some(3)); + + let x = Arc::new(4); + let y = Arc::clone(&x); + assert_eq!(Arc::into_inner(x), None); + assert_eq!(Arc::into_inner(y), Some(4)); + + let x = Arc::new(5); + let _w = Arc::downgrade(&x); + assert_eq!(Arc::into_inner(x), Some(5)); +} + #[test] fn into_from_raw() { let x = Arc::new(Box::new("hello")); From 415c14129b02cc3c61f294cff885cee6c589940d Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Sat, 21 Jan 2023 00:59:19 +0400 Subject: [PATCH 04/17] rustc_metadata: Encode `doc(hidden)` flag to metadata To retrieve these flags rustdoc currently has to mass decode full attributes for items in the whole crate tree, so it's better to pre-compute it in advance. This is especially for short-term performance of https://github.com/rust-lang/rust/pull/107054 because resolver cannot use memoization of query results yet. --- Cargo.lock | 1 + compiler/rustc_metadata/Cargo.toml | 1 + compiler/rustc_metadata/src/rmeta/decoder.rs | 4 ++-- .../src/rmeta/decoder/cstore_impl.rs | 8 ++++++-- compiler/rustc_metadata/src/rmeta/encoder.rs | 19 +++++++++++++++---- compiler/rustc_metadata/src/rmeta/mod.rs | 10 +++++++++- compiler/rustc_metadata/src/rmeta/table.rs | 14 ++++++++++++++ compiler/rustc_middle/src/query/mod.rs | 1 + compiler/rustc_middle/src/ty/util.rs | 3 ++- src/librustdoc/clean/types.rs | 9 +-------- 10 files changed, 52 insertions(+), 18 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 640d4d2ec269..56b1078b7f36 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4326,6 +4326,7 @@ dependencies = [ name = "rustc_metadata" version = "0.0.0" dependencies = [ + "bitflags", "libloading", "odht", "rustc_ast", diff --git a/compiler/rustc_metadata/Cargo.toml b/compiler/rustc_metadata/Cargo.toml index 6d85103c9d13..bee5c8541d68 100644 --- a/compiler/rustc_metadata/Cargo.toml +++ b/compiler/rustc_metadata/Cargo.toml @@ -6,6 +6,7 @@ edition = "2021" [lib] [dependencies] +bitflags = "1.2.1" libloading = "0.7.1" odht = { version = "0.3.1", features = ["nightly"] } snap = "1" diff --git a/compiler/rustc_metadata/src/rmeta/decoder.rs b/compiler/rustc_metadata/src/rmeta/decoder.rs index 143d8f2f1e18..44da3fbe300c 100644 --- a/compiler/rustc_metadata/src/rmeta/decoder.rs +++ b/compiler/rustc_metadata/src/rmeta/decoder.rs @@ -1594,8 +1594,8 @@ impl<'a, 'tcx> CrateMetadataRef<'a> { }) } - fn get_may_have_doc_links(self, index: DefIndex) -> bool { - self.root.tables.may_have_doc_links.get(self, index).is_some() + fn get_attr_flags(self, index: DefIndex) -> AttrFlags { + self.root.tables.attr_flags.get(self, index).unwrap_or(AttrFlags::empty()) } fn get_is_intrinsic(self, index: DefIndex) -> bool { diff --git a/compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs b/compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs index 6fd5bd52abe2..2fa645cd9e33 100644 --- a/compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs +++ b/compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs @@ -1,6 +1,7 @@ use crate::creader::{CStore, LoadedMacro}; use crate::foreign_modules; use crate::native_libs; +use crate::rmeta::AttrFlags; use rustc_ast as ast; use rustc_attr::Deprecation; @@ -338,6 +339,7 @@ provide! { tcx, def_id, other, cdata, crate_extern_paths => { cdata.source().paths().cloned().collect() } expn_that_defined => { cdata.get_expn_that_defined(def_id.index, tcx.sess) } generator_diagnostic_data => { cdata.get_generator_diagnostic_data(tcx, def_id.index) } + is_doc_hidden => { cdata.get_attr_flags(def_id.index).contains(AttrFlags::IS_DOC_HIDDEN) } } pub(in crate::rmeta) fn provide(providers: &mut Providers) { @@ -425,7 +427,7 @@ pub(in crate::rmeta) fn provide(providers: &mut Providers) { return; } - if ty::util::is_doc_hidden(tcx, parent) { + if tcx.is_doc_hidden(parent) { fallback_map.push((def_id, parent)); return; } @@ -631,7 +633,9 @@ impl CStore { } pub fn may_have_doc_links_untracked(&self, def_id: DefId) -> bool { - self.get_crate_data(def_id.krate).get_may_have_doc_links(def_id.index) + self.get_crate_data(def_id.krate) + .get_attr_flags(def_id.index) + .contains(AttrFlags::MAY_HAVE_DOC_LINKS) } } diff --git a/compiler/rustc_metadata/src/rmeta/encoder.rs b/compiler/rustc_metadata/src/rmeta/encoder.rs index 8f7a61b72f81..2ecaa33d4d31 100644 --- a/compiler/rustc_metadata/src/rmeta/encoder.rs +++ b/compiler/rustc_metadata/src/rmeta/encoder.rs @@ -1111,15 +1111,26 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> { let tcx = self.tcx; let mut is_public: Option = None; - let mut attrs = tcx - .hir() - .attrs(tcx.hir().local_def_id_to_hir_id(def_id)) + let hir_attrs = tcx.hir().attrs(tcx.hir().local_def_id_to_hir_id(def_id)); + let mut attrs = hir_attrs .iter() .filter(move |attr| should_encode_attr(tcx, attr, def_id, &mut is_public)); record_array!(self.tables.attributes[def_id.to_def_id()] <- attrs.clone()); + let mut attr_flags = AttrFlags::empty(); if attrs.any(|attr| attr.may_have_doc_links()) { - self.tables.may_have_doc_links.set(def_id.local_def_index, ()); + attr_flags |= AttrFlags::MAY_HAVE_DOC_LINKS; + } + if hir_attrs + .iter() + .filter(|attr| attr.has_name(sym::doc)) + .filter_map(|attr| attr.meta_item_list()) + .any(|items| items.iter().any(|item| item.has_name(sym::hidden))) + { + attr_flags |= AttrFlags::IS_DOC_HIDDEN; + } + if !attr_flags.is_empty() { + self.tables.attr_flags.set(def_id.local_def_index, attr_flags); } } diff --git a/compiler/rustc_metadata/src/rmeta/mod.rs b/compiler/rustc_metadata/src/rmeta/mod.rs index 5066dbbb90f3..69690264ae4e 100644 --- a/compiler/rustc_metadata/src/rmeta/mod.rs +++ b/compiler/rustc_metadata/src/rmeta/mod.rs @@ -395,7 +395,7 @@ define_tables! { def_path_hashes: Table, proc_macro_quoted_spans: Table>, generator_diagnostic_data: Table>>, - may_have_doc_links: Table, + attr_flags: Table, variant_data: Table>, assoc_container: Table, // Slot is full when macro is macro_rules. @@ -418,6 +418,13 @@ struct VariantData { is_non_exhaustive: bool, } +bitflags::bitflags! { + pub struct AttrFlags: u8 { + const MAY_HAVE_DOC_LINKS = 1 << 0; + const IS_DOC_HIDDEN = 1 << 1; + } +} + // Tags used for encoding Spans: const TAG_VALID_SPAN_LOCAL: u8 = 0; const TAG_VALID_SPAN_FOREIGN: u8 = 1; @@ -440,4 +447,5 @@ trivially_parameterized_over_tcx! { IncoherentImpls, CrateRoot, CrateDep, + AttrFlags, } diff --git a/compiler/rustc_metadata/src/rmeta/table.rs b/compiler/rustc_metadata/src/rmeta/table.rs index 716655c7f144..dc003227d40b 100644 --- a/compiler/rustc_metadata/src/rmeta/table.rs +++ b/compiler/rustc_metadata/src/rmeta/table.rs @@ -199,6 +199,20 @@ impl FixedSizeEncoding for Option { } } +impl FixedSizeEncoding for Option { + type ByteArray = [u8; 1]; + + #[inline] + fn from_bytes(b: &[u8; 1]) -> Self { + (b[0] != 0).then(|| AttrFlags::from_bits_truncate(b[0])) + } + + #[inline] + fn write_to_bytes(self, b: &mut [u8; 1]) { + b[0] = self.map_or(0, |flags| flags.bits()) + } +} + impl FixedSizeEncoding for Option<()> { type ByteArray = [u8; 1]; diff --git a/compiler/rustc_middle/src/query/mod.rs b/compiler/rustc_middle/src/query/mod.rs index 6bbf7fa3914e..7db86c8d0d45 100644 --- a/compiler/rustc_middle/src/query/mod.rs +++ b/compiler/rustc_middle/src/query/mod.rs @@ -1157,6 +1157,7 @@ rustc_queries! { /// Determines whether an item is annotated with `doc(hidden)`. query is_doc_hidden(def_id: DefId) -> bool { desc { |tcx| "checking whether `{}` is `doc(hidden)`", tcx.def_path_str(def_id) } + separate_provide_extern } /// Determines whether an item is annotated with `doc(notable_trait)`. diff --git a/compiler/rustc_middle/src/ty/util.rs b/compiler/rustc_middle/src/ty/util.rs index d0d1dcc584f1..60076c8cb5f9 100644 --- a/compiler/rustc_middle/src/ty/util.rs +++ b/compiler/rustc_middle/src/ty/util.rs @@ -1310,7 +1310,8 @@ pub fn reveal_opaque_types_in_bounds<'tcx>( } /// Determines whether an item is annotated with `doc(hidden)`. -pub fn is_doc_hidden(tcx: TyCtxt<'_>, def_id: DefId) -> bool { +fn is_doc_hidden(tcx: TyCtxt<'_>, def_id: DefId) -> bool { + assert!(def_id.is_local()); tcx.get_attrs(def_id, sym::doc) .filter_map(|attr| attr.meta_item_list()) .any(|items| items.iter().any(|item| item.has_name(sym::hidden))) diff --git a/src/librustdoc/clean/types.rs b/src/librustdoc/clean/types.rs index 87de41fde63c..a020ccd53b84 100644 --- a/src/librustdoc/clean/types.rs +++ b/src/librustdoc/clean/types.rs @@ -2498,14 +2498,7 @@ impl Import { } pub(crate) fn imported_item_is_doc_hidden(&self, tcx: TyCtxt<'_>) -> bool { - match self.source.did { - Some(did) => tcx - .get_attrs(did, sym::doc) - .filter_map(ast::Attribute::meta_item_list) - .flatten() - .has_word(sym::hidden), - None => false, - } + self.source.did.map_or(false, |did| tcx.is_doc_hidden(did)) } } From 26e2360b263930aa919a0604a79e8dba2c1284cf Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Sun, 22 Jan 2023 11:11:29 +0100 Subject: [PATCH 05/17] Use correct pseudo-element selector --- src/librustdoc/html/static/css/rustdoc.css | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/librustdoc/html/static/css/rustdoc.css b/src/librustdoc/html/static/css/rustdoc.css index 8ff8ea088be9..260e5c1c44ca 100644 --- a/src/librustdoc/html/static/css/rustdoc.css +++ b/src/librustdoc/html/static/css/rustdoc.css @@ -1897,21 +1897,21 @@ in storage.js right: 0.25em; } -.scraped-example:not(.expanded) .code-wrapper:before, -.scraped-example:not(.expanded) .code-wrapper:after { +.scraped-example:not(.expanded) .code-wrapper::before, +.scraped-example:not(.expanded) .code-wrapper::after { content: " "; width: 100%; height: 5px; position: absolute; z-index: 1; } -.scraped-example:not(.expanded) .code-wrapper:before { +.scraped-example:not(.expanded) .code-wrapper::before { top: 0; background: linear-gradient(to bottom, var(--scrape-example-code-wrapper-background-start), var(--scrape-example-code-wrapper-background-end)); } -.scraped-example:not(.expanded) .code-wrapper:after { +.scraped-example:not(.expanded) .code-wrapper::after { bottom: 0; background: linear-gradient(to top, var(--scrape-example-code-wrapper-background-start), From e0b3d7290e113457dd779b9ec98f221acd62361d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Le=C3=B3n=20Orell=20Valerian=20Liehr?= Date: Sun, 22 Jan 2023 12:36:04 +0100 Subject: [PATCH 06/17] add fmease to mailmap --- .mailmap | 1 + 1 file changed, 1 insertion(+) diff --git a/.mailmap b/.mailmap index 022cdd0fd50c..43646db44aa3 100644 --- a/.mailmap +++ b/.mailmap @@ -324,6 +324,7 @@ Lennart Kudling Léo Lanteri Thauvin Léo Lanteri Thauvin <38361244+LeSeulArtichaut@users.noreply.github.com> Léo Testard +León Orell Valerian Liehr Leonardo Yvens Liigo Zhuang Lily Ballard From 12a72f0329476aa57a465c4fc3043ee40028b325 Mon Sep 17 00:00:00 2001 From: Samuel Moelius <35515885+smoelius@users.noreply.github.com> Date: Sun, 22 Jan 2023 07:38:02 -0500 Subject: [PATCH 07/17] Update universal_regions.rs --- compiler/rustc_borrowck/src/universal_regions.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/rustc_borrowck/src/universal_regions.rs b/compiler/rustc_borrowck/src/universal_regions.rs index 5b4d99682d98..8bff66f8d5cc 100644 --- a/compiler/rustc_borrowck/src/universal_regions.rs +++ b/compiler/rustc_borrowck/src/universal_regions.rs @@ -162,7 +162,7 @@ struct UniversalRegionIndices<'tcx> { /// `ty::Region` to the internal `RegionVid` we are using. This is /// used because trait matching and type-checking will feed us /// region constraints that reference those regions and we need to - /// be able to map them our internal `RegionVid`. This is + /// be able to map them to our internal `RegionVid`. This is /// basically equivalent to an `InternalSubsts`, except that it also /// contains an entry for `ReStatic` -- it might be nice to just /// use a substs, and then handle `ReStatic` another way. From 81ee6aebaa3d93c5e86d4d7c0fe80c3af74d3ec3 Mon Sep 17 00:00:00 2001 From: Santiago Pastorino Date: Thu, 24 Nov 2022 14:26:57 -0300 Subject: [PATCH 08/17] Remove duplicated debug call --- compiler/rustc_hir_typeck/src/fallback.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/compiler/rustc_hir_typeck/src/fallback.rs b/compiler/rustc_hir_typeck/src/fallback.rs index dde8797804f0..1015cea74b35 100644 --- a/compiler/rustc_hir_typeck/src/fallback.rs +++ b/compiler/rustc_hir_typeck/src/fallback.rs @@ -281,7 +281,6 @@ impl<'tcx> FnCtxt<'_, 'tcx> { roots_reachable_from_non_diverging, ); - debug!("inherited: {:#?}", self.inh.fulfillment_cx.borrow_mut().pending_obligations()); debug!("obligations: {:#?}", self.fulfillment_cx.borrow_mut().pending_obligations()); debug!("relationships: {:#?}", relationships); From 7fe472223e4bcdd960d73e323979d15168ce4e39 Mon Sep 17 00:00:00 2001 From: Santiago Pastorino Date: Fri, 25 Nov 2022 18:18:03 -0300 Subject: [PATCH 09/17] Store relationships on Inherent --- compiler/rustc_hir_typeck/src/fallback.rs | 7 ++----- compiler/rustc_hir_typeck/src/inherited.rs | 12 ++++++++++- compiler/rustc_hir_typeck/src/lib.rs | 1 + .../src}/relationships.rs | 20 ++++++++---------- compiler/rustc_infer/src/traits/engine.rs | 3 --- .../src/solve/fulfill.rs | 6 ------ .../src/traits/chalk_fulfill.rs | 18 +++------------- .../src/traits/fulfill.rs | 21 ++----------------- .../rustc_trait_selection/src/traits/mod.rs | 1 - 9 files changed, 28 insertions(+), 61 deletions(-) rename compiler/{rustc_trait_selection/src/traits => rustc_hir_typeck/src}/relationships.rs (77%) diff --git a/compiler/rustc_hir_typeck/src/fallback.rs b/compiler/rustc_hir_typeck/src/fallback.rs index 1015cea74b35..4ef32023e7d8 100644 --- a/compiler/rustc_hir_typeck/src/fallback.rs +++ b/compiler/rustc_hir_typeck/src/fallback.rs @@ -196,8 +196,6 @@ impl<'tcx> FnCtxt<'_, 'tcx> { ) -> FxHashMap, Ty<'tcx>> { debug!("calculate_diverging_fallback({:?})", unsolved_variables); - let relationships = self.fulfillment_cx.borrow_mut().relationships().clone(); - // Construct a coercion graph where an edge `A -> B` indicates // a type variable is that is coerced let coercion_graph = self.create_coercion_graph(); @@ -282,7 +280,6 @@ impl<'tcx> FnCtxt<'_, 'tcx> { ); debug!("obligations: {:#?}", self.fulfillment_cx.borrow_mut().pending_obligations()); - debug!("relationships: {:#?}", relationships); // For each diverging variable, figure out whether it can // reach a member of N. If so, it falls back to `()`. Else @@ -298,8 +295,8 @@ impl<'tcx> FnCtxt<'_, 'tcx> { let mut relationship = ty::FoundRelationships { self_in_trait: false, output: false }; - for (vid, rel) in relationships.iter() { - if self.root_var(*vid) == root_vid { + for (vid, rel) in self.inh.relationships.borrow().iter() { + if self.infcx.root_var(*vid) == root_vid { relationship.self_in_trait |= rel.self_in_trait; relationship.output |= rel.output; } diff --git a/compiler/rustc_hir_typeck/src/inherited.rs b/compiler/rustc_hir_typeck/src/inherited.rs index b33e7b8d68cf..f5b6578a9d32 100644 --- a/compiler/rustc_hir_typeck/src/inherited.rs +++ b/compiler/rustc_hir_typeck/src/inherited.rs @@ -1,6 +1,6 @@ use super::callee::DeferredCallResolution; -use rustc_data_structures::fx::FxHashSet; +use rustc_data_structures::fx::{FxHashMap, FxHashSet}; use rustc_hir as hir; use rustc_hir::def_id::LocalDefId; use rustc_hir::HirIdMap; @@ -63,6 +63,8 @@ pub struct Inherited<'tcx> { /// we record that type variable here. This is later used to inform /// fallback. See the `fallback` module for details. pub(super) diverging_type_vars: RefCell>>, + + pub(super) relationships: RefCell>, } impl<'tcx> Deref for Inherited<'tcx> { @@ -128,6 +130,7 @@ impl<'tcx> Inherited<'tcx> { deferred_generator_interiors: RefCell::new(Vec::new()), diverging_type_vars: RefCell::new(Default::default()), body_id, + relationships: RefCell::new(Default::default()), } } @@ -136,6 +139,13 @@ impl<'tcx> Inherited<'tcx> { if obligation.has_escaping_bound_vars() { span_bug!(obligation.cause.span, "escaping bound vars in predicate {:?}", obligation); } + + super::relationships::update( + &self.infcx, + &mut self.relationships.borrow_mut(), + &obligation, + ); + self.fulfillment_cx.borrow_mut().register_predicate_obligation(self, obligation); } diff --git a/compiler/rustc_hir_typeck/src/lib.rs b/compiler/rustc_hir_typeck/src/lib.rs index 7ddf9eaa4d89..54fd03d6493b 100644 --- a/compiler/rustc_hir_typeck/src/lib.rs +++ b/compiler/rustc_hir_typeck/src/lib.rs @@ -40,6 +40,7 @@ mod method; mod op; mod pat; mod place_op; +mod relationships; mod rvalue_scopes; mod upvar; mod writeback; diff --git a/compiler/rustc_trait_selection/src/traits/relationships.rs b/compiler/rustc_hir_typeck/src/relationships.rs similarity index 77% rename from compiler/rustc_trait_selection/src/traits/relationships.rs rename to compiler/rustc_hir_typeck/src/relationships.rs index 34b5fc4891eb..66aba0849745 100644 --- a/compiler/rustc_trait_selection/src/traits/relationships.rs +++ b/compiler/rustc_hir_typeck/src/relationships.rs @@ -1,16 +1,14 @@ -use crate::infer::InferCtxt; -use crate::traits::query::evaluate_obligation::InferCtxtExt; -use crate::traits::PredicateObligation; -use rustc_infer::traits::TraitEngine; +use rustc_data_structures::fx::FxHashMap; use rustc_middle::ty; +use rustc_trait_selection::infer::InferCtxt; +use rustc_trait_selection::traits::query::evaluate_obligation::InferCtxtExt; +use rustc_trait_selection::traits::PredicateObligation; -pub(crate) fn update<'tcx, T>( - engine: &mut T, +pub fn update<'tcx>( infcx: &InferCtxt<'tcx>, + relationships: &mut FxHashMap, obligation: &PredicateObligation<'tcx>, -) where - T: TraitEngine<'tcx>, -{ +) { // (*) binder skipped if let ty::PredicateKind::Clause(ty::Clause::Trait(tpred)) = obligation.predicate.kind().skip_binder() && let Some(ty) = infcx.shallow_resolve(tpred.self_ty()).ty_vid().map(|t| infcx.root_var(t)) @@ -31,7 +29,7 @@ pub(crate) fn update<'tcx, T>( ); // Don't report overflow errors. Otherwise equivalent to may_hold. if let Ok(result) = infcx.probe(|_| infcx.evaluate_obligation(&o)) && result.may_apply() { - engine.relationships().entry(ty).or_default().self_in_trait = true; + relationships.entry(ty).or_default().self_in_trait = true; } } @@ -42,7 +40,7 @@ pub(crate) fn update<'tcx, T>( // we need to make it into one. if let Some(vid) = predicate.term.ty().and_then(|ty| ty.ty_vid()) { debug!("relationship: {:?}.output = true", vid); - engine.relationships().entry(vid).or_default().output = true; + relationships.entry(vid).or_default().output = true; } } } diff --git a/compiler/rustc_infer/src/traits/engine.rs b/compiler/rustc_infer/src/traits/engine.rs index d3519f4b37b8..fcde00056cbf 100644 --- a/compiler/rustc_infer/src/traits/engine.rs +++ b/compiler/rustc_infer/src/traits/engine.rs @@ -1,6 +1,5 @@ use crate::infer::InferCtxt; use crate::traits::Obligation; -use rustc_data_structures::fx::FxHashMap; use rustc_hir::def_id::DefId; use rustc_middle::ty::{self, ToPredicate, Ty}; @@ -42,8 +41,6 @@ pub trait TraitEngine<'tcx>: 'tcx { fn select_where_possible(&mut self, infcx: &InferCtxt<'tcx>) -> Vec>; fn pending_obligations(&self) -> Vec>; - - fn relationships(&mut self) -> &mut FxHashMap; } pub trait TraitEngineExt<'tcx> { diff --git a/compiler/rustc_trait_selection/src/solve/fulfill.rs b/compiler/rustc_trait_selection/src/solve/fulfill.rs index a6240666ed43..40b9bedc84fd 100644 --- a/compiler/rustc_trait_selection/src/solve/fulfill.rs +++ b/compiler/rustc_trait_selection/src/solve/fulfill.rs @@ -1,6 +1,5 @@ use std::mem; -use rustc_data_structures::fx::FxHashMap; use rustc_infer::{ infer::InferCtxt, traits::{ @@ -8,7 +7,6 @@ use rustc_infer::{ SelectionError, TraitEngine, }, }; -use rustc_middle::ty; use super::{search_graph, Certainty, EvalCtxt}; @@ -102,8 +100,4 @@ impl<'tcx> TraitEngine<'tcx> for FulfillmentCtxt<'tcx> { fn pending_obligations(&self) -> Vec> { self.obligations.clone() } - - fn relationships(&mut self) -> &mut FxHashMap { - unimplemented!("Should be moved out of `TraitEngine`") - } } diff --git a/compiler/rustc_trait_selection/src/traits/chalk_fulfill.rs b/compiler/rustc_trait_selection/src/traits/chalk_fulfill.rs index e88950523537..61d09189798e 100644 --- a/compiler/rustc_trait_selection/src/traits/chalk_fulfill.rs +++ b/compiler/rustc_trait_selection/src/traits/chalk_fulfill.rs @@ -7,24 +7,18 @@ use crate::traits::{ ChalkEnvironmentAndGoal, FulfillmentError, FulfillmentErrorCode, PredicateObligation, SelectionError, TraitEngine, }; -use rustc_data_structures::fx::{FxHashMap, FxIndexSet}; -use rustc_middle::ty::{self, TypeVisitable}; +use rustc_data_structures::fx::FxIndexSet; +use rustc_middle::ty::TypeVisitable; pub struct FulfillmentContext<'tcx> { obligations: FxIndexSet>, - relationships: FxHashMap, - usable_in_snapshot: bool, } impl FulfillmentContext<'_> { pub(super) fn new() -> Self { - FulfillmentContext { - obligations: FxIndexSet::default(), - relationships: FxHashMap::default(), - usable_in_snapshot: false, - } + FulfillmentContext { obligations: FxIndexSet::default(), usable_in_snapshot: false } } pub(crate) fn new_in_snapshot() -> Self { @@ -43,8 +37,6 @@ impl<'tcx> TraitEngine<'tcx> for FulfillmentContext<'tcx> { } let obligation = infcx.resolve_vars_if_possible(obligation); - super::relationships::update(self, infcx, &obligation); - self.obligations.insert(obligation); } @@ -154,8 +146,4 @@ impl<'tcx> TraitEngine<'tcx> for FulfillmentContext<'tcx> { fn pending_obligations(&self) -> Vec> { self.obligations.iter().cloned().collect() } - - fn relationships(&mut self) -> &mut FxHashMap { - &mut self.relationships - } } diff --git a/compiler/rustc_trait_selection/src/traits/fulfill.rs b/compiler/rustc_trait_selection/src/traits/fulfill.rs index 76a755ed9e09..5a58d37e1836 100644 --- a/compiler/rustc_trait_selection/src/traits/fulfill.rs +++ b/compiler/rustc_trait_selection/src/traits/fulfill.rs @@ -1,5 +1,4 @@ use crate::infer::{InferCtxt, TyOrConstInferVar}; -use rustc_data_structures::fx::FxHashMap; use rustc_data_structures::obligation_forest::ProcessResult; use rustc_data_structures::obligation_forest::{Error, ForestObligation, Outcome}; use rustc_data_structures::obligation_forest::{ObligationForest, ObligationProcessor}; @@ -54,8 +53,6 @@ pub struct FulfillmentContext<'tcx> { // fulfillment context. predicates: ObligationForest>, - relationships: FxHashMap, - // Is it OK to register obligations into this infcx inside // an infcx snapshot? // @@ -85,19 +82,11 @@ static_assert_size!(PendingPredicateObligation<'_>, 72); impl<'a, 'tcx> FulfillmentContext<'tcx> { /// Creates a new fulfillment context. pub(super) fn new() -> FulfillmentContext<'tcx> { - FulfillmentContext { - predicates: ObligationForest::new(), - relationships: FxHashMap::default(), - usable_in_snapshot: false, - } + FulfillmentContext { predicates: ObligationForest::new(), usable_in_snapshot: false } } pub(super) fn new_in_snapshot() -> FulfillmentContext<'tcx> { - FulfillmentContext { - predicates: ObligationForest::new(), - relationships: FxHashMap::default(), - usable_in_snapshot: true, - } + FulfillmentContext { predicates: ObligationForest::new(), usable_in_snapshot: true } } /// Attempts to select obligations using `selcx`. @@ -139,8 +128,6 @@ impl<'tcx> TraitEngine<'tcx> for FulfillmentContext<'tcx> { assert!(!infcx.is_in_snapshot() || self.usable_in_snapshot); - super::relationships::update(self, infcx, &obligation); - self.predicates .register_obligation(PendingPredicateObligation { obligation, stalled_on: vec![] }); } @@ -164,10 +151,6 @@ impl<'tcx> TraitEngine<'tcx> for FulfillmentContext<'tcx> { fn pending_obligations(&self) -> Vec> { self.predicates.map_pending_obligations(|o| o.obligation.clone()) } - - fn relationships(&mut self) -> &mut FxHashMap { - &mut self.relationships - } } struct FulfillProcessor<'a, 'tcx> { diff --git a/compiler/rustc_trait_selection/src/traits/mod.rs b/compiler/rustc_trait_selection/src/traits/mod.rs index f036a311d464..3c640cdc503c 100644 --- a/compiler/rustc_trait_selection/src/traits/mod.rs +++ b/compiler/rustc_trait_selection/src/traits/mod.rs @@ -14,7 +14,6 @@ mod object_safety; pub mod outlives_bounds; mod project; pub mod query; -pub(crate) mod relationships; mod select; mod specialize; mod structural_match; From fb0a4e958950ed4dabc954c8a258594847b1d9bc Mon Sep 17 00:00:00 2001 From: Santiago Pastorino Date: Fri, 20 Jan 2023 16:42:22 -0300 Subject: [PATCH 10/17] Move relationships::update to Inherited::update_infer_var_info --- compiler/rustc_hir_typeck/src/inherited.rs | 48 ++++++++++++++++--- compiler/rustc_hir_typeck/src/lib.rs | 1 - .../rustc_hir_typeck/src/relationships.rs | 46 ------------------ 3 files changed, 42 insertions(+), 53 deletions(-) delete mode 100644 compiler/rustc_hir_typeck/src/relationships.rs diff --git a/compiler/rustc_hir_typeck/src/inherited.rs b/compiler/rustc_hir_typeck/src/inherited.rs index f5b6578a9d32..ce7e5caaefc5 100644 --- a/compiler/rustc_hir_typeck/src/inherited.rs +++ b/compiler/rustc_hir_typeck/src/inherited.rs @@ -10,7 +10,8 @@ use rustc_middle::ty::visit::TypeVisitable; use rustc_middle::ty::{self, Ty, TyCtxt}; use rustc_span::def_id::LocalDefIdMap; use rustc_span::{self, Span}; -use rustc_trait_selection::traits::{self, TraitEngine, TraitEngineExt as _}; +use rustc_trait_selection::traits::query::evaluate_obligation::InferCtxtExt; +use rustc_trait_selection::traits::{self, PredicateObligation, TraitEngine, TraitEngineExt as _}; use std::cell::RefCell; use std::ops::Deref; @@ -140,11 +141,7 @@ impl<'tcx> Inherited<'tcx> { span_bug!(obligation.cause.span, "escaping bound vars in predicate {:?}", obligation); } - super::relationships::update( - &self.infcx, - &mut self.relationships.borrow_mut(), - &obligation, - ); + self.update_infer_var_info(&obligation); self.fulfillment_cx.borrow_mut().register_predicate_obligation(self, obligation); } @@ -162,4 +159,43 @@ impl<'tcx> Inherited<'tcx> { self.register_predicates(infer_ok.obligations); infer_ok.value } + + pub fn update_infer_var_info(&self, obligation: &PredicateObligation<'tcx>) { + let relationships = &mut self.relationships.borrow_mut(); + + // (*) binder skipped + if let ty::PredicateKind::Clause(ty::Clause::Trait(tpred)) = obligation.predicate.kind().skip_binder() + && let Some(ty) = self.shallow_resolve(tpred.self_ty()).ty_vid().map(|t| self.root_var(t)) + && self.tcx.lang_items().sized_trait().map_or(false, |st| st != tpred.trait_ref.def_id) + { + let new_self_ty = self.tcx.types.unit; + + // Then construct a new obligation with Self = () added + // to the ParamEnv, and see if it holds. + let o = obligation.with(self.tcx, + obligation + .predicate + .kind() + .rebind( + // (*) binder moved here + ty::PredicateKind::Clause(ty::Clause::Trait(tpred.with_self_ty(self.tcx, new_self_ty))) + ), + ); + // Don't report overflow errors. Otherwise equivalent to may_hold. + if let Ok(result) = self.probe(|_| self.evaluate_obligation(&o)) && result.may_apply() { + relationships.entry(ty).or_default().self_in_trait = true; + } + } + + if let ty::PredicateKind::Clause(ty::Clause::Projection(predicate)) = + obligation.predicate.kind().skip_binder() + { + // If the projection predicate (Foo::Bar == X) has X as a non-TyVid, + // we need to make it into one. + if let Some(vid) = predicate.term.ty().and_then(|ty| ty.ty_vid()) { + debug!("relationships: {:?}.output = true", vid); + relationships.entry(vid).or_default().output = true; + } + } + } } diff --git a/compiler/rustc_hir_typeck/src/lib.rs b/compiler/rustc_hir_typeck/src/lib.rs index 54fd03d6493b..7ddf9eaa4d89 100644 --- a/compiler/rustc_hir_typeck/src/lib.rs +++ b/compiler/rustc_hir_typeck/src/lib.rs @@ -40,7 +40,6 @@ mod method; mod op; mod pat; mod place_op; -mod relationships; mod rvalue_scopes; mod upvar; mod writeback; diff --git a/compiler/rustc_hir_typeck/src/relationships.rs b/compiler/rustc_hir_typeck/src/relationships.rs deleted file mode 100644 index 66aba0849745..000000000000 --- a/compiler/rustc_hir_typeck/src/relationships.rs +++ /dev/null @@ -1,46 +0,0 @@ -use rustc_data_structures::fx::FxHashMap; -use rustc_middle::ty; -use rustc_trait_selection::infer::InferCtxt; -use rustc_trait_selection::traits::query::evaluate_obligation::InferCtxtExt; -use rustc_trait_selection::traits::PredicateObligation; - -pub fn update<'tcx>( - infcx: &InferCtxt<'tcx>, - relationships: &mut FxHashMap, - obligation: &PredicateObligation<'tcx>, -) { - // (*) binder skipped - if let ty::PredicateKind::Clause(ty::Clause::Trait(tpred)) = obligation.predicate.kind().skip_binder() - && let Some(ty) = infcx.shallow_resolve(tpred.self_ty()).ty_vid().map(|t| infcx.root_var(t)) - && infcx.tcx.lang_items().sized_trait().map_or(false, |st| st != tpred.trait_ref.def_id) - { - let new_self_ty = infcx.tcx.types.unit; - - // Then construct a new obligation with Self = () added - // to the ParamEnv, and see if it holds. - let o = obligation.with(infcx.tcx, - obligation - .predicate - .kind() - .rebind( - // (*) binder moved here - ty::PredicateKind::Clause(ty::Clause::Trait(tpred.with_self_ty(infcx.tcx, new_self_ty))) - ), - ); - // Don't report overflow errors. Otherwise equivalent to may_hold. - if let Ok(result) = infcx.probe(|_| infcx.evaluate_obligation(&o)) && result.may_apply() { - relationships.entry(ty).or_default().self_in_trait = true; - } - } - - if let ty::PredicateKind::Clause(ty::Clause::Projection(predicate)) = - obligation.predicate.kind().skip_binder() - { - // If the projection predicate (Foo::Bar == X) has X as a non-TyVid, - // we need to make it into one. - if let Some(vid) = predicate.term.ty().and_then(|ty| ty.ty_vid()) { - debug!("relationship: {:?}.output = true", vid); - relationships.entry(vid).or_default().output = true; - } - } -} From 6155a803805acaddd2518f09c2da70fbc320b274 Mon Sep 17 00:00:00 2001 From: Santiago Pastorino Date: Fri, 20 Jan 2023 16:45:01 -0300 Subject: [PATCH 11/17] Rename relationships to infer_var_info --- compiler/rustc_hir_typeck/src/fallback.rs | 10 +++++----- compiler/rustc_hir_typeck/src/inherited.rs | 12 ++++++------ compiler/rustc_middle/src/ty/mod.rs | 2 +- 3 files changed, 12 insertions(+), 12 deletions(-) diff --git a/compiler/rustc_hir_typeck/src/fallback.rs b/compiler/rustc_hir_typeck/src/fallback.rs index 4ef32023e7d8..943dc9b9646f 100644 --- a/compiler/rustc_hir_typeck/src/fallback.rs +++ b/compiler/rustc_hir_typeck/src/fallback.rs @@ -293,16 +293,16 @@ impl<'tcx> FnCtxt<'_, 'tcx> { .depth_first_search(root_vid) .any(|n| roots_reachable_from_non_diverging.visited(n)); - let mut relationship = ty::FoundRelationships { self_in_trait: false, output: false }; + let mut found_infer_var_info = ty::InferVarInfo { self_in_trait: false, output: false }; - for (vid, rel) in self.inh.relationships.borrow().iter() { + for (vid, info) in self.inh.infer_var_info.borrow().iter() { if self.infcx.root_var(*vid) == root_vid { - relationship.self_in_trait |= rel.self_in_trait; - relationship.output |= rel.output; + found_infer_var_info.self_in_trait |= info.self_in_trait; + found_infer_var_info.output |= info.output; } } - if relationship.self_in_trait && relationship.output { + if found_infer_var_info.self_in_trait && found_infer_var_info.output { // This case falls back to () to ensure that the code pattern in // tests/ui/never_type/fallback-closure-ret.rs continues to // compile when never_type_fallback is enabled. diff --git a/compiler/rustc_hir_typeck/src/inherited.rs b/compiler/rustc_hir_typeck/src/inherited.rs index ce7e5caaefc5..ba34f299453e 100644 --- a/compiler/rustc_hir_typeck/src/inherited.rs +++ b/compiler/rustc_hir_typeck/src/inherited.rs @@ -65,7 +65,7 @@ pub struct Inherited<'tcx> { /// fallback. See the `fallback` module for details. pub(super) diverging_type_vars: RefCell>>, - pub(super) relationships: RefCell>, + pub(super) infer_var_info: RefCell>, } impl<'tcx> Deref for Inherited<'tcx> { @@ -131,7 +131,7 @@ impl<'tcx> Inherited<'tcx> { deferred_generator_interiors: RefCell::new(Vec::new()), diverging_type_vars: RefCell::new(Default::default()), body_id, - relationships: RefCell::new(Default::default()), + infer_var_info: RefCell::new(Default::default()), } } @@ -161,7 +161,7 @@ impl<'tcx> Inherited<'tcx> { } pub fn update_infer_var_info(&self, obligation: &PredicateObligation<'tcx>) { - let relationships = &mut self.relationships.borrow_mut(); + let infer_var_info = &mut self.infer_var_info.borrow_mut(); // (*) binder skipped if let ty::PredicateKind::Clause(ty::Clause::Trait(tpred)) = obligation.predicate.kind().skip_binder() @@ -183,7 +183,7 @@ impl<'tcx> Inherited<'tcx> { ); // Don't report overflow errors. Otherwise equivalent to may_hold. if let Ok(result) = self.probe(|_| self.evaluate_obligation(&o)) && result.may_apply() { - relationships.entry(ty).or_default().self_in_trait = true; + infer_var_info.entry(ty).or_default().self_in_trait = true; } } @@ -193,8 +193,8 @@ impl<'tcx> Inherited<'tcx> { // If the projection predicate (Foo::Bar == X) has X as a non-TyVid, // we need to make it into one. if let Some(vid) = predicate.term.ty().and_then(|ty| ty.ty_vid()) { - debug!("relationships: {:?}.output = true", vid); - relationships.entry(vid).or_default().output = true; + debug!("infer_var_info: {:?}.output = true", vid); + infer_var_info.entry(vid).or_default().output = true; } } } diff --git a/compiler/rustc_middle/src/ty/mod.rs b/compiler/rustc_middle/src/ty/mod.rs index 7dfcd1bb5074..f83bceca3b53 100644 --- a/compiler/rustc_middle/src/ty/mod.rs +++ b/compiler/rustc_middle/src/ty/mod.rs @@ -2619,7 +2619,7 @@ impl<'tcx> fmt::Debug for SymbolName<'tcx> { } #[derive(Debug, Default, Copy, Clone)] -pub struct FoundRelationships { +pub struct InferVarInfo { /// This is true if we identified that this Ty (`?T`) is found in a `?T: Foo` /// obligation, where: /// From b905f80036a886c359149934b0772edd2f556c2a Mon Sep 17 00:00:00 2001 From: Santiago Pastorino Date: Sun, 22 Jan 2023 12:36:58 -0300 Subject: [PATCH 12/17] fn-trait-closure test now pass on new solver --- tests/ui/traits/new-solver/fn-trait-closure.rs | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/tests/ui/traits/new-solver/fn-trait-closure.rs b/tests/ui/traits/new-solver/fn-trait-closure.rs index c0ecf1c91fb3..bd65737ee398 100644 --- a/tests/ui/traits/new-solver/fn-trait-closure.rs +++ b/tests/ui/traits/new-solver/fn-trait-closure.rs @@ -1,12 +1,5 @@ // compile-flags: -Ztrait-solver=next -// known-bug: unknown -// failure-status: 101 -// dont-check-compiler-stderr - -// This test will fail until we fix `FulfillmentCtxt::relationships`. That's -// because we create a type variable for closure upvar types, which is not -// constrained until after we try to do fallback on diverging type variables. -// Thus, we will call that function, which is unimplemented. +// check-pass fn require_fn(_: impl Fn() -> i32) {} From 2aa5555ad3911536e2d0d363633f1a4cb67b1c49 Mon Sep 17 00:00:00 2001 From: yukang Date: Mon, 23 Jan 2023 00:42:20 +0800 Subject: [PATCH 13/17] Fix #106496, suggest remove deref for type mismatch --- compiler/rustc_hir_typeck/src/demand.rs | 16 +++++++ .../issue-88360.fixed | 20 +++++++++ .../generic-associated-types/issue-88360.rs | 2 + .../issue-88360.stderr | 12 +++--- .../ui/suggestions/suggest-remove-deref.fixed | 28 ++++++++++++ tests/ui/suggestions/suggest-remove-deref.rs | 28 ++++++++++++ .../suggestions/suggest-remove-deref.stderr | 43 +++++++++++++++++++ 7 files changed, 144 insertions(+), 5 deletions(-) create mode 100644 tests/ui/generic-associated-types/issue-88360.fixed create mode 100644 tests/ui/suggestions/suggest-remove-deref.fixed create mode 100644 tests/ui/suggestions/suggest-remove-deref.rs create mode 100644 tests/ui/suggestions/suggest-remove-deref.stderr diff --git a/compiler/rustc_hir_typeck/src/demand.rs b/compiler/rustc_hir_typeck/src/demand.rs index 665dc8b6a2f2..bd1626dff795 100644 --- a/compiler/rustc_hir_typeck/src/demand.rs +++ b/compiler/rustc_hir_typeck/src/demand.rs @@ -1233,6 +1233,22 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { sugg_sp = receiver.span; } } + + if let hir::ExprKind::Unary(hir::UnOp::Deref, ref inner) = expr.kind + && let Some(1) = self.deref_steps(expected, checked_ty) { + // We have `*&T`, check if what was expected was `&T`. + // If so, we may want to suggest removing a `*`. + sugg_sp = sugg_sp.with_hi(inner.span.lo()); + return Some(( + sugg_sp, + "consider removing deref here".to_string(), + "".to_string(), + Applicability::MachineApplicable, + true, + false, + )); + } + if let Ok(src) = sm.span_to_snippet(sugg_sp) { let needs_parens = match expr.kind { // parenthesize if needed (Issue #46756) diff --git a/tests/ui/generic-associated-types/issue-88360.fixed b/tests/ui/generic-associated-types/issue-88360.fixed new file mode 100644 index 000000000000..3dea8bf7ac81 --- /dev/null +++ b/tests/ui/generic-associated-types/issue-88360.fixed @@ -0,0 +1,20 @@ +// run-rustfix + +trait GatTrait { + type Gat<'a> where Self: 'a; + + fn test(&self) -> Self::Gat<'_>; +} + +trait SuperTrait +where + Self: 'static, + for<'a> Self: GatTrait = &'a T>, +{ + fn copy(&self) -> Self::Gat<'_> where T: Copy { + self.test() + //~^ mismatched types + } +} + +fn main() {} diff --git a/tests/ui/generic-associated-types/issue-88360.rs b/tests/ui/generic-associated-types/issue-88360.rs index c02690618d0e..4d4c7ea31807 100644 --- a/tests/ui/generic-associated-types/issue-88360.rs +++ b/tests/ui/generic-associated-types/issue-88360.rs @@ -1,3 +1,5 @@ +// run-rustfix + trait GatTrait { type Gat<'a> where Self: 'a; diff --git a/tests/ui/generic-associated-types/issue-88360.stderr b/tests/ui/generic-associated-types/issue-88360.stderr index cd3750344dda..520aeff18948 100644 --- a/tests/ui/generic-associated-types/issue-88360.stderr +++ b/tests/ui/generic-associated-types/issue-88360.stderr @@ -1,5 +1,5 @@ error[E0308]: mismatched types - --> $DIR/issue-88360.rs:13:9 + --> $DIR/issue-88360.rs:15:9 | LL | trait SuperTrait | - this type parameter @@ -7,13 +7,15 @@ LL | trait SuperTrait LL | fn copy(&self) -> Self::Gat<'_> where T: Copy { | ------------- expected `&T` because of return type LL | *self.test() - | ^^^^^^^^^^^^ - | | - | expected `&T`, found type parameter `T` - | help: consider borrowing here: `&*self.test()` + | ^^^^^^^^^^^^ expected `&T`, found type parameter `T` | = note: expected reference `&T` found type parameter `T` +help: consider removing deref here + | +LL - *self.test() +LL + self.test() + | error: aborting due to previous error diff --git a/tests/ui/suggestions/suggest-remove-deref.fixed b/tests/ui/suggestions/suggest-remove-deref.fixed new file mode 100644 index 000000000000..4dc12da03dd0 --- /dev/null +++ b/tests/ui/suggestions/suggest-remove-deref.fixed @@ -0,0 +1,28 @@ +// run-rustfix + +//issue #106496 + +struct S; + +trait X {} +impl X for S {} + +fn foo(_: &T) {} +fn test_foo() { + let hello = &S; + foo(hello); + //~^ ERROR mismatched types +} + +fn bar(_: &String) {} +fn test_bar() { + let v = String::from("hello"); + let s = &v; + bar(s); + //~^ ERROR mismatched types +} + +fn main() { + test_foo(); + test_bar(); +} diff --git a/tests/ui/suggestions/suggest-remove-deref.rs b/tests/ui/suggestions/suggest-remove-deref.rs new file mode 100644 index 000000000000..c2d385cbdc37 --- /dev/null +++ b/tests/ui/suggestions/suggest-remove-deref.rs @@ -0,0 +1,28 @@ +// run-rustfix + +//issue #106496 + +struct S; + +trait X {} +impl X for S {} + +fn foo(_: &T) {} +fn test_foo() { + let hello = &S; + foo(*hello); + //~^ ERROR mismatched types +} + +fn bar(_: &String) {} +fn test_bar() { + let v = String::from("hello"); + let s = &v; + bar(*s); + //~^ ERROR mismatched types +} + +fn main() { + test_foo(); + test_bar(); +} diff --git a/tests/ui/suggestions/suggest-remove-deref.stderr b/tests/ui/suggestions/suggest-remove-deref.stderr new file mode 100644 index 000000000000..f5d810e36f03 --- /dev/null +++ b/tests/ui/suggestions/suggest-remove-deref.stderr @@ -0,0 +1,43 @@ +error[E0308]: mismatched types + --> $DIR/suggest-remove-deref.rs:13:9 + | +LL | foo(*hello); + | --- ^^^^^^ expected reference, found struct `S` + | | + | arguments to this function are incorrect + | + = note: expected reference `&_` + found struct `S` +note: function defined here + --> $DIR/suggest-remove-deref.rs:10:4 + | +LL | fn foo(_: &T) {} + | ^^^ ----- +help: consider removing deref here + | +LL - foo(*hello); +LL + foo(hello); + | + +error[E0308]: mismatched types + --> $DIR/suggest-remove-deref.rs:21:9 + | +LL | bar(*s); + | --- ^^ expected `&String`, found struct `String` + | | + | arguments to this function are incorrect + | +note: function defined here + --> $DIR/suggest-remove-deref.rs:17:4 + | +LL | fn bar(_: &String) {} + | ^^^ ---------- +help: consider removing deref here + | +LL - bar(*s); +LL + bar(s); + | + +error: aborting due to 2 previous errors + +For more information about this error, try `rustc --explain E0308`. From f908f0be5a088a0e1748c7853a8acf75be6d1c18 Mon Sep 17 00:00:00 2001 From: Robin Schroer Date: Wed, 18 Jan 2023 19:58:50 +0900 Subject: [PATCH 14/17] Consider doc(alias) when providing typo suggestions This means that ```rust impl Foo { #[doc(alias = "quux")] fn bar(&self) {} } fn main() { (Foo {}).quux(); } ``` will suggest `bar`. This currently uses the "there is a method with a similar name" help text, because the point where we choose and emit a suggestion is different from where we gather the suggestions. Changes have mainly been made to the latter. The selection code will now fall back to aliased candidates, but generally only if there is no candidate that matches based on the existing Levenshtein methodology. Fixes #83968. --- compiler/rustc_hir_typeck/src/method/mod.rs | 2 +- compiler/rustc_hir_typeck/src/method/probe.rs | 49 +++++++++++++++++-- .../rustc_hir_typeck/src/method/suggest.rs | 16 +++--- .../methods/method-not-found-but-doc-alias.rs | 11 +++++ .../method-not-found-but-doc-alias.stderr | 12 +++++ 5 files changed, 77 insertions(+), 13 deletions(-) create mode 100644 tests/ui/methods/method-not-found-but-doc-alias.rs create mode 100644 tests/ui/methods/method-not-found-but-doc-alias.stderr diff --git a/compiler/rustc_hir_typeck/src/method/mod.rs b/compiler/rustc_hir_typeck/src/method/mod.rs index b810a967a245..47396204b14e 100644 --- a/compiler/rustc_hir_typeck/src/method/mod.rs +++ b/compiler/rustc_hir_typeck/src/method/mod.rs @@ -76,7 +76,7 @@ pub struct NoMatchData<'tcx> { pub unsatisfied_predicates: Vec<(ty::Predicate<'tcx>, Option>, Option>)>, pub out_of_scope_traits: Vec, - pub lev_candidate: Option, + pub similar_candidate: Option, pub mode: probe::Mode, } diff --git a/compiler/rustc_hir_typeck/src/method/probe.rs b/compiler/rustc_hir_typeck/src/method/probe.rs index a2481431363d..9c06a22315bc 100644 --- a/compiler/rustc_hir_typeck/src/method/probe.rs +++ b/compiler/rustc_hir_typeck/src/method/probe.rs @@ -461,7 +461,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { static_candidates: Vec::new(), unsatisfied_predicates: Vec::new(), out_of_scope_traits: Vec::new(), - lev_candidate: None, + similar_candidate: None, mode, })); } @@ -1076,13 +1076,13 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> { if let Some((kind, def_id)) = private_candidate { return Err(MethodError::PrivateMatch(kind, def_id, out_of_scope_traits)); } - let lev_candidate = self.probe_for_lev_candidate()?; + let similar_candidate = self.probe_for_similar_candidate()?; Err(MethodError::NoMatch(NoMatchData { static_candidates, unsatisfied_predicates, out_of_scope_traits, - lev_candidate, + similar_candidate, mode: self.mode, })) } @@ -1787,7 +1787,7 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> { /// Similarly to `probe_for_return_type`, this method attempts to find the best matching /// candidate method where the method name may have been misspelled. Similarly to other /// Levenshtein based suggestions, we provide at most one such suggestion. - fn probe_for_lev_candidate(&mut self) -> Result, MethodError<'tcx>> { + fn probe_for_similar_candidate(&mut self) -> Result, MethodError<'tcx>> { debug!("probing for method names similar to {:?}", self.method_name); let steps = self.steps.clone(); @@ -1831,6 +1831,12 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> { None, ) } + .or_else(|| { + applicable_close_candidates + .iter() + .find(|cand| self.matches_by_doc_alias(cand.def_id)) + .map(|cand| cand.name) + }) .unwrap(); Ok(applicable_close_candidates.into_iter().find(|method| method.name == best_name)) } @@ -1981,6 +1987,38 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> { } } + /// Determine if the associated item withe the given DefId matches + /// the desired name via a doc alias. + fn matches_by_doc_alias(&self, def_id: DefId) -> bool { + let Some(name) = self.method_name else { return false; }; + let Some(local_def_id) = def_id.as_local() else { return false; }; + let hir_id = self.fcx.tcx.hir().local_def_id_to_hir_id(local_def_id); + let attrs = self.fcx.tcx.hir().attrs(hir_id); + for attr in attrs { + let sym::doc = attr.name_or_empty() else { continue; }; + let Some(values) = attr.meta_item_list() else { continue; }; + for v in values { + if v.name_or_empty() != sym::alias { + continue; + } + if let Some(nested) = v.meta_item_list() { + // #[doc(alias("foo", "bar"))] + for n in nested { + if let Some(lit) = n.lit() && name.as_str() == lit.symbol.as_str() { + return true; + } + } + } else if let Some(meta) = v.meta_item() + && let Some(lit) = meta.name_value_literal() + && name.as_str() == lit.symbol.as_str() { + // #[doc(alias = "foo")] + return true; + } + } + } + false + } + /// Finds the method with the appropriate name (or return type, as the case may be). If /// `allow_similar_names` is set, find methods with close-matching names. // The length of the returned iterator is nearly always 0 or 1 and this @@ -1996,6 +2034,9 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> { if !self.is_relevant_kind_for_mode(x.kind) { return false; } + if self.matches_by_doc_alias(x.def_id) { + return true; + } match lev_distance_with_substrings(name.as_str(), x.name.as_str(), max_dist) { Some(d) => d > 0, diff --git a/compiler/rustc_hir_typeck/src/method/suggest.rs b/compiler/rustc_hir_typeck/src/method/suggest.rs index 2e1fc4c38b54..8c54e9bdb5fb 100644 --- a/compiler/rustc_hir_typeck/src/method/suggest.rs +++ b/compiler/rustc_hir_typeck/src/method/suggest.rs @@ -262,7 +262,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { let ty_str = with_forced_trimmed_paths!(self.ty_to_string(rcvr_ty)); let is_method = mode == Mode::MethodCall; let unsatisfied_predicates = &no_match_data.unsatisfied_predicates; - let lev_candidate = no_match_data.lev_candidate; + let similar_candidate = no_match_data.similar_candidate; let item_kind = if is_method { "method" } else if rcvr_ty.is_enum() { @@ -937,7 +937,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { // give a helping note that it has to be called as `(x.f)(...)`. if let SelfSource::MethodCall(expr) = source { if !self.suggest_calling_field_as_fn(span, rcvr_ty, expr, item_name, &mut err) - && lev_candidate.is_none() + && similar_candidate.is_none() && !custom_span_label { label_span_not_found(&mut err); @@ -1015,20 +1015,20 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { if fallback_span { err.span_label(span, msg); } - } else if let Some(lev_candidate) = lev_candidate { + } else if let Some(similar_candidate) = similar_candidate { // Don't emit a suggestion if we found an actual method // that had unsatisfied trait bounds if unsatisfied_predicates.is_empty() { - let def_kind = lev_candidate.kind.as_def_kind(); + let def_kind = similar_candidate.kind.as_def_kind(); // Methods are defined within the context of a struct and their first parameter is always self, // which represents the instance of the struct the method is being called on // Associated functions don’t take self as a parameter and // they are not methods because they don’t have an instance of the struct to work with. - if def_kind == DefKind::AssocFn && lev_candidate.fn_has_self_parameter { + if def_kind == DefKind::AssocFn && similar_candidate.fn_has_self_parameter { err.span_suggestion( span, "there is a method with a similar name", - lev_candidate.name, + similar_candidate.name, Applicability::MaybeIncorrect, ); } else { @@ -1037,9 +1037,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { &format!( "there is {} {} with a similar name", def_kind.article(), - def_kind.descr(lev_candidate.def_id), + def_kind.descr(similar_candidate.def_id), ), - lev_candidate.name, + similar_candidate.name, Applicability::MaybeIncorrect, ); } diff --git a/tests/ui/methods/method-not-found-but-doc-alias.rs b/tests/ui/methods/method-not-found-but-doc-alias.rs new file mode 100644 index 000000000000..9c6d10029239 --- /dev/null +++ b/tests/ui/methods/method-not-found-but-doc-alias.rs @@ -0,0 +1,11 @@ +struct Foo; + +impl Foo { + #[doc(alias = "quux")] + fn bar(&self) {} +} + +fn main() { + Foo.quux(); + //~^ ERROR no method named `quux` found for struct `Foo` in the current scope +} diff --git a/tests/ui/methods/method-not-found-but-doc-alias.stderr b/tests/ui/methods/method-not-found-but-doc-alias.stderr new file mode 100644 index 000000000000..5102a452f0c2 --- /dev/null +++ b/tests/ui/methods/method-not-found-but-doc-alias.stderr @@ -0,0 +1,12 @@ +error[E0599]: no method named `quux` found for struct `Foo` in the current scope + --> $DIR/method-not-found-but-doc-alias.rs:9:9 + | +LL | struct Foo; + | ---------- method `quux` not found for this struct +... +LL | Foo.quux(); + | ^^^^ help: there is a method with a similar name: `bar` + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0599`. From c1cced8d040a26f27490531e7dbffda54642982f Mon Sep 17 00:00:00 2001 From: joboet Date: Thu, 26 Jan 2023 16:16:03 +0100 Subject: [PATCH 15/17] std: optimize `LazyLock` size --- library/std/src/sync/lazy_lock.rs | 82 +++++++++++++++++++----- library/std/src/sync/mod.rs | 2 +- library/std/src/sync/once.rs | 16 +++++ library/std/src/sys/unsupported/once.rs | 11 ++++ library/std/src/sys_common/once/futex.rs | 11 ++++ library/std/src/sys_common/once/queue.rs | 11 ++++ 6 files changed, 116 insertions(+), 17 deletions(-) diff --git a/library/std/src/sync/lazy_lock.rs b/library/std/src/sync/lazy_lock.rs index 4a15305301d6..d7509bb97324 100644 --- a/library/std/src/sync/lazy_lock.rs +++ b/library/std/src/sync/lazy_lock.rs @@ -1,8 +1,22 @@ -use crate::cell::Cell; +use crate::mem::ManuallyDrop; + +use crate::cell::UnsafeCell; use crate::fmt; use crate::ops::Deref; use crate::panic::{RefUnwindSafe, UnwindSafe}; -use crate::sync::OnceLock; +use crate::sync::Once; + +use super::once::ExclusiveState; + +// We use the state of a Once as discriminant value. Upon creation, the state is +// "incomplete" and `f` contains the initialization closure. In the first call to +// `call_once`, `f` is taken and run. If it succeeds, `value` is set and the state +// is changed to "complete". If it panics, the Once is poisoned, so none of the +// two fields is initialized. +union Data { + value: ManuallyDrop, + f: ManuallyDrop, +} /// A value which is initialized on the first access. /// @@ -43,16 +57,17 @@ use crate::sync::OnceLock; /// ``` #[unstable(feature = "once_cell", issue = "74465")] pub struct LazyLock T> { - cell: OnceLock, - init: Cell>, + once: Once, + data: UnsafeCell>, } + impl T> LazyLock { /// Creates a new lazy value with the given initializing /// function. #[inline] #[unstable(feature = "once_cell", issue = "74465")] pub const fn new(f: F) -> LazyLock { - LazyLock { cell: OnceLock::new(), init: Cell::new(Some(f)) } + LazyLock { once: Once::new(), data: UnsafeCell::new(Data { f: ManuallyDrop::new(f) }) } } /// Forces the evaluation of this lazy value and @@ -74,10 +89,43 @@ impl T> LazyLock { #[inline] #[unstable(feature = "once_cell", issue = "74465")] pub fn force(this: &LazyLock) -> &T { - this.cell.get_or_init(|| match this.init.take() { - Some(f) => f(), - None => panic!("Lazy instance has previously been poisoned"), - }) + this.once.call_once(|| { + // SAFETY: `call_once` only runs this closure once, ever. + let data = unsafe { &mut *this.data.get() }; + let f = unsafe { ManuallyDrop::take(&mut data.f) }; + let value = f(); + data.value = ManuallyDrop::new(value); + }); + + // SAFETY: + // There are four possible scenarios: + // * the closure was called and initialized `value`. + // * the closure was called and panicked, so this point is never reached. + // * the closure was not called, but a previous call initialized `value`. + // * the closure was not called because the Once is poisoned, so this point + // is never reached. + // So `value` has definitely been initialized and will not be modified again. + unsafe { &*(*this.data.get()).value } + } +} + +impl LazyLock { + /// Get the inner value if it has already been initialized. + fn get(&self) -> Option<&T> { + if self.once.is_completed() { Some(unsafe { &*(*self.data.get()).value }) } else { None } + } +} + +#[unstable(feature = "once_cell", issue = "74465")] +impl Drop for LazyLock { + fn drop(&mut self) { + match self.once.state() { + ExclusiveState::Incomplete => unsafe { ManuallyDrop::drop(&mut self.data.get_mut().f) }, + ExclusiveState::Complete => unsafe { + ManuallyDrop::drop(&mut self.data.get_mut().value) + }, + ExclusiveState::Poisoned => {} + } } } @@ -103,23 +151,25 @@ impl Default for LazyLock { #[unstable(feature = "once_cell", issue = "74465")] impl fmt::Debug for LazyLock { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - f.debug_struct("Lazy").field("cell", &self.cell).finish_non_exhaustive() + f.debug_tuple("LazyLock") + .field(match self.get() { + Some(v) => v, + None => &"Uninit", + }) + .finish() } } // We never create a `&F` from a `&LazyLock` so it is fine // to not impl `Sync` for `F` -// we do create a `&mut Option` in `force`, but this is -// properly synchronized, so it only happens once -// so it also does not contribute to this impl. #[unstable(feature = "once_cell", issue = "74465")] -unsafe impl Sync for LazyLock where OnceLock: Sync {} +unsafe impl Sync for LazyLock {} // auto-derived `Send` impl is OK. #[unstable(feature = "once_cell", issue = "74465")] -impl RefUnwindSafe for LazyLock where OnceLock: RefUnwindSafe {} +impl RefUnwindSafe for LazyLock {} #[unstable(feature = "once_cell", issue = "74465")] -impl UnwindSafe for LazyLock where OnceLock: UnwindSafe {} +impl UnwindSafe for LazyLock {} #[cfg(test)] mod tests; diff --git a/library/std/src/sync/mod.rs b/library/std/src/sync/mod.rs index ba20bab87a40..4edc956173b0 100644 --- a/library/std/src/sync/mod.rs +++ b/library/std/src/sync/mod.rs @@ -186,7 +186,7 @@ mod condvar; mod lazy_lock; mod mpmc; mod mutex; -mod once; +pub(crate) mod once; mod once_lock; mod poison; mod remutex; diff --git a/library/std/src/sync/once.rs b/library/std/src/sync/once.rs index 0f25417d6b5d..1b17c31089ff 100644 --- a/library/std/src/sync/once.rs +++ b/library/std/src/sync/once.rs @@ -43,6 +43,12 @@ pub struct OnceState { pub(crate) inner: sys::OnceState, } +pub(crate) enum ExclusiveState { + Incomplete, + Poisoned, + Complete, +} + /// Initialization value for static [`Once`] values. /// /// # Examples @@ -248,6 +254,16 @@ impl Once { pub fn is_completed(&self) -> bool { self.inner.is_completed() } + + /// Returns the current state of the `Once` instance. + /// + /// Since this takes a mutable reference, no initialization can currently + /// be running, so the state must be either "incomplete", "poisoned" or + /// "complete". + #[inline] + pub(crate) fn state(&mut self) -> ExclusiveState { + self.inner.state() + } } #[stable(feature = "std_debug", since = "1.16.0")] diff --git a/library/std/src/sys/unsupported/once.rs b/library/std/src/sys/unsupported/once.rs index b4bb4975f41c..11fde1888ba7 100644 --- a/library/std/src/sys/unsupported/once.rs +++ b/library/std/src/sys/unsupported/once.rs @@ -1,5 +1,6 @@ use crate::cell::Cell; use crate::sync as public; +use crate::sync::once::ExclusiveState; pub struct Once { state: Cell, @@ -44,6 +45,16 @@ impl Once { self.state.get() == State::Complete } + #[inline] + pub(crate) fn state(&mut self) -> ExclusiveState { + match self.state.get() { + State::Incomplete => ExclusiveState::Incomplete, + State::Poisoned => ExclusiveState::Poisoned, + State::Complete => ExclusiveState::Complete, + _ => unreachable!("invalid Once state"), + } + } + #[cold] #[track_caller] pub fn call(&self, ignore_poisoning: bool, f: &mut impl FnMut(&public::OnceState)) { diff --git a/library/std/src/sys_common/once/futex.rs b/library/std/src/sys_common/once/futex.rs index 5c7e6c013715..42db5fad4b45 100644 --- a/library/std/src/sys_common/once/futex.rs +++ b/library/std/src/sys_common/once/futex.rs @@ -4,6 +4,7 @@ use crate::sync::atomic::{ AtomicU32, Ordering::{Acquire, Relaxed, Release}, }; +use crate::sync::once::ExclusiveState; use crate::sys::futex::{futex_wait, futex_wake_all}; // On some platforms, the OS is very nice and handles the waiter queue for us. @@ -78,6 +79,16 @@ impl Once { self.state.load(Acquire) == COMPLETE } + #[inline] + pub(crate) fn state(&mut self) -> ExclusiveState { + match *self.state.get_mut() { + INCOMPLETE => ExclusiveState::Incomplete, + POISONED => ExclusiveState::Poisoned, + COMPLETE => ExclusiveState::Complete, + _ => unreachable!("invalid Once state"), + } + } + // This uses FnMut to match the API of the generic implementation. As this // implementation is quite light-weight, it is generic over the closure and // so avoids the cost of dynamic dispatch. diff --git a/library/std/src/sys_common/once/queue.rs b/library/std/src/sys_common/once/queue.rs index d953a6745923..def0bcd6fac4 100644 --- a/library/std/src/sys_common/once/queue.rs +++ b/library/std/src/sys_common/once/queue.rs @@ -60,6 +60,7 @@ use crate::fmt; use crate::ptr; use crate::sync as public; use crate::sync::atomic::{AtomicBool, AtomicPtr, Ordering}; +use crate::sync::once::ExclusiveState; use crate::thread::{self, Thread}; type Masked = (); @@ -121,6 +122,16 @@ impl Once { self.state_and_queue.load(Ordering::Acquire).addr() == COMPLETE } + #[inline] + pub(crate) fn state(&mut self) -> ExclusiveState { + match self.state_and_queue.get_mut().addr() { + INCOMPLETE => ExclusiveState::Incomplete, + POISONED => ExclusiveState::Poisoned, + COMPLETE => ExclusiveState::Complete, + _ => unreachable!("invalid Once state"), + } + } + // This is a non-generic function to reduce the monomorphization cost of // using `call_once` (this isn't exactly a trivial or small implementation). // From 7165e610a21ff78a76775ad2941ef882814f4342 Mon Sep 17 00:00:00 2001 From: joboet Date: Fri, 27 Jan 2023 10:09:38 +0100 Subject: [PATCH 16/17] std: fix `Debug` implementation on `LazyLock` --- library/std/src/sync/lazy_lock.rs | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/library/std/src/sync/lazy_lock.rs b/library/std/src/sync/lazy_lock.rs index d7509bb97324..98da015423a5 100644 --- a/library/std/src/sync/lazy_lock.rs +++ b/library/std/src/sync/lazy_lock.rs @@ -1,7 +1,6 @@ -use crate::mem::ManuallyDrop; - use crate::cell::UnsafeCell; use crate::fmt; +use crate::mem::ManuallyDrop; use crate::ops::Deref; use crate::panic::{RefUnwindSafe, UnwindSafe}; use crate::sync::Once; @@ -151,12 +150,10 @@ impl Default for LazyLock { #[unstable(feature = "once_cell", issue = "74465")] impl fmt::Debug for LazyLock { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - f.debug_tuple("LazyLock") - .field(match self.get() { - Some(v) => v, - None => &"Uninit", - }) - .finish() + match self.get() { + Some(v) => f.debug_tuple("LazyLock").field(v).finish(), + None => f.write_str("LazyLock(Uninit)"), + } } } From 6520488e37f39a11affd776ab1283a0a3fe8087e Mon Sep 17 00:00:00 2001 From: joboet Date: Fri, 27 Jan 2023 10:11:42 +0100 Subject: [PATCH 17/17] std: add safety comment in `LazyLock::get` --- library/std/src/sync/lazy_lock.rs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/library/std/src/sync/lazy_lock.rs b/library/std/src/sync/lazy_lock.rs index 98da015423a5..7e85d6a063a7 100644 --- a/library/std/src/sync/lazy_lock.rs +++ b/library/std/src/sync/lazy_lock.rs @@ -111,7 +111,14 @@ impl T> LazyLock { impl LazyLock { /// Get the inner value if it has already been initialized. fn get(&self) -> Option<&T> { - if self.once.is_completed() { Some(unsafe { &*(*self.data.get()).value }) } else { None } + if self.once.is_completed() { + // SAFETY: + // The closure has been run successfully, so `value` has been initialized + // and will not be modified again. + Some(unsafe { &*(*self.data.get()).value }) + } else { + None + } } }