Skip to content

Commit 500b05a

Browse files
borsbors
committed
Auto merge of #128792 - compiler-errors:foreign-sig, r=spastorino
Use `FnSig` instead of raw `FnDecl` for `ForeignItemKind::Fn`, fix ICE for `Fn` trait error on safe foreign fn Let's use `hir::FnSig` instead of `hir::FnDecl + hir::Safety` for `ForeignItemKind::Fn`. This consolidates some handling code between normal fns and foreign fns. Separetly, fix an ICE where we weren't handling `Fn` trait errors for safe foreign fns. If perf is bad for the first commit, I can rework the ICE fix to not rely on it. But if perf is good, I prefer we fix and clean up things all at once 👍 r? spastorino Fixes #128764
2 parents 9337f7a + 2be5599 commit 500b05a

27 files changed

+178
-108
lines changed

Diff for: ‎compiler/rustc_ast_lowering/src/delegation.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -189,7 +189,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
189189
) -> hir::FnSig<'hir> {
190190
let header = if let Some(local_sig_id) = sig_id.as_local() {
191191
match self.resolver.delegation_fn_sigs.get(&local_sig_id) {
192-
Some(sig) => self.lower_fn_header(sig.header),
192+
Some(sig) => self.lower_fn_header(sig.header, hir::Safety::Safe),
193193
None => self.generate_header_error(),
194194
}
195195
} else {

Diff for: ‎compiler/rustc_ast_lowering/src/item.rs

+17-7
Original file line numberDiff line numberDiff line change
@@ -237,7 +237,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
237237
});
238238
let sig = hir::FnSig {
239239
decl,
240-
header: this.lower_fn_header(*header),
240+
header: this.lower_fn_header(*header, hir::Safety::Safe),
241241
span: this.lower_span(*fn_sig_span),
242242
};
243243
hir::ItemKind::Fn(sig, generics, body_id)
@@ -668,7 +668,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
668668
ForeignItemKind::Fn(box Fn { sig, generics, .. }) => {
669669
let fdec = &sig.decl;
670670
let itctx = ImplTraitContext::Universal;
671-
let (generics, (fn_dec, fn_args)) =
671+
let (generics, (decl, fn_args)) =
672672
self.lower_generics(generics, Const::No, false, i.id, itctx, |this| {
673673
(
674674
// Disallow `impl Trait` in foreign items.
@@ -682,9 +682,15 @@ impl<'hir> LoweringContext<'_, 'hir> {
682682
this.lower_fn_params_to_names(fdec),
683683
)
684684
});
685-
let safety = self.lower_safety(sig.header.safety, hir::Safety::Unsafe);
686685

687-
hir::ForeignItemKind::Fn(fn_dec, fn_args, generics, safety)
686+
// Unmarked safety in unsafe block defaults to unsafe.
687+
let header = self.lower_fn_header(sig.header, hir::Safety::Unsafe);
688+
689+
hir::ForeignItemKind::Fn(
690+
hir::FnSig { header, decl, span: self.lower_span(sig.span) },
691+
fn_args,
692+
generics,
693+
)
688694
}
689695
ForeignItemKind::Static(box StaticItem { ty, mutability, expr: _, safety }) => {
690696
let ty = self
@@ -1390,7 +1396,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
13901396
coroutine_kind: Option<CoroutineKind>,
13911397
parent_constness: Const,
13921398
) -> (&'hir hir::Generics<'hir>, hir::FnSig<'hir>) {
1393-
let header = self.lower_fn_header(sig.header);
1399+
let header = self.lower_fn_header(sig.header, hir::Safety::Safe);
13941400
// Don't pass along the user-provided constness of trait associated functions; we don't want to
13951401
// synthesize a host effect param for them. We reject `const` on them during AST validation.
13961402
let constness =
@@ -1403,14 +1409,18 @@ impl<'hir> LoweringContext<'_, 'hir> {
14031409
(generics, hir::FnSig { header, decl, span: self.lower_span(sig.span) })
14041410
}
14051411

1406-
pub(super) fn lower_fn_header(&mut self, h: FnHeader) -> hir::FnHeader {
1412+
pub(super) fn lower_fn_header(
1413+
&mut self,
1414+
h: FnHeader,
1415+
default_safety: hir::Safety,
1416+
) -> hir::FnHeader {
14071417
let asyncness = if let Some(CoroutineKind::Async { span, .. }) = h.coroutine_kind {
14081418
hir::IsAsync::Async(span)
14091419
} else {
14101420
hir::IsAsync::NotAsync
14111421
};
14121422
hir::FnHeader {
1413-
safety: self.lower_safety(h.safety, hir::Safety::Safe),
1423+
safety: self.lower_safety(h.safety, default_safety),
14141424
asyncness: asyncness,
14151425
constness: self.lower_constness(h.constness),
14161426
abi: self.lower_extern(h.ext),

Diff for: ‎compiler/rustc_hir/src/hir.rs

+23-16
Original file line numberDiff line numberDiff line change
@@ -3586,7 +3586,7 @@ impl ForeignItem<'_> {
35863586
#[derive(Debug, Clone, Copy, HashStable_Generic)]
35873587
pub enum ForeignItemKind<'hir> {
35883588
/// A foreign function.
3589-
Fn(&'hir FnDecl<'hir>, &'hir [Ident], &'hir Generics<'hir>, Safety),
3589+
Fn(FnSig<'hir>, &'hir [Ident], &'hir Generics<'hir>),
35903590
/// A foreign static item (`static ext: u8`).
35913591
Static(&'hir Ty<'hir>, Mutability, Safety),
35923592
/// A foreign type.
@@ -3645,7 +3645,10 @@ impl<'hir> OwnerNode<'hir> {
36453645
match self {
36463646
OwnerNode::TraitItem(TraitItem { kind: TraitItemKind::Fn(fn_sig, _), .. })
36473647
| OwnerNode::ImplItem(ImplItem { kind: ImplItemKind::Fn(fn_sig, _), .. })
3648-
| OwnerNode::Item(Item { kind: ItemKind::Fn(fn_sig, _, _), .. }) => Some(fn_sig),
3648+
| OwnerNode::Item(Item { kind: ItemKind::Fn(fn_sig, _, _), .. })
3649+
| OwnerNode::ForeignItem(ForeignItem {
3650+
kind: ForeignItemKind::Fn(fn_sig, _, _), ..
3651+
}) => Some(fn_sig),
36493652
_ => None,
36503653
}
36513654
}
@@ -3654,11 +3657,10 @@ impl<'hir> OwnerNode<'hir> {
36543657
match self {
36553658
OwnerNode::TraitItem(TraitItem { kind: TraitItemKind::Fn(fn_sig, _), .. })
36563659
| OwnerNode::ImplItem(ImplItem { kind: ImplItemKind::Fn(fn_sig, _), .. })
3657-
| OwnerNode::Item(Item { kind: ItemKind::Fn(fn_sig, _, _), .. }) => Some(fn_sig.decl),
3658-
OwnerNode::ForeignItem(ForeignItem {
3659-
kind: ForeignItemKind::Fn(fn_decl, _, _, _),
3660-
..
3661-
}) => Some(fn_decl),
3660+
| OwnerNode::Item(Item { kind: ItemKind::Fn(fn_sig, _, _), .. })
3661+
| OwnerNode::ForeignItem(ForeignItem {
3662+
kind: ForeignItemKind::Fn(fn_sig, _, _), ..
3663+
}) => Some(fn_sig.decl),
36623664
_ => None,
36633665
}
36643666
}
@@ -3846,11 +3848,13 @@ impl<'hir> Node<'hir> {
38463848
match self {
38473849
Node::TraitItem(TraitItem { kind: TraitItemKind::Fn(fn_sig, _), .. })
38483850
| Node::ImplItem(ImplItem { kind: ImplItemKind::Fn(fn_sig, _), .. })
3849-
| Node::Item(Item { kind: ItemKind::Fn(fn_sig, _, _), .. }) => Some(fn_sig.decl),
3850-
Node::Expr(Expr { kind: ExprKind::Closure(Closure { fn_decl, .. }), .. })
3851-
| Node::ForeignItem(ForeignItem {
3852-
kind: ForeignItemKind::Fn(fn_decl, _, _, _), ..
3853-
}) => Some(fn_decl),
3851+
| Node::Item(Item { kind: ItemKind::Fn(fn_sig, _, _), .. })
3852+
| Node::ForeignItem(ForeignItem { kind: ForeignItemKind::Fn(fn_sig, _, _), .. }) => {
3853+
Some(fn_sig.decl)
3854+
}
3855+
Node::Expr(Expr { kind: ExprKind::Closure(Closure { fn_decl, .. }), .. }) => {
3856+
Some(fn_decl)
3857+
}
38543858
_ => None,
38553859
}
38563860
}
@@ -3874,7 +3878,10 @@ impl<'hir> Node<'hir> {
38743878
match self {
38753879
Node::TraitItem(TraitItem { kind: TraitItemKind::Fn(fn_sig, _), .. })
38763880
| Node::ImplItem(ImplItem { kind: ImplItemKind::Fn(fn_sig, _), .. })
3877-
| Node::Item(Item { kind: ItemKind::Fn(fn_sig, _, _), .. }) => Some(fn_sig),
3881+
| Node::Item(Item { kind: ItemKind::Fn(fn_sig, _, _), .. })
3882+
| Node::ForeignItem(ForeignItem { kind: ForeignItemKind::Fn(fn_sig, _, _), .. }) => {
3883+
Some(fn_sig)
3884+
}
38783885
_ => None,
38793886
}
38803887
}
@@ -3949,7 +3956,7 @@ impl<'hir> Node<'hir> {
39493956
pub fn generics(self) -> Option<&'hir Generics<'hir>> {
39503957
match self {
39513958
Node::ForeignItem(ForeignItem {
3952-
kind: ForeignItemKind::Fn(_, _, generics, _), ..
3959+
kind: ForeignItemKind::Fn(_, _, generics), ..
39533960
})
39543961
| Node::TraitItem(TraitItem { generics, .. })
39553962
| Node::ImplItem(ImplItem { generics, .. }) => Some(generics),
@@ -4039,8 +4046,8 @@ mod size_asserts {
40394046
static_assert_size!(Expr<'_>, 64);
40404047
static_assert_size!(ExprKind<'_>, 48);
40414048
static_assert_size!(FnDecl<'_>, 40);
4042-
static_assert_size!(ForeignItem<'_>, 72);
4043-
static_assert_size!(ForeignItemKind<'_>, 40);
4049+
static_assert_size!(ForeignItem<'_>, 88);
4050+
static_assert_size!(ForeignItemKind<'_>, 56);
40444051
static_assert_size!(GenericArg<'_>, 16);
40454052
static_assert_size!(GenericBound<'_>, 48);
40464053
static_assert_size!(Generics<'_>, 56);

Diff for: ‎compiler/rustc_hir/src/intravisit.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -611,9 +611,9 @@ pub fn walk_foreign_item<'v, V: Visitor<'v>>(
611611
try_visit!(visitor.visit_ident(foreign_item.ident));
612612

613613
match foreign_item.kind {
614-
ForeignItemKind::Fn(ref function_declaration, param_names, ref generics, _) => {
614+
ForeignItemKind::Fn(ref sig, param_names, ref generics) => {
615615
try_visit!(visitor.visit_generics(generics));
616-
try_visit!(visitor.visit_fn_decl(function_declaration));
616+
try_visit!(visitor.visit_fn_decl(sig.decl));
617617
walk_list!(visitor, visit_ident, param_names.iter().copied());
618618
}
619619
ForeignItemKind::Static(ref typ, _, _) => {

Diff for: ‎compiler/rustc_hir_analysis/src/check/check.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -804,8 +804,8 @@ pub(crate) fn check_item_type(tcx: TyCtxt<'_>, def_id: LocalDefId) {
804804

805805
let item = tcx.hir().foreign_item(item.id);
806806
match &item.kind {
807-
hir::ForeignItemKind::Fn(fn_decl, _, _, _) => {
808-
require_c_abi_if_c_variadic(tcx, fn_decl, abi, item.span);
807+
hir::ForeignItemKind::Fn(sig, _, _) => {
808+
require_c_abi_if_c_variadic(tcx, sig.decl, abi, item.span);
809809
}
810810
hir::ForeignItemKind::Static(..) => {
811811
check_static_inhabited(tcx, def_id);

Diff for: ‎compiler/rustc_hir_analysis/src/check/intrinsic.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ fn equate_intrinsic_type<'tcx>(
3030
let (generics, span) = match tcx.hir_node_by_def_id(def_id) {
3131
hir::Node::Item(hir::Item { kind: hir::ItemKind::Fn(_, generics, _), .. })
3232
| hir::Node::ForeignItem(hir::ForeignItem {
33-
kind: hir::ForeignItemKind::Fn(.., generics, _),
33+
kind: hir::ForeignItemKind::Fn(_, _, generics),
3434
..
3535
}) => (tcx.generics_of(def_id), generics.span),
3636
_ => {

Diff for: ‎compiler/rustc_hir_analysis/src/check/wfcheck.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -350,8 +350,8 @@ fn check_foreign_item<'tcx>(
350350
);
351351

352352
match item.kind {
353-
hir::ForeignItemKind::Fn(decl, ..) => {
354-
check_item_fn(tcx, def_id, item.ident, item.span, decl)
353+
hir::ForeignItemKind::Fn(sig, ..) => {
354+
check_item_fn(tcx, def_id, item.ident, item.span, sig.decl)
355355
}
356356
hir::ForeignItemKind::Static(ty, ..) => {
357357
check_item_type(tcx, def_id, ty.span, UnsizedHandling::AllowIfForeignTail)

Diff for: ‎compiler/rustc_hir_analysis/src/collect.rs

+2-4
Original file line numberDiff line numberDiff line change
@@ -1439,11 +1439,9 @@ fn fn_sig(tcx: TyCtxt<'_>, def_id: LocalDefId) -> ty::EarlyBinder<'_, ty::PolyFn
14391439
icx.lowerer().lower_fn_ty(hir_id, header.safety, header.abi, decl, Some(generics), None)
14401440
}
14411441

1442-
ForeignItem(&hir::ForeignItem {
1443-
kind: ForeignItemKind::Fn(fn_decl, _, _, safety), ..
1444-
}) => {
1442+
ForeignItem(&hir::ForeignItem { kind: ForeignItemKind::Fn(sig, _, _), .. }) => {
14451443
let abi = tcx.hir().get_foreign_abi(hir_id);
1446-
compute_sig_of_foreign_fn_decl(tcx, def_id, fn_decl, abi, safety)
1444+
compute_sig_of_foreign_fn_decl(tcx, def_id, sig.decl, abi, sig.header.safety)
14471445
}
14481446

14491447
Ctor(data) | Variant(hir::Variant { data, .. }) if data.ctor().is_some() => {

Diff for: ‎compiler/rustc_hir_analysis/src/collect/resolve_bound_vars.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -604,7 +604,7 @@ impl<'a, 'tcx> Visitor<'tcx> for BoundVarContext<'a, 'tcx> {
604604

605605
fn visit_foreign_item(&mut self, item: &'tcx hir::ForeignItem<'tcx>) {
606606
match item.kind {
607-
hir::ForeignItemKind::Fn(_, _, generics, _) => {
607+
hir::ForeignItemKind::Fn(_, _, generics) => {
608608
self.visit_early_late(item.hir_id(), generics, |this| {
609609
intravisit::walk_foreign_item(this, item);
610610
})

Diff for: ‎compiler/rustc_hir_pretty/src/lib.rs

+3-8
Original file line numberDiff line numberDiff line change
@@ -352,16 +352,11 @@ impl<'a> State<'a> {
352352
self.maybe_print_comment(item.span.lo());
353353
self.print_outer_attributes(self.attrs(item.hir_id()));
354354
match item.kind {
355-
hir::ForeignItemKind::Fn(decl, arg_names, generics, safety) => {
355+
hir::ForeignItemKind::Fn(sig, arg_names, generics) => {
356356
self.head("");
357357
self.print_fn(
358-
decl,
359-
hir::FnHeader {
360-
safety,
361-
constness: hir::Constness::NotConst,
362-
abi: Abi::Rust,
363-
asyncness: hir::IsAsync::NotAsync,
364-
},
358+
sig.decl,
359+
sig.header,
365360
Some(item.ident.name),
366361
generics,
367362
arg_names,

Diff for: ‎compiler/rustc_lint/src/types.rs

+6-3
Original file line numberDiff line numberDiff line change
@@ -1736,13 +1736,16 @@ impl<'tcx> LateLintPass<'tcx> for ImproperCTypesDeclarations {
17361736
let abi = cx.tcx.hir().get_foreign_abi(it.hir_id());
17371737

17381738
match it.kind {
1739-
hir::ForeignItemKind::Fn(decl, _, _, _) if !vis.is_internal_abi(abi) => {
1740-
vis.check_foreign_fn(it.owner_id.def_id, decl);
1739+
hir::ForeignItemKind::Fn(sig, _, _) => {
1740+
if vis.is_internal_abi(abi) {
1741+
vis.check_fn(it.owner_id.def_id, sig.decl)
1742+
} else {
1743+
vis.check_foreign_fn(it.owner_id.def_id, sig.decl);
1744+
}
17411745
}
17421746
hir::ForeignItemKind::Static(ty, _, _) if !vis.is_internal_abi(abi) => {
17431747
vis.check_foreign_static(it.owner_id, ty.span);
17441748
}
1745-
hir::ForeignItemKind::Fn(decl, _, _, _) => vis.check_fn(it.owner_id.def_id, decl),
17461749
hir::ForeignItemKind::Static(..) | hir::ForeignItemKind::Type => (),
17471750
}
17481751
}

Diff for: ‎compiler/rustc_middle/src/hir/map/mod.rs

+6-4
Original file line numberDiff line numberDiff line change
@@ -826,6 +826,11 @@ impl<'hir> Map<'hir> {
826826
})
827827
| Node::ImplItem(ImplItem {
828828
kind: ImplItemKind::Fn(sig, ..), span: outer_span, ..
829+
})
830+
| Node::ForeignItem(ForeignItem {
831+
kind: ForeignItemKind::Fn(sig, ..),
832+
span: outer_span,
833+
..
829834
}) => {
830835
// Ensure that the returned span has the item's SyntaxContext, and not the
831836
// SyntaxContext of the visibility.
@@ -884,10 +889,7 @@ impl<'hir> Map<'hir> {
884889
},
885890
Node::Variant(variant) => named_span(variant.span, variant.ident, None),
886891
Node::ImplItem(item) => named_span(item.span, item.ident, Some(item.generics)),
887-
Node::ForeignItem(item) => match item.kind {
888-
ForeignItemKind::Fn(decl, _, _, _) => until_within(item.span, decl.output.span()),
889-
_ => named_span(item.span, item.ident, None),
890-
},
892+
Node::ForeignItem(item) => named_span(item.span, item.ident, None),
891893
Node::Ctor(_) => return self.span(self.tcx.parent_hir_id(hir_id)),
892894
Node::Expr(Expr {
893895
kind: ExprKind::Closure(Closure { fn_decl_span, .. }),

Diff for: ‎compiler/rustc_middle/src/hir/mod.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -202,7 +202,7 @@ pub fn provide(providers: &mut Providers) {
202202
..
203203
})
204204
| Node::ForeignItem(&ForeignItem {
205-
kind: ForeignItemKind::Fn(_, idents, _, _),
205+
kind: ForeignItemKind::Fn(_, idents, _),
206206
..
207207
}) = tcx.hir_node(tcx.local_def_id_to_hir_id(def_id))
208208
{

Diff for: ‎compiler/rustc_trait_selection/src/error_reporting/traits/fulfillment_errors.rs

+4
Original file line numberDiff line numberDiff line change
@@ -2743,6 +2743,10 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> {
27432743
| Node::ImplItem(&hir::ImplItem { kind: hir::ImplItemKind::Fn(ref sig, _), .. })
27442744
| Node::TraitItem(&hir::TraitItem {
27452745
kind: hir::TraitItemKind::Fn(ref sig, _), ..
2746+
})
2747+
| Node::ForeignItem(&hir::ForeignItem {
2748+
kind: hir::ForeignItemKind::Fn(ref sig, _, _),
2749+
..
27462750
}) => (
27472751
sig.span,
27482752
None,

Diff for: ‎src/librustdoc/clean/mod.rs

+4-10
Original file line numberDiff line numberDiff line change
@@ -3094,16 +3094,10 @@ fn clean_maybe_renamed_foreign_item<'tcx>(
30943094
let def_id = item.owner_id.to_def_id();
30953095
cx.with_param_env(def_id, |cx| {
30963096
let kind = match item.kind {
3097-
hir::ForeignItemKind::Fn(decl, names, generics, safety) => {
3098-
let (generics, decl) = enter_impl_trait(cx, |cx| {
3099-
// NOTE: generics must be cleaned before args
3100-
let generics = clean_generics(generics, cx);
3101-
let args = clean_args_from_types_and_names(cx, decl.inputs, names);
3102-
let decl = clean_fn_decl_with_args(cx, decl, None, args);
3103-
(generics, decl)
3104-
});
3105-
ForeignFunctionItem(Box::new(Function { decl, generics }), safety)
3106-
}
3097+
hir::ForeignItemKind::Fn(sig, names, generics) => ForeignFunctionItem(
3098+
clean_function(cx, &sig, generics, FunctionArgs::Names(names)),
3099+
sig.header.safety,
3100+
),
31073101
hir::ForeignItemKind::Static(ty, mutability, safety) => ForeignStaticItem(
31083102
Static { type_: Box::new(clean_ty(ty, cx)), mutability, expr: None },
31093103
safety,

Diff for: ‎tests/ui/extern/extern-main-issue-86110.stderr

+1-1
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ error: the `main` function cannot be declared in an `extern` block
22
--> $DIR/extern-main-issue-86110.rs:4:5
33
|
44
LL | fn main();
5-
| ^^^^^^^^^
5+
| ^^^^^^^^^^
66

77
error: aborting due to 1 previous error
88

Diff for: ‎tests/ui/foreign/foreign-safe-fn-arg-mismatch.rs

+13
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
// Make sure we don't ICE when a foreign fn doesn't implement `Fn` due to arg mismatch.
2+
3+
unsafe extern "Rust" {
4+
pub safe fn foo();
5+
pub safe fn bar(x: u32);
6+
}
7+
8+
fn test(_: impl Fn(i32)) {}
9+
10+
fn main() {
11+
test(foo); //~ ERROR function is expected to take 1 argument, but it takes 0 arguments
12+
test(bar); //~ ERROR type mismatch in function arguments
13+
}

0 commit comments

Comments
 (0)