From d14af130f60a8e35ea0bf441b33dd9eb6527f859 Mon Sep 17 00:00:00 2001 From: Michael Hewson Date: Mon, 2 Jul 2018 18:30:59 -0400 Subject: [PATCH 01/19] Make all object-safety methods require a global TyCtxt --- src/librustc/traits/error_reporting.rs | 5 +++-- src/librustc/traits/object_safety.rs | 2 +- src/librustc_typeck/astconv.rs | 2 +- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/librustc/traits/error_reporting.rs b/src/librustc/traits/error_reporting.rs index b463faef1921a..18ee98c515fb2 100644 --- a/src/librustc/traits/error_reporting.rs +++ b/src/librustc/traits/error_reporting.rs @@ -754,7 +754,8 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> { } ty::Predicate::ObjectSafe(trait_def_id) => { - let violations = self.tcx.object_safety_violations(trait_def_id); + let violations = self.tcx.global_tcx() + .object_safety_violations(trait_def_id); self.tcx.report_object_safety_error(span, trait_def_id, violations) @@ -875,7 +876,7 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> { } TraitNotObjectSafe(did) => { - let violations = self.tcx.object_safety_violations(did); + let violations = self.tcx.global_tcx().object_safety_violations(did); self.tcx.report_object_safety_error(span, did, violations) } diff --git a/src/librustc/traits/object_safety.rs b/src/librustc/traits/object_safety.rs index d5942e738fdd9..bb0828e6eb8a0 100644 --- a/src/librustc/traits/object_safety.rs +++ b/src/librustc/traits/object_safety.rs @@ -89,7 +89,7 @@ pub enum MethodViolationCode { NonStandardSelfType, } -impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> { +impl<'a, 'tcx> TyCtxt<'a, 'tcx, 'tcx> { /// Returns the object safety violations that affect /// astconv - currently, Self in supertraits. This is needed diff --git a/src/librustc_typeck/astconv.rs b/src/librustc_typeck/astconv.rs index 7ddc56974d816..afd8c251b7650 100644 --- a/src/librustc_typeck/astconv.rs +++ b/src/librustc_typeck/astconv.rs @@ -1013,7 +1013,7 @@ impl<'o, 'gcx: 'tcx, 'tcx> dyn AstConv<'gcx, 'tcx>+'o { // most importantly, that the supertraits don't contain Self, // to avoid ICE-s. let object_safety_violations = - tcx.astconv_object_safety_violations(principal.def_id()); + tcx.global_tcx().astconv_object_safety_violations(principal.def_id()); if !object_safety_violations.is_empty() { tcx.report_object_safety_error( span, principal.def_id(), object_safety_violations) From a920036f4fe6403396ea05e7c70d0d87c769ff0a Mon Sep 17 00:00:00 2001 From: Michael Hewson Date: Thu, 20 Sep 2018 02:56:10 -0400 Subject: [PATCH 02/19] move some code around to avoid query cycles --- src/librustc/traits/select.rs | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/src/librustc/traits/select.rs b/src/librustc/traits/select.rs index 2ea16823cc65d..0742f377f18c2 100644 --- a/src/librustc/traits/select.rs +++ b/src/librustc/traits/select.rs @@ -2090,18 +2090,6 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> { obligation.self_ty().skip_binder() ); - // Object-safety candidates are only applicable to object-safe - // traits. Including this check is useful because it helps - // inference in cases of traits like `BorrowFrom`, which are - // not object-safe, and which rely on being able to infer the - // self-type from one of the other inputs. Without this check, - // these cases wind up being considered ambiguous due to a - // (spurious) ambiguity introduced here. - let predicate_trait_ref = obligation.predicate.to_poly_trait_ref(); - if !self.tcx().is_object_safe(predicate_trait_ref.def_id()) { - return; - } - self.probe(|this, _snapshot| { // the code below doesn't care about regions, and the // self-ty here doesn't escape this probe, so just erase @@ -2123,6 +2111,17 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> { data.principal().with_self_ty(this.tcx(), self_ty) } ty::Infer(ty::TyVar(_)) => { + // Object-safety candidates are only applicable to object-safe + // traits. Including this check is useful because it helps + // inference in cases of traits like `BorrowFrom`, which are + // not object-safe, and which rely on being able to infer the + // self-type from one of the other inputs. Without this check, + // these cases wind up being considered ambiguous due to a + // (spurious) ambiguity introduced here. + let predicate_trait_ref = obligation.predicate.to_poly_trait_ref(); + if !this.tcx().is_object_safe(predicate_trait_ref.def_id()) { + return; + } debug!("assemble_candidates_from_object_ty: ambiguous"); candidates.ambiguous = true; // could wind up being an object type return; From be80a79a1ea247a71e5ffa408356b9b72cddb644 Mon Sep 17 00:00:00 2001 From: Michael Hewson Date: Thu, 20 Sep 2018 03:04:08 -0400 Subject: [PATCH 03/19] Add CoerceSized trait and lang item This trait is more-or-less the reverse of CoerceUnsized, and will be used for object-safety checks. Receiver types like `Rc` will have to implement `CoerceSized` so that methods that use `Rc` as the receiver will be considered object-safe. --- src/libcore/ops/mod.rs | 3 +++ src/libcore/ops/unsize.rs | 33 ++++++++++++++++++++++++++++++- src/librustc/middle/lang_items.rs | 1 + 3 files changed, 36 insertions(+), 1 deletion(-) diff --git a/src/libcore/ops/mod.rs b/src/libcore/ops/mod.rs index ce4f45762de48..bf9775e2ae813 100644 --- a/src/libcore/ops/mod.rs +++ b/src/libcore/ops/mod.rs @@ -201,3 +201,6 @@ pub use self::generator::{Generator, GeneratorState}; #[unstable(feature = "coerce_unsized", issue = "27732")] pub use self::unsize::CoerceUnsized; + +#[unstable(feature = "coerce_sized", issue = "0")] +pub use self::unsize::CoerceSized; diff --git a/src/libcore/ops/unsize.rs b/src/libcore/ops/unsize.rs index da72f3748425d..4faace26b02f9 100644 --- a/src/libcore/ops/unsize.rs +++ b/src/libcore/ops/unsize.rs @@ -43,7 +43,7 @@ use marker::Unsize; /// [nomicon-coerce]: ../../nomicon/coercions.html #[unstable(feature = "coerce_unsized", issue = "27732")] #[lang = "coerce_unsized"] -pub trait CoerceUnsized { +pub trait CoerceUnsized { // Empty. } @@ -77,3 +77,34 @@ impl, U: ?Sized> CoerceUnsized<*const U> for *mut T {} // *const T -> *const U #[unstable(feature = "coerce_unsized", issue = "27732")] impl, U: ?Sized> CoerceUnsized<*const U> for *const T {} + + +/// Pointers to unsized types that can be coerced to a pointer to a sized type, +/// as long as pointee is actually a value of that sized type. This is used for +/// object safety, to check that a method's receiver type can be coerced from the version +/// where `Self = dyn Trait` to the version where `Self = T`, the erased, sized type +/// of the underlying object. +/// +/// CoerceSized is implemented for: +/// - &[T] is CoerceSized<&[T; N]> for any N +/// - &Trait is CoerceSized<&T> for any T: Trait +/// - and similarly for &mut T, *const T, *mut T, Box, Rc, Arc +#[unstable(feature = "coerce_sized", issue = "0")] +#[cfg_attr(not(stage0), lang = "coerce_sized")] +pub trait CoerceSized where T: CoerceUnsized { + // Empty. +} + +// &U -> &T +#[unstable(feature = "coerce_sized", issue = "0")] +impl<'a, T: ?Sized+Unsize, U: ?Sized> CoerceSized<&'a T> for &'a U {} +// &mut U -> &mut T +#[unstable(feature = "coerce_sized", issue = "0")] +impl<'a, T: ?Sized+Unsize, U: ?Sized> CoerceSized<&'a mut T> for &'a mut U {} +// *const U -> *const T +#[unstable(feature = "coerce_sized", issue = "0")] +impl, U: ?Sized> CoerceSized<*const T> for *const U {} +// *mut U -> *mut T +#[unstable(feature = "coerce_sized", issue = "0")] +impl, U: ?Sized> CoerceSized<*mut T> for *mut U {} + diff --git a/src/librustc/middle/lang_items.rs b/src/librustc/middle/lang_items.rs index 45de958e72eba..67864f67bfc3a 100644 --- a/src/librustc/middle/lang_items.rs +++ b/src/librustc/middle/lang_items.rs @@ -271,6 +271,7 @@ language_item_table! { DropTraitLangItem, "drop", drop_trait, Target::Trait; CoerceUnsizedTraitLangItem, "coerce_unsized", coerce_unsized_trait, Target::Trait; + CoerceSizedTraitLangItem, "coerce_sized", coerce_sized_trait, Target::Trait; AddTraitLangItem, "add", add_trait, Target::Trait; SubTraitLangItem, "sub", sub_trait, Target::Trait; From d5c2c4a4339b2a6c64d16282d85bfd27bf01d361 Mon Sep 17 00:00:00 2001 From: Michael Hewson Date: Thu, 20 Sep 2018 03:12:00 -0400 Subject: [PATCH 04/19] Implement the object-safety checks for arbitrary_self_types: part 1 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit For a trait method to be considered object-safe, the receiver type must satisfy certain properties: first, we need to be able to get the vtable to so we can look up the method, and second, we need to convert the receiver from the version where `Self=dyn Trait`, to the version where `Self=T`, `T` being some unknown, `Sized` type that implements `Trait`. To check that the receiver satisfies those properties, we use the following query: forall (U) { if (Self: Unsize) { Receiver[Self => U]: CoerceSized } } where `Receiver` is the receiver type of the method (e.g. `Rc`), and `Receiver[Self => U]` is the receiver type where `Self = U`, e.g. `Rc`. forall queries like this aren’t implemented in the trait system yet, so for now we are using a bit of a hack — see the code for explanation. --- src/librustc/traits/object_safety.rs | 164 +++++++++++++++++++++++---- 1 file changed, 144 insertions(+), 20 deletions(-) diff --git a/src/librustc/traits/object_safety.rs b/src/librustc/traits/object_safety.rs index bb0828e6eb8a0..470a5c6bc9ebd 100644 --- a/src/librustc/traits/object_safety.rs +++ b/src/librustc/traits/object_safety.rs @@ -13,7 +13,8 @@ //! object if all of their methods meet certain criteria. In particular, //! they must: //! -//! - have a suitable receiver from which we can extract a vtable; +//! - have a suitable receiver from which we can extract a vtable and coerce to a "thin" version +//! that doesn't contain the vtable; //! - not reference the erased type `Self` except for in this receiver; //! - not have generic type parameters @@ -21,11 +22,12 @@ use super::elaborate_predicates; use hir::def_id::DefId; use lint; -use traits; -use ty::{self, Ty, TyCtxt, TypeFoldable}; -use ty::util::ExplicitSelf; +use traits::{self, Obligation, ObligationCause}; +use ty::{self, Ty, TyCtxt, TypeFoldable, Predicate, ToPredicate}; +use ty::subst::{Subst, Substs}; use std::borrow::Cow; -use syntax::ast; +use std::iter::{self}; +use syntax::ast::{self, Name}; use syntax_pos::Span; #[derive(Copy, Clone, Debug, PartialEq, Eq, Hash)] @@ -62,8 +64,8 @@ impl ObjectSafetyViolation { format!("method `{}` references the `Self` type in where clauses", name).into(), ObjectSafetyViolation::Method(name, MethodViolationCode::Generic) => format!("method `{}` has generic type parameters", name).into(), - ObjectSafetyViolation::Method(name, MethodViolationCode::NonStandardSelfType) => - format!("method `{}` has a non-standard `self` type", name).into(), + ObjectSafetyViolation::Method(name, MethodViolationCode::UncoercibleReceiver) => + format!("method `{}` has an uncoercible receiver type", name).into(), ObjectSafetyViolation::AssociatedConst(name) => format!("the trait cannot contain associated consts like `{}`", name).into(), } @@ -85,8 +87,8 @@ pub enum MethodViolationCode { /// e.g., `fn foo()` Generic, - /// arbitrary `self` type, e.g. `self: Rc` - NonStandardSelfType, + /// the self argument can't be coerced from Self=dyn Trait to Self=T where T: Trait + UncoercibleReceiver, } impl<'a, 'tcx> TyCtxt<'a, 'tcx, 'tcx> { @@ -113,6 +115,8 @@ impl<'a, 'tcx> TyCtxt<'a, 'tcx, 'tcx> { pub fn object_safety_violations(self, trait_def_id: DefId) -> Vec { + debug!("object_safety_violations: {:?}", trait_def_id); + traits::supertrait_def_ids(self, trait_def_id) .flat_map(|def_id| self.object_safety_violations_for_trait(def_id)) .collect() @@ -277,23 +281,13 @@ impl<'a, 'tcx> TyCtxt<'a, 'tcx, 'tcx> { method: &ty::AssociatedItem) -> Option { - // The method's first parameter must be something that derefs (or - // autorefs) to `&self`. For now, we only accept `self`, `&self` - // and `Box`. + // The method's first parameter must be named `self` if !method.method_has_self_argument { return Some(MethodViolationCode::StaticMethod); } let sig = self.fn_sig(method.def_id); - let self_ty = self.mk_self_type(); - let self_arg_ty = sig.skip_binder().inputs()[0]; - if let ExplicitSelf::Other = ExplicitSelf::determine(self_arg_ty, |ty| ty == self_ty) { - return Some(MethodViolationCode::NonStandardSelfType); - } - - // The `Self` type is erased, so it should not appear in list of - // arguments or return type apart from the receiver. for input_ty in &sig.skip_binder().inputs()[1..] { if self.contains_illegal_self_type_reference(trait_def_id, input_ty) { return Some(MethodViolationCode::ReferencesSelf); @@ -320,9 +314,139 @@ impl<'a, 'tcx> TyCtxt<'a, 'tcx, 'tcx> { return Some(MethodViolationCode::WhereClauseReferencesSelf(span)); } + let receiver_ty = self.liberate_late_bound_regions( + method.def_id, + &sig.map_bound(|sig| sig.inputs()[0]), + ); + + // until `unsized_locals` is fully implemented, `self: Self` can't be coerced from + // `Self=dyn Trait` to `Self=T`. However, this is already considered object-safe. We allow + // it as a special case here. + // FIXME(mikeyhew) get rid of this `if` statement once `receiver_is_coercible` allows + // `Receiver: Unsize dyn Trait]>` + if receiver_ty != self.mk_self_type() { + if !self.receiver_is_coercible(method, receiver_ty) { + return Some(MethodViolationCode::UncoercibleReceiver); + } + } + None } + /// checks the method's receiver (the `self` argument) can be coerced from + /// a fat pointer, including the trait object vtable, to a thin pointer. + /// e.g. from `Rc` to `Rc`, where `T` is the erased type of the underlying object. + /// More formally: + /// - let `Receiver` be the type of the `self` argument, i.e `Self`, `&Self`, `Rc` + /// - require the following bound: + /// forall(T: Trait) { + /// Receiver[Self => dyn Trait]: CoerceSized T]> + /// } + /// where `Foo[X => Y]` means "the same type as `Foo`, but with `X` replaced with `Y`" + /// (substitution notation). + /// + /// some examples of receiver types and their required obligation + /// - `&'a mut self` requires `&'a mut dyn Trait: CoerceSized<&'a mut T>` + /// - `self: Rc` requires `Rc: CoerceSized>` + /// + /// The only case where the receiver is not coercible, but is still a valid receiver + /// type (just not object-safe), is when there is more than one level of pointer indirection. + /// e.g. `self: &&Self`, `self: &Rc`, `self: Box>`. In these cases, there + /// is no way, or at least no inexpensive way, to coerce the receiver, because the object that + /// needs to be coerced is behind a pointer. + /// + /// In practice, there are issues with the above bound: `where` clauses that apply to `Self` + /// would have to apply to `T`, trait object types have a lot of parameters that need to + /// be filled in (lifetime and type parameters, and the lifetime of the actual object), and + /// I'm pretty sure using `dyn Trait` in the query causes another object-safety query for + /// `Trait`, resulting in cyclic queries. So in the implementation, we use the following, + /// more general bound: + /// + /// forall (U: ?Sized) { + /// if (Self: Unsize) { + /// Receiver[Self => U]: CoerceSized + /// } + /// } + /// + /// for `self: &'a mut Self`, this means `&'a mut U: CoerceSized<&'a mut Self>` + /// for `self: Rc`, this means `Rc: CoerceSized>` + // + // FIXME(mikeyhew) when unsized receivers are implemented as part of unsized rvalues, add this + // fallback query: `Receiver: Unsize U]>` to support receivers like + // `self: Wrapper`. + #[allow(dead_code)] + fn receiver_is_coercible( + self, + method: &ty::AssociatedItem, + receiver_ty: Ty<'tcx>, + ) -> bool { + debug!("receiver_is_coercible: method = {:?}, receiver_ty = {:?}", method, receiver_ty); + + let traits = (self.lang_items().unsize_trait(), + self.lang_items().coerce_sized_trait()); + let (unsize_did, coerce_sized_did) = if let (Some(u), Some(cu)) = traits { + (u, cu) + } else { + debug!("receiver_is_coercible: Missing Unsize or CoerceSized traits"); + return false; + }; + + // use a bogus type parameter to mimick a forall(U) query using u32::MAX for now. + // FIXME(mikeyhew) this is a total hack, and we should replace it when real forall queries + // are implemented + let target_self_ty: Ty<'tcx> = self.mk_ty_param( + ::std::u32::MAX, + Name::intern("RustaceansAreAwesome").as_interned_str(), + ); + + // create a modified param env, with `Self: Unsize` added to the caller bounds + let param_env = { + let mut param_env = self.param_env(method.def_id); + + let predicate = ty::TraitRef { + def_id: unsize_did, + substs: self.mk_substs_trait(self.mk_self_type(), &[target_self_ty.into()]), + }.to_predicate(); + + let caller_bounds: Vec> = param_env.caller_bounds.iter().cloned() + .chain(iter::once(predicate)) + .collect(); + + param_env.caller_bounds = self.intern_predicates(&caller_bounds); + + param_env + }; + + let receiver_substs = Substs::for_item(self, method.def_id, |param, _| { + if param.index == 0 { + target_self_ty.into() + } else { + self.mk_param_from_def(param) + } + }); + // the type `Receiver[Self => U]` in the query + let unsized_receiver_ty = receiver_ty.subst(self, receiver_substs); + + // Receiver[Self => U]: CoerceSized + let obligation = { + let predicate = ty::TraitRef { + def_id: coerce_sized_did, + substs: self.mk_substs_trait(unsized_receiver_ty, &[receiver_ty.into()]), + }.to_predicate(); + + Obligation::new( + ObligationCause::dummy(), + param_env, + predicate, + ) + }; + + self.infer_ctxt().enter(|ref infcx| { + // the receiver is coercible iff the obligation holds + infcx.predicate_must_hold(&obligation) + }) + } + fn contains_illegal_self_type_reference(self, trait_def_id: DefId, ty: Ty<'tcx>) From 9f59da28648c57d7c4fcac371f9f86adddeb20a3 Mon Sep 17 00:00:00 2001 From: Michael Hewson Date: Thu, 20 Sep 2018 03:16:10 -0400 Subject: [PATCH 05/19] Implement object-safety for arbitrary_self_types: part 2 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit For now, all of the receivers that we care about are just a newtyped pointer — i.e. `Box`, `Rc`, `Pin>`, `Pin<&mut Self>`. This is much simpler to implement in codeine than the more general case, because the ABI is the same as a pointer. So we add some checks in typeck/coherence/builtin.rs to make sure that implementors of CoerceSized are just newtyped pointers. In this commit, we also implement the codegen bits. --- src/librustc_codegen_llvm/abi.rs | 49 +++++-- src/librustc_codegen_llvm/mir/block.rs | 40 +++++- src/librustc_typeck/coherence/builtin.rs | 162 ++++++++++++++++++++++- src/librustc_typeck/diagnostics.rs | 74 +++++++++++ 4 files changed, 304 insertions(+), 21 deletions(-) diff --git a/src/librustc_codegen_llvm/abi.rs b/src/librustc_codegen_llvm/abi.rs index 7c7662a88de53..1662b57b8b319 100644 --- a/src/librustc_codegen_llvm/abi.rs +++ b/src/librustc_codegen_llvm/abi.rs @@ -19,7 +19,7 @@ use type_::Type; use type_of::{LayoutLlvmExt, PointerKind}; use value::Value; -use rustc_target::abi::{LayoutOf, Size, TyLayout}; +use rustc_target::abi::{LayoutOf, Size, TyLayout, Abi as LayoutAbi}; use rustc::ty::{self, Ty}; use rustc::ty::layout; @@ -302,21 +302,44 @@ impl<'tcx> FnTypeExt<'tcx> for FnType<'tcx, Ty<'tcx>> { FnType::new_internal(cx, sig, extra_args, |ty, arg_idx| { let mut layout = cx.layout_of(ty); // 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`. + // Instead, pass just the data pointer, but give it the type `*const/mut dyn Trait` + // or `&/&mut dyn Trait` because this is special-cased elsewhere in codegen if arg_idx == Some(0) { - // FIXME(eddyb) `layout.field(cx, 0)` is not enough because e.g. - // `Box` has a few newtype wrappers around the raw - // pointer, so we'd have to "dig down" to find `*dyn Trait`. - let pointee = if layout.is_unsized() { - layout.ty + let fat_pointer_ty = if layout.is_unsized() { + // unsized `self` is passed as a pointer to `self` + // FIXME (mikeyhew) change this to use &own if it is ever added to the language + cx.tcx.mk_mut_ptr(layout.ty) } else { - layout.ty.builtin_deref(true) - .unwrap_or_else(|| { - bug!("FnType::new_vtable: non-pointer self {:?}", layout) - }).ty + match layout.abi { + LayoutAbi::ScalarPair(..) => (), + _ => bug!("receiver type has unsupported layout: {:?}", layout) + } + + let mut fat_pointer_layout = layout; + 'descend_newtypes: while !fat_pointer_layout.ty.is_unsafe_ptr() + && !fat_pointer_layout.ty.is_region_ptr() + { + 'iter_fields: for i in 0..fat_pointer_layout.fields.count() { + let field_layout = fat_pointer_layout.field(cx, i); + + if !field_layout.is_zst() { + fat_pointer_layout = field_layout; + continue 'descend_newtypes + } + } + + bug!("receiver has no non-zero-sized fields {:?}", fat_pointer_layout); + } + + fat_pointer_layout.ty }; - let fat_ptr_ty = cx.tcx.mk_mut_ptr(pointee); - layout = cx.layout_of(fat_ptr_ty).field(cx, 0); + + // we now have a type like `*mut RcBox` + // change its layout to that of `*mut ()`, a thin pointer, but keep the same type + // this is understood as a special case elsewhere in the compiler + let unit_pointer_ty = cx.tcx.mk_mut_ptr(cx.tcx.mk_unit()); + layout = cx.layout_of(unit_pointer_ty); + layout.ty = fat_pointer_ty; } ArgType::new(layout) }) diff --git a/src/librustc_codegen_llvm/mir/block.rs b/src/librustc_codegen_llvm/mir/block.rs index d98b7869ae98e..0cb4963f97fae 100644 --- a/src/librustc_codegen_llvm/mir/block.rs +++ b/src/librustc_codegen_llvm/mir/block.rs @@ -642,14 +642,42 @@ impl FunctionCx<'a, 'll, 'tcx> { (&args[..], None) }; - for (i, arg) in first_args.iter().enumerate() { + 'make_args: for (i, arg) in first_args.iter().enumerate() { let mut op = self.codegen_operand(&bx, arg); + if let (0, Some(ty::InstanceDef::Virtual(_, idx))) = (i, def) { - if let Pair(data_ptr, meta) = op.val { - llfn = Some(meth::VirtualIndex::from_index(idx) - .get_fn(&bx, meta, &fn_ty)); - llargs.push(data_ptr); - continue; + if let Pair(..) = op.val { + // descend through newtype wrappers until `op` is a builtin pointer to + // `dyn Trait`, e.g. `*const dyn Trait`, `&mut dyn Trait` + 'descend_newtypes: while !op.layout.ty.is_unsafe_ptr() + && !op.layout.ty.is_region_ptr() + { + 'iter_fields: for i in 0..op.layout.fields.count() { + let field = op.extract_field(&bx, i); + if !field.layout.is_zst() { + // we found the one non-zero-sized field that is allowed + // now find *its* non-zero-sized field, or stop if it's a + // pointer + op = field; + continue 'descend_newtypes + } + } + + span_bug!(span, "receiver has no non-zero-sized fields {:?}", op); + } + + // now that we have `*dyn Trait` or `&dyn Trait`, split it up into its + // data pointer and vtable. Look up the method in the vtable, and pass + // the data pointer as the first argument + match op.val { + Pair(data_ptr, meta) => { + llfn = Some(meth::VirtualIndex::from_index(idx) + .get_fn(&bx, meta, &fn_ty)); + llargs.push(data_ptr); + continue 'make_args + } + other => bug!("expected a Pair, got {:?}", other) + } } else if let Ref(data_ptr, Some(meta), _) = op.val { // by-value dynamic dispatch llfn = Some(meth::VirtualIndex::from_index(idx) diff --git a/src/librustc_typeck/coherence/builtin.rs b/src/librustc_typeck/coherence/builtin.rs index 05a83dd307c38..eba73b1da9aab 100644 --- a/src/librustc_typeck/coherence/builtin.rs +++ b/src/librustc_typeck/coherence/builtin.rs @@ -31,8 +31,8 @@ pub fn check_trait<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, trait_def_id: DefId) { Checker { tcx, trait_def_id } .check(tcx.lang_items().drop_trait(), visit_implementation_of_drop) .check(tcx.lang_items().copy_trait(), visit_implementation_of_copy) - .check(tcx.lang_items().coerce_unsized_trait(), - visit_implementation_of_coerce_unsized); + .check(tcx.lang_items().coerce_unsized_trait(), visit_implementation_of_coerce_unsized) + .check(tcx.lang_items().coerce_sized_trait(), visit_implementation_of_coerce_sized); } struct Checker<'a, 'tcx: 'a> { @@ -162,6 +162,164 @@ fn visit_implementation_of_coerce_unsized<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, } } +fn visit_implementation_of_coerce_sized<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, impl_did: DefId) { + debug!("visit_implementation_of_coerce_sized: impl_did={:?}", + impl_did); + if impl_did.is_local() { + let coerce_sized_trait = tcx.lang_items().coerce_sized_trait().unwrap(); + + let impl_node_id = tcx.hir.as_local_node_id(impl_did).unwrap(); + let span = tcx.hir.span(impl_node_id); + + let source = tcx.type_of(impl_did); + assert!(!source.has_escaping_regions()); + let target = { + let trait_ref = tcx.impl_trait_ref(impl_did).unwrap(); + assert_eq!(trait_ref.def_id, coerce_sized_trait); + + trait_ref.substs.type_at(1) + }; + + debug!("visit_implementation_of_coerce_sized: {:?} -> {:?}", + source, + target); + + let param_env = tcx.param_env(impl_did); + + let create_err = |msg: &str| { + struct_span_err!(tcx.sess, span, E0378, "{}", msg) + }; + + tcx.infer_ctxt().enter(|infcx| { + let cause = ObligationCause::misc(span, impl_node_id); + + use ty::TyKind::*; + match (&source.sty, &target.sty) { + (&Ref(r_a, _, mutbl_a), Ref(r_b, _, mutbl_b)) + if infcx.at(&cause, param_env).eq(r_a, r_b).is_ok() + && mutbl_a == *mutbl_b => (), + (&RawPtr(tm_a), &RawPtr(tm_b)) + if tm_a.mutbl == tm_b.mutbl => (), + (&Adt(def_a, substs_a), &Adt(def_b, substs_b)) + if def_a.is_struct() && def_b.is_struct() => + { + if def_a != def_b { + let source_path = tcx.item_path_str(def_a.did); + let target_path = tcx.item_path_str(def_b.did); + + create_err( + &format!( + "the trait `CoerceSized` may only be implemented \ + for a coercion between structures with the same \ + definition; expected {}, found {}", + source_path, target_path, + ) + ).emit(); + + return + } + + let fields = &def_a.non_enum_variant().fields; + + let coerced_fields = fields.iter().filter_map(|field| { + if tcx.type_of(field.did).is_phantom_data() { + // ignore PhantomData fields + return None + } + + let ty_a = field.ty(tcx, substs_a); + let ty_b = field.ty(tcx, substs_b); + if let Ok(ok) = infcx.at(&cause, param_env).eq(ty_a, ty_b) { + if ok.obligations.is_empty() { + create_err( + "the trait `CoerceSized` may only be implemented for structs \ + containing the field being coerced, `PhantomData` fields, \ + and nothing else" + ).note( + &format!( + "extra field `{}` of type `{}` is not allowed", + field.ident, ty_a, + ) + ).emit(); + + return None; + } + } + + Some(field) + }).collect::>(); + + if coerced_fields.is_empty() { + create_err( + "the trait `CoerceSized` may only be implemented \ + for a coercion between structures with a single field \ + being coerced, none found" + ).emit(); + } else if coerced_fields.len() > 1 { + create_err( + "implementing the `CoerceSized` trait requires multiple coercions", + ).note( + "the trait `CoerceSized` may only be implemented \ + for a coercion between structures with a single field \ + being coerced" + ).note( + &format!( + "currently, {} fields need coercions: {}", + coerced_fields.len(), + coerced_fields.iter().map(|field| { + format!("{} ({} to {})", + field.ident, + field.ty(tcx, substs_a), + field.ty(tcx, substs_b), + ) + }).collect::>() + .join(", ") + ) + ).emit(); + } else { + let mut fulfill_cx = TraitEngine::new(infcx.tcx); + + for field in coerced_fields { + + let predicate = tcx.predicate_for_trait_def( + param_env, + cause.clone(), + coerce_sized_trait, + 0, + field.ty(tcx, substs_a), + &[field.ty(tcx, substs_b).into()] + ); + + fulfill_cx.register_predicate_obligation(&infcx, predicate); + } + + // Check that all transitive obligations are satisfied. + if let Err(errors) = fulfill_cx.select_all_or_error(&infcx) { + infcx.report_fulfillment_errors(&errors, None, false); + } + + // Finally, resolve all regions. + let region_scope_tree = region::ScopeTree::default(); + let outlives_env = OutlivesEnvironment::new(param_env); + infcx.resolve_regions_and_report_errors( + impl_did, + ®ion_scope_tree, + &outlives_env, + SuppressRegionErrors::default(), + ); + } + } + _ => { + create_err( + "the trait `CoerceSsized` may only be implemented \ + for a coercion between structures" + ).emit(); + } + } + }) + } +} + pub fn coerce_unsized_info<'a, 'gcx>(gcx: TyCtxt<'a, 'gcx, 'gcx>, impl_did: DefId) -> CoerceUnsizedInfo { diff --git a/src/librustc_typeck/diagnostics.rs b/src/librustc_typeck/diagnostics.rs index f57d050fa2d77..2cfca345c27f8 100644 --- a/src/librustc_typeck/diagnostics.rs +++ b/src/librustc_typeck/diagnostics.rs @@ -3084,6 +3084,80 @@ containing the unsized type is the last and only unsized type field in the struct. "##, +E0378: r##" +The `CoerceSized` trait currently can only be implemented for builtin pointer +types and structs that are newtype wrappers around them — that is, the struct +must have only one field (except for`PhantomData`), and that field must itself +implement `CoerceSized`. + +Examples: + +``` +#![feature(coerce_sized, unsize)] +use std::{ + marker::Unsize, + ops::CoerceSized, +}; + +struct Ptr(*const T); + +impl CoerceUnsized> for Ptr +where + T: Unsize, +{} + +impl CoerceSized> for Ptr +where + T: Unsize, +{} +``` + +``` +#![feature(coerce_unsized, coerce_sized)] +use std::ops::{CoerceUnsized, CoerceSized}; + +struct Wrapper { + ptr: T, + _phantom: PhantomData<()>, +} + +impl CoerceUnsized> for Wrapper +where + T: CoerceUnsized, +{} + +impl CoerceSized> for Wrapper +where + T: CoerceUnsized, + U: CoerceSized, +{} +``` + +Example of illegal CoerceSized implementation +(illegal because of extra field) + +```compile-fail,E0378 +#![feature(coerce_unsized, coerce_sized)] +use std::ops::{CoerceUnsized, CoerceSized}; + +struct WrapperWithExtraField { + ptr: T, + extra_stuff: i32, +} + +impl CoerceUnsized> for WrapperWithExtraField +where + T: CoerceUnsized, +{} + +impl CoerceSized> for WrapperWithExtraField +where + T: CoerceUnsized, + U: CoerceSized, +{} +``` +"##, + E0390: r##" You tried to implement methods for a primitive type. Erroneous code example: From 192900e7c2792b2afafb055ddba530252ce5c0a3 Mon Sep 17 00:00:00 2001 From: Michael Hewson Date: Thu, 20 Sep 2018 03:18:00 -0400 Subject: [PATCH 06/19] Add CoerceSized impls throughout libstd This will make receiver types like `Rc` and `Pin<&mut Self>` object-safe. --- src/liballoc/boxed.rs | 5 ++++- src/liballoc/lib.rs | 1 + src/liballoc/rc.rs | 8 +++++++- src/liballoc/sync.rs | 7 ++++++- src/libcore/nonzero.rs | 4 +++- src/libcore/pin.rs | 9 ++++++++- src/libcore/ptr.rs | 8 +++++++- 7 files changed, 36 insertions(+), 6 deletions(-) diff --git a/src/liballoc/boxed.rs b/src/liballoc/boxed.rs index f989e701913a5..8a4e646f278ec 100644 --- a/src/liballoc/boxed.rs +++ b/src/liballoc/boxed.rs @@ -77,7 +77,7 @@ use core::iter::FusedIterator; use core::marker::{Unpin, Unsize}; use core::mem; use core::pin::Pin; -use core::ops::{CoerceUnsized, Deref, DerefMut, Generator, GeneratorState}; +use core::ops::{CoerceUnsized, CoerceSized, Deref, DerefMut, Generator, GeneratorState}; use core::ptr::{self, NonNull, Unique}; use core::task::{LocalWaker, Poll}; @@ -696,6 +696,9 @@ impl<'a, A, R> FnOnce for Box + Send + 'a> { #[unstable(feature = "coerce_unsized", issue = "27732")] impl, U: ?Sized> CoerceUnsized> for Box {} +#[unstable(feature = "coerce_sized", issue = "0")] +impl, U: ?Sized> CoerceSized> for Box {} + #[stable(feature = "box_slice_clone", since = "1.3.0")] impl Clone for Box<[T]> { fn clone(&self) -> Self { diff --git a/src/liballoc/lib.rs b/src/liballoc/lib.rs index 7db6261a01c0b..93c340a6dbd2d 100644 --- a/src/liballoc/lib.rs +++ b/src/liballoc/lib.rs @@ -86,6 +86,7 @@ #![feature(box_syntax)] #![feature(cfg_target_has_atomic)] #![feature(coerce_unsized)] +#![feature(coerce_sized)] #![feature(core_intrinsics)] #![feature(custom_attribute)] #![feature(dropck_eyepatch)] diff --git a/src/liballoc/rc.rs b/src/liballoc/rc.rs index 3e8dfd105de2e..c1cc7ac976ded 100644 --- a/src/liballoc/rc.rs +++ b/src/liballoc/rc.rs @@ -255,7 +255,7 @@ use core::marker; use core::marker::{Unpin, Unsize, PhantomData}; use core::mem::{self, align_of_val, forget, size_of_val}; use core::ops::Deref; -use core::ops::CoerceUnsized; +use core::ops::{CoerceUnsized, CoerceSized}; use core::pin::Pin; use core::ptr::{self, NonNull}; use core::convert::From; @@ -297,6 +297,9 @@ impl !marker::Sync for Rc {} #[unstable(feature = "coerce_unsized", issue = "27732")] impl, U: ?Sized> CoerceUnsized> for Rc {} +#[unstable(feature = "coerce_sized", issue = "0")] +impl, U: ?Sized> CoerceSized> for Rc {} + impl Rc { /// Constructs a new `Rc`. /// @@ -1176,6 +1179,9 @@ impl !marker::Sync for Weak {} #[unstable(feature = "coerce_unsized", issue = "27732")] impl, U: ?Sized> CoerceUnsized> for Weak {} +#[unstable(feature = "coerce_sized", issue = "0")] +impl, U: ?Sized> CoerceSized> for Weak {} + impl Weak { /// Constructs a new `Weak`, without allocating any memory. /// Calling [`upgrade`][Weak::upgrade] on the return value always gives [`None`]. diff --git a/src/liballoc/sync.rs b/src/liballoc/sync.rs index bcf5212f1ff6f..358e18bbe1b90 100644 --- a/src/liballoc/sync.rs +++ b/src/liballoc/sync.rs @@ -25,7 +25,7 @@ use core::cmp::Ordering; use core::intrinsics::abort; use core::mem::{self, align_of_val, size_of_val}; use core::ops::Deref; -use core::ops::CoerceUnsized; +use core::ops::{CoerceUnsized, CoerceSized}; use core::pin::Pin; use core::ptr::{self, NonNull}; use core::marker::{Unpin, Unsize, PhantomData}; @@ -214,6 +214,9 @@ unsafe impl Sync for Arc {} #[unstable(feature = "coerce_unsized", issue = "27732")] impl, U: ?Sized> CoerceUnsized> for Arc {} +#[unstable(feature = "coerce_sized", issue = "0")] +impl, U: ?Sized> CoerceSized> for Arc {} + /// `Weak` is a version of [`Arc`] that holds a non-owning reference to the /// managed value. The value is accessed by calling [`upgrade`] on the `Weak` /// pointer, which returns an [`Option`]`<`[`Arc`]`>`. @@ -254,6 +257,8 @@ unsafe impl Sync for Weak {} #[unstable(feature = "coerce_unsized", issue = "27732")] impl, U: ?Sized> CoerceUnsized> for Weak {} +#[unstable(feature = "coerce_sized", issue = "0")] +impl, U: ?Sized> CoerceSized> for Weak {} #[stable(feature = "arc_weak", since = "1.4.0")] impl fmt::Debug for Weak { diff --git a/src/libcore/nonzero.rs b/src/libcore/nonzero.rs index 118e75e1ee704..2bc6c36171ce3 100644 --- a/src/libcore/nonzero.rs +++ b/src/libcore/nonzero.rs @@ -10,7 +10,7 @@ //! Exposes the NonZero lang item which provides optimization hints. -use ops::CoerceUnsized; +use ops::{CoerceUnsized, CoerceSized}; /// A wrapper type for raw pointers and integers that will never be /// NULL or 0 that might allow certain optimizations. @@ -20,3 +20,5 @@ use ops::CoerceUnsized; pub(crate) struct NonZero(pub(crate) T); impl, U> CoerceUnsized> for NonZero {} + +impl, U: CoerceSized> CoerceSized> for NonZero {} diff --git a/src/libcore/pin.rs b/src/libcore/pin.rs index a03c080fb3f34..87ca193aeee5c 100644 --- a/src/libcore/pin.rs +++ b/src/libcore/pin.rs @@ -91,7 +91,7 @@ use fmt; use marker::Sized; -use ops::{Deref, DerefMut, CoerceUnsized}; +use ops::{Deref, DerefMut, CoerceUnsized, CoerceSized}; #[doc(inline)] pub use marker::Unpin; @@ -324,5 +324,12 @@ where P: CoerceUnsized, {} +#[unstable(feature = "pin", issue = "49150")] +impl<'a, P, U> CoerceSized> for Pin +where + P: CoerceUnsized, + U: CoerceSized

, +{} + #[unstable(feature = "pin", issue = "49150")] impl

Unpin for Pin

{} diff --git a/src/libcore/ptr.rs b/src/libcore/ptr.rs index 36852b10facde..24ef028b49d5d 100644 --- a/src/libcore/ptr.rs +++ b/src/libcore/ptr.rs @@ -75,7 +75,7 @@ use convert::From; use intrinsics; -use ops::CoerceUnsized; +use ops::{CoerceUnsized, CoerceSized}; use fmt; use hash; use marker::{PhantomData, Unsize}; @@ -2795,6 +2795,9 @@ impl Copy for Unique { } #[unstable(feature = "ptr_internals", issue = "0")] impl CoerceUnsized> for Unique where T: Unsize { } +#[unstable(feature = "ptr_internals", issue = "0")] +impl CoerceSized> for Unique where T: Unsize { } + #[unstable(feature = "ptr_internals", issue = "0")] impl fmt::Pointer for Unique { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { @@ -2951,6 +2954,9 @@ impl Copy for NonNull { } #[unstable(feature = "coerce_unsized", issue = "27732")] impl CoerceUnsized> for NonNull where T: Unsize { } +#[unstable(feature = "coerce_sized", issue = "0")] +impl CoerceSized> for NonNull where T: Unsize { } + #[stable(feature = "nonnull", since = "1.25.0")] impl fmt::Debug for NonNull { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { From a0f23f8405c25b8547654b40ad5fa05fbe4c4b47 Mon Sep 17 00:00:00 2001 From: Michael Hewson Date: Thu, 20 Sep 2018 03:21:29 -0400 Subject: [PATCH 07/19] update tests that have changed output MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit I’m not sure why these tests have different output now, but they do. In all cases, the error message that is missing looks like this: “the trait bound `dyn Trait: Trait` is not satisfied” My guess is that the error message is going away because object-safety now involves trait solving, and these extra error messages are no longer leaking out. --- src/test/ui/traits/trait-item-privacy.rs | 2 -- src/test/ui/traits/trait-item-privacy.stderr | 38 ++++---------------- src/test/ui/traits/trait-test-2.rs | 1 - src/test/ui/traits/trait-test-2.stderr | 10 ++---- 4 files changed, 9 insertions(+), 42 deletions(-) diff --git a/src/test/ui/traits/trait-item-privacy.rs b/src/test/ui/traits/trait-item-privacy.rs index f8e4f0d596e20..1db5ec097376c 100644 --- a/src/test/ui/traits/trait-item-privacy.rs +++ b/src/test/ui/traits/trait-item-privacy.rs @@ -110,9 +110,7 @@ fn check_assoc_const() { // A, B, C are resolved as inherent items, their traits don't need to be in scope C::A; //~ ERROR associated constant `A` is private //~^ ERROR the trait `assoc_const::C` cannot be made into an object - //~| ERROR the trait bound `dyn assoc_const::C: assoc_const::A` is not satisfied C::B; // ERROR the trait `assoc_const::C` cannot be made into an object - //~^ ERROR the trait bound `dyn assoc_const::C: assoc_const::B` is not satisfied C::C; // OK } diff --git a/src/test/ui/traits/trait-item-privacy.stderr b/src/test/ui/traits/trait-item-privacy.stderr index fc14ae91d7b36..4ede83d5d7362 100644 --- a/src/test/ui/traits/trait-item-privacy.stderr +++ b/src/test/ui/traits/trait-item-privacy.stderr @@ -100,30 +100,6 @@ error[E0624]: associated constant `A` is private LL | C::A; //~ ERROR associated constant `A` is private | ^^^^ -error[E0277]: the trait bound `dyn assoc_const::C: assoc_const::A` is not satisfied - --> $DIR/trait-item-privacy.rs:111:5 - | -LL | C::A; //~ ERROR associated constant `A` is private - | ^^^^ the trait `assoc_const::A` is not implemented for `dyn assoc_const::C` - | -note: required by `assoc_const::A::A` - --> $DIR/trait-item-privacy.rs:35:9 - | -LL | const A: u8 = 0; - | ^^^^^^^^^^^^^^^^ - -error[E0277]: the trait bound `dyn assoc_const::C: assoc_const::B` is not satisfied - --> $DIR/trait-item-privacy.rs:114:5 - | -LL | C::B; // ERROR the trait `assoc_const::C` cannot be made into an object - | ^^^^ the trait `assoc_const::B` is not implemented for `dyn assoc_const::C` - | -note: required by `assoc_const::B::B` - --> $DIR/trait-item-privacy.rs:39:9 - | -LL | const B: u8 = 0; - | ^^^^^^^^^^^^^^^^ - error[E0038]: the trait `assoc_const::C` cannot be made into an object --> $DIR/trait-item-privacy.rs:111:5 | @@ -135,36 +111,36 @@ LL | C::A; //~ ERROR associated constant `A` is private = note: the trait cannot contain associated consts like `A` error[E0223]: ambiguous associated type - --> $DIR/trait-item-privacy.rs:127:12 + --> $DIR/trait-item-privacy.rs:125:12 | LL | let _: S::A; //~ ERROR ambiguous associated type | ^^^^ help: use fully-qualified syntax: `::A` error[E0223]: ambiguous associated type - --> $DIR/trait-item-privacy.rs:128:12 + --> $DIR/trait-item-privacy.rs:126:12 | LL | let _: S::B; //~ ERROR ambiguous associated type | ^^^^ help: use fully-qualified syntax: `::B` error[E0223]: ambiguous associated type - --> $DIR/trait-item-privacy.rs:129:12 + --> $DIR/trait-item-privacy.rs:127:12 | LL | let _: S::C; //~ ERROR ambiguous associated type | ^^^^ help: use fully-qualified syntax: `::C` error: associated type `A` is private - --> $DIR/trait-item-privacy.rs:131:12 + --> $DIR/trait-item-privacy.rs:129:12 | LL | let _: T::A; //~ ERROR associated type `A` is private | ^^^^ error: associated type `A` is private - --> $DIR/trait-item-privacy.rs:140:9 + --> $DIR/trait-item-privacy.rs:138:9 | LL | A = u8, //~ ERROR associated type `A` is private | ^^^^^^ -error: aborting due to 17 previous errors +error: aborting due to 15 previous errors -Some errors occurred: E0038, E0223, E0277, E0599, E0624. +Some errors occurred: E0038, E0223, E0599, E0624. For more information about an error, try `rustc --explain E0038`. diff --git a/src/test/ui/traits/trait-test-2.rs b/src/test/ui/traits/trait-test-2.rs index dac76fb57fd7c..01d7e89847a89 100644 --- a/src/test/ui/traits/trait-test-2.rs +++ b/src/test/ui/traits/trait-test-2.rs @@ -20,5 +20,4 @@ fn main() { (box 10 as Box).dup(); //~^ ERROR E0038 //~| ERROR E0038 - //~| ERROR E0277 } diff --git a/src/test/ui/traits/trait-test-2.stderr b/src/test/ui/traits/trait-test-2.stderr index 1e1fcbe340e58..db0cd38cb6a68 100644 --- a/src/test/ui/traits/trait-test-2.stderr +++ b/src/test/ui/traits/trait-test-2.stderr @@ -10,12 +10,6 @@ error[E0107]: wrong number of type arguments: expected 1, found 2 LL | 10.blah::(); //~ ERROR wrong number of type arguments: expected 1, found 2 | ^^^ unexpected type argument -error[E0277]: the trait bound `dyn bar: bar` is not satisfied - --> $DIR/trait-test-2.rs:20:26 - | -LL | (box 10 as Box).dup(); - | ^^^ the trait `bar` is not implemented for `dyn bar` - error[E0038]: the trait `bar` cannot be made into an object --> $DIR/trait-test-2.rs:20:16 | @@ -35,7 +29,7 @@ LL | (box 10 as Box).dup(); = note: method `blah` has generic type parameters = note: required because of the requirements on the impl of `std::ops::CoerceUnsized>` for `std::boxed::Box<{integer}>` -error: aborting due to 5 previous errors +error: aborting due to 4 previous errors -Some errors occurred: E0038, E0107, E0277. +Some errors occurred: E0038, E0107. For more information about an error, try `rustc --explain E0038`. From 82f1f9a5b4ccd1eae1ac6f00d98f9fb61b94946d Mon Sep 17 00:00:00 2001 From: Michael Hewson Date: Thu, 20 Sep 2018 03:28:57 -0400 Subject: [PATCH 08/19] Add new tests and update existing for object-safe custom receivers --- ...itrary_self_types_pointers_and_wrappers.rs | 79 +++++++++++++++++++ .../arbitrary_self_types_stdlib_pointers.rs | 56 +++++++++++++ .../arbitrary-self-types-not-object-safe.rs | 20 ++--- ...rbitrary-self-types-not-object-safe.stderr | 16 ++-- src/test/ui/invalid_coerce_sized_impls.rs | 54 +++++++++++++ src/test/ui/invalid_coerce_sized_impls.stderr | 33 ++++++++ 6 files changed, 240 insertions(+), 18 deletions(-) create mode 100644 src/test/run-pass/arbitrary_self_types_pointers_and_wrappers.rs create mode 100644 src/test/run-pass/arbitrary_self_types_stdlib_pointers.rs create mode 100644 src/test/ui/invalid_coerce_sized_impls.rs create mode 100644 src/test/ui/invalid_coerce_sized_impls.stderr diff --git a/src/test/run-pass/arbitrary_self_types_pointers_and_wrappers.rs b/src/test/run-pass/arbitrary_self_types_pointers_and_wrappers.rs new file mode 100644 index 0000000000000..3abe806c9a9d3 --- /dev/null +++ b/src/test/run-pass/arbitrary_self_types_pointers_and_wrappers.rs @@ -0,0 +1,79 @@ +// Copyright 2017 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. +#![feature(arbitrary_self_types, unsize, coerce_unsized, coerce_sized)] +#![feature(rustc_attrs)] + +use std::{ + ops::{Deref, CoerceUnsized, CoerceSized}, + marker::Unsize, + fmt::Debug, +}; + +struct Ptr(Box); + +impl Deref for Ptr { + type Target = T; + + fn deref(&self) -> &T { + &*self.0 + } +} + +impl + ?Sized, U: ?Sized> CoerceUnsized> for Ptr {} +impl + ?Sized, U: ?Sized> CoerceSized> for Ptr {} + +struct Wrapper(T); + +impl Deref for Wrapper { + type Target = T; + + fn deref(&self) -> &T { + &self.0 + } +} + +impl, U> CoerceUnsized> for Wrapper {} +impl, U: CoerceSized> CoerceSized> for Wrapper {} + + +trait Trait { + // This method can't be called on trait objects, since the receiver would be unsized, + // but should not cause an object safety error + // fn wrapper(self: Wrapper) -> i32; + fn ptr_wrapper(self: Ptr>) -> i32; + fn wrapper_ptr(self: Wrapper>) -> i32; + fn wrapper_ptr_wrapper(self: Wrapper>>) -> i32; +} + +impl Trait for i32 { + // fn wrapper(self: Wrapper) -> i32 { + // *self + // } + fn ptr_wrapper(self: Ptr>) -> i32 { + **self + } + fn wrapper_ptr(self: Wrapper>) -> i32 { + **self + } + fn wrapper_ptr_wrapper(self: Wrapper>>) -> i32 { + ***self + } +} + +fn main() { + let pw = Ptr(Box::new(Wrapper(5))) as Ptr>; + assert_eq!(pw.ptr_wrapper(), 5); + + let wp = Wrapper(Ptr(Box::new(6))) as Wrapper>; + assert_eq!(wp.wrapper_ptr(), 6); + + let wpw = Wrapper(Ptr(Box::new(Wrapper(7)))) as Wrapper>>; + assert_eq!(wpw.wrapper_ptr_wrapper(), 7); +} diff --git a/src/test/run-pass/arbitrary_self_types_stdlib_pointers.rs b/src/test/run-pass/arbitrary_self_types_stdlib_pointers.rs new file mode 100644 index 0000000000000..80a7ce9691126 --- /dev/null +++ b/src/test/run-pass/arbitrary_self_types_stdlib_pointers.rs @@ -0,0 +1,56 @@ +// Copyright 2018 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +#![feature(arbitrary_self_types)] +#![feature(pin)] +#![feature(rustc_attrs)] + +use std::{ + rc::Rc, + sync::Arc, + pin::Pin, +}; + +trait Trait { + fn by_rc(self: Rc) -> i64; + fn by_arc(self: Arc) -> i64; + fn by_pin_mut(self: Pin<&mut Self>) -> i64; + fn by_pin_box(self: Pin>) -> i64; +} + +impl Trait for i64 { + fn by_rc(self: Rc) -> i64 { + *self + } + fn by_arc(self: Arc) -> i64 { + *self + } + fn by_pin_mut(self: Pin<&mut Self>) -> i64 { + *self + } + fn by_pin_box(self: Pin>) -> i64 { + *self + } +} + +fn main() { + let rc = Rc::new(1i64) as Rc; + assert_eq!(1, rc.by_rc()); + + let arc = Arc::new(2i64) as Arc; + assert_eq!(2, arc.by_arc()); + + let mut value = 3i64; + let pin_mut = Pin::new(&mut value) as Pin<&mut dyn Trait>; + assert_eq!(3, pin_mut.by_pin_mut()); + + let pin_box = Into::>>::into(Box::new(4i64)) as Pin>; + assert_eq!(4, pin_box.by_pin_box()); +} diff --git a/src/test/ui/arbitrary-self-types-not-object-safe.rs b/src/test/ui/arbitrary-self-types-not-object-safe.rs index 48918b996ef59..4dc481174a45d 100644 --- a/src/test/ui/arbitrary-self-types-not-object-safe.rs +++ b/src/test/ui/arbitrary-self-types-not-object-safe.rs @@ -12,38 +12,38 @@ use std::rc::Rc; trait Foo { - fn foo(self: Rc) -> usize; + fn foo(self: &Rc) -> usize; } trait Bar { - fn foo(self: Rc) -> usize where Self: Sized; - fn bar(self: Box) -> usize; + fn foo(self: &Rc) -> usize where Self: Sized; + fn bar(self: Rc) -> usize; } impl Foo for usize { - fn foo(self: Rc) -> usize { - *self + fn foo(self: &Rc) -> usize { + **self } } impl Bar for usize { - fn foo(self: Rc) -> usize { - *self + fn foo(self: &Rc) -> usize { + **self } - fn bar(self: Box) -> usize { + fn bar(self: Rc) -> usize { *self } } fn make_foo() { - let x = Box::new(5usize) as Box; + let x = Rc::new(5usize) as Rc; //~^ ERROR E0038 //~| ERROR E0038 } fn make_bar() { - let x = Box::new(5usize) as Box; + let x = Rc::new(5usize) as Rc; x.bar(); } diff --git a/src/test/ui/arbitrary-self-types-not-object-safe.stderr b/src/test/ui/arbitrary-self-types-not-object-safe.stderr index ec9e65fc4c62d..715fc86517bee 100644 --- a/src/test/ui/arbitrary-self-types-not-object-safe.stderr +++ b/src/test/ui/arbitrary-self-types-not-object-safe.stderr @@ -1,19 +1,19 @@ error[E0038]: the trait `Foo` cannot be made into an object - --> $DIR/arbitrary-self-types-not-object-safe.rs:40:33 + --> $DIR/arbitrary-self-types-not-object-safe.rs:40:32 | -LL | let x = Box::new(5usize) as Box; - | ^^^^^^^^ the trait `Foo` cannot be made into an object +LL | let x = Rc::new(5usize) as Rc; + | ^^^^^^^ the trait `Foo` cannot be made into an object | - = note: method `foo` has a non-standard `self` type + = note: method `foo` has an uncoercible receiver type error[E0038]: the trait `Foo` cannot be made into an object --> $DIR/arbitrary-self-types-not-object-safe.rs:40:13 | -LL | let x = Box::new(5usize) as Box; - | ^^^^^^^^^^^^^^^^ the trait `Foo` cannot be made into an object +LL | let x = Rc::new(5usize) as Rc; + | ^^^^^^^^^^^^^^^ the trait `Foo` cannot be made into an object | - = note: method `foo` has a non-standard `self` type - = note: required because of the requirements on the impl of `std::ops::CoerceUnsized>` for `std::boxed::Box` + = note: method `foo` has an uncoercible receiver type + = note: required because of the requirements on the impl of `std::ops::CoerceUnsized>` for `std::rc::Rc` error: aborting due to 2 previous errors diff --git a/src/test/ui/invalid_coerce_sized_impls.rs b/src/test/ui/invalid_coerce_sized_impls.rs new file mode 100644 index 0000000000000..27d65b880fa15 --- /dev/null +++ b/src/test/ui/invalid_coerce_sized_impls.rs @@ -0,0 +1,54 @@ +// Copyright 2018 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +#![feature(unsize, coerce_sized, coerce_unsized)] + +use std::{ + ops::{CoerceSized, CoerceUnsized}, + marker::{Unsize, PhantomData}, +}; + +struct WrapperWithExtraField(T, i32); + +impl CoerceUnsized> for WrapperWithExtraField +where + T: CoerceUnsized, +{} + +impl CoerceSized> for WrapperWithExtraField +where + T: CoerceUnsized, + U: CoerceSized, +{} //~^^^^ ERROR [E0378] + + +struct MultiplePointers{ + ptr1: *const T, + ptr2: *const T, +} + +// No CoerceUnsized impl + +impl CoerceSized> for MultiplePointers +where + T: Unsize, +{} //~^^^ ERROR [E0378] + + +struct NothingToCoerce { + data: PhantomData, +} + +// No CoerceUnsized impl + +impl CoerceSized> for NothingToCoerce {} +//~^ ERROR [E0378] + +fn main() {} diff --git a/src/test/ui/invalid_coerce_sized_impls.stderr b/src/test/ui/invalid_coerce_sized_impls.stderr new file mode 100644 index 0000000000000..70fb464d426ed --- /dev/null +++ b/src/test/ui/invalid_coerce_sized_impls.stderr @@ -0,0 +1,33 @@ +error[E0378]: the trait `CoerceSized` may only be implemented for structs containing the field being coerced, `PhantomData` fields, and nothing else + --> $DIR/invalid_coerce_sized_impls.rs:25:1 + | +LL | / impl CoerceSized> for WrapperWithExtraField +LL | | where +LL | | T: CoerceUnsized, +LL | | U: CoerceSized, +LL | | {} //~^^^^ ERROR [E0378] + | |__^ + | + = note: extra field `1` of type `i32` is not allowed + +error[E0378]: implementing the `CoerceSized` trait requires multiple coercions + --> $DIR/invalid_coerce_sized_impls.rs:39:1 + | +LL | / impl CoerceSized> for MultiplePointers +LL | | where +LL | | T: Unsize, +LL | | {} //~^^^ ERROR [E0378] + | |__^ + | + = note: the trait `CoerceSized` may only be implemented for a coercion between structures with a single field being coerced + = note: currently, 2 fields need coercions: ptr1 (*const U to *const T), ptr2 (*const U to *const T) + +error[E0378]: the trait `CoerceSized` may only be implemented for a coercion between structures with a single field being coerced, none found + --> $DIR/invalid_coerce_sized_impls.rs:51:1 + | +LL | impl CoerceSized> for NothingToCoerce {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: aborting due to 3 previous errors + +For more information about this error, try `rustc --explain E0378`. From c29641e067d44c754fbc6a40e463a7e4d45ab31e Mon Sep 17 00:00:00 2001 From: Michael Hewson Date: Fri, 21 Sep 2018 02:16:29 -0400 Subject: [PATCH 09/19] fix docs on trait tests were failing because I didn't wrap code snippets like in backticks. fixed that now, so hopefully tests will pass on travis --- src/libcore/ops/unsize.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/libcore/ops/unsize.rs b/src/libcore/ops/unsize.rs index 4faace26b02f9..0112258134dd0 100644 --- a/src/libcore/ops/unsize.rs +++ b/src/libcore/ops/unsize.rs @@ -85,10 +85,10 @@ impl, U: ?Sized> CoerceUnsized<*const U> for *const T {} /// where `Self = dyn Trait` to the version where `Self = T`, the erased, sized type /// of the underlying object. /// -/// CoerceSized is implemented for: -/// - &[T] is CoerceSized<&[T; N]> for any N -/// - &Trait is CoerceSized<&T> for any T: Trait -/// - and similarly for &mut T, *const T, *mut T, Box, Rc, Arc +/// `CoerceSized` is implemented for: +/// - `&[T]` is `CoerceSized<&[T; N]>` for any `N` +/// - `&Trait` is `CoerceSized<&T>` for any `T: Trait` +/// - and similarly for `&mut T`, `*const T`, `*mut T`, `Box`, `Rc`, `Arc` #[unstable(feature = "coerce_sized", issue = "0")] #[cfg_attr(not(stage0), lang = "coerce_sized")] pub trait CoerceSized where T: CoerceUnsized { From f12c250e40650e6103161f986f02a84b7357bdc9 Mon Sep 17 00:00:00 2001 From: Michael Hewson Date: Wed, 3 Oct 2018 23:40:21 -0400 Subject: [PATCH 10/19] Replace CoerceSized trait with DispatchFromDyn MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Rename `CoerceSized` to `DispatchFromDyn`, and reverse the direction so that, for example, you write ``` impl, U> DispatchFromDyn<*const U> for *const T {} ``` instead of ``` impl, U> DispatchFromDyn<*const T> for *const U {} ``` this way the trait is really just a subset of `CoerceUnsized`. The checks in object_safety.rs are updated for the new trait, and some documentation and method names in there are updated for the new trait name — e.g. `receiver_is_coercible` is now called `receiver_is_dispatchable`. Since the trait now works in the opposite direction, some code had to updated here for that too. I did not update the error messages for invalid `CoerceSized` (now `DispatchFromDyn`) implementations, except to find/replace `CoerceSized` with `DispatchFromDyn`. Will ask for suggestions in the PR thread. --- src/liballoc/boxed.rs | 6 +- src/liballoc/lib.rs | 2 +- src/liballoc/rc.rs | 10 +-- src/liballoc/sync.rs | 10 +-- src/libcore/nonzero.rs | 4 +- src/libcore/ops/mod.rs | 4 +- src/libcore/ops/unsize.rs | 48 +++++++------- src/libcore/pin.rs | 7 +-- src/libcore/ptr.rs | 8 +-- src/librustc/middle/lang_items.rs | 2 +- src/librustc/traits/object_safety.rs | 62 ++++++++++--------- src/librustc_typeck/coherence/builtin.rs | 34 +++++----- src/librustc_typeck/diagnostics.rs | 51 +++++---------- ...itrary_self_types_pointers_and_wrappers.rs | 17 +++-- src/test/ui/invalid_coerce_sized_impls.stderr | 33 ---------- ....rs => invalid_dispatch_from_dyn_impls.rs} | 24 +++---- .../ui/invalid_dispatch_from_dyn_impls.stderr | 32 ++++++++++ 17 files changed, 164 insertions(+), 190 deletions(-) delete mode 100644 src/test/ui/invalid_coerce_sized_impls.stderr rename src/test/ui/{invalid_coerce_sized_impls.rs => invalid_dispatch_from_dyn_impls.rs} (57%) create mode 100644 src/test/ui/invalid_dispatch_from_dyn_impls.stderr diff --git a/src/liballoc/boxed.rs b/src/liballoc/boxed.rs index 8a4e646f278ec..74354f605e537 100644 --- a/src/liballoc/boxed.rs +++ b/src/liballoc/boxed.rs @@ -77,7 +77,7 @@ use core::iter::FusedIterator; use core::marker::{Unpin, Unsize}; use core::mem; use core::pin::Pin; -use core::ops::{CoerceUnsized, CoerceSized, Deref, DerefMut, Generator, GeneratorState}; +use core::ops::{CoerceUnsized, DispatchFromDyn, Deref, DerefMut, Generator, GeneratorState}; use core::ptr::{self, NonNull, Unique}; use core::task::{LocalWaker, Poll}; @@ -696,8 +696,8 @@ impl<'a, A, R> FnOnce for Box + Send + 'a> { #[unstable(feature = "coerce_unsized", issue = "27732")] impl, U: ?Sized> CoerceUnsized> for Box {} -#[unstable(feature = "coerce_sized", issue = "0")] -impl, U: ?Sized> CoerceSized> for Box {} +#[unstable(feature = "dispatch_from_dyn", issue = "0")] +impl, U: ?Sized> DispatchFromDyn> for Box {} #[stable(feature = "box_slice_clone", since = "1.3.0")] impl Clone for Box<[T]> { diff --git a/src/liballoc/lib.rs b/src/liballoc/lib.rs index 93c340a6dbd2d..ad6e594c884af 100644 --- a/src/liballoc/lib.rs +++ b/src/liballoc/lib.rs @@ -86,7 +86,7 @@ #![feature(box_syntax)] #![feature(cfg_target_has_atomic)] #![feature(coerce_unsized)] -#![feature(coerce_sized)] +#![feature(dispatch_from_dyn)] #![feature(core_intrinsics)] #![feature(custom_attribute)] #![feature(dropck_eyepatch)] diff --git a/src/liballoc/rc.rs b/src/liballoc/rc.rs index c1cc7ac976ded..be452ebb45a3c 100644 --- a/src/liballoc/rc.rs +++ b/src/liballoc/rc.rs @@ -255,7 +255,7 @@ use core::marker; use core::marker::{Unpin, Unsize, PhantomData}; use core::mem::{self, align_of_val, forget, size_of_val}; use core::ops::Deref; -use core::ops::{CoerceUnsized, CoerceSized}; +use core::ops::{CoerceUnsized, DispatchFromDyn}; use core::pin::Pin; use core::ptr::{self, NonNull}; use core::convert::From; @@ -297,8 +297,8 @@ impl !marker::Sync for Rc {} #[unstable(feature = "coerce_unsized", issue = "27732")] impl, U: ?Sized> CoerceUnsized> for Rc {} -#[unstable(feature = "coerce_sized", issue = "0")] -impl, U: ?Sized> CoerceSized> for Rc {} +#[unstable(feature = "dispatch_from_dyn", issue = "0")] +impl, U: ?Sized> DispatchFromDyn> for Rc {} impl Rc { /// Constructs a new `Rc`. @@ -1179,8 +1179,8 @@ impl !marker::Sync for Weak {} #[unstable(feature = "coerce_unsized", issue = "27732")] impl, U: ?Sized> CoerceUnsized> for Weak {} -#[unstable(feature = "coerce_sized", issue = "0")] -impl, U: ?Sized> CoerceSized> for Weak {} +#[unstable(feature = "dispatch_from_dyn", issue = "0")] +impl, U: ?Sized> DispatchFromDyn> for Weak {} impl Weak { /// Constructs a new `Weak`, without allocating any memory. diff --git a/src/liballoc/sync.rs b/src/liballoc/sync.rs index 358e18bbe1b90..d388f76d8e84c 100644 --- a/src/liballoc/sync.rs +++ b/src/liballoc/sync.rs @@ -25,7 +25,7 @@ use core::cmp::Ordering; use core::intrinsics::abort; use core::mem::{self, align_of_val, size_of_val}; use core::ops::Deref; -use core::ops::{CoerceUnsized, CoerceSized}; +use core::ops::{CoerceUnsized, DispatchFromDyn}; use core::pin::Pin; use core::ptr::{self, NonNull}; use core::marker::{Unpin, Unsize, PhantomData}; @@ -214,8 +214,8 @@ unsafe impl Sync for Arc {} #[unstable(feature = "coerce_unsized", issue = "27732")] impl, U: ?Sized> CoerceUnsized> for Arc {} -#[unstable(feature = "coerce_sized", issue = "0")] -impl, U: ?Sized> CoerceSized> for Arc {} +#[unstable(feature = "dispatch_from_dyn", issue = "0")] +impl, U: ?Sized> DispatchFromDyn> for Arc {} /// `Weak` is a version of [`Arc`] that holds a non-owning reference to the /// managed value. The value is accessed by calling [`upgrade`] on the `Weak` @@ -257,8 +257,8 @@ unsafe impl Sync for Weak {} #[unstable(feature = "coerce_unsized", issue = "27732")] impl, U: ?Sized> CoerceUnsized> for Weak {} -#[unstable(feature = "coerce_sized", issue = "0")] -impl, U: ?Sized> CoerceSized> for Weak {} +#[unstable(feature = "dispatch_from_dyn", issue = "0")] +impl, U: ?Sized> DispatchFromDyn> for Weak {} #[stable(feature = "arc_weak", since = "1.4.0")] impl fmt::Debug for Weak { diff --git a/src/libcore/nonzero.rs b/src/libcore/nonzero.rs index 2bc6c36171ce3..436cd1fc05728 100644 --- a/src/libcore/nonzero.rs +++ b/src/libcore/nonzero.rs @@ -10,7 +10,7 @@ //! Exposes the NonZero lang item which provides optimization hints. -use ops::{CoerceUnsized, CoerceSized}; +use ops::{CoerceUnsized, DispatchFromDyn}; /// A wrapper type for raw pointers and integers that will never be /// NULL or 0 that might allow certain optimizations. @@ -21,4 +21,4 @@ pub(crate) struct NonZero(pub(crate) T); impl, U> CoerceUnsized> for NonZero {} -impl, U: CoerceSized> CoerceSized> for NonZero {} +impl, U> DispatchFromDyn> for NonZero {} diff --git a/src/libcore/ops/mod.rs b/src/libcore/ops/mod.rs index bf9775e2ae813..edfa6df11aceb 100644 --- a/src/libcore/ops/mod.rs +++ b/src/libcore/ops/mod.rs @@ -202,5 +202,5 @@ pub use self::generator::{Generator, GeneratorState}; #[unstable(feature = "coerce_unsized", issue = "27732")] pub use self::unsize::CoerceUnsized; -#[unstable(feature = "coerce_sized", issue = "0")] -pub use self::unsize::CoerceSized; +#[unstable(feature = "dispatch_from_dyn", issue = "0")] +pub use self::unsize::DispatchFromDyn; diff --git a/src/libcore/ops/unsize.rs b/src/libcore/ops/unsize.rs index 0112258134dd0..822c80fadde79 100644 --- a/src/libcore/ops/unsize.rs +++ b/src/libcore/ops/unsize.rs @@ -79,32 +79,32 @@ impl, U: ?Sized> CoerceUnsized<*const U> for *mut T {} impl, U: ?Sized> CoerceUnsized<*const U> for *const T {} -/// Pointers to unsized types that can be coerced to a pointer to a sized type, -/// as long as pointee is actually a value of that sized type. This is used for -/// object safety, to check that a method's receiver type can be coerced from the version -/// where `Self = dyn Trait` to the version where `Self = T`, the erased, sized type -/// of the underlying object. +/// This is used for object safety, to check that a method's receiver type can be dispatched on. /// -/// `CoerceSized` is implemented for: -/// - `&[T]` is `CoerceSized<&[T; N]>` for any `N` -/// - `&Trait` is `CoerceSized<&T>` for any `T: Trait` -/// - and similarly for `&mut T`, `*const T`, `*mut T`, `Box`, `Rc`, `Arc` -#[unstable(feature = "coerce_sized", issue = "0")] -#[cfg_attr(not(stage0), lang = "coerce_sized")] -pub trait CoerceSized where T: CoerceUnsized { +/// example impl: +/// +/// ``` +/// impl DispatchFromDyn> for Rc +/// where +/// T: Unsize, +/// {} +/// ``` +#[unstable(feature = "dispatch_from_dyn", issue = "0")] +#[cfg_attr(not(stage0), lang = "dispatch_from_dyn")] +pub trait DispatchFromDyn { // Empty. } -// &U -> &T -#[unstable(feature = "coerce_sized", issue = "0")] -impl<'a, T: ?Sized+Unsize, U: ?Sized> CoerceSized<&'a T> for &'a U {} -// &mut U -> &mut T -#[unstable(feature = "coerce_sized", issue = "0")] -impl<'a, T: ?Sized+Unsize, U: ?Sized> CoerceSized<&'a mut T> for &'a mut U {} -// *const U -> *const T -#[unstable(feature = "coerce_sized", issue = "0")] -impl, U: ?Sized> CoerceSized<*const T> for *const U {} -// *mut U -> *mut T -#[unstable(feature = "coerce_sized", issue = "0")] -impl, U: ?Sized> CoerceSized<*mut T> for *mut U {} +// &T -> &U +#[unstable(feature = "dispatch_from_dyn", issue = "0")] +impl<'a, T: ?Sized+Unsize, U: ?Sized> DispatchFromDyn<&'a U> for &'a T {} +// &mut T -> &mut U +#[unstable(feature = "dispatch_from_dyn", issue = "0")] +impl<'a, T: ?Sized+Unsize, U: ?Sized> DispatchFromDyn<&'a mut U> for &'a mut T {} +// *const T -> *const U +#[unstable(feature = "dispatch_from_dyn", issue = "0")] +impl, U: ?Sized> DispatchFromDyn<*const U> for *const T {} +// *mut T -> *mut U +#[unstable(feature = "dispatch_from_dyn", issue = "0")] +impl, U: ?Sized> DispatchFromDyn<*mut U> for *mut T {} diff --git a/src/libcore/pin.rs b/src/libcore/pin.rs index 87ca193aeee5c..68de82d294529 100644 --- a/src/libcore/pin.rs +++ b/src/libcore/pin.rs @@ -91,7 +91,7 @@ use fmt; use marker::Sized; -use ops::{Deref, DerefMut, CoerceUnsized, CoerceSized}; +use ops::{Deref, DerefMut, CoerceUnsized, DispatchFromDyn}; #[doc(inline)] pub use marker::Unpin; @@ -325,10 +325,9 @@ where {} #[unstable(feature = "pin", issue = "49150")] -impl<'a, P, U> CoerceSized> for Pin +impl<'a, P, U> DispatchFromDyn> for Pin

where - P: CoerceUnsized, - U: CoerceSized

, + P: DispatchFromDyn, {} #[unstable(feature = "pin", issue = "49150")] diff --git a/src/libcore/ptr.rs b/src/libcore/ptr.rs index 24ef028b49d5d..62ccf6c865cd9 100644 --- a/src/libcore/ptr.rs +++ b/src/libcore/ptr.rs @@ -75,7 +75,7 @@ use convert::From; use intrinsics; -use ops::{CoerceUnsized, CoerceSized}; +use ops::{CoerceUnsized, DispatchFromDyn}; use fmt; use hash; use marker::{PhantomData, Unsize}; @@ -2796,7 +2796,7 @@ impl Copy for Unique { } impl CoerceUnsized> for Unique where T: Unsize { } #[unstable(feature = "ptr_internals", issue = "0")] -impl CoerceSized> for Unique where T: Unsize { } +impl DispatchFromDyn> for Unique where T: Unsize { } #[unstable(feature = "ptr_internals", issue = "0")] impl fmt::Pointer for Unique { @@ -2954,8 +2954,8 @@ impl Copy for NonNull { } #[unstable(feature = "coerce_unsized", issue = "27732")] impl CoerceUnsized> for NonNull where T: Unsize { } -#[unstable(feature = "coerce_sized", issue = "0")] -impl CoerceSized> for NonNull where T: Unsize { } +#[unstable(feature = "dispatch_from_dyn", issue = "0")] +impl DispatchFromDyn> for NonNull where T: Unsize { } #[stable(feature = "nonnull", since = "1.25.0")] impl fmt::Debug for NonNull { diff --git a/src/librustc/middle/lang_items.rs b/src/librustc/middle/lang_items.rs index 67864f67bfc3a..cce8081daf28e 100644 --- a/src/librustc/middle/lang_items.rs +++ b/src/librustc/middle/lang_items.rs @@ -271,7 +271,7 @@ language_item_table! { DropTraitLangItem, "drop", drop_trait, Target::Trait; CoerceUnsizedTraitLangItem, "coerce_unsized", coerce_unsized_trait, Target::Trait; - CoerceSizedTraitLangItem, "coerce_sized", coerce_sized_trait, Target::Trait; + DispatchFromDynTraitLangItem,"dispatch_from_dyn", dispatch_from_dyn_trait, Target::Trait; AddTraitLangItem, "add", add_trait, Target::Trait; SubTraitLangItem, "sub", sub_trait, Target::Trait; diff --git a/src/librustc/traits/object_safety.rs b/src/librustc/traits/object_safety.rs index 470a5c6bc9ebd..5e7a3043ae73e 100644 --- a/src/librustc/traits/object_safety.rs +++ b/src/librustc/traits/object_safety.rs @@ -319,13 +319,12 @@ impl<'a, 'tcx> TyCtxt<'a, 'tcx, 'tcx> { &sig.map_bound(|sig| sig.inputs()[0]), ); - // until `unsized_locals` is fully implemented, `self: Self` can't be coerced from - // `Self=dyn Trait` to `Self=T`. However, this is already considered object-safe. We allow - // it as a special case here. - // FIXME(mikeyhew) get rid of this `if` statement once `receiver_is_coercible` allows + // until `unsized_locals` is fully implemented, `self: Self` can't be dispatched on. + // However, this is already considered object-safe. We allow it as a special case here. + // FIXME(mikeyhew) get rid of this `if` statement once `receiver_is_dispatchable` allows // `Receiver: Unsize dyn Trait]>` if receiver_ty != self.mk_self_type() { - if !self.receiver_is_coercible(method, receiver_ty) { + if !self.receiver_is_dispatchable(method, receiver_ty) { return Some(MethodViolationCode::UncoercibleReceiver); } } @@ -333,27 +332,29 @@ impl<'a, 'tcx> TyCtxt<'a, 'tcx, 'tcx> { None } - /// checks the method's receiver (the `self` argument) can be coerced from - /// a fat pointer, including the trait object vtable, to a thin pointer. - /// e.g. from `Rc` to `Rc`, where `T` is the erased type of the underlying object. - /// More formally: + /// checks the method's receiver (the `self` argument) can be dispatched on when `Self` is a + /// trait object. We require that `DispatchableFromDyn` be implemented for the receiver type + /// in the following way: /// - let `Receiver` be the type of the `self` argument, i.e `Self`, `&Self`, `Rc` /// - require the following bound: /// forall(T: Trait) { - /// Receiver[Self => dyn Trait]: CoerceSized T]> + /// Receiver[Self => T]: DispatchFromDyn dyn Trait]> /// } /// where `Foo[X => Y]` means "the same type as `Foo`, but with `X` replaced with `Y`" /// (substitution notation). /// /// some examples of receiver types and their required obligation - /// - `&'a mut self` requires `&'a mut dyn Trait: CoerceSized<&'a mut T>` - /// - `self: Rc` requires `Rc: CoerceSized>` + /// - `&'a mut self` requires `&'a mut T: DispatchFromDyn<&'a mut dyn Trait>` + /// - `self: Rc` requires `Rc: DispatchFromDyn>` + /// - `self: Pin>` requires `Pin>: DispatchFromDyn>>` /// - /// The only case where the receiver is not coercible, but is still a valid receiver + /// The only case where the receiver is not dispatchable, but is still a valid receiver /// type (just not object-safe), is when there is more than one level of pointer indirection. /// e.g. `self: &&Self`, `self: &Rc`, `self: Box>`. In these cases, there - /// is no way, or at least no inexpensive way, to coerce the receiver, because the object that - /// needs to be coerced is behind a pointer. + /// is no way, or at least no inexpensive way, to coerce the receiver from the version where + /// `Self = dyn Trait` to the version where `Self = T`, where `T` is the unknown erased type + /// contained by the trait object, because the object that needs to be coerced is behind + /// a pointer. /// /// In practice, there are issues with the above bound: `where` clauses that apply to `Self` /// would have to apply to `T`, trait object types have a lot of parameters that need to @@ -364,37 +365,38 @@ impl<'a, 'tcx> TyCtxt<'a, 'tcx, 'tcx> { /// /// forall (U: ?Sized) { /// if (Self: Unsize) { - /// Receiver[Self => U]: CoerceSized + /// Receiver: DispatchFromDyn U]> /// } /// } /// - /// for `self: &'a mut Self`, this means `&'a mut U: CoerceSized<&'a mut Self>` - /// for `self: Rc`, this means `Rc: CoerceSized>` + /// for `self: &'a mut Self`, this means `&'a mut Self: DispatchFromDyn<&'a mut U>` + /// for `self: Rc`, this means `Rc: DispatchFromDyn>` + /// for `self: Pin>, this means `Pin>: DispatchFromDyn>>` // // FIXME(mikeyhew) when unsized receivers are implemented as part of unsized rvalues, add this // fallback query: `Receiver: Unsize U]>` to support receivers like // `self: Wrapper`. #[allow(dead_code)] - fn receiver_is_coercible( + fn receiver_is_dispatchable( self, method: &ty::AssociatedItem, receiver_ty: Ty<'tcx>, ) -> bool { - debug!("receiver_is_coercible: method = {:?}, receiver_ty = {:?}", method, receiver_ty); + debug!("receiver_is_dispatchable: method = {:?}, receiver_ty = {:?}", method, receiver_ty); let traits = (self.lang_items().unsize_trait(), - self.lang_items().coerce_sized_trait()); - let (unsize_did, coerce_sized_did) = if let (Some(u), Some(cu)) = traits { + self.lang_items().dispatch_from_dyn_trait()); + let (unsize_did, dispatch_from_dyn_did) = if let (Some(u), Some(cu)) = traits { (u, cu) } else { - debug!("receiver_is_coercible: Missing Unsize or CoerceSized traits"); + debug!("receiver_is_dispatchable: Missing Unsize or DispatchFromDyn traits"); return false; }; // use a bogus type parameter to mimick a forall(U) query using u32::MAX for now. // FIXME(mikeyhew) this is a total hack, and we should replace it when real forall queries // are implemented - let target_self_ty: Ty<'tcx> = self.mk_ty_param( + let unsized_self_ty: Ty<'tcx> = self.mk_ty_param( ::std::u32::MAX, Name::intern("RustaceansAreAwesome").as_interned_str(), ); @@ -405,7 +407,7 @@ impl<'a, 'tcx> TyCtxt<'a, 'tcx, 'tcx> { let predicate = ty::TraitRef { def_id: unsize_did, - substs: self.mk_substs_trait(self.mk_self_type(), &[target_self_ty.into()]), + substs: self.mk_substs_trait(self.mk_self_type(), &[unsized_self_ty.into()]), }.to_predicate(); let caller_bounds: Vec> = param_env.caller_bounds.iter().cloned() @@ -419,7 +421,7 @@ impl<'a, 'tcx> TyCtxt<'a, 'tcx, 'tcx> { let receiver_substs = Substs::for_item(self, method.def_id, |param, _| { if param.index == 0 { - target_self_ty.into() + unsized_self_ty.into() } else { self.mk_param_from_def(param) } @@ -427,11 +429,11 @@ impl<'a, 'tcx> TyCtxt<'a, 'tcx, 'tcx> { // the type `Receiver[Self => U]` in the query let unsized_receiver_ty = receiver_ty.subst(self, receiver_substs); - // Receiver[Self => U]: CoerceSized + // Receiver: DispatchFromDyn U]> let obligation = { let predicate = ty::TraitRef { - def_id: coerce_sized_did, - substs: self.mk_substs_trait(unsized_receiver_ty, &[receiver_ty.into()]), + def_id: dispatch_from_dyn_did, + substs: self.mk_substs_trait(receiver_ty, &[unsized_receiver_ty.into()]), }.to_predicate(); Obligation::new( @@ -442,7 +444,7 @@ impl<'a, 'tcx> TyCtxt<'a, 'tcx, 'tcx> { }; self.infer_ctxt().enter(|ref infcx| { - // the receiver is coercible iff the obligation holds + // the receiver is dispatchable iff the obligation holds infcx.predicate_must_hold(&obligation) }) } diff --git a/src/librustc_typeck/coherence/builtin.rs b/src/librustc_typeck/coherence/builtin.rs index eba73b1da9aab..a01718bc12870 100644 --- a/src/librustc_typeck/coherence/builtin.rs +++ b/src/librustc_typeck/coherence/builtin.rs @@ -32,7 +32,8 @@ pub fn check_trait<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, trait_def_id: DefId) { .check(tcx.lang_items().drop_trait(), visit_implementation_of_drop) .check(tcx.lang_items().copy_trait(), visit_implementation_of_copy) .check(tcx.lang_items().coerce_unsized_trait(), visit_implementation_of_coerce_unsized) - .check(tcx.lang_items().coerce_sized_trait(), visit_implementation_of_coerce_sized); + .check(tcx.lang_items().dispatch_from_dyn_trait(), + visit_implementation_of_dispatch_from_dyn); } struct Checker<'a, 'tcx: 'a> { @@ -162,11 +163,14 @@ fn visit_implementation_of_coerce_unsized<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, } } -fn visit_implementation_of_coerce_sized<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, impl_did: DefId) { - debug!("visit_implementation_of_coerce_sized: impl_did={:?}", +fn visit_implementation_of_dispatch_from_dyn<'a, 'tcx>( + tcx: TyCtxt<'a, 'tcx, 'tcx>, + impl_did: DefId, +) { + debug!("visit_implementation_of_dispatch_from_dyn: impl_did={:?}", impl_did); if impl_did.is_local() { - let coerce_sized_trait = tcx.lang_items().coerce_sized_trait().unwrap(); + let dispatch_from_dyn_trait = tcx.lang_items().dispatch_from_dyn_trait().unwrap(); let impl_node_id = tcx.hir.as_local_node_id(impl_did).unwrap(); let span = tcx.hir.span(impl_node_id); @@ -175,12 +179,12 @@ fn visit_implementation_of_coerce_sized<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, i assert!(!source.has_escaping_regions()); let target = { let trait_ref = tcx.impl_trait_ref(impl_did).unwrap(); - assert_eq!(trait_ref.def_id, coerce_sized_trait); + assert_eq!(trait_ref.def_id, dispatch_from_dyn_trait); trait_ref.substs.type_at(1) }; - debug!("visit_implementation_of_coerce_sized: {:?} -> {:?}", + debug!("visit_implementation_of_dispatch_from_dyn: {:?} -> {:?}", source, target); @@ -209,7 +213,7 @@ fn visit_implementation_of_coerce_sized<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, i create_err( &format!( - "the trait `CoerceSized` may only be implemented \ + "the trait `DispatchFromDyn` may only be implemented \ for a coercion between structures with the same \ definition; expected {}, found {}", source_path, target_path, @@ -232,9 +236,9 @@ fn visit_implementation_of_coerce_sized<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, i if let Ok(ok) = infcx.at(&cause, param_env).eq(ty_a, ty_b) { if ok.obligations.is_empty() { create_err( - "the trait `CoerceSized` may only be implemented for structs \ - containing the field being coerced, `PhantomData` fields, \ - and nothing else" + "the trait `DispatchFromDyn` may only be implemented \ + for structs containing the field being coerced, \ + `PhantomData` fields, and nothing else" ).note( &format!( "extra field `{}` of type `{}` is not allowed", @@ -251,15 +255,15 @@ fn visit_implementation_of_coerce_sized<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, i if coerced_fields.is_empty() { create_err( - "the trait `CoerceSized` may only be implemented \ + "the trait `DispatchFromDyn` may only be implemented \ for a coercion between structures with a single field \ being coerced, none found" ).emit(); } else if coerced_fields.len() > 1 { create_err( - "implementing the `CoerceSized` trait requires multiple coercions", + "implementing the `DispatchFromDyn` trait requires multiple coercions", ).note( - "the trait `CoerceSized` may only be implemented \ + "the trait `DispatchFromDyn` may only be implemented \ for a coercion between structures with a single field \ being coerced" ).note( @@ -284,7 +288,7 @@ fn visit_implementation_of_coerce_sized<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, i let predicate = tcx.predicate_for_trait_def( param_env, cause.clone(), - coerce_sized_trait, + dispatch_from_dyn_trait, 0, field.ty(tcx, substs_a), &[field.ty(tcx, substs_b).into()] @@ -311,7 +315,7 @@ fn visit_implementation_of_coerce_sized<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, i } _ => { create_err( - "the trait `CoerceSsized` may only be implemented \ + "the trait `DispatchFromDyn` may only be implemented \ for a coercion between structures" ).emit(); } diff --git a/src/librustc_typeck/diagnostics.rs b/src/librustc_typeck/diagnostics.rs index 2cfca345c27f8..e53536fc55a2c 100644 --- a/src/librustc_typeck/diagnostics.rs +++ b/src/librustc_typeck/diagnostics.rs @@ -3085,75 +3085,58 @@ struct. "##, E0378: r##" -The `CoerceSized` trait currently can only be implemented for builtin pointer -types and structs that are newtype wrappers around them — that is, the struct -must have only one field (except for`PhantomData`), and that field must itself -implement `CoerceSized`. +The `DispatchFromDyn` trait currently can only be implemented for +builtin pointer types and structs that are newtype wrappers around them +— that is, the struct must have only one field (except for`PhantomData`), +and that field must itself implement `DispatchFromDyn`. Examples: ``` -#![feature(coerce_sized, unsize)] +#![feature(dispatch_from_dyn, unsize)] use std::{ marker::Unsize, - ops::CoerceSized, + ops::DispatchFromDyn, }; struct Ptr(*const T); -impl CoerceUnsized> for Ptr -where - T: Unsize, -{} - -impl CoerceSized> for Ptr +impl DispatchFromDyn> for Ptr where T: Unsize, {} ``` ``` -#![feature(coerce_unsized, coerce_sized)] -use std::ops::{CoerceUnsized, CoerceSized}; +#![feature(dispatch_from_dyn)] +use std::ops::DispatchFromDyn; struct Wrapper { ptr: T, _phantom: PhantomData<()>, } -impl CoerceUnsized> for Wrapper -where - T: CoerceUnsized, -{} - -impl CoerceSized> for Wrapper +impl DispatchFromDyn> for Wrapper where - T: CoerceUnsized, - U: CoerceSized, + T: DispatchFromDyn, {} ``` -Example of illegal CoerceSized implementation +Example of illegal `DispatchFromDyn` implementation (illegal because of extra field) ```compile-fail,E0378 -#![feature(coerce_unsized, coerce_sized)] -use std::ops::{CoerceUnsized, CoerceSized}; +#![feature(dispatch_from_dyn)] +use std::ops::DispatchFromDyn; -struct WrapperWithExtraField { +struct WrapperExtraField { ptr: T, extra_stuff: i32, } -impl CoerceUnsized> for WrapperWithExtraField -where - T: CoerceUnsized, -{} - -impl CoerceSized> for WrapperWithExtraField +impl DispatchFromDyn> for WrapperExtraField where - T: CoerceUnsized, - U: CoerceSized, + T: DispatchFromDyn, {} ``` "##, diff --git a/src/test/run-pass/arbitrary_self_types_pointers_and_wrappers.rs b/src/test/run-pass/arbitrary_self_types_pointers_and_wrappers.rs index 3abe806c9a9d3..e1663563cec0a 100644 --- a/src/test/run-pass/arbitrary_self_types_pointers_and_wrappers.rs +++ b/src/test/run-pass/arbitrary_self_types_pointers_and_wrappers.rs @@ -7,13 +7,12 @@ // , at your // option. This file may not be copied, modified, or distributed // except according to those terms. -#![feature(arbitrary_self_types, unsize, coerce_unsized, coerce_sized)] +#![feature(arbitrary_self_types, unsize, coerce_unsized, dispatch_from_dyn)] #![feature(rustc_attrs)] use std::{ - ops::{Deref, CoerceUnsized, CoerceSized}, + ops::{Deref, CoerceUnsized, DispatchFromDyn}, marker::Unsize, - fmt::Debug, }; struct Ptr(Box); @@ -27,7 +26,7 @@ impl Deref for Ptr { } impl + ?Sized, U: ?Sized> CoerceUnsized> for Ptr {} -impl + ?Sized, U: ?Sized> CoerceSized> for Ptr {} +impl + ?Sized, U: ?Sized> DispatchFromDyn> for Ptr {} struct Wrapper(T); @@ -40,12 +39,13 @@ impl Deref for Wrapper { } impl, U> CoerceUnsized> for Wrapper {} -impl, U: CoerceSized> CoerceSized> for Wrapper {} +impl, U> DispatchFromDyn> for Wrapper {} trait Trait { - // This method can't be called on trait objects, since the receiver would be unsized, - // but should not cause an object safety error + // This method isn't object-safe yet. Unsized by-value `self` is object-safe (but not callable + // without unsized_locals), but wrappers arond `Self` currently are not. + // FIXME (mikeyhew) uncomment this when unsized rvalues object-safety is implemented // fn wrapper(self: Wrapper) -> i32; fn ptr_wrapper(self: Ptr>) -> i32; fn wrapper_ptr(self: Wrapper>) -> i32; @@ -53,9 +53,6 @@ trait Trait { } impl Trait for i32 { - // fn wrapper(self: Wrapper) -> i32 { - // *self - // } fn ptr_wrapper(self: Ptr>) -> i32 { **self } diff --git a/src/test/ui/invalid_coerce_sized_impls.stderr b/src/test/ui/invalid_coerce_sized_impls.stderr deleted file mode 100644 index 70fb464d426ed..0000000000000 --- a/src/test/ui/invalid_coerce_sized_impls.stderr +++ /dev/null @@ -1,33 +0,0 @@ -error[E0378]: the trait `CoerceSized` may only be implemented for structs containing the field being coerced, `PhantomData` fields, and nothing else - --> $DIR/invalid_coerce_sized_impls.rs:25:1 - | -LL | / impl CoerceSized> for WrapperWithExtraField -LL | | where -LL | | T: CoerceUnsized, -LL | | U: CoerceSized, -LL | | {} //~^^^^ ERROR [E0378] - | |__^ - | - = note: extra field `1` of type `i32` is not allowed - -error[E0378]: implementing the `CoerceSized` trait requires multiple coercions - --> $DIR/invalid_coerce_sized_impls.rs:39:1 - | -LL | / impl CoerceSized> for MultiplePointers -LL | | where -LL | | T: Unsize, -LL | | {} //~^^^ ERROR [E0378] - | |__^ - | - = note: the trait `CoerceSized` may only be implemented for a coercion between structures with a single field being coerced - = note: currently, 2 fields need coercions: ptr1 (*const U to *const T), ptr2 (*const U to *const T) - -error[E0378]: the trait `CoerceSized` may only be implemented for a coercion between structures with a single field being coerced, none found - --> $DIR/invalid_coerce_sized_impls.rs:51:1 - | -LL | impl CoerceSized> for NothingToCoerce {} - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - -error: aborting due to 3 previous errors - -For more information about this error, try `rustc --explain E0378`. diff --git a/src/test/ui/invalid_coerce_sized_impls.rs b/src/test/ui/invalid_dispatch_from_dyn_impls.rs similarity index 57% rename from src/test/ui/invalid_coerce_sized_impls.rs rename to src/test/ui/invalid_dispatch_from_dyn_impls.rs index 27d65b880fa15..101e1eb6e3071 100644 --- a/src/test/ui/invalid_coerce_sized_impls.rs +++ b/src/test/ui/invalid_dispatch_from_dyn_impls.rs @@ -8,25 +8,19 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -#![feature(unsize, coerce_sized, coerce_unsized)] +#![feature(unsize, dispatch_from_dyn)] use std::{ - ops::{CoerceSized, CoerceUnsized}, + ops::DispatchFromDyn, marker::{Unsize, PhantomData}, }; struct WrapperWithExtraField(T, i32); -impl CoerceUnsized> for WrapperWithExtraField +impl DispatchFromDyn> for WrapperWithExtraField where - T: CoerceUnsized, -{} - -impl CoerceSized> for WrapperWithExtraField -where - T: CoerceUnsized, - U: CoerceSized, -{} //~^^^^ ERROR [E0378] + T: DispatchFromDyn, +{} //~^^^ ERROR [E0378] struct MultiplePointers{ @@ -34,9 +28,7 @@ struct MultiplePointers{ ptr2: *const T, } -// No CoerceUnsized impl - -impl CoerceSized> for MultiplePointers +impl DispatchFromDyn> for MultiplePointers where T: Unsize, {} //~^^^ ERROR [E0378] @@ -46,9 +38,7 @@ struct NothingToCoerce { data: PhantomData, } -// No CoerceUnsized impl - -impl CoerceSized> for NothingToCoerce {} +impl DispatchFromDyn> for NothingToCoerce {} //~^ ERROR [E0378] fn main() {} diff --git a/src/test/ui/invalid_dispatch_from_dyn_impls.stderr b/src/test/ui/invalid_dispatch_from_dyn_impls.stderr new file mode 100644 index 0000000000000..5e394f2fb9152 --- /dev/null +++ b/src/test/ui/invalid_dispatch_from_dyn_impls.stderr @@ -0,0 +1,32 @@ +error[E0378]: the trait `DispatchFromDyn` may only be implemented for structs containing the field being coerced, `PhantomData` fields, and nothing else + --> $DIR/invalid_dispatch_from_dyn_impls.rs:20:1 + | +LL | / impl DispatchFromDyn> for WrapperWithExtraField +LL | | where +LL | | T: DispatchFromDyn, +LL | | {} //~^^^ ERROR [E0378] + | |__^ + | + = note: extra field `1` of type `i32` is not allowed + +error[E0378]: implementing the `DispatchFromDyn` trait requires multiple coercions + --> $DIR/invalid_dispatch_from_dyn_impls.rs:31:1 + | +LL | / impl DispatchFromDyn> for MultiplePointers +LL | | where +LL | | T: Unsize, +LL | | {} //~^^^ ERROR [E0378] + | |__^ + | + = note: the trait `DispatchFromDyn` may only be implemented for a coercion between structures with a single field being coerced + = note: currently, 2 fields need coercions: ptr1 (*const T to *const U), ptr2 (*const T to *const U) + +error[E0378]: the trait `DispatchFromDyn` may only be implemented for a coercion between structures with a single field being coerced, none found + --> $DIR/invalid_dispatch_from_dyn_impls.rs:41:1 + | +LL | impl DispatchFromDyn> for NothingToCoerce {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: aborting due to 3 previous errors + +For more information about this error, try `rustc --explain E0378`. From 48d7f8db3a64d8eb9bfb755e959d11fed7fdfbcc Mon Sep 17 00:00:00 2001 From: Michael Hewson Date: Fri, 5 Oct 2018 02:09:06 -0400 Subject: [PATCH 11/19] update DispatchFromDyn doctest --- src/libcore/ops/unsize.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/libcore/ops/unsize.rs b/src/libcore/ops/unsize.rs index 822c80fadde79..4d9a40a1b9089 100644 --- a/src/libcore/ops/unsize.rs +++ b/src/libcore/ops/unsize.rs @@ -84,6 +84,9 @@ impl, U: ?Sized> CoerceUnsized<*const U> for *const T {} /// example impl: /// /// ``` +/// # #![feature(dispatch_from_dyn, unsize)] +/// # use std::{ops::DispatchFromDyn, marker::Unsize}; +/// # struct Rc(::std::rc::Rc); /// impl DispatchFromDyn> for Rc /// where /// T: Unsize, From 3c56d8d0c21e15be8ea41835c647a0caecc0d0da Mon Sep 17 00:00:00 2001 From: Michael Hewson Date: Fri, 5 Oct 2018 16:35:42 -0400 Subject: [PATCH 12/19] fix error-index test --- src/librustc_typeck/diagnostics.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/librustc_typeck/diagnostics.rs b/src/librustc_typeck/diagnostics.rs index e53536fc55a2c..c81aea2465b7b 100644 --- a/src/librustc_typeck/diagnostics.rs +++ b/src/librustc_typeck/diagnostics.rs @@ -3109,7 +3109,10 @@ where ``` #![feature(dispatch_from_dyn)] -use std::ops::DispatchFromDyn; +use std::{ + ops::DispatchFromDyn, + marker::PhantomData, +}; struct Wrapper { ptr: T, From 74ef46cfa2c7d8befbd82faf973268957d5b718d Mon Sep 17 00:00:00 2001 From: Michael Hewson Date: Mon, 8 Oct 2018 20:37:58 -0400 Subject: [PATCH 13/19] Replace UncoeribleReceiver error message with UndispatchableReceiver --- src/librustc/traits/object_safety.rs | 10 +++++----- .../ui/arbitrary-self-types-not-object-safe.stderr | 4 ++-- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/librustc/traits/object_safety.rs b/src/librustc/traits/object_safety.rs index 5e7a3043ae73e..cd7e2774371fe 100644 --- a/src/librustc/traits/object_safety.rs +++ b/src/librustc/traits/object_safety.rs @@ -64,8 +64,8 @@ impl ObjectSafetyViolation { format!("method `{}` references the `Self` type in where clauses", name).into(), ObjectSafetyViolation::Method(name, MethodViolationCode::Generic) => format!("method `{}` has generic type parameters", name).into(), - ObjectSafetyViolation::Method(name, MethodViolationCode::UncoercibleReceiver) => - format!("method `{}` has an uncoercible receiver type", name).into(), + ObjectSafetyViolation::Method(name, MethodViolationCode::UndispatchableReceiver) => + format!("method `{}`'s receiver cannot be dispatched on", name).into(), ObjectSafetyViolation::AssociatedConst(name) => format!("the trait cannot contain associated consts like `{}`", name).into(), } @@ -87,8 +87,8 @@ pub enum MethodViolationCode { /// e.g., `fn foo()` Generic, - /// the self argument can't be coerced from Self=dyn Trait to Self=T where T: Trait - UncoercibleReceiver, + /// the method's receiver (`self` argument) can't be dispatched on + UndispatchableReceiver, } impl<'a, 'tcx> TyCtxt<'a, 'tcx, 'tcx> { @@ -325,7 +325,7 @@ impl<'a, 'tcx> TyCtxt<'a, 'tcx, 'tcx> { // `Receiver: Unsize dyn Trait]>` if receiver_ty != self.mk_self_type() { if !self.receiver_is_dispatchable(method, receiver_ty) { - return Some(MethodViolationCode::UncoercibleReceiver); + return Some(MethodViolationCode::UndispatchableReceiver); } } diff --git a/src/test/ui/arbitrary-self-types-not-object-safe.stderr b/src/test/ui/arbitrary-self-types-not-object-safe.stderr index 715fc86517bee..77ca118471db5 100644 --- a/src/test/ui/arbitrary-self-types-not-object-safe.stderr +++ b/src/test/ui/arbitrary-self-types-not-object-safe.stderr @@ -4,7 +4,7 @@ error[E0038]: the trait `Foo` cannot be made into an object LL | let x = Rc::new(5usize) as Rc; | ^^^^^^^ the trait `Foo` cannot be made into an object | - = note: method `foo` has an uncoercible receiver type + = note: method `foo`'s receiver cannot be dispatched on error[E0038]: the trait `Foo` cannot be made into an object --> $DIR/arbitrary-self-types-not-object-safe.rs:40:13 @@ -12,7 +12,7 @@ error[E0038]: the trait `Foo` cannot be made into an object LL | let x = Rc::new(5usize) as Rc; | ^^^^^^^^^^^^^^^ the trait `Foo` cannot be made into an object | - = note: method `foo` has an uncoercible receiver type + = note: method `foo`'s receiver cannot be dispatched on = note: required because of the requirements on the impl of `std::ops::CoerceUnsized>` for `std::rc::Rc` error: aborting due to 2 previous errors From 6f2a161b1bbe6234188d6cfb3cabddef1e6ef20f Mon Sep 17 00:00:00 2001 From: Michael Hewson Date: Mon, 8 Oct 2018 20:40:57 -0400 Subject: [PATCH 14/19] Add layout sanity checks in object safety MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If object-safety checks succeed for a receiver type, make sure the receiver’s abi is a) a Scalar, when Self = () b) a ScalarPair, when Self = dyn Trait --- src/librustc/traits/object_safety.rs | 100 +++++++++++++++++++++++++++ 1 file changed, 100 insertions(+) diff --git a/src/librustc/traits/object_safety.rs b/src/librustc/traits/object_safety.rs index cd7e2774371fe..7e2b56379521a 100644 --- a/src/librustc/traits/object_safety.rs +++ b/src/librustc/traits/object_safety.rs @@ -326,12 +326,112 @@ impl<'a, 'tcx> TyCtxt<'a, 'tcx, 'tcx> { if receiver_ty != self.mk_self_type() { if !self.receiver_is_dispatchable(method, receiver_ty) { return Some(MethodViolationCode::UndispatchableReceiver); + } else { + // sanity check to make sure the receiver actually has the layout of a pointer + + use ty::layout::Abi; + + let param_env = self.param_env(method.def_id); + + let abi_of_ty = |ty: Ty<'tcx>| -> &Abi { + match self.layout_of(param_env.and(ty)) { + Ok(layout) => &layout.abi, + Err(err) => bug!( + "Error: {}\n while computing layout for type {:?}", err, ty + ) + } + }; + + // e.g. Rc<()> + let unit_receiver_ty = self.receiver_for_self_ty( + receiver_ty, self.mk_unit(), method.def_id + ); + + match abi_of_ty(unit_receiver_ty) { + &Abi::Scalar(..) => (), + abi => bug!("Receiver when Self = () should have a Scalar ABI, found {:?}", abi) + } + + let trait_object_ty = self.object_ty_for_trait( + trait_def_id, self.mk_region(ty::ReStatic) + ); + + // e.g. Rc + let trait_object_receiver = self.receiver_for_self_ty( + receiver_ty, trait_object_ty, method.def_id + ); + + match abi_of_ty(trait_object_receiver) { + &Abi::ScalarPair(..) => (), + abi => bug!( + "Receiver when Self = {} should have a ScalarPair ABI, found {:?}", + trait_object_ty, abi + ) + } } } None } + /// performs a type substitution to produce the version of receiver_ty when `Self = self_ty` + /// e.g. for receiver_ty = `Rc` and self_ty = `Foo`, returns `Rc` + fn receiver_for_self_ty( + self, receiver_ty: Ty<'tcx>, self_ty: Ty<'tcx>, method_def_id: DefId + ) -> Ty<'tcx> { + let substs = Substs::for_item(self, method_def_id, |param, _| { + if param.index == 0 { + self_ty.into() + } else { + self.mk_param_from_def(param) + } + }); + + receiver_ty.subst(self, substs) + } + + /// creates the object type for the current trait. For example, + /// if the current trait is `Deref`, then this will be + /// `dyn Deref + 'static` + fn object_ty_for_trait(self, trait_def_id: DefId, lifetime: ty::Region<'tcx>) -> Ty<'tcx> { + debug!("object_ty_for_trait: trait_def_id={:?}", trait_def_id); + + let trait_ref = ty::TraitRef::identity(self, trait_def_id); + + let trait_predicate = ty::ExistentialPredicate::Trait( + ty::ExistentialTraitRef::erase_self_ty(self, trait_ref) + ); + + let mut associated_types = traits::supertraits(self, ty::Binder::dummy(trait_ref)) + .flat_map(|trait_ref| self.associated_items(trait_ref.def_id())) + .filter(|item| item.kind == ty::AssociatedKind::Type) + .collect::>(); + + // existential predicates need to be in a specific order + associated_types.sort_by_key(|item| self.def_path_hash(item.def_id)); + + let projection_predicates = associated_types.into_iter().map(|item| { + ty::ExistentialPredicate::Projection(ty::ExistentialProjection { + ty: self.mk_projection(item.def_id, trait_ref.substs), + item_def_id: item.def_id, + substs: trait_ref.substs, + }) + }); + + let existential_predicates = self.mk_existential_predicates( + iter::once(trait_predicate).chain(projection_predicates) + ); + + let object_ty = self.mk_dynamic( + ty::Binder::dummy(existential_predicates), + lifetime, + ); + + debug!("object_ty_for_trait: object_ty=`{}`", object_ty); + + object_ty + } + /// checks the method's receiver (the `self` argument) can be dispatched on when `Self` is a /// trait object. We require that `DispatchableFromDyn` be implemented for the receiver type /// in the following way: From b5b25f8196567b5a86eb0f805852e11889d80075 Mon Sep 17 00:00:00 2001 From: Michael Hewson Date: Tue, 9 Oct 2018 01:13:07 -0400 Subject: [PATCH 15/19] Put backticks around field names, types and paths in error messages Added to `DispatchFromDyn` and `CoerceUnsized` error messages --- src/librustc_typeck/coherence/builtin.rs | 8 ++++---- src/test/ui/error-codes/E0375.stderr | 2 +- src/test/ui/invalid_dispatch_from_dyn_impls.stderr | 2 +- src/test/ui/issues/issue-26905.stderr | 2 +- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/librustc_typeck/coherence/builtin.rs b/src/librustc_typeck/coherence/builtin.rs index a01718bc12870..e5a52a1f6ac61 100644 --- a/src/librustc_typeck/coherence/builtin.rs +++ b/src/librustc_typeck/coherence/builtin.rs @@ -215,7 +215,7 @@ fn visit_implementation_of_dispatch_from_dyn<'a, 'tcx>( &format!( "the trait `DispatchFromDyn` may only be implemented \ for a coercion between structures with the same \ - definition; expected {}, found {}", + definition; expected `{}`, found `{}`", source_path, target_path, ) ).emit(); @@ -271,7 +271,7 @@ fn visit_implementation_of_dispatch_from_dyn<'a, 'tcx>( "currently, {} fields need coercions: {}", coerced_fields.len(), coerced_fields.iter().map(|field| { - format!("{} ({} to {})", + format!("`{}` (`{}` to `{}`)", field.ident, field.ty(tcx, substs_a), field.ty(tcx, substs_b), @@ -398,7 +398,7 @@ pub fn coerce_unsized_info<'a, 'gcx>(gcx: TyCtxt<'a, 'gcx, 'gcx>, E0377, "the trait `CoerceUnsized` may only be implemented \ for a coercion between structures with the same \ - definition; expected {}, found {}", + definition; expected `{}`, found `{}`", source_path, target_path); return err_info; @@ -503,7 +503,7 @@ pub fn coerce_unsized_info<'a, 'gcx>(gcx: TyCtxt<'a, 'gcx, 'gcx>, diff_fields.len(), diff_fields.iter() .map(|&(i, a, b)| { - format!("{} ({} to {})", fields[i].ident, a, b) + format!("`{}` (`{}` to `{}`)", fields[i].ident, a, b) }) .collect::>() .join(", "))); diff --git a/src/test/ui/error-codes/E0375.stderr b/src/test/ui/error-codes/E0375.stderr index 3ffd25084b829..f3db697790c5e 100644 --- a/src/test/ui/error-codes/E0375.stderr +++ b/src/test/ui/error-codes/E0375.stderr @@ -5,7 +5,7 @@ LL | impl CoerceUnsized> for Foo {} | ^^^^^^^^^^^^^^^^^^^^^^^^ requires multiple coercions | = note: `CoerceUnsized` may only be implemented for a coercion between structures with one field being coerced - = note: currently, 2 fields need coercions: b (T to U), c (U to T) + = note: currently, 2 fields need coercions: `b` (`T` to `U`), `c` (`U` to `T`) error: aborting due to previous error diff --git a/src/test/ui/invalid_dispatch_from_dyn_impls.stderr b/src/test/ui/invalid_dispatch_from_dyn_impls.stderr index 5e394f2fb9152..f4affb8551462 100644 --- a/src/test/ui/invalid_dispatch_from_dyn_impls.stderr +++ b/src/test/ui/invalid_dispatch_from_dyn_impls.stderr @@ -19,7 +19,7 @@ LL | | {} //~^^^ ERROR [E0378] | |__^ | = note: the trait `DispatchFromDyn` may only be implemented for a coercion between structures with a single field being coerced - = note: currently, 2 fields need coercions: ptr1 (*const T to *const U), ptr2 (*const T to *const U) + = note: currently, 2 fields need coercions: `ptr1` (`*const T` to `*const U`), `ptr2` (`*const T` to `*const U`) error[E0378]: the trait `DispatchFromDyn` may only be implemented for a coercion between structures with a single field being coerced, none found --> $DIR/invalid_dispatch_from_dyn_impls.rs:41:1 diff --git a/src/test/ui/issues/issue-26905.stderr b/src/test/ui/issues/issue-26905.stderr index f18b58a8330e0..7feabef56609a 100644 --- a/src/test/ui/issues/issue-26905.stderr +++ b/src/test/ui/issues/issue-26905.stderr @@ -5,7 +5,7 @@ LL | impl, U: ?Sized> CoerceUnsized> for MyRc{ | ^^^^^^^^^^^^^^^^^^^^^^ requires multiple coercions | = note: `CoerceUnsized` may only be implemented for a coercion between structures with one field being coerced - = note: currently, 2 fields need coercions: _ptr (*const T to *const U), _boo (NotPhantomData to NotPhantomData) + = note: currently, 2 fields need coercions: `_ptr` (`*const T` to `*const U`), `_boo` (`NotPhantomData` to `NotPhantomData`) error: aborting due to previous error From eb997d76d5a1c0ed933ab76459233e3650d2771d Mon Sep 17 00:00:00 2001 From: Michael Hewson Date: Fri, 12 Oct 2018 00:12:56 -0400 Subject: [PATCH 16/19] add `U: Trait` to the param env during DispatchFromDyn check also updated the doc on `receiver_is_dispatchable` to reflect current state of the implementation --- src/librustc/traits/object_safety.rs | 67 +++++++++++++++++----------- 1 file changed, 40 insertions(+), 27 deletions(-) diff --git a/src/librustc/traits/object_safety.rs b/src/librustc/traits/object_safety.rs index 7e2b56379521a..1d76ccdca3161 100644 --- a/src/librustc/traits/object_safety.rs +++ b/src/librustc/traits/object_safety.rs @@ -437,16 +437,16 @@ impl<'a, 'tcx> TyCtxt<'a, 'tcx, 'tcx> { /// in the following way: /// - let `Receiver` be the type of the `self` argument, i.e `Self`, `&Self`, `Rc` /// - require the following bound: - /// forall(T: Trait) { - /// Receiver[Self => T]: DispatchFromDyn dyn Trait]> - /// } - /// where `Foo[X => Y]` means "the same type as `Foo`, but with `X` replaced with `Y`" + /// + /// Receiver[Self => T]: DispatchFromDyn dyn Trait]> + /// + /// where `Foo[X => Y]` means "the same type as `Foo`, but with `X` replaced with `Y`" /// (substitution notation). /// /// some examples of receiver types and their required obligation - /// - `&'a mut self` requires `&'a mut T: DispatchFromDyn<&'a mut dyn Trait>` - /// - `self: Rc` requires `Rc: DispatchFromDyn>` - /// - `self: Pin>` requires `Pin>: DispatchFromDyn>>` + /// - `&'a mut self` requires `&'a mut Self: DispatchFromDyn<&'a mut dyn Trait>` + /// - `self: Rc` requires `Rc: DispatchFromDyn>` + /// - `self: Pin>` requires `Pin>: DispatchFromDyn>>` /// /// The only case where the receiver is not dispatchable, but is still a valid receiver /// type (just not object-safe), is when there is more than one level of pointer indirection. @@ -456,14 +456,12 @@ impl<'a, 'tcx> TyCtxt<'a, 'tcx, 'tcx> { /// contained by the trait object, because the object that needs to be coerced is behind /// a pointer. /// - /// In practice, there are issues with the above bound: `where` clauses that apply to `Self` - /// would have to apply to `T`, trait object types have a lot of parameters that need to - /// be filled in (lifetime and type parameters, and the lifetime of the actual object), and - /// I'm pretty sure using `dyn Trait` in the query causes another object-safety query for - /// `Trait`, resulting in cyclic queries. So in the implementation, we use the following, - /// more general bound: + /// In practice, we cannot use `dyn Trait` explicitly in the obligation because it would result + /// in a new check that `Trait` is object safe, creating a cycle. So instead, we fudge a little + /// by introducing a new type parameter `U` such that `Self: Unsize` and `U: Trait + ?Sized`, + /// and use `U` in place of `dyn Trait`. Written as a chalk-style query: /// - /// forall (U: ?Sized) { + /// forall (U: Trait + ?Sized) { /// if (Self: Unsize) { /// Receiver: DispatchFromDyn U]> /// } @@ -493,6 +491,7 @@ impl<'a, 'tcx> TyCtxt<'a, 'tcx, 'tcx> { return false; }; + // the type `U` in the query // use a bogus type parameter to mimick a forall(U) query using u32::MAX for now. // FIXME(mikeyhew) this is a total hack, and we should replace it when real forall queries // are implemented @@ -501,17 +500,41 @@ impl<'a, 'tcx> TyCtxt<'a, 'tcx, 'tcx> { Name::intern("RustaceansAreAwesome").as_interned_str(), ); - // create a modified param env, with `Self: Unsize` added to the caller bounds + // `Receiver[Self => U]` + let unsized_receiver_ty = self.receiver_for_self_ty( + receiver_ty, unsized_self_ty, method.def_id + ); + + // create a modified param env, with `Self: Unsize` and `U: Trait` added to caller bounds + // `U: ?Sized` is already implied here let param_env = { let mut param_env = self.param_env(method.def_id); - let predicate = ty::TraitRef { + // Self: Unsize + let unsize_predicate = ty::TraitRef { def_id: unsize_did, substs: self.mk_substs_trait(self.mk_self_type(), &[unsized_self_ty.into()]), }.to_predicate(); + // U: Trait + let trait_predicate = { + let substs = Substs::for_item(self, method.container.assert_trait(), |param, _| { + if param.index == 0 { + unsized_self_ty.into() + } else { + self.mk_param_from_def(param) + } + }); + + ty::TraitRef { + def_id: unsize_did, + substs, + }.to_predicate() + }; + let caller_bounds: Vec> = param_env.caller_bounds.iter().cloned() - .chain(iter::once(predicate)) + .chain(iter::once(unsize_predicate)) + .chain(iter::once(trait_predicate)) .collect(); param_env.caller_bounds = self.intern_predicates(&caller_bounds); @@ -519,16 +542,6 @@ impl<'a, 'tcx> TyCtxt<'a, 'tcx, 'tcx> { param_env }; - let receiver_substs = Substs::for_item(self, method.def_id, |param, _| { - if param.index == 0 { - unsized_self_ty.into() - } else { - self.mk_param_from_def(param) - } - }); - // the type `Receiver[Self => U]` in the query - let unsized_receiver_ty = receiver_ty.subst(self, receiver_substs); - // Receiver: DispatchFromDyn U]> let obligation = { let predicate = ty::TraitRef { From 3db22039dceb36fc15e770c24ec99e2db1ca586d Mon Sep 17 00:00:00 2001 From: Michael Hewson Date: Wed, 17 Oct 2018 18:54:13 -0400 Subject: [PATCH 17/19] Remove this check for object-safety during selection of trait object candidates I don't really understand what it's for, but see the comment here: https://github.com/rust-lang/rust/pull/50173#discussion_r204222336 where arielb1 said > Does this check do anything these days? I think `$0: Trait` is always considered ambiguous and nikomatsakis agreed we may be able to get rid of it --- src/librustc/traits/select.rs | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/src/librustc/traits/select.rs b/src/librustc/traits/select.rs index 0742f377f18c2..126c158f846bd 100644 --- a/src/librustc/traits/select.rs +++ b/src/librustc/traits/select.rs @@ -2111,17 +2111,6 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> { data.principal().with_self_ty(this.tcx(), self_ty) } ty::Infer(ty::TyVar(_)) => { - // Object-safety candidates are only applicable to object-safe - // traits. Including this check is useful because it helps - // inference in cases of traits like `BorrowFrom`, which are - // not object-safe, and which rely on being able to infer the - // self-type from one of the other inputs. Without this check, - // these cases wind up being considered ambiguous due to a - // (spurious) ambiguity introduced here. - let predicate_trait_ref = obligation.predicate.to_poly_trait_ref(); - if !this.tcx().is_object_safe(predicate_trait_ref.def_id()) { - return; - } debug!("assemble_candidates_from_object_ty: ambiguous"); candidates.ambiguous = true; // could wind up being an object type return; From a468da9cfb98df126c0ba709816843115654e3dc Mon Sep 17 00:00:00 2001 From: Michael Hewson Date: Fri, 26 Oct 2018 01:09:33 -0400 Subject: [PATCH 18/19] Add a check for reprs that could change the ABI disallow `#[repr(C)] and `#[repr(packed)]` on structs implementing DispatchFromDyn because they will change the ABI from Scalar/ScalarPair to Aggregrate, resulting in an ICE during object-safety checks or codegen --- src/librustc_typeck/coherence/builtin.rs | 7 +++++++ src/test/ui/invalid_dispatch_from_dyn_impls.rs | 8 ++++++++ src/test/ui/invalid_dispatch_from_dyn_impls.stderr | 11 ++++++++++- 3 files changed, 25 insertions(+), 1 deletion(-) diff --git a/src/librustc_typeck/coherence/builtin.rs b/src/librustc_typeck/coherence/builtin.rs index e5a52a1f6ac61..99c6ba457faa0 100644 --- a/src/librustc_typeck/coherence/builtin.rs +++ b/src/librustc_typeck/coherence/builtin.rs @@ -223,6 +223,13 @@ fn visit_implementation_of_dispatch_from_dyn<'a, 'tcx>( return } + if def_a.repr.c() || def_a.repr.packed() { + create_err( + "structs implementing `DispatchFromDyn` may not have \ + `#[repr(packed)]` or `#[repr(C)]`" + ).emit(); + } + let fields = &def_a.non_enum_variant().fields; let coerced_fields = fields.iter().filter_map(|field| { diff --git a/src/test/ui/invalid_dispatch_from_dyn_impls.rs b/src/test/ui/invalid_dispatch_from_dyn_impls.rs index 101e1eb6e3071..1cf5c73ab138f 100644 --- a/src/test/ui/invalid_dispatch_from_dyn_impls.rs +++ b/src/test/ui/invalid_dispatch_from_dyn_impls.rs @@ -41,4 +41,12 @@ struct NothingToCoerce { impl DispatchFromDyn> for NothingToCoerce {} //~^ ERROR [E0378] +#[repr(C)] +struct HasReprC(Box); + +impl DispatchFromDyn> for HasReprC +where + T: Unsize, +{} //~^^^ ERROR [E0378] + fn main() {} diff --git a/src/test/ui/invalid_dispatch_from_dyn_impls.stderr b/src/test/ui/invalid_dispatch_from_dyn_impls.stderr index f4affb8551462..82186b67d97fd 100644 --- a/src/test/ui/invalid_dispatch_from_dyn_impls.stderr +++ b/src/test/ui/invalid_dispatch_from_dyn_impls.stderr @@ -27,6 +27,15 @@ error[E0378]: the trait `DispatchFromDyn` may only be implemented for a coercion LL | impl DispatchFromDyn> for NothingToCoerce {} | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -error: aborting due to 3 previous errors +error[E0378]: structs implementing `DispatchFromDyn` may not have `#[repr(packed)]` or `#[repr(C)]` + --> $DIR/invalid_dispatch_from_dyn_impls.rs:47:1 + | +LL | / impl DispatchFromDyn> for HasReprC +LL | | where +LL | | T: Unsize, +LL | | {} //~^^^ ERROR [E0378] + | |__^ + +error: aborting due to 4 previous errors For more information about this error, try `rustc --explain E0378`. From 192e7c4b51ab35f4cfbbacde9eeffa8e12dba873 Mon Sep 17 00:00:00 2001 From: Michael Hewson Date: Thu, 1 Nov 2018 18:28:52 -0400 Subject: [PATCH 19/19] Add comments explaining how codegen works for `dyn Trait` methods --- src/librustc_codegen_llvm/abi.rs | 5 +++++ src/librustc_codegen_llvm/mir/block.rs | 8 ++++++-- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/src/librustc_codegen_llvm/abi.rs b/src/librustc_codegen_llvm/abi.rs index 1662b57b8b319..e50534a4e1dc9 100644 --- a/src/librustc_codegen_llvm/abi.rs +++ b/src/librustc_codegen_llvm/abi.rs @@ -315,6 +315,11 @@ impl<'tcx> FnTypeExt<'tcx> for FnType<'tcx, Ty<'tcx>> { _ => bug!("receiver type has unsupported layout: {:?}", layout) } + // In the case of Rc, we need to explicitly pass a *mut RcBox + // with a Scalar (not ScalarPair) ABI. This is a hack that is understood + // elsewhere in the compiler as a method on a `dyn Trait`. + // To get the type `*mut RcBox`, we just keep unwrapping newtypes until we + // get a built-in pointer type let mut fat_pointer_layout = layout; 'descend_newtypes: while !fat_pointer_layout.ty.is_unsafe_ptr() && !fat_pointer_layout.ty.is_region_ptr() diff --git a/src/librustc_codegen_llvm/mir/block.rs b/src/librustc_codegen_llvm/mir/block.rs index 0cb4963f97fae..a7f4c48c89bd6 100644 --- a/src/librustc_codegen_llvm/mir/block.rs +++ b/src/librustc_codegen_llvm/mir/block.rs @@ -647,8 +647,12 @@ impl FunctionCx<'a, 'll, 'tcx> { if let (0, Some(ty::InstanceDef::Virtual(_, idx))) = (i, def) { if let Pair(..) = op.val { - // descend through newtype wrappers until `op` is a builtin pointer to - // `dyn Trait`, e.g. `*const dyn Trait`, `&mut dyn Trait` + // In the case of Rc, we need to explicitly pass a + // *mut RcBox with a Scalar (not ScalarPair) ABI. This is a hack + // that is understood elsewhere in the compiler as a method on + // `dyn Trait`. + // To get a `*mut RcBox`, we just keep unwrapping newtypes until + // we get a value of a built-in pointer type 'descend_newtypes: while !op.layout.ty.is_unsafe_ptr() && !op.layout.ty.is_region_ptr() {