From b4a6945880392930e43d32993427de8fe8cdc38f Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Sat, 25 Aug 2018 00:32:41 +0200 Subject: [PATCH 1/5] Replace bug! call with Overflow --- src/librustc/traits/structural_impls.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/librustc/traits/structural_impls.rs b/src/librustc/traits/structural_impls.rs index 10e930d1c92d9..6b0b1c35a7e83 100644 --- a/src/librustc/traits/structural_impls.rs +++ b/src/librustc/traits/structural_impls.rs @@ -175,7 +175,7 @@ impl<'a, 'tcx> Lift<'tcx> for traits::SelectionError<'a> { super::ConstEvalFailure(ref err) => tcx.lift(&**err).map(|err| super::ConstEvalFailure( err.into(), )), - super::Overflow => bug!(), // FIXME: ape ConstEvalFailure? + super::Overflow => Some(super::Overflow), } } } From 7cb74d5a8567f92864d452b16cd73ab5cb872e25 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Thu, 13 Sep 2018 13:40:15 -0400 Subject: [PATCH 2/5] give a more informative failure in this case --- src/librustc/traits/query/evaluate_obligation.rs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/librustc/traits/query/evaluate_obligation.rs b/src/librustc/traits/query/evaluate_obligation.rs index 6bd9267836254..1a906b5da6f32 100644 --- a/src/librustc/traits/query/evaluate_obligation.rs +++ b/src/librustc/traits/query/evaluate_obligation.rs @@ -51,7 +51,14 @@ impl<'cx, 'gcx, 'tcx> InferCtxt<'cx, 'gcx, 'tcx> { let mut selcx = SelectionContext::with_query_mode(&self, TraitQueryMode::Standard); selcx.evaluate_obligation_recursively(obligation) - .expect("Overflow should be caught earlier in standard query mode") + .unwrap_or_else(|r| { + span_bug!( + obligation.cause.span, + "Overflow should be caught earlier in standard query mode: {:?}, {:?}", + obligation, + r, + ) + }) } } } From 15bab9470209797c61529bb4cbb59c7bb74caa99 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Thu, 13 Sep 2018 13:40:25 -0400 Subject: [PATCH 3/5] don't cache overflow results globally --- src/librustc/traits/select.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/librustc/traits/select.rs b/src/librustc/traits/select.rs index 232ef108537fe..706d038812e0e 100644 --- a/src/librustc/traits/select.rs +++ b/src/librustc/traits/select.rs @@ -1376,7 +1376,10 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> { let tcx = self.tcx(); let trait_ref = cache_fresh_trait_pred.skip_binder().trait_ref; if self.can_use_global_caches(param_env) { - if let Some(trait_ref) = tcx.lift_to_global(&trait_ref) { + if let Err(Overflow) = candidate { + // Don't cache overflow globally; we only produce this + // in certain modes. + } else if let Some(trait_ref) = tcx.lift_to_global(&trait_ref) { if let Some(candidate) = tcx.lift_to_global(&candidate) { debug!( "insert_candidate_cache(trait_ref={:?}, candidate={:?}) global", From 0150fe179641cc44265a434843eb8b1c42615d90 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Thu, 13 Sep 2018 14:59:01 -0400 Subject: [PATCH 4/5] expose `evaluate_obligation` that captures overflow, use in rustdoc --- .../traits/query/evaluate_obligation.rs | 23 +++++++++++++------ src/librustdoc/clean/blanket_impl.rs | 19 +++++++++++---- 2 files changed, 30 insertions(+), 12 deletions(-) diff --git a/src/librustc/traits/query/evaluate_obligation.rs b/src/librustc/traits/query/evaluate_obligation.rs index 1a906b5da6f32..f573b1ef45e9c 100644 --- a/src/librustc/traits/query/evaluate_obligation.rs +++ b/src/librustc/traits/query/evaluate_obligation.rs @@ -20,7 +20,7 @@ impl<'cx, 'gcx, 'tcx> InferCtxt<'cx, 'gcx, 'tcx> { &self, obligation: &PredicateObligation<'tcx>, ) -> bool { - self.evaluate_obligation(obligation).may_apply() + self.evaluate_obligation_no_overflow(obligation).may_apply() } /// Evaluates whether the predicate can be satisfied in the given @@ -30,22 +30,31 @@ impl<'cx, 'gcx, 'tcx> InferCtxt<'cx, 'gcx, 'tcx> { &self, obligation: &PredicateObligation<'tcx>, ) -> bool { - self.evaluate_obligation(obligation) == EvaluationResult::EvaluatedToOk + self.evaluate_obligation_no_overflow(obligation) == EvaluationResult::EvaluatedToOk } - // Helper function that canonicalizes and runs the query, as well as handles - // overflow. - fn evaluate_obligation( + /// Evaluate a given predicate, capturing overflow and propagating it back. + pub fn evaluate_obligation( &self, obligation: &PredicateObligation<'tcx>, - ) -> EvaluationResult { + ) -> Result { let mut _orig_values = SmallVec::new(); let c_pred = self.canonicalize_query(&obligation.param_env.and(obligation.predicate), &mut _orig_values); // Run canonical query. If overflow occurs, rerun from scratch but this time // in standard trait query mode so that overflow is handled appropriately // within `SelectionContext`. - match self.tcx.global_tcx().evaluate_obligation(c_pred) { + self.tcx.global_tcx().evaluate_obligation(c_pred) + } + + // Helper function that canonicalizes and runs the query. If an + // overflow results, we re-run it in the local context so we can + // report a nice error. + fn evaluate_obligation_no_overflow( + &self, + obligation: &PredicateObligation<'tcx>, + ) -> EvaluationResult { + match self.evaluate_obligation(obligation) { Ok(result) => result, Err(OverflowError) => { let mut selcx = diff --git a/src/librustdoc/clean/blanket_impl.rs b/src/librustdoc/clean/blanket_impl.rs index e7e371cd56785..3d591a702aaa9 100644 --- a/src/librustdoc/clean/blanket_impl.rs +++ b/src/librustdoc/clean/blanket_impl.rs @@ -103,11 +103,20 @@ impl<'a, 'tcx, 'rcx, 'cstore> BlanketImplFinder <'a, 'tcx, 'rcx, 'cstore> { // FIXME(eddyb) ignoring `obligations` might cause false positives. drop(obligations); - let may_apply = infcx.predicate_may_hold(&traits::Obligation::new( - cause.clone(), - param_env, - trait_ref.to_predicate(), - )); + debug!( + "invoking predicate_may_hold: {:?}", + trait_ref, + ); + let may_apply = match infcx.evaluate_obligation( + &traits::Obligation::new( + cause.clone(), + param_env, + trait_ref.to_predicate(), + ), + ) { + Ok(eval_result) => eval_result.may_apply(), + Err(traits::OverflowError) => true, // overflow doesn't mean yes *or* no + }; if !may_apply { return } From ffaf7ee3a5c65062af1b808746142e16f76eebc8 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Tue, 25 Sep 2018 13:54:51 -0400 Subject: [PATCH 5/5] add regression test --- src/test/rustdoc/issue-52873.rs | 171 ++++++++++++++++++++++++++++++++ 1 file changed, 171 insertions(+) create mode 100644 src/test/rustdoc/issue-52873.rs diff --git a/src/test/rustdoc/issue-52873.rs b/src/test/rustdoc/issue-52873.rs new file mode 100644 index 0000000000000..9138dd50defa0 --- /dev/null +++ b/src/test/rustdoc/issue-52873.rs @@ -0,0 +1,171 @@ +// Regression test for #52873. We used to ICE due to unexpected +// overflows when checking for "blanket impl inclusion". + +use std::marker::PhantomData; +use std::cmp::Ordering; +use std::ops::{Add, Mul}; + +pub type True = B1; +pub type False = B0; +pub type U0 = UTerm; +pub type U1 = UInt; + +pub trait NonZero {} + +pub trait Bit { +} + +pub trait Unsigned { +} + +#[derive(Eq, PartialEq, Ord, PartialOrd, Clone, Copy, Hash, Debug, Default)] +pub struct B0; + +impl B0 { + #[inline] + pub fn new() -> B0 { + B0 + } +} + +#[derive(Eq, PartialEq, Ord, PartialOrd, Clone, Copy, Hash, Debug, Default)] +pub struct B1; + +impl B1 { + #[inline] + pub fn new() -> B1 { + B1 + } +} + +impl Bit for B0 { +} + +impl Bit for B1 { +} + +impl NonZero for B1 {} + +pub trait PrivatePow { + type Output; +} +pub type PrivatePowOut = >::Output; + +pub type Add1 = >::Output; +pub type Prod = >::Output; +pub type Square = ::Output; +pub type Sum = >::Output; + +#[derive(Eq, PartialEq, Ord, PartialOrd, Clone, Copy, Hash, Debug, Default)] +pub struct UTerm; + +impl UTerm { + #[inline] + pub fn new() -> UTerm { + UTerm + } +} + +impl Unsigned for UTerm { +} + +#[derive(Eq, PartialEq, Ord, PartialOrd, Clone, Copy, Hash, Debug, Default)] +pub struct UInt { + _marker: PhantomData<(U, B)>, +} + +impl UInt { + #[inline] + pub fn new() -> UInt { + UInt { + _marker: PhantomData, + } + } +} + +impl Unsigned for UInt { +} + +impl NonZero for UInt {} + +impl Add for UTerm { + type Output = UTerm; + fn add(self, _: B0) -> Self::Output { + UTerm + } +} + +impl Add for UInt { + type Output = UInt; + fn add(self, _: B0) -> Self::Output { + UInt::new() + } +} + +impl Add for UTerm { + type Output = U; + fn add(self, _: U) -> Self::Output { + unsafe { ::std::mem::uninitialized() } + } +} + +impl Mul for UInt { + type Output = UTerm; + fn mul(self, _: B0) -> Self::Output { + UTerm + } +} + +impl Mul for UInt { + type Output = UInt; + fn mul(self, _: B1) -> Self::Output { + UInt::new() + } +} + +impl Mul for UTerm { + type Output = UTerm; + fn mul(self, _: U) -> Self::Output { + UTerm + } +} + +impl Mul> for UInt +where + Ul: Mul>, +{ + type Output = UInt>, B0>; + fn mul(self, _: UInt) -> Self::Output { + unsafe { ::std::mem::uninitialized() } + } +} + +pub trait Pow { + type Output; +} + +impl Pow for X +where + X: PrivatePow, +{ + type Output = PrivatePowOut; +} + +impl PrivatePow for X { + type Output = Y; +} + +impl PrivatePow for X +where + X: Mul, +{ + type Output = Prod; +} + +impl PrivatePow, B0>> for X +where + X: Mul, + Square: PrivatePow>, +{ + type Output = PrivatePowOut, Y, UInt>; +}