Auto merge of #141879 - matthiaskrgr:rollup-jqjk26e, r=matthiaskrgr

Rollup of 6 pull requests

Successful merges:

 - #140715 (Clarify &mut-methods' docs on sync::OnceLock)
 - #141309 (x86 (32/64): go back to passing SIMD vectors by-ptr)
 - #141677 (Async drop - type instead of async drop fn, fixes #140484)
 - #141733 (C-variadic functions must be unsafe)
 - #141858 (Fix typo in `StructuralPartialEq` docs)
 - #141874 (add f16_epsilon and f128_epsilon diagnostic items)

r? `@ghost`
`@rustbot` modify labels: rollup

Author: bors

compiler/rustc_ast_passes/messages.ftl

@@ -24,8 +24,6 @@ ast_passes_auto_super_lifetime = auto traits cannot have super traits or lifetim
     .label = {ast_passes_auto_super_lifetime}
     .suggestion = remove the super traits or lifetime bounds
 
-ast_passes_bad_c_variadic = only foreign, `unsafe extern "C"`, or `unsafe extern "C-unwind"` functions may have a C-variadic arg
-
 ast_passes_bare_fn_invalid_safety = function pointers cannot be declared with `safe` safety qualifier
     .suggestion = remove safe from this item
 
@@ -36,6 +34,13 @@ ast_passes_body_in_extern = incorrect `{$kind}` inside `extern` block
 
 ast_passes_bound_in_context = bounds on `type`s in {$ctx} have no effect
 
+ast_passes_c_variadic_bad_calling_convention =
+    only foreign, `unsafe extern "C"`, or `unsafe extern "C-unwind"` functions may have a C-variadic arg
+
+ast_passes_c_variadic_safe_foreign_function =
+    foreign functions with a C-variadic argument cannot be safe
+    .suggestion = remove the `safe` keyword from this definition
+
 ast_passes_const_and_c_variadic = functions cannot be both `const` and C-variadic
     .const = `const` because of this
     .variadic = C-variadic because of this

compiler/rustc_ast_passes/src/ast_validation.rs

@@ -554,7 +554,21 @@ impl<'a> AstValidator<'a> {
         }
 
         match (fk.ctxt(), fk.header()) {
-            (Some(FnCtxt::Foreign), _) => return,
+            (Some(FnCtxt::Foreign), Some(header)) => match header.safety {
+                Safety::Default | Safety::Unsafe(_) => return,
+                Safety::Safe(span) => {
+                    self.dcx().emit_err(errors::CVariadicSafeForeignFunction {
+                        // The span of the "safe " string that should be removed.
+                        safe_span: self
+                            .sess
+                            .psess
+                            .source_map()
+                            .span_until_non_whitespace(span.until(fk.decl().output.span())),
+                    });
+                    return;
+                }
+            },
+
             (Some(FnCtxt::Free), Some(header)) => match header.ext {
                 Extern::Explicit(StrLit { symbol_unescaped: sym::C, .. }, _)
                 | Extern::Explicit(StrLit { symbol_unescaped: sym::C_dash_unwind, .. }, _)

compiler/rustc_ast_passes/src/errors.rs

@@ -308,12 +308,25 @@ pub(crate) struct ExternItemAscii {
 }
 
 #[derive(Diagnostic)]
-#[diag(ast_passes_bad_c_variadic)]
+#[diag(ast_passes_c_variadic_bad_calling_convention)]
 pub(crate) struct BadCVariadic {
     #[primary_span]
     pub span: Vec<Span>,
 }
 
+#[derive(Diagnostic)]
+#[diag(ast_passes_c_variadic_safe_foreign_function)]
+pub(crate) struct CVariadicSafeForeignFunction {
+    #[primary_span]
+    #[suggestion(
+        ast_passes_suggestion,
+        applicability = "machine-applicable",
+        code = "",
+        style = "verbose"
+    )]
+    pub safe_span: Span,
+}
+
 #[derive(Diagnostic)]
 #[diag(ast_passes_item_underscore)]
 pub(crate) struct ItemUnderscore<'a> {

compiler/rustc_mir_transform/src/elaborate_drop.rs

@@ -1,6 +1,7 @@
 use std::{fmt, iter, mem};
 
 use rustc_abi::{FIRST_VARIANT, FieldIdx, VariantIdx};
+use rustc_hir::def::DefKind;
 use rustc_hir::lang_items::LangItem;
 use rustc_index::Idx;
 use rustc_middle::mir::*;
@@ -254,8 +255,19 @@ where
             // impl_item_refs may be empty if drop fn is not implemented in 'impl AsyncDrop for ...'
             // (#140974).
             // Such code will report error, so just generate sync drop here and return
-            let Some(drop_fn_def_id) =
-                tcx.associated_item_def_ids(drop_trait).into_iter().nth(0).copied()
+            let Some(drop_fn_def_id) = tcx
+                .associated_item_def_ids(drop_trait)
+                .first()
+                .and_then(|def_id| {
+                    if tcx.def_kind(def_id) == DefKind::AssocFn
+                        && tcx.check_args_compatible(*def_id, trait_args)
+                    {
+                        Some(def_id)
+                    } else {
+                        None
+                    }
+                })
+                .copied()
             else {
                 tcx.dcx().span_delayed_bug(
                     self.elaborator.body().span,

compiler/rustc_monomorphize/src/mono_checks/abi_check.rs

@@ -35,8 +35,6 @@ fn do_check_simd_vector_abi<'tcx>(
     is_call: bool,
     loc: impl Fn() -> (Span, HirId),
 ) {
-    // We check this on all functions, including those using the "Rust" ABI.
-    // For the "Rust" ABI it would be a bug if the lint ever triggered, but better safe than sorry.
     let feature_def = tcx.sess.target.features_for_correct_vector_abi();
     let codegen_attrs = tcx.codegen_fn_attrs(def_id);
     let have_feature = |feat: Symbol| {
@@ -123,8 +121,9 @@ fn do_check_wasm_abi<'tcx>(
     is_call: bool,
     loc: impl Fn() -> (Span, HirId),
 ) {
-    // Only proceed for `extern "C" fn` on wasm32-unknown-unknown (same check as what `adjust_for_foreign_abi` uses to call `compute_wasm_abi_info`),
-    // and only proceed if `wasm_c_abi_opt` indicates we should emit the lint.
+    // Only proceed for `extern "C" fn` on wasm32-unknown-unknown (same check as what
+    // `adjust_for_foreign_abi` uses to call `compute_wasm_abi_info`), and only proceed if
+    // `wasm_c_abi_opt` indicates we should emit the lint.
     if !(tcx.sess.target.arch == "wasm32"
         && tcx.sess.target.os == "unknown"
         && tcx.wasm_c_abi_opt() == WasmCAbi::Legacy { with_lint: true }
@@ -157,8 +156,15 @@ fn check_instance_abi<'tcx>(tcx: TyCtxt<'tcx>, instance: Instance<'tcx>) {
     else {
         // An error will be reported during codegen if we cannot determine the ABI of this
         // function.
+        tcx.dcx().delayed_bug("ABI computation failure should lead to compilation failure");
         return;
     };
+    // Unlike the call-site check, we do also check "Rust" ABI functions here.
+    // This should never trigger, *except* if we start making use of vector registers
+    // for the "Rust" ABI and the user disables those vector registers (which should trigger a
+    // warning as that's clearly disabling a "required" target feature for this target).
+    // Using such a function is where disabling the vector register actually can start leading
+    // to soundness issues, so erroring here seems good.
     let loc = || {
         let def_id = instance.def_id();
         (
@@ -179,7 +185,8 @@ fn check_call_site_abi<'tcx>(
     loc: impl Fn() -> (Span, HirId) + Copy,
 ) {
     if callee.fn_sig(tcx).abi().is_rustic_abi() {
-        // we directly handle the soundness of Rust ABIs
+        // We directly handle the soundness of Rust ABIs -- so let's skip the majority of
+        // call sites to avoid a perf regression.
         return;
     }
     let typing_env = ty::TypingEnv::fully_monomorphized();

compiler/rustc_span/src/symbol.rs

@@ -937,8 +937,10 @@ symbols! {
         external_doc,
         f,
         f128,
+        f128_epsilon,
         f128_nan,
         f16,
+        f16_epsilon,
         f16_nan,
         f16c_target_feature,
         f32,

compiler/rustc_target/src/callconv/mod.rs

@@ -8,7 +8,7 @@ use rustc_abi::{
 };
 use rustc_macros::HashStable_Generic;
 
-use crate::spec::{HasTargetSpec, HasWasmCAbiOpt, HasX86AbiOpt, RustcAbi, WasmCAbi};
+use crate::spec::{HasTargetSpec, HasWasmCAbiOpt, HasX86AbiOpt, WasmCAbi};
 
 mod aarch64;
 mod amdgpu;
@@ -733,24 +733,6 @@ impl<'a, Ty> FnAbi<'a, Ty> {
             _ => {}
         };
 
-        // Decides whether we can pass the given SIMD argument via `PassMode::Direct`.
-        // May only return `true` if the target will always pass those arguments the same way,
-        // no matter what the user does with `-Ctarget-feature`! In other words, whatever
-        // target features are required to pass a SIMD value in registers must be listed in
-        // the `abi_required_features` for the current target and ABI.
-        let can_pass_simd_directly = |arg: &ArgAbi<'_, Ty>| match &*spec.arch {
-            // On x86, if we have SSE2 (which we have by default for x86_64), we can always pass up
-            // to 128-bit-sized vectors.
-            "x86" if spec.rustc_abi == Some(RustcAbi::X86Sse2) => arg.layout.size.bits() <= 128,
-            "x86_64" if spec.rustc_abi != Some(RustcAbi::X86Softfloat) => {
-                // FIXME once https://github.com/bytecodealliance/wasmtime/issues/10254 is fixed
-                // accept vectors up to 128bit rather than vectors of exactly 128bit.
-                arg.layout.size.bits() == 128
-            }
-            // So far, we haven't implemented this logic for any other target.
-            _ => false,
-        };
-
         for (arg_idx, arg) in self
             .args
             .iter_mut()
@@ -850,9 +832,10 @@ impl<'a, Ty> FnAbi<'a, Ty> {
                     // target feature sets. Some more information about this
                     // issue can be found in #44367.
                     //
-                    // Note that the intrinsic ABI is exempt here as those are not
-                    // real functions anyway, and the backend expects very specific types.
-                    if spec.simd_types_indirect && !can_pass_simd_directly(arg) {
+                    // We *could* do better in some cases, e.g. on x86_64 targets where SSE2 is
+                    // required. However, it turns out that that makes LLVM worse at optimizing this
+                    // code, so we pass things indirectly even there. See #139029 for more on that.
+                    if spec.simd_types_indirect {
                         arg.make_indirect();
                     }
                 }

library/core/src/marker.rs

@@ -200,7 +200,7 @@ pub trait Unsize<T: ?Sized> {
 ///
 /// Constants are only allowed as patterns if (a) their type implements
 /// `PartialEq`, and (b) interpreting the value of the constant as a pattern
-/// is equialent to calling `PartialEq`. This ensures that constants used as
+/// is equivalent to calling `PartialEq`. This ensures that constants used as
 /// patterns cannot expose implementation details in an unexpected way or
 /// cause semver hazards.
 ///

library/core/src/num/f128.rs

@@ -171,6 +171,7 @@ impl f128 {
     /// [Machine epsilon]: https://en.wikipedia.org/wiki/Machine_epsilon
     /// [`MANTISSA_DIGITS`]: f128::MANTISSA_DIGITS
     #[unstable(feature = "f128", issue = "116909")]
+    #[rustc_diagnostic_item = "f128_epsilon"]
     pub const EPSILON: f128 = 1.92592994438723585305597794258492732e-34_f128;
 
     /// Smallest finite `f128` value.

library/core/src/num/f16.rs

@@ -168,6 +168,7 @@ impl f16 {
     /// [Machine epsilon]: https://en.wikipedia.org/wiki/Machine_epsilon
     /// [`MANTISSA_DIGITS`]: f16::MANTISSA_DIGITS
     #[unstable(feature = "f16", issue = "116909")]
+    #[rustc_diagnostic_item = "f16_epsilon"]
     pub const EPSILON: f16 = 9.7656e-4_f16;
 
     /// Smallest finite `f16` value.

library/std/src/sync/once_lock.rs

@@ -159,8 +159,11 @@ impl<T> OnceLock<T> {
 
     /// Gets the mutable reference to the underlying value.
     ///
-    /// Returns `None` if the cell is uninitialized, or being initialized.
-    /// This method never blocks.
+    /// Returns `None` if the cell is uninitialized.
+    ///
+    /// This method never blocks. Since it borrows the `OnceLock` mutably,
+    /// it is statically guaranteed that no active borrows to the `OnceLock`
+    /// exist, including from other threads.
     #[inline]
     #[stable(feature = "once_cell", since = "1.70.0")]
     pub fn get_mut(&mut self) -> Option<&mut T> {
@@ -315,7 +318,9 @@ impl<T> OnceLock<T> {
     /// Gets the mutable reference of the contents of the cell, initializing
     /// it to `f()` if the cell was uninitialized.
     ///
-    /// This method never blocks.
+    /// This method never blocks. Since it borrows the `OnceLock` mutably,
+    /// it is statically guaranteed that no active borrows to the `OnceLock`
+    /// exist, including from other threads.
     ///
     /// # Panics
     ///
@@ -405,7 +410,9 @@ impl<T> OnceLock<T> {
     /// it to `f()` if the cell was uninitialized. If the cell was uninitialized
     /// and `f()` failed, an error is returned.
     ///
-    /// This method never blocks.
+    /// This method never blocks. Since it borrows the `OnceLock` mutably,
+    /// it is statically guaranteed that no active borrows to the `OnceLock`
+    /// exist, including from other threads.
     ///
     /// # Panics
     ///
@@ -469,7 +476,8 @@ impl<T> OnceLock<T> {
     ///
     /// Has no effect and returns `None` if the `OnceLock` was uninitialized.
     ///
-    /// Safety is guaranteed by requiring a mutable reference.
+    /// Since this method borrows the `OnceLock` mutably, it is statically guaranteed that
+    /// no active borrows to the `OnceLock` exist, including from other threads.
     ///
     /// # Examples
     ///

tests/codegen/abi-x86-sse.rs

@@ -27,8 +27,9 @@ trait Copy {}
 #[repr(simd)]
 pub struct Sse([f32; 4]);
 
-// x86-64: <4 x float> @sse_id(<4 x float> {{[^,]*}})
-// x86-32: <4 x float> @sse_id(<4 x float> {{[^,]*}})
+// FIXME: due to #139029 we are passing them all indirectly.
+// x86-64: void @sse_id(ptr{{( [^,]*)?}} sret([16 x i8]){{( .*)?}}, ptr{{( [^,]*)?}})
+// x86-32: void @sse_id(ptr{{( [^,]*)?}} sret([16 x i8]){{( .*)?}}, ptr{{( [^,]*)?}})
 // x86-32-nosse: void @sse_id(ptr{{( [^,]*)?}} sret([16 x i8]){{( .*)?}}, ptr{{( [^,]*)?}})
 #[no_mangle]
 pub fn sse_id(x: Sse) -> Sse {

tests/codegen/simd-intrinsic/simd-intrinsic-transmute-array.rs

@@ -1,14 +1,5 @@
 //
 //@ compile-flags: -C no-prepopulate-passes
-// LLVM IR isn't very portable and the one tested here depends on the ABI
-// which is different between x86 (where we use SSE registers) and others.
-// `x86-64` and `x86-32-sse2` are identical, but compiletest does not support
-// taking the union of multiple `only` annotations.
-//@ revisions: x86-64 x86-32-sse2 other
-//@[x86-64] only-x86_64
-//@[x86-32-sse2] only-rustc_abi-x86-sse2
-//@[other] ignore-rustc_abi-x86-sse2
-//@[other] ignore-x86_64
 
 #![crate_type = "lib"]
 #![allow(non_camel_case_types)]
@@ -47,9 +38,7 @@ pub fn build_array_s(x: [f32; 4]) -> S<4> {
 #[no_mangle]
 pub fn build_array_transmute_s(x: [f32; 4]) -> S<4> {
     // CHECK: %[[VAL:.+]] = load <4 x float>, ptr %x, align [[ARRAY_ALIGN]]
-    // x86-32: ret <4 x float> %[[VAL:.+]]
-    // x86-64: ret <4 x float> %[[VAL:.+]]
-    // other: store <4 x float> %[[VAL:.+]], ptr %_0, align [[VECTOR_ALIGN]]
+    // CHECK: store <4 x float> %[[VAL:.+]], ptr %_0, align [[VECTOR_ALIGN]]
     unsafe { std::mem::transmute(x) }
 }
 
@@ -64,8 +53,6 @@ pub fn build_array_t(x: [f32; 4]) -> T {
 #[no_mangle]
 pub fn build_array_transmute_t(x: [f32; 4]) -> T {
     // CHECK: %[[VAL:.+]] = load <4 x float>, ptr %x, align [[ARRAY_ALIGN]]
-    // x86-32: ret <4 x float> %[[VAL:.+]]
-    // x86-64: ret <4 x float> %[[VAL:.+]]
-    // other: store <4 x float> %[[VAL:.+]], ptr %_0, align [[VECTOR_ALIGN]]
+    // CHECK: store <4 x float> %[[VAL:.+]], ptr %_0, align [[VECTOR_ALIGN]]
     unsafe { std::mem::transmute(x) }
 }

tests/codegen/simd/packed-simd.rs

@@ -30,16 +30,18 @@ fn load<T, const N: usize>(v: PackedSimd<T, N>) -> FullSimd<T, N> {
     }
 }
 
-// CHECK-LABEL: define <3 x float> @square_packed_full(ptr{{[a-z_ ]*}} align 4 {{[^,]*}})
+// CHECK-LABEL: square_packed_full
+// CHECK-SAME: ptr{{[a-z_ ]*}} sret([[RET_TYPE:[^)]+]]) [[RET_ALIGN:align (8|16)]]{{[^%]*}} [[RET_VREG:%[_0-9]*]]
+// CHECK-SAME: ptr{{[a-z_ ]*}} align 4
 #[no_mangle]
 pub fn square_packed_full(x: PackedSimd<f32, 3>) -> FullSimd<f32, 3> {
-    // The unoptimized version of this is not very interesting to check
-    // since `load` does not get inlined.
-    // opt3-NEXT: start:
-    // opt3-NEXT: load <3 x float>
+    // CHECK-NEXT: start
+    // noopt: alloca [[RET_TYPE]], [[RET_ALIGN]]
+    // CHECK: load <3 x float>
     let x = load(x);
-    // opt3-NEXT: [[VREG:%[a-z0-9_]+]] = fmul <3 x float>
-    // opt3-NEXT: ret <3 x float> [[VREG:%[a-z0-9_]+]]
+    // CHECK: [[VREG:%[a-z0-9_]+]] = fmul <3 x float>
+    // CHECK-NEXT: store <3 x float> [[VREG]], ptr [[RET_VREG]], [[RET_ALIGN]]
+    // CHECK-NEXT: ret void
     unsafe { intrinsics::simd_mul(x, x) }
 }