Skip to content

Commit

Permalink
Don't run coherence twice for future-compat lints
Browse files Browse the repository at this point in the history
  • Loading branch information
jonas-schievink committed Feb 10, 2020
1 parent e6ec0d1 commit f667937
Showing 1 changed file with 43 additions and 30 deletions.
73 changes: 43 additions & 30 deletions src/librustc/traits/specialize/specialization_graph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -86,10 +86,10 @@ impl<'tcx> Children {
impl_def_id, simplified_self, possible_sibling,
);

let overlap_error = |overlap: traits::coherence::OverlapResult<'_>| {
// Found overlap, but no specialization; error out.
let create_overlap_error = |overlap: traits::coherence::OverlapResult<'_>| {
let trait_ref = overlap.impl_header.trait_ref.unwrap();
let self_ty = trait_ref.self_ty();

OverlapError {
with_impl: possible_sibling,
trait_desc: trait_ref.print_only_trait_path().to_string(),
Expand All @@ -106,21 +106,49 @@ impl<'tcx> Children {
}
};

let allowed_to_overlap =
tcx.impls_are_allowed_to_overlap(impl_def_id, possible_sibling);
let report_overlap_error = |overlap: traits::coherence::OverlapResult<'_>,
last_lint: &mut _| {
// Found overlap, but no specialization; error out or report future-compat warning.

// Do we *still* get overlap if we disable the future-incompatible modes?
let should_err = traits::overlapping_impls(
tcx,
possible_sibling,
impl_def_id,
traits::SkipLeakCheck::default(),
|_| true,
|| false,
);

let error = create_overlap_error(overlap);

if should_err {
Err(error)
} else {
*last_lint = Some(FutureCompatOverlapError {
error,
kind: FutureCompatOverlapErrorKind::LeakCheck,
});

Ok((false, false))
}
};

let last_lint_mut = &mut last_lint;
let (le, ge) = traits::overlapping_impls(
tcx,
possible_sibling,
impl_def_id,
traits::SkipLeakCheck::default(),
traits::SkipLeakCheck::Yes,
|overlap| {
if let Some(overlap_kind) = &allowed_to_overlap {
if let Some(overlap_kind) =
tcx.impls_are_allowed_to_overlap(impl_def_id, possible_sibling)
{
match overlap_kind {
ty::ImplOverlapKind::Permitted { marker: _ } => {}
ty::ImplOverlapKind::Issue33140 => {
last_lint = Some(FutureCompatOverlapError {
error: overlap_error(overlap),
*last_lint_mut = Some(FutureCompatOverlapError {
error: create_overlap_error(overlap),
kind: FutureCompatOverlapErrorKind::Issue33140,
});
}
Expand All @@ -132,7 +160,11 @@ impl<'tcx> Children {
let le = tcx.specializes((impl_def_id, possible_sibling));
let ge = tcx.specializes((possible_sibling, impl_def_id));

if le == ge { Err(overlap_error(overlap)) } else { Ok((le, ge)) }
if le == ge {
report_overlap_error(overlap, last_lint_mut)
} else {
Ok((le, ge))
}
},
|| Ok((false, false)),
)?;
Expand All @@ -153,27 +185,8 @@ impl<'tcx> Children {

replace_children.push(possible_sibling);
} else {
if let None = allowed_to_overlap {
// Do future-compat checks for overlap.

if last_lint.is_none() {
traits::overlapping_impls(
tcx,
possible_sibling,
impl_def_id,
traits::SkipLeakCheck::Yes,
|overlap| {
last_lint = Some(FutureCompatOverlapError {
error: overlap_error(overlap),
kind: FutureCompatOverlapErrorKind::LeakCheck,
});
},
|| (),
);
}
}

// no overlap (error bailed already via ?)
// Either there's no overlap, or the overlap was already reported by
// `overlap_error`.
}
}

Expand Down

0 comments on commit f667937

Please sign in to comment.