From 144206e6d8c1ab4ffdbaf6d7b0f5a4201c0f2da4 Mon Sep 17 00:00:00 2001 From: Caleb Zulawski Date: Sat, 13 Jun 2020 01:18:53 -0400 Subject: [PATCH 1/4] Don't implement Fn* traits for #[target_feature] functions --- .../traits/select/candidate_assembly.rs | 16 +++++++- .../rfc-2396-target_feature-11/fn-traits.rs | 24 ++++++++++++ .../fn-traits.stderr | 39 +++++++++++++++++++ 3 files changed, 78 insertions(+), 1 deletion(-) create mode 100644 src/test/ui/rfcs/rfc-2396-target_feature-11/fn-traits.rs create mode 100644 src/test/ui/rfcs/rfc-2396-target_feature-11/fn-traits.stderr diff --git a/src/librustc_trait_selection/traits/select/candidate_assembly.rs b/src/librustc_trait_selection/traits/select/candidate_assembly.rs index 9045451056b19..91c4d1d62aa70 100644 --- a/src/librustc_trait_selection/traits/select/candidate_assembly.rs +++ b/src/librustc_trait_selection/traits/select/candidate_assembly.rs @@ -306,7 +306,7 @@ 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::FnDef(..) | ty::FnPtr(_) => { + ty::FnPtr(_) => { if let ty::FnSig { unsafety: hir::Unsafety::Normal, abi: Abi::Rust, @@ -317,6 +317,20 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { candidates.vec.push(FnPointerCandidate); } } + // 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().codegen_fn_attrs(def_id).target_features.is_empty() { + candidates.vec.push(FnPointerCandidate); + } + } + } _ => {} } diff --git a/src/test/ui/rfcs/rfc-2396-target_feature-11/fn-traits.rs b/src/test/ui/rfcs/rfc-2396-target_feature-11/fn-traits.rs new file mode 100644 index 0000000000000..58070b69bc1f8 --- /dev/null +++ b/src/test/ui/rfcs/rfc-2396-target_feature-11/fn-traits.rs @@ -0,0 +1,24 @@ +// only-x86_64 + +#![feature(target_feature_11)] + +#[target_feature(enable = "avx")] +fn foo() {} + +fn call(f: impl Fn()) { + f() +} + +fn call_mut(f: impl FnMut()) { + f() +} + +fn call_once(f: impl FnOnce()) { + f() +} + +fn main() { + call(foo); //~ ERROR expected a `std::ops::Fn<()>` closure, found `fn() {foo}` + call_mut(foo); //~ ERROR expected a `std::ops::FnMut<()>` closure, found `fn() {foo}` + call_once(foo); //~ ERROR expected a `std::ops::FnOnce<()>` closure, found `fn() {foo}` +} diff --git a/src/test/ui/rfcs/rfc-2396-target_feature-11/fn-traits.stderr b/src/test/ui/rfcs/rfc-2396-target_feature-11/fn-traits.stderr new file mode 100644 index 0000000000000..6dcf31b4c742a --- /dev/null +++ b/src/test/ui/rfcs/rfc-2396-target_feature-11/fn-traits.stderr @@ -0,0 +1,39 @@ +error[E0277]: expected a `std::ops::Fn<()>` closure, found `fn() {foo}` + --> $DIR/fn-traits.rs:21:10 + | +LL | fn call(f: impl Fn()) { + | ---- required by this bound in `call` +... +LL | call(foo); + | ^^^ expected an `Fn<()>` closure, found `fn() {foo}` + | + = help: the trait `std::ops::Fn<()>` is not implemented for `fn() {foo}` + = note: wrap the `fn() {foo}` in a closure with no arguments: `|| { /* code */ } + +error[E0277]: expected a `std::ops::FnMut<()>` closure, found `fn() {foo}` + --> $DIR/fn-traits.rs:22:14 + | +LL | fn call_mut(f: impl FnMut()) { + | ------- required by this bound in `call_mut` +... +LL | call_mut(foo); + | ^^^ expected an `FnMut<()>` closure, found `fn() {foo}` + | + = help: the trait `std::ops::FnMut<()>` is not implemented for `fn() {foo}` + = note: wrap the `fn() {foo}` in a closure with no arguments: `|| { /* code */ } + +error[E0277]: expected a `std::ops::FnOnce<()>` closure, found `fn() {foo}` + --> $DIR/fn-traits.rs:23:15 + | +LL | fn call_once(f: impl FnOnce()) { + | -------- required by this bound in `call_once` +... +LL | call_once(foo); + | ^^^ expected an `FnOnce<()>` closure, found `fn() {foo}` + | + = help: the trait `std::ops::FnOnce<()>` is not implemented for `fn() {foo}` + = note: wrap the `fn() {foo}` in a closure with no arguments: `|| { /* code */ } + +error: aborting due to 3 previous errors + +For more information about this error, try `rustc --explain E0277`. From c98b4c8fdde7812d7af5a060a1e22fd7e3775d3f Mon Sep 17 00:00:00 2001 From: Caleb Zulawski Date: Sat, 13 Jun 2020 11:03:31 -0400 Subject: [PATCH 2/4] Add error note when trying fn as Fn trait --- .../traits/error_reporting/mod.rs | 21 +++++++++ .../rfc-2396-target_feature-11/fn-traits.rs | 10 ++++ .../fn-traits.stderr | 47 +++++++++++++++++-- 3 files changed, 74 insertions(+), 4 deletions(-) diff --git a/src/librustc_trait_selection/traits/error_reporting/mod.rs b/src/librustc_trait_selection/traits/error_reporting/mod.rs index 1b72a4bf84f19..39530853318e3 100644 --- a/src/librustc_trait_selection/traits/error_reporting/mod.rs +++ b/src/librustc_trait_selection/traits/error_reporting/mod.rs @@ -286,6 +286,20 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { .starts_with("std::convert::From<"); let is_unsize = { Some(trait_ref.def_id()) == self.tcx.lang_items().unsize_trait() }; + let is_fn_trait = [ + self.tcx.lang_items().fn_trait(), + self.tcx.lang_items().fn_mut_trait(), + self.tcx.lang_items().fn_once_trait(), + ] + .contains(&Some(trait_ref.def_id())); + let is_safe_target_feature_fn = + if let ty::FnDef(def_id, _) = trait_ref.skip_binder().self_ty().kind { + trait_ref.skip_binder().self_ty().fn_sig(self.tcx).unsafety() + == hir::Unsafety::Normal + && !self.tcx.codegen_fn_attrs(def_id).target_features.is_empty() + } else { + false + }; let (message, note) = if is_try && is_from { ( Some(format!( @@ -427,6 +441,13 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { ); } + if is_fn_trait && is_safe_target_feature_fn { + err.note(&format!( + "`{}` has `#[target_feature]` and is unsafe to call", + trait_ref.skip_binder().self_ty(), + )); + } + // Try to report a help message if !trait_ref.has_infer_types_or_consts() && self.predicate_can_apply(obligation.param_env, trait_ref) diff --git a/src/test/ui/rfcs/rfc-2396-target_feature-11/fn-traits.rs b/src/test/ui/rfcs/rfc-2396-target_feature-11/fn-traits.rs index 58070b69bc1f8..5c838fd719cd9 100644 --- a/src/test/ui/rfcs/rfc-2396-target_feature-11/fn-traits.rs +++ b/src/test/ui/rfcs/rfc-2396-target_feature-11/fn-traits.rs @@ -5,6 +5,9 @@ #[target_feature(enable = "avx")] fn foo() {} +#[target_feature(enable = "avx")] +unsafe fn foo_unsafe() {} + fn call(f: impl Fn()) { f() } @@ -21,4 +24,11 @@ fn main() { call(foo); //~ ERROR expected a `std::ops::Fn<()>` closure, found `fn() {foo}` call_mut(foo); //~ ERROR expected a `std::ops::FnMut<()>` closure, found `fn() {foo}` call_once(foo); //~ ERROR expected a `std::ops::FnOnce<()>` closure, found `fn() {foo}` + + call(foo_unsafe); + //~^ ERROR expected a `std::ops::Fn<()>` closure, found `unsafe fn() {foo_unsafe}` + call_mut(foo_unsafe); + //~^ ERROR expected a `std::ops::FnMut<()>` closure, found `unsafe fn() {foo_unsafe}` + call_once(foo_unsafe); + //~^ ERROR expected a `std::ops::FnOnce<()>` closure, found `unsafe fn() {foo_unsafe}` } diff --git a/src/test/ui/rfcs/rfc-2396-target_feature-11/fn-traits.stderr b/src/test/ui/rfcs/rfc-2396-target_feature-11/fn-traits.stderr index 6dcf31b4c742a..3ae85af76d306 100644 --- a/src/test/ui/rfcs/rfc-2396-target_feature-11/fn-traits.stderr +++ b/src/test/ui/rfcs/rfc-2396-target_feature-11/fn-traits.stderr @@ -1,5 +1,5 @@ error[E0277]: expected a `std::ops::Fn<()>` closure, found `fn() {foo}` - --> $DIR/fn-traits.rs:21:10 + --> $DIR/fn-traits.rs:24:10 | LL | fn call(f: impl Fn()) { | ---- required by this bound in `call` @@ -9,9 +9,10 @@ LL | call(foo); | = help: the trait `std::ops::Fn<()>` is not implemented for `fn() {foo}` = note: wrap the `fn() {foo}` in a closure with no arguments: `|| { /* code */ } + = note: `fn() {foo}` has `#[target_feature]` and is unsafe to call error[E0277]: expected a `std::ops::FnMut<()>` closure, found `fn() {foo}` - --> $DIR/fn-traits.rs:22:14 + --> $DIR/fn-traits.rs:25:14 | LL | fn call_mut(f: impl FnMut()) { | ------- required by this bound in `call_mut` @@ -21,9 +22,10 @@ LL | call_mut(foo); | = help: the trait `std::ops::FnMut<()>` is not implemented for `fn() {foo}` = note: wrap the `fn() {foo}` in a closure with no arguments: `|| { /* code */ } + = note: `fn() {foo}` has `#[target_feature]` and is unsafe to call error[E0277]: expected a `std::ops::FnOnce<()>` closure, found `fn() {foo}` - --> $DIR/fn-traits.rs:23:15 + --> $DIR/fn-traits.rs:26:15 | LL | fn call_once(f: impl FnOnce()) { | -------- required by this bound in `call_once` @@ -33,7 +35,44 @@ LL | call_once(foo); | = help: the trait `std::ops::FnOnce<()>` is not implemented for `fn() {foo}` = note: wrap the `fn() {foo}` in a closure with no arguments: `|| { /* code */ } + = note: `fn() {foo}` has `#[target_feature]` and is unsafe to call -error: aborting due to 3 previous errors +error[E0277]: expected a `std::ops::Fn<()>` closure, found `unsafe fn() {foo_unsafe}` + --> $DIR/fn-traits.rs:28:10 + | +LL | fn call(f: impl Fn()) { + | ---- required by this bound in `call` +... +LL | call(foo_unsafe); + | ^^^^^^^^^^ expected an `Fn<()>` closure, found `unsafe fn() {foo_unsafe}` + | + = help: the trait `std::ops::Fn<()>` is not implemented for `unsafe fn() {foo_unsafe}` + = note: wrap the `unsafe fn() {foo_unsafe}` in a closure with no arguments: `|| { /* code */ } + +error[E0277]: expected a `std::ops::FnMut<()>` closure, found `unsafe fn() {foo_unsafe}` + --> $DIR/fn-traits.rs:30:14 + | +LL | fn call_mut(f: impl FnMut()) { + | ------- required by this bound in `call_mut` +... +LL | call_mut(foo_unsafe); + | ^^^^^^^^^^ expected an `FnMut<()>` closure, found `unsafe fn() {foo_unsafe}` + | + = help: the trait `std::ops::FnMut<()>` is not implemented for `unsafe fn() {foo_unsafe}` + = note: wrap the `unsafe fn() {foo_unsafe}` in a closure with no arguments: `|| { /* code */ } + +error[E0277]: expected a `std::ops::FnOnce<()>` closure, found `unsafe fn() {foo_unsafe}` + --> $DIR/fn-traits.rs:32:15 + | +LL | fn call_once(f: impl FnOnce()) { + | -------- required by this bound in `call_once` +... +LL | call_once(foo_unsafe); + | ^^^^^^^^^^ expected an `FnOnce<()>` closure, found `unsafe fn() {foo_unsafe}` + | + = help: the trait `std::ops::FnOnce<()>` is not implemented for `unsafe fn() {foo_unsafe}` + = note: wrap the `unsafe fn() {foo_unsafe}` in a closure with no arguments: `|| { /* code */ } + +error: aborting due to 6 previous errors For more information about this error, try `rustc --explain E0277`. From 8e899b1cbe6209842e112f0b7ec450a3cdfdb36d Mon Sep 17 00:00:00 2001 From: Caleb Zulawski Date: Sat, 27 Jun 2020 15:51:51 -0400 Subject: [PATCH 3/4] Don't implement Fn* for unsafe #[target_feature] functions --- .../traits/error_reporting/mod.rs | 8 +++----- .../ui/rfcs/rfc-2396-target_feature-11/fn-traits.stderr | 3 +++ 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/src/librustc_trait_selection/traits/error_reporting/mod.rs b/src/librustc_trait_selection/traits/error_reporting/mod.rs index 39530853318e3..76e01a40a2362 100644 --- a/src/librustc_trait_selection/traits/error_reporting/mod.rs +++ b/src/librustc_trait_selection/traits/error_reporting/mod.rs @@ -292,11 +292,9 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { self.tcx.lang_items().fn_once_trait(), ] .contains(&Some(trait_ref.def_id())); - let is_safe_target_feature_fn = + let is_target_feature_fn = if let ty::FnDef(def_id, _) = trait_ref.skip_binder().self_ty().kind { - trait_ref.skip_binder().self_ty().fn_sig(self.tcx).unsafety() - == hir::Unsafety::Normal - && !self.tcx.codegen_fn_attrs(def_id).target_features.is_empty() + !self.tcx.codegen_fn_attrs(def_id).target_features.is_empty() } else { false }; @@ -441,7 +439,7 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { ); } - if is_fn_trait && is_safe_target_feature_fn { + if is_fn_trait && is_target_feature_fn { err.note(&format!( "`{}` has `#[target_feature]` and is unsafe to call", trait_ref.skip_binder().self_ty(), diff --git a/src/test/ui/rfcs/rfc-2396-target_feature-11/fn-traits.stderr b/src/test/ui/rfcs/rfc-2396-target_feature-11/fn-traits.stderr index 3ae85af76d306..ba3d2eac1a6b0 100644 --- a/src/test/ui/rfcs/rfc-2396-target_feature-11/fn-traits.stderr +++ b/src/test/ui/rfcs/rfc-2396-target_feature-11/fn-traits.stderr @@ -48,6 +48,7 @@ LL | call(foo_unsafe); | = help: the trait `std::ops::Fn<()>` is not implemented for `unsafe fn() {foo_unsafe}` = note: wrap the `unsafe fn() {foo_unsafe}` in a closure with no arguments: `|| { /* code */ } + = note: `unsafe fn() {foo_unsafe}` has `#[target_feature]` and is unsafe to call error[E0277]: expected a `std::ops::FnMut<()>` closure, found `unsafe fn() {foo_unsafe}` --> $DIR/fn-traits.rs:30:14 @@ -60,6 +61,7 @@ LL | call_mut(foo_unsafe); | = help: the trait `std::ops::FnMut<()>` is not implemented for `unsafe fn() {foo_unsafe}` = note: wrap the `unsafe fn() {foo_unsafe}` in a closure with no arguments: `|| { /* code */ } + = note: `unsafe fn() {foo_unsafe}` has `#[target_feature]` and is unsafe to call error[E0277]: expected a `std::ops::FnOnce<()>` closure, found `unsafe fn() {foo_unsafe}` --> $DIR/fn-traits.rs:32:15 @@ -72,6 +74,7 @@ LL | call_once(foo_unsafe); | = help: the trait `std::ops::FnOnce<()>` is not implemented for `unsafe fn() {foo_unsafe}` = note: wrap the `unsafe fn() {foo_unsafe}` in a closure with no arguments: `|| { /* code */ } + = note: `unsafe fn() {foo_unsafe}` has `#[target_feature]` and is unsafe to call error: aborting due to 6 previous errors From 51858dae1e0008f0cef35b5ba73ff115ed8f3c1e Mon Sep 17 00:00:00 2001 From: Caleb Zulawski Date: Tue, 30 Jun 2020 18:42:55 -0400 Subject: [PATCH 4/4] Make #[target_feature] Fn trait error message less confusing --- .../traits/error_reporting/mod.rs | 31 +++++++++---------- .../fn-traits.stderr | 12 +++---- 2 files changed, 21 insertions(+), 22 deletions(-) diff --git a/src/librustc_trait_selection/traits/error_reporting/mod.rs b/src/librustc_trait_selection/traits/error_reporting/mod.rs index 76e01a40a2362..ade3dee0b1286 100644 --- a/src/librustc_trait_selection/traits/error_reporting/mod.rs +++ b/src/librustc_trait_selection/traits/error_reporting/mod.rs @@ -286,18 +286,6 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { .starts_with("std::convert::From<"); let is_unsize = { Some(trait_ref.def_id()) == self.tcx.lang_items().unsize_trait() }; - let is_fn_trait = [ - self.tcx.lang_items().fn_trait(), - self.tcx.lang_items().fn_mut_trait(), - self.tcx.lang_items().fn_once_trait(), - ] - .contains(&Some(trait_ref.def_id())); - let is_target_feature_fn = - if let ty::FnDef(def_id, _) = trait_ref.skip_binder().self_ty().kind { - !self.tcx.codegen_fn_attrs(def_id).target_features.is_empty() - } else { - false - }; let (message, note) = if is_try && is_from { ( Some(format!( @@ -439,11 +427,22 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { ); } + let is_fn_trait = [ + self.tcx.lang_items().fn_trait(), + self.tcx.lang_items().fn_mut_trait(), + self.tcx.lang_items().fn_once_trait(), + ] + .contains(&Some(trait_ref.def_id())); + let is_target_feature_fn = + if let ty::FnDef(def_id, _) = trait_ref.skip_binder().self_ty().kind { + !self.tcx.codegen_fn_attrs(def_id).target_features.is_empty() + } else { + false + }; if is_fn_trait && is_target_feature_fn { - err.note(&format!( - "`{}` has `#[target_feature]` and is unsafe to call", - trait_ref.skip_binder().self_ty(), - )); + err.note( + "`#[target_feature]` functions do not implement the `Fn` traits", + ); } // Try to report a help message diff --git a/src/test/ui/rfcs/rfc-2396-target_feature-11/fn-traits.stderr b/src/test/ui/rfcs/rfc-2396-target_feature-11/fn-traits.stderr index ba3d2eac1a6b0..448077b439e80 100644 --- a/src/test/ui/rfcs/rfc-2396-target_feature-11/fn-traits.stderr +++ b/src/test/ui/rfcs/rfc-2396-target_feature-11/fn-traits.stderr @@ -9,7 +9,7 @@ LL | call(foo); | = help: the trait `std::ops::Fn<()>` is not implemented for `fn() {foo}` = note: wrap the `fn() {foo}` in a closure with no arguments: `|| { /* code */ } - = note: `fn() {foo}` has `#[target_feature]` and is unsafe to call + = note: `#[target_feature]` functions do not implement the `Fn` traits error[E0277]: expected a `std::ops::FnMut<()>` closure, found `fn() {foo}` --> $DIR/fn-traits.rs:25:14 @@ -22,7 +22,7 @@ LL | call_mut(foo); | = help: the trait `std::ops::FnMut<()>` is not implemented for `fn() {foo}` = note: wrap the `fn() {foo}` in a closure with no arguments: `|| { /* code */ } - = note: `fn() {foo}` has `#[target_feature]` and is unsafe to call + = note: `#[target_feature]` functions do not implement the `Fn` traits error[E0277]: expected a `std::ops::FnOnce<()>` closure, found `fn() {foo}` --> $DIR/fn-traits.rs:26:15 @@ -35,7 +35,7 @@ LL | call_once(foo); | = help: the trait `std::ops::FnOnce<()>` is not implemented for `fn() {foo}` = note: wrap the `fn() {foo}` in a closure with no arguments: `|| { /* code */ } - = note: `fn() {foo}` has `#[target_feature]` and is unsafe to call + = note: `#[target_feature]` functions do not implement the `Fn` traits error[E0277]: expected a `std::ops::Fn<()>` closure, found `unsafe fn() {foo_unsafe}` --> $DIR/fn-traits.rs:28:10 @@ -48,7 +48,7 @@ LL | call(foo_unsafe); | = help: the trait `std::ops::Fn<()>` is not implemented for `unsafe fn() {foo_unsafe}` = note: wrap the `unsafe fn() {foo_unsafe}` in a closure with no arguments: `|| { /* code */ } - = note: `unsafe fn() {foo_unsafe}` has `#[target_feature]` and is unsafe to call + = note: `#[target_feature]` functions do not implement the `Fn` traits error[E0277]: expected a `std::ops::FnMut<()>` closure, found `unsafe fn() {foo_unsafe}` --> $DIR/fn-traits.rs:30:14 @@ -61,7 +61,7 @@ LL | call_mut(foo_unsafe); | = help: the trait `std::ops::FnMut<()>` is not implemented for `unsafe fn() {foo_unsafe}` = note: wrap the `unsafe fn() {foo_unsafe}` in a closure with no arguments: `|| { /* code */ } - = note: `unsafe fn() {foo_unsafe}` has `#[target_feature]` and is unsafe to call + = note: `#[target_feature]` functions do not implement the `Fn` traits error[E0277]: expected a `std::ops::FnOnce<()>` closure, found `unsafe fn() {foo_unsafe}` --> $DIR/fn-traits.rs:32:15 @@ -74,7 +74,7 @@ LL | call_once(foo_unsafe); | = help: the trait `std::ops::FnOnce<()>` is not implemented for `unsafe fn() {foo_unsafe}` = note: wrap the `unsafe fn() {foo_unsafe}` in a closure with no arguments: `|| { /* code */ } - = note: `unsafe fn() {foo_unsafe}` has `#[target_feature]` and is unsafe to call + = note: `#[target_feature]` functions do not implement the `Fn` traits error: aborting due to 6 previous errors