Skip to content

Commit 0d6a16a

Browse files
committed
mentioned_items: record all callee and coerced closure types, whether they are FnDef/Closure or not
They may become FnDef during monomorphization!
1 parent f1ec494 commit 0d6a16a

15 files changed

+355
-76
lines changed

compiler/rustc_middle/src/mir/mod.rs

+18-6
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ use std::ops::{Index, IndexMut};
4545
use std::{iter, mem};
4646

4747
pub use self::query::*;
48+
use self::visit::TyContext;
4849
pub use basic_blocks::BasicBlocks;
4950

5051
mod basic_blocks;
@@ -317,15 +318,15 @@ impl<'tcx> CoroutineInfo<'tcx> {
317318
#[derive(Copy, Clone, PartialEq, Eq, Debug, Hash, HashStable, TyEncodable, TyDecodable)]
318319
#[derive(TypeFoldable, TypeVisitable)]
319320
pub enum MentionedItem<'tcx> {
320-
Fn(DefId, GenericArgsRef<'tcx>),
321+
/// A function that gets called. We don't necessarily know its precise type yet, since it can be
322+
/// hidden behind a generic.
323+
Fn(Ty<'tcx>),
324+
/// A type that has its drop shim called.
321325
Drop(Ty<'tcx>),
322326
/// Unsizing casts might require vtables, so we have to record them.
323-
UnsizeCast {
324-
source_ty: Ty<'tcx>,
325-
target_ty: Ty<'tcx>,
326-
},
327+
UnsizeCast { source_ty: Ty<'tcx>, target_ty: Ty<'tcx> },
327328
/// A closure that is coerced to a function pointer.
328-
Closure(DefId, GenericArgsRef<'tcx>),
329+
Closure(Ty<'tcx>),
329330
}
330331

331332
/// The lowered representation of a single function.
@@ -610,6 +611,17 @@ impl<'tcx> Body<'tcx> {
610611
}
611612
}
612613

614+
pub fn span_for_ty_context(&self, ty_context: TyContext) -> Span {
615+
match ty_context {
616+
TyContext::UserTy(span) => span,
617+
TyContext::ReturnTy(source_info)
618+
| TyContext::LocalDecl { source_info, .. }
619+
| TyContext::YieldTy(source_info)
620+
| TyContext::ResumeTy(source_info) => source_info.span,
621+
TyContext::Location(loc) => self.source_info(loc).span,
622+
}
623+
}
624+
613625
/// Returns the return type; it always return first element from `local_decls` array.
614626
#[inline]
615627
pub fn return_ty(&self) -> Ty<'tcx> {

compiler/rustc_mir_transform/src/inline.rs

+3-9
Original file line numberDiff line numberDiff line change
@@ -723,15 +723,9 @@ impl<'tcx> Inliner<'tcx> {
723723
// some extra work here to save the monomorphization collector work later. It helps a lot,
724724
// since monomorphization can avoid a lot of work when the "mentioned items" are similar to
725725
// the actually used items. By doing this we can entirely avoid visiting the callee!
726-
let callee_item = {
727-
// We need to reconstruct the `required_item` for the callee so that we can find and
728-
// remove it.
729-
let func_ty = func.ty(caller_body, self.tcx);
730-
match func_ty.kind() {
731-
ty::FnDef(def_id, args) => MentionedItem::Fn(*def_id, args),
732-
_ => bug!(),
733-
}
734-
};
726+
// We need to reconstruct the `required_item` for the callee so that we can find and
727+
// remove it.
728+
let callee_item = MentionedItem::Fn(func.ty(caller_body, self.tcx));
735729
if let Some(idx) =
736730
caller_body.mentioned_items.iter().position(|item| item.node == callee_item)
737731
{

compiler/rustc_mir_transform/src/mentioned_items.rs

+42-29
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
use rustc_middle::mir::visit::Visitor;
2-
use rustc_middle::mir::{self, ConstOperand, Location, MentionedItem, MirPass};
3-
use rustc_middle::ty::{self, adjustment::PointerCoercion, TyCtxt};
2+
use rustc_middle::mir::{self, Location, MentionedItem, MirPass};
3+
use rustc_middle::ty::{adjustment::PointerCoercion, TyCtxt};
44
use rustc_session::Session;
55
use rustc_span::source_map::Spanned;
66

@@ -29,34 +29,44 @@ impl<'tcx> MirPass<'tcx> for MentionedItems {
2929
}
3030
}
3131

32+
// This visitor is carefully in sync with the one in `rustc_monomorphize::collector`. We are
33+
// visiting the exact same places but then instead of monomorphizing and creating `MonoItems`, we
34+
// have to remain generic and just recording the relevant information in `mentioned_items`, where it
35+
// will then be monomorphized later during "mentioned items" collection.
3236
impl<'tcx> Visitor<'tcx> for MentionedItemsVisitor<'_, 'tcx> {
33-
fn visit_constant(&mut self, constant: &ConstOperand<'tcx>, _: Location) {
34-
let const_ = constant.const_;
35-
// This is how function items get referenced: via constants of `FnDef` type. This handles
36-
// both functions that are called and those that are just turned to function pointers.
37-
if let ty::FnDef(def_id, args) = const_.ty().kind() {
38-
debug!("adding to required_items: {def_id:?}");
39-
self.mentioned_items
40-
.push(Spanned { node: MentionedItem::Fn(*def_id, args), span: constant.span });
41-
}
42-
}
43-
4437
fn visit_terminator(&mut self, terminator: &mir::Terminator<'tcx>, location: Location) {
4538
self.super_terminator(terminator, location);
46-
match terminator.kind {
47-
// We don't need to handle `Call` as we already handled all function type operands in
48-
// `visit_constant`. But we do need to handle `Drop`.
39+
let span = || self.body.source_info(location).span;
40+
match &terminator.kind {
41+
mir::TerminatorKind::Call { func, .. } => {
42+
let callee_ty = func.ty(self.body, self.tcx);
43+
self.mentioned_items
44+
.push(Spanned { node: MentionedItem::Fn(callee_ty), span: span() });
45+
}
4946
mir::TerminatorKind::Drop { place, .. } => {
5047
let ty = place.ty(self.body, self.tcx).ty;
51-
let span = self.body.source_info(location).span;
52-
self.mentioned_items.push(Spanned { node: MentionedItem::Drop(ty), span });
48+
self.mentioned_items.push(Spanned { node: MentionedItem::Drop(ty), span: span() });
49+
}
50+
mir::TerminatorKind::InlineAsm { ref operands, .. } => {
51+
for op in operands {
52+
match *op {
53+
mir::InlineAsmOperand::SymFn { ref value } => {
54+
self.mentioned_items.push(Spanned {
55+
node: MentionedItem::Fn(value.const_.ty()),
56+
span: span(),
57+
});
58+
}
59+
_ => {}
60+
}
61+
}
5362
}
5463
_ => {}
5564
}
5665
}
5766

5867
fn visit_rvalue(&mut self, rvalue: &mir::Rvalue<'tcx>, location: Location) {
5968
self.super_rvalue(rvalue, location);
69+
let span = || self.body.source_info(location).span;
6070
match *rvalue {
6171
// We need to detect unsizing casts that required vtables.
6272
mir::Rvalue::Cast(
@@ -65,13 +75,14 @@ impl<'tcx> Visitor<'tcx> for MentionedItemsVisitor<'_, 'tcx> {
6575
target_ty,
6676
)
6777
| mir::Rvalue::Cast(mir::CastKind::DynStar, ref operand, target_ty) => {
68-
let span = self.body.source_info(location).span;
78+
// This isn't monomorphized yet so we can't tell what the actual types are -- just
79+
// add everything.
6980
self.mentioned_items.push(Spanned {
7081
node: MentionedItem::UnsizeCast {
7182
source_ty: operand.ty(self.body, self.tcx),
7283
target_ty,
7384
},
74-
span,
85+
span: span(),
7586
});
7687
}
7788
// Similarly, record closures that are turned into function pointers.
@@ -80,17 +91,19 @@ impl<'tcx> Visitor<'tcx> for MentionedItemsVisitor<'_, 'tcx> {
8091
ref operand,
8192
_,
8293
) => {
83-
let span = self.body.source_info(location).span;
8494
let source_ty = operand.ty(self.body, self.tcx);
85-
match *source_ty.kind() {
86-
ty::Closure(def_id, args) => {
87-
self.mentioned_items
88-
.push(Spanned { node: MentionedItem::Closure(def_id, args), span });
89-
}
90-
_ => bug!(),
91-
}
95+
self.mentioned_items
96+
.push(Spanned { node: MentionedItem::Closure(source_ty), span: span() });
97+
}
98+
// And finally, function pointer reification casts.
99+
mir::Rvalue::Cast(
100+
mir::CastKind::PointerCoercion(PointerCoercion::ReifyFnPointer),
101+
ref operand,
102+
_,
103+
) => {
104+
let fn_ty = operand.ty(self.body, self.tcx);
105+
self.mentioned_items.push(Spanned { node: MentionedItem::Fn(fn_ty), span: span() });
92106
}
93-
// Function pointer casts are already handled by `visit_constant` above.
94107
_ => {}
95108
}
96109
}

compiler/rustc_monomorphize/src/collector.rs

+63-30
Original file line numberDiff line numberDiff line change
@@ -173,6 +173,26 @@
173173
//! do not use a failing constant. This is reflected via the [`CollectionMode`], which determines
174174
//! whether we are visiting a used item or merely a mentioned item.
175175
//!
176+
//! The collector and "mentioned items" gathering (which lives in `rustc_mir_transform::mentioned_items`)
177+
//! need to stay in sync in the following sense:
178+
//!
179+
//! - For every item that the collector gather that could eventually lead to build failure (most
180+
//! likely due to containing a constant that fails to evaluate), a corresponding mentioned item
181+
//! must be added. This should use the exact same strategy as the ecollector to make sure they are
182+
//! in sync. However, while the collector works on monomorphized types, mentioned items are
183+
//! collected on generic MIR -- so any time the collector checks for a particular type (such as
184+
//! `ty::FnDef`), we have to just onconditionally add this as a mentioned item.
185+
//! - In `visit_mentioned_item`, we then do with that mentioned item exactly what the collector
186+
//! would have done during regular MIR visiting. Basically you can think of the collector having
187+
//! two stages, a pre-monomorphization stage and a post-monomorphization stage (usually quite
188+
//! literally separated by a call to `self.monomorphize`); the pre-monomorphizationn stage is
189+
//! duplicated in mentioned items gathering and the post-monomorphization stage is duplicated in
190+
//! `visit_mentioned_item`.
191+
//! - Finally, as a performance optimization, the collector should fill `used_mentioned_item` during
192+
//! its MIR traversal with exactly what mentioned item gathering would have added in the same
193+
//! situation. This detects mentioned items that have *not* been optimized away and hence don't
194+
//! need a dedicated traversal.
195+
//!
176196
//! Open Issues
177197
//! -----------
178198
//! Some things are not yet fully implemented in the current version of this
@@ -904,8 +924,11 @@ impl<'a, 'tcx> MirVisitor<'tcx> for MirUsedCollector<'a, 'tcx> {
904924
target_ty,
905925
)
906926
| mir::Rvalue::Cast(mir::CastKind::DynStar, ref operand, target_ty) => {
907-
let target_ty = self.monomorphize(target_ty);
908927
let source_ty = operand.ty(self.body, self.tcx);
928+
// *Before* monomorphizing, record that we already handled this mention.
929+
self.used_mentioned_items
930+
.insert(MentionedItem::UnsizeCast { source_ty, target_ty });
931+
let target_ty = self.monomorphize(target_ty);
909932
let source_ty = self.monomorphize(source_ty);
910933
let (source_ty, target_ty) =
911934
find_vtable_types_for_unsizing(self.tcx.at(span), source_ty, target_ty);
@@ -930,6 +953,8 @@ impl<'a, 'tcx> MirVisitor<'tcx> for MirUsedCollector<'a, 'tcx> {
930953
_,
931954
) => {
932955
let fn_ty = operand.ty(self.body, self.tcx);
956+
// *Before* monomorphizing, record that we already handled this mention.
957+
self.used_mentioned_items.insert(MentionedItem::Fn(fn_ty));
933958
let fn_ty = self.monomorphize(fn_ty);
934959
visit_fn_use(self.tcx, fn_ty, false, span, self.used_items);
935960
}
@@ -939,20 +964,17 @@ impl<'a, 'tcx> MirVisitor<'tcx> for MirUsedCollector<'a, 'tcx> {
939964
_,
940965
) => {
941966
let source_ty = operand.ty(self.body, self.tcx);
967+
// *Before* monomorphizing, record that we already handled this mention.
968+
self.used_mentioned_items.insert(MentionedItem::Closure(source_ty));
942969
let source_ty = self.monomorphize(source_ty);
943-
match *source_ty.kind() {
944-
ty::Closure(def_id, args) => {
945-
let instance = Instance::resolve_closure(
946-
self.tcx,
947-
def_id,
948-
args,
949-
ty::ClosureKind::FnOnce,
950-
);
951-
if should_codegen_locally(self.tcx, &instance) {
952-
self.used_items.push(create_fn_mono_item(self.tcx, instance, span));
953-
}
970+
if let ty::Closure(def_id, args) = *source_ty.kind() {
971+
let instance =
972+
Instance::resolve_closure(self.tcx, def_id, args, ty::ClosureKind::FnOnce);
973+
if should_codegen_locally(self.tcx, &instance) {
974+
self.used_items.push(create_fn_mono_item(self.tcx, instance, span));
954975
}
955-
_ => bug!(),
976+
} else {
977+
bug!()
956978
}
957979
}
958980
mir::Rvalue::ThreadLocalRef(def_id) => {
@@ -994,9 +1016,7 @@ impl<'a, 'tcx> MirVisitor<'tcx> for MirUsedCollector<'a, 'tcx> {
9941016
mir::TerminatorKind::Call { ref func, ref args, ref fn_span, .. } => {
9951017
let callee_ty = func.ty(self.body, tcx);
9961018
// *Before* monomorphizing, record that we already handled this mention.
997-
if let ty::FnDef(def_id, args) = callee_ty.kind() {
998-
self.used_mentioned_items.insert(MentionedItem::Fn(*def_id, args));
999-
}
1019+
self.used_mentioned_items.insert(MentionedItem::Fn(callee_ty));
10001020
let callee_ty = self.monomorphize(callee_ty);
10011021
self.check_fn_args_move_size(callee_ty, args, *fn_span, location);
10021022
visit_fn_use(self.tcx, callee_ty, true, source, &mut self.used_items)
@@ -1012,7 +1032,10 @@ impl<'a, 'tcx> MirVisitor<'tcx> for MirUsedCollector<'a, 'tcx> {
10121032
for op in operands {
10131033
match *op {
10141034
mir::InlineAsmOperand::SymFn { ref value } => {
1015-
let fn_ty = self.monomorphize(value.const_.ty());
1035+
let fn_ty = value.const_.ty();
1036+
// *Before* monomorphizing, record that we already handled this mention.
1037+
self.used_mentioned_items.insert(MentionedItem::Fn(fn_ty));
1038+
let fn_ty = self.monomorphize(fn_ty);
10161039
visit_fn_use(self.tcx, fn_ty, false, source, self.used_items);
10171040
}
10181041
mir::InlineAsmOperand::SymStatic { def_id } => {
@@ -1076,6 +1099,8 @@ fn visit_drop_use<'tcx>(
10761099
visit_instance_use(tcx, instance, is_direct_call, source, output);
10771100
}
10781101

1102+
/// For every call of this function in the visitor, make sure there is a matching call in the
1103+
/// `mentioned_items` pass!
10791104
fn visit_fn_use<'tcx>(
10801105
tcx: TyCtxt<'tcx>,
10811106
ty: Ty<'tcx>,
@@ -1653,13 +1678,13 @@ fn collect_items_of_instance<'tcx>(
16531678
// Naively, in "used" collection mode, all functions get added to *both* `used_items` and
16541679
// `mentioned_items`. Mentioned items processing will then notice that they have already been
16551680
// visited, but at that point each mentioned item has been monomorphized, added to the
1656-
// `mentioned_items` worklist, and checked in the global set of visited items. To removes that
1681+
// `mentioned_items` worklist, and checked in the global set of visited items. To remove that
16571682
// overhead, we have a special optimization that avoids adding items to `mentioned_items` when
16581683
// they are already added in `used_items`. We could just scan `used_items`, but that's a linear
16591684
// scan and not very efficient. Furthermore we can only do that *after* monomorphizing the
16601685
// mentioned item. So instead we collect all pre-monomorphized `MentionedItem` that were already
16611686
// added to `used_items` in a hash set, which can efficiently query in the
1662-
// `body.mentioned_items` loop below.
1687+
// `body.mentioned_items` loop below without even having to monomorphize the item.
16631688
let mut used_mentioned_items = FxHashSet::<MentionedItem<'tcx>>::default();
16641689
let mut collector = MirUsedCollector {
16651690
tcx,
@@ -1704,13 +1729,16 @@ fn visit_mentioned_item<'tcx>(
17041729
output: &mut MonoItems<'tcx>,
17051730
) {
17061731
match *item {
1707-
MentionedItem::Fn(def_id, args) => {
1708-
let instance = Instance::expect_resolve(tcx, ty::ParamEnv::reveal_all(), def_id, args);
1709-
// `visit_instance_use` was written for "used" item collection but works just as well
1710-
// for "mentioned" item collection.
1711-
// We can set `is_direct_call`; that just means we'll skip a bunch of shims that anyway
1712-
// can't have their own failing constants.
1713-
visit_instance_use(tcx, instance, /*is_direct_call*/ true, span, output);
1732+
MentionedItem::Fn(ty) => {
1733+
if let ty::FnDef(def_id, args) = *ty.kind() {
1734+
let instance =
1735+
Instance::expect_resolve(tcx, ty::ParamEnv::reveal_all(), def_id, args);
1736+
// `visit_instance_use` was written for "used" item collection but works just as well
1737+
// for "mentioned" item collection.
1738+
// We can set `is_direct_call`; that just means we'll skip a bunch of shims that anyway
1739+
// can't have their own failing constants.
1740+
visit_instance_use(tcx, instance, /*is_direct_call*/ true, span, output);
1741+
}
17141742
}
17151743
MentionedItem::Drop(ty) => {
17161744
visit_drop_use(tcx, ty, /*is_direct_call*/ true, span, output);
@@ -1727,10 +1755,15 @@ fn visit_mentioned_item<'tcx>(
17271755
create_mono_items_for_vtable_methods(tcx, target_ty, source_ty, span, output);
17281756
}
17291757
}
1730-
MentionedItem::Closure(def_id, args) => {
1731-
let instance = Instance::resolve_closure(tcx, def_id, args, ty::ClosureKind::FnOnce);
1732-
if should_codegen_locally(tcx, &instance) {
1733-
output.push(create_fn_mono_item(tcx, instance, span));
1758+
MentionedItem::Closure(source_ty) => {
1759+
if let ty::Closure(def_id, args) = *source_ty.kind() {
1760+
let instance =
1761+
Instance::resolve_closure(tcx, def_id, args, ty::ClosureKind::FnOnce);
1762+
if should_codegen_locally(tcx, &instance) {
1763+
output.push(create_fn_mono_item(tcx, instance, span));
1764+
}
1765+
} else {
1766+
bug!()
17341767
}
17351768
}
17361769
}

tests/ui/consts/required-consts/collect-in-dead-closure.opt.stderr

+1-1
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ note: the above error was encountered while instantiating `fn not_called::<i32>`
1616
--> $DIR/collect-in-dead-closure.rs:23:33
1717
|
1818
LL | let _closure: fn() = || not_called::<T>();
19-
| ^^^^^^^^^^^^^^^
19+
| ^^^^^^^^^^^^^^^^^
2020

2121
error: aborting due to 1 previous error
2222

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
error[E0080]: evaluation of `Fail::<i32>::C` failed
2+
--> $DIR/collect-in-dead-fn-behind-assoc-type.rs:9:19
3+
|
4+
LL | const C: () = panic!();
5+
| ^^^^^^^^ the evaluated program panicked at 'explicit panic', $DIR/collect-in-dead-fn-behind-assoc-type.rs:9:19
6+
|
7+
= note: this error originates in the macro `$crate::panic::panic_2015` which comes from the expansion of the macro `panic` (in Nightly builds, run with -Z macro-backtrace for more info)
8+
9+
note: erroneous constant encountered
10+
--> $DIR/collect-in-dead-fn-behind-assoc-type.rs:14:17
11+
|
12+
LL | let _ = Fail::<T>::C;
13+
| ^^^^^^^^^^^^
14+
15+
note: the above error was encountered while instantiating `fn not_called::<i32>`
16+
--> $SRC_DIR/core/src/ops/function.rs:LL:COL
17+
18+
error: aborting due to 1 previous error
19+
20+
For more information about this error, try `rustc --explain E0080`.

0 commit comments

Comments
 (0)