From fe7ccf0761180b8e879eec0cfee305a67af96e9d Mon Sep 17 00:00:00 2001 From: Ariel Ben-Yehuda Date: Sat, 2 Jul 2016 02:11:00 +0300 Subject: [PATCH 1/4] fail obligations that depend on erroring obligations Fix a bug where an obligation that depend on an erroring obligation would be regarded as successful, leading to global cache pollution and random lossage. Fixes #33723. Fixes #34503. --- .../obligation_forest/mod.rs | 35 ++++++++++++---- .../obligation_forest/test.rs | 40 +++++++++++++++++++ src/test/run-pass/issue-34503.rs | 20 ++++++++++ 3 files changed, 87 insertions(+), 8 deletions(-) create mode 100644 src/test/run-pass/issue-34503.rs diff --git a/src/librustc_data_structures/obligation_forest/mod.rs b/src/librustc_data_structures/obligation_forest/mod.rs index b713b2285a65f..c079146edbf42 100644 --- a/src/librustc_data_structures/obligation_forest/mod.rs +++ b/src/librustc_data_structures/obligation_forest/mod.rs @@ -208,11 +208,17 @@ impl ObligationForest { /// /// This CAN be done in a snapshot pub fn register_obligation(&mut self, obligation: O) { - self.register_obligation_at(obligation, None) + // Ignore errors here - there is no guarantee of success. + let _ = self.register_obligation_at(obligation, None); } - fn register_obligation_at(&mut self, obligation: O, parent: Option) { - if self.done_cache.contains(obligation.as_predicate()) { return } + // returns Err(()) if we already know this obligation failed. + fn register_obligation_at(&mut self, obligation: O, parent: Option) + -> Result<(), ()> + { + if self.done_cache.contains(obligation.as_predicate()) { + return Ok(()) + } match self.waiting_cache.entry(obligation.as_predicate().clone()) { Entry::Occupied(o) => { @@ -226,6 +232,11 @@ impl ObligationForest { self.nodes[o.get().get()].dependents.push(parent); } } + if let NodeState::Error = self.nodes[o.get().get()].state.get() { + Err(()) + } else { + Ok(()) + } } Entry::Vacant(v) => { debug!("register_obligation_at({:?}, {:?}) - ok", @@ -233,8 +244,9 @@ impl ObligationForest { v.insert(NodeIndex::new(self.nodes.len())); self.cache_list.push(obligation.as_predicate().clone()); self.nodes.push(Node::new(parent, obligation)); + Ok(()) } - }; + } } /// Convert all remaining obligations to the given error. @@ -306,12 +318,19 @@ impl ObligationForest { Ok(Some(children)) => { // if we saw a Some(_) result, we are not (yet) stalled stalled = false; + self.nodes[index].state.set(NodeState::Success); + for child in children { - self.register_obligation_at(child, - Some(NodeIndex::new(index))); + let st = self.register_obligation_at( + child, + Some(NodeIndex::new(index)) + ); + if let Err(()) = st { + // error already reported - propagate it + // to our node. + self.error_at(index); + } } - - self.nodes[index].state.set(NodeState::Success); } Err(err) => { let backtrace = self.error_at(index); diff --git a/src/librustc_data_structures/obligation_forest/test.rs b/src/librustc_data_structures/obligation_forest/test.rs index 8eac8892a3efe..a95b2b84b34c8 100644 --- a/src/librustc_data_structures/obligation_forest/test.rs +++ b/src/librustc_data_structures/obligation_forest/test.rs @@ -418,3 +418,43 @@ fn orphan() { let errors = forest.to_errors(()); assert_eq!(errors.len(), 0); } + +#[test] +fn simultaneous_register_and_error() { + // check that registering a failed obligation works correctly + let mut forest = ObligationForest::new(); + forest.register_obligation("A"); + forest.register_obligation("B"); + + let Outcome { completed: ok, errors: err, .. } = + forest.process_obligations(&mut C(|obligation| { + match *obligation { + "A" => Err("An error"), + "B" => Ok(Some(vec!["A"])), + _ => unreachable!(), + } + }, |_|{})); + assert_eq!(ok.len(), 0); + assert_eq!(err, vec![super::Error { + error: "An error", + backtrace: vec!["A"] + }]); + + let mut forest = ObligationForest::new(); + forest.register_obligation("B"); + forest.register_obligation("A"); + + let Outcome { completed: ok, errors: err, .. } = + forest.process_obligations(&mut C(|obligation| { + match *obligation { + "A" => Err("An error"), + "B" => Ok(Some(vec!["A"])), + _ => unreachable!(), + } + }, |_|{})); + assert_eq!(ok.len(), 0); + assert_eq!(err, vec![super::Error { + error: "An error", + backtrace: vec!["A"] + }]); +} diff --git a/src/test/run-pass/issue-34503.rs b/src/test/run-pass/issue-34503.rs new file mode 100644 index 0000000000000..e6217243eeb4a --- /dev/null +++ b/src/test/run-pass/issue-34503.rs @@ -0,0 +1,20 @@ +// 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. + +fn main() { + struct X; + trait Foo { + fn foo(&self) where (T, Option): Ord {} + fn bar(&self, x: &Option) -> bool + where Option: Ord { *x < *x } + } + impl Foo for () {} + let _ = &() as &Foo; +} From acf3ccdf47b4b33370574a36aaca3981aeabfa5c Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Sun, 3 Jul 2016 10:50:37 -0700 Subject: [PATCH 2/4] Revert "skip double negation in const eval" This reverts commit 735c018974e5570ea13fd887aa70a011a5b8e7b8. --- src/librustc_const_eval/eval.rs | 83 +++++++++----------- src/test/compile-fail/lint-type-overflow2.rs | 1 + 2 files changed, 39 insertions(+), 45 deletions(-) diff --git a/src/librustc_const_eval/eval.rs b/src/librustc_const_eval/eval.rs index 9db24fa4770fe..d0be7e203fa40 100644 --- a/src/librustc_const_eval/eval.rs +++ b/src/librustc_const_eval/eval.rs @@ -556,51 +556,44 @@ pub fn eval_const_expr_partial<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, let result = match e.node { hir::ExprUnary(hir::UnNeg, ref inner) => { // unary neg literals already got their sign during creation - match inner.node { - hir::ExprLit(ref lit) => { - use syntax::ast::*; - use syntax::ast::LitIntType::*; - const I8_OVERFLOW: u64 = ::std::i8::MAX as u64 + 1; - const I16_OVERFLOW: u64 = ::std::i16::MAX as u64 + 1; - const I32_OVERFLOW: u64 = ::std::i32::MAX as u64 + 1; - const I64_OVERFLOW: u64 = ::std::i64::MAX as u64 + 1; - match (&lit.node, ety.map(|t| &t.sty)) { - (&LitKind::Int(I8_OVERFLOW, Unsuffixed), Some(&ty::TyInt(IntTy::I8))) | - (&LitKind::Int(I8_OVERFLOW, Signed(IntTy::I8)), _) => { - return Ok(Integral(I8(::std::i8::MIN))) - }, - (&LitKind::Int(I16_OVERFLOW, Unsuffixed), Some(&ty::TyInt(IntTy::I16))) | - (&LitKind::Int(I16_OVERFLOW, Signed(IntTy::I16)), _) => { - return Ok(Integral(I16(::std::i16::MIN))) - }, - (&LitKind::Int(I32_OVERFLOW, Unsuffixed), Some(&ty::TyInt(IntTy::I32))) | - (&LitKind::Int(I32_OVERFLOW, Signed(IntTy::I32)), _) => { - return Ok(Integral(I32(::std::i32::MIN))) - }, - (&LitKind::Int(I64_OVERFLOW, Unsuffixed), Some(&ty::TyInt(IntTy::I64))) | - (&LitKind::Int(I64_OVERFLOW, Signed(IntTy::I64)), _) => { - return Ok(Integral(I64(::std::i64::MIN))) - }, - (&LitKind::Int(n, Unsuffixed), Some(&ty::TyInt(IntTy::Is))) | - (&LitKind::Int(n, Signed(IntTy::Is)), _) => { - match tcx.sess.target.int_type { - IntTy::I32 => if n == I32_OVERFLOW { - return Ok(Integral(Isize(Is32(::std::i32::MIN)))); - }, - IntTy::I64 => if n == I64_OVERFLOW { - return Ok(Integral(Isize(Is64(::std::i64::MIN)))); - }, - _ => bug!(), - } - }, - _ => {}, - } - }, - hir::ExprUnary(hir::UnNeg, ref inner) => { - // skip `--$expr` - return eval_const_expr_partial(tcx, inner, ty_hint, fn_args); - }, - _ => {}, + if let hir::ExprLit(ref lit) = inner.node { + use syntax::ast::*; + use syntax::ast::LitIntType::*; + const I8_OVERFLOW: u64 = ::std::i8::MAX as u64 + 1; + const I16_OVERFLOW: u64 = ::std::i16::MAX as u64 + 1; + const I32_OVERFLOW: u64 = ::std::i32::MAX as u64 + 1; + const I64_OVERFLOW: u64 = ::std::i64::MAX as u64 + 1; + match (&lit.node, ety.map(|t| &t.sty)) { + (&LitKind::Int(I8_OVERFLOW, Unsuffixed), Some(&ty::TyInt(IntTy::I8))) | + (&LitKind::Int(I8_OVERFLOW, Signed(IntTy::I8)), _) => { + return Ok(Integral(I8(::std::i8::MIN))) + }, + (&LitKind::Int(I16_OVERFLOW, Unsuffixed), Some(&ty::TyInt(IntTy::I16))) | + (&LitKind::Int(I16_OVERFLOW, Signed(IntTy::I16)), _) => { + return Ok(Integral(I16(::std::i16::MIN))) + }, + (&LitKind::Int(I32_OVERFLOW, Unsuffixed), Some(&ty::TyInt(IntTy::I32))) | + (&LitKind::Int(I32_OVERFLOW, Signed(IntTy::I32)), _) => { + return Ok(Integral(I32(::std::i32::MIN))) + }, + (&LitKind::Int(I64_OVERFLOW, Unsuffixed), Some(&ty::TyInt(IntTy::I64))) | + (&LitKind::Int(I64_OVERFLOW, Signed(IntTy::I64)), _) => { + return Ok(Integral(I64(::std::i64::MIN))) + }, + (&LitKind::Int(n, Unsuffixed), Some(&ty::TyInt(IntTy::Is))) | + (&LitKind::Int(n, Signed(IntTy::Is)), _) => { + match tcx.sess.target.int_type { + IntTy::I32 => if n == I32_OVERFLOW { + return Ok(Integral(Isize(Is32(::std::i32::MIN)))); + }, + IntTy::I64 => if n == I64_OVERFLOW { + return Ok(Integral(Isize(Is64(::std::i64::MIN)))); + }, + _ => bug!(), + } + }, + _ => {}, + } } match eval_const_expr_partial(tcx, &inner, ty_hint, fn_args)? { Float(f) => Float(-f), diff --git a/src/test/compile-fail/lint-type-overflow2.rs b/src/test/compile-fail/lint-type-overflow2.rs index 9499d732a3835..e99dfb9aa0f0e 100644 --- a/src/test/compile-fail/lint-type-overflow2.rs +++ b/src/test/compile-fail/lint-type-overflow2.rs @@ -15,6 +15,7 @@ #[allow(unused_variables)] fn main() { let x2: i8 = --128; //~ error: literal out of range for i8 + //~^ error: attempted to negate with overflow let x = -3.40282348e+38_f32; //~ error: literal out of range for f32 let x = 3.40282348e+38_f32; //~ error: literal out of range for f32 From 9a0c6859487ec3bcd8c714279c223d9217c3de69 Mon Sep 17 00:00:00 2001 From: Eduard Burtescu Date: Thu, 23 Jun 2016 03:30:01 +0300 Subject: [PATCH 3/4] Don't translate vtable methods with Self: Sized bounds. --- src/librustc/traits/object_safety.rs | 7 +++++- src/test/run-pass/trait-object-exclusion.rs | 28 +++++++++++++++++++++ 2 files changed, 34 insertions(+), 1 deletion(-) create mode 100644 src/test/run-pass/trait-object-exclusion.rs diff --git a/src/librustc/traits/object_safety.rs b/src/librustc/traits/object_safety.rs index 8cafa77973909..ffa1530a14e26 100644 --- a/src/librustc/traits/object_safety.rs +++ b/src/librustc/traits/object_safety.rs @@ -228,9 +228,14 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> { /// otherwise ensure that they cannot be used when `Self=Trait`. pub fn is_vtable_safe_method(self, trait_def_id: DefId, - method: &ty::Method<'tcx>) + method: &ty::Method<'gcx>) -> bool { + // Any method that has a `Self : Sized` requisite can't be called. + if self.generics_require_sized_self(&method.generics, &method.predicates) { + return false; + } + self.virtual_call_violation_for_method(trait_def_id, method).is_none() } diff --git a/src/test/run-pass/trait-object-exclusion.rs b/src/test/run-pass/trait-object-exclusion.rs new file mode 100644 index 0000000000000..13b725b7c9eff --- /dev/null +++ b/src/test/run-pass/trait-object-exclusion.rs @@ -0,0 +1,28 @@ +// 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. + +trait Future: 'static { + // The requirement for Self: Sized must prevent instantiation of + // Future::forget in vtables, otherwise there's an infinite type + // recursion through as Future>::forget. + fn forget(self) where Self: Sized { + Box::new(Map(self)) as Box; + } +} + +struct Map(A); +impl Future for Map {} + +pub struct Promise; +impl Future for Promise {} + +fn main() { + Promise.forget(); +} From 609ae53e9287ac1afbf04d8544da692cb08a5209 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Sun, 3 Jul 2016 10:51:41 -0700 Subject: [PATCH 4/4] Bump the beta version to 4 --- mk/main.mk | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mk/main.mk b/mk/main.mk index 086487578122c..c47020c9f9671 100644 --- a/mk/main.mk +++ b/mk/main.mk @@ -18,7 +18,7 @@ CFG_RELEASE_NUM=1.10.0 # An optional number to put after the label, e.g. '.2' -> '-beta.2' # NB Make sure it starts with a dot to conform to semver pre-release # versions (section 9) -CFG_PRERELEASE_VERSION=.3 +CFG_PRERELEASE_VERSION=.4 # Append a version-dependent hash to each library, so we can install different # versions in the same place