From 5a856b82f3a1bf335e0e62d92c800d8436977af1 Mon Sep 17 00:00:00 2001 From: joboet Date: Thu, 7 Nov 2024 17:13:33 +0100 Subject: [PATCH 1/8] std: allow after-main use of synchronization primitives By creating an unnamed thread handle when the actual one has already been destroyed, synchronization primitives using thread parking can be used even outside the Rust runtime. This also fixes an inefficiency in the queue-based `RwLock`: if `thread::current` was not initialized yet, it will create a new handle on every parking attempt without initializing `thread::current`. The private `current_or_unnamed` function introduced here fixes this. --- library/std/src/lib.rs | 3 --- library/std/src/sync/mpmc/array.rs | 6 ++++-- library/std/src/sync/mpmc/context.rs | 13 +++++++++---- library/std/src/sync/mpmc/list.rs | 3 ++- library/std/src/sync/mpmc/zero.rs | 6 ++++-- library/std/src/sys/sync/once/queue.rs | 9 +++++---- library/std/src/sys/sync/rwlock/queue.rs | 6 ++---- library/std/src/thread/current.rs | 17 +++++++++++++++++ library/std/src/thread/mod.rs | 15 ++++++++++++--- library/std/src/thread/scoped.rs | 7 ++++--- 10 files changed, 59 insertions(+), 26 deletions(-) diff --git a/library/std/src/lib.rs b/library/std/src/lib.rs index 5b94f036248..73fae60baff 100644 --- a/library/std/src/lib.rs +++ b/library/std/src/lib.rs @@ -174,9 +174,6 @@ //! //! - after-main use of thread-locals, which also affects additional features: //! - [`thread::current()`] -//! - [`thread::scope()`] -//! - [`sync::mpmc`] -//! - [`sync::mpsc`] //! - before-main stdio file descriptors are not guaranteed to be open on unix platforms //! //! diff --git a/library/std/src/sync/mpmc/array.rs b/library/std/src/sync/mpmc/array.rs index 2c8ba411f30..a467237fef1 100644 --- a/library/std/src/sync/mpmc/array.rs +++ b/library/std/src/sync/mpmc/array.rs @@ -346,7 +346,8 @@ impl Channel { } // Block the current thread. - let sel = cx.wait_until(deadline); + // SAFETY: the context belongs to the current thread. + let sel = unsafe { cx.wait_until(deadline) }; match sel { Selected::Waiting => unreachable!(), @@ -397,7 +398,8 @@ impl Channel { } // Block the current thread. - let sel = cx.wait_until(deadline); + // SAFETY: the context belongs to the current thread. + let sel = unsafe { cx.wait_until(deadline) }; match sel { Selected::Waiting => unreachable!(), diff --git a/library/std/src/sync/mpmc/context.rs b/library/std/src/sync/mpmc/context.rs index 2371d32d4ea..51aa7e82e78 100644 --- a/library/std/src/sync/mpmc/context.rs +++ b/library/std/src/sync/mpmc/context.rs @@ -69,7 +69,7 @@ impl Context { inner: Arc::new(Inner { select: AtomicUsize::new(Selected::Waiting.into()), packet: AtomicPtr::new(ptr::null_mut()), - thread: thread::current(), + thread: thread::current_or_unnamed(), thread_id: current_thread_id(), }), } @@ -112,8 +112,11 @@ impl Context { /// Waits until an operation is selected and returns it. /// /// If the deadline is reached, `Selected::Aborted` will be selected. + /// + /// # Safety + /// This may only be called from the thread this `Context` belongs to. #[inline] - pub fn wait_until(&self, deadline: Option) -> Selected { + pub unsafe fn wait_until(&self, deadline: Option) -> Selected { loop { // Check whether an operation has been selected. let sel = Selected::from(self.inner.select.load(Ordering::Acquire)); @@ -126,7 +129,8 @@ impl Context { let now = Instant::now(); if now < end { - thread::park_timeout(end - now); + // SAFETY: guaranteed by caller. + unsafe { self.inner.thread.park_timeout(end - now) }; } else { // The deadline has been reached. Try aborting select. return match self.try_select(Selected::Aborted) { @@ -135,7 +139,8 @@ impl Context { }; } } else { - thread::park(); + // SAFETY: guaranteed by caller. + unsafe { self.inner.thread.park() }; } } } diff --git a/library/std/src/sync/mpmc/list.rs b/library/std/src/sync/mpmc/list.rs index 523e6d2f3bb..d88914f5291 100644 --- a/library/std/src/sync/mpmc/list.rs +++ b/library/std/src/sync/mpmc/list.rs @@ -444,7 +444,8 @@ impl Channel { } // Block the current thread. - let sel = cx.wait_until(deadline); + // SAFETY: the context belongs to the current thread. + let sel = unsafe { cx.wait_until(deadline) }; match sel { Selected::Waiting => unreachable!(), diff --git a/library/std/src/sync/mpmc/zero.rs b/library/std/src/sync/mpmc/zero.rs index 446881291e6..577997c07a6 100644 --- a/library/std/src/sync/mpmc/zero.rs +++ b/library/std/src/sync/mpmc/zero.rs @@ -190,7 +190,8 @@ impl Channel { drop(inner); // Block the current thread. - let sel = cx.wait_until(deadline); + // SAFETY: the context belongs to the current thread. + let sel = unsafe { cx.wait_until(deadline) }; match sel { Selected::Waiting => unreachable!(), @@ -257,7 +258,8 @@ impl Channel { drop(inner); // Block the current thread. - let sel = cx.wait_until(deadline); + // SAFETY: the context belongs to the current thread. + let sel = unsafe { cx.wait_until(deadline) }; match sel { Selected::Waiting => unreachable!(), diff --git a/library/std/src/sys/sync/once/queue.rs b/library/std/src/sys/sync/once/queue.rs index 177d0d7744a..87837915b39 100644 --- a/library/std/src/sys/sync/once/queue.rs +++ b/library/std/src/sys/sync/once/queue.rs @@ -93,7 +93,7 @@ const QUEUE_MASK: usize = !STATE_MASK; // use interior mutability. #[repr(align(4))] // Ensure the two lower bits are free to use as state bits. struct Waiter { - thread: Cell>, + thread: Thread, signaled: AtomicBool, next: Cell<*const Waiter>, } @@ -238,7 +238,7 @@ fn wait( return_on_poisoned: bool, ) -> StateAndQueue { let node = &Waiter { - thread: Cell::new(Some(thread::current())), + thread: thread::current_or_unnamed(), signaled: AtomicBool::new(false), next: Cell::new(ptr::null()), }; @@ -277,7 +277,8 @@ fn wait( // can park ourselves, the result could be this thread never gets // unparked. Luckily `park` comes with the guarantee that if it got // an `unpark` just before on an unparked thread it does not park. - thread::park(); + // SAFETY: we retrieved this handle on the current thread above. + unsafe { node.thread.park() } } return state_and_queue.load(Acquire); @@ -309,7 +310,7 @@ impl Drop for WaiterQueue<'_> { let mut queue = to_queue(current); while !queue.is_null() { let next = (*queue).next.get(); - let thread = (*queue).thread.take().unwrap(); + let thread = (*queue).thread.clone(); (*queue).signaled.store(true, Release); thread.unpark(); queue = next; diff --git a/library/std/src/sys/sync/rwlock/queue.rs b/library/std/src/sys/sync/rwlock/queue.rs index 51330f8fafe..bd15f8ee952 100644 --- a/library/std/src/sys/sync/rwlock/queue.rs +++ b/library/std/src/sys/sync/rwlock/queue.rs @@ -118,7 +118,7 @@ use crate::mem; use crate::ptr::{self, NonNull, null_mut, without_provenance_mut}; use crate::sync::atomic::Ordering::{AcqRel, Acquire, Relaxed, Release}; use crate::sync::atomic::{AtomicBool, AtomicPtr}; -use crate::thread::{self, Thread, ThreadId}; +use crate::thread::{self, Thread}; /// The atomic lock state. type AtomicState = AtomicPtr<()>; @@ -217,9 +217,7 @@ impl Node { /// Prepare this node for waiting. fn prepare(&mut self) { // Fall back to creating an unnamed `Thread` handle to allow locking in TLS destructors. - self.thread.get_or_init(|| { - thread::try_current().unwrap_or_else(|| Thread::new_unnamed(ThreadId::new())) - }); + self.thread.get_or_init(thread::current_or_unnamed); self.completed = AtomicBool::new(false); } diff --git a/library/std/src/thread/current.rs b/library/std/src/thread/current.rs index e6eb90c4c30..b9b959f9894 100644 --- a/library/std/src/thread/current.rs +++ b/library/std/src/thread/current.rs @@ -165,6 +165,23 @@ pub(crate) fn try_current() -> Option { } } +/// Gets a handle to the thread that invokes it. If the handle stored in thread- +/// local storage was already destroyed, this creates a new unnamed temporary +/// handle to allow thread parking in nearly all situations. +pub(crate) fn current_or_unnamed() -> Thread { + let current = CURRENT.get(); + if current > DESTROYED { + unsafe { + let current = ManuallyDrop::new(Thread::from_raw(current)); + (*current).clone() + } + } else if current == DESTROYED { + Thread::new_unnamed(id::get_or_init()) + } else { + init_current(current) + } +} + /// Gets a handle to the thread that invokes it. /// /// # Examples diff --git a/library/std/src/thread/mod.rs b/library/std/src/thread/mod.rs index 227ee9d64f3..27d7d239581 100644 --- a/library/std/src/thread/mod.rs +++ b/library/std/src/thread/mod.rs @@ -186,7 +186,7 @@ mod current; #[stable(feature = "rust1", since = "1.0.0")] pub use current::current; -pub(crate) use current::{current_id, drop_current, set_current, try_current}; +pub(crate) use current::{current_id, current_or_unnamed, drop_current, set_current, try_current}; //////////////////////////////////////////////////////////////////////////////// // Thread-local storage @@ -1126,9 +1126,9 @@ pub fn park_timeout_ms(ms: u32) { #[stable(feature = "park_timeout", since = "1.4.0")] pub fn park_timeout(dur: Duration) { let guard = PanicGuard; - // SAFETY: park_timeout is called on the parker owned by this thread. + // SAFETY: park_timeout is called on a handle owned by this thread. unsafe { - current().0.parker().park_timeout(dur); + current().park_timeout(dur); } // No panic occurred, do not abort. forget(guard); @@ -1426,6 +1426,15 @@ impl Thread { unsafe { self.0.parker().park() } } + /// Like the public [`park_timeout`], but callable on any handle. This is + /// used to allow parking in TLS destructors. + /// + /// # Safety + /// May only be called from the thread to which this handle belongs. + pub(crate) unsafe fn park_timeout(&self, dur: Duration) { + unsafe { self.0.parker().park_timeout(dur) } + } + /// Atomically makes the handle's token available if it is not already. /// /// Every thread is equipped with some basic low-level blocking support, via diff --git a/library/std/src/thread/scoped.rs b/library/std/src/thread/scoped.rs index b2305b1eda7..22cae3dfc21 100644 --- a/library/std/src/thread/scoped.rs +++ b/library/std/src/thread/scoped.rs @@ -1,4 +1,4 @@ -use super::{Builder, JoinInner, Result, Thread, current, park}; +use super::{Builder, JoinInner, Result, Thread, current_or_unnamed}; use crate::marker::PhantomData; use crate::panic::{AssertUnwindSafe, catch_unwind, resume_unwind}; use crate::sync::Arc; @@ -140,7 +140,7 @@ where let scope = Scope { data: Arc::new(ScopeData { num_running_threads: AtomicUsize::new(0), - main_thread: current(), + main_thread: current_or_unnamed(), a_thread_panicked: AtomicBool::new(false), }), env: PhantomData, @@ -152,7 +152,8 @@ where // Wait until all the threads are finished. while scope.data.num_running_threads.load(Ordering::Acquire) != 0 { - park(); + // SAFETY: this is the main thread, the handle belongs to us. + unsafe { scope.data.main_thread.park() }; } // Throw any panic from `f`, or the return value of `f` if no thread panicked. From 197bba5a2e834ab62aadff6932fe9059fa41f950 Mon Sep 17 00:00:00 2001 From: "Panagiotis \"Ivory\" Vasilopoulos" Date: Thu, 21 Nov 2024 15:58:06 +0100 Subject: [PATCH 2/8] Mention that std::fs::remove_dir_all fails on files This is explicitly mentioned for std::fs::remove_file's documentation, but not in the aforementioned function. It is more likely for a slightly lazy programmer to believe that removing a file would work and that they do not have to distinguish between directories (with contents) and files themself, because of the function's recursive nature and how it distinguishes between files and directories when removing them. --- library/std/src/fs.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/library/std/src/fs.rs b/library/std/src/fs.rs index 555e8804eab..d846a4e5f09 100644 --- a/library/std/src/fs.rs +++ b/library/std/src/fs.rs @@ -2804,8 +2804,9 @@ pub fn remove_dir>(path: P) -> io::Result<()> { /// /// See [`fs::remove_file`] and [`fs::remove_dir`]. /// -/// `remove_dir_all` will fail if `remove_dir` or `remove_file` fail on any constituent paths, including the root path. +/// `remove_dir_all` will fail if `remove_dir` or `remove_file` fail on any constituent paths, including the root `path`. /// As a result, the directory you are deleting must exist, meaning that this function is not idempotent. +/// Additionally, `remove_dir_all` will also fail if the `path` is not a directory. /// /// Consider ignoring the error if validating the removal is not required for your use case. /// From 30d68eb9aada6929b8322115d44ecd24629b920b Mon Sep 17 00:00:00 2001 From: bohan Date: Fri, 22 Nov 2024 09:24:11 +0800 Subject: [PATCH 3/8] only store valid proc marco item for doc link --- compiler/rustc_resolve/src/late.rs | 32 ++++++++++--------- .../auxiliary/in-proc-item-comment.rs | 20 ++++++++++++ tests/rustdoc-ui/intra-doc/pub-proc-item.rs | 8 +++++ 3 files changed, 45 insertions(+), 15 deletions(-) create mode 100644 tests/rustdoc-ui/intra-doc/auxiliary/in-proc-item-comment.rs create mode 100644 tests/rustdoc-ui/intra-doc/pub-proc-item.rs diff --git a/compiler/rustc_resolve/src/late.rs b/compiler/rustc_resolve/src/late.rs index 26b345f5941..9ae7b404d69 100644 --- a/compiler/rustc_resolve/src/late.rs +++ b/compiler/rustc_resolve/src/late.rs @@ -4794,14 +4794,10 @@ impl<'a, 'ast, 'ra: 'ast, 'tcx> LateResolutionVisitor<'a, 'ast, 'ra, 'tcx> { let res = self.r.resolve_rustdoc_path(path.as_str(), *ns, self.parent_scope); if let Some(res) = res && let Some(def_id) = res.opt_def_id() - && !def_id.is_local() - && self.r.tcx.crate_types().contains(&CrateType::ProcMacro) - && matches!( - self.r.tcx.sess.opts.resolve_doc_links, - ResolveDocLinks::ExportedMetadata - ) + && self.is_invalid_proc_macro_item_for_doc(def_id) { - // Encoding foreign def ids in proc macro crate metadata will ICE. + // Encoding def ids in proc macro crate metadata will ICE, + // because it will only store proc macros for it. return None; } res @@ -4810,6 +4806,17 @@ impl<'a, 'ast, 'ra: 'ast, 'tcx> LateResolutionVisitor<'a, 'ast, 'ra, 'tcx> { res } + fn is_invalid_proc_macro_item_for_doc(&self, did: DefId) -> bool { + if !matches!(self.r.tcx.sess.opts.resolve_doc_links, ResolveDocLinks::ExportedMetadata) + || !self.r.tcx.crate_types().contains(&CrateType::ProcMacro) + { + return false; + } + let Some(local_did) = did.as_local() else { return true }; + let Some(node_id) = self.r.def_id_to_node_id.get(local_did) else { return true }; + !self.r.proc_macros.contains(node_id) + } + fn resolve_doc_links(&mut self, attrs: &[Attribute], maybe_exported: MaybeExported<'_>) { match self.r.tcx.sess.opts.resolve_doc_links { ResolveDocLinks::None => return, @@ -4872,14 +4879,9 @@ impl<'a, 'ast, 'ra: 'ast, 'tcx> LateResolutionVisitor<'a, 'ast, 'ra, 'tcx> { .traits_in_scope(None, &self.parent_scope, SyntaxContext::root(), None) .into_iter() .filter_map(|tr| { - if !tr.def_id.is_local() - && self.r.tcx.crate_types().contains(&CrateType::ProcMacro) - && matches!( - self.r.tcx.sess.opts.resolve_doc_links, - ResolveDocLinks::ExportedMetadata - ) - { - // Encoding foreign def ids in proc macro crate metadata will ICE. + if self.is_invalid_proc_macro_item_for_doc(tr.def_id) { + // Encoding def ids in proc macro crate metadata will ICE. + // because it will only store proc macros for it. return None; } Some(tr.def_id) diff --git a/tests/rustdoc-ui/intra-doc/auxiliary/in-proc-item-comment.rs b/tests/rustdoc-ui/intra-doc/auxiliary/in-proc-item-comment.rs new file mode 100644 index 00000000000..5d3d3c196e1 --- /dev/null +++ b/tests/rustdoc-ui/intra-doc/auxiliary/in-proc-item-comment.rs @@ -0,0 +1,20 @@ +//@ force-host +//@ no-prefer-dynamic +#![crate_type = "proc-macro"] + +extern crate proc_macro; +use proc_macro::TokenStream; + +mod view {} + +/// [`view`] +#[proc_macro] +pub fn f(_: TokenStream) -> TokenStream { + todo!() +} + +/// [`f()`] +#[proc_macro] +pub fn g(_: TokenStream) -> TokenStream { + todo!() +} diff --git a/tests/rustdoc-ui/intra-doc/pub-proc-item.rs b/tests/rustdoc-ui/intra-doc/pub-proc-item.rs new file mode 100644 index 00000000000..413efb40b0d --- /dev/null +++ b/tests/rustdoc-ui/intra-doc/pub-proc-item.rs @@ -0,0 +1,8 @@ +//@ aux-build:in-proc-item-comment.rs +//@ check-pass + +// issue#132743 + +extern crate in_proc_item_comment; + +pub use in_proc_item_comment::{f, g}; From 04d1bdc377a97e61a44689e6ac701905cd54fd3a Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Wed, 20 Nov 2024 18:48:54 +0000 Subject: [PATCH 4/8] Constify Deref and DerefMut --- library/core/src/ops/deref.rs | 47 +++++++++++++++++++ tests/ui/issues/issue-25901.rs | 2 +- tests/ui/issues/issue-25901.stderr | 20 ++------ .../arbitrary-self-from-method-substs-ice.rs | 2 +- ...bitrary-self-from-method-substs-ice.stderr | 10 ++-- 5 files changed, 60 insertions(+), 21 deletions(-) diff --git a/library/core/src/ops/deref.rs b/library/core/src/ops/deref.rs index 45688727c9b..e9bb40d0fdd 100644 --- a/library/core/src/ops/deref.rs +++ b/library/core/src/ops/deref.rs @@ -133,6 +133,7 @@ #[doc(alias = "&*")] #[stable(feature = "rust1", since = "1.0.0")] #[rustc_diagnostic_item = "Deref"] +#[cfg_attr(not(bootstrap), const_trait)] pub trait Deref { /// The resulting type after dereferencing. #[stable(feature = "rust1", since = "1.0.0")] @@ -147,6 +148,7 @@ pub trait Deref { fn deref(&self) -> &Self::Target; } +#[cfg(bootstrap)] #[stable(feature = "rust1", since = "1.0.0")] impl Deref for &T { type Target = T; @@ -157,9 +159,21 @@ impl Deref for &T { } } +#[cfg(not(bootstrap))] +#[stable(feature = "rust1", since = "1.0.0")] +impl const Deref for &T { + type Target = T; + + #[rustc_diagnostic_item = "noop_method_deref"] + fn deref(&self) -> &T { + *self + } +} + #[stable(feature = "rust1", since = "1.0.0")] impl !DerefMut for &T {} +#[cfg(bootstrap)] #[stable(feature = "rust1", since = "1.0.0")] impl Deref for &mut T { type Target = T; @@ -169,6 +183,16 @@ impl Deref for &mut T { } } +#[cfg(not(bootstrap))] +#[stable(feature = "rust1", since = "1.0.0")] +impl const Deref for &mut T { + type Target = T; + + fn deref(&self) -> &T { + *self + } +} + /// Used for mutable dereferencing operations, like in `*v = 1;`. /// /// In addition to being used for explicit dereferencing operations with the @@ -258,9 +282,23 @@ impl Deref for &mut T { /// *x = 'b'; /// assert_eq!('b', x.value); /// ``` +#[cfg(not(bootstrap))] #[lang = "deref_mut"] #[doc(alias = "*")] #[stable(feature = "rust1", since = "1.0.0")] +#[const_trait] +pub trait DerefMut: ~const Deref { + /// Mutably dereferences the value. + #[stable(feature = "rust1", since = "1.0.0")] + #[rustc_diagnostic_item = "deref_mut_method"] + fn deref_mut(&mut self) -> &mut Self::Target; +} + +/// Bootstrap +#[lang = "deref_mut"] +#[doc(alias = "*")] +#[stable(feature = "rust1", since = "1.0.0")] +#[cfg(bootstrap)] pub trait DerefMut: Deref { /// Mutably dereferences the value. #[stable(feature = "rust1", since = "1.0.0")] @@ -268,6 +306,7 @@ pub trait DerefMut: Deref { fn deref_mut(&mut self) -> &mut Self::Target; } +#[cfg(bootstrap)] #[stable(feature = "rust1", since = "1.0.0")] impl DerefMut for &mut T { fn deref_mut(&mut self) -> &mut T { @@ -275,6 +314,14 @@ impl DerefMut for &mut T { } } +#[cfg(not(bootstrap))] +#[stable(feature = "rust1", since = "1.0.0")] +impl const DerefMut for &mut T { + fn deref_mut(&mut self) -> &mut T { + *self + } +} + /// Perma-unstable marker trait. Indicates that the type has a well-behaved [`Deref`] /// (and, if applicable, [`DerefMut`]) implementation. This is relied on for soundness /// of deref patterns. diff --git a/tests/ui/issues/issue-25901.rs b/tests/ui/issues/issue-25901.rs index 85e12463a90..eae038c71a0 100644 --- a/tests/ui/issues/issue-25901.rs +++ b/tests/ui/issues/issue-25901.rs @@ -2,7 +2,7 @@ struct A; struct B; static S: &'static B = &A; -//~^ ERROR cannot perform deref coercion +//~^ ERROR cannot call conditionally-const method use std::ops::Deref; diff --git a/tests/ui/issues/issue-25901.stderr b/tests/ui/issues/issue-25901.stderr index bcbc805908f..655a8b78c6a 100644 --- a/tests/ui/issues/issue-25901.stderr +++ b/tests/ui/issues/issue-25901.stderr @@ -1,23 +1,13 @@ -error[E0015]: cannot perform deref coercion on `A` in statics +error[E0658]: cannot call conditionally-const method `::deref` in statics --> $DIR/issue-25901.rs:4:24 | LL | static S: &'static B = &A; | ^^ | - = note: attempting to deref into `B` -note: deref defined here - --> $DIR/issue-25901.rs:10:5 - | -LL | type Target = B; - | ^^^^^^^^^^^ -note: impl defined here, but it is not `const` - --> $DIR/issue-25901.rs:9:1 - | -LL | impl Deref for A { - | ^^^^^^^^^^^^^^^^ - = note: calls in statics are limited to constant functions, tuple structs and tuple variants - = note: consider wrapping this expression in `std::sync::LazyLock::new(|| ...)` + = note: see issue #67792 for more information + = help: add `#![feature(const_trait_impl)]` to the crate attributes to enable + = note: this compiler was built on YYYY-MM-DD; consider upgrading it if it is out of date error: aborting due to 1 previous error -For more information about this error, try `rustc --explain E0015`. +For more information about this error, try `rustc --explain E0658`. diff --git a/tests/ui/self/arbitrary-self-from-method-substs-ice.rs b/tests/ui/self/arbitrary-self-from-method-substs-ice.rs index d121a194be6..2c0f25fc6ff 100644 --- a/tests/ui/self/arbitrary-self-from-method-substs-ice.rs +++ b/tests/ui/self/arbitrary-self-from-method-substs-ice.rs @@ -11,7 +11,7 @@ impl Foo { //~^ ERROR invalid generic `self` parameter type //~| ERROR destructor of `R` cannot be evaluated at compile-time self.0 - //~^ ERROR cannot call non-const fn `::deref` in constant function + //~^ ERROR cannot call conditionally-const method `::deref` in constant function } } diff --git a/tests/ui/self/arbitrary-self-from-method-substs-ice.stderr b/tests/ui/self/arbitrary-self-from-method-substs-ice.stderr index 7252b5890fd..cf4c219215e 100644 --- a/tests/ui/self/arbitrary-self-from-method-substs-ice.stderr +++ b/tests/ui/self/arbitrary-self-from-method-substs-ice.stderr @@ -1,10 +1,12 @@ -error[E0015]: cannot call non-const fn `::deref` in constant functions +error[E0658]: cannot call conditionally-const method `::deref` in constant functions --> $DIR/arbitrary-self-from-method-substs-ice.rs:13:9 | LL | self.0 | ^^^^^^ | - = note: calls in constant functions are limited to constant functions, tuple structs and tuple variants + = note: see issue #67792 for more information + = help: add `#![feature(const_trait_impl)]` to the crate attributes to enable + = note: this compiler was built on YYYY-MM-DD; consider upgrading it if it is out of date error[E0493]: destructor of `R` cannot be evaluated at compile-time --> $DIR/arbitrary-self-from-method-substs-ice.rs:10:43 @@ -26,5 +28,5 @@ LL | const fn get>(self: R) -> u32 { error: aborting due to 3 previous errors -Some errors have detailed explanations: E0015, E0493, E0801. -For more information about an error, try `rustc --explain E0015`. +Some errors have detailed explanations: E0493, E0658, E0801. +For more information about an error, try `rustc --explain E0493`. From 3a23669787db1e43a8bea7d71337f31325d00305 Mon Sep 17 00:00:00 2001 From: DianQK Date: Wed, 20 Nov 2024 22:45:32 +0800 Subject: [PATCH 5/8] embed-bitcode is no longer used in iOS --- compiler/rustc_codegen_llvm/src/back/write.rs | 19 +------------------ src/doc/rustc/src/codegen-options/index.md | 13 +++++-------- 2 files changed, 6 insertions(+), 26 deletions(-) diff --git a/compiler/rustc_codegen_llvm/src/back/write.rs b/compiler/rustc_codegen_llvm/src/back/write.rs index a65ae4df1e3..00f7b479fa7 100644 --- a/compiler/rustc_codegen_llvm/src/back/write.rs +++ b/compiler/rustc_codegen_llvm/src/back/write.rs @@ -955,24 +955,7 @@ pub(crate) fn bitcode_section_name(cgcx: &CodegenContext) -> } } -/// Embed the bitcode of an LLVM module in the LLVM module itself. -/// -/// This is done primarily for iOS where it appears to be standard to compile C -/// code at least with `-fembed-bitcode` which creates two sections in the -/// executable: -/// -/// * __LLVM,__bitcode -/// * __LLVM,__cmdline -/// -/// It appears *both* of these sections are necessary to get the linker to -/// recognize what's going on. A suitable cmdline value is taken from the -/// target spec. -/// -/// Furthermore debug/O1 builds don't actually embed bitcode but rather just -/// embed an empty section. -/// -/// Basically all of this is us attempting to follow in the footsteps of clang -/// on iOS. See #35968 for lots more info. +/// Embed the bitcode of an LLVM module for LTO in the LLVM module itself. unsafe fn embed_bitcode( cgcx: &CodegenContext, llcx: &llvm::Context, diff --git a/src/doc/rustc/src/codegen-options/index.md b/src/doc/rustc/src/codegen-options/index.md index cb43feca758..e987d06b0f3 100644 --- a/src/doc/rustc/src/codegen-options/index.md +++ b/src/doc/rustc/src/codegen-options/index.md @@ -119,17 +119,14 @@ files. It takes one of the following values: * `n`, `no`, `off` or `false`: omit bitcode from rlibs. LLVM bitcode is required when rustc is performing link-time optimization (LTO). -It is also required on some targets like iOS ones where vendors look for LLVM -bitcode. Embedded bitcode will appear in rustc-generated object files inside of -a section whose name is defined by the target platform. Most of the time this is -`.llvmbc`. +Embedded bitcode will appear in rustc-generated object files inside of a section +whose name is defined by the target platform. Most of the time this is `.llvmbc`. The use of `-C embed-bitcode=no` can significantly improve compile times and reduce generated file sizes if your compilation does not actually need bitcode -(e.g. if you're not compiling for iOS or you're not performing LTO). For these -reasons, Cargo uses `-C embed-bitcode=no` whenever possible. Likewise, if you -are building directly with `rustc` we recommend using `-C embed-bitcode=no` -whenever you are not using LTO. +(e.g. if you're not performing LTO). For these reasons, Cargo uses `-C embed-bitcode=no` +whenever possible. Likewise, if you are building directly with `rustc` we recommend +using `-C embed-bitcode=no` whenever you are not using LTO. If combined with `-C lto`, `-C embed-bitcode=no` will cause `rustc` to abort at start-up, because the combination is invalid. From 7cc5feea4d31a96e23c95f26fbee8e77a968c937 Mon Sep 17 00:00:00 2001 From: DianQK Date: Wed, 20 Nov 2024 22:54:46 +0800 Subject: [PATCH 6/8] Remove forces_embed_bitcode --- compiler/rustc_codegen_ssa/src/back/write.rs | 6 ++---- compiler/rustc_target/src/spec/mod.rs | 5 ----- 2 files changed, 2 insertions(+), 9 deletions(-) diff --git a/compiler/rustc_codegen_ssa/src/back/write.rs b/compiler/rustc_codegen_ssa/src/back/write.rs index a2285bf9204..7c0e9cfd5a7 100644 --- a/compiler/rustc_codegen_ssa/src/back/write.rs +++ b/compiler/rustc_codegen_ssa/src/back/write.rs @@ -432,11 +432,9 @@ struct CompiledModules { fn need_bitcode_in_object(tcx: TyCtxt<'_>) -> bool { let sess = tcx.sess; - let requested_for_rlib = sess.opts.cg.embed_bitcode + sess.opts.cg.embed_bitcode && tcx.crate_types().contains(&CrateType::Rlib) - && sess.opts.output_types.contains_key(&OutputType::Exe); - let forced_by_target = sess.target.forces_embed_bitcode; - requested_for_rlib || forced_by_target + && sess.opts.output_types.contains_key(&OutputType::Exe) } fn need_pre_lto_bitcode_for_incr_comp(sess: &Session) -> bool { diff --git a/compiler/rustc_target/src/spec/mod.rs b/compiler/rustc_target/src/spec/mod.rs index 321ab40403a..fead20ec7d1 100644 --- a/compiler/rustc_target/src/spec/mod.rs +++ b/compiler/rustc_target/src/spec/mod.rs @@ -2327,8 +2327,6 @@ pub struct TargetOptions { /// If we give emcc .o files that are actually .bc files it /// will 'just work'. pub obj_is_bitcode: bool, - /// Whether the target requires that emitted object code includes bitcode. - pub forces_embed_bitcode: bool, /// Content of the LLVM cmdline section associated with embedded bitcode. pub bitcode_llvm_cmdline: StaticCow, @@ -2671,7 +2669,6 @@ impl Default for TargetOptions { allow_asm: true, has_thread_local: false, obj_is_bitcode: false, - forces_embed_bitcode: false, bitcode_llvm_cmdline: "".into(), min_atomic_width: None, max_atomic_width: None, @@ -3412,7 +3409,6 @@ impl Target { key!(main_needs_argc_argv, bool); key!(has_thread_local, bool); key!(obj_is_bitcode, bool); - key!(forces_embed_bitcode, bool); key!(bitcode_llvm_cmdline); key!(max_atomic_width, Option); key!(min_atomic_width, Option); @@ -3687,7 +3683,6 @@ impl ToJson for Target { target_option_val!(main_needs_argc_argv); target_option_val!(has_thread_local); target_option_val!(obj_is_bitcode); - target_option_val!(forces_embed_bitcode); target_option_val!(bitcode_llvm_cmdline); target_option_val!(min_atomic_width); target_option_val!(max_atomic_width); From b9a0e57b0cce981460595598ee74c1cd55049377 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 23 Nov 2024 16:42:20 +0100 Subject: [PATCH 7/8] add a test for target-feature-ABI warnings in closures --- .../rustc_codegen_ssa/src/codegen_attrs.rs | 6 ++-- tests/ui/simd-abi-checks.rs | 13 ++++++++ tests/ui/simd-abi-checks.stderr | 30 ++++++++++++------- 3 files changed, 36 insertions(+), 13 deletions(-) diff --git a/compiler/rustc_codegen_ssa/src/codegen_attrs.rs b/compiler/rustc_codegen_ssa/src/codegen_attrs.rs index 32e9422e005..a5acd8170ab 100644 --- a/compiler/rustc_codegen_ssa/src/codegen_attrs.rs +++ b/compiler/rustc_codegen_ssa/src/codegen_attrs.rs @@ -576,9 +576,9 @@ fn codegen_fn_attrs(tcx: TyCtxt<'_>, did: LocalDefId) -> CodegenFnAttrs { // If this closure is marked `#[inline(always)]`, simply skip adding `#[target_feature]`. // // At this point, `unsafe` has already been checked and `#[target_feature]` only affects codegen. - // Emitting both `#[inline(always)]` and `#[target_feature]` can potentially result in an - // ICE, because LLVM errors when the function fails to be inlined due to a target feature - // mismatch. + // Due to LLVM limitations, emitting both `#[inline(always)]` and `#[target_feature]` is *unsound*: + // the function may be inlined into a caller with fewer target features. Also see + // . // // Using `#[inline(always)]` implies that this closure will most likely be inlined into // its parent function, which effectively inherits the features anyway. Boxing this closure diff --git a/tests/ui/simd-abi-checks.rs b/tests/ui/simd-abi-checks.rs index 9e3af26e9c4..04e58e8aeb0 100644 --- a/tests/ui/simd-abi-checks.rs +++ b/tests/ui/simd-abi-checks.rs @@ -4,6 +4,7 @@ #![feature(avx512_target_feature)] #![feature(portable_simd)] +#![feature(target_feature_11)] #![allow(improper_ctypes_definitions)] use std::arch::x86_64::*; @@ -50,6 +51,14 @@ unsafe fn test() { as_f64x8(arg); } +#[target_feature(enable = "avx")] +unsafe fn in_closure() -> impl FnOnce() -> __m256 { + #[inline(always)] // this disables target-feature inheritance + || g() + //~^ WARNING this function call uses a SIMD vector type that (with the chosen ABI) requires the `avx` target feature, which is not enabled in the caller + //~| WARNING this was previously accepted by the compiler +} + fn main() { unsafe { f(g()); @@ -78,4 +87,8 @@ fn main() { //~| WARNING this was previously accepted by the compiler //~| WARNING this was previously accepted by the compiler } + + unsafe { + in_closure()(); + } } diff --git a/tests/ui/simd-abi-checks.stderr b/tests/ui/simd-abi-checks.stderr index cd04cfa6e14..e03318bbecf 100644 --- a/tests/ui/simd-abi-checks.stderr +++ b/tests/ui/simd-abi-checks.stderr @@ -1,5 +1,5 @@ warning: this function call uses a SIMD vector type that (with the chosen ABI) requires the `avx` target feature, which is not enabled in the caller - --> $DIR/simd-abi-checks.rs:55:11 + --> $DIR/simd-abi-checks.rs:64:11 | LL | f(g()); | ^^^ function called here @@ -10,7 +10,7 @@ LL | f(g()); = note: `#[warn(abi_unsupported_vector_types)]` on by default warning: this function call uses a SIMD vector type that (with the chosen ABI) requires the `avx` target feature, which is not enabled in the caller - --> $DIR/simd-abi-checks.rs:55:9 + --> $DIR/simd-abi-checks.rs:64:9 | LL | f(g()); | ^^^^^^ function called here @@ -20,7 +20,7 @@ LL | f(g()); = help: consider enabling it globally (`-C target-feature=+avx`) or locally (`#[target_feature(enable="avx")]`) warning: this function call uses a SIMD vector type that (with the chosen ABI) requires the `avx` target feature, which is not enabled in the caller - --> $DIR/simd-abi-checks.rs:63:14 + --> $DIR/simd-abi-checks.rs:72:14 | LL | gavx(favx()); | ^^^^^^ function called here @@ -30,7 +30,7 @@ LL | gavx(favx()); = help: consider enabling it globally (`-C target-feature=+avx`) or locally (`#[target_feature(enable="avx")]`) warning: this function call uses a SIMD vector type that (with the chosen ABI) requires the `avx` target feature, which is not enabled in the caller - --> $DIR/simd-abi-checks.rs:63:9 + --> $DIR/simd-abi-checks.rs:72:9 | LL | gavx(favx()); | ^^^^^^^^^^^^ function called here @@ -40,7 +40,7 @@ LL | gavx(favx()); = help: consider enabling it globally (`-C target-feature=+avx`) or locally (`#[target_feature(enable="avx")]`) warning: this function call uses a SIMD vector type that (with the chosen ABI) requires the `avx` target feature, which is not enabled in the caller - --> $DIR/simd-abi-checks.rs:75:19 + --> $DIR/simd-abi-checks.rs:84:19 | LL | w(Wrapper(g())); | ^^^ function called here @@ -50,7 +50,7 @@ LL | w(Wrapper(g())); = help: consider enabling it globally (`-C target-feature=+avx`) or locally (`#[target_feature(enable="avx")]`) warning: this function call uses a SIMD vector type that (with the chosen ABI) requires the `avx` target feature, which is not enabled in the caller - --> $DIR/simd-abi-checks.rs:75:9 + --> $DIR/simd-abi-checks.rs:84:9 | LL | w(Wrapper(g())); | ^^^^^^^^^^^^^^^ function called here @@ -60,7 +60,7 @@ LL | w(Wrapper(g())); = help: consider enabling it globally (`-C target-feature=+avx`) or locally (`#[target_feature(enable="avx")]`) warning: this function definition uses a SIMD vector type that (with the chosen ABI) requires the `avx` target feature, which is not enabled - --> $DIR/simd-abi-checks.rs:26:1 + --> $DIR/simd-abi-checks.rs:27:1 | LL | unsafe extern "C" fn g() -> __m256 { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ function defined here @@ -70,7 +70,7 @@ LL | unsafe extern "C" fn g() -> __m256 { = help: consider enabling it globally (`-C target-feature=+avx`) or locally (`#[target_feature(enable="avx")]`) warning: this function definition uses a SIMD vector type that (with the chosen ABI) requires the `avx` target feature, which is not enabled - --> $DIR/simd-abi-checks.rs:20:1 + --> $DIR/simd-abi-checks.rs:21:1 | LL | unsafe extern "C" fn f(_: __m256) { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ function defined here @@ -80,7 +80,7 @@ LL | unsafe extern "C" fn f(_: __m256) { = help: consider enabling it globally (`-C target-feature=+avx`) or locally (`#[target_feature(enable="avx")]`) warning: this function definition uses a SIMD vector type that (with the chosen ABI) requires the `avx` target feature, which is not enabled - --> $DIR/simd-abi-checks.rs:14:1 + --> $DIR/simd-abi-checks.rs:15:1 | LL | unsafe extern "C" fn w(_: Wrapper) { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ function defined here @@ -89,7 +89,17 @@ LL | unsafe extern "C" fn w(_: Wrapper) { = note: for more information, see issue #116558 = help: consider enabling it globally (`-C target-feature=+avx`) or locally (`#[target_feature(enable="avx")]`) -warning: 9 warnings emitted +warning: this function call uses a SIMD vector type that (with the chosen ABI) requires the `avx` target feature, which is not enabled in the caller + --> $DIR/simd-abi-checks.rs:57:8 + | +LL | || g() + | ^^^ function called here + | + = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! + = note: for more information, see issue #116558 + = help: consider enabling it globally (`-C target-feature=+avx`) or locally (`#[target_feature(enable="avx")]`) + +warning: 10 warnings emitted Future incompatibility report: Future breakage diagnostic: warning: this function call uses a SIMD vector type that (with the chosen ABI) requires the `avx` target feature, which is not enabled in the caller From 3440505ec8939e9f7d14f98f356bea588543db69 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 23 Nov 2024 17:18:08 +0100 Subject: [PATCH 8/8] add vector ABI check test for calling extern fn --- tests/ui/simd-abi-checks.rs | 18 ++++++++++- tests/ui/simd-abi-checks.stderr | 54 +++++++++++++++++++++++++++------ 2 files changed, 61 insertions(+), 11 deletions(-) diff --git a/tests/ui/simd-abi-checks.rs b/tests/ui/simd-abi-checks.rs index 04e58e8aeb0..c97767b2794 100644 --- a/tests/ui/simd-abi-checks.rs +++ b/tests/ui/simd-abi-checks.rs @@ -4,7 +4,7 @@ #![feature(avx512_target_feature)] #![feature(portable_simd)] -#![feature(target_feature_11)] +#![feature(target_feature_11, simd_ffi)] #![allow(improper_ctypes_definitions)] use std::arch::x86_64::*; @@ -91,4 +91,20 @@ fn main() { unsafe { in_closure()(); } + + unsafe { + #[expect(improper_ctypes)] + extern "C" { + fn some_extern() -> __m256; + } + some_extern(); + //~^ WARNING this function call uses a SIMD vector type that (with the chosen ABI) requires the `avx` target feature, which is not enabled in the caller + //~| WARNING this was previously accepted by the compiler + } +} + +#[no_mangle] +#[target_feature(enable = "avx")] +fn some_extern() -> __m256 { + todo!() } diff --git a/tests/ui/simd-abi-checks.stderr b/tests/ui/simd-abi-checks.stderr index e03318bbecf..eb7d9e81029 100644 --- a/tests/ui/simd-abi-checks.stderr +++ b/tests/ui/simd-abi-checks.stderr @@ -59,6 +59,16 @@ LL | w(Wrapper(g())); = note: for more information, see issue #116558 = help: consider enabling it globally (`-C target-feature=+avx`) or locally (`#[target_feature(enable="avx")]`) +warning: this function call uses a SIMD vector type that (with the chosen ABI) requires the `avx` target feature, which is not enabled in the caller + --> $DIR/simd-abi-checks.rs:100:9 + | +LL | some_extern(); + | ^^^^^^^^^^^^^ function called here + | + = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! + = note: for more information, see issue #116558 + = help: consider enabling it globally (`-C target-feature=+avx`) or locally (`#[target_feature(enable="avx")]`) + warning: this function definition uses a SIMD vector type that (with the chosen ABI) requires the `avx` target feature, which is not enabled --> $DIR/simd-abi-checks.rs:27:1 | @@ -99,11 +109,11 @@ LL | || g() = note: for more information, see issue #116558 = help: consider enabling it globally (`-C target-feature=+avx`) or locally (`#[target_feature(enable="avx")]`) -warning: 10 warnings emitted +warning: 11 warnings emitted Future incompatibility report: Future breakage diagnostic: warning: this function call uses a SIMD vector type that (with the chosen ABI) requires the `avx` target feature, which is not enabled in the caller - --> $DIR/simd-abi-checks.rs:55:11 + --> $DIR/simd-abi-checks.rs:64:11 | LL | f(g()); | ^^^ function called here @@ -115,7 +125,7 @@ LL | f(g()); Future breakage diagnostic: warning: this function call uses a SIMD vector type that (with the chosen ABI) requires the `avx` target feature, which is not enabled in the caller - --> $DIR/simd-abi-checks.rs:55:9 + --> $DIR/simd-abi-checks.rs:64:9 | LL | f(g()); | ^^^^^^ function called here @@ -127,7 +137,7 @@ LL | f(g()); Future breakage diagnostic: warning: this function call uses a SIMD vector type that (with the chosen ABI) requires the `avx` target feature, which is not enabled in the caller - --> $DIR/simd-abi-checks.rs:63:14 + --> $DIR/simd-abi-checks.rs:72:14 | LL | gavx(favx()); | ^^^^^^ function called here @@ -139,7 +149,7 @@ LL | gavx(favx()); Future breakage diagnostic: warning: this function call uses a SIMD vector type that (with the chosen ABI) requires the `avx` target feature, which is not enabled in the caller - --> $DIR/simd-abi-checks.rs:63:9 + --> $DIR/simd-abi-checks.rs:72:9 | LL | gavx(favx()); | ^^^^^^^^^^^^ function called here @@ -151,7 +161,7 @@ LL | gavx(favx()); Future breakage diagnostic: warning: this function call uses a SIMD vector type that (with the chosen ABI) requires the `avx` target feature, which is not enabled in the caller - --> $DIR/simd-abi-checks.rs:75:19 + --> $DIR/simd-abi-checks.rs:84:19 | LL | w(Wrapper(g())); | ^^^ function called here @@ -163,7 +173,7 @@ LL | w(Wrapper(g())); Future breakage diagnostic: warning: this function call uses a SIMD vector type that (with the chosen ABI) requires the `avx` target feature, which is not enabled in the caller - --> $DIR/simd-abi-checks.rs:75:9 + --> $DIR/simd-abi-checks.rs:84:9 | LL | w(Wrapper(g())); | ^^^^^^^^^^^^^^^ function called here @@ -173,9 +183,21 @@ LL | w(Wrapper(g())); = help: consider enabling it globally (`-C target-feature=+avx`) or locally (`#[target_feature(enable="avx")]`) = note: `#[warn(abi_unsupported_vector_types)]` on by default +Future breakage diagnostic: +warning: this function call uses a SIMD vector type that (with the chosen ABI) requires the `avx` target feature, which is not enabled in the caller + --> $DIR/simd-abi-checks.rs:100:9 + | +LL | some_extern(); + | ^^^^^^^^^^^^^ function called here + | + = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! + = note: for more information, see issue #116558 + = help: consider enabling it globally (`-C target-feature=+avx`) or locally (`#[target_feature(enable="avx")]`) + = note: `#[warn(abi_unsupported_vector_types)]` on by default + Future breakage diagnostic: warning: this function definition uses a SIMD vector type that (with the chosen ABI) requires the `avx` target feature, which is not enabled - --> $DIR/simd-abi-checks.rs:26:1 + --> $DIR/simd-abi-checks.rs:27:1 | LL | unsafe extern "C" fn g() -> __m256 { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ function defined here @@ -187,7 +209,7 @@ LL | unsafe extern "C" fn g() -> __m256 { Future breakage diagnostic: warning: this function definition uses a SIMD vector type that (with the chosen ABI) requires the `avx` target feature, which is not enabled - --> $DIR/simd-abi-checks.rs:20:1 + --> $DIR/simd-abi-checks.rs:21:1 | LL | unsafe extern "C" fn f(_: __m256) { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ function defined here @@ -199,7 +221,7 @@ LL | unsafe extern "C" fn f(_: __m256) { Future breakage diagnostic: warning: this function definition uses a SIMD vector type that (with the chosen ABI) requires the `avx` target feature, which is not enabled - --> $DIR/simd-abi-checks.rs:14:1 + --> $DIR/simd-abi-checks.rs:15:1 | LL | unsafe extern "C" fn w(_: Wrapper) { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ function defined here @@ -209,3 +231,15 @@ LL | unsafe extern "C" fn w(_: Wrapper) { = help: consider enabling it globally (`-C target-feature=+avx`) or locally (`#[target_feature(enable="avx")]`) = note: `#[warn(abi_unsupported_vector_types)]` on by default +Future breakage diagnostic: +warning: this function call uses a SIMD vector type that (with the chosen ABI) requires the `avx` target feature, which is not enabled in the caller + --> $DIR/simd-abi-checks.rs:57:8 + | +LL | || g() + | ^^^ function called here + | + = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! + = note: for more information, see issue #116558 + = help: consider enabling it globally (`-C target-feature=+avx`) or locally (`#[target_feature(enable="avx")]`) + = note: `#[warn(abi_unsupported_vector_types)]` on by default +