Skip to content

Commit e369f6b

Browse files
committed
Auto merge of #69036 - eddyb:monoshim, r=<try>
rustc: don't resolve Instances which would produce malformed shims. There are some `InstanceDef` variants (shims and drop "glue") which contain a `Ty`, and that `Ty` is used in generating the shim MIR. But if that `Ty` mentions any generic parameters, the generated shim would refer to them (but they won't match the `Substs` of the `Instance`), or worse, generating the shim would fail because not enough of the type is known. Ideally we would always produce a "skeleton" of the type, e.g. `(_, _)` for dropping any tuples with two elements, or `Vec<_>` for dropping any `Vec` value, but that's a lot of work, and they would still not match the `Substs` of the `Instance` as it exists today, so `Instance` would probably need to change. By making `Instance::resolve` return `None` in the still-generic cases, we get behavior similar to specialization, where a default can only be used if there are no more generic parameters which would allow a more specialized `impl` to match. <hr/> This was found while testing the MIR inliner with #68965, because it was trying to inline shims. cc @rust-lang/wg-mir-opt
2 parents 4d1241f + 2d03e10 commit e369f6b

File tree

2 files changed

+88
-32
lines changed

2 files changed

+88
-32
lines changed

src/librustc/ty/instance.rs

+67-20
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ use crate::ty::{self, SubstsRef, Ty, TyCtxt, TypeFoldable};
66
use rustc_hir::def::Namespace;
77
use rustc_hir::def_id::{CrateNum, DefId};
88
use rustc_macros::HashStable;
9+
use rustc_span::sym;
910
use rustc_target::spec::abi::Abi;
1011

1112
use std::fmt;
@@ -40,6 +41,11 @@ pub enum InstanceDef<'tcx> {
4041

4142
/// `<fn() as FnTrait>::call_*`
4243
/// `DefId` is `FnTrait::call_*`.
44+
///
45+
/// NB: the (`fn` pointer) type must be monomorphic for MIR shims to work.
46+
// FIXME(eddyb) support generating shims for a "shallow type",
47+
// e.g. `fn(_, _) -> _` instead of requiring a fully monomorphic
48+
// `fn(Foo, Bar) -> Baz` etc.
4349
FnPtrShim(DefId, Ty<'tcx>),
4450

4551
/// `<dyn Trait as Trait>::fn`, "direct calls" of which are implicitly
@@ -55,9 +61,19 @@ pub enum InstanceDef<'tcx> {
5561
},
5662

5763
/// `drop_in_place::<T>; None` for empty drop glue.
64+
///
65+
/// NB: the type must be monomorphic for MIR shims to work.
66+
// FIXME(eddyb) support generating shims for a "shallow type",
67+
// e.g. `Foo<_>` or `[_]` instead of requiring a fully monomorphic
68+
// `Foo<Bar>` or `[String]` etc.
5869
DropGlue(DefId, Option<Ty<'tcx>>),
5970

6071
///`<T as Clone>::clone` shim.
72+
///
73+
/// NB: the type must be monomorphic for MIR shims to work.
74+
// FIXME(eddyb) support generating shims for a "shallow type",
75+
// e.g. `Foo<_>` or `[_]` instead of requiring a fully monomorphic
76+
// `Foo<Bar>` or `[String]` etc.
6177
CloneShim(DefId, Ty<'tcx>),
6278
}
6379

@@ -282,21 +298,28 @@ impl<'tcx> Instance<'tcx> {
282298
debug!(" => intrinsic");
283299
ty::InstanceDef::Intrinsic(def_id)
284300
}
285-
_ => {
286-
if Some(def_id) == tcx.lang_items().drop_in_place_fn() {
287-
let ty = substs.type_at(0);
288-
if ty.needs_drop(tcx, ty::ParamEnv::reveal_all()) {
289-
debug!(" => nontrivial drop glue");
290-
ty::InstanceDef::DropGlue(def_id, Some(ty))
291-
} else {
292-
debug!(" => trivial drop glue");
293-
ty::InstanceDef::DropGlue(def_id, None)
301+
ty::FnDef(def_id, substs)
302+
if Some(def_id) == tcx.lang_items().drop_in_place_fn() =>
303+
{
304+
let ty = substs.type_at(0);
305+
306+
if ty.needs_drop(tcx, param_env) {
307+
// `DropGlue` requires a monomorphic aka concrete type.
308+
if ty.needs_subst() {
309+
return None;
294310
}
311+
312+
debug!(" => nontrivial drop glue");
313+
ty::InstanceDef::DropGlue(def_id, Some(ty))
295314
} else {
296-
debug!(" => free item");
297-
ty::InstanceDef::Item(def_id)
315+
debug!(" => trivial drop glue");
316+
ty::InstanceDef::DropGlue(def_id, None)
298317
}
299318
}
319+
_ => {
320+
debug!(" => free item");
321+
ty::InstanceDef::Item(def_id)
322+
}
300323
};
301324
Some(Instance { def: def, substs: substs })
302325
};
@@ -458,20 +481,44 @@ fn resolve_associated_item<'tcx>(
458481
trait_closure_kind,
459482
))
460483
}
461-
traits::VtableFnPointer(ref data) => Some(Instance {
462-
def: ty::InstanceDef::FnPtrShim(trait_item.def_id, data.fn_ty),
463-
substs: rcvr_substs,
464-
}),
484+
traits::VtableFnPointer(ref data) => {
485+
// `FnPtrShim` requires a monomorphic aka concrete type.
486+
if data.fn_ty.needs_subst() {
487+
return None;
488+
}
489+
490+
Some(Instance {
491+
def: ty::InstanceDef::FnPtrShim(trait_item.def_id, data.fn_ty),
492+
substs: rcvr_substs,
493+
})
494+
}
465495
traits::VtableObject(ref data) => {
466496
let index = traits::get_vtable_index_of_object_method(tcx, data, def_id);
467497
Some(Instance { def: ty::InstanceDef::Virtual(def_id, index), substs: rcvr_substs })
468498
}
469499
traits::VtableBuiltin(..) => {
470-
if tcx.lang_items().clone_trait().is_some() {
471-
Some(Instance {
472-
def: ty::InstanceDef::CloneShim(def_id, trait_ref.self_ty()),
473-
substs: rcvr_substs,
474-
})
500+
if Some(trait_ref.def_id) == tcx.lang_items().clone_trait() {
501+
// FIXME(eddyb) use lang items for methods instead of names.
502+
let name = tcx.item_name(def_id);
503+
if name == sym::clone {
504+
let self_ty = trait_ref.self_ty();
505+
506+
// `CloneShim` requires a monomorphic aka concrete type.
507+
if self_ty.needs_subst() {
508+
return None;
509+
}
510+
511+
Some(Instance {
512+
def: ty::InstanceDef::CloneShim(def_id, self_ty),
513+
substs: rcvr_substs,
514+
})
515+
} else {
516+
assert_eq!(name, sym::clone_from);
517+
518+
// Use the default `fn clone_from` from `trait Clone`.
519+
let substs = tcx.erase_regions(&rcvr_substs);
520+
Some(ty::Instance::new(def_id, substs))
521+
}
475522
} else {
476523
None
477524
}

src/librustc_mir/shim.rs

+21-12
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,13 @@ use rustc::mir::*;
22
use rustc::ty::layout::VariantIdx;
33
use rustc::ty::query::Providers;
44
use rustc::ty::subst::{InternalSubsts, Subst};
5-
use rustc::ty::{self, Ty, TyCtxt};
5+
use rustc::ty::{self, Ty, TyCtxt, TypeFoldable};
66
use rustc_hir as hir;
77
use rustc_hir::def_id::DefId;
88

99
use rustc_index::vec::{Idx, IndexVec};
1010

11-
use rustc_span::{sym, Span};
11+
use rustc_span::Span;
1212
use rustc_target::spec::abi::Abi;
1313

1414
use std::fmt;
@@ -39,6 +39,11 @@ fn make_shim<'tcx>(tcx: TyCtxt<'tcx>, instance: ty::InstanceDef<'tcx>) -> &'tcx
3939
None,
4040
),
4141
ty::InstanceDef::FnPtrShim(def_id, ty) => {
42+
// FIXME(eddyb) support generating shims for a "shallow type",
43+
// e.g. `Foo<_>` or `[_]` instead of requiring a fully monomorphic
44+
// `Foo<Bar>` or `[String]` etc.
45+
assert!(!ty.needs_subst());
46+
4247
let trait_ = tcx.trait_of_item(def_id).unwrap();
4348
let adjustment = match tcx.lang_items().fn_trait_kind(trait_) {
4449
Some(ty::ClosureKind::FnOnce) => Adjustment::Identity,
@@ -81,17 +86,21 @@ fn make_shim<'tcx>(tcx: TyCtxt<'tcx>, instance: ty::InstanceDef<'tcx>) -> &'tcx
8186
None,
8287
)
8388
}
84-
ty::InstanceDef::DropGlue(def_id, ty) => build_drop_shim(tcx, def_id, ty),
89+
ty::InstanceDef::DropGlue(def_id, ty) => {
90+
// FIXME(eddyb) support generating shims for a "shallow type",
91+
// e.g. `Foo<_>` or `[_]` instead of requiring a fully monomorphic
92+
// `Foo<Bar>` or `[String]` etc.
93+
assert!(!ty.needs_subst());
94+
95+
build_drop_shim(tcx, def_id, ty)
96+
}
8597
ty::InstanceDef::CloneShim(def_id, ty) => {
86-
let name = tcx.item_name(def_id);
87-
if name == sym::clone {
88-
build_clone_shim(tcx, def_id, ty)
89-
} else if name == sym::clone_from {
90-
debug!("make_shim({:?}: using default trait implementation", instance);
91-
return tcx.optimized_mir(def_id);
92-
} else {
93-
bug!("builtin clone shim {:?} not supported", instance)
94-
}
98+
// FIXME(eddyb) support generating shims for a "shallow type",
99+
// e.g. `Foo<_>` or `[_]` instead of requiring a fully monomorphic
100+
// `Foo<Bar>` or `[String]` etc.
101+
assert!(!ty.needs_subst());
102+
103+
build_clone_shim(tcx, def_id, ty)
95104
}
96105
ty::InstanceDef::Virtual(..) => {
97106
bug!("InstanceDef::Virtual ({:?}) is for direct calls only", instance)

0 commit comments

Comments
 (0)