Skip to content

Commit 60f7aa1

Browse files
authored
Rollup merge of rust-lang#69036 - eddyb:monoshim, r=nikomatsakis
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 rust-lang#68965, because it was trying to inline shims. cc @rust-lang/wg-mir-opt
2 parents af55daf + bc8ff3f commit 60f7aa1

File tree

3 files changed

+83
-32
lines changed

3 files changed

+83
-32
lines changed

src/librustc/ty/instance.rs

+12
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,10 @@ pub enum InstanceDef<'tcx> {
3939

4040
/// `<fn() as FnTrait>::call_*`
4141
/// `DefId` is `FnTrait::call_*`.
42+
///
43+
/// NB: the (`fn` pointer) type must currently be monomorphic to avoid double substitution
44+
/// problems with the MIR shim bodies. `Instance::resolve` enforces this.
45+
// FIXME(#69925) support polymorphic MIR shim bodies properly instead.
4246
FnPtrShim(DefId, Ty<'tcx>),
4347

4448
/// `<dyn Trait as Trait>::fn`, "direct calls" of which are implicitly
@@ -57,9 +61,17 @@ pub enum InstanceDef<'tcx> {
5761
/// The `DefId` is for `core::ptr::drop_in_place`.
5862
/// The `Option<Ty<'tcx>>` is either `Some(T)`, or `None` for empty drop
5963
/// glue.
64+
///
65+
/// NB: the type must currently be monomorphic to avoid double substitution
66+
/// problems with the MIR shim bodies. `Instance::resolve` enforces this.
67+
// FIXME(#69925) support polymorphic MIR shim bodies properly instead.
6068
DropGlue(DefId, Option<Ty<'tcx>>),
6169

6270
///`<T as Clone>::clone` shim.
71+
///
72+
/// NB: the type must currently be monomorphic to avoid double substitution
73+
/// problems with the MIR shim bodies. `Instance::resolve` enforces this.
74+
// FIXME(#69925) support polymorphic MIR shim bodies properly instead.
6375
CloneShim(DefId, Ty<'tcx>),
6476
}
6577

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.fn_trait_kind_from_lang_item(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)

src/librustc_ty/instance.rs

+50-20
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
use rustc::ty::subst::SubstsRef;
22
use rustc::ty::{self, Instance, TyCtxt, TypeFoldable};
33
use rustc_hir::def_id::DefId;
4+
use rustc_span::sym;
45
use rustc_target::spec::abi::Abi;
56
use rustc_trait_selection::traits;
67

@@ -31,21 +32,26 @@ pub fn resolve_instance<'tcx>(
3132
debug!(" => intrinsic");
3233
ty::InstanceDef::Intrinsic(def_id)
3334
}
34-
_ => {
35-
if Some(def_id) == tcx.lang_items().drop_in_place_fn() {
36-
let ty = substs.type_at(0);
37-
if ty.needs_drop(tcx, param_env.with_reveal_all()) {
38-
debug!(" => nontrivial drop glue");
39-
ty::InstanceDef::DropGlue(def_id, Some(ty))
40-
} else {
41-
debug!(" => trivial drop glue");
42-
ty::InstanceDef::DropGlue(def_id, None)
35+
ty::FnDef(def_id, substs) if Some(def_id) == tcx.lang_items().drop_in_place_fn() => {
36+
let ty = substs.type_at(0);
37+
38+
if ty.needs_drop(tcx, param_env) {
39+
// `DropGlue` requires a monomorphic aka concrete type.
40+
if ty.needs_subst() {
41+
return None;
4342
}
43+
44+
debug!(" => nontrivial drop glue");
45+
ty::InstanceDef::DropGlue(def_id, Some(ty))
4446
} else {
45-
debug!(" => free item");
46-
ty::InstanceDef::Item(def_id)
47+
debug!(" => trivial drop glue");
48+
ty::InstanceDef::DropGlue(def_id, None)
4749
}
4850
}
51+
_ => {
52+
debug!(" => free item");
53+
ty::InstanceDef::Item(def_id)
54+
}
4955
};
5056
Some(Instance { def, substs })
5157
};
@@ -113,20 +119,44 @@ fn resolve_associated_item<'tcx>(
113119
trait_closure_kind,
114120
))
115121
}
116-
traits::VtableFnPointer(ref data) => Some(Instance {
117-
def: ty::InstanceDef::FnPtrShim(trait_item.def_id, data.fn_ty),
118-
substs: rcvr_substs,
119-
}),
122+
traits::VtableFnPointer(ref data) => {
123+
// `FnPtrShim` requires a monomorphic aka concrete type.
124+
if data.fn_ty.needs_subst() {
125+
return None;
126+
}
127+
128+
Some(Instance {
129+
def: ty::InstanceDef::FnPtrShim(trait_item.def_id, data.fn_ty),
130+
substs: rcvr_substs,
131+
})
132+
}
120133
traits::VtableObject(ref data) => {
121134
let index = traits::get_vtable_index_of_object_method(tcx, data, def_id);
122135
Some(Instance { def: ty::InstanceDef::Virtual(def_id, index), substs: rcvr_substs })
123136
}
124137
traits::VtableBuiltin(..) => {
125-
if tcx.lang_items().clone_trait().is_some() {
126-
Some(Instance {
127-
def: ty::InstanceDef::CloneShim(def_id, trait_ref.self_ty()),
128-
substs: rcvr_substs,
129-
})
138+
if Some(trait_ref.def_id) == tcx.lang_items().clone_trait() {
139+
// FIXME(eddyb) use lang items for methods instead of names.
140+
let name = tcx.item_name(def_id);
141+
if name == sym::clone {
142+
let self_ty = trait_ref.self_ty();
143+
144+
// `CloneShim` requires a monomorphic aka concrete type.
145+
if self_ty.needs_subst() {
146+
return None;
147+
}
148+
149+
Some(Instance {
150+
def: ty::InstanceDef::CloneShim(def_id, self_ty),
151+
substs: rcvr_substs,
152+
})
153+
} else {
154+
assert_eq!(name, sym::clone_from);
155+
156+
// Use the default `fn clone_from` from `trait Clone`.
157+
let substs = tcx.erase_regions(&rcvr_substs);
158+
Some(ty::Instance::new(def_id, substs))
159+
}
130160
} else {
131161
None
132162
}

0 commit comments

Comments
 (0)