From fb9e171ab7b0303fc983b6955e684ebb2a0f5944 Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Tue, 21 Mar 2023 11:11:32 +0000 Subject: [PATCH 1/3] Only implement Fn* traits for extern "Rust" safe function pointers. --- .../solve/trait_goals/structural_traits.rs | 16 ++++- .../src/traits/select/candidate_assembly.rs | 3 + tests/ui/traits/new-solver/fn-trait.rs | 17 ++++- tests/ui/traits/new-solver/fn-trait.stderr | 64 +++++++++++++++++++ 4 files changed, 98 insertions(+), 2 deletions(-) create mode 100644 tests/ui/traits/new-solver/fn-trait.stderr diff --git a/compiler/rustc_trait_selection/src/solve/trait_goals/structural_traits.rs b/compiler/rustc_trait_selection/src/solve/trait_goals/structural_traits.rs index d7d93377cf164..51a1a88f8e770 100644 --- a/compiler/rustc_trait_selection/src/solve/trait_goals/structural_traits.rs +++ b/compiler/rustc_trait_selection/src/solve/trait_goals/structural_traits.rs @@ -2,6 +2,7 @@ use rustc_data_structures::fx::FxHashMap; use rustc_hir::{def_id::DefId, Movability, Mutability}; use rustc_infer::traits::query::NoSolution; use rustc_middle::ty::{self, Ty, TyCtxt, TypeFoldable, TypeFolder, TypeSuperFoldable}; +use rustc_target::spec::abi::Abi; use crate::solve::EvalCtxt; @@ -194,7 +195,20 @@ pub(crate) fn extract_tupled_inputs_and_output_from_callable<'tcx>( .subst(tcx, substs) .map_bound(|sig| (tcx.mk_tup(sig.inputs()), sig.output())), )), - ty::FnPtr(sig) => Ok(Some(sig.map_bound(|sig| (tcx.mk_tup(sig.inputs()), sig.output())))), + // keep this in sync with assemble_fn_pointer_candidates until the old solver is removed. + ty::FnPtr(sig) => { + if let ty::FnSig { + unsafety: rustc_hir::Unsafety::Normal, + abi: Abi::Rust, + c_variadic: false, + .. + } = sig.skip_binder() + { + Ok(Some(sig.map_bound(|sig| (tcx.mk_tup(sig.inputs()), sig.output())))) + } else { + Err(NoSolution) + } + } ty::Closure(_, substs) => { let closure_substs = substs.as_closure(); match closure_substs.kind_ty().to_opt_closure_kind() { diff --git a/compiler/rustc_trait_selection/src/traits/select/candidate_assembly.rs b/compiler/rustc_trait_selection/src/traits/select/candidate_assembly.rs index 3182af989f05a..2b3f003353c66 100644 --- a/compiler/rustc_trait_selection/src/traits/select/candidate_assembly.rs +++ b/compiler/rustc_trait_selection/src/traits/select/candidate_assembly.rs @@ -291,6 +291,9 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { return; } + // Keep this funtion in sync with extract_tupled_inputs_and_output_from_callable + // until the old solver (and thus this function) is removed. + // Okay to skip binder because what we are inspecting doesn't involve bound regions. let self_ty = obligation.self_ty().skip_binder(); match *self_ty.kind() { diff --git a/tests/ui/traits/new-solver/fn-trait.rs b/tests/ui/traits/new-solver/fn-trait.rs index d566ead105c86..8967858a8ba57 100644 --- a/tests/ui/traits/new-solver/fn-trait.rs +++ b/tests/ui/traits/new-solver/fn-trait.rs @@ -1,5 +1,4 @@ // compile-flags: -Ztrait-solver=next -// check-pass fn require_fn(_: impl Fn() -> i32) {} @@ -7,7 +6,23 @@ fn f() -> i32 { 1i32 } +extern "C" fn g() -> i32 { + 2i32 +} + +unsafe fn h() -> i32 { + 2i32 +} + fn main() { require_fn(f); require_fn(f as fn() -> i32); + require_fn(f as unsafe fn() -> i32); + //~^ ERROR: expected a `Fn<()>` closure, found `unsafe fn() -> i32` + //~| ERROR: type mismatch resolving ` i32 as FnOnce<()>>::Output == i32` + require_fn(g); + require_fn(g as extern "C" fn() -> i32); + //~^ ERROR: expected a `Fn<()>` closure, found `extern "C" fn() -> i32` + //~| ERROR: type mismatch resolving ` i32 as FnOnce<()>>::Output == i32` + require_fn(h); } diff --git a/tests/ui/traits/new-solver/fn-trait.stderr b/tests/ui/traits/new-solver/fn-trait.stderr new file mode 100644 index 0000000000000..01f1b64be20cc --- /dev/null +++ b/tests/ui/traits/new-solver/fn-trait.stderr @@ -0,0 +1,64 @@ +error[E0277]: expected a `Fn<()>` closure, found `unsafe fn() -> i32` + --> $DIR/fn-trait.rs:20:16 + | +LL | require_fn(f as unsafe fn() -> i32); + | ---------- ^^^^^^^^^^^^^^^^^^^^^^^ call the function in a closure: `|| unsafe { /* code */ }` + | | + | required by a bound introduced by this call + | + = help: the trait `Fn<()>` is not implemented for `unsafe fn() -> i32` + = note: wrap the `unsafe fn() -> i32` in a closure with no arguments: `|| { /* code */ }` +note: required by a bound in `require_fn` + --> $DIR/fn-trait.rs:3:23 + | +LL | fn require_fn(_: impl Fn() -> i32) {} + | ^^^^^^^^^^^ required by this bound in `require_fn` + +error[E0271]: type mismatch resolving ` i32 as FnOnce<()>>::Output == i32` + --> $DIR/fn-trait.rs:20:16 + | +LL | require_fn(f as unsafe fn() -> i32); + | ---------- ^^^^^^^^^^^^^^^^^^^^^^^ types differ + | | + | required by a bound introduced by this call + | +note: required by a bound in `require_fn` + --> $DIR/fn-trait.rs:3:31 + | +LL | fn require_fn(_: impl Fn() -> i32) {} + | ^^^ required by this bound in `require_fn` + +error[E0277]: expected a `Fn<()>` closure, found `extern "C" fn() -> i32` + --> $DIR/fn-trait.rs:24:16 + | +LL | require_fn(g as extern "C" fn() -> i32); + | ---------- ^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected an `Fn<()>` closure, found `extern "C" fn() -> i32` + | | + | required by a bound introduced by this call + | + = help: the trait `Fn<()>` is not implemented for `extern "C" fn() -> i32` + = note: wrap the `extern "C" fn() -> i32` in a closure with no arguments: `|| { /* code */ }` +note: required by a bound in `require_fn` + --> $DIR/fn-trait.rs:3:23 + | +LL | fn require_fn(_: impl Fn() -> i32) {} + | ^^^^^^^^^^^ required by this bound in `require_fn` + +error[E0271]: type mismatch resolving ` i32 as FnOnce<()>>::Output == i32` + --> $DIR/fn-trait.rs:24:16 + | +LL | require_fn(g as extern "C" fn() -> i32); + | ---------- ^^^^^^^^^^^^^^^^^^^^^^^^^^^ types differ + | | + | required by a bound introduced by this call + | +note: required by a bound in `require_fn` + --> $DIR/fn-trait.rs:3:31 + | +LL | fn require_fn(_: impl Fn() -> i32) {} + | ^^^ required by this bound in `require_fn` + +error: aborting due to 4 previous errors + +Some errors have detailed explanations: E0271, E0277. +For more information about an error, try `rustc --explain E0271`. From 91d913168cf357d4c7ba9b91d4656065477e3c2c Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Tue, 21 Mar 2023 11:38:13 +0000 Subject: [PATCH 2/3] Deduplicate fn trait compatibility checks --- compiler/rustc_middle/src/ty/sty.rs | 14 +++++++++- .../solve/trait_goals/structural_traits.rs | 9 +------ .../src/traits/select/candidate_assembly.rs | 27 +++++-------------- 3 files changed, 21 insertions(+), 29 deletions(-) diff --git a/compiler/rustc_middle/src/ty/sty.rs b/compiler/rustc_middle/src/ty/sty.rs index 4c606b939b25e..2a0536a1af72d 100644 --- a/compiler/rustc_middle/src/ty/sty.rs +++ b/compiler/rustc_middle/src/ty/sty.rs @@ -23,7 +23,7 @@ use rustc_macros::HashStable; use rustc_span::symbol::{kw, sym, Symbol}; use rustc_span::Span; use rustc_target::abi::VariantIdx; -use rustc_target::spec::abi; +use rustc_target::spec::abi::{self, Abi}; use std::borrow::Cow; use std::cmp::Ordering; use std::fmt; @@ -1403,6 +1403,18 @@ impl<'tcx> PolyFnSig<'tcx> { pub fn abi(&self) -> abi::Abi { self.skip_binder().abi } + + pub fn is_fn_trait_compatible(&self) -> bool { + matches!( + self.skip_binder(), + ty::FnSig { + unsafety: rustc_hir::Unsafety::Normal, + abi: Abi::Rust, + c_variadic: false, + .. + } + ) + } } pub type CanonicalPolyFnSig<'tcx> = Canonical<'tcx, Binder<'tcx, FnSig<'tcx>>>; diff --git a/compiler/rustc_trait_selection/src/solve/trait_goals/structural_traits.rs b/compiler/rustc_trait_selection/src/solve/trait_goals/structural_traits.rs index 51a1a88f8e770..ecebe3fbcfb59 100644 --- a/compiler/rustc_trait_selection/src/solve/trait_goals/structural_traits.rs +++ b/compiler/rustc_trait_selection/src/solve/trait_goals/structural_traits.rs @@ -2,7 +2,6 @@ use rustc_data_structures::fx::FxHashMap; use rustc_hir::{def_id::DefId, Movability, Mutability}; use rustc_infer::traits::query::NoSolution; use rustc_middle::ty::{self, Ty, TyCtxt, TypeFoldable, TypeFolder, TypeSuperFoldable}; -use rustc_target::spec::abi::Abi; use crate::solve::EvalCtxt; @@ -197,13 +196,7 @@ pub(crate) fn extract_tupled_inputs_and_output_from_callable<'tcx>( )), // keep this in sync with assemble_fn_pointer_candidates until the old solver is removed. ty::FnPtr(sig) => { - if let ty::FnSig { - unsafety: rustc_hir::Unsafety::Normal, - abi: Abi::Rust, - c_variadic: false, - .. - } = sig.skip_binder() - { + if sig.is_fn_trait_compatible() { Ok(Some(sig.map_bound(|sig| (tcx.mk_tup(sig.inputs()), sig.output())))) } else { Err(NoSolution) diff --git a/compiler/rustc_trait_selection/src/traits/select/candidate_assembly.rs b/compiler/rustc_trait_selection/src/traits/select/candidate_assembly.rs index 2b3f003353c66..e06eff34df21a 100644 --- a/compiler/rustc_trait_selection/src/traits/select/candidate_assembly.rs +++ b/compiler/rustc_trait_selection/src/traits/select/candidate_assembly.rs @@ -11,7 +11,6 @@ use rustc_infer::traits::ObligationCause; use rustc_infer::traits::{Obligation, SelectionError, TraitObligation}; use rustc_middle::ty::fast_reject::TreatProjections; use rustc_middle::ty::{self, Ty, TypeVisitableExt}; -use rustc_target::spec::abi::Abi; use crate::traits; use crate::traits::query::evaluate_obligation::InferCtxtExt; @@ -302,31 +301,19 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { candidates.ambiguous = true; // Could wind up being a fn() type. } // Provide an impl, but only for suitable `fn` pointers. - ty::FnPtr(_) => { - if let ty::FnSig { - unsafety: hir::Unsafety::Normal, - abi: Abi::Rust, - c_variadic: false, - .. - } = self_ty.fn_sig(self.tcx()).skip_binder() - { + ty::FnPtr(sig) => { + if sig.is_fn_trait_compatible() { candidates.vec.push(FnPointerCandidate { is_const: false }); } } // Provide an impl for suitable functions, rejecting `#[target_feature]` functions (RFC 2396). ty::FnDef(def_id, _) => { - if let ty::FnSig { - unsafety: hir::Unsafety::Normal, - abi: Abi::Rust, - c_variadic: false, - .. - } = self_ty.fn_sig(self.tcx()).skip_binder() + if self.tcx().fn_sig(def_id).skip_binder().is_fn_trait_compatible() + && self.tcx().codegen_fn_attrs(def_id).target_features.is_empty() { - if self.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 { is_const: self.tcx().is_const_fn(def_id) }); } } _ => {} From a00413f680d52b6e4ba1c1075e1310513a1d061c Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Tue, 21 Mar 2023 11:44:36 +0000 Subject: [PATCH 3/3] Also check function items' signatures for Fn* trait compatibility --- .../solve/trait_goals/structural_traits.rs | 19 ++++-- tests/ui/traits/new-solver/fn-trait.rs | 4 ++ tests/ui/traits/new-solver/fn-trait.stderr | 66 ++++++++++++++++++- 3 files changed, 81 insertions(+), 8 deletions(-) diff --git a/compiler/rustc_trait_selection/src/solve/trait_goals/structural_traits.rs b/compiler/rustc_trait_selection/src/solve/trait_goals/structural_traits.rs index ecebe3fbcfb59..ded14a87f2ce6 100644 --- a/compiler/rustc_trait_selection/src/solve/trait_goals/structural_traits.rs +++ b/compiler/rustc_trait_selection/src/solve/trait_goals/structural_traits.rs @@ -189,11 +189,20 @@ pub(crate) fn extract_tupled_inputs_and_output_from_callable<'tcx>( goal_kind: ty::ClosureKind, ) -> Result, Ty<'tcx>)>>, NoSolution> { match *self_ty.kind() { - ty::FnDef(def_id, substs) => Ok(Some( - tcx.fn_sig(def_id) - .subst(tcx, substs) - .map_bound(|sig| (tcx.mk_tup(sig.inputs()), sig.output())), - )), + // keep this in sync with assemble_fn_pointer_candidates until the old solver is removed. + ty::FnDef(def_id, substs) => { + let sig = tcx.fn_sig(def_id); + if sig.skip_binder().is_fn_trait_compatible() + && tcx.codegen_fn_attrs(def_id).target_features.is_empty() + { + Ok(Some( + sig.subst(tcx, substs) + .map_bound(|sig| (tcx.mk_tup(sig.inputs()), sig.output())), + )) + } else { + Err(NoSolution) + } + } // keep this in sync with assemble_fn_pointer_candidates until the old solver is removed. ty::FnPtr(sig) => { if sig.is_fn_trait_compatible() { diff --git a/tests/ui/traits/new-solver/fn-trait.rs b/tests/ui/traits/new-solver/fn-trait.rs index 8967858a8ba57..0599e51d7ad8c 100644 --- a/tests/ui/traits/new-solver/fn-trait.rs +++ b/tests/ui/traits/new-solver/fn-trait.rs @@ -21,8 +21,12 @@ fn main() { //~^ ERROR: expected a `Fn<()>` closure, found `unsafe fn() -> i32` //~| ERROR: type mismatch resolving ` i32 as FnOnce<()>>::Output == i32` require_fn(g); + //~^ ERROR: expected a `Fn<()>` closure, found `extern "C" fn() -> i32 {g}` + //~| ERROR: type mismatch resolving ` i32 {g} as FnOnce<()>>::Output == i32` require_fn(g as extern "C" fn() -> i32); //~^ ERROR: expected a `Fn<()>` closure, found `extern "C" fn() -> i32` //~| ERROR: type mismatch resolving ` i32 as FnOnce<()>>::Output == i32` require_fn(h); + //~^ ERROR: expected a `Fn<()>` closure, found `unsafe fn() -> i32 {h}` + //~| ERROR: type mismatch resolving ` i32 {h} as FnOnce<()>>::Output == i32` } diff --git a/tests/ui/traits/new-solver/fn-trait.stderr b/tests/ui/traits/new-solver/fn-trait.stderr index 01f1b64be20cc..d52bcaf25b87c 100644 --- a/tests/ui/traits/new-solver/fn-trait.stderr +++ b/tests/ui/traits/new-solver/fn-trait.stderr @@ -28,8 +28,38 @@ note: required by a bound in `require_fn` LL | fn require_fn(_: impl Fn() -> i32) {} | ^^^ required by this bound in `require_fn` +error[E0277]: expected a `Fn<()>` closure, found `extern "C" fn() -> i32 {g}` + --> $DIR/fn-trait.rs:23:16 + | +LL | require_fn(g); + | ---------- ^ expected an `Fn<()>` closure, found `extern "C" fn() -> i32 {g}` + | | + | required by a bound introduced by this call + | + = help: the trait `Fn<()>` is not implemented for fn item `extern "C" fn() -> i32 {g}` + = note: wrap the `extern "C" fn() -> i32 {g}` in a closure with no arguments: `|| { /* code */ }` +note: required by a bound in `require_fn` + --> $DIR/fn-trait.rs:3:23 + | +LL | fn require_fn(_: impl Fn() -> i32) {} + | ^^^^^^^^^^^ required by this bound in `require_fn` + +error[E0271]: type mismatch resolving ` i32 {g} as FnOnce<()>>::Output == i32` + --> $DIR/fn-trait.rs:23:16 + | +LL | require_fn(g); + | ---------- ^ types differ + | | + | required by a bound introduced by this call + | +note: required by a bound in `require_fn` + --> $DIR/fn-trait.rs:3:31 + | +LL | fn require_fn(_: impl Fn() -> i32) {} + | ^^^ required by this bound in `require_fn` + error[E0277]: expected a `Fn<()>` closure, found `extern "C" fn() -> i32` - --> $DIR/fn-trait.rs:24:16 + --> $DIR/fn-trait.rs:26:16 | LL | require_fn(g as extern "C" fn() -> i32); | ---------- ^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected an `Fn<()>` closure, found `extern "C" fn() -> i32` @@ -45,7 +75,7 @@ LL | fn require_fn(_: impl Fn() -> i32) {} | ^^^^^^^^^^^ required by this bound in `require_fn` error[E0271]: type mismatch resolving ` i32 as FnOnce<()>>::Output == i32` - --> $DIR/fn-trait.rs:24:16 + --> $DIR/fn-trait.rs:26:16 | LL | require_fn(g as extern "C" fn() -> i32); | ---------- ^^^^^^^^^^^^^^^^^^^^^^^^^^^ types differ @@ -58,7 +88,37 @@ note: required by a bound in `require_fn` LL | fn require_fn(_: impl Fn() -> i32) {} | ^^^ required by this bound in `require_fn` -error: aborting due to 4 previous errors +error[E0277]: expected a `Fn<()>` closure, found `unsafe fn() -> i32 {h}` + --> $DIR/fn-trait.rs:29:16 + | +LL | require_fn(h); + | ---------- ^ call the function in a closure: `|| unsafe { /* code */ }` + | | + | required by a bound introduced by this call + | + = help: the trait `Fn<()>` is not implemented for fn item `unsafe fn() -> i32 {h}` + = note: wrap the `unsafe fn() -> i32 {h}` in a closure with no arguments: `|| { /* code */ }` +note: required by a bound in `require_fn` + --> $DIR/fn-trait.rs:3:23 + | +LL | fn require_fn(_: impl Fn() -> i32) {} + | ^^^^^^^^^^^ required by this bound in `require_fn` + +error[E0271]: type mismatch resolving ` i32 {h} as FnOnce<()>>::Output == i32` + --> $DIR/fn-trait.rs:29:16 + | +LL | require_fn(h); + | ---------- ^ types differ + | | + | required by a bound introduced by this call + | +note: required by a bound in `require_fn` + --> $DIR/fn-trait.rs:3:31 + | +LL | fn require_fn(_: impl Fn() -> i32) {} + | ^^^ required by this bound in `require_fn` + +error: aborting due to 8 previous errors Some errors have detailed explanations: E0271, E0277. For more information about an error, try `rustc --explain E0271`.