From 4dded23925994103e3e793d5edbe0d30222cdaa4 Mon Sep 17 00:00:00 2001 From: Ibraheem Ahmed Date: Mon, 28 Feb 2022 19:59:21 -0500 Subject: [PATCH 01/14] add `special_module_name` lint --- compiler/rustc_lint/src/builtin.rs | 74 +++++++++++++++++++ compiler/rustc_lint/src/lib.rs | 1 + src/test/ui/modules/dummy.rs | 0 src/test/ui/modules/special_module_name.rs | 8 ++ .../ui/modules/special_module_name.stderr | 37 ++++++++++ .../ui/modules/special_module_name_ignore.rs | 9 +++ 6 files changed, 129 insertions(+) create mode 100644 src/test/ui/modules/dummy.rs create mode 100644 src/test/ui/modules/special_module_name.rs create mode 100644 src/test/ui/modules/special_module_name.stderr create mode 100644 src/test/ui/modules/special_module_name_ignore.rs diff --git a/compiler/rustc_lint/src/builtin.rs b/compiler/rustc_lint/src/builtin.rs index 30b5f9b34d099..961e1e9507b97 100644 --- a/compiler/rustc_lint/src/builtin.rs +++ b/compiler/rustc_lint/src/builtin.rs @@ -3248,3 +3248,77 @@ impl<'tcx> LateLintPass<'tcx> for NamedAsmLabels { } } } + +declare_lint! { + /// The `special_module_name` lint detects module + /// declarations for files that have a special meaning. + /// + /// ### Example + /// + /// ```rust,compile_fail + /// mod lib; + /// + /// fn main() { + /// lib::run(); + /// } + /// ``` + /// + /// {{produces}} + /// + /// ### Explanation + /// + /// Cargo recognizes `lib.rs` and `main.rs` as the root of a + /// library or binary crate, so declaring them as modules + /// will lead to miscompilation of the crate unless configured + /// explicitly. + /// + /// To access a library from a binary target within the same crate, + /// use `your_crate_name::` as the path path instead of `lib::`: + /// + /// ```rust,compile_fail + /// // bar/src/lib.rs + /// fn run() { + /// // ... + /// } + /// + /// // bar/src/main.rs + /// fn main() { + /// bar::run(); + /// } + /// ``` + /// + /// Binary targets cannot be used as libraries and so declaring + /// one as a module is not allowed. + pub SPECIAL_MODULE_NAME, + Warn, + "module declarations for files with a special meaning", +} + +declare_lint_pass!(SpecialModuleName => [SPECIAL_MODULE_NAME]); + +impl EarlyLintPass for SpecialModuleName { + fn check_crate(&mut self, cx: &EarlyContext<'_>, krate: &ast::Crate) { + for item in &krate.items { + if let ast::ItemKind::Mod(..) = item.kind { + if item.attrs.iter().any(|a| a.has_name(sym::path)) { + continue; + } + + match item.ident.name.as_str() { + "lib" => cx.struct_span_lint(SPECIAL_MODULE_NAME, item.span, |lint| { + lint.build("found module declaration for lib.rs") + .note("lib.rs is the root of this crate's library target") + .help("to refer to it from other targets, use the library's name as the path") + .emit() + }), + "main" => cx.struct_span_lint(SPECIAL_MODULE_NAME, item.span, |lint| { + lint.build("found module declaration for main.rs") + .note("a binary crate cannot be used as library") + .emit() + }), + _ => continue + } + } + } + } +} diff --git a/compiler/rustc_lint/src/lib.rs b/compiler/rustc_lint/src/lib.rs index 69863b5ff827f..107df79c3809b 100644 --- a/compiler/rustc_lint/src/lib.rs +++ b/compiler/rustc_lint/src/lib.rs @@ -129,6 +129,7 @@ macro_rules! early_lint_passes { UnusedBraces: UnusedBraces, UnusedImportBraces: UnusedImportBraces, UnsafeCode: UnsafeCode, + SpecialModuleName: SpecialModuleName, AnonymousParameters: AnonymousParameters, EllipsisInclusiveRangePatterns: EllipsisInclusiveRangePatterns::default(), NonCamelCaseTypes: NonCamelCaseTypes, diff --git a/src/test/ui/modules/dummy.rs b/src/test/ui/modules/dummy.rs new file mode 100644 index 0000000000000..e69de29bb2d1d diff --git a/src/test/ui/modules/special_module_name.rs b/src/test/ui/modules/special_module_name.rs new file mode 100644 index 0000000000000..15c59b2da828c --- /dev/null +++ b/src/test/ui/modules/special_module_name.rs @@ -0,0 +1,8 @@ +mod lib; +//~^ WARN found module declaration for lib.rs +//~| ERROR file not found for module `lib` +mod main; +//~^ WARN found module declaration for main.rs +//~| ERROR file not found for module `main` + +fn main() {} diff --git a/src/test/ui/modules/special_module_name.stderr b/src/test/ui/modules/special_module_name.stderr new file mode 100644 index 0000000000000..8b3da29386df2 --- /dev/null +++ b/src/test/ui/modules/special_module_name.stderr @@ -0,0 +1,37 @@ +error[E0583]: file not found for module `lib` + --> $DIR/special_module_name.rs:1:1 + | +LL | mod lib; + | ^^^^^^^^ + | + = help: to create the module `lib`, create file "$DIR/lib.rs" or "$DIR/lib/mod.rs" + +error[E0583]: file not found for module `main` + --> $DIR/special_module_name.rs:4:1 + | +LL | mod main; + | ^^^^^^^^^ + | + = help: to create the module `main`, create file "$DIR/main.rs" or "$DIR/main/mod.rs" + +warning: found module declaration for lib.rs + --> $DIR/special_module_name.rs:1:1 + | +LL | mod lib; + | ^^^^^^^^ + | + = note: `#[warn(special_module_name)]` on by default + = note: lib.rs is the root of this crate's library target + = help: to refer to it from other targets, use the library's name as the path + +warning: found module declaration for main.rs + --> $DIR/special_module_name.rs:4:1 + | +LL | mod main; + | ^^^^^^^^^ + | + = note: a binary crate cannot be used as library + +error: aborting due to 2 previous errors; 2 warnings emitted + +For more information about this error, try `rustc --explain E0583`. diff --git a/src/test/ui/modules/special_module_name_ignore.rs b/src/test/ui/modules/special_module_name_ignore.rs new file mode 100644 index 0000000000000..cae06b49ee0b7 --- /dev/null +++ b/src/test/ui/modules/special_module_name_ignore.rs @@ -0,0 +1,9 @@ +// run-pass + +#[path = "dummy.rs"] +mod lib; + +#[path = "dummy.rs"] +mod main; + +fn main() {} From c08f460beb86b60aab150f258d96bf99c6eae1b8 Mon Sep 17 00:00:00 2001 From: Ibraheem Ahmed Date: Fri, 15 Apr 2022 16:21:21 -0400 Subject: [PATCH 02/14] tidy --- src/test/ui/modules/dummy.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/test/ui/modules/dummy.rs b/src/test/ui/modules/dummy.rs index e69de29bb2d1d..831e38292a9ba 100644 --- a/src/test/ui/modules/dummy.rs +++ b/src/test/ui/modules/dummy.rs @@ -0,0 +1 @@ +pub struct Dummy; From f479289e78bc60fbb2aaa3abb33f7726bb12ea89 Mon Sep 17 00:00:00 2001 From: Ibraheem Ahmed Date: Sat, 4 Jun 2022 16:12:45 -0400 Subject: [PATCH 03/14] move dummy test module to auxiliary directory --- src/test/ui/modules/auxiliary/dummy_lib.rs | 2 ++ src/test/ui/modules/dummy.rs | 1 - src/test/ui/modules/special_module_name_ignore.rs | 4 ++-- 3 files changed, 4 insertions(+), 3 deletions(-) create mode 100644 src/test/ui/modules/auxiliary/dummy_lib.rs delete mode 100644 src/test/ui/modules/dummy.rs diff --git a/src/test/ui/modules/auxiliary/dummy_lib.rs b/src/test/ui/modules/auxiliary/dummy_lib.rs new file mode 100644 index 0000000000000..ef805c1f02031 --- /dev/null +++ b/src/test/ui/modules/auxiliary/dummy_lib.rs @@ -0,0 +1,2 @@ +#[allow(dead_code)] +pub struct Dummy; diff --git a/src/test/ui/modules/dummy.rs b/src/test/ui/modules/dummy.rs deleted file mode 100644 index 831e38292a9ba..0000000000000 --- a/src/test/ui/modules/dummy.rs +++ /dev/null @@ -1 +0,0 @@ -pub struct Dummy; diff --git a/src/test/ui/modules/special_module_name_ignore.rs b/src/test/ui/modules/special_module_name_ignore.rs index cae06b49ee0b7..07cea9b2b05a1 100644 --- a/src/test/ui/modules/special_module_name_ignore.rs +++ b/src/test/ui/modules/special_module_name_ignore.rs @@ -1,9 +1,9 @@ // run-pass -#[path = "dummy.rs"] +#[path = "auxiliary/dummy_lib.rs"] mod lib; -#[path = "dummy.rs"] +#[path = "auxiliary/dummy_lib.rs"] mod main; fn main() {} From f7ae92c6bd9b50e3d1cd7ce123ffa15d0e1ecd97 Mon Sep 17 00:00:00 2001 From: joboet Date: Thu, 30 Jun 2022 11:48:54 +0200 Subject: [PATCH 04/14] std: use futex-based locks on Fuchsia --- library/std/src/sys/unix/futex.rs | 25 ++- .../std/src/sys/unix/locks/fuchsia_mutex.rs | 158 ++++++++++++++++++ .../std/src/sys/unix/locks/futex_condvar.rs | 58 +++++++ .../unix/locks/{futex.rs => futex_mutex.rs} | 56 +------ library/std/src/sys/unix/locks/mod.rs | 13 +- 5 files changed, 246 insertions(+), 64 deletions(-) create mode 100644 library/std/src/sys/unix/locks/fuchsia_mutex.rs create mode 100644 library/std/src/sys/unix/locks/futex_condvar.rs rename library/std/src/sys/unix/locks/{futex.rs => futex_mutex.rs} (62%) diff --git a/library/std/src/sys/unix/futex.rs b/library/std/src/sys/unix/futex.rs index ab516a7f76dd0..9480451fc5c86 100644 --- a/library/std/src/sys/unix/futex.rs +++ b/library/std/src/sys/unix/futex.rs @@ -240,17 +240,22 @@ pub fn futex_wake_all(futex: &AtomicU32) { } #[cfg(target_os = "fuchsia")] -mod zircon { - type zx_time_t = i64; - type zx_futex_t = crate::sync::atomic::AtomicU32; - type zx_handle_t = u32; - type zx_status_t = i32; +pub mod zircon { + pub type zx_futex_t = crate::sync::atomic::AtomicU32; + pub type zx_handle_t = u32; + pub type zx_status_t = i32; + pub type zx_time_t = i64; pub const ZX_HANDLE_INVALID: zx_handle_t = 0; - pub const ZX_ERR_TIMED_OUT: zx_status_t = -21; + pub const ZX_TIME_INFINITE: zx_time_t = zx_time_t::MAX; + pub const ZX_OK: zx_status_t = 0; + pub const ZX_ERR_BAD_STATE: zx_status_t = -20; + pub const ZX_ERR_TIMED_OUT: zx_status_t = -21; + extern "C" { + pub fn zx_clock_get_monotonic() -> zx_time_t; pub fn zx_futex_wait( value_ptr: *const zx_futex_t, current_value: zx_futex_t, @@ -258,7 +263,8 @@ mod zircon { deadline: zx_time_t, ) -> zx_status_t; pub fn zx_futex_wake(value_ptr: *const zx_futex_t, wake_count: u32) -> zx_status_t; - pub fn zx_clock_get_monotonic() -> zx_time_t; + pub fn zx_futex_wake_single_owner(value_ptr: *const zx_futex_t) -> zx_status_t; + pub fn zx_thread_self() -> zx_handle_t; } } @@ -287,3 +293,8 @@ pub fn futex_wake(futex: &AtomicU32) -> bool { unsafe { zircon::zx_futex_wake(futex, 1) }; false } + +#[cfg(target_os = "fuchsia")] +pub fn futex_wake_all(futex: &AtomicU32) { + unsafe { zircon::zx_futex_wake(futex, u32::MAX) }; +} diff --git a/library/std/src/sys/unix/locks/fuchsia_mutex.rs b/library/std/src/sys/unix/locks/fuchsia_mutex.rs new file mode 100644 index 0000000000000..412e7e0018b48 --- /dev/null +++ b/library/std/src/sys/unix/locks/fuchsia_mutex.rs @@ -0,0 +1,158 @@ +//! A priority inheriting mutex for Fuchsia. +//! +//! This is a port of the [mutex in Fuchsia's libsync]. Contrary to the original, +//! it does not abort the process when reentrant locking is detected, but deadlocks. +//! +//! Priority inheritance is achieved by storing the owning thread's handle in an +//! atomic variable. Fuchsia's futex operations support setting an owner thread +//! for a futex, which can boost that thread's priority while the futex is waited +//! upon. +//! +//! libsync is licenced under the following BSD-style licence: +//! +//! Copyright 2016 The Fuchsia Authors. +//! +//! Redistribution and use in source and binary forms, with or without +//! modification, are permitted provided that the following conditions are +//! met: +//! +//! * Redistributions of source code must retain the above copyright +//! notice, this list of conditions and the following disclaimer. +//! * Redistributions in binary form must reproduce the above +//! copyright notice, this list of conditions and the following +//! disclaimer in the documentation and/or other materials provided +//! with the distribution. +//! +//! THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +//! "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +//! LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +//! A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +//! OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +//! SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +//! LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +//! DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +//! THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +//! (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +//! OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. +//! +//! [mutex in Fuchsia's libsync]: https://cs.opensource.google/fuchsia/fuchsia/+/main:zircon/system/ulib/sync/mutex.c + +use crate::sync::atomic::{ + AtomicU32, + Ordering::{Acquire, Relaxed, Release}, +}; +use crate::sys::futex::zircon::{ + zx_futex_wait, zx_futex_wake_single_owner, zx_handle_t, zx_thread_self, ZX_ERR_BAD_STATE, + ZX_OK, ZX_TIME_INFINITE, +}; + +// The lowest two bits of a `zx_handle_t` are always set, so the lowest bit is used to mark the +// mutex as contested by clearing it. +const CONTESTED_BIT: u32 = 1; +// This can never be a valid `zx_handle_t`. +const UNLOCKED: u32 = 0; + +pub type MovableMutex = Mutex; + +pub struct Mutex { + futex: AtomicU32, +} + +#[inline] +fn to_state(owner: zx_handle_t) -> u32 { + owner +} + +#[inline] +fn to_owner(state: u32) -> zx_handle_t { + state | CONTESTED_BIT +} + +#[inline] +fn is_contested(state: u32) -> bool { + state & CONTESTED_BIT == 0 +} + +#[inline] +fn mark_contested(state: u32) -> u32 { + state & !CONTESTED_BIT +} + +impl Mutex { + #[inline] + pub const fn new() -> Mutex { + Mutex { futex: AtomicU32::new(UNLOCKED) } + } + + #[inline] + pub unsafe fn init(&mut self) {} + + #[inline] + pub unsafe fn try_lock(&self) -> bool { + let thread_self = zx_thread_self(); + self.futex.compare_exchange(UNLOCKED, to_state(thread_self), Acquire, Relaxed).is_ok() + } + + #[inline] + pub unsafe fn lock(&self) { + let thread_self = zx_thread_self(); + if let Err(state) = + self.futex.compare_exchange(UNLOCKED, to_state(thread_self), Acquire, Relaxed) + { + self.lock_contested(state, thread_self); + } + } + + #[cold] + fn lock_contested(&self, mut state: u32, thread_self: zx_handle_t) { + let owned_state = mark_contested(to_state(thread_self)); + loop { + // Mark the mutex as contested if it is not already. + let contested = mark_contested(state); + if is_contested(state) + || self.futex.compare_exchange(state, contested, Relaxed, Relaxed).is_ok() + { + // The mutex has been marked as contested, wait for the state to change. + unsafe { + match zx_futex_wait( + &self.futex, + AtomicU32::new(contested), + to_owner(state), + ZX_TIME_INFINITE, + ) { + ZX_OK | ZX_ERR_BAD_STATE => (), + // Deadlock even in the case of reentrant locking, as leaking a guard + // could lead to the same condition if the thread id is reused, but + // panicking is not expected in that situation. This makes things + // quite a bit harder to debug, but encourages portable programming. + _ if to_owner(state) == thread_self => loop {}, + error => panic!("futex operation failed with error code {error}"), + } + } + } + + // The state has changed or a wakeup occured, try to lock the mutex. + match self.futex.compare_exchange(UNLOCKED, owned_state, Acquire, Relaxed) { + Ok(_) => return, + Err(updated) => state = updated, + } + } + } + + #[inline] + pub unsafe fn unlock(&self) { + if is_contested(self.futex.swap(UNLOCKED, Release)) { + // The woken thread will mark the mutex as contested again, + // and return here, waking until there are no waiters left, + // in which case this is a noop. + self.wake(); + } + } + + #[cold] + fn wake(&self) { + unsafe { + zx_futex_wake_single_owner(&self.futex); + } + } +} diff --git a/library/std/src/sys/unix/locks/futex_condvar.rs b/library/std/src/sys/unix/locks/futex_condvar.rs new file mode 100644 index 0000000000000..c0576c17880e1 --- /dev/null +++ b/library/std/src/sys/unix/locks/futex_condvar.rs @@ -0,0 +1,58 @@ +use super::Mutex; +use crate::sync::atomic::{AtomicU32, Ordering::Relaxed}; +use crate::sys::futex::{futex_wait, futex_wake, futex_wake_all}; +use crate::time::Duration; + +pub type MovableCondvar = Condvar; + +pub struct Condvar { + // The value of this atomic is simply incremented on every notification. + // This is used by `.wait()` to not miss any notifications after + // unlocking the mutex and before waiting for notifications. + futex: AtomicU32, +} + +impl Condvar { + #[inline] + pub const fn new() -> Self { + Self { futex: AtomicU32::new(0) } + } + + // All the memory orderings here are `Relaxed`, + // because synchronization is done by unlocking and locking the mutex. + + pub unsafe fn notify_one(&self) { + self.futex.fetch_add(1, Relaxed); + futex_wake(&self.futex); + } + + pub unsafe fn notify_all(&self) { + self.futex.fetch_add(1, Relaxed); + futex_wake_all(&self.futex); + } + + pub unsafe fn wait(&self, mutex: &Mutex) { + self.wait_optional_timeout(mutex, None); + } + + pub unsafe fn wait_timeout(&self, mutex: &Mutex, timeout: Duration) -> bool { + self.wait_optional_timeout(mutex, Some(timeout)) + } + + unsafe fn wait_optional_timeout(&self, mutex: &Mutex, timeout: Option) -> bool { + // Examine the notification counter _before_ we unlock the mutex. + let futex_value = self.futex.load(Relaxed); + + // Unlock the mutex before going to sleep. + mutex.unlock(); + + // Wait, but only if there hasn't been any + // notification since we unlocked the mutex. + let r = futex_wait(&self.futex, futex_value, timeout); + + // Lock the mutex again. + mutex.lock(); + + r + } +} diff --git a/library/std/src/sys/unix/locks/futex.rs b/library/std/src/sys/unix/locks/futex_mutex.rs similarity index 62% rename from library/std/src/sys/unix/locks/futex.rs rename to library/std/src/sys/unix/locks/futex_mutex.rs index a9a1a32c5afb0..99ba86e5f996d 100644 --- a/library/std/src/sys/unix/locks/futex.rs +++ b/library/std/src/sys/unix/locks/futex_mutex.rs @@ -2,11 +2,9 @@ use crate::sync::atomic::{ AtomicU32, Ordering::{Acquire, Relaxed, Release}, }; -use crate::sys::futex::{futex_wait, futex_wake, futex_wake_all}; -use crate::time::Duration; +use crate::sys::futex::{futex_wait, futex_wake}; pub type MovableMutex = Mutex; -pub type MovableCondvar = Condvar; pub struct Mutex { /// 0: unlocked @@ -101,55 +99,3 @@ impl Mutex { futex_wake(&self.futex); } } - -pub struct Condvar { - // The value of this atomic is simply incremented on every notification. - // This is used by `.wait()` to not miss any notifications after - // unlocking the mutex and before waiting for notifications. - futex: AtomicU32, -} - -impl Condvar { - #[inline] - pub const fn new() -> Self { - Self { futex: AtomicU32::new(0) } - } - - // All the memory orderings here are `Relaxed`, - // because synchronization is done by unlocking and locking the mutex. - - pub unsafe fn notify_one(&self) { - self.futex.fetch_add(1, Relaxed); - futex_wake(&self.futex); - } - - pub unsafe fn notify_all(&self) { - self.futex.fetch_add(1, Relaxed); - futex_wake_all(&self.futex); - } - - pub unsafe fn wait(&self, mutex: &Mutex) { - self.wait_optional_timeout(mutex, None); - } - - pub unsafe fn wait_timeout(&self, mutex: &Mutex, timeout: Duration) -> bool { - self.wait_optional_timeout(mutex, Some(timeout)) - } - - unsafe fn wait_optional_timeout(&self, mutex: &Mutex, timeout: Option) -> bool { - // Examine the notification counter _before_ we unlock the mutex. - let futex_value = self.futex.load(Relaxed); - - // Unlock the mutex before going to sleep. - mutex.unlock(); - - // Wait, but only if there hasn't been any - // notification since we unlocked the mutex. - let r = futex_wait(&self.futex, futex_value, timeout); - - // Lock the mutex again. - mutex.lock(); - - r - } -} diff --git a/library/std/src/sys/unix/locks/mod.rs b/library/std/src/sys/unix/locks/mod.rs index 03400efa3c9aa..f5f92f6935830 100644 --- a/library/std/src/sys/unix/locks/mod.rs +++ b/library/std/src/sys/unix/locks/mod.rs @@ -7,10 +7,19 @@ cfg_if::cfg_if! { target_os = "openbsd", target_os = "dragonfly", ))] { - mod futex; + mod futex_mutex; mod futex_rwlock; - pub(crate) use futex::{Mutex, MovableMutex, MovableCondvar}; + mod futex_condvar; + pub(crate) use futex_mutex::{Mutex, MovableMutex}; pub(crate) use futex_rwlock::{RwLock, MovableRwLock}; + pub(crate) use futex_condvar::MovableCondvar; + } else if #[cfg(target_os = "fuchsia")] { + mod fuchsia_mutex; + mod futex_rwlock; + mod futex_condvar; + pub(crate) use fuchsia_mutex::{Mutex, MovableMutex}; + pub(crate) use futex_rwlock::{RwLock, MovableRwLock}; + pub(crate) use futex_condvar::MovableCondvar; } else { mod pthread_mutex; mod pthread_rwlock; From 0d91b08970301ae586031b1b2437a44115074efc Mon Sep 17 00:00:00 2001 From: joboet Date: Tue, 12 Jul 2022 12:25:43 +0200 Subject: [PATCH 05/14] std: fix issue with perma-locked mutexes on Fuchsia --- library/std/src/sys/unix/futex.rs | 4 +++ .../std/src/sys/unix/locks/fuchsia_mutex.rs | 25 ++++++++++++------- 2 files changed, 20 insertions(+), 9 deletions(-) diff --git a/library/std/src/sys/unix/futex.rs b/library/std/src/sys/unix/futex.rs index 9480451fc5c86..96b07b510a777 100644 --- a/library/std/src/sys/unix/futex.rs +++ b/library/std/src/sys/unix/futex.rs @@ -251,6 +251,9 @@ pub mod zircon { pub const ZX_TIME_INFINITE: zx_time_t = zx_time_t::MAX; pub const ZX_OK: zx_status_t = 0; + pub const ZX_ERR_INVALID_ARGS: zx_status_t = -10; + pub const ZX_ERR_BAD_HANDLE: zx_status_t = -11; + pub const ZX_ERR_WRONG_TYPE: zx_status_t = -12; pub const ZX_ERR_BAD_STATE: zx_status_t = -20; pub const ZX_ERR_TIMED_OUT: zx_status_t = -21; @@ -264,6 +267,7 @@ pub mod zircon { ) -> zx_status_t; pub fn zx_futex_wake(value_ptr: *const zx_futex_t, wake_count: u32) -> zx_status_t; pub fn zx_futex_wake_single_owner(value_ptr: *const zx_futex_t) -> zx_status_t; + pub fn zx_nanosleep(deadline: zx_time_t) -> zx_status_t; pub fn zx_thread_self() -> zx_handle_t; } } diff --git a/library/std/src/sys/unix/locks/fuchsia_mutex.rs b/library/std/src/sys/unix/locks/fuchsia_mutex.rs index 412e7e0018b48..65d7c4eefd99a 100644 --- a/library/std/src/sys/unix/locks/fuchsia_mutex.rs +++ b/library/std/src/sys/unix/locks/fuchsia_mutex.rs @@ -42,8 +42,9 @@ use crate::sync::atomic::{ Ordering::{Acquire, Relaxed, Release}, }; use crate::sys::futex::zircon::{ - zx_futex_wait, zx_futex_wake_single_owner, zx_handle_t, zx_thread_self, ZX_ERR_BAD_STATE, - ZX_OK, ZX_TIME_INFINITE, + zx_futex_wait, zx_futex_wake_single_owner, zx_handle_t, zx_nanosleep, zx_thread_self, + ZX_ERR_BAD_HANDLE, ZX_ERR_BAD_STATE, ZX_ERR_INVALID_ARGS, ZX_ERR_TIMED_OUT, ZX_ERR_WRONG_TYPE, + ZX_OK, ZX_TIME_INFINITE, ZX_TIME_INFINITE, }; // The lowest two bits of a `zx_handle_t` are always set, so the lowest bit is used to mark the @@ -120,13 +121,19 @@ impl Mutex { to_owner(state), ZX_TIME_INFINITE, ) { - ZX_OK | ZX_ERR_BAD_STATE => (), - // Deadlock even in the case of reentrant locking, as leaking a guard - // could lead to the same condition if the thread id is reused, but - // panicking is not expected in that situation. This makes things - // quite a bit harder to debug, but encourages portable programming. - _ if to_owner(state) == thread_self => loop {}, - error => panic!("futex operation failed with error code {error}"), + ZX_OK | ZX_ERR_BAD_STATE | ZX_ERR_TIMED_OUT => (), + // Either the current thread is trying to lock a mutex it has already locked, + // or the previous owner did not unlock the mutex before exiting. Since it is + // not possible to reliably detect which is the case, the current thread is + // deadlocked. This makes debugging these cases quite a bit harder, but encourages + // portable programming, since all other platforms do the same. + // + // Note that if the thread handle is reused, an arbitrary thread's priority could + // be boosted by the wait, but there is currently no way to prevent that. + ZX_ERR_INVALID_ARGS | ZX_ERR_BAD_HANDLE | ZX_ERR_WRONG_TYPE => loop { + zx_nanosleep(ZX_TIME_INFINITE); + }, + error => unreachable!("unexpected error code in futex wait: {error}"), } } } From f3579268372723bc4ff7b76090c090aa7b9e6a3a Mon Sep 17 00:00:00 2001 From: joboet Date: Mon, 18 Jul 2022 10:56:10 +0200 Subject: [PATCH 06/14] std: panic instead of deadlocking in mutex implementation on Fuchsia --- library/std/src/sys/unix/futex.rs | 1 - .../std/src/sys/unix/locks/fuchsia_mutex.rs | 30 +++++++++---------- 2 files changed, 15 insertions(+), 16 deletions(-) diff --git a/library/std/src/sys/unix/futex.rs b/library/std/src/sys/unix/futex.rs index 96b07b510a777..8d5b540212a17 100644 --- a/library/std/src/sys/unix/futex.rs +++ b/library/std/src/sys/unix/futex.rs @@ -267,7 +267,6 @@ pub mod zircon { ) -> zx_status_t; pub fn zx_futex_wake(value_ptr: *const zx_futex_t, wake_count: u32) -> zx_status_t; pub fn zx_futex_wake_single_owner(value_ptr: *const zx_futex_t) -> zx_status_t; - pub fn zx_nanosleep(deadline: zx_time_t) -> zx_status_t; pub fn zx_thread_self() -> zx_handle_t; } } diff --git a/library/std/src/sys/unix/locks/fuchsia_mutex.rs b/library/std/src/sys/unix/locks/fuchsia_mutex.rs index 65d7c4eefd99a..7372406b32fac 100644 --- a/library/std/src/sys/unix/locks/fuchsia_mutex.rs +++ b/library/std/src/sys/unix/locks/fuchsia_mutex.rs @@ -42,9 +42,9 @@ use crate::sync::atomic::{ Ordering::{Acquire, Relaxed, Release}, }; use crate::sys::futex::zircon::{ - zx_futex_wait, zx_futex_wake_single_owner, zx_handle_t, zx_nanosleep, zx_thread_self, - ZX_ERR_BAD_HANDLE, ZX_ERR_BAD_STATE, ZX_ERR_INVALID_ARGS, ZX_ERR_TIMED_OUT, ZX_ERR_WRONG_TYPE, - ZX_OK, ZX_TIME_INFINITE, ZX_TIME_INFINITE, + zx_futex_wait, zx_futex_wake_single_owner, zx_handle_t, zx_thread_self, ZX_ERR_BAD_HANDLE, + ZX_ERR_BAD_STATE, ZX_ERR_INVALID_ARGS, ZX_ERR_TIMED_OUT, ZX_ERR_WRONG_TYPE, ZX_OK, + ZX_TIME_INFINITE, ZX_TIME_INFINITE, }; // The lowest two bits of a `zx_handle_t` are always set, so the lowest bit is used to mark the @@ -122,18 +122,18 @@ impl Mutex { ZX_TIME_INFINITE, ) { ZX_OK | ZX_ERR_BAD_STATE | ZX_ERR_TIMED_OUT => (), - // Either the current thread is trying to lock a mutex it has already locked, - // or the previous owner did not unlock the mutex before exiting. Since it is - // not possible to reliably detect which is the case, the current thread is - // deadlocked. This makes debugging these cases quite a bit harder, but encourages - // portable programming, since all other platforms do the same. - // - // Note that if the thread handle is reused, an arbitrary thread's priority could - // be boosted by the wait, but there is currently no way to prevent that. - ZX_ERR_INVALID_ARGS | ZX_ERR_BAD_HANDLE | ZX_ERR_WRONG_TYPE => loop { - zx_nanosleep(ZX_TIME_INFINITE); - }, - error => unreachable!("unexpected error code in futex wait: {error}"), + // Note that if a thread handle is reused after its associated thread + // exits without unlocking the mutex, an arbitrary thread's priority + // could be boosted by the wait, but there is currently no way to + // prevent that. + ZX_ERR_INVALID_ARGS | ZX_ERR_BAD_HANDLE | ZX_ERR_WRONG_TYPE => { + panic!( + "either the current thread is trying to lock a mutex it has + already locked, or the previous uowner did not unlock the mutex + before exiting" + ) + } + error => panic!("unexpected error in zx_futex_wait: {error}"), } } } From 110fdb642a2fe2f680a41b47611145bc6ffdee5e Mon Sep 17 00:00:00 2001 From: Frank Steffahn Date: Mon, 18 Jul 2022 12:31:34 +0200 Subject: [PATCH 07/14] Add `PhantomData` marker for dropck to `BTreeMap` closes #99408 --- library/alloc/src/collections/btree/map.rs | 30 ++++++++++++++++++--- src/test/ui/btreemap/btreemap_dropck.rs | 16 +++++++++++ src/test/ui/btreemap/btreemap_dropck.stderr | 13 +++++++++ src/test/ui/issues/issue-72554.rs | 5 +++- src/test/ui/issues/issue-72554.stderr | 19 +++++++++++-- 5 files changed, 76 insertions(+), 7 deletions(-) create mode 100644 src/test/ui/btreemap/btreemap_dropck.rs create mode 100644 src/test/ui/btreemap/btreemap_dropck.stderr diff --git a/library/alloc/src/collections/btree/map.rs b/library/alloc/src/collections/btree/map.rs index 0bddd7a990699..cacbd54b6c246 100644 --- a/library/alloc/src/collections/btree/map.rs +++ b/library/alloc/src/collections/btree/map.rs @@ -178,6 +178,8 @@ pub struct BTreeMap< length: usize, /// `ManuallyDrop` to control drop order (needs to be dropped after all the nodes). pub(super) alloc: ManuallyDrop, + // For dropck; the `Box` avoids making the `Unpin` impl more strict than before + _marker: PhantomData>, } #[stable(feature = "btree_drop", since = "1.7.0")] @@ -187,6 +189,19 @@ unsafe impl<#[may_dangle] K, #[may_dangle] V, A: Allocator + Clone> Drop for BTr } } +// FIXME: This implementation is "wrong", but changing it would be a breaking change. +// (The bounds of the automatic `UnwindSafe` implementation have been like this since Rust 1.50.) +// Maybe we can fix it nonetheless with a crater run, or if the `UnwindSafe` +// traits are deprecated, or disarmed (no longer causing hard errors) in the future. +#[stable(feature = "btree_unwindsafe", since = "1.64.0")] +impl core::panic::UnwindSafe for BTreeMap +where + A: core::panic::UnwindSafe, + K: core::panic::RefUnwindSafe, + V: core::panic::RefUnwindSafe, +{ +} + #[stable(feature = "rust1", since = "1.0.0")] impl Clone for BTreeMap { fn clone(&self) -> BTreeMap { @@ -204,6 +219,7 @@ impl Clone for BTreeMap { root: Some(Root::new(alloc.clone())), length: 0, alloc: ManuallyDrop::new(alloc), + _marker: PhantomData, }; { @@ -567,7 +583,7 @@ impl BTreeMap { #[rustc_const_unstable(feature = "const_btree_new", issue = "71835")] #[must_use] pub const fn new() -> BTreeMap { - BTreeMap { root: None, length: 0, alloc: ManuallyDrop::new(Global) } + BTreeMap { root: None, length: 0, alloc: ManuallyDrop::new(Global), _marker: PhantomData } } } @@ -593,6 +609,7 @@ impl BTreeMap { root: mem::replace(&mut self.root, None), length: mem::replace(&mut self.length, 0), alloc: self.alloc.clone(), + _marker: PhantomData, }); } @@ -615,7 +632,7 @@ impl BTreeMap { /// ``` #[unstable(feature = "btreemap_alloc", issue = "32838")] pub fn new_in(alloc: A) -> BTreeMap { - BTreeMap { root: None, length: 0, alloc: ManuallyDrop::new(alloc) } + BTreeMap { root: None, length: 0, alloc: ManuallyDrop::new(alloc), _marker: PhantomData } } } @@ -1320,7 +1337,12 @@ impl BTreeMap { let (new_left_len, right_len) = Root::calc_split_length(total_num, &left_root, &right_root); self.length = new_left_len; - BTreeMap { root: Some(right_root), length: right_len, alloc: self.alloc.clone() } + BTreeMap { + root: Some(right_root), + length: right_len, + alloc: self.alloc.clone(), + _marker: PhantomData, + } } /// Creates an iterator that visits all elements (key-value pairs) in @@ -1445,7 +1467,7 @@ impl BTreeMap { let mut root = Root::new(alloc.clone()); let mut length = 0; root.bulk_push(DedupSortedIter::new(iter.into_iter()), &mut length, alloc.clone()); - BTreeMap { root: Some(root), length, alloc: ManuallyDrop::new(alloc) } + BTreeMap { root: Some(root), length, alloc: ManuallyDrop::new(alloc), _marker: PhantomData } } } diff --git a/src/test/ui/btreemap/btreemap_dropck.rs b/src/test/ui/btreemap/btreemap_dropck.rs new file mode 100644 index 0000000000000..c58727df30cae --- /dev/null +++ b/src/test/ui/btreemap/btreemap_dropck.rs @@ -0,0 +1,16 @@ +struct PrintOnDrop<'a>(&'a str); + +impl Drop for PrintOnDrop<'_> { + fn drop(&mut self) { + println!("printint: {}", self.0); + } +} + +use std::collections::BTreeMap; +use std::iter::FromIterator; + +fn main() { + let s = String::from("Hello World!"); + let _map = BTreeMap::from_iter([((), PrintOnDrop(&s))]); + drop(s); //~ ERROR cannot move out of `s` because it is borrowed +} diff --git a/src/test/ui/btreemap/btreemap_dropck.stderr b/src/test/ui/btreemap/btreemap_dropck.stderr new file mode 100644 index 0000000000000..e953e7ae82bb8 --- /dev/null +++ b/src/test/ui/btreemap/btreemap_dropck.stderr @@ -0,0 +1,13 @@ +error[E0505]: cannot move out of `s` because it is borrowed + --> $DIR/btreemap_dropck.rs:15:10 + | +LL | let _map = BTreeMap::from_iter([((), PrintOnDrop(&s))]); + | -- borrow of `s` occurs here +LL | drop(s); + | ^ move out of `s` occurs here +LL | } + | - borrow might be used here, when `_map` is dropped and runs the `Drop` code for type `BTreeMap` + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0505`. diff --git a/src/test/ui/issues/issue-72554.rs b/src/test/ui/issues/issue-72554.rs index 47aca05d7786f..7287639c61dde 100644 --- a/src/test/ui/issues/issue-72554.rs +++ b/src/test/ui/issues/issue-72554.rs @@ -1,10 +1,13 @@ use std::collections::BTreeSet; #[derive(Hash)] -pub enum ElemDerived { //~ ERROR recursive type `ElemDerived` has infinite size +pub enum ElemDerived { + //~^ ERROR recursive type `ElemDerived` has infinite size + //~| ERROR cycle detected when computing drop-check constraints for `ElemDerived` A(ElemDerived) } + pub enum Elem { Derived(ElemDerived) } diff --git a/src/test/ui/issues/issue-72554.stderr b/src/test/ui/issues/issue-72554.stderr index a6e44be636a43..3e5adcae133ca 100644 --- a/src/test/ui/issues/issue-72554.stderr +++ b/src/test/ui/issues/issue-72554.stderr @@ -3,6 +3,7 @@ error[E0072]: recursive type `ElemDerived` has infinite size | LL | pub enum ElemDerived { | ^^^^^^^^^^^^^^^^^^^^ recursive type has infinite size +... LL | A(ElemDerived) | ----------- recursive without indirection | @@ -11,6 +12,20 @@ help: insert some indirection (e.g., a `Box`, `Rc`, or `&`) to make `ElemDerived LL | A(Box) | ++++ + -error: aborting due to previous error +error[E0391]: cycle detected when computing drop-check constraints for `ElemDerived` + --> $DIR/issue-72554.rs:4:1 + | +LL | pub enum ElemDerived { + | ^^^^^^^^^^^^^^^^^^^^ + | + = note: ...which immediately requires computing drop-check constraints for `ElemDerived` again +note: cycle used when computing drop-check constraints for `Elem` + --> $DIR/issue-72554.rs:11:1 + | +LL | pub enum Elem { + | ^^^^^^^^^^^^^ + +error: aborting due to 2 previous errors -For more information about this error, try `rustc --explain E0072`. +Some errors have detailed explanations: E0072, E0391. +For more information about an error, try `rustc --explain E0072`. From c72a77e093556e0f1e600686adb8de4419fff4c9 Mon Sep 17 00:00:00 2001 From: joboet Date: Wed, 20 Jul 2022 16:11:31 +0200 Subject: [PATCH 08/14] owner is not micro (correct typo) --- library/std/src/sys/unix/locks/fuchsia_mutex.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/std/src/sys/unix/locks/fuchsia_mutex.rs b/library/std/src/sys/unix/locks/fuchsia_mutex.rs index 7372406b32fac..fd3e9d1768b4c 100644 --- a/library/std/src/sys/unix/locks/fuchsia_mutex.rs +++ b/library/std/src/sys/unix/locks/fuchsia_mutex.rs @@ -129,7 +129,7 @@ impl Mutex { ZX_ERR_INVALID_ARGS | ZX_ERR_BAD_HANDLE | ZX_ERR_WRONG_TYPE => { panic!( "either the current thread is trying to lock a mutex it has - already locked, or the previous uowner did not unlock the mutex + already locked, or the previous owner did not unlock the mutex before exiting" ) } From bd0474d24ac6438018f02afbc66b576845c44169 Mon Sep 17 00:00:00 2001 From: Josh Stone Date: Wed, 20 Jul 2022 12:09:49 -0700 Subject: [PATCH 09/14] Fix the stable version of `AsFd for Arc` and `Box` These merged in #97437 for 1.64.0, apart from the main `io_safety` feature that stabilized in 1.63.0. --- library/std/src/os/fd/owned.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/library/std/src/os/fd/owned.rs b/library/std/src/os/fd/owned.rs index d661a13edc5e5..a463bc41db7aa 100644 --- a/library/std/src/os/fd/owned.rs +++ b/library/std/src/os/fd/owned.rs @@ -356,7 +356,7 @@ impl From for crate::net::UdpSocket { } } -#[stable(feature = "io_safety", since = "1.63.0")] +#[stable(feature = "asfd_ptrs", since = "1.64.0")] /// This impl allows implementing traits that require `AsFd` on Arc. /// ``` /// # #[cfg(any(unix, target_os = "wasi"))] mod group_cfg { @@ -379,7 +379,7 @@ impl AsFd for crate::sync::Arc { } } -#[stable(feature = "io_safety", since = "1.63.0")] +#[stable(feature = "asfd_ptrs", since = "1.64.0")] impl AsFd for Box { #[inline] fn as_fd(&self) -> BorrowedFd<'_> { From cd3204d1a272dc4739a5f86546620287941aa6e7 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Wed, 20 Jul 2022 03:05:14 +0000 Subject: [PATCH 10/14] Normalize the arg spans to be within the call span --- .../rustc_typeck/src/check/fn_ctxt/checks.rs | 26 +++++++++++++------ src/test/ui/inference/deref-suggestion.rs | 3 ++- src/test/ui/inference/deref-suggestion.stderr | 24 +++++++---------- 3 files changed, 30 insertions(+), 23 deletions(-) diff --git a/compiler/rustc_typeck/src/check/fn_ctxt/checks.rs b/compiler/rustc_typeck/src/check/fn_ctxt/checks.rs index e3db70845ddf1..64f931aca95f4 100644 --- a/compiler/rustc_typeck/src/check/fn_ctxt/checks.rs +++ b/compiler/rustc_typeck/src/check/fn_ctxt/checks.rs @@ -481,6 +481,17 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { self.set_tainted_by_errors(); let tcx = self.tcx; + // Get the argument span in the context of the call span so that + // suggestions and labels are (more) correct when an arg is a + // macro invocation. + let normalize_span = |span: Span| -> Span { + let normalized_span = span.find_ancestor_inside(error_span).unwrap_or(span); + // Sometimes macros mess up the spans, so do not normalize the + // arg span to equal the error span, because that's less useful + // than pointing out the arg expr in the wrong context. + if normalized_span.source_equal(error_span) { span } else { normalized_span } + }; + // Precompute the provided types and spans, since that's all we typically need for below let provided_arg_tys: IndexVec, Span)> = provided_args .iter() @@ -490,7 +501,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { .borrow() .expr_ty_adjusted_opt(*expr) .unwrap_or_else(|| tcx.ty_error()); - (self.resolve_vars_if_possible(ty), expr.span) + (self.resolve_vars_if_possible(ty), normalize_span(expr.span)) }) .collect(); let callee_expr = match &call_expr.peel_blocks().kind { @@ -600,11 +611,10 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { // Take some care with spans, so we don't suggest wrapping a macro's // innards in parenthesis, for example. if satisfied - && let Some(lo) = - provided_args[mismatch_idx.into()].span.find_ancestor_inside(error_span) - && let Some(hi) = provided_args[(mismatch_idx + tys.len() - 1).into()] - .span - .find_ancestor_inside(error_span) + && let Some((_, lo)) = + provided_arg_tys.get(ProvidedIdx::from_usize(mismatch_idx)) + && let Some((_, hi)) = + provided_arg_tys.get(ProvidedIdx::from_usize(mismatch_idx + tys.len() - 1)) { let mut err; if tys.len() == 1 { @@ -612,7 +622,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { // so don't do anything special here. err = self.report_and_explain_type_error( TypeTrace::types( - &self.misc(lo), + &self.misc(*lo), true, formal_and_expected_inputs[mismatch_idx.into()].1, provided_arg_tys[mismatch_idx.into()].0, @@ -1052,7 +1062,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { let suggestion_text = if let Some(provided_idx) = provided_idx && let (_, provided_span) = provided_arg_tys[*provided_idx] && let Ok(arg_text) = - source_map.span_to_snippet(provided_span.source_callsite()) + source_map.span_to_snippet(provided_span) { arg_text } else { diff --git a/src/test/ui/inference/deref-suggestion.rs b/src/test/ui/inference/deref-suggestion.rs index 4fd695585ba06..0d8e7289dc8a2 100644 --- a/src/test/ui/inference/deref-suggestion.rs +++ b/src/test/ui/inference/deref-suggestion.rs @@ -1,5 +1,5 @@ macro_rules! borrow { - ($x:expr) => { &$x } //~ ERROR mismatched types + ($x:expr) => { &$x } } fn foo(_: String) {} @@ -32,6 +32,7 @@ fn main() { foo(&mut "aaa".to_owned()); //~^ ERROR mismatched types foo3(borrow!(0)); + //~^ ERROR mismatched types foo4(&0); assert_eq!(3i32, &3i32); //~^ ERROR mismatched types diff --git a/src/test/ui/inference/deref-suggestion.stderr b/src/test/ui/inference/deref-suggestion.stderr index e763e17e51786..d729f2d682a61 100644 --- a/src/test/ui/inference/deref-suggestion.stderr +++ b/src/test/ui/inference/deref-suggestion.stderr @@ -70,13 +70,10 @@ LL + foo("aaa".to_owned()); | error[E0308]: mismatched types - --> $DIR/deref-suggestion.rs:2:20 + --> $DIR/deref-suggestion.rs:34:10 | -LL | ($x:expr) => { &$x } - | ^^^ expected `u32`, found `&{integer}` -... LL | foo3(borrow!(0)); - | ---- ---------- in this macro invocation + | ---- ^^^^^^^^^^ expected `u32`, found `&{integer}` | | | arguments to this function are incorrect | @@ -85,10 +82,9 @@ note: function defined here | LL | fn foo3(_: u32) {} | ^^^^ ------ - = note: this error originates in the macro `borrow` (in Nightly builds, run with -Z macro-backtrace for more info) error[E0308]: mismatched types - --> $DIR/deref-suggestion.rs:36:5 + --> $DIR/deref-suggestion.rs:37:5 | LL | assert_eq!(3i32, &3i32); | ^^^^^^^^^^^^^^^^^^^^^^^ expected `i32`, found `&i32` @@ -96,7 +92,7 @@ LL | assert_eq!(3i32, &3i32); = note: this error originates in the macro `assert_eq` (in Nightly builds, run with -Z macro-backtrace for more info) error[E0308]: mismatched types - --> $DIR/deref-suggestion.rs:39:17 + --> $DIR/deref-suggestion.rs:40:17 | LL | let s = S { u }; | ^ @@ -105,7 +101,7 @@ LL | let s = S { u }; | help: consider borrowing here: `u: &u` error[E0308]: mismatched types - --> $DIR/deref-suggestion.rs:41:20 + --> $DIR/deref-suggestion.rs:42:20 | LL | let s = S { u: u }; | ^ @@ -114,7 +110,7 @@ LL | let s = S { u: u }; | help: consider borrowing here: `&u` error[E0308]: mismatched types - --> $DIR/deref-suggestion.rs:44:17 + --> $DIR/deref-suggestion.rs:45:17 | LL | let r = R { i }; | ^ expected `u32`, found `&{integer}` @@ -125,7 +121,7 @@ LL | let r = R { i: *i }; | ++++ error[E0308]: mismatched types - --> $DIR/deref-suggestion.rs:46:20 + --> $DIR/deref-suggestion.rs:47:20 | LL | let r = R { i: i }; | ^ expected `u32`, found `&{integer}` @@ -136,7 +132,7 @@ LL | let r = R { i: *i }; | + error[E0308]: mismatched types - --> $DIR/deref-suggestion.rs:55:9 + --> $DIR/deref-suggestion.rs:56:9 | LL | b | ^ expected `i32`, found `&{integer}` @@ -147,7 +143,7 @@ LL | *b | + error[E0308]: mismatched types - --> $DIR/deref-suggestion.rs:63:9 + --> $DIR/deref-suggestion.rs:64:9 | LL | b | ^ expected `i32`, found `&{integer}` @@ -158,7 +154,7 @@ LL | *b | + error[E0308]: `if` and `else` have incompatible types - --> $DIR/deref-suggestion.rs:68:12 + --> $DIR/deref-suggestion.rs:69:12 | LL | let val = if true { | _______________- From 52491834804799c2b6fe9b4f587a53437c390298 Mon Sep 17 00:00:00 2001 From: Yuki Okushi Date: Thu, 21 Jul 2022 17:08:41 +0900 Subject: [PATCH 11/14] Add regression test for #52304 Signed-off-by: Yuki Okushi --- src/test/ui/generator/issue-52304.rs | 11 +++++++++++ 1 file changed, 11 insertions(+) create mode 100644 src/test/ui/generator/issue-52304.rs diff --git a/src/test/ui/generator/issue-52304.rs b/src/test/ui/generator/issue-52304.rs new file mode 100644 index 0000000000000..3e9de765b12b2 --- /dev/null +++ b/src/test/ui/generator/issue-52304.rs @@ -0,0 +1,11 @@ +// check-pass + +#![feature(generators, generator_trait)] + +use std::ops::Generator; + +pub fn example() -> impl Generator { + || yield &1 +} + +fn main() {} From 7d0a18239e72fe170818d1cf5ebea8def3830364 Mon Sep 17 00:00:00 2001 From: lcnr Date: Thu, 21 Jul 2022 10:53:54 +0200 Subject: [PATCH 12/14] orphan check: opaque types are an error --- .../src/traits/coherence.rs | 30 +------------------ .../ui/impl-trait/negative-reasoning.stderr | 2 +- 2 files changed, 2 insertions(+), 30 deletions(-) diff --git a/compiler/rustc_trait_selection/src/traits/coherence.rs b/compiler/rustc_trait_selection/src/traits/coherence.rs index 52ca23c4b303e..67ae26b0b3ad1 100644 --- a/compiler/rustc_trait_selection/src/traits/coherence.rs +++ b/compiler/rustc_trait_selection/src/traits/coherence.rs @@ -739,34 +739,6 @@ fn ty_is_local_constructor(tcx: TyCtxt<'_>, ty: Ty<'_>, in_crate: InCrate) -> bo ty::Adt(def, _) => def_id_is_local(def.did(), in_crate), ty::Foreign(did) => def_id_is_local(did, in_crate), - ty::Opaque(..) => { - // This merits some explanation. - // Normally, opaque types are not involved when performing - // coherence checking, since it is illegal to directly - // implement a trait on an opaque type. However, we might - // end up looking at an opaque type during coherence checking - // if an opaque type gets used within another type (e.g. as - // a type parameter). This requires us to decide whether or - // not an opaque type should be considered 'local' or not. - // - // We choose to treat all opaque types as non-local, even - // those that appear within the same crate. This seems - // somewhat surprising at first, but makes sense when - // you consider that opaque types are supposed to hide - // the underlying type *within the same crate*. When an - // opaque type is used from outside the module - // where it is declared, it should be impossible to observe - // anything about it other than the traits that it implements. - // - // The alternative would be to look at the underlying type - // to determine whether or not the opaque type itself should - // be considered local. However, this could make it a breaking change - // to switch the underlying ('defining') type from a local type - // to a remote type. This would violate the rule that opaque - // types should be completely opaque apart from the traits - // that they implement, so we don't use this behavior. - false - } ty::Dynamic(ref tt, ..) => { if let Some(principal) = tt.principal() { @@ -786,7 +758,7 @@ fn ty_is_local_constructor(tcx: TyCtxt<'_>, ty: Ty<'_>, in_crate: InCrate) -> bo // // See `test/ui/coherence/coherence-with-closure.rs` for an example where this // could happens. - ty::Closure(..) | ty::Generator(..) | ty::GeneratorWitness(..) => { + ty::Opaque(..) | ty::Closure(..) | ty::Generator(..) | ty::GeneratorWitness(..) => { tcx.sess.delay_span_bug( DUMMY_SP, format!("ty_is_local invoked on closure or generator: {:?}", ty), diff --git a/src/test/ui/impl-trait/negative-reasoning.stderr b/src/test/ui/impl-trait/negative-reasoning.stderr index 479b451855d55..2eea726a19c5a 100644 --- a/src/test/ui/impl-trait/negative-reasoning.stderr +++ b/src/test/ui/impl-trait/negative-reasoning.stderr @@ -7,7 +7,7 @@ LL | impl AnotherTrait for T {} LL | impl AnotherTrait for D { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ conflicting implementation for `D` | - = note: upstream crates may add a new impl of trait `std::fmt::Debug` for type `OpaqueType` in future versions + = note: downstream crates may implement trait `std::fmt::Debug` for type `OpaqueType` error: cannot implement trait on type alias impl trait --> $DIR/negative-reasoning.rs:19:25 From 84c3fcd2a0285c06a682c9b064640084e4c7271b Mon Sep 17 00:00:00 2001 From: lcnr Date: Thu, 21 Jul 2022 11:51:09 +0200 Subject: [PATCH 13/14] rewrite the orphan check to use a type visitor --- .../src/traits/coherence.rs | 293 ++++++++---------- 1 file changed, 124 insertions(+), 169 deletions(-) diff --git a/compiler/rustc_trait_selection/src/traits/coherence.rs b/compiler/rustc_trait_selection/src/traits/coherence.rs index 67ae26b0b3ad1..9983438233e1e 100644 --- a/compiler/rustc_trait_selection/src/traits/coherence.rs +++ b/compiler/rustc_trait_selection/src/traits/coherence.rs @@ -22,11 +22,12 @@ use rustc_middle::traits::specialization_graph::OverlapMode; use rustc_middle::ty::fast_reject::{DeepRejectCtxt, TreatParams}; use rustc_middle::ty::subst::Subst; use rustc_middle::ty::visit::TypeVisitable; -use rustc_middle::ty::{self, ImplSubject, Ty, TyCtxt}; +use rustc_middle::ty::{self, ImplSubject, Ty, TyCtxt, TypeVisitor}; use rustc_span::symbol::sym; use rustc_span::DUMMY_SP; use std::fmt::Debug; use std::iter; +use std::ops::ControlFlow; /// Whether we do the orphan check relative to this crate or /// to some remote crate. @@ -578,192 +579,146 @@ fn orphan_check_trait_ref<'tcx>( ); } - // Given impl Trait for T0, an impl is valid only - // if at least one of the following is true: - // - // - Trait is a local trait - // (already checked in orphan_check prior to calling this function) - // - All of - // - At least one of the types T0..=Tn must be a local type. - // Let Ti be the first such type. - // - No uncovered type parameters P1..=Pn may appear in T0..Ti (excluding Ti) - // - fn uncover_fundamental_ty<'tcx>( - tcx: TyCtxt<'tcx>, - ty: Ty<'tcx>, - in_crate: InCrate, - ) -> Vec> { - // FIXME: this is currently somewhat overly complicated, - // but fixing this requires a more complicated refactor. - if !contained_non_local_types(tcx, ty, in_crate).is_empty() { - if let Some(inner_tys) = fundamental_ty_inner_tys(tcx, ty) { - return inner_tys - .flat_map(|ty| uncover_fundamental_ty(tcx, ty, in_crate)) - .collect(); + let mut checker = OrphanChecker::new(tcx, in_crate); + match trait_ref.visit_with(&mut checker) { + ControlFlow::Continue(()) => Err(OrphanCheckErr::NonLocalInputType(checker.non_local_tys)), + ControlFlow::Break(OrphanCheckEarlyExit::ParamTy(ty)) => { + // Does there exist some local type after the `ParamTy`. + checker.search_first_local_ty = true; + if let Some(OrphanCheckEarlyExit::LocalTy(local_ty)) = + trait_ref.visit_with(&mut checker).break_value() + { + Err(OrphanCheckErr::UncoveredTy(ty, Some(local_ty))) + } else { + Err(OrphanCheckErr::UncoveredTy(ty, None)) } } - - vec![ty] - } - - let mut non_local_spans = vec![]; - for (i, input_ty) in trait_ref - .substs - .types() - .flat_map(|ty| uncover_fundamental_ty(tcx, ty, in_crate)) - .enumerate() - { - debug!("orphan_check_trait_ref: check ty `{:?}`", input_ty); - let non_local_tys = contained_non_local_types(tcx, input_ty, in_crate); - if non_local_tys.is_empty() { - debug!("orphan_check_trait_ref: ty_is_local `{:?}`", input_ty); - return Ok(()); - } else if let ty::Param(_) = input_ty.kind() { - debug!("orphan_check_trait_ref: uncovered ty: `{:?}`", input_ty); - let local_type = trait_ref - .substs - .types() - .flat_map(|ty| uncover_fundamental_ty(tcx, ty, in_crate)) - .find(|&ty| ty_is_local_constructor(tcx, ty, in_crate)); - - debug!("orphan_check_trait_ref: uncovered ty local_type: `{:?}`", local_type); - - return Err(OrphanCheckErr::UncoveredTy(input_ty, local_type)); - } - - non_local_spans.extend(non_local_tys.into_iter().map(|input_ty| (input_ty, i == 0))); + ControlFlow::Break(OrphanCheckEarlyExit::LocalTy(_)) => Ok(()), } - // If we exit above loop, never found a local type. - debug!("orphan_check_trait_ref: no local type"); - Err(OrphanCheckErr::NonLocalInputType(non_local_spans)) } -/// Returns a list of relevant non-local types for `ty`. -/// -/// This is just `ty` itself unless `ty` is `#[fundamental]`, -/// in which case we recursively look into this type. -/// -/// If `ty` is local itself, this method returns an empty `Vec`. -/// -/// # Examples -/// -/// - `u32` is not local, so this returns `[u32]`. -/// - for `Foo`, where `Foo` is a local type, this returns `[]`. -/// - `&mut u32` returns `[u32]`, as `&mut` is a fundamental type, similar to `Box`. -/// - `Box>` returns `[]`, as `Box` is a fundamental type and `Foo` is local. -fn contained_non_local_types<'tcx>( +struct OrphanChecker<'tcx> { tcx: TyCtxt<'tcx>, - ty: Ty<'tcx>, in_crate: InCrate, -) -> Vec> { - if ty_is_local_constructor(tcx, ty, in_crate) { - Vec::new() - } else { - match fundamental_ty_inner_tys(tcx, ty) { - Some(inner_tys) => { - inner_tys.flat_map(|ty| contained_non_local_types(tcx, ty, in_crate)).collect() - } - None => vec![ty], + in_self_ty: bool, + /// Ignore orphan check failures and exclusively search for the first + /// local type. + search_first_local_ty: bool, + non_local_tys: Vec<(Ty<'tcx>, bool)>, +} + +impl<'tcx> OrphanChecker<'tcx> { + fn new(tcx: TyCtxt<'tcx>, in_crate: InCrate) -> Self { + OrphanChecker { + tcx, + in_crate, + in_self_ty: true, + search_first_local_ty: false, + non_local_tys: Vec::new(), } } -} -/// For `#[fundamental]` ADTs and `&T` / `&mut T`, returns `Some` with the -/// type parameters of the ADT, or `T`, respectively. For non-fundamental -/// types, returns `None`. -fn fundamental_ty_inner_tys<'tcx>( - tcx: TyCtxt<'tcx>, - ty: Ty<'tcx>, -) -> Option>> { - let (first_ty, rest_tys) = match *ty.kind() { - ty::Ref(_, ty, _) => (ty, ty::subst::InternalSubsts::empty().types()), - ty::Adt(def, substs) if def.is_fundamental() => { - let mut types = substs.types(); - - // FIXME(eddyb) actually validate `#[fundamental]` up-front. - match types.next() { - None => { - tcx.sess.span_err( - tcx.def_span(def.did()), - "`#[fundamental]` requires at least one type parameter", - ); - - return None; - } + fn found_non_local_ty(&mut self, t: Ty<'tcx>) -> ControlFlow> { + self.non_local_tys.push((t, self.in_self_ty)); + ControlFlow::CONTINUE + } - Some(first_ty) => (first_ty, types), - } + fn found_param_ty(&mut self, t: Ty<'tcx>) -> ControlFlow> { + if self.search_first_local_ty { + ControlFlow::CONTINUE + } else { + ControlFlow::Break(OrphanCheckEarlyExit::ParamTy(t)) } - _ => return None, - }; + } + + fn def_id_is_local(&mut self, def_id: DefId) -> bool { + match self.in_crate { + InCrate::Local => def_id.is_local(), + InCrate::Remote => false, + } + } +} - Some(iter::once(first_ty).chain(rest_tys)) +enum OrphanCheckEarlyExit<'tcx> { + ParamTy(Ty<'tcx>), + LocalTy(Ty<'tcx>), } -fn def_id_is_local(def_id: DefId, in_crate: InCrate) -> bool { - match in_crate { - // The type is local to *this* crate - it will not be - // local in any other crate. - InCrate::Remote => false, - InCrate::Local => def_id.is_local(), +impl<'tcx> TypeVisitor<'tcx> for OrphanChecker<'tcx> { + type BreakTy = OrphanCheckEarlyExit<'tcx>; + fn visit_region(&mut self, _r: ty::Region<'tcx>) -> ControlFlow { + ControlFlow::CONTINUE } -} -fn ty_is_local_constructor(tcx: TyCtxt<'_>, ty: Ty<'_>, in_crate: InCrate) -> bool { - debug!("ty_is_local_constructor({:?})", ty); - - match *ty.kind() { - ty::Bool - | ty::Char - | ty::Int(..) - | ty::Uint(..) - | ty::Float(..) - | ty::Str - | ty::FnDef(..) - | ty::FnPtr(_) - | ty::Array(..) - | ty::Slice(..) - | ty::RawPtr(..) - | ty::Ref(..) - | ty::Never - | ty::Tuple(..) - | ty::Param(..) - | ty::Projection(..) => false, - - ty::Placeholder(..) | ty::Bound(..) | ty::Infer(..) => match in_crate { - InCrate::Local => false, - // The inference variable might be unified with a local - // type in that remote crate. - InCrate::Remote => true, - }, - - ty::Adt(def, _) => def_id_is_local(def.did(), in_crate), - ty::Foreign(did) => def_id_is_local(did, in_crate), - - ty::Dynamic(ref tt, ..) => { - if let Some(principal) = tt.principal() { - def_id_is_local(principal.def_id(), in_crate) - } else { - false + fn visit_ty(&mut self, ty: Ty<'tcx>) -> ControlFlow { + let result = match *ty.kind() { + ty::Bool + | ty::Char + | ty::Int(..) + | ty::Uint(..) + | ty::Float(..) + | ty::Str + | ty::FnDef(..) + | ty::FnPtr(_) + | ty::Array(..) + | ty::Slice(..) + | ty::RawPtr(..) + | ty::Never + | ty::Tuple(..) + | ty::Projection(..) => self.found_non_local_ty(ty), + + ty::Param(..) => self.found_param_ty(ty), + + ty::Placeholder(..) | ty::Bound(..) | ty::Infer(..) => match self.in_crate { + InCrate::Local => self.found_non_local_ty(ty), + // The inference variable might be unified with a local + // type in that remote crate. + InCrate::Remote => ControlFlow::Break(OrphanCheckEarlyExit::LocalTy(ty)), + }, + + // For fundamental types, we just look inside of them. + ty::Ref(_, ty, _) => ty.visit_with(self), + ty::Adt(def, substs) => { + if self.def_id_is_local(def.did()) { + ControlFlow::Break(OrphanCheckEarlyExit::LocalTy(ty)) + } else if def.is_fundamental() { + substs.visit_with(self) + } else { + self.found_non_local_ty(ty) + } } - } + ty::Foreign(def_id) => { + if self.def_id_is_local(def_id) { + ControlFlow::Break(OrphanCheckEarlyExit::LocalTy(ty)) + } else { + self.found_non_local_ty(ty) + } + } + ty::Dynamic(tt, ..) => { + let principal = tt.principal().map(|p| p.def_id()); + if principal.map_or(false, |p| self.def_id_is_local(p)) { + ControlFlow::Break(OrphanCheckEarlyExit::LocalTy(ty)) + } else { + self.found_non_local_ty(ty) + } + } + ty::Error(_) => ControlFlow::Break(OrphanCheckEarlyExit::LocalTy(ty)), + ty::Opaque(..) | ty::Closure(..) | ty::Generator(..) | ty::GeneratorWitness(..) => { + self.tcx.sess.delay_span_bug( + DUMMY_SP, + format!("ty_is_local invoked on closure or generator: {:?}", ty), + ); + ControlFlow::Break(OrphanCheckEarlyExit::LocalTy(ty)) + } + }; + // A bit of a hack, the `OrphanChecker` is only used to visit a `TraitRef`, so + // the first type we visit is always the self type. + self.in_self_ty = false; + result + } - ty::Error(_) => true, - - // These variants should never appear during coherence checking because they - // cannot be named directly. - // - // They could be indirectly used through an opaque type. While using opaque types - // in impls causes an error, this path can still be hit afterwards. - // - // See `test/ui/coherence/coherence-with-closure.rs` for an example where this - // could happens. - ty::Opaque(..) | ty::Closure(..) | ty::Generator(..) | ty::GeneratorWitness(..) => { - tcx.sess.delay_span_bug( - DUMMY_SP, - format!("ty_is_local invoked on closure or generator: {:?}", ty), - ); - true - } + // FIXME: Constants should participate in orphan checking. + fn visit_const(&mut self, _c: ty::Const<'tcx>) -> ControlFlow { + ControlFlow::CONTINUE } } From 8ba02f18b813aa8ea2599979567797bbb9dc3ecb Mon Sep 17 00:00:00 2001 From: joboet Date: Thu, 21 Jul 2022 11:51:26 +0200 Subject: [PATCH 14/14] remove unused import --- library/std/src/sys/unix/locks/fuchsia_mutex.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/std/src/sys/unix/locks/fuchsia_mutex.rs b/library/std/src/sys/unix/locks/fuchsia_mutex.rs index fd3e9d1768b4c..ce427599c3bdd 100644 --- a/library/std/src/sys/unix/locks/fuchsia_mutex.rs +++ b/library/std/src/sys/unix/locks/fuchsia_mutex.rs @@ -44,7 +44,7 @@ use crate::sync::atomic::{ use crate::sys::futex::zircon::{ zx_futex_wait, zx_futex_wake_single_owner, zx_handle_t, zx_thread_self, ZX_ERR_BAD_HANDLE, ZX_ERR_BAD_STATE, ZX_ERR_INVALID_ARGS, ZX_ERR_TIMED_OUT, ZX_ERR_WRONG_TYPE, ZX_OK, - ZX_TIME_INFINITE, ZX_TIME_INFINITE, + ZX_TIME_INFINITE, }; // The lowest two bits of a `zx_handle_t` are always set, so the lowest bit is used to mark the