From 2fc573945afe6b1ea4846f5e8fbeb7d18be45e9d Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Fri, 26 Jan 2018 17:21:43 -0500 Subject: [PATCH 1/1] track intercrate ambiguity only when there is a coherence error --- src/librustc/traits/coherence.rs | 27 +++++--- src/librustc/traits/select.rs | 104 +++++++++++++++++++------------ 2 files changed, 82 insertions(+), 49 deletions(-) diff --git a/src/librustc/traits/coherence.rs b/src/librustc/traits/coherence.rs index 41c824a5e0e..81bec308a89 100644 --- a/src/librustc/traits/coherence.rs +++ b/src/librustc/traits/coherence.rs @@ -63,13 +63,22 @@ pub fn overlapping_impls<'gcx, F1, F2, R>( 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()) }) } @@ -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>, diff --git a/src/librustc/traits/select.rs b/src/librustc/traits/select.rs index 51d2bc8701a..87fc2356908 100644 --- a/src/librustc/traits/select.rs +++ b/src/librustc/traits/select.rs @@ -92,10 +92,10 @@ pub struct SelectionContext<'cx, 'gcx: 'cx+'tcx, 'tcx: 'cx> { inferred_obligations: SnapshotVec>, - intercrate_ambiguity_causes: Vec, + intercrate_ambiguity_causes: Option>, } -#[derive(Clone)] +#[derive(Clone, Debug)] pub enum IntercrateAmbiguityCause { DownstreamCrate { trait_desc: String, @@ -423,7 +423,7 @@ pub fn new(infcx: &'cx InferCtxt<'cx, 'gcx, 'tcx>) -> SelectionContext<'cx, 'gcx freshener: infcx.freshener(), intercrate: None, inferred_obligations: SnapshotVec::new(), - intercrate_ambiguity_causes: Vec::new(), + intercrate_ambiguity_causes: None, } } @@ -435,10 +435,30 @@ pub fn intercrate(infcx: &'cx InferCtxt<'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 { + assert!(self.intercrate.is_some()); + self.intercrate_ambiguity_causes.take().unwrap_or(vec![]) + } + pub fn infcx(&self) -> &'cx InferCtxt<'cx, 'gcx, 'tcx> { self.infcx } @@ -451,10 +471,6 @@ pub fn closure_typer(&self) -> &'cx InferCtxt<'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(&mut self, f: F) -> R @@ -828,19 +844,23 @@ fn evaluate_stack<'o>(&mut self, 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; @@ -1092,25 +1112,29 @@ fn candidate_from_obligation_no_cache<'o>(&mut self, 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); } -- 2.44.0