From d74d67c8369fd759d860d215aaa576a03d99b692 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Fri, 1 Mar 2024 15:58:12 +0000 Subject: [PATCH 01/12] Implement async closure signature deduction --- compiler/rustc_hir_typeck/src/closure.rs | 74 ++++++++++++------- .../async-closures/signature-deduction.rs | 10 +++ 2 files changed, 57 insertions(+), 27 deletions(-) create mode 100644 tests/ui/async-await/async-closures/signature-deduction.rs diff --git a/compiler/rustc_hir_typeck/src/closure.rs b/compiler/rustc_hir_typeck/src/closure.rs index 5bdd9412d0e51..b6bfc81bc1975 100644 --- a/compiler/rustc_hir_typeck/src/closure.rs +++ b/compiler/rustc_hir_typeck/src/closure.rs @@ -56,18 +56,12 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { // It's always helpful for inference if we know the kind of // closure sooner rather than later, so first examine the expected // type, and see if can glean a closure kind from there. - let (expected_sig, expected_kind) = match closure.kind { - hir::ClosureKind::Closure => match expected.to_option(self) { - Some(ty) => { - self.deduce_closure_signature(self.try_structurally_resolve_type(expr_span, ty)) - } - None => (None, None), - }, - // We don't want to deduce a signature from `Fn` bounds for coroutines - // or coroutine-closures, because the former does not implement `Fn` - // ever, and the latter's signature doesn't correspond to the coroutine - // type that it returns. - hir::ClosureKind::Coroutine(_) | hir::ClosureKind::CoroutineClosure(_) => (None, None), + let (expected_sig, expected_kind) = match expected.to_option(self) { + Some(ty) => self.deduce_closure_signature( + self.try_structurally_resolve_type(expr_span, ty), + closure.kind, + ), + None => (None, None), }; let ClosureSignatures { bound_sig, mut liberated_sig } = @@ -323,11 +317,13 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { fn deduce_closure_signature( &self, expected_ty: Ty<'tcx>, + closure_kind: hir::ClosureKind, ) -> (Option>, Option) { match *expected_ty.kind() { ty::Alias(ty::Opaque, ty::AliasTy { def_id, args, .. }) => self .deduce_closure_signature_from_predicates( expected_ty, + closure_kind, self.tcx .explicit_item_bounds(def_id) .iter_instantiated_copied(self.tcx, args) @@ -336,7 +332,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { ty::Dynamic(object_type, ..) => { let sig = object_type.projection_bounds().find_map(|pb| { let pb = pb.with_self_ty(self.tcx, self.tcx.types.trait_object_dummy_self); - self.deduce_sig_from_projection(None, pb) + self.deduce_sig_from_projection(None, closure_kind, pb) }); let kind = object_type .principal_def_id() @@ -345,12 +341,18 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { } ty::Infer(ty::TyVar(vid)) => self.deduce_closure_signature_from_predicates( Ty::new_var(self.tcx, self.root_var(vid)), + closure_kind, self.obligations_for_self_ty(vid).map(|obl| (obl.predicate, obl.cause.span)), ), - ty::FnPtr(sig) => { - let expected_sig = ExpectedSig { cause_span: None, sig }; - (Some(expected_sig), Some(ty::ClosureKind::Fn)) - } + ty::FnPtr(sig) => match closure_kind { + hir::ClosureKind::Closure => { + let expected_sig = ExpectedSig { cause_span: None, sig }; + (Some(expected_sig), Some(ty::ClosureKind::Fn)) + } + hir::ClosureKind::Coroutine(_) | hir::ClosureKind::CoroutineClosure(_) => { + (None, None) + } + }, _ => (None, None), } } @@ -358,6 +360,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { fn deduce_closure_signature_from_predicates( &self, expected_ty: Ty<'tcx>, + closure_kind: hir::ClosureKind, predicates: impl DoubleEndedIterator, Span)>, ) -> (Option>, Option) { let mut expected_sig = None; @@ -386,6 +389,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { span, self.deduce_sig_from_projection( Some(span), + closure_kind, bound_predicate.rebind(proj_predicate), ), ); @@ -422,13 +426,22 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { ty::PredicateKind::Clause(ty::ClauseKind::Trait(data)) => Some(data.def_id()), _ => None, }; - if let Some(closure_kind) = - trait_def_id.and_then(|def_id| self.tcx.fn_trait_kind_from_def_id(def_id)) - { - expected_kind = Some( - expected_kind - .map_or_else(|| closure_kind, |current| cmp::min(current, closure_kind)), - ); + + if let Some(trait_def_id) = trait_def_id { + let found_kind = match closure_kind { + hir::ClosureKind::Closure => self.tcx.fn_trait_kind_from_def_id(trait_def_id), + hir::ClosureKind::CoroutineClosure(hir::CoroutineDesugaring::Async) => { + self.tcx.async_fn_trait_kind_from_def_id(trait_def_id) + } + _ => None, + }; + + if let Some(found_kind) = found_kind { + expected_kind = Some( + expected_kind + .map_or_else(|| found_kind, |current| cmp::min(current, found_kind)), + ); + } } } @@ -445,14 +458,21 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { fn deduce_sig_from_projection( &self, cause_span: Option, + closure_kind: hir::ClosureKind, projection: ty::PolyProjectionPredicate<'tcx>, ) -> Option> { let tcx = self.tcx; let trait_def_id = projection.trait_def_id(tcx); - // For now, we only do signature deduction based off of the `Fn` traits. - if !tcx.is_fn_trait(trait_def_id) { - return None; + + // For now, we only do signature deduction based off of the `Fn` and `AsyncFn` traits, + // for closures and async closures, respectively. + match closure_kind { + hir::ClosureKind::Closure + if self.tcx.fn_trait_kind_from_def_id(trait_def_id).is_some() => {} + hir::ClosureKind::CoroutineClosure(hir::CoroutineDesugaring::Async) + if self.tcx.async_fn_trait_kind_from_def_id(trait_def_id).is_some() => {} + _ => return None, } let arg_param_ty = projection.skip_binder().projection_ty.args.type_at(1); diff --git a/tests/ui/async-await/async-closures/signature-deduction.rs b/tests/ui/async-await/async-closures/signature-deduction.rs new file mode 100644 index 0000000000000..031dab1029658 --- /dev/null +++ b/tests/ui/async-await/async-closures/signature-deduction.rs @@ -0,0 +1,10 @@ +//@ check-pass +//@ edition: 2021 + +#![feature(async_closure)] + +async fn foo(x: impl async Fn(&str) -> &str) {} + +fn main() { + foo(async |x| x); +} From 374607d6b9abb6e005200e79d1817b4a4f609e41 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 2 Mar 2024 12:53:28 +0100 Subject: [PATCH 02/12] const_eval_select: make it safe but be careful with what we expose on stable for now --- .../rustc_hir_analysis/src/check/intrinsic.rs | 3 +- library/alloc/src/alloc.rs | 1 + library/core/src/ffi/c_str.rs | 10 ++++- library/core/src/intrinsics.rs | 42 ++++++++++--------- library/core/src/num/f32.rs | 10 ++++- library/core/src/num/f64.rs | 10 ++++- library/core/src/panicking.rs | 1 + library/core/src/ptr/const_ptr.rs | 11 ++++- library/core/src/ptr/mod.rs | 1 + library/core/src/ptr/mut_ptr.rs | 10 ++++- library/core/src/slice/index.rs | 7 +++- library/core/src/str/mod.rs | 1 + .../intrinsics/const-eval-select-backtrace.rs | 5 +-- .../const-eval-select-backtrace.run.stderr | 2 +- .../intrinsics/const-eval-select-stability.rs | 2 +- .../ui/intrinsics/const-eval-select-x86_64.rs | 4 +- tests/ui/intrinsics/const-eval-select.rs | 4 +- .../effects/minicore.rs | 3 +- 18 files changed, 84 insertions(+), 43 deletions(-) diff --git a/compiler/rustc_hir_analysis/src/check/intrinsic.rs b/compiler/rustc_hir_analysis/src/check/intrinsic.rs index 6e9b4236e2088..98e81a0e0f902 100644 --- a/compiler/rustc_hir_analysis/src/check/intrinsic.rs +++ b/compiler/rustc_hir_analysis/src/check/intrinsic.rs @@ -128,7 +128,8 @@ pub fn intrinsic_operation_unsafety(tcx: TyCtxt<'_>, intrinsic_id: LocalDefId) - | sym::fsub_algebraic | sym::fmul_algebraic | sym::fdiv_algebraic - | sym::frem_algebraic => hir::Unsafety::Normal, + | sym::frem_algebraic + | sym::const_eval_select => hir::Unsafety::Normal, _ => hir::Unsafety::Unsafe, }; diff --git a/library/alloc/src/alloc.rs b/library/alloc/src/alloc.rs index 0b1429397559d..e99d6268e535b 100644 --- a/library/alloc/src/alloc.rs +++ b/library/alloc/src/alloc.rs @@ -385,6 +385,7 @@ pub const fn handle_alloc_error(layout: Layout) -> ! { } #[cfg(not(feature = "panic_immediate_abort"))] + #[cfg_attr(not(bootstrap), allow(unused_unsafe))] // on bootstrap bump, remove unsafe block unsafe { core::intrinsics::const_eval_select((layout,), ct_error, rt_error) } diff --git a/library/core/src/ffi/c_str.rs b/library/core/src/ffi/c_str.rs index 20186a2de0fd8..0825281090cd1 100644 --- a/library/core/src/ffi/c_str.rs +++ b/library/core/src/ffi/c_str.rs @@ -428,10 +428,13 @@ impl CStr { unsafe { &*(bytes as *const [u8] as *const CStr) } } + #[cfg_attr(not(bootstrap), allow(unused_unsafe))] // on bootstrap bump, remove unsafe block // SAFETY: The const and runtime versions have identical behavior // unless the safety contract of `from_bytes_with_nul_unchecked` is // violated, which is UB. - unsafe { intrinsics::const_eval_select((bytes,), const_impl, rt_impl) } + unsafe { + intrinsics::const_eval_select((bytes,), const_impl, rt_impl) + } } /// Returns the inner pointer to this C string. @@ -719,6 +722,9 @@ const unsafe fn const_strlen(ptr: *const c_char) -> usize { unsafe { strlen(s) } } + #[cfg_attr(not(bootstrap), allow(unused_unsafe))] // on bootstrap bump, remove unsafe block // SAFETY: the two functions always provide equivalent functionality - unsafe { intrinsics::const_eval_select((ptr,), strlen_ct, strlen_rt) } + unsafe { + intrinsics::const_eval_select((ptr,), strlen_ct, strlen_rt) + } } diff --git a/library/core/src/intrinsics.rs b/library/core/src/intrinsics.rs index 96e667d63c5f3..9f8f99420f128 100644 --- a/library/core/src/intrinsics.rs +++ b/library/core/src/intrinsics.rs @@ -2515,6 +2515,8 @@ extern "rust-intrinsic" { /// intrinsic will be replaced with a call to `called_in_const`. It gets /// replaced with a call to `called_at_rt` otherwise. /// + /// This function is safe to call, but note the stability concerns below. + /// /// # Type Requirements /// /// The two functions must be both function items. They cannot be function @@ -2524,45 +2526,47 @@ extern "rust-intrinsic" { /// the two functions, therefore, both functions must accept the same type of /// arguments. Both functions must return RET. /// - /// # Safety + /// # Stability concerns /// - /// The two functions must behave observably equivalent. Safe code in other - /// crates may assume that calling a `const fn` at compile-time and at run-time - /// produces the same result. A function that produces a different result when - /// evaluated at run-time, or has any other observable side-effects, is - /// *unsound*. + /// Rust has not yet decided that `const fn` are allowed to tell whether + /// they run at compile-time or at runtime. Therefore, when using this + /// intrinsic anywhere that can be reached from stable, it is crucial that + /// the end-to-end behavior of the stable `const fn` is the same for both + /// modes of execution. (Here, Undefined Behavior is considerd "the same" + /// as any other behavior, so if the function exhibits UB at runtime then + /// it may do whatever it wants at compile-time.) /// /// Here is an example of how this could cause a problem: /// ```no_run /// #![feature(const_eval_select)] /// #![feature(core_intrinsics)] /// # #![allow(internal_features)] - /// use std::hint::unreachable_unchecked; + /// # #![cfg_attr(bootstrap, allow(unused))] /// use std::intrinsics::const_eval_select; /// - /// // Crate A + /// // Standard library + /// # #[cfg(not(bootstrap))] /// pub const fn inconsistent() -> i32 { /// fn runtime() -> i32 { 1 } /// const fn compiletime() -> i32 { 2 } /// - /// unsafe { - // // ⚠ This code violates the required equivalence of `compiletime` - /// // and `runtime`. - /// const_eval_select((), compiletime, runtime) - /// } + // // ⚠ This code violates the required equivalence of `compiletime` + /// // and `runtime`. + /// const_eval_select((), compiletime, runtime) /// } + /// # #[cfg(bootstrap)] + /// # pub const fn inconsistent() -> i32 { 0 } /// - /// // Crate B + /// // User Crate /// const X: i32 = inconsistent(); /// let x = inconsistent(); - /// if x != X { unsafe { unreachable_unchecked(); }} + /// assert_eq!(x, X); /// ``` /// - /// This code causes Undefined Behavior when being run, since the - /// `unreachable_unchecked` is actually being reached. The bug is in *crate A*, - /// which violates the principle that a `const fn` must behave the same at - /// compile-time and at run-time. The unsafe code in crate B is fine. + /// Currently such an assertion would always succeed; until Rust decides + /// otherwise, that principle should not be violated. #[rustc_const_unstable(feature = "const_eval_select", issue = "none")] + #[cfg_attr(not(bootstrap), rustc_safe_intrinsic)] pub fn const_eval_select( arg: ARG, called_in_const: F, diff --git a/library/core/src/num/f32.rs b/library/core/src/num/f32.rs index 47e16018a47ce..abdcb7099cacb 100644 --- a/library/core/src/num/f32.rs +++ b/library/core/src/num/f32.rs @@ -1153,8 +1153,11 @@ impl f32 { // Stability concerns. unsafe { mem::transmute(x) } } + #[cfg_attr(not(bootstrap), allow(unused_unsafe))] // on bootstrap bump, remove unsafe block // SAFETY: We use internal implementations that either always work or fail at compile time. - unsafe { intrinsics::const_eval_select((self,), ct_f32_to_u32, rt_f32_to_u32) } + unsafe { + intrinsics::const_eval_select((self,), ct_f32_to_u32, rt_f32_to_u32) + } } /// Raw transmutation from `u32`. @@ -1245,8 +1248,11 @@ impl f32 { // Stability concerns. unsafe { mem::transmute(x) } } + #[cfg_attr(not(bootstrap), allow(unused_unsafe))] // on bootstrap bump, remove unsafe block // SAFETY: We use internal implementations that either always work or fail at compile time. - unsafe { intrinsics::const_eval_select((v,), ct_u32_to_f32, rt_u32_to_f32) } + unsafe { + intrinsics::const_eval_select((v,), ct_u32_to_f32, rt_u32_to_f32) + } } /// Return the memory representation of this floating point number as a byte array in diff --git a/library/core/src/num/f64.rs b/library/core/src/num/f64.rs index cd69e758d28da..f4d2a4f216734 100644 --- a/library/core/src/num/f64.rs +++ b/library/core/src/num/f64.rs @@ -1146,8 +1146,11 @@ impl f64 { // Stability concerns. unsafe { mem::transmute::(rt) } } + #[cfg_attr(not(bootstrap), allow(unused_unsafe))] // on bootstrap bump, remove unsafe block // SAFETY: We use internal implementations that either always work or fail at compile time. - unsafe { intrinsics::const_eval_select((self,), ct_f64_to_u64, rt_f64_to_u64) } + unsafe { + intrinsics::const_eval_select((self,), ct_f64_to_u64, rt_f64_to_u64) + } } /// Raw transmutation from `u64`. @@ -1243,8 +1246,11 @@ impl f64 { // Stability concerns. unsafe { mem::transmute::(rt) } } + #[cfg_attr(not(bootstrap), allow(unused_unsafe))] // on bootstrap bump, remove unsafe block // SAFETY: We use internal implementations that either always work or fail at compile time. - unsafe { intrinsics::const_eval_select((v,), ct_u64_to_f64, rt_u64_to_f64) } + unsafe { + intrinsics::const_eval_select((v,), ct_u64_to_f64, rt_u64_to_f64) + } } /// Return the memory representation of this floating point number as a byte array in diff --git a/library/core/src/panicking.rs b/library/core/src/panicking.rs index 0819334b600b6..9e1184c8f5b88 100644 --- a/library/core/src/panicking.rs +++ b/library/core/src/panicking.rs @@ -117,6 +117,7 @@ pub const fn panic_nounwind_fmt(fmt: fmt::Arguments<'_>, force_no_backtrace: boo panic_fmt(fmt); } + #[cfg_attr(not(bootstrap), allow(unused_unsafe))] // on bootstrap bump, remove unsafe block // SAFETY: const panic does not care about unwinding unsafe { super::intrinsics::const_eval_select((fmt, force_no_backtrace), comptime, runtime); diff --git a/library/core/src/ptr/const_ptr.rs b/library/core/src/ptr/const_ptr.rs index 85a56d37ab75c..f2566fe9bccae 100644 --- a/library/core/src/ptr/const_ptr.rs +++ b/library/core/src/ptr/const_ptr.rs @@ -48,8 +48,11 @@ impl *const T { } } + #[cfg_attr(not(bootstrap), allow(unused_unsafe))] // on bootstrap bump, remove unsafe block // SAFETY: The two versions are equivalent at runtime. - unsafe { const_eval_select((self as *const u8,), const_impl, runtime_impl) } + unsafe { + const_eval_select((self as *const u8,), const_impl, runtime_impl) + } } /// Casts to a pointer of another type. @@ -806,6 +809,7 @@ impl *const T { where T: Sized, { + #[cfg_attr(not(bootstrap), allow(unused_unsafe))] // on bootstrap bump, remove unsafe block // SAFETY: The comparison has no side-effects, and the intrinsic // does this check internally in the CTFE implementation. unsafe { @@ -1623,8 +1627,11 @@ impl *const T { ptr.align_offset(align) == 0 } + #[cfg_attr(not(bootstrap), allow(unused_unsafe))] // on bootstrap bump, remove unsafe block // SAFETY: The two versions are equivalent at runtime. - unsafe { const_eval_select((self.cast::<()>(), align), const_impl, runtime_impl) } + unsafe { + const_eval_select((self.cast::<()>(), align), const_impl, runtime_impl) + } } } diff --git a/library/core/src/ptr/mod.rs b/library/core/src/ptr/mod.rs index fc5b08c9801a8..c0a3bbdb7048e 100644 --- a/library/core/src/ptr/mod.rs +++ b/library/core/src/ptr/mod.rs @@ -991,6 +991,7 @@ pub const unsafe fn swap_nonoverlapping(x: *mut T, y: *mut T, count: usize) { }; } + #[cfg_attr(not(bootstrap), allow(unused_unsafe))] // on bootstrap bump, remove unsafe block // SAFETY: the caller must guarantee that `x` and `y` are // valid for writes and properly aligned. unsafe { diff --git a/library/core/src/ptr/mut_ptr.rs b/library/core/src/ptr/mut_ptr.rs index 28ba26f5c16c4..8b0b22a02f2ba 100644 --- a/library/core/src/ptr/mut_ptr.rs +++ b/library/core/src/ptr/mut_ptr.rs @@ -48,8 +48,11 @@ impl *mut T { } } + #[cfg_attr(not(bootstrap), allow(unused_unsafe))] // on bootstrap bump, remove unsafe block // SAFETY: The two versions are equivalent at runtime. - unsafe { const_eval_select((self as *mut u8,), const_impl, runtime_impl) } + unsafe { + const_eval_select((self as *mut u8,), const_impl, runtime_impl) + } } /// Casts to a pointer of another type. @@ -1896,8 +1899,11 @@ impl *mut T { ptr.align_offset(align) == 0 } + #[cfg_attr(not(bootstrap), allow(unused_unsafe))] // on bootstrap bump, remove unsafe block // SAFETY: The two versions are equivalent at runtime. - unsafe { const_eval_select((self.cast::<()>(), align), const_impl, runtime_impl) } + unsafe { + const_eval_select((self.cast::<()>(), align), const_impl, runtime_impl) + } } } diff --git a/library/core/src/slice/index.rs b/library/core/src/slice/index.rs index 0e2d45c4ada6d..c771ea7047242 100644 --- a/library/core/src/slice/index.rs +++ b/library/core/src/slice/index.rs @@ -35,6 +35,7 @@ where #[track_caller] #[rustc_const_unstable(feature = "const_slice_index", issue = "none")] const fn slice_start_index_len_fail(index: usize, len: usize) -> ! { + #[cfg_attr(not(bootstrap), allow(unused_unsafe))] // on bootstrap bump, remove unsafe block // SAFETY: we are just panicking here unsafe { const_eval_select( @@ -63,6 +64,7 @@ const fn slice_start_index_len_fail_ct(_: usize, _: usize) -> ! { #[track_caller] #[rustc_const_unstable(feature = "const_slice_index", issue = "none")] const fn slice_end_index_len_fail(index: usize, len: usize) -> ! { + #[cfg_attr(not(bootstrap), allow(unused_unsafe))] // on bootstrap bump, remove unsafe block // SAFETY: we are just panicking here unsafe { const_eval_select((index, len), slice_end_index_len_fail_ct, slice_end_index_len_fail_rt) @@ -87,8 +89,11 @@ const fn slice_end_index_len_fail_ct(_: usize, _: usize) -> ! { #[track_caller] #[rustc_const_unstable(feature = "const_slice_index", issue = "none")] const fn slice_index_order_fail(index: usize, end: usize) -> ! { + #[cfg_attr(not(bootstrap), allow(unused_unsafe))] // on bootstrap bump, remove unsafe block // SAFETY: we are just panicking here - unsafe { const_eval_select((index, end), slice_index_order_fail_ct, slice_index_order_fail_rt) } + unsafe { + const_eval_select((index, end), slice_index_order_fail_ct, slice_index_order_fail_rt) + } } // FIXME const-hack diff --git a/library/core/src/str/mod.rs b/library/core/src/str/mod.rs index f965a50058b1b..4943bbc45d07a 100644 --- a/library/core/src/str/mod.rs +++ b/library/core/src/str/mod.rs @@ -86,6 +86,7 @@ use iter::{MatchesInternal, SplitNInternal}; #[rustc_allow_const_fn_unstable(const_eval_select)] #[cfg(not(feature = "panic_immediate_abort"))] const fn slice_error_fail(s: &str, begin: usize, end: usize) -> ! { + #[cfg_attr(not(bootstrap), allow(unused_unsafe))] // on bootstrap bump, remove unsafe block // SAFETY: panics for both branches unsafe { crate::intrinsics::const_eval_select( diff --git a/tests/ui/intrinsics/const-eval-select-backtrace.rs b/tests/ui/intrinsics/const-eval-select-backtrace.rs index d6b1c865bdfc7..ea4374f5d328e 100644 --- a/tests/ui/intrinsics/const-eval-select-backtrace.rs +++ b/tests/ui/intrinsics/const-eval-select-backtrace.rs @@ -12,8 +12,5 @@ fn uhoh() { const fn c() {} fn main() { - // safety: this is unsound and just used to test - unsafe { - std::intrinsics::const_eval_select((), c, uhoh); - } + std::intrinsics::const_eval_select((), c, uhoh); } diff --git a/tests/ui/intrinsics/const-eval-select-backtrace.run.stderr b/tests/ui/intrinsics/const-eval-select-backtrace.run.stderr index 3f196bd8abc39..8f38d54146b88 100644 --- a/tests/ui/intrinsics/const-eval-select-backtrace.run.stderr +++ b/tests/ui/intrinsics/const-eval-select-backtrace.run.stderr @@ -1,3 +1,3 @@ -thread 'main' panicked at $DIR/const-eval-select-backtrace.rs:17:9: +thread 'main' panicked at $DIR/const-eval-select-backtrace.rs:15:5: Aaah! note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace diff --git a/tests/ui/intrinsics/const-eval-select-stability.rs b/tests/ui/intrinsics/const-eval-select-stability.rs index f9554decec16b..575bc0cadda1b 100644 --- a/tests/ui/intrinsics/const-eval-select-stability.rs +++ b/tests/ui/intrinsics/const-eval-select-stability.rs @@ -13,7 +13,7 @@ const fn nothing(){} #[stable(since = "1.0", feature = "hey")] #[rustc_const_stable(since = "1.0", feature = "const_hey")] -pub const unsafe fn hey() { +pub const fn hey() { const_eval_select((), nothing, log); //~^ ERROR `const_eval_select` is not yet stable as a const fn } diff --git a/tests/ui/intrinsics/const-eval-select-x86_64.rs b/tests/ui/intrinsics/const-eval-select-x86_64.rs index 5ba7a443d0bb9..c17be2ddaca39 100644 --- a/tests/ui/intrinsics/const-eval-select-x86_64.rs +++ b/tests/ui/intrinsics/const-eval-select-x86_64.rs @@ -22,9 +22,7 @@ fn eq_rt(x: [i32; 4], y: [i32; 4]) -> bool { } const fn eq(x: [i32; 4], y: [i32; 4]) -> bool { - unsafe { - const_eval_select((x, y), eq_ct, eq_rt) - } + const_eval_select((x, y), eq_ct, eq_rt) } fn main() { diff --git a/tests/ui/intrinsics/const-eval-select.rs b/tests/ui/intrinsics/const-eval-select.rs index 353105cb0a163..66d83585fcec8 100644 --- a/tests/ui/intrinsics/const-eval-select.rs +++ b/tests/ui/intrinsics/const-eval-select.rs @@ -13,9 +13,9 @@ fn no() -> bool { false } -// not a sound use case; testing only +// not allowed on stable; testing only const fn is_const_eval() -> bool { - unsafe { const_eval_select((), yes, no) } + const_eval_select((), yes, no) } fn main() { diff --git a/tests/ui/rfcs/rfc-2632-const-trait-impl/effects/minicore.rs b/tests/ui/rfcs/rfc-2632-const-trait-impl/effects/minicore.rs index 5ee38078a2950..1b380c989fa4a 100644 --- a/tests/ui/rfcs/rfc-2632-const-trait-impl/effects/minicore.rs +++ b/tests/ui/rfcs/rfc-2632-const-trait-impl/effects/minicore.rs @@ -511,6 +511,7 @@ const fn drop(_: T) {} extern "rust-intrinsic" { #[rustc_const_stable(feature = "const_eval_select", since = "1.0.0")] + #[rustc_safe_intrinsic] fn const_eval_select( arg: ARG, called_in_const: F, @@ -525,5 +526,5 @@ fn test_const_eval_select() { const fn const_fn() {} fn rt_fn() {} - unsafe { const_eval_select((), const_fn, rt_fn); } + const_eval_select((), const_fn, rt_fn); } From d858809ca5f4a945377c47eb18fc7ed9202da24c Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 2 Mar 2024 19:03:50 +0100 Subject: [PATCH 03/12] typo Co-authored-by: bjorn3 <17426603+bjorn3@users.noreply.github.com> --- library/core/src/intrinsics.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/core/src/intrinsics.rs b/library/core/src/intrinsics.rs index 9f8f99420f128..5e93f495ddf1d 100644 --- a/library/core/src/intrinsics.rs +++ b/library/core/src/intrinsics.rs @@ -2532,7 +2532,7 @@ extern "rust-intrinsic" { /// they run at compile-time or at runtime. Therefore, when using this /// intrinsic anywhere that can be reached from stable, it is crucial that /// the end-to-end behavior of the stable `const fn` is the same for both - /// modes of execution. (Here, Undefined Behavior is considerd "the same" + /// modes of execution. (Here, Undefined Behavior is considered "the same" /// as any other behavior, so if the function exhibits UB at runtime then /// it may do whatever it wants at compile-time.) /// From 4dbd2562b4e2c464218a2949442becad34d55955 Mon Sep 17 00:00:00 2001 From: Christopher Durham Date: Tue, 13 Feb 2024 22:24:29 -0500 Subject: [PATCH 04/12] Explain use of display adapters --- library/core/src/fmt/mod.rs | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/library/core/src/fmt/mod.rs b/library/core/src/fmt/mod.rs index e1b7b46a1ed2f..f82e8a0d60bff 100644 --- a/library/core/src/fmt/mod.rs +++ b/library/core/src/fmt/mod.rs @@ -633,6 +633,23 @@ pub use macros::Debug; /// [tostring]: ../../std/string/trait.ToString.html /// [tostring_function]: ../../std/string/trait.ToString.html#tymethod.to_string /// +/// # Internationalization +/// +/// Because a type only has one `Display` implementation, it is often preferable +/// to only implement `Display` when there is a single most "obvious" way that +/// values can be formatted as text. This could mean formatting according to the +/// "invariant" culture and "undefined" locale, or it could mean that the type +/// display is designed for a specific culture/locale, such as developer logs. +/// +/// If not all values have a justifiably canonical textual format or if you want +/// to support alternative formats not covered by the standard set of possible +/// [formatting traits], the most flexible approach is display adapters: methods +/// like [`str::escape_default`] or [`Path::display`] which create a wrapper +/// implementing `Display` to output the specific display format. +/// +/// [formatting traits]: ../../std/fmt/index.html#formatting-traits +/// [`Path::display`]: ../../std/path/struct.Path.html#method.display +/// /// # Examples /// /// Implementing `Display` on a type: From 215a4b6c2d71f2f7dd6254fcecbfc15bbc6fb392 Mon Sep 17 00:00:00 2001 From: Christopher Durham Date: Mon, 4 Mar 2024 19:49:45 -0500 Subject: [PATCH 05/12] doc wording improvements Co-authored-by: Simon Farnsworth --- library/core/src/fmt/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/core/src/fmt/mod.rs b/library/core/src/fmt/mod.rs index f82e8a0d60bff..4d73c8c3f99a9 100644 --- a/library/core/src/fmt/mod.rs +++ b/library/core/src/fmt/mod.rs @@ -635,7 +635,7 @@ pub use macros::Debug; /// /// # Internationalization /// -/// Because a type only has one `Display` implementation, it is often preferable +/// Because a type can only have one `Display` implementation, it is often preferable /// to only implement `Display` when there is a single most "obvious" way that /// values can be formatted as text. This could mean formatting according to the /// "invariant" culture and "undefined" locale, or it could mean that the type From b5d7da878fa329b37f06d37c6a1a4d049953e7a0 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Fri, 1 Mar 2024 13:00:06 +1100 Subject: [PATCH 06/12] Decouple `DummyAstNode` and `DummyResult`. They are two different ways of creating dummy results, with two different purposes. Their implementations are separate except for crates, where `DummyResult` depends on `DummyAstNode`. This commit removes that dependency, so they are now fully separate. It also expands the comment on `DummyAstNode`. --- compiler/rustc_ast/src/mut_visit.rs | 5 ++++- compiler/rustc_expand/src/base.rs | 14 ++++++++++++-- 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/compiler/rustc_ast/src/mut_visit.rs b/compiler/rustc_ast/src/mut_visit.rs index bbbb34f7f2f48..da4328d8ce267 100644 --- a/compiler/rustc_ast/src/mut_visit.rs +++ b/compiler/rustc_ast/src/mut_visit.rs @@ -1604,7 +1604,10 @@ pub fn noop_visit_capture_by(capture_by: &mut CaptureBy, vis: &mu } } -/// Some value for the AST node that is valid but possibly meaningless. +/// Some value for the AST node that is valid but possibly meaningless. Similar +/// to `Default` but not intended for wide use. The value will never be used +/// meaningfully, it exists just to support unwinding in `visit_clobber` in the +/// case where its closure panics. pub trait DummyAstNode { fn dummy() -> Self; } diff --git a/compiler/rustc_expand/src/base.rs b/compiler/rustc_expand/src/base.rs index 485f0e7e46deb..d8ac4a540378e 100644 --- a/compiler/rustc_expand/src/base.rs +++ b/compiler/rustc_expand/src/base.rs @@ -4,7 +4,6 @@ use crate::expand::{self, AstFragment, Invocation}; use crate::module::DirOwnership; use rustc_ast::attr::MarkedAttrs; -use rustc_ast::mut_visit::DummyAstNode; use rustc_ast::ptr::P; use rustc_ast::token::{self, Nonterminal}; use rustc_ast::tokenstream::TokenStream; @@ -582,6 +581,17 @@ impl DummyResult { tokens: None, }) } + + /// A plain dummy crate. + pub fn raw_crate() -> ast::Crate { + ast::Crate { + attrs: Default::default(), + items: Default::default(), + spans: Default::default(), + id: ast::DUMMY_NODE_ID, + is_placeholder: Default::default(), + } + } } impl MacResult for DummyResult { @@ -650,7 +660,7 @@ impl MacResult for DummyResult { } fn make_crate(self: Box) -> Option { - Some(DummyAstNode::dummy()) + Some(DummyResult::raw_crate()) } } From a9dff2d931ad6844344c1b81f343ccdd1f4abdd9 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Sat, 2 Mar 2024 08:24:45 +1100 Subject: [PATCH 07/12] Remove unused `impl DummyAstNode for Block`. --- compiler/rustc_ast/src/mut_visit.rs | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/compiler/rustc_ast/src/mut_visit.rs b/compiler/rustc_ast/src/mut_visit.rs index da4328d8ce267..c517c6138ceb6 100644 --- a/compiler/rustc_ast/src/mut_visit.rs +++ b/compiler/rustc_ast/src/mut_visit.rs @@ -1682,19 +1682,6 @@ impl DummyAstNode for Stmt { } } -impl DummyAstNode for Block { - fn dummy() -> Self { - Block { - stmts: Default::default(), - id: DUMMY_NODE_ID, - rules: BlockCheckMode::Default, - span: Default::default(), - tokens: Default::default(), - could_be_bare_literal: Default::default(), - } - } -} - impl DummyAstNode for Crate { fn dummy() -> Self { Crate { From c98be3254fdd8bde32489e4a1a9b0d88185a506f Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Wed, 28 Feb 2024 12:20:04 +0000 Subject: [PATCH 08/12] Stop using Bubble in coherence and instead emulate it with an intercrate check --- compiler/rustc_infer/src/infer/opaque_types.rs | 15 +++++++++++++-- .../rustc_trait_selection/src/traits/coherence.rs | 2 -- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/compiler/rustc_infer/src/infer/opaque_types.rs b/compiler/rustc_infer/src/infer/opaque_types.rs index 7a789a1b41bca..70a07f06b1f23 100644 --- a/compiler/rustc_infer/src/infer/opaque_types.rs +++ b/compiler/rustc_infer/src/infer/opaque_types.rs @@ -101,6 +101,15 @@ impl<'tcx> InferCtxt<'tcx> { let process = |a: Ty<'tcx>, b: Ty<'tcx>| match *a.kind() { ty::Alias(ty::Opaque, ty::AliasTy { def_id, args, .. }) if def_id.is_local() => { let def_id = def_id.expect_local(); + if self.intercrate { + // See comment on `insert_hidden_type` for why this is sufficient in coherence + return Some(self.register_hidden_type( + OpaqueTypeKey { def_id, args }, + cause.clone(), + param_env, + b, + )); + } match self.defining_use_anchor { DefiningAnchor::Bind(_) => { // Check that this is `impl Trait` type is @@ -142,8 +151,10 @@ impl<'tcx> InferCtxt<'tcx> { } } DefiningAnchor::Bubble => {} - DefiningAnchor::Error => return None, - }; + DefiningAnchor::Error => { + return None; + } + } if let ty::Alias(ty::Opaque, ty::AliasTy { def_id: b_def_id, .. }) = *b.kind() { // We could accept this, but there are various ways to handle this situation, and we don't // want to make a decision on it right now. Likely this case is so super rare anyway, that diff --git a/compiler/rustc_trait_selection/src/traits/coherence.rs b/compiler/rustc_trait_selection/src/traits/coherence.rs index 68111c4cc1fdc..63962c59ff2ea 100644 --- a/compiler/rustc_trait_selection/src/traits/coherence.rs +++ b/compiler/rustc_trait_selection/src/traits/coherence.rs @@ -26,7 +26,6 @@ use rustc_infer::traits::{util, FulfillmentErrorCode, TraitEngine, TraitEngineEx use rustc_middle::traits::query::NoSolution; use rustc_middle::traits::solve::{CandidateSource, Certainty, Goal}; use rustc_middle::traits::specialization_graph::OverlapMode; -use rustc_middle::traits::DefiningAnchor; use rustc_middle::ty::fast_reject::{DeepRejectCtxt, TreatParams}; use rustc_middle::ty::visit::{TypeVisitable, TypeVisitableExt}; use rustc_middle::ty::{self, Ty, TyCtxt, TypeSuperVisitable, TypeVisitor}; @@ -207,7 +206,6 @@ fn overlap<'tcx>( let infcx = tcx .infer_ctxt() - .with_opaque_type_inference(DefiningAnchor::Bubble) .skip_leak_check(skip_leak_check.is_yes()) .intercrate(true) .with_next_trait_solver(tcx.next_trait_solver_in_coherence()) From 960dd38abe18f7da6a08866736ba77aa8259de33 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 5 Mar 2024 09:33:55 +0100 Subject: [PATCH 09/12] will_wake tests fail on Miri and that is expected --- library/alloc/tests/task.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/library/alloc/tests/task.rs b/library/alloc/tests/task.rs index 0f8d9218980e9..034039a1eae9d 100644 --- a/library/alloc/tests/task.rs +++ b/library/alloc/tests/task.rs @@ -4,6 +4,7 @@ use alloc::task::{LocalWake, Wake}; use core::task::{LocalWaker, Waker}; #[test] +#[cfg_attr(miri, should_panic)] // `will_wake` doesn't guarantee that this test will work, and indeed on Miri it fails fn test_waker_will_wake_clone() { struct NoopWaker; @@ -19,6 +20,7 @@ fn test_waker_will_wake_clone() { } #[test] +#[cfg_attr(miri, should_panic)] // `will_wake` doesn't guarantee that this test will work, and indeed on Miri it fails fn test_local_waker_will_wake_clone() { struct NoopWaker; From 8dd126d84791d674c688164c9341b5c81b852335 Mon Sep 17 00:00:00 2001 From: surechen Date: Tue, 5 Mar 2024 15:46:49 +0800 Subject: [PATCH 10/12] Change some attributes to only_local. Modified according to https://github.com/rust-lang/compiler-team/issues/505. --- compiler/rustc_feature/src/builtin_attrs.rs | 39 ++++++++++++++------- 1 file changed, 26 insertions(+), 13 deletions(-) diff --git a/compiler/rustc_feature/src/builtin_attrs.rs b/compiler/rustc_feature/src/builtin_attrs.rs index 10867ae230a8c..b291d22c0e477 100644 --- a/compiler/rustc_feature/src/builtin_attrs.rs +++ b/compiler/rustc_feature/src/builtin_attrs.rs @@ -709,7 +709,9 @@ pub const BUILTIN_ATTRIBUTES: &[BuiltinAttribute] = &[ // Internal attributes, Const related: // ========================================================================== - rustc_attr!(rustc_promotable, Normal, template!(Word), WarnFollowing, IMPL_DETAIL), + rustc_attr!( + rustc_promotable, Normal, template!(Word), WarnFollowing, + @only_local: true, IMPL_DETAIL), rustc_attr!( rustc_legacy_const_generics, Normal, template!(List: "N"), ErrorFollowing, INTERNAL_UNSTABLE @@ -784,7 +786,7 @@ pub const BUILTIN_ATTRIBUTES: &[BuiltinAttribute] = &[ the given type by annotating all impl items with #[rustc_allow_incoherent_impl]." ), rustc_attr!( - rustc_box, AttributeType::Normal, template!(Word), ErrorFollowing, + rustc_box, AttributeType::Normal, template!(Word), ErrorFollowing, @only_local: true, "#[rustc_box] allows creating boxes \ and it is only intended to be used in `alloc`." ), @@ -806,11 +808,11 @@ pub const BUILTIN_ATTRIBUTES: &[BuiltinAttribute] = &[ gated!( // Used in resolve: prelude_import, Normal, template!(Word), WarnFollowing, - "`#[prelude_import]` is for use by rustc only", + @only_local: true, "`#[prelude_import]` is for use by rustc only", ), gated!( - rustc_paren_sugar, Normal, template!(Word), WarnFollowing, unboxed_closures, - "unboxed_closures are still evolving", + rustc_paren_sugar, Normal, template!(Word), WarnFollowing, @only_local: true, + unboxed_closures, "unboxed_closures are still evolving", ), rustc_attr!( rustc_inherit_overflow_checks, Normal, template!(Word), WarnFollowing, @only_local: true, @@ -826,27 +828,31 @@ pub const BUILTIN_ATTRIBUTES: &[BuiltinAttribute] = &[ ), rustc_attr!( rustc_test_marker, Normal, template!(NameValueStr: "name"), WarnFollowing, - "the `#[rustc_test_marker]` attribute is used internally to track tests", + @only_local: true, "the `#[rustc_test_marker]` attribute is used internally to track tests", ), rustc_attr!( - rustc_unsafe_specialization_marker, Normal, template!(Word), WarnFollowing, + rustc_unsafe_specialization_marker, Normal, template!(Word), + WarnFollowing, @only_local: true, "the `#[rustc_unsafe_specialization_marker]` attribute is used to check specializations" ), rustc_attr!( - rustc_specialization_trait, Normal, template!(Word), WarnFollowing, + rustc_specialization_trait, Normal, template!(Word), + WarnFollowing, @only_local: true, "the `#[rustc_specialization_trait]` attribute is used to check specializations" ), rustc_attr!( - rustc_main, Normal, template!(Word), WarnFollowing, + rustc_main, Normal, template!(Word), WarnFollowing, @only_local: true, "the `#[rustc_main]` attribute is used internally to specify test entry point function", ), rustc_attr!( - rustc_skip_array_during_method_dispatch, Normal, template!(Word), WarnFollowing, + rustc_skip_array_during_method_dispatch, Normal, template!(Word), + WarnFollowing, @only_local: true, "the `#[rustc_skip_array_during_method_dispatch]` attribute is used to exclude a trait \ from method dispatch when the receiver is an array, for compatibility in editions < 2021." ), rustc_attr!( - rustc_must_implement_one_of, Normal, template!(List: "function1, function2, ..."), ErrorFollowing, + rustc_must_implement_one_of, Normal, template!(List: "function1, function2, ..."), + ErrorFollowing, @only_local: true, "the `#[rustc_must_implement_one_of]` attribute is used to change minimal complete \ definition of a trait, it's currently in experimental form and should be changed before \ being exposed outside of the std" @@ -857,6 +863,7 @@ pub const BUILTIN_ATTRIBUTES: &[BuiltinAttribute] = &[ ), rustc_attr!( rustc_safe_intrinsic, Normal, template!(Word), WarnFollowing, + @only_local: true, "the `#[rustc_safe_intrinsic]` attribute is used internally to mark intrinsics as safe" ), rustc_attr!( @@ -873,8 +880,14 @@ pub const BUILTIN_ATTRIBUTES: &[BuiltinAttribute] = &[ // ========================================================================== rustc_attr!(TEST, rustc_effective_visibility, Normal, template!(Word), WarnFollowing), - rustc_attr!(TEST, rustc_outlives, Normal, template!(Word), WarnFollowing), - rustc_attr!(TEST, rustc_capture_analysis, Normal, template!(Word), WarnFollowing), + rustc_attr!( + TEST, rustc_outlives, Normal, template!(Word), + WarnFollowing, @only_local: true + ), + rustc_attr!( + TEST, rustc_capture_analysis, Normal, template!(Word), + WarnFollowing, @only_local: true + ), rustc_attr!(TEST, rustc_insignificant_dtor, Normal, template!(Word), WarnFollowing), rustc_attr!(TEST, rustc_strict_coherence, Normal, template!(Word), WarnFollowing), rustc_attr!(TEST, rustc_variance, Normal, template!(Word), WarnFollowing), From f391c0793b443d30ef8c4d4228550439d4dbfead Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 5 Mar 2024 11:32:03 +0100 Subject: [PATCH 11/12] only set noalias on Box with the global allocator --- compiler/rustc_abi/src/lib.rs | 7 +++++-- .../example/mini_core.rs | 7 +++++-- .../rustc_codegen_cranelift/src/unsize.rs | 4 ---- .../rustc_codegen_gcc/example/mini_core.rs | 1 + .../src/debuginfo/metadata.rs | 10 ++++++--- compiler/rustc_codegen_ssa/src/mir/operand.rs | 1 + .../rustc_const_eval/src/interpret/place.rs | 1 + .../src/interpret/terminator.rs | 10 ++------- compiler/rustc_hir/src/lang_items.rs | 2 ++ compiler/rustc_middle/src/ty/layout.rs | 20 +++++++++++------- compiler/rustc_middle/src/ty/sty.rs | 21 +++++++++++++++++++ compiler/rustc_span/src/symbol.rs | 1 + compiler/rustc_ty_utils/src/abi.rs | 2 +- library/alloc/src/alloc.rs | 2 ++ library/alloc/src/boxed.rs | 3 +++ tests/codegen/function-arguments.rs | 10 +++++++++ tests/ui/abi/compatibility.rs | 1 + 17 files changed, 75 insertions(+), 28 deletions(-) diff --git a/compiler/rustc_abi/src/lib.rs b/compiler/rustc_abi/src/lib.rs index 34529e0a086f3..4f2b9d0ef50d9 100644 --- a/compiler/rustc_abi/src/lib.rs +++ b/compiler/rustc_abi/src/lib.rs @@ -1612,8 +1612,9 @@ pub enum PointerKind { SharedRef { frozen: bool }, /// Mutable reference. `unpin` indicates the absence of any pinned data. MutableRef { unpin: bool }, - /// Box. `unpin` indicates the absence of any pinned data. - Box { unpin: bool }, + /// Box. `unpin` indicates the absence of any pinned data. `global` indicates whether this box + /// uses the global allocator or a custom one. + Box { unpin: bool, global: bool }, } /// Note that this information is advisory only, and backends are free to ignore it. @@ -1622,6 +1623,8 @@ pub enum PointerKind { pub struct PointeeInfo { pub size: Size, pub align: Align, + /// If this is `None`, then this is a raw pointer, so size and alignment are not guaranteed to + /// be reliable. pub safe: Option, } diff --git a/compiler/rustc_codegen_cranelift/example/mini_core.rs b/compiler/rustc_codegen_cranelift/example/mini_core.rs index a79909ce0c878..67a0d0dabea94 100644 --- a/compiler/rustc_codegen_cranelift/example/mini_core.rs +++ b/compiler/rustc_codegen_cranelift/example/mini_core.rs @@ -525,8 +525,11 @@ pub struct Unique { impl CoerceUnsized> for Unique where T: Unsize {} impl DispatchFromDyn> for Unique where T: Unsize {} +#[lang = "global_alloc_ty"] +pub struct Global; + #[lang = "owned_box"] -pub struct Box(Unique, A); +pub struct Box(Unique, A); impl, U: ?Sized> CoerceUnsized> for Box {} @@ -536,7 +539,7 @@ impl Box { let size = intrinsics::size_of::(); let ptr = libc::malloc(size); intrinsics::copy(&val as *const T as *const u8, ptr, size); - Box(Unique { pointer: NonNull(ptr as *const T), _marker: PhantomData }, ()) + Box(Unique { pointer: NonNull(ptr as *const T), _marker: PhantomData }, Global) } } } diff --git a/compiler/rustc_codegen_cranelift/src/unsize.rs b/compiler/rustc_codegen_cranelift/src/unsize.rs index acfa461a6f30b..7b61dc64cb1ae 100644 --- a/compiler/rustc_codegen_cranelift/src/unsize.rs +++ b/compiler/rustc_codegen_cranelift/src/unsize.rs @@ -74,10 +74,6 @@ fn unsize_ptr<'tcx>( | (&ty::RawPtr(ty::TypeAndMut { ty: a, .. }), &ty::RawPtr(ty::TypeAndMut { ty: b, .. })) => { (src, unsized_info(fx, *a, *b, old_info)) } - (&ty::Adt(def_a, _), &ty::Adt(def_b, _)) if def_a.is_box() && def_b.is_box() => { - let (a, b) = (src_layout.ty.boxed_ty(), dst_layout.ty.boxed_ty()); - (src, unsized_info(fx, a, b, old_info)) - } (&ty::Adt(def_a, _), &ty::Adt(def_b, _)) => { assert_eq!(def_a, def_b); diff --git a/compiler/rustc_codegen_gcc/example/mini_core.rs b/compiler/rustc_codegen_gcc/example/mini_core.rs index 230009741dc41..cc3d647c8c82f 100644 --- a/compiler/rustc_codegen_gcc/example/mini_core.rs +++ b/compiler/rustc_codegen_gcc/example/mini_core.rs @@ -472,6 +472,7 @@ pub trait Allocator { impl Allocator for () {} +#[lang = "global_alloc_ty"] pub struct Global; impl Allocator for Global {} diff --git a/compiler/rustc_codegen_llvm/src/debuginfo/metadata.rs b/compiler/rustc_codegen_llvm/src/debuginfo/metadata.rs index 1a5f9b4294754..660f164736775 100644 --- a/compiler/rustc_codegen_llvm/src/debuginfo/metadata.rs +++ b/compiler/rustc_codegen_llvm/src/debuginfo/metadata.rs @@ -454,9 +454,13 @@ pub fn type_di_node<'ll, 'tcx>(cx: &CodegenCx<'ll, 'tcx>, t: Ty<'tcx>) -> &'ll D ty::RawPtr(ty::TypeAndMut { ty: pointee_type, .. }) | ty::Ref(_, pointee_type, _) => { build_pointer_or_reference_di_node(cx, t, pointee_type, unique_type_id) } - // Box may have a non-1-ZST allocator A. In that case, we - // cannot treat Box as just an owned alias of `*mut T`. - ty::Adt(def, args) if def.is_box() && cx.layout_of(args.type_at(1)).is_1zst() => { + // Some `Box` are newtyped pointers, make debuginfo aware of that. + // Only works if the allocator argument is a 1-ZST and hence irrelevant for layout + // (or if there is no allocator argument). + ty::Adt(def, args) + if def.is_box() + && args.get(1).map_or(true, |arg| cx.layout_of(arg.expect_ty()).is_1zst()) => + { build_pointer_or_reference_di_node(cx, t, t.boxed_ty(), unique_type_id) } ty::FnDef(..) | ty::FnPtr(_) => build_subroutine_type_di_node(cx, unique_type_id), diff --git a/compiler/rustc_codegen_ssa/src/mir/operand.rs b/compiler/rustc_codegen_ssa/src/mir/operand.rs index 94eb37e78e07d..932926976b58d 100644 --- a/compiler/rustc_codegen_ssa/src/mir/operand.rs +++ b/compiler/rustc_codegen_ssa/src/mir/operand.rs @@ -204,6 +204,7 @@ impl<'a, 'tcx, V: CodegenObject> OperandRef<'tcx, V> { pub fn deref>(self, cx: &Cx) -> PlaceRef<'tcx, V> { if self.layout.ty.is_box() { + // Derefer should have removed all Box derefs bug!("dereferencing {:?} in codegen", self.layout.ty); } diff --git a/compiler/rustc_const_eval/src/interpret/place.rs b/compiler/rustc_const_eval/src/interpret/place.rs index 6e987784ff9ee..672008edfd36f 100644 --- a/compiler/rustc_const_eval/src/interpret/place.rs +++ b/compiler/rustc_const_eval/src/interpret/place.rs @@ -437,6 +437,7 @@ where trace!("deref to {} on {:?}", val.layout.ty, *val); if val.layout.ty.is_box() { + // Derefer should have removed all Box derefs bug!("dereferencing {}", val.layout.ty); } diff --git a/compiler/rustc_const_eval/src/interpret/terminator.rs b/compiler/rustc_const_eval/src/interpret/terminator.rs index e72ace8be3559..d29e69d753ebb 100644 --- a/compiler/rustc_const_eval/src/interpret/terminator.rs +++ b/compiler/rustc_const_eval/src/interpret/terminator.rs @@ -359,14 +359,8 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { Ok(Some(match ty.kind() { ty::Ref(_, ty, _) => *ty, ty::RawPtr(mt) => mt.ty, - // We should only accept `Box` with the default allocator. - // It's hard to test for that though so we accept every 1-ZST allocator. - ty::Adt(def, args) - if def.is_box() - && self.layout_of(args[1].expect_ty()).is_ok_and(|l| l.is_1zst()) => - { - args[0].expect_ty() - } + // We only accept `Box` with the default allocator. + _ if ty.is_box_global(*self.tcx) => ty.boxed_ty(), _ => return Ok(None), })) }; diff --git a/compiler/rustc_hir/src/lang_items.rs b/compiler/rustc_hir/src/lang_items.rs index 8a89a3b5faa5e..5118bf5c3b7ab 100644 --- a/compiler/rustc_hir/src/lang_items.rs +++ b/compiler/rustc_hir/src/lang_items.rs @@ -267,6 +267,8 @@ language_item_table! { EhCatchTypeinfo, sym::eh_catch_typeinfo, eh_catch_typeinfo, Target::Static, GenericRequirement::None; OwnedBox, sym::owned_box, owned_box, Target::Struct, GenericRequirement::Minimum(1); + GlobalAlloc, sym::global_alloc_ty, global_alloc_ty, Target::Struct, GenericRequirement::None; + // Experimental language item for Miri PtrUnique, sym::ptr_unique, ptr_unique, Target::Struct, GenericRequirement::Exact(1); diff --git a/compiler/rustc_middle/src/ty/layout.rs b/compiler/rustc_middle/src/ty/layout.rs index 5a0e2aa691d58..dda41f70f067a 100644 --- a/compiler/rustc_middle/src/ty/layout.rs +++ b/compiler/rustc_middle/src/ty/layout.rs @@ -969,6 +969,8 @@ where } } + /// Compute the information for the pointer stored at the given offset inside this type. + /// This will recurse into fields of ADTs to find the inner pointer. fn ty_and_layout_pointee_info_at( this: TyAndLayout<'tcx>, cx: &C, @@ -1068,15 +1070,17 @@ where } } - // FIXME(eddyb) This should be for `ptr::Unique`, not `Box`. + // Fixup info for the first field of a `Box`. Recursive traversal will have found + // the raw pointer, so size and align are set to the boxed type, but `pointee.safe` + // will still be `None`. if let Some(ref mut pointee) = result { - if let ty::Adt(def, _) = this.ty.kind() { - if def.is_box() && offset.bytes() == 0 { - let optimize = tcx.sess.opts.optimize != OptLevel::No; - pointee.safe = Some(PointerKind::Box { - unpin: optimize && this.ty.boxed_ty().is_unpin(tcx, cx.param_env()), - }); - } + if offset.bytes() == 0 && this.ty.is_box() { + debug_assert!(pointee.safe.is_none()); + let optimize = tcx.sess.opts.optimize != OptLevel::No; + pointee.safe = Some(PointerKind::Box { + unpin: optimize && this.ty.boxed_ty().is_unpin(tcx, cx.param_env()), + global: this.ty.is_box_global(tcx), + }); } } diff --git a/compiler/rustc_middle/src/ty/sty.rs b/compiler/rustc_middle/src/ty/sty.rs index 17659ac23307c..1f68571cd376b 100644 --- a/compiler/rustc_middle/src/ty/sty.rs +++ b/compiler/rustc_middle/src/ty/sty.rs @@ -1999,6 +1999,27 @@ impl<'tcx> Ty<'tcx> { } } + /// Tests whether this is a Box using the global allocator. + #[inline] + pub fn is_box_global(self, tcx: TyCtxt<'tcx>) -> bool { + match self.kind() { + Adt(def, args) if def.is_box() => { + let Some(alloc) = args.get(1) else { + // Single-argument Box is always global. (for "minicore" tests) + return true; + }; + if let Some(alloc_adt) = alloc.expect_ty().ty_adt_def() { + let global_alloc = tcx.require_lang_item(LangItem::GlobalAlloc, None); + alloc_adt.did() == global_alloc + } else { + // Allocator is not an ADT... + false + } + } + _ => false, + } + } + /// Panics if called on any type other than `Box`. pub fn boxed_ty(self) -> Ty<'tcx> { match self.kind() { diff --git a/compiler/rustc_span/src/symbol.rs b/compiler/rustc_span/src/symbol.rs index 3784a08b1b720..9e628e4ef91cf 100644 --- a/compiler/rustc_span/src/symbol.rs +++ b/compiler/rustc_span/src/symbol.rs @@ -896,6 +896,7 @@ symbols! { generic_const_items, generic_param_attrs, get_context, + global_alloc_ty, global_allocator, global_asm, globs, diff --git a/compiler/rustc_ty_utils/src/abi.rs b/compiler/rustc_ty_utils/src/abi.rs index 43042dbd36641..a5328baadb5fc 100644 --- a/compiler/rustc_ty_utils/src/abi.rs +++ b/compiler/rustc_ty_utils/src/abi.rs @@ -452,7 +452,7 @@ fn adjust_for_rust_scalar<'tcx>( let no_alias = match kind { PointerKind::SharedRef { frozen } => frozen, PointerKind::MutableRef { unpin } => unpin && noalias_mut_ref, - PointerKind::Box { unpin } => unpin && noalias_for_box, + PointerKind::Box { unpin, global } => unpin && global && noalias_for_box, }; // We can never add `noalias` in return position; that LLVM attribute has some very surprising semantics // (see ). diff --git a/library/alloc/src/alloc.rs b/library/alloc/src/alloc.rs index 0b1429397559d..ed7b0618e9be2 100644 --- a/library/alloc/src/alloc.rs +++ b/library/alloc/src/alloc.rs @@ -50,6 +50,8 @@ extern "Rust" { #[unstable(feature = "allocator_api", issue = "32838")] #[derive(Copy, Clone, Default, Debug)] #[cfg(not(test))] +// the compiler needs to know when a Box uses the global allocator vs a custom one +#[cfg_attr(not(bootstrap), lang = "global_alloc_ty")] pub struct Global; #[cfg(test)] diff --git a/library/alloc/src/boxed.rs b/library/alloc/src/boxed.rs index b202d9250d5e4..2736e5ee6c588 100644 --- a/library/alloc/src/boxed.rs +++ b/library/alloc/src/boxed.rs @@ -2062,6 +2062,9 @@ impl + ?Sized, A: Allocator> AsyncFn for Box #[unstable(feature = "coerce_unsized", issue = "18598")] impl, U: ?Sized, A: Allocator> CoerceUnsized> for Box {} +// It is quite crucial that we only allow the `Global` allocator here. +// Handling arbitrary custom allocators (which can affect the `Box` layout heavily!) +// would need a lot of codegen and interpreter adjustments. #[unstable(feature = "dispatch_from_dyn", issue = "none")] impl, U: ?Sized> DispatchFromDyn> for Box {} diff --git a/tests/codegen/function-arguments.rs b/tests/codegen/function-arguments.rs index b75c188f51a9b..e6e2f64571497 100644 --- a/tests/codegen/function-arguments.rs +++ b/tests/codegen/function-arguments.rs @@ -2,6 +2,7 @@ #![crate_type = "lib"] #![feature(dyn_star)] #![feature(generic_nonzero)] +#![feature(allocator_api)] use std::mem::MaybeUninit; use std::num::NonZero; @@ -182,6 +183,15 @@ pub fn _box(x: Box) -> Box { x } +// With a custom allocator, it should *not* have `noalias`. (See +// for why.) The second argument is the allocator, +// which is a reference here that still carries `noalias` as usual. +// CHECK: @_box_custom(ptr noundef nonnull align 4 %x.0, ptr noalias noundef nonnull readonly align 1 %x.1) +#[no_mangle] +pub fn _box_custom(x: Box) { + drop(x) +} + // CHECK: noundef nonnull align 4 ptr @notunpin_box(ptr noundef nonnull align 4 %x) #[no_mangle] pub fn notunpin_box(x: Box) -> Box { diff --git a/tests/ui/abi/compatibility.rs b/tests/ui/abi/compatibility.rs index 2449e515f5f5f..a4f60ea268464 100644 --- a/tests/ui/abi/compatibility.rs +++ b/tests/ui/abi/compatibility.rs @@ -160,6 +160,7 @@ mod prelude { pub _marker: PhantomData, } + #[lang = "global_alloc_ty"] pub struct Global; #[lang = "owned_box"] From 5a16aebe9d83b5e7938ebd4ca7a17a0f44d7010c Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Tue, 5 Mar 2024 09:15:20 +0000 Subject: [PATCH 12/12] Remove some dead code drop_in_place has been a lang item, not an intrinsic, for forever --- compiler/rustc_codegen_ssa/src/mir/block.rs | 2 +- compiler/rustc_hir_analysis/src/check/intrinsic.rs | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/compiler/rustc_codegen_ssa/src/mir/block.rs b/compiler/rustc_codegen_ssa/src/mir/block.rs index 95a587b8181f6..b0efb646ef332 100644 --- a/compiler/rustc_codegen_ssa/src/mir/block.rs +++ b/compiler/rustc_codegen_ssa/src/mir/block.rs @@ -852,7 +852,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { } let instance = match intrinsic { - None | Some(ty::IntrinsicDef { name: sym::drop_in_place, .. }) => instance, + None => instance, Some(intrinsic) => { let mut llargs = Vec::with_capacity(1); let ret_dest = self.make_return_dest( diff --git a/compiler/rustc_hir_analysis/src/check/intrinsic.rs b/compiler/rustc_hir_analysis/src/check/intrinsic.rs index 64137f298d649..39145c2aeb7fa 100644 --- a/compiler/rustc_hir_analysis/src/check/intrinsic.rs +++ b/compiler/rustc_hir_analysis/src/check/intrinsic.rs @@ -247,7 +247,6 @@ pub fn check_intrinsic_type( ], Ty::new_unit(tcx), ), - sym::drop_in_place => (1, 0, vec![Ty::new_mut_ptr(tcx, param(0))], Ty::new_unit(tcx)), sym::needs_drop => (1, 0, vec![], tcx.types.bool), sym::type_name => (1, 0, vec![], Ty::new_static_str(tcx)),