Skip to content

Commit

Permalink
mentioned items: also handle vtables
Browse files Browse the repository at this point in the history
  • Loading branch information
RalfJung committed Mar 20, 2024
1 parent ee4b758 commit 347ca50
Show file tree
Hide file tree
Showing 6 changed files with 107 additions and 45 deletions.
6 changes: 5 additions & 1 deletion compiler/rustc_middle/src/mir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -319,7 +319,11 @@ impl<'tcx> CoroutineInfo<'tcx> {
pub enum MentionedItem<'tcx> {
Fn(DefId, GenericArgsRef<'tcx>),
Drop(Ty<'tcx>),
// FIXME: add Vtable { source_ty: Ty<'tcx>, target_ty: Ty<'tcx> },
/// Unsizing casts might require vtables, so we have to record them.
UnsizeCast {
source_ty: Ty<'tcx>,
target_ty: Ty<'tcx>,
},
// FIXME: do we have to add closures?
}

Expand Down
26 changes: 25 additions & 1 deletion compiler/rustc_mir_transform/src/mentioned_items.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use rustc_middle::mir::visit::Visitor;
use rustc_middle::mir::{self, ConstOperand, Location, MentionedItem, MirPass};
use rustc_middle::ty::{self, TyCtxt};
use rustc_middle::ty::{self, adjustment::PointerCoercion, TyCtxt};
use rustc_session::Session;
use rustc_span::source_map::Spanned;

Expand Down Expand Up @@ -54,4 +54,28 @@ impl<'tcx> Visitor<'tcx> for MentionedItemsVisitor<'_, 'tcx> {
_ => {}
}
}

fn visit_rvalue(&mut self, rvalue: &mir::Rvalue<'tcx>, location: Location) {
self.super_rvalue(rvalue, location);
match *rvalue {
// We need to detect unsizing casts that required vtables.
mir::Rvalue::Cast(
mir::CastKind::PointerCoercion(PointerCoercion::Unsize),
ref operand,
target_ty,
)
| mir::Rvalue::Cast(mir::CastKind::DynStar, ref operand, target_ty) => {
let span = self.body.source_info(location).span;
self.mentioned_items.push(Spanned {
node: MentionedItem::UnsizeCast {
source_ty: operand.ty(self.body, self.tcx),
target_ty,
},
span,
});
}
// Function pointer casts are already handled by `visit_constant` above.
_ => {}
}
}
}
75 changes: 45 additions & 30 deletions compiler/rustc_monomorphize/src/collector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -736,7 +736,7 @@ impl<'a, 'tcx> MirUsedCollector<'a, 'tcx> {
where
T: TypeFoldable<TyCtxt<'tcx>>,
{
debug!("monomorphize: self.instance={:?}", self.instance);
trace!("monomorphize: self.instance={:?}", self.instance);
self.instance.instantiate_mir_and_normalize_erasing_regions(
self.tcx,
ty::ParamEnv::reveal_all(),
Expand Down Expand Up @@ -1342,35 +1342,37 @@ fn create_mono_items_for_vtable_methods<'tcx>(
) {
assert!(!trait_ty.has_escaping_bound_vars() && !impl_ty.has_escaping_bound_vars());

if let ty::Dynamic(trait_ty, ..) = trait_ty.kind() {
if let Some(principal) = trait_ty.principal() {
let poly_trait_ref = principal.with_self_ty(tcx, impl_ty);
assert!(!poly_trait_ref.has_escaping_bound_vars());

// Walk all methods of the trait, including those of its supertraits
let entries = tcx.vtable_entries(poly_trait_ref);
let methods = entries
.iter()
.filter_map(|entry| match entry {
VtblEntry::MetadataDropInPlace
| VtblEntry::MetadataSize
| VtblEntry::MetadataAlign
| VtblEntry::Vacant => None,
VtblEntry::TraitVPtr(_) => {
// all super trait items already covered, so skip them.
None
}
VtblEntry::Method(instance) => {
Some(*instance).filter(|instance| should_codegen_locally(tcx, instance))
}
})
.map(|item| create_fn_mono_item(tcx, item, source));
output.extend(methods);
}

// Also add the destructor.
visit_drop_use(tcx, impl_ty, false, source, output);
let ty::Dynamic(trait_ty, ..) = trait_ty.kind() else {
bug!("create_mono_items_for_vtable_methods: {trait_ty:?} not a trait type");
};
if let Some(principal) = trait_ty.principal() {
let poly_trait_ref = principal.with_self_ty(tcx, impl_ty);
assert!(!poly_trait_ref.has_escaping_bound_vars());

// Walk all methods of the trait, including those of its supertraits
let entries = tcx.vtable_entries(poly_trait_ref);
debug!(?entries);
let methods = entries
.iter()
.filter_map(|entry| match entry {
VtblEntry::MetadataDropInPlace
| VtblEntry::MetadataSize
| VtblEntry::MetadataAlign
| VtblEntry::Vacant => None,
VtblEntry::TraitVPtr(_) => {
// all super trait items already covered, so skip them.
None
}
VtblEntry::Method(instance) => {
Some(*instance).filter(|instance| should_codegen_locally(tcx, instance))
}
})
.map(|item| create_fn_mono_item(tcx, item, source));
output.extend(methods);
}

// Also add the destructor.
visit_drop_use(tcx, impl_ty, false, source, output);
}

//=-----------------------------------------------------------------------------
Expand Down Expand Up @@ -1693,7 +1695,8 @@ fn collect_items_of_instance<'tcx>(
}
}

/// `item` must be already monomorphized
/// `item` must be already monomorphized.
#[instrument(skip(tcx, span, output), level = "debug")]
fn visit_mentioned_item<'tcx>(
tcx: TyCtxt<'tcx>,
item: &MentionedItem<'tcx>,
Expand All @@ -1712,6 +1715,18 @@ fn visit_mentioned_item<'tcx>(
MentionedItem::Drop(ty) => {
visit_drop_use(tcx, ty, /*is_direct_call*/ true, span, output);
}
MentionedItem::UnsizeCast { source_ty, target_ty } => {
let (source_ty, target_ty) =
find_vtable_types_for_unsizing(tcx.at(span), source_ty, target_ty);
// This could also be a different Unsize instruction, like
// from a fixed sized array to a slice. But we are only
// interested in things that produce a vtable.
if (target_ty.is_trait() && !source_ty.is_trait())
|| (target_ty.is_dyn_star() && !source_ty.is_dyn_star())
{
create_mono_items_for_vtable_methods(tcx, target_ty, source_ty, span, output);
}
}
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,21 +1,21 @@
error[E0080]: evaluation of `Fail::<i32>::C` failed
--> $DIR/collect-in-dead-vtable.rs:11:19
--> $DIR/collect-in-dead-vtable.rs:8:19
|
LL | const C: () = panic!();
| ^^^^^^^^ the evaluated program panicked at 'explicit panic', $DIR/collect-in-dead-vtable.rs:11:19
| ^^^^^^^^ the evaluated program panicked at 'explicit panic', $DIR/collect-in-dead-vtable.rs:8:19
|
= 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)

note: erroneous constant encountered
--> $DIR/collect-in-dead-vtable.rs:25:21
--> $DIR/collect-in-dead-vtable.rs:21:21
|
LL | let _ = Fail::<T>::C;
| ^^^^^^^^^^^^

note: the above error was encountered while instantiating `fn <std::vec::Vec<i32> as MyTrait>::not_called`
--> $DIR/collect-in-dead-vtable.rs:34:40
--> $DIR/collect-in-dead-vtable.rs:30:40
|
LL | let gen_vtable: &dyn MyTrait = &v; // vtable "appears" here
LL | let gen_vtable: &dyn MyTrait = &v; // vtable is "mentioned" here
| ^^

error: aborting due to 1 previous error
Expand Down
23 changes: 23 additions & 0 deletions tests/ui/consts/required-consts/collect-in-dead-vtable.opt.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
error[E0080]: evaluation of `Fail::<i32>::C` failed
--> $DIR/collect-in-dead-vtable.rs:8:19
|
LL | const C: () = panic!();
| ^^^^^^^^ the evaluated program panicked at 'explicit panic', $DIR/collect-in-dead-vtable.rs:8:19
|
= 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)

note: erroneous constant encountered
--> $DIR/collect-in-dead-vtable.rs:21:21
|
LL | let _ = Fail::<T>::C;
| ^^^^^^^^^^^^

note: the above error was encountered while instantiating `fn <std::vec::Vec<i32> as MyTrait>::not_called`
--> $DIR/collect-in-dead-vtable.rs:30:40
|
LL | let gen_vtable: &dyn MyTrait = &v; // vtable is "mentioned" here
| ^^

error: aborting due to 1 previous error

For more information about this error, try `rustc --explain E0080`.
12 changes: 4 additions & 8 deletions tests/ui/consts/required-consts/collect-in-dead-vtable.rs
Original file line number Diff line number Diff line change
@@ -1,14 +1,11 @@
//@revisions: noopt opt
//@[noopt] build-fail
//@ build-fail
//@[opt] compile-flags: -O
//FIXME: `opt` revision currently does not stop with an error due to
//<https://github.com/rust-lang/rust/issues/107503>.
//@[opt] build-pass
//! This fails without optimizations, so it should also fail with optimizations.
struct Fail<T>(T);
impl<T> Fail<T> {
const C: () = panic!(); //[noopt]~ERROR evaluation of `Fail::<i32>::C` failed
const C: () = panic!(); //~ERROR evaluation of `Fail::<i32>::C` failed
}

trait MyTrait {
Expand All @@ -17,8 +14,7 @@ trait MyTrait {

// This function is not actually called, but it is mentioned in a vtable in a function that is
// called. Make sure we still find this error.
// This relies on mono-item collection checking `required_consts` in functions that are referenced
// in vtables that syntactically appear in collected functions (even inside dead code).
// This ensures that we are properly considering vtables when gathering "mentioned" items.
impl<T> MyTrait for Vec<T> {
fn not_called(&self) {
if false {
Expand All @@ -31,7 +27,7 @@ impl<T> MyTrait for Vec<T> {
fn called<T>() {
if false {
let v: Vec<T> = Vec::new();
let gen_vtable: &dyn MyTrait = &v; // vtable "appears" here
let gen_vtable: &dyn MyTrait = &v; // vtable is "mentioned" here
}
}

Expand Down

0 comments on commit 347ca50

Please sign in to comment.