Auto merge of #100096 - compiler-errors:fn-return-must-be-sized, r=jackh726

a fn pointer doesn't implement `Fn`/`FnMut`/`FnOnce` if its return type isn't sized

I stumbled upon #83915 which hasn't received much attention recently, and I wanted to revive it since this is one existing soundness hole that seems pretty easy to fix.

I'm not actually sure that the [alternative approach described here](https://github.com/rust-lang/rust/pull/83915#issuecomment-823643322) is sufficient, given the `src/test/ui/function-pointer/unsized-ret.rs` example I provided below. Rebasing the branch mentioned in that comment and testing that UI test, it seems that we actually end up only observing that `str: !Sized` during monomorphization, whereupon we ICE. Even if we were to fix that ICE, ideally we'd be raising an error that a fn pointer is being used badly during _typecheck_ instead of monomorphization, hence adapting the original approach in #83915.

I am happy to close this if people would prefer we rebase the original PR and land that -- I am partly opening to be annoying and get people thinking about this unsoundness again :heart: :smile_cat:

cc: `@estebank` and `@nikomatsakis`
r? types

Here's a link to the thread: https://rust-lang.zulipchat.com/#narrow/stream/144729-t-types/topic/PR.20.2383915/near/235421351 for more context.

1 files changed, 30 insertions, 1 deletions

diff --git a/compiler/rustc_trait_selection/src/traits/select/confirmation.rs b/compiler/rustc_trait_selection/src/traits/select/confirmation.rs
index 52bc8115fa0..e08f03a270c 100644
--- a/compiler/rustc_trait_selection/src/traits/select/confirmation.rs
+++ b/compiler/rustc_trait_selection/src/traits/select/confirmation.rs
@@ -618,7 +618,36 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
         )
         .map_bound(|(trait_ref, _)| trait_ref);
 
-        let nested = self.confirm_poly_trait_refs(obligation, trait_ref)?;
+        let mut nested = self.confirm_poly_trait_refs(obligation, trait_ref)?;
+
+        // Confirm the `type Output: Sized;` bound that is present on `FnOnce`
+        let cause = obligation.derived_cause(BuiltinDerivedObligation);
+        // The binder on the Fn obligation is "less" important than the one on
+        // the signature, as evidenced by how we treat it during projection.
+        // The safe thing to do here is to liberate it, though, which should
+        // have no worse effect than skipping the binder here.
+        let liberated_fn_ty = self.infcx.replace_bound_vars_with_placeholders(obligation.self_ty());
+        let output_ty = self
+            .infcx
+            .replace_bound_vars_with_placeholders(liberated_fn_ty.fn_sig(self.tcx()).output());
+        let output_ty = normalize_with_depth_to(
+            self,
+            obligation.param_env,
+            cause.clone(),
+            obligation.recursion_depth,
+            output_ty,
+            &mut nested,
+        );
+        let tr = ty::Binder::dummy(ty::TraitRef::new(
+            self.tcx().require_lang_item(LangItem::Sized, None),
+            self.tcx().mk_substs_trait(output_ty, &[]),
+        ));
+        nested.push(Obligation::new(
+            cause,
+            obligation.param_env,
+            tr.to_poly_trait_predicate().to_predicate(self.tcx()),
+        ));
+
         Ok(ImplSourceFnPointerData { fn_ty: self_ty, nested })
     }