wf: ensure that non-Rust-functions have sized arguments, and everyone has sized return types

compiler/rustc_codegen_llvm/src/abi.rs

@@ -348,50 +348,18 @@ impl<'ll, 'tcx> FnAbiLlvmExt<'ll, 'tcx> for FnAbi<'tcx, Ty<'tcx>> {
                 PassMode::Direct(_) => {
                     // ABI-compatible Rust types have the same `layout.abi` (up to validity ranges),
                     // and for Scalar ABIs the LLVM type is fully determined by `layout.abi`,
-                    // guarnateeing that we generate ABI-compatible LLVM IR. Things get tricky for
-                    // aggregates...
-                    if matches!(arg.layout.abi, abi::Abi::Aggregate { .. }) {
-                        assert!(
-                            arg.layout.is_sized(),
-                            "`PassMode::Direct` for unsized type: {}",
-                            arg.layout.ty
-                        );
-                        // This really shouldn't happen, since `immediate_llvm_type` will use
-                        // `layout.fields` to turn this Rust type into an LLVM type. This means all
-                        // sorts of Rust type details leak into the ABI. However wasm sadly *does*
-                        // currently use this mode so we have to allow it -- but we absolutely
-                        // shouldn't let any more targets do that.
-                        // (Also see <https://github.com/rust-lang/rust/issues/115666>.)
-                        //
-                        // The unstable abi `PtxKernel` also uses Direct for now.
-                        // It needs to switch to something else before stabilization can happen.
-                        // (See issue: https://github.com/rust-lang/rust/issues/117271)
-                        assert!(
-                            matches!(&*cx.tcx.sess.target.arch, "wasm32" | "wasm64")
-                                || self.conv == Conv::PtxKernel,
-                            "`PassMode::Direct` for aggregates only allowed on wasm and `extern \"ptx-kernel\"` fns\nProblematic type: {:#?}",
-                            arg.layout,
-                        );
-                    }
+                    // guarnateeing that we generate ABI-compatible LLVM IR.
                     arg.layout.immediate_llvm_type(cx)
                 }
                 PassMode::Pair(..) => {
                     // ABI-compatible Rust types have the same `layout.abi` (up to validity ranges),
                     // so for ScalarPair we can easily be sure that we are generating ABI-compatible
                     // LLVM IR.
-                    assert!(
-                        matches!(arg.layout.abi, abi::Abi::ScalarPair(..)),
-                        "PassMode::Pair for type {}",
-                        arg.layout.ty
-                    );
                     llargument_tys.push(arg.layout.scalar_pair_element_llvm_type(cx, 0, true));
                     llargument_tys.push(arg.layout.scalar_pair_element_llvm_type(cx, 1, true));
                     continue;
                 }
-                PassMode::Indirect { attrs: _, meta_attrs: Some(_), on_stack } => {
-                    // `Indirect` with metadata is only for unsized types, and doesn't work with
-                    // on-stack passing.
-                    assert!(arg.layout.is_unsized() && !on_stack);
+                PassMode::Indirect { attrs: _, meta_attrs: Some(_), on_stack: _ } => {
                     // Construct the type of a (wide) pointer to `ty`, and pass its two fields.
                     // Any two ABI-compatible unsized types have the same metadata type and
                     // moreover the same metadata value leads to the same dynamic size and
@@ -402,13 +370,8 @@ impl<'ll, 'tcx> FnAbiLlvmExt<'ll, 'tcx> for FnAbi<'tcx, Ty<'tcx>> {
                     llargument_tys.push(ptr_layout.scalar_pair_element_llvm_type(cx, 1, true));
                     continue;
                 }
-                PassMode::Indirect { attrs: _, meta_attrs: None, on_stack: _ } => {
-                    assert!(arg.layout.is_sized());
-                    cx.type_ptr()
-                }
+                PassMode::Indirect { attrs: _, meta_attrs: None, on_stack: _ } => cx.type_ptr(),
                 PassMode::Cast { cast, pad_i32 } => {
-                    // `Cast` means "transmute to `CastType`"; that only makes sense for sized types.
-                    assert!(arg.layout.is_sized());
                     // add padding
                     if *pad_i32 {
                         llargument_tys.push(Reg::i32().llvm_type(cx));

compiler/rustc_error_codes/src/error_codes/E0178.md

@@ -8,7 +8,6 @@ trait Foo {}
 struct Bar<'a> {
     x: &'a Foo + 'a,     // error!
     y: &'a mut Foo + 'a, // error!
-    z: fn() -> Foo + 'a, // error!
 }
 ```
 
@@ -21,7 +20,6 @@ trait Foo {}
 struct Bar<'a> {
     x: &'a (Foo + 'a),     // ok!
     y: &'a mut (Foo + 'a), // ok!
-    z: fn() -> (Foo + 'a), // ok!
 }
 ```
 

compiler/rustc_hir_analysis/src/check/wfcheck.rs

@@ -1509,6 +1509,7 @@ fn check_fn_or_method<'tcx>(
         let has_implicit_self = hir_decl.implicit_self != hir::ImplicitSelfKind::None;
         let mut inputs = sig.inputs().iter().skip(if has_implicit_self { 1 } else { 0 });
         // Check that the argument is a tuple and is sized
+        // FIXME: move this to WF check? Currently it is duplicated here and in `confirm_builtin_call` in callee.rs.
         if let Some(ty) = inputs.next() {
             wfcx.register_bound(
                 ObligationCause::new(span, wfcx.body_def_id, ObligationCauseCode::RustCall),

compiler/rustc_hir_analysis/src/hir_wf_check.rs

@@ -13,6 +13,9 @@ pub fn provide(providers: &mut Providers) {
     *providers = Providers { diagnostic_hir_wf_check, ..*providers };
 }
 
+/// HIR-based well-formedness check, for diagnostics only.
+/// This is run after there was a WF error, to try get a better message pointing out what went wrong
+/// here.
 // Ideally, this would be in `rustc_trait_selection`, but we
 // need access to `ItemCtxt`
 fn diagnostic_hir_wf_check<'tcx>(

compiler/rustc_hir_typeck/src/callee.rs

@@ -458,6 +458,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
         );
 
         if fn_sig.abi == abi::Abi::RustCall {
+            // FIXME: move this to WF check? Currently it is duplicated here and in `check_fn_or_method` in wfcheck.rs.
             let sp = arg_exprs.last().map_or(call_expr.span, |expr| expr.span);
             if let Some(ty) = fn_sig.inputs().last().copied() {
                 self.register_bound(

compiler/rustc_hir_typeck/src/check.rs

@@ -97,7 +97,10 @@ pub(super) fn check_fn<'a, 'tcx>(
         fcx.check_pat_top(&param.pat, param_ty, ty_span, None, None);
 
         // Check that argument is Sized.
-        if !params_can_be_unsized {
+        // FIXME: can we share this (and the return type check below) with WF-checking on function
+        // signatures? However, here we have much better spans available than if we fire an
+        // obligation for our signature to be well-formed.
+        if !params_can_be_unsized || !fn_sig.abi.supports_unsized_args() {
             fcx.require_type_is_sized(
                 param_ty,
                 param.pat.span,

compiler/rustc_hir_typeck/src/expr.rs

@@ -503,6 +503,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
         self.resolve_lang_item_path(lang_item, expr.span, expr.hir_id, hir_id).1
     }
 
+    /// Called for any way that a path is mentioned in an expression.
+    /// If the path is used in a function call, `args` has the arguments, otherwise it is empty.
     pub(crate) fn check_expr_path(
         &self,
         qpath: &'tcx hir::QPath<'tcx>,

compiler/rustc_target/src/abi/call/x86_64.rs

@@ -153,9 +153,9 @@ fn reg_component(cls: &[Option<Class>], i: &mut usize, size: Size) -> Option<Reg
     }
 }
 
-fn cast_target(cls: &[Option<Class>], size: Size) -> Option<CastTarget> {
+fn cast_target(cls: &[Option<Class>], size: Size) -> CastTarget {
     let mut i = 0;
-    let lo = reg_component(cls, &mut i, size)?;
+    let lo = reg_component(cls, &mut i, size).unwrap();
     let offset = Size::from_bytes(8) * (i as u64);
     let mut target = CastTarget::from(lo);
     if size > offset {
@@ -164,7 +164,7 @@ fn cast_target(cls: &[Option<Class>], size: Size) -> Option<CastTarget> {
         }
     }
     assert_eq!(reg_component(cls, &mut i, Size::ZERO), None);
-    Some(target)
+    target
 }
 
 const MAX_INT_REGS: usize = 6; // RDI, RSI, RDX, RCX, R8, R9
@@ -227,9 +227,7 @@ where
                 // split into sized chunks passed individually
                 if arg.layout.is_aggregate() {
                     let size = arg.layout.size;
-                    if let Some(cast_target) = cast_target(cls, size) {
-                        arg.cast_to(cast_target);
-                    }
+                    arg.cast_to(cast_target(cls, size));
                 } else {
                     arg.extend_integer_width_to(32);
                 }

compiler/rustc_target/src/spec/abi.rs

@@ -86,6 +86,15 @@ impl Abi {
             _ => false,
         }
     }
+
+    /// Whether this ABI can in principle support unsized arguments.
+    /// There might be further restrictions such as nightly feature flags!
+    pub fn supports_unsized_args(self) -> bool {
+        match self {
+            Self::Rust | Self::RustCall | Self::RustIntrinsic | Self::PlatformIntrinsic => true,
+            _ => false,
+        }
+    }
 }
 
 #[derive(Copy, Clone)]

compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs

@@ -1863,7 +1863,10 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> {
             );
         }
 
-        let body = self.tcx.hir().body(self.tcx.hir().body_owned_by(obligation.cause.body_id));
+        let Some(body) = self.tcx.hir().maybe_body_owned_by(obligation.cause.body_id) else {
+            return false;
+        };
+        let body = self.tcx.hir().body(body);
 
         let mut visitor = ReturnsVisitor::default();
         visitor.visit_body(&body);
@@ -1922,15 +1925,19 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> {
             // Point at all the `return`s in the function as they have failed trait bounds.
             let mut visitor = ReturnsVisitor::default();
             visitor.visit_body(&body);
-            let typeck_results = self.typeck_results.as_ref().unwrap();
-            for expr in &visitor.returns {
-                if let Some(returned_ty) = typeck_results.node_type_opt(expr.hir_id) {
-                    let ty = self.resolve_vars_if_possible(returned_ty);
-                    if ty.references_error() {
-                        // don't print out the [type error] here
-                        err.delay_as_bug();
-                    } else {
-                        err.span_label(expr.span, format!("this returned value is of type `{ty}`"));
+            if let Some(typeck_results) = self.typeck_results.as_ref() {
+                for expr in &visitor.returns {
+                    if let Some(returned_ty) = typeck_results.node_type_opt(expr.hir_id) {
+                        let ty = self.resolve_vars_if_possible(returned_ty);
+                        if ty.references_error() {
+                            // don't print out the [type error] here
+                            err.delay_as_bug();
+                        } else {
+                            err.span_label(
+                                expr.span,
+                                format!("this returned value is of type `{ty}`"),
+                            );
+                        }
                     }
                 }
             }

compiler/rustc_trait_selection/src/traits/wf.rs

@@ -727,9 +727,10 @@ impl<'a, 'tcx> WfPredicates<'a, 'tcx> {
                     self.out.extend(obligations);
                 }
 
-                ty::FnPtr(_) => {
-                    // let the loop iterate into the argument/return
-                    // types appearing in the fn signature
+                ty::FnPtr(fn_sig) => {
+                    // The loop iterates into the argument/return types appearing in the fn
+                    // signature, but we need to do some extra checks.
+                    self.compute_fn_sig_obligations(fn_sig)
                 }
 
                 ty::Alias(ty::Opaque, ty::AliasTy { def_id, args, .. }) => {
@@ -806,6 +807,22 @@ impl<'a, 'tcx> WfPredicates<'a, 'tcx> {
         }
     }
 
+    /// Add the obligations for this signature to be well-formed to `out`.
+    fn compute_fn_sig_obligations(&mut self, sig: ty::PolyFnSig<'tcx>) {
+        // The return type must always be sized.
+        // FIXME(RalfJung): is skip_binder right? It's what the type walker used in `compute` also does.
+        self.require_sized(sig.skip_binder().output(), traits::SizedReturnType);
+        // For non-Rust ABIs, the argument type must always be sized.
+        // FIXME(RalfJung): we don't do the Rust ABI check here, since that depends on feature gates
+        // and it's not clear to me whether WF depending on feature gates (which can differ across
+        // crates) is possible or not.
+        if !sig.skip_binder().abi.supports_unsized_args() {
+            for &arg in sig.skip_binder().inputs() {
+                self.require_sized(arg, traits::SizedArgumentType(None));
+            }
+        }
+    }
+
     #[instrument(level = "debug", skip(self))]
     fn nominal_obligations(
         &mut self,

compiler/rustc_ty_utils/src/abi.rs

@@ -327,6 +327,74 @@ fn adjust_for_rust_scalar<'tcx>(
     }
 }
 
+/// Ensure that the ABI makes basic sense.
+fn fn_abi_sanity_check<'tcx>(cx: &LayoutCx<'tcx, TyCtxt<'tcx>>, fn_abi: &FnAbi<'tcx, Ty<'tcx>>) {
+    fn fn_arg_sanity_check<'tcx>(
+        cx: &LayoutCx<'tcx, TyCtxt<'tcx>>,
+        fn_abi: &FnAbi<'tcx, Ty<'tcx>>,
+        arg: &ArgAbi<'tcx, Ty<'tcx>>,
+    ) {
+        match &arg.mode {
+            PassMode::Ignore => {}
+            PassMode::Direct(_) => {
+                // Here the Rust type is used to determine the actual ABI, so we have to be very
+                // careful. Scalar/ScalarPair is fine, since backends will generally use
+                // `layout.abi` and ignore everything else. We should just reject `Aggregate`
+                // entirely here, but some targets need to be fixed first.
+                if matches!(arg.layout.abi, Abi::Aggregate { .. }) {
+                    assert!(
+                        arg.layout.is_sized(),
+                        "`PassMode::Direct` for unsized type in ABI: {:#?}",
+                        fn_abi
+                    );
+                    // This really shouldn't happen, since `immediate_llvm_type` will use
+                    // `layout.fields` to turn this Rust type into an LLVM type. This means all
+                    // sorts of Rust type details leak into the ABI. However wasm sadly *does*
+                    // currently use this mode so we have to allow it -- but we absolutely
+                    // shouldn't let any more targets do that.
+                    // (Also see <https://github.com/rust-lang/rust/issues/115666>.)
+                    //
+                    // The unstable abi `PtxKernel` also uses Direct for now.
+                    // It needs to switch to something else before stabilization can happen.
+                    // (See issue: https://github.com/rust-lang/rust/issues/117271)
+                    assert!(
+                        matches!(&*cx.tcx.sess.target.arch, "wasm32" | "wasm64")
+                            || fn_abi.conv == Conv::PtxKernel,
+                        "`PassMode::Direct` for aggregates only allowed on wasm and `extern \"ptx-kernel\"` fns\nProblematic type: {:#?}",
+                        arg.layout,
+                    );
+                }
+            }
+            PassMode::Pair(_, _) => {
+                // Similar to `Direct`, we need to make sure that backends use `layout.abi` and
+                // ignore the rest of the layout.
+                assert!(
+                    matches!(arg.layout.abi, Abi::ScalarPair(..)),
+                    "PassMode::Pair for type {}",
+                    arg.layout.ty
+                );
+            }
+            PassMode::Cast { .. } => {
+                // `Cast` means "transmute to `CastType`"; that only makes sense for sized types.
+                assert!(arg.layout.is_sized());
+            }
+            PassMode::Indirect { meta_attrs: None, .. } => {
+                // No metadata, must be sized.
+                assert!(arg.layout.is_sized());
+            }
+            PassMode::Indirect { meta_attrs: Some(_), on_stack, .. } => {
+                // With metadata. Must be unsized and not on the stack.
+                assert!(arg.layout.is_unsized() && !on_stack);
+            }
+        }
+    }
+
+    for arg in fn_abi.args.iter() {
+        fn_arg_sanity_check(cx, fn_abi, arg);
+    }
+    fn_arg_sanity_check(cx, fn_abi, &fn_abi.ret);
+}
+
 // FIXME(eddyb) perhaps group the signature/type-containing (or all of them?)
 // arguments of this method, into a separate `struct`.
 #[tracing::instrument(level = "debug", skip(cx, caller_location, fn_def_id, force_thin_self_ptr))]
@@ -453,6 +521,7 @@ fn fn_abi_new_uncached<'tcx>(
     };
     fn_abi_adjust_for_abi(cx, &mut fn_abi, sig.abi, fn_def_id)?;
     debug!("fn_abi_new_uncached = {:?}", fn_abi);
+    fn_abi_sanity_check(cx, &fn_abi);
     Ok(cx.tcx.arena.alloc(fn_abi))
 }
 

tests/ui/abi/compatibility.rs

@@ -37,9 +37,11 @@
 // revisions: wasi
 //[wasi] compile-flags: --target wasm32-wasi
 //[wasi] needs-llvm-components: webassembly
-// revisions: nvptx64
-//[nvptx64] compile-flags: --target nvptx64-nvidia-cuda
-//[nvptx64] needs-llvm-components: nvptx
+// FIXME: disabled on nvptx64 since the target ABI fails the sanity check
+/* revisions: nvptx64
+  [nvptx64] compile-flags: --target nvptx64-nvidia-cuda
+  [nvptx64] needs-llvm-components: nvptx
+*/
 #![feature(rustc_attrs, unsized_fn_params, transparent_unions)]
 #![cfg_attr(not(host), feature(no_core, lang_items), no_std, no_core)]
 #![allow(unused, improper_ctypes_definitions, internal_features)]
@@ -307,19 +309,30 @@ mod arrays {
 }
 
 // Some tests with unsized types (not all wrappers are compatible with that).
+macro_rules! assert_abi_compatible_unsized {
+    ($name:ident, $t1:ty, $t2:ty) => {
+        mod $name {
+            use super::*;
+            // Declaring a `type` doesn't even check well-formedness, so we also declare a function.
+            fn check_wf(_x: $t1, _y: $t2) {}
+            // Can only test arguments and only the Rust ABI, since it's unsized.
+            #[rustc_abi(assert_eq)]
+            type TestRust = (fn($t1), fn($t2));
+        }
+    };
+}
 macro_rules! test_transparent_unsized {
     ($name:ident, $t:ty) => {
         mod $name {
             use super::*;
-            assert_abi_compatible!(wrap1, $t, Wrapper1<$t>);
-            assert_abi_compatible!(wrap1_reprc, ReprC1<$t>, ReprC1<Wrapper1<$t>>);
-            assert_abi_compatible!(wrap2, $t, Wrapper2<$t>);
-            assert_abi_compatible!(wrap2_reprc, ReprC1<$t>, ReprC1<Wrapper2<$t>>);
+            assert_abi_compatible_unsized!(wrap1, $t, Wrapper1<$t>);
+            assert_abi_compatible_unsized!(wrap1_reprc, ReprC1<$t>, ReprC1<Wrapper1<$t>>);
+            assert_abi_compatible_unsized!(wrap2, $t, Wrapper2<$t>);
+            assert_abi_compatible_unsized!(wrap2_reprc, ReprC1<$t>, ReprC1<Wrapper2<$t>>);
         }
     };
 }
 
-#[cfg(not(any(target_arch = "mips64", target_arch = "sparc64")))]
 mod unsized_ {
     use super::*;
     test_transparent_unsized!(str_, str);