Auto merge of #119023 - compiler-errors:fn-trait-constness, r=fee1-dead

 Check `FnPtr`/`FnDef` built-in fn traits correctly with effects

1. Teach the (old) trait solver how to handle the constness for built-in impls of the `Fn*` family of traits. This unfortunately doesn't support const closures just yet.
2. Fix the `const_eval_select`. It turns out that the `where` clause bounds on `const_eval_select` force the effect parameter for both fndefs to be `true` -- with effects, we will leave off any explicit where clauses and register these obligations manually.

I can elaborate on (2.) if you think it needs a better explanation!

r? fee1-dead

Author: bors

compiler/rustc_hir_typeck/src/callee.rs

@@ -471,6 +471,65 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
             }
         }
 
+        if let Some(def_id) = def_id
+            && self.tcx.def_kind(def_id) == hir::def::DefKind::Fn
+            && self.tcx.is_intrinsic(def_id)
+            && self.tcx.item_name(def_id) == sym::const_eval_select
+        {
+            let fn_sig = self.resolve_vars_if_possible(fn_sig);
+            for idx in 0..=1 {
+                let arg_ty = fn_sig.inputs()[idx + 1];
+                let span = arg_exprs.get(idx + 1).map_or(call_expr.span, |arg| arg.span);
+                // Check that second and third argument of `const_eval_select` must be `FnDef`, and additionally that
+                // the second argument must be `const fn`. The first argument must be a tuple, but this is already expressed
+                // in the function signature (`F: FnOnce<ARG>`), so I did not bother to add another check here.
+                //
+                // This check is here because there is currently no way to express a trait bound for `FnDef` types only.
+                if let ty::FnDef(def_id, _args) = *arg_ty.kind() {
+                    let fn_once_def_id =
+                        self.tcx.require_lang_item(hir::LangItem::FnOnce, Some(span));
+                    let fn_once_output_def_id =
+                        self.tcx.require_lang_item(hir::LangItem::FnOnceOutput, Some(span));
+                    if self.tcx.generics_of(fn_once_def_id).host_effect_index.is_none() {
+                        if idx == 0 && !self.tcx.is_const_fn_raw(def_id) {
+                            self.tcx.sess.emit_err(errors::ConstSelectMustBeConst { span });
+                        }
+                    } else {
+                        let const_param: ty::GenericArg<'tcx> =
+                            ([self.tcx.consts.false_, self.tcx.consts.true_])[idx].into();
+                        self.register_predicate(traits::Obligation::new(
+                            self.tcx,
+                            self.misc(span),
+                            self.param_env,
+                            ty::TraitRef::new(
+                                self.tcx,
+                                fn_once_def_id,
+                                [arg_ty.into(), fn_sig.inputs()[0].into(), const_param],
+                            ),
+                        ));
+
+                        self.register_predicate(traits::Obligation::new(
+                            self.tcx,
+                            self.misc(span),
+                            self.param_env,
+                            ty::ProjectionPredicate {
+                                projection_ty: ty::AliasTy::new(
+                                    self.tcx,
+                                    fn_once_output_def_id,
+                                    [arg_ty.into(), fn_sig.inputs()[0].into(), const_param],
+                                ),
+                                term: fn_sig.output().into(),
+                            },
+                        ));
+
+                        self.select_obligations_where_possible(|_| {});
+                    }
+                } else {
+                    self.tcx.sess.emit_err(errors::ConstSelectMustBeFn { span, ty: arg_ty });
+                }
+            }
+        }
+
         fn_sig.output()
     }
 

compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs

@@ -230,11 +230,6 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
         let minimum_input_count = expected_input_tys.len();
         let provided_arg_count = provided_args.len();
 
-        let is_const_eval_select = matches!(fn_def_id, Some(def_id) if
-            self.tcx.def_kind(def_id) == hir::def::DefKind::Fn
-            && self.tcx.is_intrinsic(def_id)
-            && self.tcx.item_name(def_id) == sym::const_eval_select);
-
         // We introduce a helper function to demand that a given argument satisfy a given input
         // This is more complicated than just checking type equality, as arguments could be coerced
         // This version writes those types back so further type checking uses the narrowed types
@@ -269,30 +264,6 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
                 return Compatibility::Incompatible(coerce_error);
             }
 
-            // Check that second and third argument of `const_eval_select` must be `FnDef`, and additionally that
-            // the second argument must be `const fn`. The first argument must be a tuple, but this is already expressed
-            // in the function signature (`F: FnOnce<ARG>`), so I did not bother to add another check here.
-            //
-            // This check is here because there is currently no way to express a trait bound for `FnDef` types only.
-            if is_const_eval_select && (1..=2).contains(&idx) {
-                if let ty::FnDef(def_id, args) = *checked_ty.kind() {
-                    if idx == 1 {
-                        if !self.tcx.is_const_fn_raw(def_id) {
-                            self.tcx.sess.emit_err(errors::ConstSelectMustBeConst {
-                                span: provided_arg.span,
-                            });
-                        } else {
-                            self.enforce_context_effects(provided_arg.span, def_id, args)
-                        }
-                    }
-                } else {
-                    self.tcx.sess.emit_err(errors::ConstSelectMustBeFn {
-                        span: provided_arg.span,
-                        ty: checked_ty,
-                    });
-                }
-            }
-
             // 3. Check if the formal type is a supertype of the checked one
             //    and register any such obligations for future type checks
             let supertype_error = self.at(&self.misc(provided_arg.span), self.param_env).sup(

compiler/rustc_middle/src/traits/select.rs

@@ -153,7 +153,7 @@ pub enum SelectionCandidate<'tcx> {
     /// Implementation of a `Fn`-family trait by one of the anonymous
     /// types generated for a fn pointer type (e.g., `fn(int) -> int`)
     FnPointerCandidate {
-        is_const: bool,
+        fn_host_effect: ty::Const<'tcx>,
     },
 
     TraitAliasCandidate,

compiler/rustc_trait_selection/src/traits/project.rs

@@ -2327,8 +2327,9 @@ fn confirm_fn_pointer_candidate<'cx, 'tcx>(
     obligation: &ProjectionTyObligation<'tcx>,
     nested: Vec<PredicateObligation<'tcx>>,
 ) -> Progress<'tcx> {
+    let tcx = selcx.tcx();
     let fn_type = selcx.infcx.shallow_resolve(obligation.predicate.self_ty());
-    let sig = fn_type.fn_sig(selcx.tcx());
+    let sig = fn_type.fn_sig(tcx);
     let Normalized { value: sig, obligations } = normalize_with_depth(
         selcx,
         obligation.param_env,
@@ -2337,9 +2338,24 @@ fn confirm_fn_pointer_candidate<'cx, 'tcx>(
         sig,
     );
 
-    confirm_callable_candidate(selcx, obligation, sig, util::TupleArgumentsFlag::Yes)
-        .with_addl_obligations(nested)
-        .with_addl_obligations(obligations)
+    let host_effect_param = match *fn_type.kind() {
+        ty::FnDef(def_id, args) => tcx
+            .generics_of(def_id)
+            .host_effect_index
+            .map_or(tcx.consts.true_, |idx| args.const_at(idx)),
+        ty::FnPtr(_) => tcx.consts.true_,
+        _ => unreachable!("only expected FnPtr or FnDef in `confirm_fn_pointer_candidate`"),
+    };
+
+    confirm_callable_candidate(
+        selcx,
+        obligation,
+        sig,
+        util::TupleArgumentsFlag::Yes,
+        host_effect_param,
+    )
+    .with_addl_obligations(nested)
+    .with_addl_obligations(obligations)
 }
 
 fn confirm_closure_candidate<'cx, 'tcx>(
@@ -2362,16 +2378,24 @@ fn confirm_closure_candidate<'cx, 'tcx>(
 
     debug!(?obligation, ?closure_sig, ?obligations, "confirm_closure_candidate");
 
-    confirm_callable_candidate(selcx, obligation, closure_sig, util::TupleArgumentsFlag::No)
-        .with_addl_obligations(nested)
-        .with_addl_obligations(obligations)
+    confirm_callable_candidate(
+        selcx,
+        obligation,
+        closure_sig,
+        util::TupleArgumentsFlag::No,
+        // FIXME(effects): This doesn't handle const closures correctly!
+        selcx.tcx().consts.true_,
+    )
+    .with_addl_obligations(nested)
+    .with_addl_obligations(obligations)
 }
 
 fn confirm_callable_candidate<'cx, 'tcx>(
     selcx: &mut SelectionContext<'cx, 'tcx>,
     obligation: &ProjectionTyObligation<'tcx>,
     fn_sig: ty::PolyFnSig<'tcx>,
     flag: util::TupleArgumentsFlag,
+    fn_host_effect: ty::Const<'tcx>,
 ) -> Progress<'tcx> {
     let tcx = selcx.tcx();
 
@@ -2386,6 +2410,7 @@ fn confirm_callable_candidate<'cx, 'tcx>(
         obligation.predicate.self_ty(),
         fn_sig,
         flag,
+        fn_host_effect,
     )
     .map_bound(|(trait_ref, ret_type)| ty::ProjectionPredicate {
         projection_ty: ty::AliasTy::new(tcx, fn_once_output_def_id, trait_ref.args),

compiler/rustc_trait_selection/src/traits/select/candidate_assembly.rs

@@ -355,17 +355,23 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
             // Provide an impl, but only for suitable `fn` pointers.
             ty::FnPtr(sig) => {
                 if sig.is_fn_trait_compatible() {
-                    candidates.vec.push(FnPointerCandidate { is_const: false });
+                    candidates
+                        .vec
+                        .push(FnPointerCandidate { fn_host_effect: self.tcx().consts.true_ });
                 }
             }
             // Provide an impl for suitable functions, rejecting `#[target_feature]` functions (RFC 2396).
-            ty::FnDef(def_id, _) => {
-                if self.tcx().fn_sig(def_id).skip_binder().is_fn_trait_compatible()
-                    && self.tcx().codegen_fn_attrs(def_id).target_features.is_empty()
+            ty::FnDef(def_id, args) => {
+                let tcx = self.tcx();
+                if tcx.fn_sig(def_id).skip_binder().is_fn_trait_compatible()
+                    && tcx.codegen_fn_attrs(def_id).target_features.is_empty()
                 {
-                    candidates
-                        .vec
-                        .push(FnPointerCandidate { is_const: self.tcx().is_const_fn(def_id) });
+                    candidates.vec.push(FnPointerCandidate {
+                        fn_host_effect: tcx
+                            .generics_of(def_id)
+                            .host_effect_index
+                            .map_or(tcx.consts.true_, |idx| args.const_at(idx)),
+                    });
                 }
             }
             _ => {}

compiler/rustc_trait_selection/src/traits/select/confirmation.rs

@@ -103,8 +103,8 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
                 ImplSource::Builtin(BuiltinImplSource::Misc, vtable_iterator)
             }
 
-            FnPointerCandidate { is_const } => {
-                let data = self.confirm_fn_pointer_candidate(obligation, is_const)?;
+            FnPointerCandidate { fn_host_effect } => {
+                let data = self.confirm_fn_pointer_candidate(obligation, fn_host_effect)?;
                 ImplSource::Builtin(BuiltinImplSource::Misc, data)
             }
 
@@ -653,8 +653,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
     fn confirm_fn_pointer_candidate(
         &mut self,
         obligation: &PolyTraitObligation<'tcx>,
-        // FIXME(effects)
-        _is_const: bool,
+        fn_host_effect: ty::Const<'tcx>,
     ) -> Result<Vec<PredicateObligation<'tcx>>, SelectionError<'tcx>> {
         debug!(?obligation, "confirm_fn_pointer_candidate");
 
@@ -675,6 +674,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
             self_ty,
             sig,
             util::TupleArgumentsFlag::Yes,
+            fn_host_effect,
         )
         .map_bound(|(trait_ref, _)| trait_ref);
 
@@ -860,7 +860,8 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
             bug!("closure candidate for non-closure {:?}", obligation);
         };
 
-        let trait_ref = self.closure_trait_ref_unnormalized(obligation, args);
+        let trait_ref =
+            self.closure_trait_ref_unnormalized(obligation, args, self.tcx().consts.true_);
         let nested = self.confirm_poly_trait_refs(obligation, trait_ref)?;
 
         debug!(?closure_def_id, ?trait_ref, ?nested, "confirm closure candidate obligations");

compiler/rustc_trait_selection/src/traits/select/mod.rs

@@ -1865,7 +1865,9 @@ impl<'tcx> SelectionContext<'_, 'tcx> {
             }
 
             // Drop otherwise equivalent non-const fn pointer candidates
-            (FnPointerCandidate { .. }, FnPointerCandidate { is_const: false }) => DropVictim::Yes,
+            (FnPointerCandidate { .. }, FnPointerCandidate { fn_host_effect }) => {
+                DropVictim::drop_if(*fn_host_effect == self.tcx().consts.true_)
+            }
 
             (
                 ParamCandidate(ref other_cand),
@@ -2660,6 +2662,7 @@ impl<'tcx> SelectionContext<'_, 'tcx> {
         &mut self,
         obligation: &PolyTraitObligation<'tcx>,
         args: GenericArgsRef<'tcx>,
+        fn_host_effect: ty::Const<'tcx>,
     ) -> ty::PolyTraitRef<'tcx> {
         let closure_sig = args.as_closure().sig();
 
@@ -2680,6 +2683,7 @@ impl<'tcx> SelectionContext<'_, 'tcx> {
             self_ty,
             closure_sig,
             util::TupleArgumentsFlag::No,
+            fn_host_effect,
         )
         .map_bound(|(trait_ref, _)| trait_ref)
     }

compiler/rustc_trait_selection/src/traits/util.rs

@@ -264,13 +264,26 @@ pub fn closure_trait_ref_and_return_type<'tcx>(
     self_ty: Ty<'tcx>,
     sig: ty::PolyFnSig<'tcx>,
     tuple_arguments: TupleArgumentsFlag,
+    fn_host_effect: ty::Const<'tcx>,
 ) -> ty::Binder<'tcx, (ty::TraitRef<'tcx>, Ty<'tcx>)> {
     assert!(!self_ty.has_escaping_bound_vars());
     let arguments_tuple = match tuple_arguments {
         TupleArgumentsFlag::No => sig.skip_binder().inputs()[0],
         TupleArgumentsFlag::Yes => Ty::new_tup(tcx, sig.skip_binder().inputs()),
     };
-    let trait_ref = ty::TraitRef::new(tcx, fn_trait_def_id, [self_ty, arguments_tuple]);
+    let trait_ref = if tcx.generics_of(fn_trait_def_id).host_effect_index.is_some() {
+        ty::TraitRef::new(
+            tcx,
+            fn_trait_def_id,
+            [
+                ty::GenericArg::from(self_ty),
+                ty::GenericArg::from(arguments_tuple),
+                ty::GenericArg::from(fn_host_effect),
+            ],
+        )
+    } else {
+        ty::TraitRef::new(tcx, fn_trait_def_id, [self_ty, arguments_tuple])
+    };
     sig.map_bound(|sig| (trait_ref, sig.output()))
 }
 

tests/ui/intrinsics/const-eval-select-bad.stderr

@@ -16,20 +16,24 @@ LL |     const_eval_select((), || {}, || {});
    = note: expected a function item, found {closure@$DIR/const-eval-select-bad.rs:7:34: 7:36}
    = help: consult the documentation on `const_eval_select` for more information
 
-error: this argument must be a function item
+error[E0277]: expected a `FnOnce()` closure, found `{integer}`
   --> $DIR/const-eval-select-bad.rs:10:27
    |
 LL |     const_eval_select((), 42, 0xDEADBEEF);
-   |                           ^^
+   |     -----------------     ^^ expected an `FnOnce()` closure, found `{integer}`
+   |     |
+   |     required by a bound introduced by this call
    |
-   = note: expected a function item, found {integer}
-   = help: consult the documentation on `const_eval_select` for more information
+   = help: the trait `FnOnce<()>` is not implemented for `{integer}`
+   = note: wrap the `{integer}` in a closure with no arguments: `|| { /* code */ }`
+note: required by a bound in `const_eval_select`
+  --> $SRC_DIR/core/src/intrinsics.rs:LL:COL
 
 error[E0277]: expected a `FnOnce()` closure, found `{integer}`
-  --> $DIR/const-eval-select-bad.rs:10:27
+  --> $DIR/const-eval-select-bad.rs:10:31
    |
 LL |     const_eval_select((), 42, 0xDEADBEEF);
-   |     -----------------     ^^ expected an `FnOnce()` closure, found `{integer}`
+   |     -----------------         ^^^^^^^^^^ expected an `FnOnce()` closure, found `{integer}`
    |     |
    |     required by a bound introduced by this call
    |
@@ -39,26 +43,22 @@ note: required by a bound in `const_eval_select`
   --> $SRC_DIR/core/src/intrinsics.rs:LL:COL
 
 error: this argument must be a function item
-  --> $DIR/const-eval-select-bad.rs:10:31
+  --> $DIR/const-eval-select-bad.rs:10:27
    |
 LL |     const_eval_select((), 42, 0xDEADBEEF);
-   |                               ^^^^^^^^^^
+   |                           ^^
    |
    = note: expected a function item, found {integer}
    = help: consult the documentation on `const_eval_select` for more information
 
-error[E0277]: expected a `FnOnce()` closure, found `{integer}`
+error: this argument must be a function item
   --> $DIR/const-eval-select-bad.rs:10:31
    |
 LL |     const_eval_select((), 42, 0xDEADBEEF);
-   |     -----------------         ^^^^^^^^^^ expected an `FnOnce()` closure, found `{integer}`
-   |     |
-   |     required by a bound introduced by this call
+   |                               ^^^^^^^^^^
    |
-   = help: the trait `FnOnce<()>` is not implemented for `{integer}`
-   = note: wrap the `{integer}` in a closure with no arguments: `|| { /* code */ }`
-note: required by a bound in `const_eval_select`
-  --> $SRC_DIR/core/src/intrinsics.rs:LL:COL
+   = note: expected a function item, found {integer}
+   = help: consult the documentation on `const_eval_select` for more information
 
 error[E0271]: expected `bar` to be a fn item that returns `i32`, but it returns `bool`
   --> $DIR/const-eval-select-bad.rs:32:34