Skip to content

Commit

Permalink
Auto merge of rust-lang#119023 - compiler-errors:fn-trait-constness, …
Browse files Browse the repository at this point in the history
…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
  • Loading branch information
bors committed Dec 18, 2023
2 parents bf9229a + faea6ad commit 3f28fe1
Show file tree
Hide file tree
Showing 10 changed files with 164 additions and 69 deletions.
59 changes: 59 additions & 0 deletions compiler/rustc_hir_typeck/src/callee.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}

Expand Down
29 changes: 0 additions & 29 deletions compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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(
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_middle/src/traits/select.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
39 changes: 32 additions & 7 deletions compiler/rustc_trait_selection/src/traits/project.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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>(
Expand All @@ -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();

Expand All @@ -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),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)),
});
}
}
_ => {}
Expand Down
11 changes: 6 additions & 5 deletions compiler/rustc_trait_selection/src/traits/select/confirmation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down Expand Up @@ -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");

Expand All @@ -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);

Expand Down Expand Up @@ -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");
Expand Down
6 changes: 5 additions & 1 deletion compiler/rustc_trait_selection/src/traits/select/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down Expand Up @@ -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();

Expand All @@ -2680,6 +2683,7 @@ impl<'tcx> SelectionContext<'_, 'tcx> {
self_ty,
closure_sig,
util::TupleArgumentsFlag::No,
fn_host_effect,
)
.map_bound(|(trait_ref, _)| trait_ref)
}
Expand Down
15 changes: 14 additions & 1 deletion compiler/rustc_trait_selection/src/traits/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()))
}

Expand Down
32 changes: 16 additions & 16 deletions tests/ui/intrinsics/const-eval-select-bad.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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
|
Expand All @@ -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
Expand Down

0 comments on commit 3f28fe1

Please sign in to comment.