From 91eabf59d5ceb0e45010786e2d5df994d03fd424 Mon Sep 17 00:00:00 2001 From: Rune Tynan Date: Wed, 11 Nov 2020 18:15:39 -0500 Subject: [PATCH 1/5] Add a sane error for rust-call functions not taking tuples during type checking, and associated UI tests --- compiler/rustc_typeck/src/check/mod.rs | 32 ++++++++++++++++++- .../ui/abi/issues/issue-22565-rust-call.rs | 8 +++++ .../abi/issues/issue-22565-rust-call.stderr | 8 +++++ src/test/ui/abi/rustcall-generic.rs | 9 ++++++ .../ui/feature-gates/feature-gate-abi.stderr | 12 +++---- 5 files changed, 62 insertions(+), 7 deletions(-) create mode 100644 src/test/ui/abi/issues/issue-22565-rust-call.rs create mode 100644 src/test/ui/abi/issues/issue-22565-rust-call.stderr create mode 100644 src/test/ui/abi/rustcall-generic.rs diff --git a/compiler/rustc_typeck/src/check/mod.rs b/compiler/rustc_typeck/src/check/mod.rs index 169ad0df3a5c9..67592fa7840b8 100644 --- a/compiler/rustc_typeck/src/check/mod.rs +++ b/compiler/rustc_typeck/src/check/mod.rs @@ -108,7 +108,7 @@ use rustc_hir::def::Res; use rustc_hir::def_id::{CrateNum, DefId, LocalDefId, LOCAL_CRATE}; use rustc_hir::intravisit::Visitor; use rustc_hir::itemlikevisit::ItemLikeVisitor; -use rustc_hir::{HirIdMap, Node}; +use rustc_hir::{HirIdMap, ImplicitSelfKind, Node}; use rustc_index::bit_set::BitSet; use rustc_index::vec::Idx; use rustc_infer::infer::type_variable::{TypeVariableOrigin, TypeVariableOriginKind}; @@ -137,6 +137,7 @@ use crate::util::common::indenter; use self::coercion::DynamicCoerceMany; pub use self::Expectation::*; +use rustc_target::spec::abi; #[macro_export] macro_rules! type_error_struct { @@ -520,6 +521,35 @@ fn typeck_with_fallback<'tcx>( let fn_sig = fixup_opaque_types(tcx, &fn_sig); + if fn_sig.abi == abi::Abi::RustCall { + let expected_args = if let ImplicitSelfKind::None = decl.implicit_self { 1 } else { 2 }; + + let err = || { + if let Node::Item(item) = tcx.hir().get(id) { + if let hir::ItemKind::Fn(header, ..) = &item.kind { + tcx.sess.span_err(header.span, "A function with the \"rust-call\" ABI must take a single non-self argument that is a tuple") + } + } else { + bug!("Couldn't get span of FnHeader being checked") + } + }; + + if fn_sig.inputs().len() != expected_args { + err() + } else { + match fn_sig.inputs()[expected_args - 1].kind() { + ty::Tuple(_) => (), + // FIXME(CraftSpider) Add a check on parameter expansion, so we don't just make the ICE happen later on + // This will probably require wide-scale changes to support a TupleKind obligation + // We can't resolve this without knowing the type of the param + ty::Param(_) => (), + _ => { + err() + } + } + } + } + let fcx = check_fn(&inh, param_env, fn_sig, decl, id, body, None).0; fcx } else { diff --git a/src/test/ui/abi/issues/issue-22565-rust-call.rs b/src/test/ui/abi/issues/issue-22565-rust-call.rs new file mode 100644 index 0000000000000..055d959b46e10 --- /dev/null +++ b/src/test/ui/abi/issues/issue-22565-rust-call.rs @@ -0,0 +1,8 @@ +#![feature(unboxed_closures)] + +extern "rust-call" fn b(_i: i32) {} +//~^ ERROR A function with the "rust-call" ABI must take a single non-self argument that is a tuple + +fn main () { + b(10); +} diff --git a/src/test/ui/abi/issues/issue-22565-rust-call.stderr b/src/test/ui/abi/issues/issue-22565-rust-call.stderr new file mode 100644 index 0000000000000..31fb035eb99af --- /dev/null +++ b/src/test/ui/abi/issues/issue-22565-rust-call.stderr @@ -0,0 +1,8 @@ +error: A function with the "rust-call" ABI must take a single non-self argument that is a tuple + --> $DIR/issue-22565-rust-call.rs:3:1 + | +LL | extern "rust-call" fn b(_i: i32) {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: aborting due to previous error + diff --git a/src/test/ui/abi/rustcall-generic.rs b/src/test/ui/abi/rustcall-generic.rs new file mode 100644 index 0000000000000..2fa41a7e35a74 --- /dev/null +++ b/src/test/ui/abi/rustcall-generic.rs @@ -0,0 +1,9 @@ +// check-pass +#![feature(unboxed_closures)] + +extern "rust-call" fn foo(_: T) {} + +fn main() { + foo(()); + foo((1, 2)); +} diff --git a/src/test/ui/feature-gates/feature-gate-abi.stderr b/src/test/ui/feature-gates/feature-gate-abi.stderr index 50cd8bc68a2a2..25f0c259d111c 100644 --- a/src/test/ui/feature-gates/feature-gate-abi.stderr +++ b/src/test/ui/feature-gates/feature-gate-abi.stderr @@ -26,7 +26,7 @@ LL | extern "vectorcall" fn f3() {} error[E0658]: rust-call ABI is subject to change --> $DIR/feature-gate-abi.rs:18:8 | -LL | extern "rust-call" fn f4() {} +LL | extern "rust-call" fn f4(_: ()) {} | ^^^^^^^^^^^ | = note: see issue #29625 for more information @@ -113,7 +113,7 @@ LL | extern "vectorcall" fn m3(); error[E0658]: rust-call ABI is subject to change --> $DIR/feature-gate-abi.rs:33:12 | -LL | extern "rust-call" fn m4(); +LL | extern "rust-call" fn m4(_: ()); | ^^^^^^^^^^^ | = note: see issue #29625 for more information @@ -183,7 +183,7 @@ LL | extern "vectorcall" fn dm3() {} error[E0658]: rust-call ABI is subject to change --> $DIR/feature-gate-abi.rs:42:12 | -LL | extern "rust-call" fn dm4() {} +LL | extern "rust-call" fn dm4(_: ()) {} | ^^^^^^^^^^^ | = note: see issue #29625 for more information @@ -270,7 +270,7 @@ LL | extern "vectorcall" fn m3() {} error[E0658]: rust-call ABI is subject to change --> $DIR/feature-gate-abi.rs:60:12 | -LL | extern "rust-call" fn m4() {} +LL | extern "rust-call" fn m4(_: ()) {} | ^^^^^^^^^^^ | = note: see issue #29625 for more information @@ -357,7 +357,7 @@ LL | extern "vectorcall" fn im3() {} error[E0658]: rust-call ABI is subject to change --> $DIR/feature-gate-abi.rs:76:12 | -LL | extern "rust-call" fn im4() {} +LL | extern "rust-call" fn im4(_: ()) {} | ^^^^^^^^^^^ | = note: see issue #29625 for more information @@ -444,7 +444,7 @@ LL | type A3 = extern "vectorcall" fn(); error[E0658]: rust-call ABI is subject to change --> $DIR/feature-gate-abi.rs:89:18 | -LL | type A4 = extern "rust-call" fn(); +LL | type A4 = extern "rust-call" fn(_: ()); | ^^^^^^^^^^^ | = note: see issue #29625 for more information From bf04b0428fe6c2f85213ba4f4f3a39fbae2377e7 Mon Sep 17 00:00:00 2001 From: Rune Tynan Date: Wed, 11 Nov 2020 18:38:42 -0500 Subject: [PATCH 2/5] Missing feature-gate-abi file --- src/test/ui/feature-gates/feature-gate-abi.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/test/ui/feature-gates/feature-gate-abi.rs b/src/test/ui/feature-gates/feature-gate-abi.rs index e89dc4d5a05a5..31f09cc61f9ac 100644 --- a/src/test/ui/feature-gates/feature-gate-abi.rs +++ b/src/test/ui/feature-gates/feature-gate-abi.rs @@ -15,7 +15,7 @@ extern "rust-intrinsic" fn f1() {} //~ ERROR intrinsics are subject to change extern "platform-intrinsic" fn f2() {} //~ ERROR platform intrinsics are experimental //~^ ERROR intrinsic must be in extern "vectorcall" fn f3() {} //~ ERROR vectorcall is experimental and subject to change -extern "rust-call" fn f4() {} //~ ERROR rust-call ABI is subject to change +extern "rust-call" fn f4(_: ()) {} //~ ERROR rust-call ABI is subject to change extern "msp430-interrupt" fn f5() {} //~ ERROR msp430-interrupt ABI is experimental extern "ptx-kernel" fn f6() {} //~ ERROR PTX ABIs are experimental and subject to change extern "x86-interrupt" fn f7() {} //~ ERROR x86-interrupt ABI is experimental @@ -30,7 +30,7 @@ trait Tr { extern "platform-intrinsic" fn m2(); //~ ERROR platform intrinsics are experimental //~^ ERROR intrinsic must be in extern "vectorcall" fn m3(); //~ ERROR vectorcall is experimental and subject to change - extern "rust-call" fn m4(); //~ ERROR rust-call ABI is subject to change + extern "rust-call" fn m4(_: ()); //~ ERROR rust-call ABI is subject to change extern "msp430-interrupt" fn m5(); //~ ERROR msp430-interrupt ABI is experimental extern "ptx-kernel" fn m6(); //~ ERROR PTX ABIs are experimental and subject to change extern "x86-interrupt" fn m7(); //~ ERROR x86-interrupt ABI is experimental @@ -39,7 +39,7 @@ trait Tr { extern "efiapi" fn m10(); //~ ERROR efiapi ABI is experimental and subject to change extern "vectorcall" fn dm3() {} //~ ERROR vectorcall is experimental and subject to change - extern "rust-call" fn dm4() {} //~ ERROR rust-call ABI is subject to change + extern "rust-call" fn dm4(_: ()) {} //~ ERROR rust-call ABI is subject to change extern "msp430-interrupt" fn dm5() {} //~ ERROR msp430-interrupt ABI is experimental extern "ptx-kernel" fn dm6() {} //~ ERROR PTX ABIs are experimental and subject to change extern "x86-interrupt" fn dm7() {} //~ ERROR x86-interrupt ABI is experimental @@ -57,7 +57,7 @@ impl Tr for S { extern "platform-intrinsic" fn m2() {} //~ ERROR platform intrinsics are experimental //~^ ERROR intrinsic must be in extern "vectorcall" fn m3() {} //~ ERROR vectorcall is experimental and subject to change - extern "rust-call" fn m4() {} //~ ERROR rust-call ABI is subject to change + extern "rust-call" fn m4(_: ()) {} //~ ERROR rust-call ABI is subject to change extern "msp430-interrupt" fn m5() {} //~ ERROR msp430-interrupt ABI is experimental extern "ptx-kernel" fn m6() {} //~ ERROR PTX ABIs are experimental and subject to change extern "x86-interrupt" fn m7() {} //~ ERROR x86-interrupt ABI is experimental @@ -73,7 +73,7 @@ impl S { extern "platform-intrinsic" fn im2() {} //~ ERROR platform intrinsics are experimental //~^ ERROR intrinsic must be in extern "vectorcall" fn im3() {} //~ ERROR vectorcall is experimental and subject to change - extern "rust-call" fn im4() {} //~ ERROR rust-call ABI is subject to change + extern "rust-call" fn im4(_: ()) {} //~ ERROR rust-call ABI is subject to change extern "msp430-interrupt" fn im5() {} //~ ERROR msp430-interrupt ABI is experimental extern "ptx-kernel" fn im6() {} //~ ERROR PTX ABIs are experimental and subject to change extern "x86-interrupt" fn im7() {} //~ ERROR x86-interrupt ABI is experimental @@ -86,7 +86,7 @@ impl S { type A1 = extern "rust-intrinsic" fn(); //~ ERROR intrinsics are subject to change type A2 = extern "platform-intrinsic" fn(); //~ ERROR platform intrinsics are experimental type A3 = extern "vectorcall" fn(); //~ ERROR vectorcall is experimental and subject to change -type A4 = extern "rust-call" fn(); //~ ERROR rust-call ABI is subject to change +type A4 = extern "rust-call" fn(_: ()); //~ ERROR rust-call ABI is subject to change type A5 = extern "msp430-interrupt" fn(); //~ ERROR msp430-interrupt ABI is experimental type A6 = extern "ptx-kernel" fn (); //~ ERROR PTX ABIs are experimental and subject to change type A7 = extern "x86-interrupt" fn(); //~ ERROR x86-interrupt ABI is experimental From 1e9d5c70c1c435cd9ae217ab0adbb053e643b01f Mon Sep 17 00:00:00 2001 From: Rune Tynan Date: Wed, 11 Nov 2020 20:16:31 -0500 Subject: [PATCH 3/5] Minor stylistic / review changes --- compiler/rustc_typeck/src/check/mod.rs | 24 ++++++++++-------------- 1 file changed, 10 insertions(+), 14 deletions(-) diff --git a/compiler/rustc_typeck/src/check/mod.rs b/compiler/rustc_typeck/src/check/mod.rs index 67592fa7840b8..04cf1bd5b2564 100644 --- a/compiler/rustc_typeck/src/check/mod.rs +++ b/compiler/rustc_typeck/src/check/mod.rs @@ -525,27 +525,23 @@ fn typeck_with_fallback<'tcx>( let expected_args = if let ImplicitSelfKind::None = decl.implicit_self { 1 } else { 2 }; let err = || { - if let Node::Item(item) = tcx.hir().get(id) { - if let hir::ItemKind::Fn(header, ..) = &item.kind { - tcx.sess.span_err(header.span, "A function with the \"rust-call\" ABI must take a single non-self argument that is a tuple") - } + let item = tcx.hir().expect_item(id); + + if let hir::ItemKind::Fn(header, ..) = &item.kind { + tcx.sess.span_err(header.span, "A function with the \"rust-call\" ABI must take a single non-self argument that is a tuple") } else { - bug!("Couldn't get span of FnHeader being checked") + bug!("Item being checked wasn't a function") } }; if fn_sig.inputs().len() != expected_args { err() } else { - match fn_sig.inputs()[expected_args - 1].kind() { - ty::Tuple(_) => (), - // FIXME(CraftSpider) Add a check on parameter expansion, so we don't just make the ICE happen later on - // This will probably require wide-scale changes to support a TupleKind obligation - // We can't resolve this without knowing the type of the param - ty::Param(_) => (), - _ => { - err() - } + // FIXME(CraftSpider) Add a check on parameter expansion, so we don't just make the ICE happen later on + // This will probably require wide-scale changes to support a TupleKind obligation + // We can't resolve this without knowing the type of the param + if !matches!(fn_sig.inputs()[expected_args - 1].kind(), ty::Tuple(_) | ty::Param(_)) { + err() } } } From c825c74dc9ab5e5f72e9e1736ddf3cb58e3547ed Mon Sep 17 00:00:00 2001 From: Rune Tynan Date: Tue, 17 Nov 2020 14:42:29 -0500 Subject: [PATCH 4/5] Move change to check_fn, fix up overloaded-calls-nontuple --- compiler/rustc_typeck/src/check/check.rs | 31 ++++++++++++++++++++ compiler/rustc_typeck/src/check/mod.rs | 25 ---------------- src/test/ui/overloaded-calls-nontuple.rs | 2 ++ src/test/ui/overloaded-calls-nontuple.stderr | 16 ++++++++-- 4 files changed, 47 insertions(+), 27 deletions(-) diff --git a/compiler/rustc_typeck/src/check/check.rs b/compiler/rustc_typeck/src/check/check.rs index 70d94ef869d05..9d5d1e8c8e235 100644 --- a/compiler/rustc_typeck/src/check/check.rs +++ b/compiler/rustc_typeck/src/check/check.rs @@ -94,6 +94,37 @@ pub(super) fn check_fn<'a, 'tcx>( fn_maybe_err(tcx, span, fn_sig.abi); + if fn_sig.abi == abi::Abi::RustCall { + let expected_args = if let ImplicitSelfKind::None = decl.implicit_self { 1 } else { 2 }; + + let err = || { + let item = match tcx.hir().get(fn_id) { + Node::Item(hir::Item { kind: ItemKind::Fn(header, ..), .. }) => Some(header), + Node::ImplItem(hir::ImplItem { + kind: hir::ImplItemKind::Fn(header, ..), .. + }) => Some(header), + // Closures are RustCall, but they tuple their arguments, so shouldn't be checked + Node::Expr(hir::Expr { kind: hir::ExprKind::Closure(..), .. }) => None, + node => bug!("Item being checked wasn't a function/closure: {:?}", node), + }; + + if let Some(header) = item { + tcx.sess.span_err(header.span, "A function with the \"rust-call\" ABI must take a single non-self argument that is a tuple") + } + }; + + if fn_sig.inputs().len() != expected_args { + err() + } else { + // FIXME(CraftSpider) Add a check on parameter expansion, so we don't just make the ICE happen later on + // This will probably require wide-scale changes to support a TupleKind obligation + // We can't resolve this without knowing the type of the param + if !matches!(fn_sig.inputs()[expected_args - 1].kind(), ty::Tuple(_) | ty::Param(_)) { + err() + } + } + } + if body.generator_kind.is_some() && can_be_generator.is_some() { let yield_ty = fcx .next_ty_var(TypeVariableOrigin { kind: TypeVariableOriginKind::TypeInference, span }); diff --git a/compiler/rustc_typeck/src/check/mod.rs b/compiler/rustc_typeck/src/check/mod.rs index 04cf1bd5b2564..35854b0340eee 100644 --- a/compiler/rustc_typeck/src/check/mod.rs +++ b/compiler/rustc_typeck/src/check/mod.rs @@ -521,31 +521,6 @@ fn typeck_with_fallback<'tcx>( let fn_sig = fixup_opaque_types(tcx, &fn_sig); - if fn_sig.abi == abi::Abi::RustCall { - let expected_args = if let ImplicitSelfKind::None = decl.implicit_self { 1 } else { 2 }; - - let err = || { - let item = tcx.hir().expect_item(id); - - if let hir::ItemKind::Fn(header, ..) = &item.kind { - tcx.sess.span_err(header.span, "A function with the \"rust-call\" ABI must take a single non-self argument that is a tuple") - } else { - bug!("Item being checked wasn't a function") - } - }; - - if fn_sig.inputs().len() != expected_args { - err() - } else { - // FIXME(CraftSpider) Add a check on parameter expansion, so we don't just make the ICE happen later on - // This will probably require wide-scale changes to support a TupleKind obligation - // We can't resolve this without knowing the type of the param - if !matches!(fn_sig.inputs()[expected_args - 1].kind(), ty::Tuple(_) | ty::Param(_)) { - err() - } - } - } - let fcx = check_fn(&inh, param_env, fn_sig, decl, id, body, None).0; fcx } else { diff --git a/src/test/ui/overloaded-calls-nontuple.rs b/src/test/ui/overloaded-calls-nontuple.rs index 62a7130ddebd1..76b114c55925b 100644 --- a/src/test/ui/overloaded-calls-nontuple.rs +++ b/src/test/ui/overloaded-calls-nontuple.rs @@ -11,11 +11,13 @@ impl FnMut for S { extern "rust-call" fn call_mut(&mut self, z: isize) -> isize { self.x + self.y + z } + //~^^^ ERROR A function with the "rust-call" ABI must take a single non-self argument } impl FnOnce for S { type Output = isize; extern "rust-call" fn call_once(mut self, z: isize) -> isize { self.call_mut(z) } + //~^ ERROR A function with the "rust-call" ABI must take a single non-self argument } fn main() { diff --git a/src/test/ui/overloaded-calls-nontuple.stderr b/src/test/ui/overloaded-calls-nontuple.stderr index 82c12c4b6e19c..bdadb95db2947 100644 --- a/src/test/ui/overloaded-calls-nontuple.stderr +++ b/src/test/ui/overloaded-calls-nontuple.stderr @@ -1,9 +1,21 @@ +error: A function with the "rust-call" ABI must take a single non-self argument that is a tuple + --> $DIR/overloaded-calls-nontuple.rs:11:5 + | +LL | extern "rust-call" fn call_mut(&mut self, z: isize) -> isize { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: A function with the "rust-call" ABI must take a single non-self argument that is a tuple + --> $DIR/overloaded-calls-nontuple.rs:19:5 + | +LL | extern "rust-call" fn call_once(mut self, z: isize) -> isize { self.call_mut(z) } + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + error[E0059]: cannot use call notation; the first type parameter for the function trait is neither a tuple nor unit - --> $DIR/overloaded-calls-nontuple.rs:26:10 + --> $DIR/overloaded-calls-nontuple.rs:28:10 | LL | drop(s(3)) | ^^^^ -error: aborting due to previous error +error: aborting due to 3 previous errors For more information about this error, try `rustc --explain E0059`. From e8426a617b87af7b8254e732ca85951cbae0a6b9 Mon Sep 17 00:00:00 2001 From: Rune Tynan Date: Tue, 17 Nov 2020 15:49:40 -0500 Subject: [PATCH 5/5] Remove unnecessary abi import --- compiler/rustc_typeck/src/check/check.rs | 2 +- compiler/rustc_typeck/src/check/mod.rs | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/compiler/rustc_typeck/src/check/check.rs b/compiler/rustc_typeck/src/check/check.rs index 9d5d1e8c8e235..11a2794d6288e 100644 --- a/compiler/rustc_typeck/src/check/check.rs +++ b/compiler/rustc_typeck/src/check/check.rs @@ -94,7 +94,7 @@ pub(super) fn check_fn<'a, 'tcx>( fn_maybe_err(tcx, span, fn_sig.abi); - if fn_sig.abi == abi::Abi::RustCall { + if fn_sig.abi == Abi::RustCall { let expected_args = if let ImplicitSelfKind::None = decl.implicit_self { 1 } else { 2 }; let err = || { diff --git a/compiler/rustc_typeck/src/check/mod.rs b/compiler/rustc_typeck/src/check/mod.rs index 35854b0340eee..bd3ecc4abb251 100644 --- a/compiler/rustc_typeck/src/check/mod.rs +++ b/compiler/rustc_typeck/src/check/mod.rs @@ -137,7 +137,6 @@ use crate::util::common::indenter; use self::coercion::DynamicCoerceMany; pub use self::Expectation::*; -use rustc_target::spec::abi; #[macro_export] macro_rules! type_error_struct {