diff --git a/src/librustc/lib.rs b/src/librustc/lib.rs index becaf78f7eca5..550daf0f172d0 100644 --- a/src/librustc/lib.rs +++ b/src/librustc/lib.rs @@ -64,6 +64,7 @@ #![feature(specialization)] #![feature(unboxed_closures)] #![feature(underscore_lifetimes)] +#![feature(universal_impl_trait)] #![feature(trace_macros)] #![feature(catch_expr)] #![feature(test)] diff --git a/src/librustc/traits/coherence.rs b/src/librustc/traits/coherence.rs index 7d1f3b31bfc27..81bec308a8981 100644 --- a/src/librustc/traits/coherence.rs +++ b/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, } -/// 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> +/// 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>, diff --git a/src/librustc/traits/select.rs b/src/librustc/traits/select.rs index 6482ecc7ee161..9d507c4fd9ccb 100644 --- a/src/librustc/traits/select.rs +++ b/src/librustc/traits/select.rs @@ -92,10 +92,10 @@ pub struct SelectionContext<'cx, 'gcx: 'cx+'tcx, 'tcx: 'cx> { inferred_obligations: SnapshotVec>, - intercrate_ambiguity_causes: Vec, + intercrate_ambiguity_causes: Option>, } -#[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 { + 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(&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); } diff --git a/src/librustc/traits/specialize/specialization_graph.rs b/src/librustc/traits/specialize/specialization_graph.rs index 834389e5d009c..a10169e13e60a 100644 --- a/src/librustc/traits/specialize/specialization_graph.rs +++ b/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 ?) diff --git a/src/librustc_typeck/coherence/inherent_impls_overlap.rs b/src/librustc_typeck/coherence/inherent_impls_overlap.rs index 07d5f813cbbce..88a2dc817ae63 100644 --- a/src/librustc_typeck/coherence/inherent_impls_overlap.rs +++ b/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, + ), + || (), + ); } } } diff --git a/src/test/run-pass/issue-47139-1.rs b/src/test/run-pass/issue-47139-1.rs new file mode 100644 index 0000000000000..cb87991a491db --- /dev/null +++ b/src/test/run-pass/issue-47139-1.rs @@ -0,0 +1,87 @@ +// Copyright 2016 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// Regression test for issue #47139: +// +// Coherence was encountering an (unnecessary) overflow trying to +// decide if the two impls of dummy overlap. +// +// The overflow went something like: +// +// - `&'a ?T: Insertable` ? +// - let ?T = Option ? +// - `Option: Insertable` ? +// - `Option<&'a ?U>: Insertable` ? +// - `&'a ?U: Insertable` ? +// +// While somewhere in the middle, a projection would occur, which +// broke cycle detection. +// +// It turned out that this cycle was being kicked off due to some +// extended diagnostic attempts in coherence, so removing those +// sidestepped the issue for now. + +#![allow(dead_code)] + +pub trait Insertable { + type Values; + + fn values(self) -> Self::Values; +} + +impl Insertable for Option + where + T: Insertable, + T::Values: Default, +{ + type Values = T::Values; + + fn values(self) -> Self::Values { + self.map(Insertable::values).unwrap_or_default() + } +} + +impl<'a, T> Insertable for &'a Option + where + Option<&'a T>: Insertable, +{ + type Values = as Insertable>::Values; + + fn values(self) -> Self::Values { + self.as_ref().values() + } +} + +impl<'a, T> Insertable for &'a [T] +{ + type Values = Self; + + fn values(self) -> Self::Values { + self + } +} + +trait Unimplemented { } + +trait Dummy { } + +struct Foo { t: T } + +impl<'a, U> Dummy for Foo<&'a U> + where &'a U: Insertable +{ +} + +impl Dummy for T + where T: Unimplemented +{ } + +fn main() { +} diff --git a/src/test/run-pass/issue-47139-2.rs b/src/test/run-pass/issue-47139-2.rs new file mode 100644 index 0000000000000..08eaee5acd730 --- /dev/null +++ b/src/test/run-pass/issue-47139-2.rs @@ -0,0 +1,75 @@ +// Copyright 2016 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// Regression test for issue #47139: +// +// Same as issue-47139-1.rs, but the impls of dummy are in the +// opposite order. This influenced the way that coherence ran and in +// some cases caused the overflow to occur when it wouldn't otherwise. +// In an effort to make the regr test more robust, I am including both +// orderings. + +#![allow(dead_code)] + +pub trait Insertable { + type Values; + + fn values(self) -> Self::Values; +} + +impl Insertable for Option + where + T: Insertable, + T::Values: Default, +{ + type Values = T::Values; + + fn values(self) -> Self::Values { + self.map(Insertable::values).unwrap_or_default() + } +} + +impl<'a, T> Insertable for &'a Option + where + Option<&'a T>: Insertable, +{ + type Values = as Insertable>::Values; + + fn values(self) -> Self::Values { + self.as_ref().values() + } +} + +impl<'a, T> Insertable for &'a [T] +{ + type Values = Self; + + fn values(self) -> Self::Values { + self + } +} + +trait Unimplemented { } + +trait Dummy { } + +struct Foo { t: T } + +impl Dummy for T + where T: Unimplemented +{ } + +impl<'a, U> Dummy for Foo<&'a U> + where &'a U: Insertable +{ +} + +fn main() { +}