Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Trait upcasting #60900

Closed
wants to merge 14 commits into from
26 changes: 26 additions & 0 deletions src/doc/unstable-book/src/language-features/trait-upcasting.md
@@ -0,0 +1,26 @@
# `trait_upcasting`

The tracking issue for this feature is: [#31436]

[#65991]: https://github.com/rust-lang/rust/issues/65991

------------------------

The `trait_upcasting` feature adds support for trait upcasting. This allows a
trait object of type `dyn Foo` to be cast to a trait object of type `dyn Bar`
so long as `Foo: Bar`.

```rust,edition2018
#![feature(trait_upcasting)]

trait Foo {}

trait Bar: Foo {}

impl Foo for i32 {}

impl<T: Foo + ?Sized> Bar for T {}

let foo: &dyn Foo = &123;
let bar: &dyn Bar = foo;
```
160 changes: 124 additions & 36 deletions src/librustc_codegen_ssa/base.rs
Expand Up @@ -20,7 +20,7 @@ use crate::back::write::{
use crate::common::{IntPredicate, RealPredicate, TypeKind};
use crate::meth;
use crate::mir;
use crate::mir::operand::OperandValue;
use crate::mir::operand::{OperandRef, OperandValue};
use crate::mir::place::PlaceRef;
use crate::traits::*;
use crate::{CachedModuleCodegen, CrateInfo, MemFlags, ModuleCodegen, ModuleKind};
Expand All @@ -38,8 +38,8 @@ use rustc_middle::middle::cstore::EncodedMetadata;
use rustc_middle::middle::cstore::{self, LinkagePreference};
use rustc_middle::middle::lang_items;
use rustc_middle::mir::mono::{CodegenUnit, CodegenUnitNameBuilder, MonoItem};
use rustc_middle::ty::layout::{self, HasTyCtxt, TyAndLayout};
use rustc_middle::ty::layout::{FAT_PTR_ADDR, FAT_PTR_EXTRA};
use rustc_middle::traits::Vtable;
use rustc_middle::ty::layout::{HasTyCtxt, TyAndLayout};
use rustc_middle::ty::query::Providers;
use rustc_middle::ty::{self, Instance, Ty, TyCtxt};
use rustc_session::cgu_reuse_tracker::CguReuse;
Expand Down Expand Up @@ -144,29 +144,63 @@ pub fn compare_simd_types<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(
///
/// The `old_info` argument is a bit odd. It is intended for use in an upcast,
/// where the new vtable for an object will be derived from the old one.
pub fn unsized_info<'tcx, Cx: CodegenMethods<'tcx>>(
cx: &Cx,
pub fn unsized_info<'tcx, 'a, Bx: BuilderMethods<'a, 'tcx>>(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some ancient context for this being where it is, and only taking Cx: it was shared between the old codegen logic (based off of AST/HIR, pre-MIR), MIR-based codegen, but also some constant stuff that is now all in miri.

We should've moved this into a better place a long time ago.
Before this PR, it should probably be in miri, since it can only ever return a constant, except for the case when old_info is used - which is the case in which this PR changes anything anyway.

bx: &mut Bx,
source: Ty<'tcx>,
target: Ty<'tcx>,
old_info: Option<Cx::Value>,
) -> Cx::Value {
old_info: Option<Bx::Value>,
) -> Bx::Value {
let tcx = bx.tcx();
let (source, target) =
cx.tcx().struct_lockstep_tails_erasing_lifetimes(source, target, cx.param_env());
tcx.struct_lockstep_tails_erasing_lifetimes(source, target, bx.param_env());
match (&source.kind, &target.kind) {
(&ty::Array(_, len), &ty::Slice(_)) => {
cx.const_usize(len.eval_usize(cx.tcx(), ty::ParamEnv::reveal_all()))
bx.cx().const_usize(len.eval_usize(tcx, ty::ParamEnv::reveal_all()))
}
(&ty::Dynamic(..), &ty::Dynamic(..)) => {
// For now, upcasts are limited to changes in marker
// traits, and hence never actually require an actual
// change to the vtable.
old_info.expect("unsized_info: missing old info for trait upcast")
(&ty::Dynamic(..), &ty::Dynamic(ref target_data, ..)) => {
nikomatsakis marked this conversation as resolved.
Show resolved Hide resolved
// Trait upcast

let source_ptr = old_info.expect("unsized_info: missing old info for trait upcast");
// Only bother offsetting into the parent vtable if we are upcasting to a
// non-auto trait.
let target_ptr = if let Some(target_trait_ref) = target_data.principal() {
// Find the offset of the supertrait's vtable within the subtrait (parent) vtable.
let trait_ref = target_trait_ref.with_self_ty(tcx, source);
let vtable = tcx
.codegen_fulfill_obligation((ty::ParamEnv::reveal_all(), trait_ref))
.unwrap_or_else(|| bug!("unsized_info: vtable not found"));
let offset = match vtable {
Vtable::VtableObject(ref data) => data.vtable_base,
// HACK(alexreg): slightly dubious solution to ICE in
// `manual-self-impl-for-unsafe-obj` test.
Vtable::VtableImpl(_) => 0,
_ => bug!("unsized_info: unexpected vtable kind {:?}", vtable),
};

// Ensure the pointer to the parent vtable has the right LLVM type.
let vtable_layout = bx.cx().layout_of(tcx.mk_mut_ptr(source));
let source_ptr = bx.pointercast(
source_ptr,
bx.cx().scalar_pair_element_backend_type(vtable_layout, 1, true),
);

// Perform the offset within the parent vtable.
bx.struct_gep(source_ptr, offset as u64)
} else {
source_ptr
};

let vtable_layout = bx.cx().layout_of(tcx.mk_mut_ptr(target));
bx.pointercast(
target_ptr,
bx.cx().scalar_pair_element_backend_type(vtable_layout, 1, true),
)
}
(_, &ty::Dynamic(ref data, ..)) => {
let vtable_ptr = cx.layout_of(cx.tcx().mk_mut_ptr(target)).field(cx, FAT_PTR_EXTRA);
cx.const_ptrcast(
meth::get_vtable(cx, source, data.principal()),
cx.backend_type(vtable_ptr),
let vtable_layout = bx.cx().layout_of(tcx.mk_mut_ptr(target));
bx.pointercast(
meth::get_vtable(bx.cx(), source, data.principal()),
bx.cx().scalar_pair_element_backend_type(vtable_layout, 1, true),
)
}
_ => bug!("unsized_info: invalid unsizing {:?} -> {:?}", source, target),
Expand All @@ -186,7 +220,7 @@ pub fn unsize_thin_ptr<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(
| (&ty::RawPtr(ty::TypeAndMut { ty: a, .. }), &ty::RawPtr(ty::TypeAndMut { ty: b, .. })) => {
assert!(bx.cx().type_is_sized(a));
let ptr_ty = bx.cx().type_ptr_to(bx.cx().backend_type(bx.cx().layout_of(b)));
(bx.pointercast(src, ptr_ty), unsized_info(bx.cx(), a, b, None))
(bx.pointercast(src, ptr_ty), unsized_info(bx, a, b, None))
}
(&ty::Adt(def_a, _), &ty::Adt(def_b, _)) => {
assert_eq!(def_a, def_b);
Expand Down Expand Up @@ -221,6 +255,73 @@ pub fn unsize_thin_ptr<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(
}
}

/// Coerces `op` to `dst.ty`. `op` may be a fat pointer.
pub fn coerce_ptr_unsized<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Something I should've said a while back: base and common were always organizational disasters, we should be moving abstractions closer to where they're used (e.g. mir::rvalue in this case, and it would probably be a method, there).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair enough, but probably something to tidy up in a follow-up PR.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would help this PR make sense if it happened before, IMO.

bx: &mut Bx,
op: OperandRef<'tcx, Bx::Value>,
dst: TyAndLayout<'tcx>,
) -> OperandValue<Bx::Value> {
assert!(bx.cx().is_backend_scalar_pair(dst));
let src = op.layout;
let (base, info) = match op.val {
OperandValue::Pair(base, info) => {
// Fat-ptr to fat-ptr unsize (e.g., `&'a fmt::Debug + Send` => `&'a fmt::Debug`).
// We always need to `pointercast` the base to ensure the types match up.
// In some cases, e.g., `&'a Bar` => `&'a Foo` where `Bar: Foo`, we also need to
// modify the info (i.e., the pointer to the vtable), by offsetting into the
// existing vtable. See the `get_vtable` fn in `meth.rs` for more details of vtable
// layout.

let base = bx.pointercast(base, bx.cx().scalar_pair_element_backend_type(dst, 0, true));
let info = match (&src.ty.kind, &dst.ty.kind) {
(&ty::Ref(_, a, _), &ty::Ref(_, b, _))
| (&ty::Ref(_, a, _), &ty::RawPtr(ty::TypeAndMut { ty: b, .. }))
| (
&ty::RawPtr(ty::TypeAndMut { ty: a, .. }),
&ty::RawPtr(ty::TypeAndMut { ty: b, .. }),
) => unsized_info(bx, a, b, Some(info)),
(&ty::Adt(def_a, _), &ty::Adt(def_b, _)) => {
assert_eq!(def_a, def_b);

let mut result = None;
for i in 0..src.fields.count() {
let src_f = src.field(bx.cx(), i);
assert_eq!(src.fields.offset(i).bytes(), 0);
assert_eq!(dst.fields.offset(i).bytes(), 0);
if src_f.is_zst() {
continue;
}
assert_eq!(src.size, src_f.size);

let dst_f = dst.field(bx.cx(), i);
assert_eq!(result, None);
result = if src_f == dst_f {
Some(info)
} else {
let f = op.extract_field(bx, i);
return coerce_ptr_unsized(bx, f, dst_f);
};
}
result.unwrap()
}
_ => bug!("coerce_ptr_unsized: called on bad types"),
};
// HACK(eddyb) have to bitcast pointers until LLVM removes pointee types.
// FIXME(eddyb) move these out of this `match` arm, so they're always
// applied, uniformly, no matter the source/destination types.
(
bx.bitcast(base, bx.cx().scalar_pair_element_backend_type(dst, 0, true)),
bx.bitcast(info, bx.cx().scalar_pair_element_backend_type(dst, 1, true)),
)
}
OperandValue::Immediate(base) => unsize_thin_ptr(bx, base, src.ty, dst.ty),
OperandValue::Ref(..) => {
bug!("coerce_ptr_unsized: unexpected by-ref operand {:?}", op);
}
};
OperandValue::Pair(base, info)
}

/// Coerces `src`, which is a reference to a value of type `src_ty`,
/// to a value of type `dst_ty`, and stores the result in `dst`.
pub fn coerce_unsized_into<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(
Expand All @@ -232,23 +333,10 @@ pub fn coerce_unsized_into<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(
let dst_ty = dst.layout.ty;
match (&src_ty.kind, &dst_ty.kind) {
(&ty::Ref(..), &ty::Ref(..) | &ty::RawPtr(..)) | (&ty::RawPtr(..), &ty::RawPtr(..)) => {
let (base, info) = match bx.load_operand(src).val {
OperandValue::Pair(base, info) => {
// fat-ptr to fat-ptr unsize preserves the vtable
// i.e., &'a fmt::Debug+Send => &'a fmt::Debug
// So we need to pointercast the base to ensure
// the types match up.
// FIXME(eddyb) use `scalar_pair_element_backend_type` here,
// like `unsize_thin_ptr` does.
let thin_ptr = dst.layout.field(bx.cx(), FAT_PTR_ADDR);
(bx.pointercast(base, bx.cx().backend_type(thin_ptr)), info)
}
OperandValue::Immediate(base) => unsize_thin_ptr(bx, base, src_ty, dst_ty),
OperandValue::Ref(..) => bug!(),
};
OperandValue::Pair(base, info).store(bx, dst);
let src_op = bx.load_operand(src);
let dst_op = coerce_ptr_unsized(bx, src_op, dst.layout);
dst_op.store(bx, dst);
}

(&ty::Adt(def_a, _), &ty::Adt(def_b, _)) => {
assert_eq!(def_a, def_b);

Expand Down Expand Up @@ -339,7 +427,7 @@ pub fn from_immediate<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(
pub fn to_immediate<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(
bx: &mut Bx,
val: Bx::Value,
layout: layout::TyAndLayout<'_>,
layout: TyAndLayout<'_>,
) -> Bx::Value {
if let Abi::Scalar(ref scalar) = layout.abi {
return to_immediate_scalar(bx, val, scalar);
Expand Down
41 changes: 25 additions & 16 deletions src/librustc_codegen_ssa/meth.rs
Expand Up @@ -85,17 +85,21 @@ pub fn get_vtable<'tcx, Cx: CodegenMethods<'tcx>>(
(&[]).iter()
};

let methods = methods.cloned().map(|opt_mth| {
opt_mth.map_or(nullptr, |(def_id, substs)| {
cx.get_fn_addr(
ty::Instance::resolve_for_vtable(
cx.tcx(),
ty::ParamEnv::reveal_all(),
def_id,
substs,
// Resolve instances for each method in the vtable.
// Preserve nested (per-supertrait) structure of list of methods.
let methods = methods.cloned().map(|trait_methods| {
alexreg marked this conversation as resolved.
Show resolved Hide resolved
trait_methods.into_iter().map(|opt_meth| {
opt_meth.map_or(nullptr, |(def_id, substs)| {
cx.get_fn_addr(
ty::Instance::resolve_for_vtable(
cx.tcx(),
ty::ParamEnv::reveal_all(),
def_id,
substs,
)
.unwrap(),
)
.unwrap(),
)
})
})
});

Expand All @@ -104,15 +108,20 @@ pub fn get_vtable<'tcx, Cx: CodegenMethods<'tcx>>(
// If you touch this code, be sure to also make the corresponding changes to
// `get_vtable` in `rust_mir/interpret/traits.rs`.
// /////////////////////////////////////////////////////////////////////////////////////////////
let components: Vec<_> = [
let metadata = [
cx.get_fn_addr(Instance::resolve_drop_in_place(cx.tcx(), ty)),
cx.const_usize(layout.size.bytes()),
cx.const_usize(layout.align.abi.bytes()),
]
.iter()
.cloned()
.chain(methods)
.collect();
];
let mut components: Vec<_> = methods
.flat_map(|trait_methods| {
// Insert copy of metadata before methods for the current (super)trait.
metadata.iter().cloned().chain(trait_methods)
})
.collect();
if components.is_empty() {
components.extend(&metadata);
}

let vtable_const = cx.const_struct(&components, false);
let align = cx.data_layout().pointer_align.abi;
Expand Down
30 changes: 1 addition & 29 deletions src/librustc_codegen_ssa/mir/rvalue.rs
Expand Up @@ -224,35 +224,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
operand.val
}
mir::CastKind::Pointer(PointerCast::Unsize) => {
assert!(bx.cx().is_backend_scalar_pair(cast));
match operand.val {
OperandValue::Pair(lldata, llextra) => {
// unsize from a fat pointer -- this is a
// "trait-object-to-supertrait" coercion, for
// example, `&'a fmt::Debug + Send => &'a fmt::Debug`.

// HACK(eddyb) have to bitcast pointers
// until LLVM removes pointee types.
let lldata = bx.pointercast(
lldata,
bx.cx().scalar_pair_element_backend_type(cast, 0, true),
);
OperandValue::Pair(lldata, llextra)
}
OperandValue::Immediate(lldata) => {
// "standard" unsize
let (lldata, llextra) = base::unsize_thin_ptr(
&mut bx,
lldata,
operand.layout.ty,
cast.ty,
);
OperandValue::Pair(lldata, llextra)
}
OperandValue::Ref(..) => {
bug!("by-ref operand {:?} in `codegen_rvalue_operand`", operand);
}
}
base::coerce_ptr_unsized(&mut bx, operand, cast)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, it might be useful to pull this change, without any of the trait upcasting feature, into a separate PR, so we can discuss there the best organization (ideally it would just be moving code around, while making it easy to implement this PR).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a worthwhile discussion, but please just open an issue about this. Optimisations can come later, as long as we have something working now. I believe it's Niko and my intention to get this merged ASAP now.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not an optimization, I'm not comfortable landing this sizeable feature PR with several refactors embedded in it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But that's not what I wrote. This is a change to get trait upcasting working. Whether this can be optimised further seems like a separate point... Anyway, Niko seemed happy with this before, so let's see what he says.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, I definitely want to get this landed, but I also agree with @eddyb that it would be better if we can extract refactorings and land them independently. This PR is definitely very large and it would be great if we could land it in pieces, each one being relatively easy to comprehend.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nikomatsakis Fair enough, but this one is very much intertwined with functional enhancements to support trait upcasting. Do you have another refactoring in mind here?

}
mir::CastKind::Pointer(PointerCast::MutToConstPointer)
| mir::CastKind::Misc
Expand Down
12 changes: 8 additions & 4 deletions src/librustc_feature/active.rs
Expand Up @@ -497,7 +497,7 @@ declare_features! (
/// Allows the use of raw-dylibs (RFC 2627).
(active, raw_dylib, "1.40.0", Some(58713), None),

/// Allows `#[track_caller]` to be used which provides
/// Allows `#[track_caller]` to be used, which provides
/// accurate caller location reporting during panic (RFC 2091).
(active, track_caller, "1.40.0", Some(47809), None),

Expand All @@ -524,7 +524,7 @@ declare_features! (
/// Allows the use of `if` and `match` in constants.
(active, const_if_match, "1.41.0", Some(49146), None),

/// Allows the use of `#[cfg(sanitize = "option")]`; set when -Zsanitizer is used.
/// Allows the use of `#[cfg(sanitize = "option")]`; set when `-Zsanitizer` is used.
(active, cfg_sanitize, "1.41.0", Some(39699), None),

/// Allows using `..X`, `..=X`, `...X`, and `X..` as a pattern.
Expand Down Expand Up @@ -553,10 +553,10 @@ declare_features! (
/// Allows the use of `no_sanitize` attribute.
(active, no_sanitize, "1.42.0", Some(39699), None),

// Allows limiting the evaluation steps of const expressions
/// Allows limiting the evaluation steps of const expressions.
(active, const_eval_limit, "1.43.0", Some(67217), None),

/// Allow negative trait implementations.
/// Allows negative trait implementations.
(active, negative_impls, "1.44.0", Some(68318), None),

/// Allows the use of `#[target_feature]` on safe functions.
Expand All @@ -565,6 +565,10 @@ declare_features! (
/// Allow conditional compilation depending on rust version
(active, cfg_version, "1.45.0", Some(64796), None),

/// Allows upcasting trait objects via supertraits.
/// Trait upcasting is casting, e.g., `dyn Foo -> dyn Bar` where `Foo: Bar`.
(active, trait_upcasting, "1.45.0", Some(65991), None),

// -------------------------------------------------------------------------
// feature-group-end: actual feature gates
// -------------------------------------------------------------------------
Expand Down