Skip to content

Commit

Permalink
Auto merge of #47918 - nikomatsakis:issue-47139-beta, r=eddyb
Browse files Browse the repository at this point in the history
Backport 47738 to beta

Backport of #47738 to beta branch.
  • Loading branch information
bors committed Feb 2, 2018
2 parents ed9751a + 3bda277 commit 61bc3af
Show file tree
Hide file tree
Showing 7 changed files with 307 additions and 93 deletions.
1 change: 1 addition & 0 deletions src/librustc/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@
#![feature(specialization)]
#![feature(unboxed_closures)]
#![feature(underscore_lifetimes)]
#![feature(universal_impl_trait)]
#![feature(trace_macros)]
#![feature(catch_expr)]
#![feature(test)]
Expand Down
50 changes: 36 additions & 14 deletions src/librustc/traits/coherence.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ use ty::{self, Ty, TyCtxt};
use ty::fold::TypeFoldable;
use ty::subst::Subst;

use infer::{InferCtxt, InferOk};
use infer::{InferOk};

/// Whether we do the orphan check relative to this crate or
/// to some remote crate.
Expand All @@ -40,13 +40,20 @@ pub struct OverlapResult<'tcx> {
pub intercrate_ambiguity_causes: Vec<IntercrateAmbiguityCause>,
}

/// If there are types that satisfy both impls, returns a suitably-freshened
/// `ImplHeader` with those types substituted
pub fn overlapping_impls<'cx, 'gcx, 'tcx>(infcx: &InferCtxt<'cx, 'gcx, 'tcx>,
impl1_def_id: DefId,
impl2_def_id: DefId,
intercrate_mode: IntercrateMode)
-> Option<OverlapResult<'tcx>>
/// If there are types that satisfy both impls, invokes `on_overlap`
/// with a suitably-freshened `ImplHeader` with those types
/// substituted. Otherwise, invokes `no_overlap`.
pub fn overlapping_impls<'gcx, F1, F2, R>(
tcx: TyCtxt<'_, 'gcx, 'gcx>,
impl1_def_id: DefId,
impl2_def_id: DefId,
intercrate_mode: IntercrateMode,
on_overlap: F1,
no_overlap: F2,
) -> R
where
F1: FnOnce(OverlapResult<'_>) -> R,
F2: FnOnce() -> R,
{
debug!("impl_can_satisfy(\
impl1_def_id={:?}, \
Expand All @@ -56,8 +63,23 @@ pub fn overlapping_impls<'cx, 'gcx, 'tcx>(infcx: &InferCtxt<'cx, 'gcx, 'tcx>,
impl2_def_id,
intercrate_mode);

let selcx = &mut SelectionContext::intercrate(infcx, intercrate_mode);
overlap(selcx, impl1_def_id, impl2_def_id)
let overlaps = tcx.infer_ctxt().enter(|infcx| {
let selcx = &mut SelectionContext::intercrate(&infcx, intercrate_mode);
overlap(selcx, impl1_def_id, impl2_def_id).is_some()
});

if !overlaps {
return no_overlap();
}

// In the case where we detect an error, run the check again, but
// this time tracking intercrate ambuiguity causes for better
// diagnostics. (These take time and can lead to false errors.)
tcx.infer_ctxt().enter(|infcx| {
let selcx = &mut SelectionContext::intercrate(&infcx, intercrate_mode);
selcx.enable_tracking_intercrate_ambiguity_causes();
on_overlap(overlap(selcx, impl1_def_id, impl2_def_id).unwrap())
})
}

fn with_fresh_ty_vars<'cx, 'gcx, 'tcx>(selcx: &mut SelectionContext<'cx, 'gcx, 'tcx>,
Expand Down Expand Up @@ -135,10 +157,10 @@ fn overlap<'cx, 'gcx, 'tcx>(selcx: &mut SelectionContext<'cx, 'gcx, 'tcx>,
return None
}

Some(OverlapResult {
impl_header: selcx.infcx().resolve_type_vars_if_possible(&a_impl_header),
intercrate_ambiguity_causes: selcx.intercrate_ambiguity_causes().to_vec(),
})
let impl_header = selcx.infcx().resolve_type_vars_if_possible(&a_impl_header);
let intercrate_ambiguity_causes = selcx.take_intercrate_ambiguity_causes();
debug!("overlap: intercrate_ambiguity_causes={:#?}", intercrate_ambiguity_causes);
Some(OverlapResult { impl_header, intercrate_ambiguity_causes })
}

pub fn trait_ref_is_knowable<'a, 'gcx, 'tcx>(tcx: TyCtxt<'a, 'gcx, 'tcx>,
Expand Down
104 changes: 64 additions & 40 deletions src/librustc/traits/select.rs
Original file line number Diff line number Diff line change
Expand Up @@ -92,10 +92,10 @@ pub struct SelectionContext<'cx, 'gcx: 'cx+'tcx, 'tcx: 'cx> {

inferred_obligations: SnapshotVec<InferredObligationsSnapshotVecDelegate<'tcx>>,

intercrate_ambiguity_causes: Vec<IntercrateAmbiguityCause>,
intercrate_ambiguity_causes: Option<Vec<IntercrateAmbiguityCause>>,
}

#[derive(Clone)]
#[derive(Clone, Debug)]
pub enum IntercrateAmbiguityCause {
DownstreamCrate {
trait_desc: String,
Expand Down Expand Up @@ -423,7 +423,7 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> {
freshener: infcx.freshener(),
intercrate: None,
inferred_obligations: SnapshotVec::new(),
intercrate_ambiguity_causes: Vec::new(),
intercrate_ambiguity_causes: None,
}
}

Expand All @@ -435,10 +435,30 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> {
freshener: infcx.freshener(),
intercrate: Some(mode),
inferred_obligations: SnapshotVec::new(),
intercrate_ambiguity_causes: Vec::new(),
intercrate_ambiguity_causes: None,
}
}

/// Enables tracking of intercrate ambiguity causes. These are
/// used in coherence to give improved diagnostics. We don't do
/// this until we detect a coherence error because it can lead to
/// false overflow results (#47139) and because it costs
/// computation time.
pub fn enable_tracking_intercrate_ambiguity_causes(&mut self) {
assert!(self.intercrate.is_some());
assert!(self.intercrate_ambiguity_causes.is_none());
self.intercrate_ambiguity_causes = Some(vec![]);
debug!("selcx: enable_tracking_intercrate_ambiguity_causes");
}

/// Gets the intercrate ambiguity causes collected since tracking
/// was enabled and disables tracking at the same time. If
/// tracking is not enabled, just returns an empty vector.
pub fn take_intercrate_ambiguity_causes(&mut self) -> Vec<IntercrateAmbiguityCause> {
assert!(self.intercrate.is_some());
self.intercrate_ambiguity_causes.take().unwrap_or(vec![])
}

pub fn infcx(&self) -> &'cx InferCtxt<'cx, 'gcx, 'tcx> {
self.infcx
}
Expand All @@ -451,10 +471,6 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> {
self.infcx
}

pub fn intercrate_ambiguity_causes(&self) -> &[IntercrateAmbiguityCause] {
&self.intercrate_ambiguity_causes
}

/// Wraps the inference context's in_snapshot s.t. snapshot handling is only from the selection
/// context's self.
fn in_snapshot<R, F>(&mut self, f: F) -> R
Expand Down Expand Up @@ -828,19 +844,23 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> {
debug!("evaluate_stack({:?}) --> unbound argument, intercrate --> ambiguous",
stack.fresh_trait_ref);
// Heuristics: show the diagnostics when there are no candidates in crate.
if let Ok(candidate_set) = self.assemble_candidates(stack) {
if !candidate_set.ambiguous && candidate_set.vec.is_empty() {
let trait_ref = stack.obligation.predicate.skip_binder().trait_ref;
let self_ty = trait_ref.self_ty();
let cause = IntercrateAmbiguityCause::DownstreamCrate {
trait_desc: trait_ref.to_string(),
self_desc: if self_ty.has_concrete_skeleton() {
Some(self_ty.to_string())
} else {
None
},
};
self.intercrate_ambiguity_causes.push(cause);
if self.intercrate_ambiguity_causes.is_some() {
debug!("evaluate_stack: intercrate_ambiguity_causes is some");
if let Ok(candidate_set) = self.assemble_candidates(stack) {
if !candidate_set.ambiguous && candidate_set.vec.is_empty() {
let trait_ref = stack.obligation.predicate.skip_binder().trait_ref;
let self_ty = trait_ref.self_ty();
let cause = IntercrateAmbiguityCause::DownstreamCrate {
trait_desc: trait_ref.to_string(),
self_desc: if self_ty.has_concrete_skeleton() {
Some(self_ty.to_string())
} else {
None
},
};
debug!("evaluate_stack: pushing cause = {:?}", cause);
self.intercrate_ambiguity_causes.as_mut().unwrap().push(cause);
}
}
}
return EvaluatedToAmbig;
Expand Down Expand Up @@ -1092,25 +1112,29 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> {
None => {}
Some(conflict) => {
debug!("coherence stage: not knowable");
// Heuristics: show the diagnostics when there are no candidates in crate.
let candidate_set = self.assemble_candidates(stack)?;
if !candidate_set.ambiguous && candidate_set.vec.iter().all(|c| {
!self.evaluate_candidate(stack, &c).may_apply()
}) {
let trait_ref = stack.obligation.predicate.skip_binder().trait_ref;
let self_ty = trait_ref.self_ty();
let trait_desc = trait_ref.to_string();
let self_desc = if self_ty.has_concrete_skeleton() {
Some(self_ty.to_string())
} else {
None
};
let cause = if let Conflict::Upstream = conflict {
IntercrateAmbiguityCause::UpstreamCrateUpdate { trait_desc, self_desc }
} else {
IntercrateAmbiguityCause::DownstreamCrate { trait_desc, self_desc }
};
self.intercrate_ambiguity_causes.push(cause);
if self.intercrate_ambiguity_causes.is_some() {
debug!("evaluate_stack: intercrate_ambiguity_causes is some");
// Heuristics: show the diagnostics when there are no candidates in crate.
let candidate_set = self.assemble_candidates(stack)?;
if !candidate_set.ambiguous && candidate_set.vec.iter().all(|c| {
!self.evaluate_candidate(stack, &c).may_apply()
}) {
let trait_ref = stack.obligation.predicate.skip_binder().trait_ref;
let self_ty = trait_ref.self_ty();
let trait_desc = trait_ref.to_string();
let self_desc = if self_ty.has_concrete_skeleton() {
Some(self_ty.to_string())
} else {
None
};
let cause = if let Conflict::Upstream = conflict {
IntercrateAmbiguityCause::UpstreamCrateUpdate { trait_desc, self_desc }
} else {
IntercrateAmbiguityCause::DownstreamCrate { trait_desc, self_desc }
};
debug!("evaluate_stack: pushing cause = {:?}", cause);
self.intercrate_ambiguity_causes.as_mut().unwrap().push(cause);
}
}
return Ok(None);
}
Expand Down
37 changes: 17 additions & 20 deletions src/librustc/traits/specialize/specialization_graph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -133,12 +133,12 @@ impl<'a, 'gcx, 'tcx> Children {
};

let tcx = tcx.global_tcx();
let (le, ge) = tcx.infer_ctxt().enter(|infcx| {
let overlap = traits::overlapping_impls(&infcx,
possible_sibling,
impl_def_id,
traits::IntercrateMode::Issue43355);
if let Some(overlap) = overlap {
let (le, ge) = traits::overlapping_impls(
tcx,
possible_sibling,
impl_def_id,
traits::IntercrateMode::Issue43355,
|overlap| {
if tcx.impls_are_allowed_to_overlap(impl_def_id, possible_sibling) {
return Ok((false, false));
}
Expand All @@ -151,10 +151,9 @@ impl<'a, 'gcx, 'tcx> Children {
} else {
Ok((le, ge))
}
} else {
Ok((false, false))
}
})?;
},
|| Ok((false, false)),
)?;

if le && !ge {
debug!("descending as child of TraitRef {:?}",
Expand All @@ -171,16 +170,14 @@ impl<'a, 'gcx, 'tcx> Children {
return Ok(Inserted::Replaced(possible_sibling));
} else {
if !tcx.impls_are_allowed_to_overlap(impl_def_id, possible_sibling) {
tcx.infer_ctxt().enter(|infcx| {
if let Some(overlap) = traits::overlapping_impls(
&infcx,
possible_sibling,
impl_def_id,
traits::IntercrateMode::Fixed)
{
last_lint = Some(overlap_error(overlap));
}
});
traits::overlapping_impls(
tcx,
possible_sibling,
impl_def_id,
traits::IntercrateMode::Fixed,
|overlap| last_lint = Some(overlap_error(overlap)),
|| (),
);
}

// no overlap (error bailed already via ?)
Expand Down
46 changes: 27 additions & 19 deletions src/librustc_typeck/coherence/inherent_impls_overlap.rs
Original file line number Diff line number Diff line change
Expand Up @@ -82,29 +82,37 @@ impl<'a, 'tcx> InherentOverlapChecker<'a, 'tcx> {

for (i, &impl1_def_id) in impls.iter().enumerate() {
for &impl2_def_id in &impls[(i + 1)..] {
let used_to_be_allowed = self.tcx.infer_ctxt().enter(|infcx| {
if let Some(overlap) =
traits::overlapping_impls(&infcx, impl1_def_id, impl2_def_id,
IntercrateMode::Issue43355)
{
let used_to_be_allowed = traits::overlapping_impls(
self.tcx,
impl1_def_id,
impl2_def_id,
IntercrateMode::Issue43355,
|overlap| {
self.check_for_common_items_in_impls(
impl1_def_id, impl2_def_id, overlap, false);
impl1_def_id,
impl2_def_id,
overlap,
false,
);
false
} else {
true
}
});
},
|| true,
);

if used_to_be_allowed {
self.tcx.infer_ctxt().enter(|infcx| {
if let Some(overlap) =
traits::overlapping_impls(&infcx, impl1_def_id, impl2_def_id,
IntercrateMode::Fixed)
{
self.check_for_common_items_in_impls(
impl1_def_id, impl2_def_id, overlap, true);
}
});
traits::overlapping_impls(
self.tcx,
impl1_def_id,
impl2_def_id,
IntercrateMode::Fixed,
|overlap| self.check_for_common_items_in_impls(
impl1_def_id,
impl2_def_id,
overlap,
true,
),
|| (),
);
}
}
}
Expand Down
Loading

0 comments on commit 61bc3af

Please sign in to comment.