From c48650ec25d2e7e872912137e68496248743f1fe Mon Sep 17 00:00:00 2001 From: Ariel Ben-Yehuda Date: Wed, 22 Nov 2017 22:39:40 +0200 Subject: [PATCH] fix treatment of local types in "remote coherence" mode --- src/librustc/traits/coherence.rs | 125 +++++++++++++++++++------------ src/librustc/traits/select.rs | 4 +- 2 files changed, 81 insertions(+), 48 deletions(-) diff --git a/src/librustc/traits/coherence.rs b/src/librustc/traits/coherence.rs index 10a32c26e74..2ca4628ab13 100644 --- a/src/librustc/traits/coherence.rs +++ b/src/librustc/traits/coherence.rs @@ -19,8 +19,18 @@ use ty::subst::Subst; use infer::{InferCtxt, InferOk}; -#[derive(Copy, Clone)] -struct InferIsLocal(bool); +#[derive(Copy, Clone, Debug)] +enum InferIsLocal { + BrokenYes, + Yes, + No +} + +#[derive(Debug, Copy, Clone)] +pub enum Conflict { + Upstream, + Downstream +} pub struct OverlapResult<'tcx> { pub impl_header: ty::ImplHeader<'tcx>, @@ -126,32 +136,46 @@ fn overlap<'cx, 'gcx, 'tcx>(selcx: &mut SelectionContext<'cx, 'gcx, 'tcx>, } pub fn trait_ref_is_knowable<'a, 'gcx, 'tcx>(tcx: TyCtxt<'a, 'gcx, 'tcx>, - trait_ref: ty::TraitRef<'tcx>) -> bool + trait_ref: ty::TraitRef<'tcx>, + broken: bool) + -> Option { - debug!("trait_ref_is_knowable(trait_ref={:?})", trait_ref); + debug!("trait_ref_is_knowable(trait_ref={:?}, broken={:?})", trait_ref, broken); + let mode = if broken { + InferIsLocal::BrokenYes + } else { + InferIsLocal::Yes + }; + if orphan_check_trait_ref(tcx, trait_ref, mode).is_ok() { + // A downstream or cousin crate is allowed to implement some + // substitution of this trait-ref. + debug!("trait_ref_is_knowable: downstream crate might implement"); + return Some(Conflict::Downstream); + } - // if the orphan rules pass, that means that no ancestor crate can - // impl this, so it's up to us. - if orphan_check_trait_ref(tcx, trait_ref, InferIsLocal(false)).is_ok() { + if trait_ref_is_local_or_fundamental(tcx, trait_ref) { + // This is a local or fundamental trait, so future-compatibility + // is no concern. We know that downstream/cousin crates are not + // allowed to implement a substitution of this trait ref, which + // means impls could only come from dependencies of this crate, + // which we already know about. + return None; + } + // This is a remote non-fundamental trait, so if another crate + // can be the "final owner" of a substitution of this trait-ref, + // they are allowed to implement it future-compatibly. + // + // However, if we are a final owner, then nobody else can be, + // and if we are an intermediate owner, then we don't care + // about future-compatibility, which means that we're OK if + // we are an owner. + if orphan_check_trait_ref(tcx, trait_ref, InferIsLocal::No).is_ok() { debug!("trait_ref_is_knowable: orphan check passed"); - return true; + return None; + } else { + debug!("trait_ref_is_knowable: nonlocal, nonfundamental, unowned"); + return Some(Conflict::Upstream); } - - // if the trait is not marked fundamental, then it's always possible that - // an ancestor crate will impl this in the future, if they haven't - // already - if !trait_ref_is_local_or_fundamental(tcx, trait_ref) { - debug!("trait_ref_is_knowable: trait is neither local nor fundamental"); - return false; - } - - // find out when some downstream (or cousin) crate could impl this - // trait-ref, presuming that all the parameters were instantiated - // with downstream types. If not, then it could only be - // implemented by an upstream crate, which means that the impl - // must be visible to us, and -- since the trait is fundamental - // -- we can test. - orphan_check_trait_ref(tcx, trait_ref, InferIsLocal(true)).is_err() } pub fn trait_ref_is_local_or_fundamental<'a, 'gcx, 'tcx>(tcx: TyCtxt<'a, 'gcx, 'tcx>, @@ -189,7 +213,7 @@ pub fn orphan_check<'a, 'gcx, 'tcx>(tcx: TyCtxt<'a, 'gcx, 'tcx>, return Ok(()); } - orphan_check_trait_ref(tcx, trait_ref, InferIsLocal(false)) + orphan_check_trait_ref(tcx, trait_ref, InferIsLocal::No) } fn orphan_check_trait_ref<'tcx>(tcx: TyCtxt, @@ -197,8 +221,8 @@ fn orphan_check_trait_ref<'tcx>(tcx: TyCtxt, infer_is_local: InferIsLocal) -> Result<(), OrphanCheckErr<'tcx>> { - debug!("orphan_check_trait_ref(trait_ref={:?}, infer_is_local={})", - trait_ref, infer_is_local.0); + debug!("orphan_check_trait_ref(trait_ref={:?}, infer_is_local={:?})", + trait_ref, infer_is_local); // First, create an ordered iterator over all the type parameters to the trait, with the self // type appearing first. @@ -212,7 +236,9 @@ fn orphan_check_trait_ref<'tcx>(tcx: TyCtxt, // uncovered type parameters. let uncovered_tys = uncovered_tys(tcx, input_ty, infer_is_local); for uncovered_ty in uncovered_tys { - if let Some(param) = uncovered_ty.walk().find(|t| is_type_parameter(t)) { + if let Some(param) = uncovered_ty.walk() + .find(|t| is_possibly_remote_type(t, infer_is_local)) + { debug!("orphan_check_trait_ref: uncovered type `{:?}`", param); return Err(OrphanCheckErr::UncoveredTy(param)); } @@ -224,11 +250,11 @@ fn orphan_check_trait_ref<'tcx>(tcx: TyCtxt, // Otherwise, enforce invariant that there are no type // parameters reachable. - if !infer_is_local.0 { - if let Some(param) = input_ty.walk().find(|t| is_type_parameter(t)) { - debug!("orphan_check_trait_ref: uncovered type `{:?}`", param); - return Err(OrphanCheckErr::UncoveredTy(param)); - } + if let Some(param) = input_ty.walk() + .find(|t| is_possibly_remote_type(t, infer_is_local)) + { + debug!("orphan_check_trait_ref: uncovered type `{:?}`", param); + return Err(OrphanCheckErr::UncoveredTy(param)); } } @@ -250,7 +276,7 @@ fn uncovered_tys<'tcx>(tcx: TyCtxt, ty: Ty<'tcx>, infer_is_local: InferIsLocal) } } -fn is_type_parameter(ty: Ty) -> bool { +fn is_possibly_remote_type(ty: Ty, _infer_is_local: InferIsLocal) -> bool { match ty.sty { ty::TyProjection(..) | ty::TyParam(..) => true, _ => false, @@ -273,7 +299,15 @@ fn fundamental_ty(tcx: TyCtxt, ty: Ty) -> bool { } } -fn ty_is_local_constructor(ty: Ty, infer_is_local: InferIsLocal)-> bool { +fn def_id_is_local(def_id: DefId, infer_is_local: InferIsLocal) -> bool { + match infer_is_local { + InferIsLocal::Yes => false, + InferIsLocal::No | + InferIsLocal::BrokenYes => def_id.is_local() + } +} + +fn ty_is_local_constructor(ty: Ty, infer_is_local: InferIsLocal) -> bool { debug!("ty_is_local_constructor({:?})", ty); match ty.sty { @@ -296,20 +330,19 @@ fn ty_is_local_constructor(ty: Ty, infer_is_local: InferIsLocal)-> bool { false } - ty::TyInfer(..) => { - infer_is_local.0 - } + ty::TyInfer(..) => match infer_is_local { + InferIsLocal::No => false, + InferIsLocal::Yes | + InferIsLocal::BrokenYes => true + }, - ty::TyAdt(def, _) => { - def.did.is_local() - } - - ty::TyForeign(did) => { - did.is_local() - } + ty::TyAdt(def, _) => def_id_is_local(def.did, infer_is_local), + ty::TyForeign(did) => def_id_is_local(did, infer_is_local), ty::TyDynamic(ref tt, ..) => { - tt.principal().map_or(false, |p| p.def_id().is_local()) + tt.principal().map_or(false, |p| { + def_id_is_local(p.def_id(), infer_is_local) + }) } ty::TyError => { diff --git a/src/librustc/traits/select.rs b/src/librustc/traits/select.rs index 4bc3e2dd4d8..f236216c096 100644 --- a/src/librustc/traits/select.rs +++ b/src/librustc/traits/select.rs @@ -814,7 +814,7 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> { // terms of `Fn` etc, but we could probably make this more // precise still. let unbound_input_types = stack.fresh_trait_ref.input_types().any(|ty| ty.is_fresh()); - if unbound_input_types && self.intercrate { + if unbound_input_types && self.intercrate && false { debug!("evaluate_stack({:?}) --> unbound argument, intercrate --> ambiguous", stack.fresh_trait_ref); // Heuristics: show the diagnostics when there are no candidates in crate. @@ -1221,7 +1221,7 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> { // bound regions let trait_ref = predicate.skip_binder().trait_ref; - coherence::trait_ref_is_knowable(self.tcx(), trait_ref) + coherence::trait_ref_is_knowable(self.tcx(), trait_ref, false).is_none() } /// Returns true if the global caches can be used.