OpenE2K/rust
2
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Rollup merge of #69044 - jonas-schievink:dont-run-coherence-twice, r=davidtwco

Browse Source 
Don't run coherence twice for future-compat lints

This fixes the regression introduced by https://github.com/rust-lang/rust/pull/65232 (which I mentioned in https://github.com/rust-lang/rust/pull/65232#issuecomment-583739037).

Old algorithm:
* Run coherence with all future-incompatible checks off, reporting errors on any overlap.
* If there's no overlap (common case), run it *again*, with the future-incompatible checks on. Report warnings for any overlap found.

New algorithm:
* Run coherence with all additional future-incompatible checks *on*, which means that we'll find *all* potentially overlapping impls immediately.
* If this found overlap, run coherence again, with the future-incompatible checks off. If that *still* gives an error, we report it. If not, it ought to be a warning.

This reduces time spent in coherence checking for the nrf52810-pac by roughly 50% relative to current master.
This commit is contained in:
Dylan DPC 2020-02-11 16:37:02 +01:00 committed by GitHub
commit 0ba5e8a88f
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
1 changed files with 43 additions and 30 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

73
src/librustc/traits/specialize/specialization_graph.rs
View File

@ -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(),
@ -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,
});
}
@ -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)),
)?;
@ -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`.
}
}