diff options
| author | Dylan DPC <dylan.dpc@gmail.com> | 2020-02-11 16:37:02 +0100 |
|---|---|---|
| committer | GitHub <noreply@github.com> | 2020-02-11 16:37:02 +0100 |
| commit | 0ba5e8a88ff05ccf4803e0bd54b3aabbc617956f (patch) | |
| tree | 47c1fc9cccbc960b4effbabe99eb11386fe96add /src | |
| parent | dc98cb09c0479710fa6d59c3f1a6e1ac962f0aa4 (diff) | |
| parent | f66793757fd1cdfa6098ec1b8532ba0382792c4d (diff) | |
Rollup merge of #69044 - jonas-schievink:dont-run-coherence-twice, r=davidtwco
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.
Diffstat (limited to 'src')
| -rw-r--r-- | src/librustc/traits/specialize/specialization_graph.rs | 73 |
1 files changed, 43 insertions, 30 deletions
diff --git a/src/librustc/traits/specialize/specialization_graph.rs b/src/librustc/traits/specialize/specialization_graph.rs index ca7740199ec..e09bcdcbc62 100644 --- a/src/librustc/traits/specialize/specialization_graph.rs +++ b/src/librustc/traits/specialize/specialization_graph.rs @@ -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`. } } |
