From 51ea260ee7ee896701ef7296d7f63e7ef455b383 Mon Sep 17 00:00:00 2001 From: Eduard-Mihai Burtescu Date: Sun, 9 Feb 2020 02:39:14 +0200 Subject: [PATCH 1/2] rustc: don't resolve Instances which would produce malformed shims. --- src/librustc/ty/instance.rs | 15 ++++++++ src/librustc_mir/shim.rs | 33 ++++++++++------- src/librustc_ty/instance.rs | 70 ++++++++++++++++++++++++++----------- 3 files changed, 86 insertions(+), 32 deletions(-) diff --git a/src/librustc/ty/instance.rs b/src/librustc/ty/instance.rs index 445df76cd32be..07acb7abfdbb8 100644 --- a/src/librustc/ty/instance.rs +++ b/src/librustc/ty/instance.rs @@ -39,6 +39,11 @@ pub enum InstanceDef<'tcx> { /// `::call_*` /// `DefId` is `FnTrait::call_*`. + /// + /// NB: the (`fn` pointer) type must be monomorphic for MIR shims to work. + // FIXME(eddyb) support generating shims for a "shallow type", + // e.g. `fn(_, _) -> _` instead of requiring a fully monomorphic + // `fn(Foo, Bar) -> Baz` etc. FnPtrShim(DefId, Ty<'tcx>), /// `::fn`, "direct calls" of which are implicitly @@ -57,9 +62,19 @@ pub enum InstanceDef<'tcx> { /// The `DefId` is for `core::ptr::drop_in_place`. /// The `Option>` is either `Some(T)`, or `None` for empty drop /// glue. + /// + /// NB: the type must be monomorphic for MIR shims to work. + // FIXME(eddyb) support generating shims for a "shallow type", + // e.g. `Foo<_>` or `[_]` instead of requiring a fully monomorphic + // `Foo` or `[String]` etc. DropGlue(DefId, Option>), ///`::clone` shim. + /// + /// NB: the type must be monomorphic for MIR shims to work. + // FIXME(eddyb) support generating shims for a "shallow type", + // e.g. `Foo<_>` or `[_]` instead of requiring a fully monomorphic + // `Foo` or `[String]` etc. CloneShim(DefId, Ty<'tcx>), } diff --git a/src/librustc_mir/shim.rs b/src/librustc_mir/shim.rs index 1f7db2861a2eb..c1d969a4b5163 100644 --- a/src/librustc_mir/shim.rs +++ b/src/librustc_mir/shim.rs @@ -2,13 +2,13 @@ use rustc::mir::*; use rustc::ty::layout::VariantIdx; use rustc::ty::query::Providers; use rustc::ty::subst::{InternalSubsts, Subst}; -use rustc::ty::{self, Ty, TyCtxt}; +use rustc::ty::{self, Ty, TyCtxt, TypeFoldable}; use rustc_hir as hir; use rustc_hir::def_id::DefId; use rustc_index::vec::{Idx, IndexVec}; -use rustc_span::{sym, Span}; +use rustc_span::Span; use rustc_target::spec::abi::Abi; use std::fmt; @@ -39,6 +39,11 @@ fn make_shim<'tcx>(tcx: TyCtxt<'tcx>, instance: ty::InstanceDef<'tcx>) -> &'tcx None, ), ty::InstanceDef::FnPtrShim(def_id, ty) => { + // FIXME(eddyb) support generating shims for a "shallow type", + // e.g. `Foo<_>` or `[_]` instead of requiring a fully monomorphic + // `Foo` or `[String]` etc. + assert!(!ty.needs_subst()); + let trait_ = tcx.trait_of_item(def_id).unwrap(); let adjustment = match tcx.fn_trait_kind_from_lang_item(trait_) { Some(ty::ClosureKind::FnOnce) => Adjustment::Identity, @@ -81,17 +86,21 @@ fn make_shim<'tcx>(tcx: TyCtxt<'tcx>, instance: ty::InstanceDef<'tcx>) -> &'tcx None, ) } - ty::InstanceDef::DropGlue(def_id, ty) => build_drop_shim(tcx, def_id, ty), + ty::InstanceDef::DropGlue(def_id, ty) => { + // FIXME(eddyb) support generating shims for a "shallow type", + // e.g. `Foo<_>` or `[_]` instead of requiring a fully monomorphic + // `Foo` or `[String]` etc. + assert!(!ty.needs_subst()); + + build_drop_shim(tcx, def_id, ty) + } ty::InstanceDef::CloneShim(def_id, ty) => { - let name = tcx.item_name(def_id); - if name == sym::clone { - build_clone_shim(tcx, def_id, ty) - } else if name == sym::clone_from { - debug!("make_shim({:?}: using default trait implementation", instance); - return tcx.optimized_mir(def_id); - } else { - bug!("builtin clone shim {:?} not supported", instance) - } + // FIXME(eddyb) support generating shims for a "shallow type", + // e.g. `Foo<_>` or `[_]` instead of requiring a fully monomorphic + // `Foo` or `[String]` etc. + assert!(!ty.needs_subst()); + + build_clone_shim(tcx, def_id, ty) } ty::InstanceDef::Virtual(..) => { bug!("InstanceDef::Virtual ({:?}) is for direct calls only", instance) diff --git a/src/librustc_ty/instance.rs b/src/librustc_ty/instance.rs index 10cc2c0e3033c..a5abe7b6413cc 100644 --- a/src/librustc_ty/instance.rs +++ b/src/librustc_ty/instance.rs @@ -1,6 +1,7 @@ use rustc::ty::subst::SubstsRef; use rustc::ty::{self, Instance, TyCtxt, TypeFoldable}; use rustc_hir::def_id::DefId; +use rustc_span::sym; use rustc_target::spec::abi::Abi; use rustc_trait_selection::traits; @@ -31,21 +32,26 @@ pub fn resolve_instance<'tcx>( debug!(" => intrinsic"); ty::InstanceDef::Intrinsic(def_id) } - _ => { - if Some(def_id) == tcx.lang_items().drop_in_place_fn() { - let ty = substs.type_at(0); - if ty.needs_drop(tcx, param_env.with_reveal_all()) { - debug!(" => nontrivial drop glue"); - ty::InstanceDef::DropGlue(def_id, Some(ty)) - } else { - debug!(" => trivial drop glue"); - ty::InstanceDef::DropGlue(def_id, None) + ty::FnDef(def_id, substs) if Some(def_id) == tcx.lang_items().drop_in_place_fn() => { + let ty = substs.type_at(0); + + if ty.needs_drop(tcx, param_env) { + // `DropGlue` requires a monomorphic aka concrete type. + if ty.needs_subst() { + return None; } + + debug!(" => nontrivial drop glue"); + ty::InstanceDef::DropGlue(def_id, Some(ty)) } else { - debug!(" => free item"); - ty::InstanceDef::Item(def_id) + debug!(" => trivial drop glue"); + ty::InstanceDef::DropGlue(def_id, None) } } + _ => { + debug!(" => free item"); + ty::InstanceDef::Item(def_id) + } }; Some(Instance { def, substs }) }; @@ -113,20 +119,44 @@ fn resolve_associated_item<'tcx>( trait_closure_kind, )) } - traits::VtableFnPointer(ref data) => Some(Instance { - def: ty::InstanceDef::FnPtrShim(trait_item.def_id, data.fn_ty), - substs: rcvr_substs, - }), + traits::VtableFnPointer(ref data) => { + // `FnPtrShim` requires a monomorphic aka concrete type. + if data.fn_ty.needs_subst() { + return None; + } + + Some(Instance { + def: ty::InstanceDef::FnPtrShim(trait_item.def_id, data.fn_ty), + substs: rcvr_substs, + }) + } traits::VtableObject(ref data) => { let index = traits::get_vtable_index_of_object_method(tcx, data, def_id); Some(Instance { def: ty::InstanceDef::Virtual(def_id, index), substs: rcvr_substs }) } traits::VtableBuiltin(..) => { - if tcx.lang_items().clone_trait().is_some() { - Some(Instance { - def: ty::InstanceDef::CloneShim(def_id, trait_ref.self_ty()), - substs: rcvr_substs, - }) + if Some(trait_ref.def_id) == tcx.lang_items().clone_trait() { + // FIXME(eddyb) use lang items for methods instead of names. + let name = tcx.item_name(def_id); + if name == sym::clone { + let self_ty = trait_ref.self_ty(); + + // `CloneShim` requires a monomorphic aka concrete type. + if self_ty.needs_subst() { + return None; + } + + Some(Instance { + def: ty::InstanceDef::CloneShim(def_id, self_ty), + substs: rcvr_substs, + }) + } else { + assert_eq!(name, sym::clone_from); + + // Use the default `fn clone_from` from `trait Clone`. + let substs = tcx.erase_regions(&rcvr_substs); + Some(ty::Instance::new(def_id, substs)) + } } else { None } From bc8ff3fe8e8cebc9e4855aaf44c19c887f8f4be4 Mon Sep 17 00:00:00 2001 From: Ana-Maria Mihalache Date: Wed, 11 Mar 2020 15:20:31 +0000 Subject: [PATCH 2/2] rustc: tweak comments on InstanceDef. --- src/librustc/ty/instance.rs | 21 +++++++++------------ 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/src/librustc/ty/instance.rs b/src/librustc/ty/instance.rs index 07acb7abfdbb8..c9ad8707a7454 100644 --- a/src/librustc/ty/instance.rs +++ b/src/librustc/ty/instance.rs @@ -40,10 +40,9 @@ pub enum InstanceDef<'tcx> { /// `::call_*` /// `DefId` is `FnTrait::call_*`. /// - /// NB: the (`fn` pointer) type must be monomorphic for MIR shims to work. - // FIXME(eddyb) support generating shims for a "shallow type", - // e.g. `fn(_, _) -> _` instead of requiring a fully monomorphic - // `fn(Foo, Bar) -> Baz` etc. + /// NB: the (`fn` pointer) type must currently be monomorphic to avoid double substitution + /// problems with the MIR shim bodies. `Instance::resolve` enforces this. + // FIXME(#69925) support polymorphic MIR shim bodies properly instead. FnPtrShim(DefId, Ty<'tcx>), /// `::fn`, "direct calls" of which are implicitly @@ -63,18 +62,16 @@ pub enum InstanceDef<'tcx> { /// The `Option>` is either `Some(T)`, or `None` for empty drop /// glue. /// - /// NB: the type must be monomorphic for MIR shims to work. - // FIXME(eddyb) support generating shims for a "shallow type", - // e.g. `Foo<_>` or `[_]` instead of requiring a fully monomorphic - // `Foo` or `[String]` etc. + /// NB: the type must currently be monomorphic to avoid double substitution + /// problems with the MIR shim bodies. `Instance::resolve` enforces this. + // FIXME(#69925) support polymorphic MIR shim bodies properly instead. DropGlue(DefId, Option>), ///`::clone` shim. /// - /// NB: the type must be monomorphic for MIR shims to work. - // FIXME(eddyb) support generating shims for a "shallow type", - // e.g. `Foo<_>` or `[_]` instead of requiring a fully monomorphic - // `Foo` or `[String]` etc. + /// NB: the type must currently be monomorphic to avoid double substitution + /// problems with the MIR shim bodies. `Instance::resolve` enforces this. + // FIXME(#69925) support polymorphic MIR shim bodies properly instead. CloneShim(DefId, Ty<'tcx>), }