Skip to content

Commit

Permalink
Auto merge of #54183 - qnighy:by-value-object-safety, r=oli-obk
Browse files Browse the repository at this point in the history
Implement by-value object safety

This PR implements **by-value object safety**, which is part of unsized rvalues #48055. That means, with `#![feature(unsized_locals)]`, you can call a method `fn foo(self, ...)` on trait objects. One aim of this is to enable `Box<FnOnce>`  in the near future.

The difficulty here is this: when constructing a vtable for a trait `Foo`, we can't just put the function `<T as Foo>::foo` into the table. If `T` is no larger than `usize`, `self` is usually passed directly. However, as the caller of the vtable doesn't know the concrete `Self` type, we want a variant of `<T as Foo>::foo` where `self` is always passed by reference.

Therefore, when the compiler encounters such a method to be generated as a vtable entry, it produces a newly introduced instance called `InstanceDef::VtableShim(def_id)` (that wraps the original instance). the shim just derefs the receiver and calls the original method. We give different symbol names for the shims by appending `::{{vtable-shim}}` to the symbol path (and also adding vtable-shimness as an ingredient to the symbol hash).

r? @eddyb
  • Loading branch information
bors committed Oct 27, 2018
2 parents b3b8760 + 2f7ea4a commit cae6efc
Show file tree
Hide file tree
Showing 46 changed files with 870 additions and 171 deletions.
6 changes: 2 additions & 4 deletions src/doc/unstable-book/src/language-features/unsized-locals.md
Original file line number Diff line number Diff line change
Expand Up @@ -101,9 +101,9 @@ fn main() {
}
```

And `Foo` will also be object-safe. However, this object-safety is not yet implemented.
And `Foo` will also be object-safe.

```rust,ignore
```rust
#![feature(unsized_locals)]

trait Foo {
Expand All @@ -119,8 +119,6 @@ fn main () {
}
```

Unfortunately, this is not implemented yet.

One of the objectives of this feature is to allow `Box<dyn FnOnce>`, instead of `Box<dyn FnBox>` in the future. See [#28796] for details.

[#28796]: https://github.com/rust-lang/rust/issues/28796
Expand Down
3 changes: 3 additions & 0 deletions src/librustc/ich/impls_ty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1007,6 +1007,9 @@ impl<'a, 'gcx> HashStable<StableHashingContext<'a>> for ty::InstanceDef<'gcx> {
ty::InstanceDef::Item(def_id) => {
def_id.hash_stable(hcx, hasher);
}
ty::InstanceDef::VtableShim(def_id) => {
def_id.hash_stable(hcx, hasher);
}
ty::InstanceDef::Intrinsic(def_id) => {
def_id.hash_stable(hcx, hasher);
}
Expand Down
97 changes: 96 additions & 1 deletion src/librustc/ty/instance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,15 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.

use hir::Unsafety;
use hir::def_id::DefId;
use ty::{self, Ty, TypeFoldable, Substs, TyCtxt};
use ty::{self, Ty, PolyFnSig, TypeFoldable, Substs, TyCtxt};
use traits;
use rustc_target::spec::abi::Abi;
use util::ppaux;

use std::fmt;
use std::iter;

#[derive(Copy, Clone, PartialEq, Eq, Hash, Debug, RustcEncodable, RustcDecodable)]
pub struct Instance<'tcx> {
Expand All @@ -27,6 +29,9 @@ pub enum InstanceDef<'tcx> {
Item(DefId),
Intrinsic(DefId),

/// `<T as Trait>::method` where `method` receives unsizeable `self: Self`.
VtableShim(DefId),

/// \<fn() as FnTrait>::call_*
/// def-id is FnTrait::call_*
FnPtrShim(DefId, Ty<'tcx>),
Expand Down Expand Up @@ -56,13 +61,73 @@ impl<'a, 'tcx> Instance<'tcx> {
&ty,
)
}

fn fn_sig_noadjust(&self, tcx: TyCtxt<'a, 'tcx, 'tcx>) -> PolyFnSig<'tcx> {
let ty = self.ty(tcx);
match ty.sty {
ty::FnDef(..) |
// Shims currently have type FnPtr. Not sure this should remain.
ty::FnPtr(_) => ty.fn_sig(tcx),
ty::Closure(def_id, substs) => {
let sig = substs.closure_sig(def_id, tcx);

let env_ty = tcx.closure_env_ty(def_id, substs).unwrap();
sig.map_bound(|sig| tcx.mk_fn_sig(
iter::once(*env_ty.skip_binder()).chain(sig.inputs().iter().cloned()),
sig.output(),
sig.variadic,
sig.unsafety,
sig.abi
))
}
ty::Generator(def_id, substs, _) => {
let sig = substs.poly_sig(def_id, tcx);

let env_region = ty::ReLateBound(ty::INNERMOST, ty::BrEnv);
let env_ty = tcx.mk_mut_ref(tcx.mk_region(env_region), ty);

sig.map_bound(|sig| {
let state_did = tcx.lang_items().gen_state().unwrap();
let state_adt_ref = tcx.adt_def(state_did);
let state_substs = tcx.intern_substs(&[
sig.yield_ty.into(),
sig.return_ty.into(),
]);
let ret_ty = tcx.mk_adt(state_adt_ref, state_substs);

tcx.mk_fn_sig(iter::once(env_ty),
ret_ty,
false,
Unsafety::Normal,
Abi::Rust
)
})
}
_ => bug!("unexpected type {:?} in Instance::fn_sig_noadjust", ty)
}
}

pub fn fn_sig(&self, tcx: TyCtxt<'a, 'tcx, 'tcx>) -> ty::PolyFnSig<'tcx> {
let mut fn_sig = self.fn_sig_noadjust(tcx);
if let InstanceDef::VtableShim(..) = self.def {
// Modify fn(self, ...) to fn(self: *mut Self, ...)
fn_sig = fn_sig.map_bound(|mut fn_sig| {
let mut inputs_and_output = fn_sig.inputs_and_output.to_vec();
inputs_and_output[0] = tcx.mk_mut_ptr(inputs_and_output[0]);
fn_sig.inputs_and_output = tcx.intern_type_list(&inputs_and_output);
fn_sig
});
}
fn_sig
}
}

impl<'tcx> InstanceDef<'tcx> {
#[inline]
pub fn def_id(&self) -> DefId {
match *self {
InstanceDef::Item(def_id) |
InstanceDef::VtableShim(def_id) |
InstanceDef::FnPtrShim(def_id, _) |
InstanceDef::Virtual(def_id, _) |
InstanceDef::Intrinsic(def_id, ) |
Expand Down Expand Up @@ -120,6 +185,9 @@ impl<'tcx> fmt::Display for Instance<'tcx> {
ppaux::parameterized(f, self.substs, self.def_id(), &[])?;
match self.def {
InstanceDef::Item(_) => Ok(()),
InstanceDef::VtableShim(_) => {
write!(f, " - shim(vtable)")
}
InstanceDef::Intrinsic(_) => {
write!(f, " - intrinsic")
}
Expand Down Expand Up @@ -230,6 +298,25 @@ impl<'a, 'b, 'tcx> Instance<'tcx> {
result
}

pub fn resolve_for_vtable(tcx: TyCtxt<'a, 'tcx, 'tcx>,
param_env: ty::ParamEnv<'tcx>,
def_id: DefId,
substs: &'tcx Substs<'tcx>) -> Option<Instance<'tcx>> {
debug!("resolve(def_id={:?}, substs={:?})", def_id, substs);
let fn_sig = tcx.fn_sig(def_id);
let is_vtable_shim =
fn_sig.inputs().skip_binder().len() > 0 && fn_sig.input(0).skip_binder().is_self();
if is_vtable_shim {
debug!(" => associated item with unsizeable self: Self");
Some(Instance {
def: InstanceDef::VtableShim(def_id),
substs,
})
} else {
Instance::resolve(tcx, param_env, def_id, substs)
}
}

pub fn resolve_closure(
tcx: TyCtxt<'a, 'tcx, 'tcx>,
def_id: DefId,
Expand All @@ -244,6 +331,14 @@ impl<'a, 'b, 'tcx> Instance<'tcx> {
_ => Instance::new(def_id, substs.substs)
}
}

pub fn is_vtable_shim(&self) -> bool {
if let InstanceDef::VtableShim(..) = self.def {
true
} else {
false
}
}
}

fn resolve_associated_item<'a, 'tcx>(
Expand Down
1 change: 1 addition & 0 deletions src/librustc/ty/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2761,6 +2761,7 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> {
ty::InstanceDef::Item(did) => {
self.optimized_mir(did)
}
ty::InstanceDef::VtableShim(..) |
ty::InstanceDef::Intrinsic(..) |
ty::InstanceDef::FnPtrShim(..) |
ty::InstanceDef::Virtual(..) |
Expand Down
5 changes: 4 additions & 1 deletion src/librustc/ty/structural_impls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -467,6 +467,8 @@ impl<'a, 'tcx> Lift<'tcx> for ty::InstanceDef<'a> {
match *self {
ty::InstanceDef::Item(def_id) =>
Some(ty::InstanceDef::Item(def_id)),
ty::InstanceDef::VtableShim(def_id) =>
Some(ty::InstanceDef::VtableShim(def_id)),
ty::InstanceDef::Intrinsic(def_id) =>
Some(ty::InstanceDef::Intrinsic(def_id)),
ty::InstanceDef::FnPtrShim(def_id, ref ty) =>
Expand Down Expand Up @@ -647,6 +649,7 @@ impl<'tcx> TypeFoldable<'tcx> for ty::instance::Instance<'tcx> {
substs: self.substs.fold_with(folder),
def: match self.def {
Item(did) => Item(did.fold_with(folder)),
VtableShim(did) => VtableShim(did.fold_with(folder)),
Intrinsic(did) => Intrinsic(did.fold_with(folder)),
FnPtrShim(did, ty) => FnPtrShim(
did.fold_with(folder),
Expand Down Expand Up @@ -675,7 +678,7 @@ impl<'tcx> TypeFoldable<'tcx> for ty::instance::Instance<'tcx> {
use ty::InstanceDef::*;
self.substs.visit_with(visitor) ||
match self.def {
Item(did) | Intrinsic(did) | Virtual(did, _) => {
Item(did) | VtableShim(did) | Intrinsic(did) | Virtual(did, _) => {
did.visit_with(visitor)
},
FnPtrShim(did, ty) | CloneShim(did, ty) => {
Expand Down
21 changes: 10 additions & 11 deletions src/librustc_codegen_llvm/abi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
use llvm::{self, AttributePlace};
use base;
use builder::{Builder, MemFlags};
use common::{ty_fn_sig, C_usize};
use common::C_usize;
use context::CodegenCx;
use mir::place::PlaceRef;
use mir::operand::OperandValue;
Expand Down Expand Up @@ -283,8 +283,7 @@ pub trait FnTypeExt<'tcx> {

impl<'tcx> FnTypeExt<'tcx> for FnType<'tcx, Ty<'tcx>> {
fn of_instance(cx: &CodegenCx<'ll, 'tcx>, instance: &ty::Instance<'tcx>) -> Self {
let fn_ty = instance.ty(cx.tcx);
let sig = ty_fn_sig(cx, fn_ty);
let sig = instance.fn_sig(cx.tcx);
let sig = cx.tcx.normalize_erasing_late_bound_regions(ty::ParamEnv::reveal_all(), &sig);
FnType::new(cx, sig, &[])
}
Expand All @@ -305,17 +304,17 @@ impl<'tcx> FnTypeExt<'tcx> for FnType<'tcx, Ty<'tcx>> {
// Don't pass the vtable, it's not an argument of the virtual fn.
// Instead, pass just the (thin pointer) first field of `*dyn Trait`.
if arg_idx == Some(0) {
if layout.is_unsized() {
unimplemented!("by-value trait object is not \
yet implemented in #![feature(unsized_locals)]");
}
// FIXME(eddyb) `layout.field(cx, 0)` is not enough because e.g.
// `Box<dyn Trait>` has a few newtype wrappers around the raw
// pointer, so we'd have to "dig down" to find `*dyn Trait`.
let pointee = layout.ty.builtin_deref(true)
.unwrap_or_else(|| {
bug!("FnType::new_vtable: non-pointer self {:?}", layout)
}).ty;
let pointee = if layout.is_unsized() {
layout.ty
} else {
layout.ty.builtin_deref(true)
.unwrap_or_else(|| {
bug!("FnType::new_vtable: non-pointer self {:?}", layout)
}).ty
};
let fat_ptr_ty = cx.tcx.mk_mut_ptr(pointee);
layout = cx.layout_of(fat_ptr_ty).field(cx, 0);
}
Expand Down
5 changes: 2 additions & 3 deletions src/librustc_codegen_llvm/base.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ use callee;
use common::{C_bool, C_bytes_in_context, C_i32, C_usize};
use rustc_mir::monomorphize::collector::{self, MonoItemCollectionMode};
use rustc_mir::monomorphize::item::DefPathBasedNames;
use common::{self, C_struct_in_context, C_array, val_ty};
use common::{C_struct_in_context, C_array, val_ty};
use consts;
use context::CodegenCx;
use debuginfo;
Expand Down Expand Up @@ -491,8 +491,7 @@ pub fn codegen_instance<'a, 'tcx>(cx: &CodegenCx<'a, 'tcx>, instance: Instance<'
// release builds.
info!("codegen_instance({})", instance);

let fn_ty = instance.ty(cx.tcx);
let sig = common::ty_fn_sig(cx, fn_ty);
let sig = instance.fn_sig(cx.tcx);
let sig = cx.tcx.normalize_erasing_late_bound_regions(ty::ParamEnv::reveal_all(), &sig);

let lldecl = cx.instances.borrow().get(&instance).cloned().unwrap_or_else(||
Expand Down
24 changes: 20 additions & 4 deletions src/librustc_codegen_llvm/callee.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,16 +47,16 @@ pub fn get_fn(
assert!(!instance.substs.has_escaping_regions());
assert!(!instance.substs.has_param_types());

let fn_ty = instance.ty(cx.tcx);
let sig = instance.fn_sig(cx.tcx);
if let Some(&llfn) = cx.instances.borrow().get(&instance) {
return llfn;
}

let sym = tcx.symbol_name(instance).as_str();
debug!("get_fn({:?}: {:?}) => {}", instance, fn_ty, sym);
debug!("get_fn({:?}: {:?}) => {}", instance, sig, sym);

// Create a fn pointer with the substituted signature.
let fn_ptr_ty = tcx.mk_fn_ptr(common::ty_fn_sig(cx, fn_ty));
let fn_ptr_ty = tcx.mk_fn_ptr(sig);
let llptrty = cx.layout_of(fn_ptr_ty).llvm_type(cx);

let llfn = if let Some(llfn) = declare::get_declared_value(cx, &sym) {
Expand Down Expand Up @@ -91,7 +91,7 @@ pub fn get_fn(
llfn
}
} else {
let llfn = declare::declare_fn(cx, &sym, fn_ty);
let llfn = declare::declare_fn(cx, &sym, sig);
assert_eq!(common::val_ty(llfn), llptrty);
debug!("get_fn: not casting pointer!");

Expand Down Expand Up @@ -220,3 +220,19 @@ pub fn resolve_and_get_fn(
).unwrap()
)
}

pub fn resolve_and_get_fn_for_vtable(
cx: &CodegenCx<'ll, 'tcx>,
def_id: DefId,
substs: &'tcx Substs<'tcx>,
) -> &'ll Value {
get_fn(
cx,
ty::Instance::resolve_for_vtable(
cx.tcx,
ty::ParamEnv::reveal_all(),
def_id,
substs
).unwrap()
)
}
51 changes: 0 additions & 51 deletions src/librustc_codegen_llvm/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,7 @@ use rustc::ty::layout::{HasDataLayout, LayoutOf};
use rustc::hir;

use libc::{c_uint, c_char};
use std::iter;

use rustc_target::spec::abi::Abi;
use syntax::symbol::LocalInternedString;
use syntax_pos::{Span, DUMMY_SP};

Expand Down Expand Up @@ -404,52 +402,3 @@ pub fn shift_mask_val(
_ => bug!("shift_mask_val: expected Integer or Vector, found {:?}", kind),
}
}

pub fn ty_fn_sig<'a, 'tcx>(cx: &CodegenCx<'a, 'tcx>,
ty: Ty<'tcx>)
-> ty::PolyFnSig<'tcx>
{
match ty.sty {
ty::FnDef(..) |
// Shims currently have type FnPtr. Not sure this should remain.
ty::FnPtr(_) => ty.fn_sig(cx.tcx),
ty::Closure(def_id, substs) => {
let tcx = cx.tcx;
let sig = substs.closure_sig(def_id, tcx);

let env_ty = tcx.closure_env_ty(def_id, substs).unwrap();
sig.map_bound(|sig| tcx.mk_fn_sig(
iter::once(*env_ty.skip_binder()).chain(sig.inputs().iter().cloned()),
sig.output(),
sig.variadic,
sig.unsafety,
sig.abi
))
}
ty::Generator(def_id, substs, _) => {
let tcx = cx.tcx;
let sig = substs.poly_sig(def_id, cx.tcx);

let env_region = ty::ReLateBound(ty::INNERMOST, ty::BrEnv);
let env_ty = tcx.mk_mut_ref(tcx.mk_region(env_region), ty);

sig.map_bound(|sig| {
let state_did = tcx.lang_items().gen_state().unwrap();
let state_adt_ref = tcx.adt_def(state_did);
let state_substs = tcx.intern_substs(&[
sig.yield_ty.into(),
sig.return_ty.into(),
]);
let ret_ty = tcx.mk_adt(state_adt_ref, state_substs);

tcx.mk_fn_sig(iter::once(env_ty),
ret_ty,
false,
hir::Unsafety::Normal,
Abi::Rust
)
})
}
_ => bug!("unexpected type {:?} to ty_fn_sig", ty)
}
}
Loading

0 comments on commit cae6efc

Please sign in to comment.