From 6d48e635f41904eac434fe5bbb9e6777ec77e7f7 Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Fri, 24 May 2019 13:56:26 +0200 Subject: [PATCH 01/13] Delay ICE in fold_region so feature gate has chance to stop compilation cleanly. --- src/librustc/ty/subst.rs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/librustc/ty/subst.rs b/src/librustc/ty/subst.rs index 72dfe581ba735..75ba1dd46ca2a 100644 --- a/src/librustc/ty/subst.rs +++ b/src/librustc/ty/subst.rs @@ -479,21 +479,22 @@ impl<'a, 'gcx, 'tcx> TypeFolder<'gcx, 'tcx> for SubstFolder<'a, 'gcx, 'tcx> { // the specialized routine `ty::replace_late_regions()`. match *r { ty::ReEarlyBound(data) => { - let r = self.substs.get(data.index as usize).map(|k| k.unpack()); - match r { + let rk = self.substs.get(data.index as usize).map(|k| k.unpack()); + match rk { Some(UnpackedKind::Lifetime(lt)) => { self.shift_region_through_binders(lt) } _ => { let span = self.span.unwrap_or(DUMMY_SP); - span_bug!( - span, + let msg = format!( "Region parameter out of range \ when substituting in region {} (root type={:?}) \ (index={})", data.name, self.root_ty, data.index); + self.tcx.sess.delay_span_bug(span, &msg); + r } } } From fd8b41091ae8529e2c31c4a823445bf9668657d0 Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Fri, 24 May 2019 14:01:07 +0200 Subject: [PATCH 02/13] Delay ICE in early_free_scope so feature gate has chance to stop compilation cleanly. --- src/librustc/middle/region.rs | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/src/librustc/middle/region.rs b/src/librustc/middle/region.rs index 37681ad7fcdd2..665a92131a3c8 100644 --- a/src/librustc/middle/region.rs +++ b/src/librustc/middle/region.rs @@ -658,12 +658,15 @@ impl<'tcx> ScopeTree { // The lifetime was defined on node that doesn't own a body, // which in practice can only mean a trait or an impl, that // is the parent of a method, and that is enforced below. - assert_eq!(Some(param_owner_id), self.root_parent, - "free_scope: {:?} not recognized by the \ - region scope tree for {:?} / {:?}", - param_owner, - self.root_parent.map(|id| tcx.hir().local_def_id_from_hir_id(id)), - self.root_body.map(|hir_id| DefId::local(hir_id.owner))); + if Some(param_owner_id) != self.root_parent { + tcx.sess.delay_span_bug( + DUMMY_SP, + &format!("free_scope: {:?} not recognized by the \ + region scope tree for {:?} / {:?}", + param_owner, + self.root_parent.map(|id| tcx.hir().local_def_id_from_hir_id(id)), + self.root_body.map(|hir_id| DefId::local(hir_id.owner)))); + } // The trait/impl lifetime is in scope for the method's body. self.root_body.unwrap().local_id From 5fa0883275f22770c511778cb7b11173a9b3df8c Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Fri, 24 May 2019 14:02:05 +0200 Subject: [PATCH 03/13] Regression test for issue #60654. --- .../gat-dont-ice-on-absent-feature.rs | 14 ++++++++++++++ .../gat-dont-ice-on-absent-feature.stderr | 12 ++++++++++++ 2 files changed, 26 insertions(+) create mode 100644 src/test/ui/rfc1598-generic-associated-types/gat-dont-ice-on-absent-feature.rs create mode 100644 src/test/ui/rfc1598-generic-associated-types/gat-dont-ice-on-absent-feature.stderr diff --git a/src/test/ui/rfc1598-generic-associated-types/gat-dont-ice-on-absent-feature.rs b/src/test/ui/rfc1598-generic-associated-types/gat-dont-ice-on-absent-feature.rs new file mode 100644 index 0000000000000..84fbb47301f04 --- /dev/null +++ b/src/test/ui/rfc1598-generic-associated-types/gat-dont-ice-on-absent-feature.rs @@ -0,0 +1,14 @@ +// rust-lang/rust#60654: Do not ICE on an attempt to use GATs that is +// missing the feature gate. + +struct Foo; + +impl Iterator for Foo { + type Item<'b> = &'b Foo; //~ ERROR generic associated types are unstable [E0658] + + fn next(&mut self) -> Option { + None + } +} + +fn main() { } diff --git a/src/test/ui/rfc1598-generic-associated-types/gat-dont-ice-on-absent-feature.stderr b/src/test/ui/rfc1598-generic-associated-types/gat-dont-ice-on-absent-feature.stderr new file mode 100644 index 0000000000000..27b1d73d0434a --- /dev/null +++ b/src/test/ui/rfc1598-generic-associated-types/gat-dont-ice-on-absent-feature.stderr @@ -0,0 +1,12 @@ +error[E0658]: generic associated types are unstable + --> $DIR/gat-dont-ice-on-absent-feature.rs:7:5 + | +LL | type Item<'b> = &'b Foo; + | ^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: for more information, see https://github.com/rust-lang/rust/issues/44265 + = help: add #![feature(generic_associated_types)] to the crate attributes to enable + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0658`. From 2a335ef4f53d3c70c864961ef62d8d537769adda Mon Sep 17 00:00:00 2001 From: Jethro Beekman Date: Mon, 3 Jun 2019 13:06:35 -0700 Subject: [PATCH 04/13] Fix cfg(test) build for x86_64-fortanix-unknown-sgx --- src/libstd/lib.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/libstd/lib.rs b/src/libstd/lib.rs index e044b46e0d076..a3356e6be2c3f 100644 --- a/src/libstd/lib.rs +++ b/src/libstd/lib.rs @@ -223,7 +223,8 @@ #![cfg_attr(all(target_vendor = "fortanix", target_env = "sgx"), feature(global_asm, slice_index_methods, decl_macro, coerce_unsized, sgx_platform, ptr_wrapping_offset_from))] -#![cfg_attr(all(test, target_vendor = "fortanix", target_env = "sgx"), feature(fixed_size_array))] +#![cfg_attr(all(test, target_vendor = "fortanix", target_env = "sgx"), + feature(fixed_size_array, maybe_uninit_extra))] // std is implemented with unstable features, many of which are internal // compiler details that will never be stable From 42c06fe9474db85db10c87ebe98fdfa3191a5363 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Thu, 6 Jun 2019 12:23:17 -0700 Subject: [PATCH 05/13] Handle index out of bound errors during const eval without panic --- src/librustc_mir/interpret/place.rs | 8 +++++-- src/test/ui/consts/array-literal-index-oob.rs | 6 +++++ .../ui/consts/array-literal-index-oob.stderr | 24 +++++++++++++++++++ 3 files changed, 36 insertions(+), 2 deletions(-) create mode 100644 src/test/ui/consts/array-literal-index-oob.rs create mode 100644 src/test/ui/consts/array-literal-index-oob.stderr diff --git a/src/librustc_mir/interpret/place.rs b/src/librustc_mir/interpret/place.rs index 0cc480028161f..1008c3ce80bb6 100644 --- a/src/librustc_mir/interpret/place.rs +++ b/src/librustc_mir/interpret/place.rs @@ -361,8 +361,12 @@ where offsets[usize::try_from(field).unwrap()], layout::FieldPlacement::Array { stride, .. } => { let len = base.len(self)?; - assert!(field < len, "Tried to access element {} of array/slice with length {}", - field, len); + if field >= len { + // This can be violated because this runs during promotion on code where the + // type system has not yet ensured that such things don't happen. + debug!("Tried to access element {} of array/slice with length {}", field, len); + return err!(BoundsCheck { len, index: field }); + } stride * field } layout::FieldPlacement::Union(count) => { diff --git a/src/test/ui/consts/array-literal-index-oob.rs b/src/test/ui/consts/array-literal-index-oob.rs new file mode 100644 index 0000000000000..76013c77de0c2 --- /dev/null +++ b/src/test/ui/consts/array-literal-index-oob.rs @@ -0,0 +1,6 @@ +fn main() { + &{[1, 2, 3][4]}; + //~^ ERROR index out of bounds + //~| ERROR reaching this expression at runtime will panic or abort + //~| ERROR this expression will panic at runtime +} diff --git a/src/test/ui/consts/array-literal-index-oob.stderr b/src/test/ui/consts/array-literal-index-oob.stderr new file mode 100644 index 0000000000000..727ce9e57a47b --- /dev/null +++ b/src/test/ui/consts/array-literal-index-oob.stderr @@ -0,0 +1,24 @@ +error: index out of bounds: the len is 3 but the index is 4 + --> $DIR/array-literal-index-oob.rs:2:7 + | +LL | &{[1, 2, 3][4]}; + | ^^^^^^^^^^^^ + | + = note: #[deny(const_err)] on by default + +error: this expression will panic at runtime + --> $DIR/array-literal-index-oob.rs:2:5 + | +LL | &{[1, 2, 3][4]}; + | ^^^^^^^^^^^^^^^ index out of bounds: the len is 3 but the index is 4 + +error: reaching this expression at runtime will panic or abort + --> $DIR/array-literal-index-oob.rs:2:7 + | +LL | &{[1, 2, 3][4]}; + | --^^^^^^^^^^^^- + | | + | index out of bounds: the len is 3 but the index is 4 + +error: aborting due to 3 previous errors + From b7f52da7ede8575fbb59d08589fd4c637446c52c Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Fri, 7 Jun 2019 18:29:48 +0300 Subject: [PATCH 06/13] Hygienize macros in the standard library --- src/liballoc/macros.rs | 2 +- src/libcore/macros.rs | 22 ++++++++--------- src/libstd/macros.rs | 16 ++++++------- src/test/ui/hygiene/no_implicit_prelude.rs | 2 +- .../ui/hygiene/no_implicit_prelude.stderr | 6 ++--- .../local-modularized-tricky-fail-1.rs | 1 - .../local-modularized-tricky-fail-1.stderr | 24 ++----------------- 7 files changed, 26 insertions(+), 47 deletions(-) diff --git a/src/liballoc/macros.rs b/src/liballoc/macros.rs index dd128e096f952..250c419c531f8 100644 --- a/src/liballoc/macros.rs +++ b/src/liballoc/macros.rs @@ -42,7 +42,7 @@ macro_rules! vec { ($($x:expr),*) => ( <[_]>::into_vec(box [$($x),*]) ); - ($($x:expr,)*) => (vec![$($x),*]) + ($($x:expr,)*) => ($crate::vec![$($x),*]) } // HACK(japaric): with cfg(test) the inherent `[T]::into_vec` method, which is diff --git a/src/libcore/macros.rs b/src/libcore/macros.rs index d2ee9b11b3640..cfe85c0c46c03 100644 --- a/src/libcore/macros.rs +++ b/src/libcore/macros.rs @@ -6,13 +6,13 @@ #[stable(feature = "core", since = "1.6.0")] macro_rules! panic { () => ( - panic!("explicit panic") + $crate::panic!("explicit panic") ); ($msg:expr) => ({ $crate::panicking::panic(&($msg, file!(), line!(), __rust_unstable_column!())) }); ($msg:expr,) => ( - panic!($msg) + $crate::panic!($msg) ); ($fmt:expr, $($arg:tt)+) => ({ $crate::panicking::panic_fmt(format_args!($fmt, $($arg)*), @@ -58,7 +58,7 @@ macro_rules! assert_eq { } }); ($left:expr, $right:expr,) => ({ - assert_eq!($left, $right) + $crate::assert_eq!($left, $right) }); ($left:expr, $right:expr, $($arg:tt)+) => ({ match (&($left), &($right)) { @@ -115,7 +115,7 @@ macro_rules! assert_ne { } }); ($left:expr, $right:expr,) => { - assert_ne!($left, $right) + $crate::assert_ne!($left, $right) }; ($left:expr, $right:expr, $($arg:tt)+) => ({ match (&($left), &($right)) { @@ -208,7 +208,7 @@ macro_rules! debug_assert { #[macro_export] #[stable(feature = "rust1", since = "1.0.0")] macro_rules! debug_assert_eq { - ($($arg:tt)*) => (if cfg!(debug_assertions) { assert_eq!($($arg)*); }) + ($($arg:tt)*) => (if cfg!(debug_assertions) { $crate::assert_eq!($($arg)*); }) } /// Asserts that two expressions are not equal to each other. @@ -235,7 +235,7 @@ macro_rules! debug_assert_eq { #[macro_export] #[stable(feature = "assert_ne", since = "1.13.0")] macro_rules! debug_assert_ne { - ($($arg:tt)*) => (if cfg!(debug_assertions) { assert_ne!($($arg)*); }) + ($($arg:tt)*) => (if cfg!(debug_assertions) { $crate::assert_ne!($($arg)*); }) } /// Unwraps a result or propagates its error. @@ -310,7 +310,7 @@ macro_rules! r#try { return $crate::result::Result::Err($crate::convert::From::from(err)) } }); - ($expr:expr,) => (r#try!($expr)); + ($expr:expr,) => ($crate::r#try!($expr)); } /// Writes formatted data into a buffer. @@ -425,10 +425,10 @@ macro_rules! write { #[allow_internal_unstable(format_args_nl)] macro_rules! writeln { ($dst:expr) => ( - write!($dst, "\n") + $crate::write!($dst, "\n") ); ($dst:expr,) => ( - writeln!($dst) + $crate::writeln!($dst) ); ($dst:expr, $($arg:tt)*) => ( $dst.write_fmt(format_args_nl!($($arg)*)) @@ -493,10 +493,10 @@ macro_rules! unreachable { panic!("internal error: entered unreachable code") }); ($msg:expr) => ({ - unreachable!("{}", $msg) + $crate::unreachable!("{}", $msg) }); ($msg:expr,) => ({ - unreachable!($msg) + $crate::unreachable!($msg) }); ($fmt:expr, $($arg:tt)*) => ({ panic!(concat!("internal error: entered unreachable code: ", $fmt), $($arg)*) diff --git a/src/libstd/macros.rs b/src/libstd/macros.rs index 9af7bba97aa58..c091434e50d76 100644 --- a/src/libstd/macros.rs +++ b/src/libstd/macros.rs @@ -56,13 +56,13 @@ #[allow_internal_unstable(__rust_unstable_column, libstd_sys_internals)] macro_rules! panic { () => ({ - panic!("explicit panic") + $crate::panic!("explicit panic") }); ($msg:expr) => ({ $crate::rt::begin_panic($msg, &(file!(), line!(), __rust_unstable_column!())) }); ($msg:expr,) => ({ - panic!($msg) + $crate::panic!($msg) }); ($fmt:expr, $($arg:tt)+) => ({ $crate::rt::begin_panic_fmt(&format_args!($fmt, $($arg)+), @@ -145,7 +145,7 @@ macro_rules! print { #[stable(feature = "rust1", since = "1.0.0")] #[allow_internal_unstable(print_internals, format_args_nl)] macro_rules! println { - () => (print!("\n")); + () => ($crate::print!("\n")); ($($arg:tt)*) => ({ $crate::io::_print(format_args_nl!($($arg)*)); }) @@ -204,7 +204,7 @@ macro_rules! eprint { #[stable(feature = "eprint", since = "1.19.0")] #[allow_internal_unstable(print_internals, format_args_nl)] macro_rules! eprintln { - () => (eprint!("\n")); + () => ($crate::eprint!("\n")); ($($arg:tt)*) => ({ $crate::io::_eprint(format_args_nl!($($arg)*)); }) @@ -337,23 +337,23 @@ macro_rules! eprintln { #[stable(feature = "dbg_macro", since = "1.32.0")] macro_rules! dbg { () => { - eprintln!("[{}:{}]", file!(), line!()); + $crate::eprintln!("[{}:{}]", file!(), line!()); }; ($val:expr) => { // Use of `match` here is intentional because it affects the lifetimes // of temporaries - https://stackoverflow.com/a/48732525/1063961 match $val { tmp => { - eprintln!("[{}:{}] {} = {:#?}", + $crate::eprintln!("[{}:{}] {} = {:#?}", file!(), line!(), stringify!($val), &tmp); tmp } } }; // Trailing comma with single argument is ignored - ($val:expr,) => { dbg!($val) }; + ($val:expr,) => { $crate::dbg!($val) }; ($($val:expr),+ $(,)?) => { - ($(dbg!($val)),+,) + ($($crate::dbg!($val)),+,) }; } diff --git a/src/test/ui/hygiene/no_implicit_prelude.rs b/src/test/ui/hygiene/no_implicit_prelude.rs index 1cd05f4d44c5e..890c8307543f3 100644 --- a/src/test/ui/hygiene/no_implicit_prelude.rs +++ b/src/test/ui/hygiene/no_implicit_prelude.rs @@ -13,7 +13,7 @@ mod bar { } fn f() { ::foo::m!(); - println!(); //~ ERROR cannot find macro `print!` in this scope + assert_eq!(0, 0); //~ ERROR cannot find macro `panic!` in this scope } } diff --git a/src/test/ui/hygiene/no_implicit_prelude.stderr b/src/test/ui/hygiene/no_implicit_prelude.stderr index dcb213f809a9d..737b375ed8971 100644 --- a/src/test/ui/hygiene/no_implicit_prelude.stderr +++ b/src/test/ui/hygiene/no_implicit_prelude.stderr @@ -7,11 +7,11 @@ LL | fn f() { ::bar::m!(); } LL | Vec::new(); | ^^^ use of undeclared type or module `Vec` -error: cannot find macro `print!` in this scope +error: cannot find macro `panic!` in this scope --> $DIR/no_implicit_prelude.rs:16:9 | -LL | println!(); - | ^^^^^^^^^^^ +LL | assert_eq!(0, 0); + | ^^^^^^^^^^^^^^^^^ | = help: have you added the `#[macro_use]` on the module/import? = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info) diff --git a/src/test/ui/imports/local-modularized-tricky-fail-1.rs b/src/test/ui/imports/local-modularized-tricky-fail-1.rs index d1cb6b07d750c..29e9b8ec841f5 100644 --- a/src/test/ui/imports/local-modularized-tricky-fail-1.rs +++ b/src/test/ui/imports/local-modularized-tricky-fail-1.rs @@ -33,7 +33,6 @@ mod inner2 { fn main() { panic!(); //~ ERROR `panic` is ambiguous - //~| ERROR `panic` is ambiguous } mod inner3 { diff --git a/src/test/ui/imports/local-modularized-tricky-fail-1.stderr b/src/test/ui/imports/local-modularized-tricky-fail-1.stderr index d7ae8e6d505d5..13d3227d8b38f 100644 --- a/src/test/ui/imports/local-modularized-tricky-fail-1.stderr +++ b/src/test/ui/imports/local-modularized-tricky-fail-1.stderr @@ -22,7 +22,7 @@ LL | use inner1::*; = help: consider adding an explicit import of `exported` to disambiguate error[E0659]: `include` is ambiguous (macro-expanded name vs less macro-expanded name from outer scope during import/macro resolution) - --> $DIR/local-modularized-tricky-fail-1.rs:47:1 + --> $DIR/local-modularized-tricky-fail-1.rs:46:1 | LL | include!(); | ^^^^^^^ ambiguous name @@ -59,26 +59,6 @@ LL | define_panic!(); | ---------------- in this macro invocation = help: use `crate::panic` to refer to this macro unambiguously -error[E0659]: `panic` is ambiguous (macro-expanded name vs less macro-expanded name from outer scope during import/macro resolution) - --> $DIR/local-modularized-tricky-fail-1.rs:35:5 - | -LL | panic!(); - | ^^^^^^^^^ ambiguous name - | - = note: `panic` could refer to a macro from prelude -note: `panic` could also refer to the macro defined here - --> $DIR/local-modularized-tricky-fail-1.rs:11:5 - | -LL | / macro_rules! panic { -LL | | () => () -LL | | } - | |_____^ -... -LL | define_panic!(); - | ---------------- in this macro invocation - = help: use `crate::panic` to refer to this macro unambiguously - = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info) - -error: aborting due to 4 previous errors +error: aborting due to 3 previous errors For more information about this error, try `rustc --explain E0659`. From 2790cdff5443a947c4e5de348c8ead1357b8fc60 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Tue, 18 Jun 2019 18:39:08 -0700 Subject: [PATCH 07/13] Fix ICE involving mut references --- src/librustc_mir/borrow_check/mod.rs | 2 +- src/test/ui/issues/issue-61623.rs | 11 +++++++++++ src/test/ui/issues/issue-61623.stderr | 22 ++++++++++++++++++++++ 3 files changed, 34 insertions(+), 1 deletion(-) create mode 100644 src/test/ui/issues/issue-61623.rs create mode 100644 src/test/ui/issues/issue-61623.stderr diff --git a/src/librustc_mir/borrow_check/mod.rs b/src/librustc_mir/borrow_check/mod.rs index fc1f5eb5d5a7a..74b7dfdc337be 100644 --- a/src/librustc_mir/borrow_check/mod.rs +++ b/src/librustc_mir/borrow_check/mod.rs @@ -1094,7 +1094,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { (Reservation(WriteKind::MutableBorrow(bk)), BorrowKind::Shallow) | (Reservation(WriteKind::MutableBorrow(bk)), BorrowKind::Shared) if { - tcx.migrate_borrowck() + tcx.migrate_borrowck() && this.borrow_set.location_map.get(&location).is_some() } => { let bi = this.borrow_set.location_map[&location]; debug!( diff --git a/src/test/ui/issues/issue-61623.rs b/src/test/ui/issues/issue-61623.rs new file mode 100644 index 0000000000000..e5b8747bd8047 --- /dev/null +++ b/src/test/ui/issues/issue-61623.rs @@ -0,0 +1,11 @@ +fn f1<'a>(_: &'a mut ()) {} + +fn f2

(_: P, _: ()) {} + +fn f3<'a>(x: &'a ((), &'a mut ())) { + f2(|| x.0, f1(x.1)) +//~^ ERROR cannot borrow `*x.1` as mutable, as it is behind a `&` reference +//~| ERROR cannot borrow `*x.1` as mutable because it is also borrowed as immutable +} + +fn main() {} diff --git a/src/test/ui/issues/issue-61623.stderr b/src/test/ui/issues/issue-61623.stderr new file mode 100644 index 0000000000000..883a1c441d6bb --- /dev/null +++ b/src/test/ui/issues/issue-61623.stderr @@ -0,0 +1,22 @@ +error[E0596]: cannot borrow `*x.1` as mutable, as it is behind a `&` reference + --> $DIR/issue-61623.rs:6:19 + | +LL | fn f3<'a>(x: &'a ((), &'a mut ())) { + | -------------------- help: consider changing this to be a mutable reference: `&'a mut ((), &'a mut ())` +LL | f2(|| x.0, f1(x.1)) + | ^^^ `x` is a `&` reference, so the data it refers to cannot be borrowed as mutable + +error[E0502]: cannot borrow `*x.1` as mutable because it is also borrowed as immutable + --> $DIR/issue-61623.rs:6:19 + | +LL | f2(|| x.0, f1(x.1)) + | -- -- - ^^^ mutable borrow occurs here + | | | | + | | | first borrow occurs due to use of `x` in closure + | | immutable borrow occurs here + | immutable borrow later used by call + +error: aborting due to 2 previous errors + +Some errors have detailed explanations: E0502, E0596. +For more information about an error, try `rustc --explain E0502`. From 681d554c31bd0246f5e4cc94d6221918ebe04f67 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Wed, 19 Jun 2019 09:25:20 -0700 Subject: [PATCH 08/13] review comment --- src/librustc_mir/borrow_check/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/librustc_mir/borrow_check/mod.rs b/src/librustc_mir/borrow_check/mod.rs index 74b7dfdc337be..d8feff0e19159 100644 --- a/src/librustc_mir/borrow_check/mod.rs +++ b/src/librustc_mir/borrow_check/mod.rs @@ -1094,7 +1094,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { (Reservation(WriteKind::MutableBorrow(bk)), BorrowKind::Shallow) | (Reservation(WriteKind::MutableBorrow(bk)), BorrowKind::Shared) if { - tcx.migrate_borrowck() && this.borrow_set.location_map.get(&location).is_some() + tcx.migrate_borrowck() && this.borrow_set.location_map.contains_key(&location) } => { let bi = this.borrow_set.location_map[&location]; debug!( From b2f5f0f4b0a58d3d853e2005b28c9471d8b87a37 Mon Sep 17 00:00:00 2001 From: Pietro Albini Date: Sun, 23 Jun 2019 11:09:04 +0200 Subject: [PATCH 09/13] bless ui tests --- src/test/ui/consts/array-literal-index-oob.rs | 1 - src/test/ui/consts/array-literal-index-oob.stderr | 8 +------- 2 files changed, 1 insertion(+), 8 deletions(-) diff --git a/src/test/ui/consts/array-literal-index-oob.rs b/src/test/ui/consts/array-literal-index-oob.rs index 76013c77de0c2..492182921ba34 100644 --- a/src/test/ui/consts/array-literal-index-oob.rs +++ b/src/test/ui/consts/array-literal-index-oob.rs @@ -2,5 +2,4 @@ fn main() { &{[1, 2, 3][4]}; //~^ ERROR index out of bounds //~| ERROR reaching this expression at runtime will panic or abort - //~| ERROR this expression will panic at runtime } diff --git a/src/test/ui/consts/array-literal-index-oob.stderr b/src/test/ui/consts/array-literal-index-oob.stderr index 727ce9e57a47b..a928fe5fe0c09 100644 --- a/src/test/ui/consts/array-literal-index-oob.stderr +++ b/src/test/ui/consts/array-literal-index-oob.stderr @@ -6,12 +6,6 @@ LL | &{[1, 2, 3][4]}; | = note: #[deny(const_err)] on by default -error: this expression will panic at runtime - --> $DIR/array-literal-index-oob.rs:2:5 - | -LL | &{[1, 2, 3][4]}; - | ^^^^^^^^^^^^^^^ index out of bounds: the len is 3 but the index is 4 - error: reaching this expression at runtime will panic or abort --> $DIR/array-literal-index-oob.rs:2:7 | @@ -20,5 +14,5 @@ LL | &{[1, 2, 3][4]}; | | | index out of bounds: the len is 3 but the index is 4 -error: aborting due to 3 previous errors +error: aborting due to 2 previous errors From ccfb34f704e5a6a2c57bf8553cc3c50948281271 Mon Sep 17 00:00:00 2001 From: Eduard-Mihai Burtescu Date: Sun, 16 Jun 2019 17:23:41 +0300 Subject: [PATCH 10/13] rustc_typeck: correctly compute `Substs` for `Res::SelfCtor`. --- src/librustc_typeck/check/mod.rs | 96 +++++++++---------- .../{run-pass => ui}/issues/issue-57924.rs | 1 + src/test/ui/issues/issue-57924.stderr | 9 ++ src/test/ui/issues/issue-61882-2.rs | 11 +++ src/test/ui/issues/issue-61882-2.stderr | 15 +++ src/test/ui/issues/issue-61882.rs | 9 ++ src/test/ui/issues/issue-61882.stderr | 21 ++++ 7 files changed, 109 insertions(+), 53 deletions(-) rename src/test/{run-pass => ui}/issues/issue-57924.rs (64%) create mode 100644 src/test/ui/issues/issue-57924.stderr create mode 100644 src/test/ui/issues/issue-61882-2.rs create mode 100644 src/test/ui/issues/issue-61882-2.stderr create mode 100644 src/test/ui/issues/issue-61882.rs create mode 100644 src/test/ui/issues/issue-61882.stderr diff --git a/src/librustc_typeck/check/mod.rs b/src/librustc_typeck/check/mod.rs index df30b7cce63e0..0f63cba36f295 100644 --- a/src/librustc_typeck/check/mod.rs +++ b/src/librustc_typeck/check/mod.rs @@ -5190,52 +5190,6 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { Some(original_span.with_lo(original_span.hi() - BytePos(1))) } - // Rewrite `SelfCtor` to `Ctor` - pub fn rewrite_self_ctor( - &self, - res: Res, - span: Span, - ) -> Result { - let tcx = self.tcx; - if let Res::SelfCtor(impl_def_id) = res { - let ty = self.impl_self_ty(span, impl_def_id).ty; - let adt_def = ty.ty_adt_def(); - - match adt_def { - Some(adt_def) if adt_def.has_ctor() => { - let variant = adt_def.non_enum_variant(); - let ctor_def_id = variant.ctor_def_id.unwrap(); - Ok(Res::Def(DefKind::Ctor(CtorOf::Struct, variant.ctor_kind), ctor_def_id)) - } - _ => { - let mut err = tcx.sess.struct_span_err(span, - "the `Self` constructor can only be used with tuple or unit structs"); - if let Some(adt_def) = adt_def { - match adt_def.adt_kind() { - AdtKind::Enum => { - err.help("did you mean to use one of the enum's variants?"); - }, - AdtKind::Struct | - AdtKind::Union => { - err.span_suggestion( - span, - "use curly brackets", - String::from("Self { /* fields */ }"), - Applicability::HasPlaceholders, - ); - } - } - } - err.emit(); - - Err(ErrorReported) - } - } - } else { - Ok(res) - } - } - // Instantiates the given path, which must refer to an item with the given // number of type parameters and type. pub fn instantiate_value_path(&self, @@ -5255,12 +5209,8 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { let tcx = self.tcx; - let res = match self.rewrite_self_ctor(res, span) { - Ok(res) => res, - Err(ErrorReported) => return (tcx.types.err, res), - }; let path_segs = match res { - Res::Local(_) | Res::Upvar(..) => Vec::new(), + Res::Local(_) | Res::Upvar(..) | Res::SelfCtor(_) => vec![], Res::Def(kind, def_id) => AstConv::def_ids_for_value_path_segments(self, segments, self_ty, kind, def_id), _ => bug!("instantiate_value_path on {:?}", res), @@ -5368,13 +5318,53 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { tcx.generics_of(*def_id).has_self }).unwrap_or(false); + let (res, self_ctor_substs) = if let Res::SelfCtor(impl_def_id) = res { + let ty = self.impl_self_ty(span, impl_def_id).ty; + let adt_def = ty.ty_adt_def(); + + match ty.sty { + ty::Adt(adt_def, substs) if adt_def.has_ctor() => { + let variant = adt_def.non_enum_variant(); + let ctor_def_id = variant.ctor_def_id.unwrap(); + ( + Res::Def(DefKind::Ctor(CtorOf::Struct, variant.ctor_kind), ctor_def_id), + Some(substs), + ) + } + _ => { + let mut err = tcx.sess.struct_span_err(span, + "the `Self` constructor can only be used with tuple or unit structs"); + if let Some(adt_def) = adt_def { + match adt_def.adt_kind() { + AdtKind::Enum => { + err.help("did you mean to use one of the enum's variants?"); + }, + AdtKind::Struct | + AdtKind::Union => { + err.span_suggestion( + span, + "use curly brackets", + String::from("Self { /* fields */ }"), + Applicability::HasPlaceholders, + ); + } + } + } + err.emit(); + + return (tcx.types.err, res) + } + } + } else { + (res, None) + }; let def_id = res.def_id(); // The things we are substituting into the type should not contain // escaping late-bound regions, and nor should the base type scheme. let ty = tcx.type_of(def_id); - let substs = AstConv::create_substs_for_generic_args( + let substs = self_ctor_substs.unwrap_or_else(|| AstConv::create_substs_for_generic_args( tcx, def_id, &[][..], @@ -5444,7 +5434,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { } } }, - ); + )); assert!(!substs.has_escaping_bound_vars()); assert!(!ty.has_escaping_bound_vars()); diff --git a/src/test/run-pass/issues/issue-57924.rs b/src/test/ui/issues/issue-57924.rs similarity index 64% rename from src/test/run-pass/issues/issue-57924.rs rename to src/test/ui/issues/issue-57924.rs index 232596334b0ed..dc2942225e3de 100644 --- a/src/test/run-pass/issues/issue-57924.rs +++ b/src/test/ui/issues/issue-57924.rs @@ -3,6 +3,7 @@ pub struct Gcm(E); impl Gcm { pub fn crash(e: E) -> Self { Self::(e) + //~^ ERROR type arguments are not allowed for this type } } diff --git a/src/test/ui/issues/issue-57924.stderr b/src/test/ui/issues/issue-57924.stderr new file mode 100644 index 0000000000000..2f184b1aae171 --- /dev/null +++ b/src/test/ui/issues/issue-57924.stderr @@ -0,0 +1,9 @@ +error[E0109]: type arguments are not allowed for this type + --> $DIR/issue-57924.rs:5:16 + | +LL | Self::(e) + | ^ type argument not allowed + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0109`. diff --git a/src/test/ui/issues/issue-61882-2.rs b/src/test/ui/issues/issue-61882-2.rs new file mode 100644 index 0000000000000..1209b54bc410e --- /dev/null +++ b/src/test/ui/issues/issue-61882-2.rs @@ -0,0 +1,11 @@ +struct A(T); + +impl A<&'static u8> { + fn f() { + let x = 0; + Self(&x); + //~^ ERROR `x` does not live long enough + } +} + +fn main() {} diff --git a/src/test/ui/issues/issue-61882-2.stderr b/src/test/ui/issues/issue-61882-2.stderr new file mode 100644 index 0000000000000..03a65540ced04 --- /dev/null +++ b/src/test/ui/issues/issue-61882-2.stderr @@ -0,0 +1,15 @@ +error[E0597]: `x` does not live long enough + --> $DIR/issue-61882-2.rs:6:14 + | +LL | Self(&x); + | ^^ + | | + | borrowed value does not live long enough + | requires that `x` is borrowed for `'static` +LL | +LL | } + | - `x` dropped here while still borrowed + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0597`. diff --git a/src/test/ui/issues/issue-61882.rs b/src/test/ui/issues/issue-61882.rs new file mode 100644 index 0000000000000..013398b4598a8 --- /dev/null +++ b/src/test/ui/issues/issue-61882.rs @@ -0,0 +1,9 @@ +struct A(T); + +impl A { + const B: A = Self(0); + //~^ ERROR mismatched types + //~| ERROR mismatched types +} + +fn main() {} diff --git a/src/test/ui/issues/issue-61882.stderr b/src/test/ui/issues/issue-61882.stderr new file mode 100644 index 0000000000000..a14e1a4dd4d44 --- /dev/null +++ b/src/test/ui/issues/issue-61882.stderr @@ -0,0 +1,21 @@ +error[E0308]: mismatched types + --> $DIR/issue-61882.rs:4:27 + | +LL | const B: A = Self(0); + | ^ expected bool, found integer + | + = note: expected type `bool` + found type `{integer}` + +error[E0308]: mismatched types + --> $DIR/issue-61882.rs:4:22 + | +LL | const B: A = Self(0); + | ^^^^^^^ expected u8, found bool + | + = note: expected type `A` + found type `A` + +error: aborting due to 2 previous errors + +For more information about this error, try `rustc --explain E0308`. From bb85316132c8279e3aa6fd47933aff653a6405f2 Mon Sep 17 00:00:00 2001 From: Oliver Middleton Date: Sat, 25 May 2019 23:54:12 +0100 Subject: [PATCH 11/13] Revert "Set test flag when rustdoc is running with --test option" This reverts commit 8ed2292dbe75b9b65e9fe1a079428d1e1e3b610f. It caused doctests in this repository to no longer be tested including all of the core crate. --- src/libcore/marker.rs | 6 +++--- src/libcore/mem.rs | 2 +- src/libcore/raw.rs | 4 ++-- src/librustdoc/config.rs | 3 --- src/test/rustdoc-ui/cfg-test.rs | 7 +++++-- src/test/rustdoc-ui/cfg-test.stdout | 2 +- 6 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/libcore/marker.rs b/src/libcore/marker.rs index 74f685a6de20e..d7cfccae29cec 100644 --- a/src/libcore/marker.rs +++ b/src/libcore/marker.rs @@ -73,9 +73,9 @@ impl !Send for *mut T { } /// impl Foo for Impl { } /// impl Bar for Impl { } /// -/// let x: &Foo = &Impl; // OK -/// // let y: &Bar = &Impl; // error: the trait `Bar` cannot -/// // be made into an object +/// let x: &dyn Foo = &Impl; // OK +/// // let y: &dyn Bar = &Impl; // error: the trait `Bar` cannot +/// // be made into an object /// ``` /// /// [trait object]: ../../book/ch17-02-trait-objects.html diff --git a/src/libcore/mem.rs b/src/libcore/mem.rs index 56869f38a4f6b..a11ff5e075005 100644 --- a/src/libcore/mem.rs +++ b/src/libcore/mem.rs @@ -1071,7 +1071,7 @@ impl DerefMut for ManuallyDrop { /// optimizations, potentially resulting in a larger size: /// /// ```rust -/// # use std::mem::{MaybeUninit, size_of, align_of}; +/// # use std::mem::{MaybeUninit, size_of}; /// assert_eq!(size_of::>(), 1); /// assert_eq!(size_of::>>(), 2); /// ``` diff --git a/src/libcore/raw.rs b/src/libcore/raw.rs index 155429b0e4f54..75c329a7d6c10 100644 --- a/src/libcore/raw.rs +++ b/src/libcore/raw.rs @@ -53,7 +53,7 @@ /// let value: i32 = 123; /// /// // let the compiler make a trait object -/// let object: &Foo = &value; +/// let object: &dyn Foo = &value; /// /// // look at the raw representation /// let raw_object: raw::TraitObject = unsafe { mem::transmute(object) }; @@ -65,7 +65,7 @@ /// /// // construct a new object, pointing to a different `i32`, being /// // careful to use the `i32` vtable from `object` -/// let synthesized: &Foo = unsafe { +/// let synthesized: &dyn Foo = unsafe { /// mem::transmute(raw::TraitObject { /// data: &other_value as *const _ as *mut (), /// vtable: raw_object.vtable, diff --git a/src/librustdoc/config.rs b/src/librustdoc/config.rs index 6b490f730afa0..67ca7f407d801 100644 --- a/src/librustdoc/config.rs +++ b/src/librustdoc/config.rs @@ -351,9 +351,6 @@ impl Options { .unwrap_or_else(|| PathBuf::from("doc")); let mut cfgs = matches.opt_strs("cfg"); cfgs.push("rustdoc".to_string()); - if should_test { - cfgs.push("test".to_string()); - } let extension_css = matches.opt_str("e").map(|s| PathBuf::from(&s)); diff --git a/src/test/rustdoc-ui/cfg-test.rs b/src/test/rustdoc-ui/cfg-test.rs index e26034371f444..6112e9b30e837 100644 --- a/src/test/rustdoc-ui/cfg-test.rs +++ b/src/test/rustdoc-ui/cfg-test.rs @@ -2,12 +2,15 @@ // compile-flags:--test // normalize-stdout-test: "src/test/rustdoc-ui" -> "$$DIR" +// Crates like core have doctests gated on `cfg(not(test))` so we need to make +// sure `cfg(test)` is not active when running `rustdoc --test`. + /// this doctest will be ignored: /// /// ``` /// assert!(false); /// ``` -#[cfg(not(test))] +#[cfg(test)] pub struct Foo; /// this doctest will be tested: @@ -15,5 +18,5 @@ pub struct Foo; /// ``` /// assert!(true); /// ``` -#[cfg(test)] +#[cfg(not(test))] pub struct Foo; diff --git a/src/test/rustdoc-ui/cfg-test.stdout b/src/test/rustdoc-ui/cfg-test.stdout index 30bb0038d1b8d..67873870e89b2 100644 --- a/src/test/rustdoc-ui/cfg-test.stdout +++ b/src/test/rustdoc-ui/cfg-test.stdout @@ -1,6 +1,6 @@ running 1 test -test $DIR/cfg-test.rs - Foo (line 15) ... ok +test $DIR/cfg-test.rs - Foo (line 18) ... ok test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out From 939f3971c520317aae11a6393a36bb0f8dadb487 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Sun, 16 Jun 2019 14:58:05 +0000 Subject: [PATCH 12/13] Beta backport of: Auto merge of #61754 - nikomatsakis:trait-caching-perf-3, r=pnkfelix create a "provisional cache" to restore performance in the case of cycles Introduce a "provisional cache" that caches the results of auto trait resolutions but keeps them from entering the *main* cache until everything is ready. This turned out a bit more complex than I hoped, but I don't see another short term fix -- happy to take suggestions! In the meantime, it's very clear we need to rework the trait solver. This resolves the extreme performance slowdown experienced in #60846 -- I plan to add a perf.rust-lang.org regression test to track this. Caveat: I've not run `x.py test` in full yet. r? @pnkfelix cc @arielb1 Fixes #60846 --- .../traits/query/evaluate_obligation.rs | 2 +- src/librustc/traits/select.rs | 486 +++++++++++++++--- src/librustc_traits/evaluate_obligation.rs | 2 +- 3 files changed, 407 insertions(+), 83 deletions(-) diff --git a/src/librustc/traits/query/evaluate_obligation.rs b/src/librustc/traits/query/evaluate_obligation.rs index d5230f15c2565..5f1af668a9e48 100644 --- a/src/librustc/traits/query/evaluate_obligation.rs +++ b/src/librustc/traits/query/evaluate_obligation.rs @@ -64,7 +64,7 @@ impl<'cx, 'gcx, 'tcx> InferCtxt<'cx, 'gcx, 'tcx> { Err(OverflowError) => { let mut selcx = SelectionContext::with_query_mode(&self, TraitQueryMode::Standard); - selcx.evaluate_obligation_recursively(obligation) + selcx.evaluate_root_obligation(obligation) .unwrap_or_else(|r| { span_bug!( obligation.cause.span, diff --git a/src/librustc/traits/select.rs b/src/librustc/traits/select.rs index c4be85050dbc2..771c0759d6501 100644 --- a/src/librustc/traits/select.rs +++ b/src/librustc/traits/select.rs @@ -43,7 +43,7 @@ use crate::hir; use rustc_data_structures::bit_set::GrowableBitSet; use rustc_data_structures::sync::Lock; use rustc_target::spec::abi::Abi; -use std::cell::Cell; +use std::cell::{Cell, RefCell}; use std::cmp; use std::fmt::{self, Display}; use std::iter; @@ -154,12 +154,12 @@ struct TraitObligationStack<'prev, 'tcx: 'prev> { /// selection-context's freshener. Used to check for recursion. fresh_trait_ref: ty::PolyTraitRef<'tcx>, - /// Starts out as false -- if, during evaluation, we encounter a - /// cycle, then we will set this flag to true for all participants - /// in the cycle (apart from the "head" node). These participants - /// will then forego caching their results. This is not the most - /// efficient solution, but it addresses #60010. The problem we - /// are trying to prevent: + /// Starts out equal to `depth` -- if, during evaluation, we + /// encounter a cycle, then we will set this flag to the minimum + /// depth of that cycle for all participants in the cycle. These + /// participants will then forego caching their results. This is + /// not the most efficient solution, but it addresses #60010. The + /// problem we are trying to prevent: /// /// - If you have `A: AutoTrait` requires `B: AutoTrait` and `C: NonAutoTrait` /// - `B: AutoTrait` requires `A: AutoTrait` (coinductive cycle, ok) @@ -182,9 +182,16 @@ struct TraitObligationStack<'prev, 'tcx: 'prev> { /// evaluate each member of a cycle up to N times, where N is the /// length of the cycle. This means the performance impact is /// bounded and we shouldn't have any terrible worst-cases. - in_cycle: Cell, + reached_depth: Cell, previous: TraitObligationStackList<'prev, 'tcx>, + + /// Number of parent frames plus one -- so the topmost frame has depth 1. + depth: usize, + + /// Depth-first number of this node in the search graph -- a + /// pre-order index. Basically a freshly incremented counter. + dfn: usize, } #[derive(Clone, Default)] @@ -603,7 +610,8 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> { debug!("select({:?})", obligation); debug_assert!(!obligation.predicate.has_escaping_bound_vars()); - let stack = self.push_stack(TraitObligationStackList::empty(), obligation); + let pec = &ProvisionalEvaluationCache::default(); + let stack = self.push_stack(TraitObligationStackList::empty(pec), obligation); let candidate = match self.candidate_from_obligation(&stack) { Err(SelectionError::Overflow) => { @@ -649,20 +657,23 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> { // where we do not expect overflow to be propagated. assert!(self.query_mode == TraitQueryMode::Standard); - self.evaluate_obligation_recursively(obligation) + self.evaluate_root_obligation(obligation) .expect("Overflow should be caught earlier in standard query mode") .may_apply() } - /// Evaluates whether the obligation `obligation` can be satisfied and returns - /// an `EvaluationResult`. - pub fn evaluate_obligation_recursively( + /// Evaluates whether the obligation `obligation` can be satisfied + /// and returns an `EvaluationResult`. This is meant for the + /// *initial* call. + pub fn evaluate_root_obligation( &mut self, obligation: &PredicateObligation<'tcx>, ) -> Result { self.evaluation_probe(|this| { - this.evaluate_predicate_recursively(TraitObligationStackList::empty(), - obligation.clone()) + this.evaluate_predicate_recursively( + TraitObligationStackList::empty(&ProvisionalEvaluationCache::default()), + obligation.clone(), + ) }) } @@ -868,23 +879,131 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> { return Ok(result); } + if let Some(result) = stack.cache().get_provisional(fresh_trait_ref) { + debug!("PROVISIONAL CACHE HIT: EVAL({:?})={:?}", fresh_trait_ref, result); + stack.update_reached_depth(stack.cache().current_reached_depth()); + return Ok(result); + } + + // Check if this is a match for something already on the + // stack. If so, we don't want to insert the result into the + // main cache (it is cycle dependent) nor the provisional + // cache (which is meant for things that have completed but + // for a "backedge" -- this result *is* the backedge). + if let Some(cycle_result) = self.check_evaluation_cycle(&stack) { + return Ok(cycle_result); + } + let (result, dep_node) = self.in_task(|this| this.evaluate_stack(&stack)); let result = result?; - if !stack.in_cycle.get() { + if !result.must_apply_modulo_regions() { + stack.cache().on_failure(stack.dfn); + } + + let reached_depth = stack.reached_depth.get(); + if reached_depth >= stack.depth { debug!("CACHE MISS: EVAL({:?})={:?}", fresh_trait_ref, result); self.insert_evaluation_cache(obligation.param_env, fresh_trait_ref, dep_node, result); + + stack.cache().on_completion(stack.depth, |fresh_trait_ref, provisional_result| { + self.insert_evaluation_cache( + obligation.param_env, + fresh_trait_ref, + dep_node, + provisional_result.max(result), + ); + }); } else { + debug!("PROVISIONAL: {:?}={:?}", fresh_trait_ref, result); debug!( - "evaluate_trait_predicate_recursively: skipping cache because {:?} \ - is a cycle participant", + "evaluate_trait_predicate_recursively: caching provisionally because {:?} \ + is a cycle participant (at depth {}, reached depth {})", + fresh_trait_ref, + stack.depth, + reached_depth, + ); + + stack.cache().insert_provisional( + stack.dfn, + reached_depth, fresh_trait_ref, + result, ); } + Ok(result) } + /// If there is any previous entry on the stack that precisely + /// matches this obligation, then we can assume that the + /// obligation is satisfied for now (still all other conditions + /// must be met of course). One obvious case this comes up is + /// marker traits like `Send`. Think of a linked list: + /// + /// struct List { data: T, next: Option>> } + /// + /// `Box>` will be `Send` if `T` is `Send` and + /// `Option>>` is `Send`, and in turn + /// `Option>>` is `Send` if `Box>` is + /// `Send`. + /// + /// Note that we do this comparison using the `fresh_trait_ref` + /// fields. Because these have all been freshened using + /// `self.freshener`, we can be sure that (a) this will not + /// affect the inferencer state and (b) that if we see two + /// fresh regions with the same index, they refer to the same + /// unbound type variable. + fn check_evaluation_cycle( + &mut self, + stack: &TraitObligationStack<'_, 'tcx>, + ) -> Option { + if let Some(cycle_depth) = stack.iter() + .skip(1) // skip top-most frame + .find(|prev| stack.obligation.param_env == prev.obligation.param_env && + stack.fresh_trait_ref == prev.fresh_trait_ref) + .map(|stack| stack.depth) + { + debug!( + "evaluate_stack({:?}) --> recursive at depth {}", + stack.fresh_trait_ref, + cycle_depth, + ); + + // If we have a stack like `A B C D E A`, where the top of + // the stack is the final `A`, then this will iterate over + // `A, E, D, C, B` -- i.e., all the participants apart + // from the cycle head. We mark them as participating in a + // cycle. This suppresses caching for those nodes. See + // `in_cycle` field for more details. + stack.update_reached_depth(cycle_depth); + + // Subtle: when checking for a coinductive cycle, we do + // not compare using the "freshened trait refs" (which + // have erased regions) but rather the fully explicit + // trait refs. This is important because it's only a cycle + // if the regions match exactly. + let cycle = stack.iter().skip(1).take_while(|s| s.depth >= cycle_depth); + let cycle = cycle.map(|stack| ty::Predicate::Trait(stack.obligation.predicate)); + if self.coinductive_match(cycle) { + debug!( + "evaluate_stack({:?}) --> recursive, coinductive", + stack.fresh_trait_ref + ); + Some(EvaluatedToOk) + } else { + debug!( + "evaluate_stack({:?}) --> recursive, inductive", + stack.fresh_trait_ref + ); + Some(EvaluatedToRecur) + } + } else { + None + } + } + fn evaluate_stack<'o>( &mut self, stack: &TraitObligationStack<'o, 'tcx>, @@ -961,65 +1080,6 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> { return Ok(EvaluatedToUnknown); } - // If there is any previous entry on the stack that precisely - // matches this obligation, then we can assume that the - // obligation is satisfied for now (still all other conditions - // must be met of course). One obvious case this comes up is - // marker traits like `Send`. Think of a linked list: - // - // struct List { data: T, next: Option>> } - // - // `Box>` will be `Send` if `T` is `Send` and - // `Option>>` is `Send`, and in turn - // `Option>>` is `Send` if `Box>` is - // `Send`. - // - // Note that we do this comparison using the `fresh_trait_ref` - // fields. Because these have all been freshened using - // `self.freshener`, we can be sure that (a) this will not - // affect the inferencer state and (b) that if we see two - // fresh regions with the same index, they refer to the same - // unbound type variable. - if let Some(rec_index) = stack.iter() - .skip(1) // skip top-most frame - .position(|prev| stack.obligation.param_env == prev.obligation.param_env && - stack.fresh_trait_ref == prev.fresh_trait_ref) - { - debug!("evaluate_stack({:?}) --> recursive", stack.fresh_trait_ref); - - // If we have a stack like `A B C D E A`, where the top of - // the stack is the final `A`, then this will iterate over - // `A, E, D, C, B` -- i.e., all the participants apart - // from the cycle head. We mark them as participating in a - // cycle. This suppresses caching for those nodes. See - // `in_cycle` field for more details. - for item in stack.iter().take(rec_index + 1) { - debug!("evaluate_stack: marking {:?} as cycle participant", item.fresh_trait_ref); - item.in_cycle.set(true); - } - - // Subtle: when checking for a coinductive cycle, we do - // not compare using the "freshened trait refs" (which - // have erased regions) but rather the fully explicit - // trait refs. This is important because it's only a cycle - // if the regions match exactly. - let cycle = stack.iter().skip(1).take(rec_index + 1); - let cycle = cycle.map(|stack| ty::Predicate::Trait(stack.obligation.predicate)); - if self.coinductive_match(cycle) { - debug!( - "evaluate_stack({:?}) --> recursive, coinductive", - stack.fresh_trait_ref - ); - return Ok(EvaluatedToOk); - } else { - debug!( - "evaluate_stack({:?}) --> recursive, inductive", - stack.fresh_trait_ref - ); - return Ok(EvaluatedToRecur); - } - } - match self.candidate_from_obligation(stack) { Ok(Some(c)) => self.evaluate_candidate(stack, &c), Ok(None) => Ok(EvaluatedToAmbig), @@ -1222,6 +1282,11 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> { } // If no match, compute result and insert into cache. + // + // FIXME(nikomatsakis) -- this cache is not taking into + // account cycles that may have occurred in forming the + // candidate. I don't know of any specific problems that + // result but it seems awfully suspicious. let (candidate, dep_node) = self.in_task(|this| this.candidate_from_obligation_no_cache(stack)); @@ -3737,11 +3802,15 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> { .to_poly_trait_ref() .fold_with(&mut self.freshener); + let dfn = previous_stack.cache.next_dfn(); + let depth = previous_stack.depth() + 1; TraitObligationStack { obligation, fresh_trait_ref, - in_cycle: Cell::new(false), + reached_depth: Cell::new(depth), previous: previous_stack, + dfn, + depth, } } @@ -3934,28 +4003,283 @@ impl<'o, 'tcx> TraitObligationStack<'o, 'tcx> { TraitObligationStackList::with(self) } + fn cache(&self) -> &'o ProvisionalEvaluationCache<'tcx> { + self.previous.cache + } + fn iter(&'o self) -> TraitObligationStackList<'o, 'tcx> { self.list() } + + /// Indicates that attempting to evaluate this stack entry + /// required accessing something from the stack at depth `reached_depth`. + fn update_reached_depth(&self, reached_depth: usize) { + assert!( + self.depth > reached_depth, + "invoked `update_reached_depth` with something under this stack: \ + self.depth={} reached_depth={}", + self.depth, + reached_depth, + ); + debug!("update_reached_depth(reached_depth={})", reached_depth); + let mut p = self; + while reached_depth < p.depth { + debug!("update_reached_depth: marking {:?} as cycle participant", p.fresh_trait_ref); + p.reached_depth.set(p.reached_depth.get().min(reached_depth)); + p = p.previous.head.unwrap(); + } + } +} + +/// The "provisional evaluation cache" is used to store intermediate cache results +/// when solving auto traits. Auto traits are unusual in that they can support +/// cycles. So, for example, a "proof tree" like this would be ok: +/// +/// - `Foo: Send` :- +/// - `Bar: Send` :- +/// - `Foo: Send` -- cycle, but ok +/// - `Baz: Send` +/// +/// Here, to prove `Foo: Send`, we have to prove `Bar: Send` and +/// `Baz: Send`. Proving `Bar: Send` in turn required `Foo: Send`. +/// For non-auto traits, this cycle would be an error, but for auto traits (because +/// they are coinductive) it is considered ok. +/// +/// However, there is a complication: at the point where we have +/// "proven" `Bar: Send`, we have in fact only proven it +/// *provisionally*. In particular, we proved that `Bar: Send` +/// *under the assumption* that `Foo: Send`. But what if we later +/// find out this assumption is wrong? Specifically, we could +/// encounter some kind of error proving `Baz: Send`. In that case, +/// `Bar: Send` didn't turn out to be true. +/// +/// In Issue #60010, we found a bug in rustc where it would cache +/// these intermediate results. This was fixed in #60444 by disabling +/// *all* caching for things involved in a cycle -- in our example, +/// that would mean we don't cache that `Bar: Send`. But this led +/// to large slowdowns. +/// +/// Specifically, imagine this scenario, where proving `Baz: Send` +/// first requires proving `Bar: Send` (which is true: +/// +/// - `Foo: Send` :- +/// - `Bar: Send` :- +/// - `Foo: Send` -- cycle, but ok +/// - `Baz: Send` +/// - `Bar: Send` -- would be nice for this to be a cache hit! +/// - `*const T: Send` -- but what if we later encounter an error? +/// +/// The *provisional evaluation cache* resolves this issue. It stores +/// cache results that we've proven but which were involved in a cycle +/// in some way. We track the minimal stack depth (i.e., the +/// farthest from the top of the stack) that we are dependent on. +/// The idea is that the cache results within are all valid -- so long as +/// none of the nodes in between the current node and the node at that minimum +/// depth result in an error (in which case the cached results are just thrown away). +/// +/// During evaluation, we consult this provisional cache and rely on +/// it. Accessing a cached value is considered equivalent to accessing +/// a result at `reached_depth`, so it marks the *current* solution as +/// provisional as well. If an error is encountered, we toss out any +/// provisional results added from the subtree that encountered the +/// error. When we pop the node at `reached_depth` from the stack, we +/// can commit all the things that remain in the provisional cache. +struct ProvisionalEvaluationCache<'tcx> { + /// next "depth first number" to issue -- just a counter + dfn: Cell, + + /// Stores the "coldest" depth (bottom of stack) reached by any of + /// the evaluation entries. The idea here is that all things in the provisional + /// cache are always dependent on *something* that is colder in the stack: + /// therefore, if we add a new entry that is dependent on something *colder still*, + /// we have to modify the depth for all entries at once. + /// + /// Example: + /// + /// Imagine we have a stack `A B C D E` (with `E` being the top of + /// the stack). We cache something with depth 2, which means that + /// it was dependent on C. Then we pop E but go on and process a + /// new node F: A B C D F. Now F adds something to the cache with + /// depth 1, meaning it is dependent on B. Our original cache + /// entry is also dependent on B, because there is a path from E + /// to C and then from C to F and from F to B. + reached_depth: Cell, + + /// Map from cache key to the provisionally evaluated thing. + /// The cache entries contain the result but also the DFN in which they + /// were added. The DFN is used to clear out values on failure. + /// + /// Imagine we have a stack like: + /// + /// - `A B C` and we add a cache for the result of C (DFN 2) + /// - Then we have a stack `A B D` where `D` has DFN 3 + /// - We try to solve D by evaluating E: `A B D E` (DFN 4) + /// - `E` generates various cache entries which have cyclic dependices on `B` + /// - `A B D E F` and so forth + /// - the DFN of `F` for example would be 5 + /// - then we determine that `E` is in error -- we will then clear + /// all cache values whose DFN is >= 4 -- in this case, that + /// means the cached value for `F`. + map: RefCell, ProvisionalEvaluation>>, +} + +/// A cache value for the provisional cache: contains the depth-first +/// number (DFN) and result. +#[derive(Copy, Clone, Debug)] +struct ProvisionalEvaluation { + from_dfn: usize, + result: EvaluationResult, +} + +impl<'tcx> Default for ProvisionalEvaluationCache<'tcx> { + fn default() -> Self { + Self { + dfn: Cell::new(0), + reached_depth: Cell::new(std::usize::MAX), + map: Default::default(), + } + } +} + +impl<'tcx> ProvisionalEvaluationCache<'tcx> { + /// Get the next DFN in sequence (basically a counter). + fn next_dfn(&self) -> usize { + let result = self.dfn.get(); + self.dfn.set(result + 1); + result + } + + /// Check the provisional cache for any result for + /// `fresh_trait_ref`. If there is a hit, then you must consider + /// it an access to the stack slots at depth + /// `self.current_reached_depth()` and above. + fn get_provisional(&self, fresh_trait_ref: ty::PolyTraitRef<'tcx>) -> Option { + debug!( + "get_provisional(fresh_trait_ref={:?}) = {:#?} with reached-depth {}", + fresh_trait_ref, + self.map.borrow().get(&fresh_trait_ref), + self.reached_depth.get(), + ); + Some(self.map.borrow().get(&fresh_trait_ref)?.result) + } + + /// Current value of the `reached_depth` counter -- all the + /// provisional cache entries are dependent on the item at this + /// depth. + fn current_reached_depth(&self) -> usize { + self.reached_depth.get() + } + + /// Insert a provisional result into the cache. The result came + /// from the node with the given DFN. It accessed a minimum depth + /// of `reached_depth` to compute. It evaluated `fresh_trait_ref` + /// and resulted in `result`. + fn insert_provisional( + &self, + from_dfn: usize, + reached_depth: usize, + fresh_trait_ref: ty::PolyTraitRef<'tcx>, + result: EvaluationResult, + ) { + debug!( + "insert_provisional(from_dfn={}, reached_depth={}, fresh_trait_ref={:?}, result={:?})", + from_dfn, + reached_depth, + fresh_trait_ref, + result, + ); + let r_d = self.reached_depth.get(); + self.reached_depth.set(r_d.min(reached_depth)); + + debug!("insert_provisional: reached_depth={:?}", self.reached_depth.get()); + + self.map.borrow_mut().insert(fresh_trait_ref, ProvisionalEvaluation { from_dfn, result }); + } + + /// Invoked when the node with dfn `dfn` does not get a successful + /// result. This will clear out any provisional cache entries + /// that were added since `dfn` was created. This is because the + /// provisional entries are things which must assume that the + /// things on the stack at the time of their creation succeeded -- + /// since the failing node is presently at the top of the stack, + /// these provisional entries must either depend on it or some + /// ancestor of it. + fn on_failure(&self, dfn: usize) { + debug!( + "on_failure(dfn={:?})", + dfn, + ); + self.map.borrow_mut().retain(|key, eval| { + if !eval.from_dfn >= dfn { + debug!("on_failure: removing {:?}", key); + false + } else { + true + } + }); + } + + /// Invoked when the node at depth `depth` completed without + /// depending on anything higher in the stack (if that completion + /// was a failure, then `on_failure` should have been invoked + /// already). The callback `op` will be invoked for each + /// provisional entry that we can now confirm. + fn on_completion( + &self, + depth: usize, + mut op: impl FnMut(ty::PolyTraitRef<'tcx>, EvaluationResult), + ) { + debug!( + "on_completion(depth={}, reached_depth={})", + depth, + self.reached_depth.get(), + ); + + if self.reached_depth.get() < depth { + debug!("on_completion: did not yet reach depth to complete"); + return; + } + + for (fresh_trait_ref, eval) in self.map.borrow_mut().drain() { + debug!( + "on_completion: fresh_trait_ref={:?} eval={:?}", + fresh_trait_ref, + eval, + ); + + op(fresh_trait_ref, eval.result); + } + + self.reached_depth.set(std::usize::MAX); + } } #[derive(Copy, Clone)] struct TraitObligationStackList<'o, 'tcx: 'o> { + cache: &'o ProvisionalEvaluationCache<'tcx>, head: Option<&'o TraitObligationStack<'o, 'tcx>>, } impl<'o, 'tcx> TraitObligationStackList<'o, 'tcx> { - fn empty() -> TraitObligationStackList<'o, 'tcx> { - TraitObligationStackList { head: None } + fn empty(cache: &'o ProvisionalEvaluationCache<'tcx>) -> TraitObligationStackList<'o, 'tcx> { + TraitObligationStackList { cache, head: None } } fn with(r: &'o TraitObligationStack<'o, 'tcx>) -> TraitObligationStackList<'o, 'tcx> { - TraitObligationStackList { head: Some(r) } + TraitObligationStackList { cache: r.cache(), head: Some(r) } } fn head(&self) -> Option<&'o TraitObligationStack<'o, 'tcx>> { self.head } + + fn depth(&self) -> usize { + if let Some(head) = self.head { + head.depth + } else { + 0 + } + } } impl<'o, 'tcx> Iterator for TraitObligationStackList<'o, 'tcx> { diff --git a/src/librustc_traits/evaluate_obligation.rs b/src/librustc_traits/evaluate_obligation.rs index 83aebd16e2400..59b743abaf14b 100644 --- a/src/librustc_traits/evaluate_obligation.rs +++ b/src/librustc_traits/evaluate_obligation.rs @@ -29,7 +29,7 @@ fn evaluate_obligation<'tcx>( let mut selcx = SelectionContext::with_query_mode(&infcx, TraitQueryMode::Canonical); let obligation = Obligation::new(ObligationCause::dummy(), param_env, predicate); - selcx.evaluate_obligation_recursively(&obligation) + selcx.evaluate_root_obligation(&obligation) }, ) } From 4b3d9d442b050a80f5d59b1a1b5d013bba56e749 Mon Sep 17 00:00:00 2001 From: Mark Rousskov Date: Wed, 26 Jun 2019 08:59:32 -0400 Subject: [PATCH 13/13] Add dummy Azure config --- .azure-pipelines/auto.yml | 15 +++++++++++++++ 1 file changed, 15 insertions(+) create mode 100644 .azure-pipelines/auto.yml diff --git a/.azure-pipelines/auto.yml b/.azure-pipelines/auto.yml new file mode 100644 index 0000000000000..e772e617b6db7 --- /dev/null +++ b/.azure-pipelines/auto.yml @@ -0,0 +1,15 @@ +# Starter pipeline +# Start with a minimal pipeline that you can customize to build and deploy your code. +# Add steps that build, run tests, deploy, and more: +# https://aka.ms/yaml + +pr: none +trigger: + - auto + +pool: + vmImage: 'Ubuntu 16.04' + +steps: +- script: echo Done + displayName: 'Dummy build'