Skip to content

remove intercrate ambiguity hints #47738

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Feb 1, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
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
1 change: 1 addition & 0 deletions src/librustc/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,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