Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Dec 18, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading