Auto merge of #47738 - nikomatsakis:issue-47139-master, r=arielb1

remove intercrate ambiguity hints

The scheme was causing overflows during coherence checking (e.g. #47139). This is sort of a temporary fix; the proper fix I think involves reworking trait selection in deeper ways.

cc @sgrif -- this *should* fix diesel

cc @qnighy -- I'd like to discuss you with alternative techniques for achieving the same end. =) Actually, it might be good to put some energy into refactoring traits first.

r? @eddyb

src/librustc/lib.rs

@@ -66,6 +66,7 @@
 #![feature(specialization)]
 #![feature(unboxed_closures)]
 #![feature(underscore_lifetimes)]
+#![feature(universal_impl_trait)]
 #![feature(trace_macros)]
 #![feature(catch_expr)]
 #![feature(test)]

src/librustc/traits/coherence.rs

@@ -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.
@@ -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={:?}, \
@@ -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>,
@@ -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>,

src/librustc/traits/select.rs

@@ -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,
@@ -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,
         }
     }
 
@@ -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
     }
@@ -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
@@ -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;
@@ -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);
             }

src/librustc/traits/specialize/specialization_graph.rs

@@ -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));
                     }
@@ -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 {:?}",
@@ -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 ?)

src/librustc_typeck/coherence/inherent_impls_overlap.rs

@@ -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,
+                        ),
+                        || (),
+                    );
                 }
             }
         }