From f66793757fd1cdfa6098ec1b8532ba0382792c4d Mon Sep 17 00:00:00 2001
From: Jonas Schievink <jonasschievink@gmail.com>
Date: Tue, 11 Feb 2020 00:17:47 +0100
Subject: [PATCH] Don't run coherence twice for future-compat lints

---
 .../traits/specialize/specialization_graph.rs | 73 +++++++++++--------
 1 file 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`.
             }
         }