Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
track intercrate ambiguity only when there is a coherence error
  • Loading branch information
nikomatsakis committed Jan 31, 2018
commit 3bda277d5d48b29afc0471e504eaab08a6e29e63
27 changes: 18 additions & 9 deletions src/librustc/traits/coherence.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,13 +63,22 @@ where
impl2_def_id,
intercrate_mode);

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);
if let Some(r) = overlap(selcx, impl1_def_id, impl2_def_id) {
on_overlap(r)
} else {
no_overlap()
}
selcx.enable_tracking_intercrate_ambiguity_causes();
on_overlap(overlap(selcx, impl1_def_id, impl2_def_id).unwrap())
})
}

Expand Down Expand Up @@ -148,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