Skip to content

Commit

Permalink
implement a hack to make traitobject 0.1.0 compile
Browse files Browse the repository at this point in the history
  • Loading branch information
arielb1 committed Jan 3, 2019
1 parent 7eb444e commit 0b511b7
Show file tree
Hide file tree
Showing 15 changed files with 478 additions and 90 deletions.
1 change: 1 addition & 0 deletions src/librustc/dep_graph/dep_node.rs
Expand Up @@ -492,6 +492,7 @@ define_dep_nodes!( <'tcx>
[] AdtDefOfItem(DefId),
[] ImplTraitRef(DefId),
[] ImplPolarity(DefId),
[] Issue33140SelfTy(DefId),
[] FnSignature(DefId),
[] CoerceUnsizedInfo(DefId),

Expand Down
2 changes: 2 additions & 0 deletions src/librustc/traits/mod.rs
Expand Up @@ -56,6 +56,8 @@ pub use self::select::{EvaluationCache, SelectionContext, SelectionCache};
pub use self::select::{EvaluationResult, IntercrateAmbiguityCause, OverflowError};
pub use self::specialize::{OverlapError, specialization_graph, translate_substs};
pub use self::specialize::find_associated_item;
pub use self::specialize::specialization_graph::FutureCompatOverlapError;
pub use self::specialize::specialization_graph::FutureCompatOverlapErrorKind;
pub use self::engine::{TraitEngine, TraitEngineExt};
pub use self::util::{elaborate_predicates, elaborate_trait_ref, elaborate_trait_refs};
pub use self::util::{supertraits, supertrait_def_ids, transitive_bounds,
Expand Down
3 changes: 2 additions & 1 deletion src/librustc/traits/select.rs
Expand Up @@ -2260,7 +2260,8 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> {
ImplCandidate(victim_def) => {
let tcx = self.tcx().global_tcx();
return tcx.specializes((other_def, victim_def))
|| tcx.impls_are_allowed_to_overlap(other_def, victim_def);
|| tcx.impls_are_allowed_to_overlap(
other_def, victim_def).is_some();
}
ParamCandidate(ref cand) => {
// Prefer the impl to a global where clause candidate.
Expand Down
21 changes: 14 additions & 7 deletions src/librustc/traits/specialize/mod.rs
Expand Up @@ -14,11 +14,10 @@ pub mod specialization_graph;
use hir::def_id::DefId;
use infer::{InferCtxt, InferOk};
use lint;
use traits::coherence;
use traits::{self, coherence, FutureCompatOverlapErrorKind, ObligationCause, TraitEngine};
use rustc_data_structures::fx::FxHashSet;
use rustc_data_structures::sync::Lrc;
use syntax_pos::DUMMY_SP;
use traits::{self, ObligationCause, TraitEngine};
use traits::select::IntercrateAmbiguityCause;
use ty::{self, TyCtxt, TypeFoldable};
use ty::subst::{Subst, Substs};
Expand All @@ -27,6 +26,7 @@ use super::{SelectionContext, FulfillmentContext};
use super::util::impl_trait_ref_and_oblig;

/// Information pertinent to an overlapping impl error.
#[derive(Debug)]
pub struct OverlapError {
pub with_impl: DefId,
pub trait_desc: String,
Expand Down Expand Up @@ -310,8 +310,9 @@ pub(super) fn specialization_graph_provider<'a, 'tcx>(
let insert_result = sg.insert(tcx, impl_def_id);
// Report error if there was one.
let (overlap, used_to_be_allowed) = match insert_result {
Err(overlap) => (Some(overlap), false),
Ok(opt_overlap) => (opt_overlap, true)
Err(overlap) => (Some(overlap), None),
Ok(Some(overlap)) => (Some(overlap.error), Some(overlap.kind)),
Ok(None) => (None, None)
};

if let Some(overlap) = overlap {
Expand All @@ -321,14 +322,20 @@ pub(super) fn specialization_graph_provider<'a, 'tcx>(
String::new(), |ty| {
format!(" for type `{}`", ty)
}),
if used_to_be_allowed { " (E0119)" } else { "" }
if used_to_be_allowed.is_some() { " (E0119)" } else { "" }
);
let impl_span = tcx.sess.source_map().def_span(
tcx.span_of_impl(impl_def_id).unwrap()
);
let mut err = if used_to_be_allowed {
let mut err = if let Some(kind) = used_to_be_allowed {
let lint = match kind {
FutureCompatOverlapErrorKind::Issue43355 =>
lint::builtin::INCOHERENT_FUNDAMENTAL_IMPLS,
FutureCompatOverlapErrorKind::Issue33140 =>
lint::builtin::ORDER_DEPENDENT_TRAIT_OBJECTS,
};
tcx.struct_span_lint_node(
lint::builtin::INCOHERENT_FUNDAMENTAL_IMPLS,
lint,
tcx.hir().as_local_node_id(impl_def_id).unwrap(),
impl_span,
&msg)
Expand Down
44 changes: 39 additions & 5 deletions src/librustc/traits/specialize/specialization_graph.rs
Expand Up @@ -58,10 +58,22 @@ struct Children {
blanket_impls: Vec<DefId>,
}

#[derive(Copy, Clone, Debug)]
pub enum FutureCompatOverlapErrorKind {
Issue43355,
Issue33140,
}

#[derive(Debug)]
pub struct FutureCompatOverlapError {
pub error: OverlapError,
pub kind: FutureCompatOverlapErrorKind
}

/// The result of attempting to insert an impl into a group of children.
enum Inserted {
/// The impl was inserted as a new child in this group of children.
BecameNewSibling(Option<OverlapError>),
BecameNewSibling(Option<FutureCompatOverlapError>),

/// The impl should replace existing impls [X1, ..], because the impl specializes X1, X2, etc.
ReplaceChildren(Vec<DefId>),
Expand Down Expand Up @@ -162,7 +174,19 @@ impl<'a, 'gcx, 'tcx> Children {
impl_def_id,
traits::IntercrateMode::Issue43355,
|overlap| {
if tcx.impls_are_allowed_to_overlap(impl_def_id, possible_sibling) {
if let Some(overlap_kind) =
tcx.impls_are_allowed_to_overlap(impl_def_id, possible_sibling)
{
match overlap_kind {
ty::ImplOverlapKind::Permitted => {}
ty::ImplOverlapKind::Issue33140 => {
last_lint = Some(FutureCompatOverlapError {
error: overlap_error(overlap),
kind: FutureCompatOverlapErrorKind::Issue33140
});
}
}

return Ok((false, false));
}

Expand Down Expand Up @@ -190,13 +214,23 @@ impl<'a, 'gcx, 'tcx> Children {

replace_children.push(possible_sibling);
} else {
if !tcx.impls_are_allowed_to_overlap(impl_def_id, possible_sibling) {
if let None = tcx.impls_are_allowed_to_overlap(
impl_def_id, possible_sibling)
{
// do future-compat checks for overlap. Have issue #33140
// errors overwrite issue #43355 errors when both are present.

traits::overlapping_impls(
tcx,
possible_sibling,
impl_def_id,
traits::IntercrateMode::Fixed,
|overlap| last_lint = Some(overlap_error(overlap)),
|overlap| {
last_lint = Some(FutureCompatOverlapError {
error: overlap_error(overlap),
kind: FutureCompatOverlapErrorKind::Issue43355
});
},
|| (),
);
}
Expand Down Expand Up @@ -263,7 +297,7 @@ impl<'a, 'gcx, 'tcx> Graph {
pub fn insert(&mut self,
tcx: TyCtxt<'a, 'gcx, 'tcx>,
impl_def_id: DefId)
-> Result<Option<OverlapError>, OverlapError> {
-> Result<Option<FutureCompatOverlapError>, OverlapError> {
assert!(impl_def_id.is_local());

let trait_ref = tcx.impl_trait_ref(impl_def_id).unwrap();
Expand Down
122 changes: 120 additions & 2 deletions src/librustc/ty/mod.rs
Expand Up @@ -2637,6 +2637,45 @@ impl<'gcx> ::std::ops::Deref for Attributes<'gcx> {
}
}

#[derive(Debug, PartialEq, Eq)]
pub enum ImplOverlapKind {
/// These impls are always allowed to overlap.
Permitted,
/// These impls are allowed to overlap, but that raises
/// an issue #33140 future-compatibility warning.
///
/// Some background: in Rust 1.0, the trait-object types `Send + Sync` (today's
/// `dyn Send + Sync`) and `Sync + Send` (now `dyn Sync + Send`) were different.
///
/// The widely-used version 0.1.0 of the crate `traitobject` had accidentally relied
/// that difference, making what reduces to the following set of impls:
///
/// ```
/// trait Trait {}
/// impl Trait for dyn Send + Sync {}
/// impl Trait for dyn Sync + Send {}
/// ```
///
/// Obviously, once we made these types be identical, that code causes a coherence
/// error and a fairly big headache for us. However, luckily for us, the trait
/// `Trait` used in this case is basically a marker trait, and therefore having
/// overlapping impls for it is sound.
///
/// To handle this, we basically regard the trait as a marker trait, with an additional
/// future-compatibility warning. To avoid accidentally "stabilizing" this feature,
/// it has the following restrictions:
///
/// 1. The trait must indeed be a marker-like trait (i.e., no items), and must be
/// positive impls.
/// 2. The trait-ref of both impls must be equal.
/// 3. The trait-ref of both impls must be a trait object type consisting only of
/// marker traits.
/// 4. Neither of the impls can have any where-clauses.
///
/// Once `traitobject` 0.1.0 is no longer an active concern, this hack can be removed.
Issue33140
}

impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> {
pub fn body_tables(self, body: hir::BodyId) -> &'gcx TypeckTables<'gcx> {
self.typeck_tables_of(self.hir().body_owner_def_id(body))
Expand Down Expand Up @@ -2788,8 +2827,10 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> {

/// Returns `true` if the impls are the same polarity and the trait either
/// has no items or is annotated #[marker] and prevents item overrides.
pub fn impls_are_allowed_to_overlap(self, def_id1: DefId, def_id2: DefId) -> bool {
if self.features().overlapping_marker_traits {
pub fn impls_are_allowed_to_overlap(self, def_id1: DefId, def_id2: DefId)
-> Option<ImplOverlapKind>
{
let is_legit = if self.features().overlapping_marker_traits {
let trait1_is_empty = self.impl_trait_ref(def_id1)
.map_or(false, |trait_ref| {
self.associated_item_def_ids(trait_ref.def_id).is_empty()
Expand All @@ -2811,6 +2852,29 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> {
&& is_marker_impl(def_id2)
} else {
false
};

if is_legit {
debug!("impls_are_allowed_to_overlap({:?}, {:?}) = Some(Permitted)",
def_id1, def_id2);
Some(ImplOverlapKind::Permitted)
} else {
if let Some(self_ty1) = self.issue33140_self_ty(def_id1) {
if let Some(self_ty2) = self.issue33140_self_ty(def_id2) {
if self_ty1 == self_ty2 {
debug!("impls_are_allowed_to_overlap({:?}, {:?}) - issue #33140 HACK",
def_id1, def_id2);
return Some(ImplOverlapKind::Issue33140);
} else {
debug!("impls_are_allowed_to_overlap({:?}, {:?}) - found {:?} != {:?}",
def_id1, def_id2, self_ty1, self_ty2);
}
}
}

debug!("impls_are_allowed_to_overlap({:?}, {:?}) = None",
def_id1, def_id2);
None
}
}

Expand Down Expand Up @@ -3203,6 +3267,59 @@ fn instance_def_size_estimate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
}
}

/// If `def_id` is an issue 33140 hack impl, return its self type. Otherwise
/// return None.
///
/// See ImplOverlapKind::Issue33140 for more details.
fn issue33140_self_ty<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
def_id: DefId)
-> Option<Ty<'tcx>>
{
debug!("issue33140_self_ty({:?})", def_id);

let trait_ref = tcx.impl_trait_ref(def_id).unwrap_or_else(|| {
bug!("issue33140_self_ty called on inherent impl {:?}", def_id)
});

debug!("issue33140_self_ty({:?}), trait-ref={:?}", def_id, trait_ref);

let is_marker_like =
tcx.impl_polarity(def_id) == hir::ImplPolarity::Positive &&
tcx.associated_item_def_ids(trait_ref.def_id).is_empty();

// Check whether these impls would be ok for a marker trait.
if !is_marker_like {
debug!("issue33140_self_ty - not marker-like!");
return None;
}

// impl must be `impl Trait for dyn Marker1 + Marker2 + ...`
if trait_ref.substs.len() != 1 {
debug!("issue33140_self_ty - impl has substs!");
return None;
}

let predicates = tcx.predicates_of(def_id);
if predicates.parent.is_some() || !predicates.predicates.is_empty() {
debug!("issue33140_self_ty - impl has predicates {:?}!", predicates);
return None;
}

let self_ty = trait_ref.self_ty();
let self_ty_matches = match self_ty.sty {
ty::Dynamic(ref data, ty::ReStatic) => data.principal().is_none(),
_ => false
};

if self_ty_matches {
debug!("issue33140_self_ty - MATCHES!");
Some(self_ty)
} else {
debug!("issue33140_self_ty - non-matching self type");
None
}
}

pub fn provide(providers: &mut ty::query::Providers<'_>) {
context::provide(providers);
erase_regions::provide(providers);
Expand All @@ -3221,6 +3338,7 @@ pub fn provide(providers: &mut ty::query::Providers<'_>) {
crate_hash,
trait_impls_of: trait_def::trait_impls_of_provider,
instance_def_size_estimate,
issue33140_self_ty,
..*providers
};
}
Expand Down
2 changes: 2 additions & 0 deletions src/librustc/ty/query/mod.rs
Expand Up @@ -202,6 +202,8 @@ define_queries! { <'tcx>

[] fn impl_trait_ref: ImplTraitRef(DefId) -> Option<ty::TraitRef<'tcx>>,
[] fn impl_polarity: ImplPolarity(DefId) -> hir::ImplPolarity,

[] fn issue33140_self_ty: Issue33140SelfTy(DefId) -> Option<ty::Ty<'tcx>>,
},

TypeChecking {
Expand Down
1 change: 1 addition & 0 deletions src/librustc/ty/query/plumbing.rs
Expand Up @@ -1275,6 +1275,7 @@ pub fn force_from_dep_node<'a, 'gcx, 'lcx>(tcx: TyCtxt<'a, 'gcx, 'lcx>,
DepKind::AdtDefOfItem => { force!(adt_def, def_id!()); }
DepKind::ImplTraitRef => { force!(impl_trait_ref, def_id!()); }
DepKind::ImplPolarity => { force!(impl_polarity, def_id!()); }
DepKind::Issue33140SelfTy => { force!(issue33140_self_ty, def_id!()); }
DepKind::FnSignature => { force!(fn_sig, def_id!()); }
DepKind::CoerceUnsizedInfo => { force!(coerce_unsized_info, def_id!()); }
DepKind::ItemVariances => { force!(variances_of, def_id!()); }
Expand Down
47 changes: 0 additions & 47 deletions src/test/run-pass/issues/issue-33140.rs

This file was deleted.

0 comments on commit 0b511b7

Please sign in to comment.