From b1375cd23cf0fa3ffefa8ec82e6cba04d233b44c Mon Sep 17 00:00:00 2001 From: Alexis Bourget Date: Fri, 10 Jul 2020 23:53:25 +0200 Subject: [PATCH 01/44] Deny unsafe op in unsafe fns without the unsafe keyword, first part for std/thread --- library/std/src/thread/local.rs | 99 +++++++++++++++++++++++---------- library/std/src/thread/mod.rs | 28 +++++++--- 2 files changed, 91 insertions(+), 36 deletions(-) diff --git a/library/std/src/thread/local.rs b/library/std/src/thread/local.rs index 66508f06b2884..13252baf731ef 100644 --- a/library/std/src/thread/local.rs +++ b/library/std/src/thread/local.rs @@ -284,7 +284,11 @@ mod lazy { } pub unsafe fn get(&self) -> Option<&'static T> { - (*self.inner.get()).as_ref() + // SAFETY: No reference is ever handed out to the inner cell nor + // mutable reference to the Option inside said cell. This make it + // safe to hand a reference, though the lifetime of 'static + // is itself unsafe, making the get method unsafe. + unsafe { (*self.inner.get()).as_ref() } } pub unsafe fn initialize T>(&self, init: F) -> &'static T { @@ -293,6 +297,8 @@ mod lazy { let value = init(); let ptr = self.inner.get(); + // SAFETY: + // // note that this can in theory just be `*ptr = Some(value)`, but due to // the compiler will currently codegen that pattern with something like: // @@ -305,22 +311,31 @@ mod lazy { // value (an aliasing violation). To avoid setting the "I'm running a // destructor" flag we just use `mem::replace` which should sequence the // operations a little differently and make this safe to call. - let _ = mem::replace(&mut *ptr, Some(value)); - - // After storing `Some` we want to get a reference to the contents of - // what we just stored. While we could use `unwrap` here and it should - // always work it empirically doesn't seem to always get optimized away, - // which means that using something like `try_with` can pull in - // panicking code and cause a large size bloat. - match *ptr { - Some(ref x) => x, - None => hint::unreachable_unchecked(), + unsafe { + let _ = mem::replace(&mut *ptr, Some(value)); + } + + // SAFETY: the *ptr operation is made safe by the `mem::replace` + // call above that made sure a valid value is present behind it. + unsafe { + // After storing `Some` we want to get a reference to the contents of + // what we just stored. While we could use `unwrap` here and it should + // always work it empirically doesn't seem to always get optimized away, + // which means that using something like `try_with` can pull in + // panicking code and cause a large size bloat. + match *ptr { + Some(ref x) => x, + None => hint::unreachable_unchecked(), + } } } #[allow(unused)] pub unsafe fn take(&mut self) -> Option { - (*self.inner.get()).take() + // SAFETY: The other methods hand out references while taking &self. + // As such, calling this method when such references are still alive + // will fail because it takes a &mut self, conflicting with them. + unsafe { (*self.inner.get()).take() } } } } @@ -409,9 +424,17 @@ pub mod fast { } pub unsafe fn get T>(&self, init: F) -> Option<&'static T> { - match self.inner.get() { - Some(val) => Some(val), - None => self.try_initialize(init), + // SAFETY: See the definitions of `LazyKeyInner::get` and + // `try_initialize` for more informations. + // + // The call to `get` is made safe because no mutable references are + // ever handed out and the `try_initialize` is dependant on the + // passed `init` function. + unsafe { + match self.inner.get() { + Some(val) => Some(val), + None => self.try_initialize(init), + } } } @@ -425,8 +448,10 @@ pub mod fast { // LLVM issue: https://bugs.llvm.org/show_bug.cgi?id=41722 #[cold] unsafe fn try_initialize T>(&self, init: F) -> Option<&'static T> { - if !mem::needs_drop::() || self.try_register_dtor() { - Some(self.inner.initialize(init)) + // SAFETY: See comment above. + if !mem::needs_drop::() || unsafe { self.try_register_dtor() } { + // SAFETY: See comment above. + Some(unsafe { self.inner.initialize(init) }) } else { None } @@ -438,8 +463,12 @@ pub mod fast { unsafe fn try_register_dtor(&self) -> bool { match self.dtor_state.get() { DtorState::Unregistered => { - // dtor registration happens before initialization. - register_dtor(self as *const _ as *mut u8, destroy_value::); + // SAFETY: dtor registration happens before initialization. + // Passing `self` as a pointer while using `destroy_value` + // is safe because the function will build a pointer to a + // Key, which is the type of self and so find the correct + // size. + unsafe { register_dtor(self as *const _ as *mut u8, destroy_value::) }; self.dtor_state.set(DtorState::Registered); true } @@ -455,13 +484,21 @@ pub mod fast { unsafe extern "C" fn destroy_value(ptr: *mut u8) { let ptr = ptr as *mut Key; + // SAFETY: + // + // The pointer `ptr` has been built just above and comes from + // `try_register_dtor` where it is originally a Key coming from `self`, + // making it non-NUL and of the correct type. + // // Right before we run the user destructor be sure to set the // `Option` to `None`, and `dtor_state` to `RunningOrHasRun`. This // causes future calls to `get` to run `try_initialize_drop` again, // which will now fail, and return `None`. - let value = (*ptr).inner.take(); - (*ptr).dtor_state.set(DtorState::RunningOrHasRun); - drop(value); + unsafe { + let value = (*ptr).inner.take(); + (*ptr).dtor_state.set(DtorState::RunningOrHasRun); + drop(value); + } } } @@ -530,11 +567,15 @@ pub mod os { ptr }; - Some((*ptr).inner.initialize(init)) + // SAFETY: ptr has been ensured as non-NUL just above an so can be + // dereferenced safely. + unsafe { Some((*ptr).inner.initialize(init)) } } } unsafe extern "C" fn destroy_value(ptr: *mut u8) { + // SAFETY: + // // The OS TLS ensures that this key contains a NULL value when this // destructor starts to run. We set it back to a sentinel value of 1 to // ensure that any future calls to `get` for this thread will return @@ -542,11 +583,13 @@ pub mod os { // // Note that to prevent an infinite loop we reset it back to null right // before we return from the destructor ourselves. - let ptr = Box::from_raw(ptr as *mut Value); - let key = ptr.key; - key.os.set(1 as *mut u8); - drop(ptr); - key.os.set(ptr::null_mut()); + unsafe { + let ptr = Box::from_raw(ptr as *mut Value); + let key = ptr.key; + key.os.set(1 as *mut u8); + drop(ptr); + key.os.set(ptr::null_mut()); + } } } diff --git a/library/std/src/thread/mod.rs b/library/std/src/thread/mod.rs index 202867258f1e4..90dc17de37704 100644 --- a/library/std/src/thread/mod.rs +++ b/library/std/src/thread/mod.rs @@ -155,6 +155,7 @@ //! [`with`]: struct.LocalKey.html#method.with #![stable(feature = "rust1", since = "1.0.0")] +#![deny(unsafe_op_in_unsafe_fn)] use crate::any::Any; use crate::cell::UnsafeCell; @@ -470,14 +471,23 @@ impl Builder { imp::Thread::set_name(name); } - thread_info::set(imp::guard::current(), their_thread); + // SAFETY: the stack guard passed is the one for the current thread. + // This means the current thread's stack and the new thread's stack + // are properly set and protected from each other. + thread_info::set(unsafe { imp::guard::current() }, their_thread); let try_result = panic::catch_unwind(panic::AssertUnwindSafe(|| { crate::sys_common::backtrace::__rust_begin_short_backtrace(f) })); - *their_packet.get() = Some(try_result); + // SAFETY: `their_packet` as been built just above and moved by the + // closure (it is an Arc<...>) and `my_packet` will be stored in the + // same `JoinInner` as this closure meaning the mutation will be + // safe (not modify it and affect a value far away). + unsafe { *their_packet.get() = Some(try_result) }; }; Ok(JoinHandle(JoinInner { + // SAFETY: + // // `imp::Thread::new` takes a closure with a `'static` lifetime, since it's passed // through FFI or otherwise used with low-level threading primitives that have no // notion of or way to enforce lifetimes. @@ -489,12 +499,14 @@ impl Builder { // Similarly, the `sys` implementation must guarantee that no references to the closure // exist after the thread has terminated, which is signaled by `Thread::join` // returning. - native: Some(imp::Thread::new( - stack_size, - mem::transmute::, Box>(Box::new( - main, - )), - )?), + native: unsafe { + Some(imp::Thread::new( + stack_size, + mem::transmute::, Box>( + Box::new(main), + ), + )?) + }, thread: my_thread, packet: Packet(my_packet), })) From 3a22b2181a0cd662be00e053a9e9c36d5330e444 Mon Sep 17 00:00:00 2001 From: Alexis Bourget Date: Sat, 11 Jul 2020 00:15:24 +0200 Subject: [PATCH 02/44] Finished documenting all unsafe op inside unsafe fn --- library/std/src/thread/local.rs | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/library/std/src/thread/local.rs b/library/std/src/thread/local.rs index 13252baf731ef..bd0945c9a0704 100644 --- a/library/std/src/thread/local.rs +++ b/library/std/src/thread/local.rs @@ -536,20 +536,28 @@ pub mod os { } pub unsafe fn get(&'static self, init: fn() -> T) -> Option<&'static T> { - let ptr = self.os.get() as *mut Value; + // SAFETY: No mutable references are ever handed out meaning getting + // the value is ok. + let ptr = unsafe { self.os.get() as *mut Value }; if ptr as usize > 1 { - if let Some(ref value) = (*ptr).inner.get() { + // SAFETY: the check ensured the pointer is safe (its destructor + // is not running) + it is coming from a trusted source (self). + if let Some(ref value) = unsafe { (*ptr).inner.get() } { return Some(value); } } - self.try_initialize(init) + // SAFETY: At this point we are sure we have no value and so + // initializing (or trying to) is safe. + unsafe { self.try_initialize(init) } } // `try_initialize` is only called once per os thread local variable, // except in corner cases where thread_local dtors reference other // thread_local's, or it is being recursively initialized. unsafe fn try_initialize(&'static self, init: fn() -> T) -> Option<&'static T> { - let ptr = self.os.get() as *mut Value; + // SAFETY: No mutable references are ever handed out meaning getting + // the value is ok. + let ptr = unsafe { self.os.get() as *mut Value }; if ptr as usize == 1 { // destructor is running return None; @@ -560,7 +568,11 @@ pub mod os { // local copy, so do that now. let ptr: Box> = box Value { inner: LazyKeyInner::new(), key: self }; let ptr = Box::into_raw(ptr); - self.os.set(ptr as *mut u8); + // SAFETY: At this point we are sure there is no value inside + // ptr so setting it will not affect anyone else. + unsafe { + self.os.set(ptr as *mut u8); + } ptr } else { // recursive initialization From ee289d2c242a3513eec4f9af617d8b96517cadf7 Mon Sep 17 00:00:00 2001 From: Alexis Bourget Date: Sat, 29 Aug 2020 20:10:05 +0200 Subject: [PATCH 03/44] Improve some SAFETY comments following suggestions --- library/std/src/thread/local.rs | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/library/std/src/thread/local.rs b/library/std/src/thread/local.rs index bd0945c9a0704..4ac1e523a97ea 100644 --- a/library/std/src/thread/local.rs +++ b/library/std/src/thread/local.rs @@ -311,12 +311,23 @@ mod lazy { // value (an aliasing violation). To avoid setting the "I'm running a // destructor" flag we just use `mem::replace` which should sequence the // operations a little differently and make this safe to call. + // + // `ptr` can be dereferenced safely since it was obtained from + // `UnsafeCell::get`, which should not return a non-aligned or NUL pointer. + // What's more a `LazyKeyInner` can only be created with `new`, which ensures + // `inner` is correctly initialized and all calls to methods on `LazyKeyInner` + // will leave `inner` initialized too. unsafe { let _ = mem::replace(&mut *ptr, Some(value)); } - // SAFETY: the *ptr operation is made safe by the `mem::replace` - // call above that made sure a valid value is present behind it. + // SAFETY: the `*ptr` operation is made safe by the `mem::replace` + // call above combined with `ptr` being correct from the beginning + // (see previous SAFETY: comment above). + // + // Plus, with the call to `mem::replace` it is guaranteed there is + // a `Some` behind `ptr`, not a `None` so `unreachable_unchecked` + // will never be reached. unsafe { // After storing `Some` we want to get a reference to the contents of // what we just stored. While we could use `unwrap` here and it should @@ -333,8 +344,8 @@ mod lazy { #[allow(unused)] pub unsafe fn take(&mut self) -> Option { // SAFETY: The other methods hand out references while taking &self. - // As such, calling this method when such references are still alive - // will fail because it takes a &mut self, conflicting with them. + // As such, callers of this method must ensure no `&` and `&mut` are + // available and used at the same time. unsafe { (*self.inner.get()).take() } } } @@ -448,9 +459,9 @@ pub mod fast { // LLVM issue: https://bugs.llvm.org/show_bug.cgi?id=41722 #[cold] unsafe fn try_initialize T>(&self, init: F) -> Option<&'static T> { - // SAFETY: See comment above. + // SAFETY: See comment above (this function doc). if !mem::needs_drop::() || unsafe { self.try_register_dtor() } { - // SAFETY: See comment above. + // SAFETY: See comment above (his function doc). Some(unsafe { self.inner.initialize(init) }) } else { None From a06edda3ad9abd4f07d07bbe46cb488efeebbbd0 Mon Sep 17 00:00:00 2001 From: Tavian Barnes Date: Wed, 9 Sep 2020 10:16:03 -0400 Subject: [PATCH 04/44] Fix segfault if pthread_getattr_np fails glibc destroys[1] the passed pthread_attr_t if pthread_getattr_np() fails. Destroying it again leads to a segfault. Fix it by only destroying it on success for glibc. [1]: https://sourceware.org/git/?p=glibc.git;a=blob;f=nptl/pthread_getattr_np.c;h=ce437205e41dc05653e435f6188768cccdd91c99;hb=HEAD#l205 --- library/std/src/sys/unix/thread.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/library/std/src/sys/unix/thread.rs b/library/std/src/sys/unix/thread.rs index 04da9812ddc45..569aedd741192 100644 --- a/library/std/src/sys/unix/thread.rs +++ b/library/std/src/sys/unix/thread.rs @@ -305,7 +305,9 @@ pub mod guard { assert_eq!(libc::pthread_attr_getstack(&attr, &mut stackaddr, &mut stacksize), 0); ret = Some(stackaddr); } - assert_eq!(libc::pthread_attr_destroy(&mut attr), 0); + if e == 0 || cfg!(not(target_env = "gnu")) { + assert_eq!(libc::pthread_attr_destroy(&mut attr), 0); + } ret } @@ -446,7 +448,9 @@ pub mod guard { Some(stackaddr..stackaddr + guardsize) }; } - assert_eq!(libc::pthread_attr_destroy(&mut attr), 0); + if e == 0 || cfg!(not(target_env = "gnu")) { + assert_eq!(libc::pthread_attr_destroy(&mut attr), 0); + } ret } } From a684153f2920729f9fc3ea27ddb77d7cc3543214 Mon Sep 17 00:00:00 2001 From: Tavian Barnes Date: Wed, 9 Sep 2020 11:10:43 -0400 Subject: [PATCH 05/44] Only call pthread_attr_destroy() after getattr_np() succeeds on all libcs The calling convention of pthread_getattr_np() is to initialize the pthread_attr_t, so _destroy() is only necessary on success (and _init() isn't necessary beforehand). On the other hand, FreeBSD wants the attr_t to be initialized before pthread_attr_get_np(), and therefore it should always be destroyed afterwards. --- library/std/src/sys/unix/thread.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/library/std/src/sys/unix/thread.rs b/library/std/src/sys/unix/thread.rs index 569aedd741192..652219e28f6e0 100644 --- a/library/std/src/sys/unix/thread.rs +++ b/library/std/src/sys/unix/thread.rs @@ -294,6 +294,7 @@ pub mod guard { unsafe fn get_stack_start() -> Option<*mut libc::c_void> { let mut ret = None; let mut attr: libc::pthread_attr_t = crate::mem::zeroed(); + #[cfg(target_os = "freebsd")] assert_eq!(libc::pthread_attr_init(&mut attr), 0); #[cfg(target_os = "freebsd")] let e = libc::pthread_attr_get_np(libc::pthread_self(), &mut attr); @@ -305,7 +306,7 @@ pub mod guard { assert_eq!(libc::pthread_attr_getstack(&attr, &mut stackaddr, &mut stacksize), 0); ret = Some(stackaddr); } - if e == 0 || cfg!(not(target_env = "gnu")) { + if e == 0 || cfg!(target_os = "freebsd") { assert_eq!(libc::pthread_attr_destroy(&mut attr), 0); } ret @@ -405,6 +406,7 @@ pub mod guard { pub unsafe fn current() -> Option { let mut ret = None; let mut attr: libc::pthread_attr_t = crate::mem::zeroed(); + #[cfg(target_os = "freebsd")] assert_eq!(libc::pthread_attr_init(&mut attr), 0); #[cfg(target_os = "freebsd")] let e = libc::pthread_attr_get_np(libc::pthread_self(), &mut attr); @@ -448,7 +450,7 @@ pub mod guard { Some(stackaddr..stackaddr + guardsize) }; } - if e == 0 || cfg!(not(target_env = "gnu")) { + if e == 0 || cfg!(target_os = "freebsd") { assert_eq!(libc::pthread_attr_destroy(&mut attr), 0); } ret From 8f27e3cb1b4140d9124d60df0850ef734e73b884 Mon Sep 17 00:00:00 2001 From: Christiaan Dirkx Date: Sun, 13 Sep 2020 01:55:34 +0200 Subject: [PATCH 06/44] Make some methods of `Pin` unstable const Make the following methods unstable const under the `const_pin` feature: - `new` - `new_unchecked` - `into_inner` - `into_inner_unchecked` - `get_ref` - `into_ref` Also adds tests for these methods in a const context. Tracking issue: #76654 --- library/core/src/lib.rs | 1 + library/core/src/pin.rs | 27 ++++++++++++++++----------- library/core/tests/lib.rs | 2 ++ library/core/tests/pin.rs | 21 +++++++++++++++++++++ 4 files changed, 40 insertions(+), 11 deletions(-) create mode 100644 library/core/tests/pin.rs diff --git a/library/core/src/lib.rs b/library/core/src/lib.rs index c3cadcbb01e31..696b6a64a9fec 100644 --- a/library/core/src/lib.rs +++ b/library/core/src/lib.rs @@ -80,6 +80,7 @@ #![feature(const_int_pow)] #![feature(constctlz)] #![feature(const_panic)] +#![feature(const_pin)] #![feature(const_fn_union)] #![feature(const_generics)] #![feature(const_option)] diff --git a/library/core/src/pin.rs b/library/core/src/pin.rs index 1cc1dfb014335..fa5b37edc36e6 100644 --- a/library/core/src/pin.rs +++ b/library/core/src/pin.rs @@ -471,9 +471,10 @@ impl> Pin

{ /// /// Unlike `Pin::new_unchecked`, this method is safe because the pointer /// `P` dereferences to an [`Unpin`] type, which cancels the pinning guarantees. - #[stable(feature = "pin", since = "1.33.0")] #[inline(always)] - pub fn new(pointer: P) -> Pin

{ + #[rustc_const_unstable(feature = "const_pin", issue = "76654")] + #[stable(feature = "pin", since = "1.33.0")] + pub const fn new(pointer: P) -> Pin

{ // SAFETY: the value pointed to is `Unpin`, and so has no requirements // around pinning. unsafe { Pin::new_unchecked(pointer) } @@ -483,9 +484,10 @@ impl> Pin

{ /// /// This requires that the data inside this `Pin` is [`Unpin`] so that we /// can ignore the pinning invariants when unwrapping it. - #[stable(feature = "pin_into_inner", since = "1.39.0")] #[inline(always)] - pub fn into_inner(pin: Pin

) -> P { + #[rustc_const_unstable(feature = "const_pin", issue = "76654")] + #[stable(feature = "pin_into_inner", since = "1.39.0")] + pub const fn into_inner(pin: Pin

) -> P { pin.pointer } } @@ -556,9 +558,10 @@ impl Pin

{ /// /// [`mem::swap`]: crate::mem::swap #[lang = "new_unchecked"] - #[stable(feature = "pin", since = "1.33.0")] #[inline(always)] - pub unsafe fn new_unchecked(pointer: P) -> Pin

{ + #[rustc_const_unstable(feature = "const_pin", issue = "76654")] + #[stable(feature = "pin", since = "1.33.0")] + pub const unsafe fn new_unchecked(pointer: P) -> Pin

{ Pin { pointer } } @@ -589,9 +592,10 @@ impl Pin

{ /// /// If the underlying data is [`Unpin`], [`Pin::into_inner`] should be used /// instead. - #[stable(feature = "pin_into_inner", since = "1.39.0")] #[inline(always)] - pub unsafe fn into_inner_unchecked(pin: Pin

) -> P { + #[rustc_const_unstable(feature = "const_pin", issue = "76654")] + #[stable(feature = "pin_into_inner", since = "1.39.0")] + pub const unsafe fn into_inner_unchecked(pin: Pin

) -> P { pin.pointer } } @@ -693,17 +697,18 @@ impl<'a, T: ?Sized> Pin<&'a T> { /// with the same lifetime as the original `Pin`. /// /// ["pinning projections"]: self#projections-and-structural-pinning - #[stable(feature = "pin", since = "1.33.0")] #[inline(always)] - pub fn get_ref(self) -> &'a T { + #[rustc_const_unstable(feature = "const_pin", issue = "76654")] + #[stable(feature = "pin", since = "1.33.0")] + pub const fn get_ref(self) -> &'a T { self.pointer } } impl<'a, T: ?Sized> Pin<&'a mut T> { /// Converts this `Pin<&mut T>` into a `Pin<&T>` with the same lifetime. - #[stable(feature = "pin", since = "1.33.0")] #[inline(always)] + #[stable(feature = "pin", since = "1.33.0")] pub fn into_ref(self) -> Pin<&'a T> { Pin { pointer: self.pointer } } diff --git a/library/core/tests/lib.rs b/library/core/tests/lib.rs index a2e294ace1860..b8d67d7266543 100644 --- a/library/core/tests/lib.rs +++ b/library/core/tests/lib.rs @@ -39,6 +39,7 @@ #![feature(iter_order_by)] #![feature(cmp_min_max_by)] #![feature(iter_map_while)] +#![feature(const_pin)] #![feature(const_slice_from_raw_parts)] #![feature(const_raw_ptr_deref)] #![feature(never_type)] @@ -74,6 +75,7 @@ mod num; mod ops; mod option; mod pattern; +mod pin; mod ptr; mod result; mod slice; diff --git a/library/core/tests/pin.rs b/library/core/tests/pin.rs new file mode 100644 index 0000000000000..1363353163829 --- /dev/null +++ b/library/core/tests/pin.rs @@ -0,0 +1,21 @@ +use core::pin::Pin; + +#[test] +fn pin_const() { + // test that the methods of `Pin` are usable in a const context + + const POINTER: &'static usize = &2; + + const PINNED: Pin<&'static usize> = Pin::new(POINTER); + const PINNED_UNCHECKED: Pin<&'static usize> = unsafe { Pin::new_unchecked(POINTER) }; + assert_eq!(PINNED_UNCHECKED, PINNED); + + const INNER: &'static usize = Pin::into_inner(PINNED); + assert_eq!(INNER, POINTER); + + const INNER_UNCHECKED: &'static usize = unsafe { Pin::into_inner_unchecked(PINNED) }; + assert_eq!(INNER_UNCHECKED, POINTER); + + const REF: &'static usize = PINNED.get_ref(); + assert_eq!(REF, POINTER) +} From e5447a22222ecc6a650e75282cb9931b910854b2 Mon Sep 17 00:00:00 2001 From: Simon Vandel Sillesen Date: Sun, 13 Sep 2020 10:47:20 +0200 Subject: [PATCH 07/44] Fix #76432 Only insert StorageDeads if we actually removed one. Fixes an issue where we added StorageDead to a place with no StorageLive --- .../transform/simplify_comparison_integral.rs | 43 +++---- src/test/mir-opt/issue_76432.rs | 16 +++ ...76432.test.SimplifyComparisonIntegral.diff | 116 ++++++++++++++++++ 3 files changed, 154 insertions(+), 21 deletions(-) create mode 100644 src/test/mir-opt/issue_76432.rs create mode 100644 src/test/mir-opt/issue_76432.test.SimplifyComparisonIntegral.diff diff --git a/compiler/rustc_mir/src/transform/simplify_comparison_integral.rs b/compiler/rustc_mir/src/transform/simplify_comparison_integral.rs index a450a75d091ef..9b460c9ecb1be 100644 --- a/compiler/rustc_mir/src/transform/simplify_comparison_integral.rs +++ b/compiler/rustc_mir/src/transform/simplify_comparison_integral.rs @@ -61,26 +61,6 @@ impl<'tcx> MirPass<'tcx> for SimplifyComparisonIntegral { _ => unreachable!(), } - let terminator = bb.terminator_mut(); - - // add StorageDead for the place switched on at the top of each target - for bb_idx in new_targets.iter() { - storage_deads_to_insert.push(( - *bb_idx, - Statement { - source_info: terminator.source_info, - kind: StatementKind::StorageDead(opt.to_switch_on.local), - }, - )); - } - - terminator.kind = TerminatorKind::SwitchInt { - discr: Operand::Move(opt.to_switch_on), - switch_ty: opt.branch_value_ty, - values: vec![new_value].into(), - targets: new_targets, - }; - // delete comparison statement if it the value being switched on was moved, which means it can not be user later on if opt.can_remove_bin_op_stmt { bb.statements[opt.bin_op_stmt_idx].make_nop(); @@ -106,14 +86,35 @@ impl<'tcx> MirPass<'tcx> for SimplifyComparisonIntegral { } } + let terminator = bb.terminator(); + // remove StorageDead (if it exists) being used in the assign of the comparison for (stmt_idx, stmt) in bb.statements.iter().enumerate() { if !matches!(stmt.kind, StatementKind::StorageDead(local) if local == opt.to_switch_on.local) { continue; } - storage_deads_to_remove.push((stmt_idx, opt.bb_idx)) + storage_deads_to_remove.push((stmt_idx, opt.bb_idx)); + // if we have StorageDeads to remove then make sure to insert them at the top of each target + for bb_idx in new_targets.iter() { + storage_deads_to_insert.push(( + *bb_idx, + Statement { + source_info: terminator.source_info, + kind: StatementKind::StorageDead(opt.to_switch_on.local), + }, + )); + } } + + let terminator = bb.terminator_mut(); + + terminator.kind = TerminatorKind::SwitchInt { + discr: Operand::Move(opt.to_switch_on), + switch_ty: opt.branch_value_ty, + values: vec![new_value].into(), + targets: new_targets, + }; } for (idx, bb_idx) in storage_deads_to_remove { diff --git a/src/test/mir-opt/issue_76432.rs b/src/test/mir-opt/issue_76432.rs new file mode 100644 index 0000000000000..c8b405ca8eaaf --- /dev/null +++ b/src/test/mir-opt/issue_76432.rs @@ -0,0 +1,16 @@ +// Check that we do not insert StorageDead at each target if StorageDead was never seen + +// EMIT_MIR issue_76432.test.SimplifyComparisonIntegral.diff +use std::fmt::Debug; + +fn test(x: T) { + let v: &[T] = &[x, x, x]; + match v { + [ref v1, ref v2, ref v3] => [v1 as *const _, v2 as *const _, v3 as *const _], + _ => unreachable!(), + }; +} + +fn main() { + test(0u32); +} diff --git a/src/test/mir-opt/issue_76432.test.SimplifyComparisonIntegral.diff b/src/test/mir-opt/issue_76432.test.SimplifyComparisonIntegral.diff new file mode 100644 index 0000000000000..499134b69919f --- /dev/null +++ b/src/test/mir-opt/issue_76432.test.SimplifyComparisonIntegral.diff @@ -0,0 +1,116 @@ +- // MIR for `test` before SimplifyComparisonIntegral ++ // MIR for `test` after SimplifyComparisonIntegral + + fn test(_1: T) -> () { + debug x => _1; // in scope 0 at $DIR/issue_76432.rs:6:38: 6:39 + let mut _0: (); // return place in scope 0 at $DIR/issue_76432.rs:6:44: 6:44 + let _2: &[T]; // in scope 0 at $DIR/issue_76432.rs:7:9: 7:10 + let mut _3: &[T; 3]; // in scope 0 at $DIR/issue_76432.rs:7:19: 7:29 + let _4: &[T; 3]; // in scope 0 at $DIR/issue_76432.rs:7:19: 7:29 + let _5: [T; 3]; // in scope 0 at $DIR/issue_76432.rs:7:20: 7:29 + let mut _6: T; // in scope 0 at $DIR/issue_76432.rs:7:21: 7:22 + let mut _7: T; // in scope 0 at $DIR/issue_76432.rs:7:24: 7:25 + let mut _8: T; // in scope 0 at $DIR/issue_76432.rs:7:27: 7:28 + let _9: [*const T; 3]; // in scope 0 at $DIR/issue_76432.rs:8:5: 11:6 + let mut _10: usize; // in scope 0 at $DIR/issue_76432.rs:9:9: 9:33 + let mut _11: usize; // in scope 0 at $DIR/issue_76432.rs:9:9: 9:33 + let mut _12: bool; // in scope 0 at $DIR/issue_76432.rs:9:9: 9:33 + let mut _16: *const T; // in scope 0 at $DIR/issue_76432.rs:9:38: 9:52 + let mut _17: *const T; // in scope 0 at $DIR/issue_76432.rs:9:38: 9:52 + let mut _18: *const T; // in scope 0 at $DIR/issue_76432.rs:9:54: 9:68 + let mut _19: *const T; // in scope 0 at $DIR/issue_76432.rs:9:54: 9:68 + let mut _20: *const T; // in scope 0 at $DIR/issue_76432.rs:9:70: 9:84 + let mut _21: *const T; // in scope 0 at $DIR/issue_76432.rs:9:70: 9:84 + let mut _22: !; // in scope 0 at $SRC_DIR/std/src/macros.rs:LL:COL + scope 1 { + debug v => _2; // in scope 1 at $DIR/issue_76432.rs:7:9: 7:10 + let _13: &T; // in scope 1 at $DIR/issue_76432.rs:9:10: 9:16 + let _14: &T; // in scope 1 at $DIR/issue_76432.rs:9:18: 9:24 + let _15: &T; // in scope 1 at $DIR/issue_76432.rs:9:26: 9:32 + scope 2 { + debug v1 => _13; // in scope 2 at $DIR/issue_76432.rs:9:10: 9:16 + debug v2 => _14; // in scope 2 at $DIR/issue_76432.rs:9:18: 9:24 + debug v3 => _15; // in scope 2 at $DIR/issue_76432.rs:9:26: 9:32 + } + } + + bb0: { + StorageLive(_2); // scope 0 at $DIR/issue_76432.rs:7:9: 7:10 + StorageLive(_3); // scope 0 at $DIR/issue_76432.rs:7:19: 7:29 + StorageLive(_4); // scope 0 at $DIR/issue_76432.rs:7:19: 7:29 + StorageLive(_5); // scope 0 at $DIR/issue_76432.rs:7:20: 7:29 + StorageLive(_6); // scope 0 at $DIR/issue_76432.rs:7:21: 7:22 + _6 = _1; // scope 0 at $DIR/issue_76432.rs:7:21: 7:22 + StorageLive(_7); // scope 0 at $DIR/issue_76432.rs:7:24: 7:25 + _7 = _1; // scope 0 at $DIR/issue_76432.rs:7:24: 7:25 + StorageLive(_8); // scope 0 at $DIR/issue_76432.rs:7:27: 7:28 + _8 = _1; // scope 0 at $DIR/issue_76432.rs:7:27: 7:28 + _5 = [move _6, move _7, move _8]; // scope 0 at $DIR/issue_76432.rs:7:20: 7:29 + StorageDead(_8); // scope 0 at $DIR/issue_76432.rs:7:28: 7:29 + StorageDead(_7); // scope 0 at $DIR/issue_76432.rs:7:28: 7:29 + StorageDead(_6); // scope 0 at $DIR/issue_76432.rs:7:28: 7:29 + _4 = &_5; // scope 0 at $DIR/issue_76432.rs:7:19: 7:29 + _3 = _4; // scope 0 at $DIR/issue_76432.rs:7:19: 7:29 + _2 = move _3 as &[T] (Pointer(Unsize)); // scope 0 at $DIR/issue_76432.rs:7:19: 7:29 + StorageDead(_3); // scope 0 at $DIR/issue_76432.rs:7:28: 7:29 + StorageDead(_4); // scope 0 at $DIR/issue_76432.rs:7:29: 7:30 + StorageLive(_9); // scope 1 at $DIR/issue_76432.rs:8:5: 11:6 + _10 = Len((*_2)); // scope 1 at $DIR/issue_76432.rs:9:9: 9:33 + _11 = const 3_usize; // scope 1 at $DIR/issue_76432.rs:9:9: 9:33 +- _12 = Eq(move _10, const 3_usize); // scope 1 at $DIR/issue_76432.rs:9:9: 9:33 +- switchInt(move _12) -> [false: bb1, otherwise: bb2]; // scope 1 at $DIR/issue_76432.rs:9:9: 9:33 ++ nop; // scope 1 at $DIR/issue_76432.rs:9:9: 9:33 ++ switchInt(move _10) -> [3_usize: bb2, otherwise: bb1]; // scope 1 at $DIR/issue_76432.rs:9:9: 9:33 + } + + bb1: { + StorageLive(_22); // scope 1 at $SRC_DIR/std/src/macros.rs:LL:COL + begin_panic::<&str>(const "internal error: entered unreachable code"); // scope 1 at $SRC_DIR/std/src/macros.rs:LL:COL + // mir::Constant + // + span: $SRC_DIR/std/src/macros.rs:LL:COL + // + literal: Const { ty: fn(&str) -> ! {std::rt::begin_panic::<&str>}, val: Value(Scalar()) } + // ty::Const + // + ty: &str + // + val: Value(Slice { data: Allocation { bytes: [105, 110, 116, 101, 114, 110, 97, 108, 32, 101, 114, 114, 111, 114, 58, 32, 101, 110, 116, 101, 114, 101, 100, 32, 117, 110, 114, 101, 97, 99, 104, 97, 98, 108, 101, 32, 99, 111, 100, 101], relocations: Relocations(SortedMap { data: [] }), init_mask: InitMask { blocks: [1099511627775], len: Size { raw: 40 } }, size: Size { raw: 40 }, align: Align { pow2: 0 }, mutability: Not, extra: () }, start: 0, end: 40 }) + // mir::Constant + // + span: $SRC_DIR/core/src/macros/mod.rs:LL:COL + // + literal: Const { ty: &str, val: Value(Slice { data: Allocation { bytes: [105, 110, 116, 101, 114, 110, 97, 108, 32, 101, 114, 114, 111, 114, 58, 32, 101, 110, 116, 101, 114, 101, 100, 32, 117, 110, 114, 101, 97, 99, 104, 97, 98, 108, 101, 32, 99, 111, 100, 101], relocations: Relocations(SortedMap { data: [] }), init_mask: InitMask { blocks: [1099511627775], len: Size { raw: 40 } }, size: Size { raw: 40 }, align: Align { pow2: 0 }, mutability: Not, extra: () }, start: 0, end: 40 }) } + } + + bb2: { + StorageLive(_13); // scope 1 at $DIR/issue_76432.rs:9:10: 9:16 + _13 = &(*_2)[0 of 3]; // scope 1 at $DIR/issue_76432.rs:9:10: 9:16 + StorageLive(_14); // scope 1 at $DIR/issue_76432.rs:9:18: 9:24 + _14 = &(*_2)[1 of 3]; // scope 1 at $DIR/issue_76432.rs:9:18: 9:24 + StorageLive(_15); // scope 1 at $DIR/issue_76432.rs:9:26: 9:32 + _15 = &(*_2)[2 of 3]; // scope 1 at $DIR/issue_76432.rs:9:26: 9:32 + StorageLive(_16); // scope 2 at $DIR/issue_76432.rs:9:38: 9:52 + StorageLive(_17); // scope 2 at $DIR/issue_76432.rs:9:38: 9:52 + _17 = &raw const (*_13); // scope 2 at $DIR/issue_76432.rs:9:38: 9:40 + _16 = _17; // scope 2 at $DIR/issue_76432.rs:9:38: 9:52 + StorageLive(_18); // scope 2 at $DIR/issue_76432.rs:9:54: 9:68 + StorageLive(_19); // scope 2 at $DIR/issue_76432.rs:9:54: 9:68 + _19 = &raw const (*_14); // scope 2 at $DIR/issue_76432.rs:9:54: 9:56 + _18 = _19; // scope 2 at $DIR/issue_76432.rs:9:54: 9:68 + StorageLive(_20); // scope 2 at $DIR/issue_76432.rs:9:70: 9:84 + StorageLive(_21); // scope 2 at $DIR/issue_76432.rs:9:70: 9:84 + _21 = &raw const (*_15); // scope 2 at $DIR/issue_76432.rs:9:70: 9:72 + _20 = _21; // scope 2 at $DIR/issue_76432.rs:9:70: 9:84 + _9 = [move _16, move _18, move _20]; // scope 2 at $DIR/issue_76432.rs:9:37: 9:85 + StorageDead(_21); // scope 2 at $DIR/issue_76432.rs:9:84: 9:85 + StorageDead(_20); // scope 2 at $DIR/issue_76432.rs:9:84: 9:85 + StorageDead(_19); // scope 2 at $DIR/issue_76432.rs:9:84: 9:85 + StorageDead(_18); // scope 2 at $DIR/issue_76432.rs:9:84: 9:85 + StorageDead(_17); // scope 2 at $DIR/issue_76432.rs:9:84: 9:85 + StorageDead(_16); // scope 2 at $DIR/issue_76432.rs:9:84: 9:85 + StorageDead(_15); // scope 1 at $DIR/issue_76432.rs:9:84: 9:85 + StorageDead(_14); // scope 1 at $DIR/issue_76432.rs:9:84: 9:85 + StorageDead(_13); // scope 1 at $DIR/issue_76432.rs:9:84: 9:85 + StorageDead(_9); // scope 1 at $DIR/issue_76432.rs:11:6: 11:7 + _0 = const (); // scope 0 at $DIR/issue_76432.rs:6:44: 12:2 + StorageDead(_5); // scope 0 at $DIR/issue_76432.rs:12:1: 12:2 + StorageDead(_2); // scope 0 at $DIR/issue_76432.rs:12:1: 12:2 + return; // scope 0 at $DIR/issue_76432.rs:12:2: 12:2 + } + } + From 9c5d0c18a73163a34c5a3fae8544537a2fed83ac Mon Sep 17 00:00:00 2001 From: Simon Vandel Sillesen Date: Sun, 13 Sep 2020 16:04:45 +0200 Subject: [PATCH 08/44] MIR pass to remove unneeded drops on types not needing drop This is heavily dependent on MIR inlining running to actually see the drop statement --- compiler/rustc_mir/src/transform/mod.rs | 2 + .../src/transform/remove_unneeded_drops.rs | 57 +++++++++++++++++++ ...annot_opt_generic.RemoveUnneededDrops.diff | 32 +++++++++++ ...ed_drops.dont_opt.RemoveUnneededDrops.diff | 32 +++++++++++ ...nneeded_drops.opt.RemoveUnneededDrops.diff | 29 ++++++++++ ....opt_generic_copy.RemoveUnneededDrops.diff | 29 ++++++++++ src/test/mir-opt/remove_unneeded_drops.rs | 28 +++++++++ 7 files changed, 209 insertions(+) create mode 100644 compiler/rustc_mir/src/transform/remove_unneeded_drops.rs create mode 100644 src/test/mir-opt/remove_unneeded_drops.cannot_opt_generic.RemoveUnneededDrops.diff create mode 100644 src/test/mir-opt/remove_unneeded_drops.dont_opt.RemoveUnneededDrops.diff create mode 100644 src/test/mir-opt/remove_unneeded_drops.opt.RemoveUnneededDrops.diff create mode 100644 src/test/mir-opt/remove_unneeded_drops.opt_generic_copy.RemoveUnneededDrops.diff create mode 100644 src/test/mir-opt/remove_unneeded_drops.rs diff --git a/compiler/rustc_mir/src/transform/mod.rs b/compiler/rustc_mir/src/transform/mod.rs index 8025b7c02043d..3e831a48c2577 100644 --- a/compiler/rustc_mir/src/transform/mod.rs +++ b/compiler/rustc_mir/src/transform/mod.rs @@ -36,6 +36,7 @@ pub mod nrvo; pub mod promote_consts; pub mod qualify_min_const_fn; pub mod remove_noop_landing_pads; +pub mod remove_unneeded_drops; pub mod required_consts; pub mod rustc_peek; pub mod simplify; @@ -455,6 +456,7 @@ fn run_optimization_passes<'tcx>( // The main optimizations that we do on MIR. let optimizations: &[&dyn MirPass<'tcx>] = &[ + &remove_unneeded_drops::RemoveUnneededDrops::new(def_id), &match_branches::MatchBranchSimplification, // inst combine is after MatchBranchSimplification to clean up Ne(_1, false) &instcombine::InstCombine, diff --git a/compiler/rustc_mir/src/transform/remove_unneeded_drops.rs b/compiler/rustc_mir/src/transform/remove_unneeded_drops.rs new file mode 100644 index 0000000000000..0f023d71dd1da --- /dev/null +++ b/compiler/rustc_mir/src/transform/remove_unneeded_drops.rs @@ -0,0 +1,57 @@ +//! This pass replaces a drop of a type that does not need dropping, with a goto + +use crate::transform::{MirPass, MirSource}; +use rustc_hir::def_id::LocalDefId; +use rustc_middle::mir::visit::Visitor; +use rustc_middle::mir::*; +use rustc_middle::ty::TyCtxt; + +pub struct RemoveUnneededDrops { + def_id: LocalDefId, +} + +impl RemoveUnneededDrops { + pub fn new(def_id: LocalDefId) -> Self { + Self { def_id } + } +} + +impl<'tcx> MirPass<'tcx> for RemoveUnneededDrops { + fn run_pass(&self, tcx: TyCtxt<'tcx>, source: MirSource<'tcx>, body: &mut Body<'tcx>) { + trace!("Running SimplifyComparisonIntegral on {:?}", source); + let mut opt_finder = RemoveUnneededDropsOptimizationFinder { + tcx, + body, + optimizations: vec![], + def_id: self.def_id, + }; + opt_finder.visit_body(body); + for (loc, target) in opt_finder.optimizations { + let terminator = body.basic_blocks_mut()[loc.block].terminator_mut(); + debug!("SUCCESS: replacing `drop` with goto({:?})", target); + terminator.kind = TerminatorKind::Goto { target }; + } + } +} + +impl<'a, 'tcx> Visitor<'tcx> for RemoveUnneededDropsOptimizationFinder<'a, 'tcx> { + fn visit_terminator(&mut self, terminator: &Terminator<'tcx>, location: Location) { + match terminator.kind { + TerminatorKind::Drop { place, target, .. } => { + let ty = place.ty(self.body, self.tcx); + let needs_drop = ty.ty.needs_drop(self.tcx, self.tcx.param_env(self.def_id)); + if !needs_drop { + self.optimizations.push((location, target)); + } + } + _ => {} + } + self.super_terminator(terminator, location); + } +} +pub struct RemoveUnneededDropsOptimizationFinder<'a, 'tcx> { + tcx: TyCtxt<'tcx>, + body: &'a Body<'tcx>, + optimizations: Vec<(Location, BasicBlock)>, + def_id: LocalDefId, +} diff --git a/src/test/mir-opt/remove_unneeded_drops.cannot_opt_generic.RemoveUnneededDrops.diff b/src/test/mir-opt/remove_unneeded_drops.cannot_opt_generic.RemoveUnneededDrops.diff new file mode 100644 index 0000000000000..545ad2794ee17 --- /dev/null +++ b/src/test/mir-opt/remove_unneeded_drops.cannot_opt_generic.RemoveUnneededDrops.diff @@ -0,0 +1,32 @@ +- // MIR for `cannot_opt_generic` before RemoveUnneededDrops ++ // MIR for `cannot_opt_generic` after RemoveUnneededDrops + + fn cannot_opt_generic(_1: T) -> () { + debug x => _1; // in scope 0 at $DIR/remove_unneeded_drops.rs:19:26: 19:27 + let mut _0: (); // return place in scope 0 at $DIR/remove_unneeded_drops.rs:19:32: 19:32 + let _2: (); // in scope 0 at $DIR/remove_unneeded_drops.rs:20:5: 20:12 + let mut _3: T; // in scope 0 at $DIR/remove_unneeded_drops.rs:20:10: 20:11 + scope 1 { + debug _x => _3; // in scope 1 at $SRC_DIR/core/src/mem/mod.rs:LL:COL + } + + bb0: { + StorageLive(_2); // scope 0 at $DIR/remove_unneeded_drops.rs:20:5: 20:12 + StorageLive(_3); // scope 0 at $DIR/remove_unneeded_drops.rs:20:10: 20:11 + _3 = move _1; // scope 0 at $DIR/remove_unneeded_drops.rs:20:10: 20:11 + _2 = const (); // scope 1 at $SRC_DIR/core/src/mem/mod.rs:LL:COL + drop(_3) -> [return: bb2, unwind: bb1]; // scope 1 at $SRC_DIR/core/src/mem/mod.rs:LL:COL + } + + bb1 (cleanup): { + resume; // scope 0 at $DIR/remove_unneeded_drops.rs:19:1: 21:2 + } + + bb2: { + StorageDead(_3); // scope 0 at $DIR/remove_unneeded_drops.rs:20:11: 20:12 + StorageDead(_2); // scope 0 at $DIR/remove_unneeded_drops.rs:20:12: 20:13 + _0 = const (); // scope 0 at $DIR/remove_unneeded_drops.rs:19:32: 21:2 + return; // scope 0 at $DIR/remove_unneeded_drops.rs:21:2: 21:2 + } + } + diff --git a/src/test/mir-opt/remove_unneeded_drops.dont_opt.RemoveUnneededDrops.diff b/src/test/mir-opt/remove_unneeded_drops.dont_opt.RemoveUnneededDrops.diff new file mode 100644 index 0000000000000..78d65d13bd1bf --- /dev/null +++ b/src/test/mir-opt/remove_unneeded_drops.dont_opt.RemoveUnneededDrops.diff @@ -0,0 +1,32 @@ +- // MIR for `dont_opt` before RemoveUnneededDrops ++ // MIR for `dont_opt` after RemoveUnneededDrops + + fn dont_opt(_1: Vec) -> () { + debug x => _1; // in scope 0 at $DIR/remove_unneeded_drops.rs:7:13: 7:14 + let mut _0: (); // return place in scope 0 at $DIR/remove_unneeded_drops.rs:7:27: 7:27 + let _2: (); // in scope 0 at $DIR/remove_unneeded_drops.rs:8:5: 8:12 + let mut _3: std::vec::Vec; // in scope 0 at $DIR/remove_unneeded_drops.rs:8:10: 8:11 + scope 1 { + debug _x => _3; // in scope 1 at $SRC_DIR/core/src/mem/mod.rs:LL:COL + } + + bb0: { + StorageLive(_2); // scope 0 at $DIR/remove_unneeded_drops.rs:8:5: 8:12 + StorageLive(_3); // scope 0 at $DIR/remove_unneeded_drops.rs:8:10: 8:11 + _3 = move _1; // scope 0 at $DIR/remove_unneeded_drops.rs:8:10: 8:11 + _2 = const (); // scope 1 at $SRC_DIR/core/src/mem/mod.rs:LL:COL + drop(_3) -> [return: bb2, unwind: bb1]; // scope 1 at $SRC_DIR/core/src/mem/mod.rs:LL:COL + } + + bb1 (cleanup): { + resume; // scope 0 at $DIR/remove_unneeded_drops.rs:7:1: 9:2 + } + + bb2: { + StorageDead(_3); // scope 0 at $DIR/remove_unneeded_drops.rs:8:11: 8:12 + StorageDead(_2); // scope 0 at $DIR/remove_unneeded_drops.rs:8:12: 8:13 + _0 = const (); // scope 0 at $DIR/remove_unneeded_drops.rs:7:27: 9:2 + return; // scope 0 at $DIR/remove_unneeded_drops.rs:9:2: 9:2 + } + } + diff --git a/src/test/mir-opt/remove_unneeded_drops.opt.RemoveUnneededDrops.diff b/src/test/mir-opt/remove_unneeded_drops.opt.RemoveUnneededDrops.diff new file mode 100644 index 0000000000000..34193ccc5c7e7 --- /dev/null +++ b/src/test/mir-opt/remove_unneeded_drops.opt.RemoveUnneededDrops.diff @@ -0,0 +1,29 @@ +- // MIR for `opt` before RemoveUnneededDrops ++ // MIR for `opt` after RemoveUnneededDrops + + fn opt(_1: bool) -> () { + debug x => _1; // in scope 0 at $DIR/remove_unneeded_drops.rs:2:8: 2:9 + let mut _0: (); // return place in scope 0 at $DIR/remove_unneeded_drops.rs:2:17: 2:17 + let _2: (); // in scope 0 at $DIR/remove_unneeded_drops.rs:3:5: 3:12 + let mut _3: bool; // in scope 0 at $DIR/remove_unneeded_drops.rs:3:10: 3:11 + scope 1 { + debug _x => _3; // in scope 1 at $SRC_DIR/core/src/mem/mod.rs:LL:COL + } + + bb0: { + StorageLive(_2); // scope 0 at $DIR/remove_unneeded_drops.rs:3:5: 3:12 + StorageLive(_3); // scope 0 at $DIR/remove_unneeded_drops.rs:3:10: 3:11 + _3 = _1; // scope 0 at $DIR/remove_unneeded_drops.rs:3:10: 3:11 + _2 = const (); // scope 1 at $SRC_DIR/core/src/mem/mod.rs:LL:COL +- drop(_3) -> bb1; // scope 1 at $SRC_DIR/core/src/mem/mod.rs:LL:COL ++ goto -> bb1; // scope 1 at $SRC_DIR/core/src/mem/mod.rs:LL:COL + } + + bb1: { + StorageDead(_3); // scope 0 at $DIR/remove_unneeded_drops.rs:3:11: 3:12 + StorageDead(_2); // scope 0 at $DIR/remove_unneeded_drops.rs:3:12: 3:13 + _0 = const (); // scope 0 at $DIR/remove_unneeded_drops.rs:2:17: 4:2 + return; // scope 0 at $DIR/remove_unneeded_drops.rs:4:2: 4:2 + } + } + diff --git a/src/test/mir-opt/remove_unneeded_drops.opt_generic_copy.RemoveUnneededDrops.diff b/src/test/mir-opt/remove_unneeded_drops.opt_generic_copy.RemoveUnneededDrops.diff new file mode 100644 index 0000000000000..2e6c8c8a78a72 --- /dev/null +++ b/src/test/mir-opt/remove_unneeded_drops.opt_generic_copy.RemoveUnneededDrops.diff @@ -0,0 +1,29 @@ +- // MIR for `opt_generic_copy` before RemoveUnneededDrops ++ // MIR for `opt_generic_copy` after RemoveUnneededDrops + + fn opt_generic_copy(_1: T) -> () { + debug x => _1; // in scope 0 at $DIR/remove_unneeded_drops.rs:12:30: 12:31 + let mut _0: (); // return place in scope 0 at $DIR/remove_unneeded_drops.rs:12:36: 12:36 + let _2: (); // in scope 0 at $DIR/remove_unneeded_drops.rs:13:5: 13:12 + let mut _3: T; // in scope 0 at $DIR/remove_unneeded_drops.rs:13:10: 13:11 + scope 1 { + debug _x => _3; // in scope 1 at $SRC_DIR/core/src/mem/mod.rs:LL:COL + } + + bb0: { + StorageLive(_2); // scope 0 at $DIR/remove_unneeded_drops.rs:13:5: 13:12 + StorageLive(_3); // scope 0 at $DIR/remove_unneeded_drops.rs:13:10: 13:11 + _3 = _1; // scope 0 at $DIR/remove_unneeded_drops.rs:13:10: 13:11 + _2 = const (); // scope 1 at $SRC_DIR/core/src/mem/mod.rs:LL:COL +- drop(_3) -> bb1; // scope 1 at $SRC_DIR/core/src/mem/mod.rs:LL:COL ++ goto -> bb1; // scope 1 at $SRC_DIR/core/src/mem/mod.rs:LL:COL + } + + bb1: { + StorageDead(_3); // scope 0 at $DIR/remove_unneeded_drops.rs:13:11: 13:12 + StorageDead(_2); // scope 0 at $DIR/remove_unneeded_drops.rs:13:12: 13:13 + _0 = const (); // scope 0 at $DIR/remove_unneeded_drops.rs:12:36: 14:2 + return; // scope 0 at $DIR/remove_unneeded_drops.rs:14:2: 14:2 + } + } + diff --git a/src/test/mir-opt/remove_unneeded_drops.rs b/src/test/mir-opt/remove_unneeded_drops.rs new file mode 100644 index 0000000000000..17d7e79a1eb0a --- /dev/null +++ b/src/test/mir-opt/remove_unneeded_drops.rs @@ -0,0 +1,28 @@ +// EMIT_MIR remove_unneeded_drops.opt.RemoveUnneededDrops.diff +fn opt(x: bool) { + drop(x); +} + +// EMIT_MIR remove_unneeded_drops.dont_opt.RemoveUnneededDrops.diff +fn dont_opt(x: Vec) { + drop(x); +} + +// EMIT_MIR remove_unneeded_drops.opt_generic_copy.RemoveUnneededDrops.diff +fn opt_generic_copy(x: T) { + drop(x); +} + +// EMIT_MIR remove_unneeded_drops.cannot_opt_generic.RemoveUnneededDrops.diff +// since the pass is not running on monomorphisized code, +// we can't (but probably should) optimize this +fn cannot_opt_generic(x: T) { + drop(x); +} + +fn main() { + opt(true); + opt_generic_copy(42); + cannot_opt_generic(42); + dont_opt(vec![true]); +} From 9d47ecf327edbb5dd1b11dd112335506d8d26165 Mon Sep 17 00:00:00 2001 From: Simon Vandel Sillesen Date: Mon, 14 Sep 2020 22:56:39 +0200 Subject: [PATCH 09/44] Suggestion from review Co-authored-by: Andreas Jonson --- compiler/rustc_mir/src/transform/remove_unneeded_drops.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/rustc_mir/src/transform/remove_unneeded_drops.rs b/compiler/rustc_mir/src/transform/remove_unneeded_drops.rs index 0f023d71dd1da..393b0f923b910 100644 --- a/compiler/rustc_mir/src/transform/remove_unneeded_drops.rs +++ b/compiler/rustc_mir/src/transform/remove_unneeded_drops.rs @@ -18,7 +18,7 @@ impl RemoveUnneededDrops { impl<'tcx> MirPass<'tcx> for RemoveUnneededDrops { fn run_pass(&self, tcx: TyCtxt<'tcx>, source: MirSource<'tcx>, body: &mut Body<'tcx>) { - trace!("Running SimplifyComparisonIntegral on {:?}", source); + trace!("Running RemoveUnneededDrops on {:?}", source); let mut opt_finder = RemoveUnneededDropsOptimizationFinder { tcx, body, From eede953c283c7bbe903a0e8abb44c923baf5cfac Mon Sep 17 00:00:00 2001 From: Lzu Tao Date: Thu, 10 Sep 2020 03:10:36 +0000 Subject: [PATCH 10/44] Only get ImplKind::Impl once --- src/librustdoc/clean/inline.rs | 41 ++++++++++++++++++---------------- 1 file changed, 22 insertions(+), 19 deletions(-) diff --git a/src/librustdoc/clean/inline.rs b/src/librustdoc/clean/inline.rs index f8987c6beca33..931355b82f503 100644 --- a/src/librustdoc/clean/inline.rs +++ b/src/librustdoc/clean/inline.rs @@ -350,14 +350,22 @@ pub fn build_impl( } } - let for_ = if let Some(did) = did.as_local() { - let hir_id = tcx.hir().local_def_id_to_hir_id(did); - match tcx.hir().expect_item(hir_id).kind { - hir::ItemKind::Impl { self_ty, .. } => self_ty.clean(cx), - _ => panic!("did given to build_impl was not an impl"), + let impl_item = match did.as_local() { + Some(did) => { + let hir_id = tcx.hir().local_def_id_to_hir_id(did); + match tcx.hir().expect_item(hir_id).kind { + hir::ItemKind::Impl { self_ty, ref generics, ref items, .. } => { + Some((self_ty, generics, items)) + } + _ => panic!("`DefID` passed to `build_impl` is not an `impl"), + } } - } else { - tcx.type_of(did).clean(cx) + None => None, + }; + + let for_ = match impl_item { + Some((self_ty, _, _)) => self_ty.clean(cx), + None => tcx.type_of(did).clean(cx), }; // Only inline impl if the implementing type is @@ -377,17 +385,12 @@ pub fn build_impl( } let predicates = tcx.explicit_predicates_of(did); - let (trait_items, generics) = if let Some(did) = did.as_local() { - let hir_id = tcx.hir().local_def_id_to_hir_id(did); - match tcx.hir().expect_item(hir_id).kind { - hir::ItemKind::Impl { ref generics, ref items, .. } => ( - items.iter().map(|item| tcx.hir().impl_item(item.id).clean(cx)).collect::>(), - generics.clean(cx), - ), - _ => panic!("did given to build_impl was not an impl"), - } - } else { - ( + let (trait_items, generics) = match impl_item { + Some((_, generics, items)) => ( + items.iter().map(|item| tcx.hir().impl_item(item.id).clean(cx)).collect::>(), + generics.clean(cx), + ), + None => ( tcx.associated_items(did) .in_definition_order() .filter_map(|item| { @@ -399,7 +402,7 @@ pub fn build_impl( }) .collect::>(), clean::enter_impl_trait(cx, || (tcx.generics_of(did), predicates).clean(cx)), - ) + ), }; let polarity = tcx.impl_polarity(did); let trait_ = associated_trait.clean(cx).map(|bound| match bound { From ed6c7efd87f17a7d9282f4bc7341cb5cbda8db4d Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Wed, 2 Sep 2020 13:25:19 -0700 Subject: [PATCH 11/44] Use enum for status of non-const ops --- .../src/transform/check_consts/ops.rs | 112 ++++++++++-------- 1 file changed, 61 insertions(+), 51 deletions(-) diff --git a/compiler/rustc_mir/src/transform/check_consts/ops.rs b/compiler/rustc_mir/src/transform/check_consts/ops.rs index ea025f208e49d..ff27d0c3a9211 100644 --- a/compiler/rustc_mir/src/transform/check_consts/ops.rs +++ b/compiler/rustc_mir/src/transform/check_consts/ops.rs @@ -14,35 +14,32 @@ use super::ConstCx; pub fn non_const(ccx: &ConstCx<'_, '_>, op: O, span: Span) { debug!("illegal_op: op={:?}", op); - if op.is_allowed_in_item(ccx) { - return; - } + let gate = match op.status_in_item(ccx) { + Status::Allowed => return, + Status::Unstable(gate) if ccx.tcx.features().enabled(gate) => return, + Status::Unstable(gate) => Some(gate), + Status::Forbidden => None, + }; if ccx.tcx.sess.opts.debugging_opts.unleash_the_miri_inside_of_you { - ccx.tcx.sess.miri_unleashed_feature(span, O::feature_gate()); + ccx.tcx.sess.miri_unleashed_feature(span, gate); return; } op.emit_error(ccx, span); } +pub enum Status { + Allowed, + Unstable(Symbol), + Forbidden, +} + /// An operation that is not *always* allowed in a const context. pub trait NonConstOp: std::fmt::Debug { - /// Returns the `Symbol` corresponding to the feature gate that would enable this operation, - /// or `None` if such a feature gate does not exist. - fn feature_gate() -> Option { - None - } - - /// Returns `true` if this operation is allowed in the given item. - /// - /// This check should assume that we are not in a non-const `fn`, where all operations are - /// legal. - /// - /// By default, it returns `true` if and only if this operation has a corresponding feature - /// gate and that gate is enabled. - fn is_allowed_in_item(&self, ccx: &ConstCx<'_, '_>) -> bool { - Self::feature_gate().map_or(false, |gate| ccx.tcx.features().enabled(gate)) + /// Returns an enum indicating whether this operation is allowed within the given item. + fn status_in_item(&self, _ccx: &ConstCx<'_, '_>) -> Status { + Status::Forbidden } fn emit_error(&self, ccx: &ConstCx<'_, '_>, span: Span) { @@ -53,9 +50,13 @@ pub trait NonConstOp: std::fmt::Debug { "{} contains unimplemented expression type", ccx.const_kind() ); - if let Some(feat) = Self::feature_gate() { - err.help(&format!("add `#![feature({})]` to the crate attributes to enable", feat)); + + if let Status::Unstable(gate) = self.status_in_item(ccx) { + if !ccx.tcx.features().enabled(gate) && nightly_options::is_nightly_build() { + err.help(&format!("add `#![feature({})]` to the crate attributes to enable", gate)); + } } + if ccx.tcx.sess.teach(&err.get_code().unwrap()) { err.note( "A function call isn't allowed in the const's initialization expression \ @@ -182,14 +183,13 @@ impl NonConstOp for CellBorrow { #[derive(Debug)] pub struct MutBorrow; impl NonConstOp for MutBorrow { - fn is_allowed_in_item(&self, ccx: &ConstCx<'_, '_>) -> bool { - // Forbid everywhere except in const fn - ccx.const_kind() == hir::ConstContext::ConstFn - && ccx.tcx.features().enabled(Self::feature_gate().unwrap()) - } - - fn feature_gate() -> Option { - Some(sym::const_mut_refs) + fn status_in_item(&self, ccx: &ConstCx<'_, '_>) -> Status { + // Forbid everywhere except in const fn with a feature gate + if ccx.const_kind() == hir::ConstContext::ConstFn { + Status::Unstable(sym::const_mut_refs) + } else { + Status::Forbidden + } } fn emit_error(&self, ccx: &ConstCx<'_, '_>, span: Span) { @@ -201,15 +201,16 @@ impl NonConstOp for MutBorrow { &format!("mutable references are not allowed in {}s", ccx.const_kind()), ) } else { - struct_span_err!( + let mut err = struct_span_err!( ccx.tcx.sess, span, E0764, "mutable references are not allowed in {}s", ccx.const_kind(), - ) + ); + err.span_label(span, format!("`&mut` is only allowed in `const fn`")); + err }; - err.span_label(span, "`&mut` is only allowed in `const fn`".to_string()); if ccx.tcx.sess.teach(&err.get_code().unwrap()) { err.note( "References in statics and constants may only refer \ @@ -226,11 +227,17 @@ impl NonConstOp for MutBorrow { } } +// FIXME(ecstaticmorse): Unify this with `MutBorrow`. It has basically the same issues. #[derive(Debug)] pub struct MutAddressOf; impl NonConstOp for MutAddressOf { - fn feature_gate() -> Option { - Some(sym::const_mut_refs) + fn status_in_item(&self, ccx: &ConstCx<'_, '_>) -> Status { + // Forbid everywhere except in const fn with a feature gate + if ccx.const_kind() == hir::ConstContext::ConstFn { + Status::Unstable(sym::const_mut_refs) + } else { + Status::Forbidden + } } fn emit_error(&self, ccx: &ConstCx<'_, '_>, span: Span) { @@ -247,16 +254,16 @@ impl NonConstOp for MutAddressOf { #[derive(Debug)] pub struct MutDeref; impl NonConstOp for MutDeref { - fn feature_gate() -> Option { - Some(sym::const_mut_refs) + fn status_in_item(&self, _: &ConstCx<'_, '_>) -> Status { + Status::Unstable(sym::const_mut_refs) } } #[derive(Debug)] pub struct Panic; impl NonConstOp for Panic { - fn feature_gate() -> Option { - Some(sym::const_panic) + fn status_in_item(&self, _: &ConstCx<'_, '_>) -> Status { + Status::Unstable(sym::const_panic) } fn emit_error(&self, ccx: &ConstCx<'_, '_>, span: Span) { @@ -289,8 +296,8 @@ impl NonConstOp for RawPtrComparison { #[derive(Debug)] pub struct RawPtrDeref; impl NonConstOp for RawPtrDeref { - fn feature_gate() -> Option { - Some(sym::const_raw_ptr_deref) + fn status_in_item(&self, _: &ConstCx<'_, '_>) -> Status { + Status::Unstable(sym::const_raw_ptr_deref) } fn emit_error(&self, ccx: &ConstCx<'_, '_>, span: Span) { @@ -307,8 +314,8 @@ impl NonConstOp for RawPtrDeref { #[derive(Debug)] pub struct RawPtrToIntCast; impl NonConstOp for RawPtrToIntCast { - fn feature_gate() -> Option { - Some(sym::const_raw_ptr_to_usize_cast) + fn status_in_item(&self, _: &ConstCx<'_, '_>) -> Status { + Status::Unstable(sym::const_raw_ptr_to_usize_cast) } fn emit_error(&self, ccx: &ConstCx<'_, '_>, span: Span) { @@ -326,8 +333,12 @@ impl NonConstOp for RawPtrToIntCast { #[derive(Debug)] pub struct StaticAccess; impl NonConstOp for StaticAccess { - fn is_allowed_in_item(&self, ccx: &ConstCx<'_, '_>) -> bool { - matches!(ccx.const_kind(), hir::ConstContext::Static(_)) + fn status_in_item(&self, ccx: &ConstCx<'_, '_>) -> Status { + if let hir::ConstContext::Static(_) = ccx.const_kind() { + Status::Allowed + } else { + Status::Forbidden + } } fn emit_error(&self, ccx: &ConstCx<'_, '_>, span: Span) { @@ -371,14 +382,13 @@ impl NonConstOp for ThreadLocalAccess { #[derive(Debug)] pub struct UnionAccess; impl NonConstOp for UnionAccess { - fn is_allowed_in_item(&self, ccx: &ConstCx<'_, '_>) -> bool { + fn status_in_item(&self, ccx: &ConstCx<'_, '_>) -> Status { // Union accesses are stable in all contexts except `const fn`. - ccx.const_kind() != hir::ConstContext::ConstFn - || ccx.tcx.features().enabled(Self::feature_gate().unwrap()) - } - - fn feature_gate() -> Option { - Some(sym::const_fn_union) + if ccx.const_kind() != hir::ConstContext::ConstFn { + Status::Allowed + } else { + Status::Unstable(sym::const_fn_union) + } } fn emit_error(&self, ccx: &ConstCx<'_, '_>, span: Span) { From c3607bd7dd323a898b8cc9d2c603dfd14172c73c Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Wed, 2 Sep 2020 14:35:02 -0700 Subject: [PATCH 12/44] Use helper function for searching `allow_internal_unstable` --- compiler/rustc_mir/src/transform/check_consts/mod.rs | 8 ++++++++ compiler/rustc_mir/src/transform/qualify_min_const_fn.rs | 7 ++----- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/compiler/rustc_mir/src/transform/check_consts/mod.rs b/compiler/rustc_mir/src/transform/check_consts/mod.rs index 81c1b0b5bd49f..c1b4cb5f1a8d5 100644 --- a/compiler/rustc_mir/src/transform/check_consts/mod.rs +++ b/compiler/rustc_mir/src/transform/check_consts/mod.rs @@ -4,10 +4,12 @@ //! has interior mutability or needs to be dropped, as well as the visitor that emits errors when //! it finds operations that are invalid in a certain context. +use rustc_attr as attr; use rustc_hir as hir; use rustc_hir::def_id::{DefId, LocalDefId}; use rustc_middle::mir; use rustc_middle::ty::{self, TyCtxt}; +use rustc_span::Symbol; pub use self::qualifs::Qualif; @@ -55,3 +57,9 @@ impl ConstCx<'mir, 'tcx> { pub fn is_lang_panic_fn(tcx: TyCtxt<'tcx>, def_id: DefId) -> bool { Some(def_id) == tcx.lang_items().panic_fn() || Some(def_id) == tcx.lang_items().begin_panic_fn() } + +pub fn allow_internal_unstable(tcx: TyCtxt<'tcx>, def_id: DefId, feature_gate: Symbol) -> bool { + let attrs = tcx.get_attrs(def_id); + attr::allow_internal_unstable(&tcx.sess, attrs) + .map_or(false, |mut features| features.any(|name| name == feature_gate)) +} diff --git a/compiler/rustc_mir/src/transform/qualify_min_const_fn.rs b/compiler/rustc_mir/src/transform/qualify_min_const_fn.rs index 5e102f5151d0c..f15a7f7c2c889 100644 --- a/compiler/rustc_mir/src/transform/qualify_min_const_fn.rs +++ b/compiler/rustc_mir/src/transform/qualify_min_const_fn.rs @@ -1,4 +1,3 @@ -use rustc_attr as attr; use rustc_hir as hir; use rustc_hir::def_id::DefId; use rustc_middle::mir::*; @@ -344,8 +343,7 @@ fn feature_allowed(tcx: TyCtxt<'tcx>, def_id: DefId, feature_gate: Symbol) -> bo // However, we cannot allow stable `const fn`s to use unstable features without an explicit // opt-in via `allow_internal_unstable`. - attr::allow_internal_unstable(&tcx.sess, &tcx.get_attrs(def_id)) - .map_or(false, |mut features| features.any(|name| name == feature_gate)) + super::check_consts::allow_internal_unstable(tcx, def_id, feature_gate) } /// Returns `true` if the given library feature gate is allowed within the function with the given `DefId`. @@ -364,8 +362,7 @@ pub fn lib_feature_allowed(tcx: TyCtxt<'tcx>, def_id: DefId, feature_gate: Symbo // However, we cannot allow stable `const fn`s to use unstable features without an explicit // opt-in via `allow_internal_unstable`. - attr::allow_internal_unstable(&tcx.sess, &tcx.get_attrs(def_id)) - .map_or(false, |mut features| features.any(|name| name == feature_gate)) + super::check_consts::allow_internal_unstable(tcx, def_id, feature_gate) } fn check_terminator( From e4edc161f20ccbc77fffb7ceb60ebb6102cbd747 Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Wed, 2 Sep 2020 14:54:55 -0700 Subject: [PATCH 13/44] Give name to extra `Span` in `LiveDrop` error --- compiler/rustc_mir/src/transform/check_consts/ops.rs | 6 ++++-- .../src/transform/check_consts/post_drop_elaboration.rs | 2 +- compiler/rustc_mir/src/transform/check_consts/validation.rs | 2 +- 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/compiler/rustc_mir/src/transform/check_consts/ops.rs b/compiler/rustc_mir/src/transform/check_consts/ops.rs index ff27d0c3a9211..4af355310ae25 100644 --- a/compiler/rustc_mir/src/transform/check_consts/ops.rs +++ b/compiler/rustc_mir/src/transform/check_consts/ops.rs @@ -148,7 +148,9 @@ pub struct InlineAsm; impl NonConstOp for InlineAsm {} #[derive(Debug)] -pub struct LiveDrop(pub Option); +pub struct LiveDrop { + pub dropped_at: Option, +} impl NonConstOp for LiveDrop { fn emit_error(&self, ccx: &ConstCx<'_, '_>, span: Span) { let mut diagnostic = struct_span_err!( @@ -158,7 +160,7 @@ impl NonConstOp for LiveDrop { "destructors cannot be evaluated at compile-time" ); diagnostic.span_label(span, format!("{}s cannot evaluate destructors", ccx.const_kind())); - if let Some(span) = self.0 { + if let Some(span) = self.dropped_at { diagnostic.span_label(span, "value is dropped here"); } diagnostic.emit(); diff --git a/compiler/rustc_mir/src/transform/check_consts/post_drop_elaboration.rs b/compiler/rustc_mir/src/transform/check_consts/post_drop_elaboration.rs index 55075b3ab5e99..a2e5c905612aa 100644 --- a/compiler/rustc_mir/src/transform/check_consts/post_drop_elaboration.rs +++ b/compiler/rustc_mir/src/transform/check_consts/post_drop_elaboration.rs @@ -52,7 +52,7 @@ impl std::ops::Deref for CheckLiveDrops<'mir, 'tcx> { impl CheckLiveDrops<'mir, 'tcx> { fn check_live_drop(&self, span: Span) { - ops::non_const(self.ccx, ops::LiveDrop(None), span); + ops::non_const(self.ccx, ops::LiveDrop { dropped_at: None }, span); } } diff --git a/compiler/rustc_mir/src/transform/check_consts/validation.rs b/compiler/rustc_mir/src/transform/check_consts/validation.rs index e8411b121e394..ebba7f29663ed 100644 --- a/compiler/rustc_mir/src/transform/check_consts/validation.rs +++ b/compiler/rustc_mir/src/transform/check_consts/validation.rs @@ -576,7 +576,7 @@ impl Visitor<'tcx> for Validator<'mir, 'tcx> { if needs_drop { self.check_op_spanned( - ops::LiveDrop(Some(terminator.source_info.span)), + ops::LiveDrop { dropped_at: Some(terminator.source_info.span) }, err_span, ); } From 81b3b66487c7001a0dbadf7f9af41ad714702533 Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Wed, 2 Sep 2020 17:11:55 -0700 Subject: [PATCH 14/44] Error if an unstable const eval feature is used in a stable const fn --- .../src/transform/check_consts/ops.rs | 26 +++++++++++++++++-- .../check_consts/post_drop_elaboration.rs | 11 +++++--- .../src/transform/check_consts/validation.rs | 2 +- 3 files changed, 33 insertions(+), 6 deletions(-) diff --git a/compiler/rustc_mir/src/transform/check_consts/ops.rs b/compiler/rustc_mir/src/transform/check_consts/ops.rs index 4af355310ae25..032cbc23a3f52 100644 --- a/compiler/rustc_mir/src/transform/check_consts/ops.rs +++ b/compiler/rustc_mir/src/transform/check_consts/ops.rs @@ -1,6 +1,6 @@ //! Concrete error types for all operations which may be invalid in a certain const context. -use rustc_errors::struct_span_err; +use rustc_errors::{struct_span_err, Applicability}; use rustc_hir as hir; use rustc_hir::def_id::DefId; use rustc_session::config::nightly_options; @@ -16,7 +16,29 @@ pub fn non_const(ccx: &ConstCx<'_, '_>, op: O, span: Span) { let gate = match op.status_in_item(ccx) { Status::Allowed => return, - Status::Unstable(gate) if ccx.tcx.features().enabled(gate) => return, + + Status::Unstable(gate) if ccx.tcx.features().enabled(gate) => { + let unstable_in_stable = ccx.const_kind() == hir::ConstContext::ConstFn + && ccx.tcx.features().enabled(sym::staged_api) + && !ccx.tcx.has_attr(ccx.def_id.to_def_id(), sym::rustc_const_unstable) + && !super::allow_internal_unstable(ccx.tcx, ccx.def_id.to_def_id(), gate); + + if unstable_in_stable { + ccx.tcx.sess + .struct_span_err(span, &format!("`#[feature({})]` cannot be depended on in a const-stable function", gate.as_str())) + .span_suggestion( + ccx.body.span, + "if it is not part of the public API, make this function unstably const", + concat!(r#"#[rustc_const_unstable(feature = "...", issue = "...")]"#, '\n').to_owned(), + Applicability::HasPlaceholders, + ) + .help("otherwise `#[allow_internal_unstable]` can be used to bypass stability checks") + .emit(); + } + + return; + } + Status::Unstable(gate) => Some(gate), Status::Forbidden => None, }; diff --git a/compiler/rustc_mir/src/transform/check_consts/post_drop_elaboration.rs b/compiler/rustc_mir/src/transform/check_consts/post_drop_elaboration.rs index a2e5c905612aa..0228f2d7de023 100644 --- a/compiler/rustc_mir/src/transform/check_consts/post_drop_elaboration.rs +++ b/compiler/rustc_mir/src/transform/check_consts/post_drop_elaboration.rs @@ -2,7 +2,7 @@ use rustc_hir::def_id::LocalDefId; use rustc_middle::mir::visit::Visitor; use rustc_middle::mir::{self, BasicBlock, Location}; use rustc_middle::ty::TyCtxt; -use rustc_span::Span; +use rustc_span::{sym, Span}; use super::ops; use super::qualifs::{NeedsDrop, Qualif}; @@ -11,7 +11,12 @@ use super::ConstCx; /// Returns `true` if we should use the more precise live drop checker that runs after drop /// elaboration. -pub fn checking_enabled(tcx: TyCtxt<'tcx>) -> bool { +pub fn checking_enabled(tcx: TyCtxt<'tcx>, def_id: LocalDefId) -> bool { + // Const-stable functions must always use the stable live drop checker. + if tcx.features().staged_api && !tcx.has_attr(def_id.to_def_id(), sym::rustc_const_unstable) { + return false; + } + tcx.features().const_precise_live_drops } @@ -25,7 +30,7 @@ pub fn check_live_drops(tcx: TyCtxt<'tcx>, def_id: LocalDefId, body: &mir::Body< return; } - if !checking_enabled(tcx) { + if !checking_enabled(tcx, def_id) { return; } diff --git a/compiler/rustc_mir/src/transform/check_consts/validation.rs b/compiler/rustc_mir/src/transform/check_consts/validation.rs index ebba7f29663ed..0501302b7610a 100644 --- a/compiler/rustc_mir/src/transform/check_consts/validation.rs +++ b/compiler/rustc_mir/src/transform/check_consts/validation.rs @@ -551,7 +551,7 @@ impl Visitor<'tcx> for Validator<'mir, 'tcx> { | TerminatorKind::DropAndReplace { place: dropped_place, .. } => { // If we are checking live drops after drop-elaboration, don't emit duplicate // errors here. - if super::post_drop_elaboration::checking_enabled(self.tcx) { + if super::post_drop_elaboration::checking_enabled(self.tcx, self.def_id) { return; } From 1e1257b8f87d1e3073020df27765dc57fcf2c0cd Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Wed, 16 Sep 2020 13:09:46 -0700 Subject: [PATCH 15/44] Bless `miri-unleashed` tests `const_mut_refs` doesn't actually work in a `const` or `static` --- src/test/ui/consts/miri_unleashed/box.stderr | 2 +- .../ui/consts/miri_unleashed/mutable_references.stderr | 8 ++++---- .../consts/miri_unleashed/mutable_references_err.stderr | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/test/ui/consts/miri_unleashed/box.stderr b/src/test/ui/consts/miri_unleashed/box.stderr index 768b795ca5b39..66ea9d5924dfb 100644 --- a/src/test/ui/consts/miri_unleashed/box.stderr +++ b/src/test/ui/consts/miri_unleashed/box.stderr @@ -21,7 +21,7 @@ help: skipping check for `const_mut_refs` feature | LL | &mut *(box 0) | ^^^^^^^^^^^^^ -help: skipping check for `const_mut_refs` feature +help: skipping check that does not even have a feature gate --> $DIR/box.rs:10:5 | LL | &mut *(box 0) diff --git a/src/test/ui/consts/miri_unleashed/mutable_references.stderr b/src/test/ui/consts/miri_unleashed/mutable_references.stderr index 7109ffd8b61d7..c6180c1e0041c 100644 --- a/src/test/ui/consts/miri_unleashed/mutable_references.stderr +++ b/src/test/ui/consts/miri_unleashed/mutable_references.stderr @@ -6,17 +6,17 @@ LL | *OH_YES = 99; warning: skipping const checks | -help: skipping check for `const_mut_refs` feature +help: skipping check that does not even have a feature gate --> $DIR/mutable_references.rs:9:26 | LL | static FOO: &&mut u32 = &&mut 42; | ^^^^^^^ -help: skipping check for `const_mut_refs` feature +help: skipping check that does not even have a feature gate --> $DIR/mutable_references.rs:13:23 | LL | static BAR: &mut () = &mut (); | ^^^^^^^ -help: skipping check for `const_mut_refs` feature +help: skipping check that does not even have a feature gate --> $DIR/mutable_references.rs:18:28 | LL | static BOO: &mut Foo<()> = &mut Foo(()); @@ -26,7 +26,7 @@ help: skipping check that does not even have a feature gate | LL | x: &UnsafeCell::new(42), | ^^^^^^^^^^^^^^^^^^^^ -help: skipping check for `const_mut_refs` feature +help: skipping check that does not even have a feature gate --> $DIR/mutable_references.rs:30:27 | LL | static OH_YES: &mut i32 = &mut 42; diff --git a/src/test/ui/consts/miri_unleashed/mutable_references_err.stderr b/src/test/ui/consts/miri_unleashed/mutable_references_err.stderr index 45e7d5a2cc3b3..7647a9ff4f6e4 100644 --- a/src/test/ui/consts/miri_unleashed/mutable_references_err.stderr +++ b/src/test/ui/consts/miri_unleashed/mutable_references_err.stderr @@ -30,7 +30,7 @@ help: skipping check that does not even have a feature gate | LL | const SNEAKY: &dyn Sync = &Synced { x: UnsafeCell::new(42) }; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -help: skipping check for `const_mut_refs` feature +help: skipping check that does not even have a feature gate --> $DIR/mutable_references_err.rs:30:25 | LL | const BLUNT: &mut i32 = &mut 42; From abc71677da866fd36c70790e768f36d2f809c493 Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Wed, 16 Sep 2020 13:19:45 -0700 Subject: [PATCH 16/44] Test that `const_precise_live_drops` can't be depended upon stably --- .../stable-precise-live-drops-in-libcore.rs | 22 ++++++++++++++++++ ...table-precise-live-drops-in-libcore.stderr | 12 ++++++++++ .../unstable-precise-live-drops-in-libcore.rs | 23 +++++++++++++++++++ 3 files changed, 57 insertions(+) create mode 100644 src/test/ui/consts/stable-precise-live-drops-in-libcore.rs create mode 100644 src/test/ui/consts/stable-precise-live-drops-in-libcore.stderr create mode 100644 src/test/ui/consts/unstable-precise-live-drops-in-libcore.rs diff --git a/src/test/ui/consts/stable-precise-live-drops-in-libcore.rs b/src/test/ui/consts/stable-precise-live-drops-in-libcore.rs new file mode 100644 index 0000000000000..651462d7ef19c --- /dev/null +++ b/src/test/ui/consts/stable-precise-live-drops-in-libcore.rs @@ -0,0 +1,22 @@ +#![stable(feature = "core", since = "1.6.0")] +#![feature(staged_api)] +#![feature(const_precise_live_drops, const_fn)] + +enum Either { + Left(T), + Right(S), +} + +impl Either { + #[stable(feature = "rust1", since = "1.0.0")] + #[rustc_const_stable(feature = "foo", since = "1.0.0")] + pub const fn unwrap(self) -> T { + //~^ ERROR destructors cannot be evaluated at compile-time + match self { + Self::Left(t) => t, + Self::Right(t) => t, + } + } +} + +fn main() {} diff --git a/src/test/ui/consts/stable-precise-live-drops-in-libcore.stderr b/src/test/ui/consts/stable-precise-live-drops-in-libcore.stderr new file mode 100644 index 0000000000000..a3f513541dd6a --- /dev/null +++ b/src/test/ui/consts/stable-precise-live-drops-in-libcore.stderr @@ -0,0 +1,12 @@ +error[E0493]: destructors cannot be evaluated at compile-time + --> $DIR/stable-precise-live-drops-in-libcore.rs:13:25 + | +LL | pub const fn unwrap(self) -> T { + | ^^^^ constant functions cannot evaluate destructors +... +LL | } + | - value is dropped here + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0493`. diff --git a/src/test/ui/consts/unstable-precise-live-drops-in-libcore.rs b/src/test/ui/consts/unstable-precise-live-drops-in-libcore.rs new file mode 100644 index 0000000000000..619084eaa517a --- /dev/null +++ b/src/test/ui/consts/unstable-precise-live-drops-in-libcore.rs @@ -0,0 +1,23 @@ +// check-pass + +#![stable(feature = "core", since = "1.6.0")] +#![feature(staged_api)] +#![feature(const_precise_live_drops)] + +enum Either { + Left(T), + Right(S), +} + +impl Either { + #[stable(feature = "rust1", since = "1.0.0")] + #[rustc_const_unstable(feature = "foo", issue = "none")] + pub const fn unwrap(self) -> T { + match self { + Self::Left(t) => t, + Self::Right(t) => t, + } + } +} + +fn main() {} From 7e24136996fd412ba2890952d5f0ddffb3cb7370 Mon Sep 17 00:00:00 2001 From: Mark Rousskov Date: Wed, 16 Sep 2020 10:29:31 -0400 Subject: [PATCH 17/44] Pass --target to lint docs Otherwise, we may not have a standard library built for the native "host" target of the rustc being run. --- src/bootstrap/doc.rs | 4 +++- src/tools/lint-docs/src/groups.rs | 8 ++++---- src/tools/lint-docs/src/lib.rs | 25 ++++++++++++++++--------- src/tools/lint-docs/src/main.rs | 15 ++++++++++++++- 4 files changed, 37 insertions(+), 15 deletions(-) diff --git a/src/bootstrap/doc.rs b/src/bootstrap/doc.rs index f90e76a4f4ea6..cf9211bc7ea86 100644 --- a/src/bootstrap/doc.rs +++ b/src/bootstrap/doc.rs @@ -752,6 +752,7 @@ impl Step for RustcBook { let out_listing = out_base.join("src/lints"); builder.cp_r(&builder.src.join("src/doc/rustc"), &out_base); builder.info(&format!("Generating lint docs ({})", self.target)); + let rustc = builder.rustc(self.compiler); // The tool runs `rustc` for extracting output examples, so it needs a // functional sysroot. @@ -762,7 +763,8 @@ impl Step for RustcBook { cmd.arg("--out"); cmd.arg(&out_listing); cmd.arg("--rustc"); - cmd.arg(rustc); + cmd.arg(&rustc); + cmd.arg("--rustc-target").arg(&self.target.rustc_target_arg()); if builder.config.verbose() { cmd.arg("--verbose"); } diff --git a/src/tools/lint-docs/src/groups.rs b/src/tools/lint-docs/src/groups.rs index a212459bb4dc6..6b32ebdc284f4 100644 --- a/src/tools/lint-docs/src/groups.rs +++ b/src/tools/lint-docs/src/groups.rs @@ -18,10 +18,10 @@ static GROUP_DESCRIPTIONS: &[(&str, &str)] = &[ /// Updates the documentation of lint groups. pub(crate) fn generate_group_docs( lints: &[Lint], - rustc_path: &Path, + rustc: crate::Rustc<'_>, out_path: &Path, ) -> Result<(), Box> { - let groups = collect_groups(rustc_path)?; + let groups = collect_groups(rustc)?; let groups_path = out_path.join("groups.md"); let contents = fs::read_to_string(&groups_path) .map_err(|e| format!("could not read {}: {}", groups_path.display(), e))?; @@ -36,9 +36,9 @@ pub(crate) fn generate_group_docs( type LintGroups = BTreeMap>; /// Collects the group names from rustc. -fn collect_groups(rustc: &Path) -> Result> { +fn collect_groups(rustc: crate::Rustc<'_>) -> Result> { let mut result = BTreeMap::new(); - let mut cmd = Command::new(rustc); + let mut cmd = Command::new(rustc.path); cmd.arg("-Whelp"); let output = cmd.output().map_err(|e| format!("failed to run command {:?}\n{}", cmd, e))?; if !output.status.success() { diff --git a/src/tools/lint-docs/src/lib.rs b/src/tools/lint-docs/src/lib.rs index 92b3d186fa141..6ca71dcaf3cd0 100644 --- a/src/tools/lint-docs/src/lib.rs +++ b/src/tools/lint-docs/src/lib.rs @@ -45,16 +45,22 @@ impl Level { } } +#[derive(Copy, Clone)] +pub struct Rustc<'a> { + pub path: &'a Path, + pub target: &'a str, +} + /// Collects all lints, and writes the markdown documentation at the given directory. pub fn extract_lint_docs( src_path: &Path, out_path: &Path, - rustc_path: &Path, + rustc: Rustc<'_>, verbose: bool, ) -> Result<(), Box> { let mut lints = gather_lints(src_path)?; for lint in &mut lints { - generate_output_example(lint, rustc_path, verbose).map_err(|e| { + generate_output_example(lint, rustc, verbose).map_err(|e| { format!( "failed to test example in lint docs for `{}` in {}:{}: {}", lint.name, @@ -65,7 +71,7 @@ pub fn extract_lint_docs( })?; } save_lints_markdown(&lints, &out_path.join("listing"))?; - groups::generate_group_docs(&lints, rustc_path, out_path)?; + groups::generate_group_docs(&lints, rustc, out_path)?; Ok(()) } @@ -208,7 +214,7 @@ fn lint_name(line: &str) -> Result { /// actual output from the compiler. fn generate_output_example( lint: &mut Lint, - rustc_path: &Path, + rustc: Rustc<'_>, verbose: bool, ) -> Result<(), Box> { // Explicit list of lints that are allowed to not have an example. Please @@ -230,7 +236,7 @@ fn generate_output_example( // separate test suite, and use an include mechanism such as mdbook's // `{{#rustdoc_include}}`. if !lint.is_ignored() { - replace_produces(lint, rustc_path, verbose)?; + replace_produces(lint, rustc, verbose)?; } Ok(()) } @@ -261,7 +267,7 @@ fn check_style(lint: &Lint) -> Result<(), Box> { /// output from the compiler. fn replace_produces( lint: &mut Lint, - rustc_path: &Path, + rustc: Rustc<'_>, verbose: bool, ) -> Result<(), Box> { let mut lines = lint.doc.iter_mut(); @@ -302,7 +308,7 @@ fn replace_produces( Some(line) if line.is_empty() => {} Some(line) if line == "{{produces}}" => { let output = - generate_lint_output(&lint.name, &example, &options, rustc_path, verbose)?; + generate_lint_output(&lint.name, &example, &options, rustc, verbose)?; line.replace_range( .., &format!( @@ -329,7 +335,7 @@ fn generate_lint_output( name: &str, example: &[&mut String], options: &[&str], - rustc_path: &Path, + rustc: Rustc<'_>, verbose: bool, ) -> Result> { if verbose { @@ -364,13 +370,14 @@ fn generate_lint_output( } fs::write(&tempfile, source) .map_err(|e| format!("failed to write {}: {}", tempfile.display(), e))?; - let mut cmd = Command::new(rustc_path); + let mut cmd = Command::new(rustc.path); if options.contains(&"edition2015") { cmd.arg("--edition=2015"); } else { cmd.arg("--edition=2018"); } cmd.arg("--error-format=json"); + cmd.arg("--target").arg(rustc.target); if options.contains(&"test") { cmd.arg("--test"); } diff --git a/src/tools/lint-docs/src/main.rs b/src/tools/lint-docs/src/main.rs index 45d97bd431791..5db49007d375c 100644 --- a/src/tools/lint-docs/src/main.rs +++ b/src/tools/lint-docs/src/main.rs @@ -13,6 +13,7 @@ fn doit() -> Result<(), Box> { let mut src_path = None; let mut out_path = None; let mut rustc_path = None; + let mut rustc_target = None; let mut verbose = false; while let Some(arg) = args.next() { match arg.as_str() { @@ -34,6 +35,12 @@ fn doit() -> Result<(), Box> { None => return Err("--rustc requires a value".into()), }; } + "--rustc-target" => { + rustc_target = match args.next() { + Some(s) => Some(s), + None => return Err("--rustc-target requires a value".into()), + }; + } "-v" | "--verbose" => verbose = true, s => return Err(format!("unexpected argument `{}`", s).into()), } @@ -47,10 +54,16 @@ fn doit() -> Result<(), Box> { if rustc_path.is_none() { return Err("--rustc must be specified to the path of rustc".into()); } + if rustc_target.is_none() { + return Err("--rustc-target must be specified to the rustc target".into()); + } lint_docs::extract_lint_docs( &src_path.unwrap(), &out_path.unwrap(), - &rustc_path.unwrap(), + lint_docs::Rustc { + path: rustc_path.as_deref().unwrap(), + target: rustc_target.as_deref().unwrap(), + }, verbose, ) } From bd4e0af0b54afc91903c282740e25ee6135224c8 Mon Sep 17 00:00:00 2001 From: Mark Rousskov Date: Wed, 16 Sep 2020 12:47:26 -0400 Subject: [PATCH 18/44] Build rustdoc for cross-compiled targets This isn't an issue for most folks who use x.py dist, which will directly depend on this. But for x.py build, if we don't properly set target here rustdoc will not be built. Currently, there is not a default-on step for generating a rustc for a given target either, so we will fail to build a rustc as well. --- src/bootstrap/tool.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/bootstrap/tool.rs b/src/bootstrap/tool.rs index 99e33e3b006fe..d2346e40e51d2 100644 --- a/src/bootstrap/tool.rs +++ b/src/bootstrap/tool.rs @@ -471,7 +471,11 @@ impl Step for Rustdoc { fn make_run(run: RunConfig<'_>) { run.builder.ensure(Rustdoc { - compiler: run.builder.compiler(run.builder.top_stage, run.build_triple()), + // Note: this is somewhat unique in that we actually want a *target* + // compiler here, because rustdoc *is* a compiler. We won't be using + // this as the compiler to build with, but rather this is "what + // compiler are we producing"? + compiler: run.builder.compiler(run.builder.top_stage, run.target), }); } From 7b5d9836c47509e16900a274ed0b552a2e30a36a Mon Sep 17 00:00:00 2001 From: Juan Aguilar Santillana Date: Thu, 17 Sep 2020 10:27:04 +0000 Subject: [PATCH 19/44] Remove redundant to_string --- compiler/rustc_errors/src/emitter.rs | 22 ++++++++-------------- 1 file changed, 8 insertions(+), 14 deletions(-) diff --git a/compiler/rustc_errors/src/emitter.rs b/compiler/rustc_errors/src/emitter.rs index 4555168af0ab5..98cbf98df92b4 100644 --- a/compiler/rustc_errors/src/emitter.rs +++ b/compiler/rustc_errors/src/emitter.rs @@ -1227,18 +1227,14 @@ impl EmitterWriter { } draw_note_separator(&mut buffer, 0, max_line_num_len + 1); if *level != Level::FailureNote { - let level_str = level.to_string(); - if !level_str.is_empty() { - buffer.append(0, &level_str, Style::MainHeaderMsg); - buffer.append(0, ": ", Style::NoStyle); - } + buffer.append(0, level.to_str(), Style::MainHeaderMsg); + buffer.append(0, ": ", Style::NoStyle); } self.msg_to_buffer(&mut buffer, msg, max_line_num_len, "note", None); } else { - let level_str = level.to_string(); // The failure note level itself does not provide any useful diagnostic information - if *level != Level::FailureNote && !level_str.is_empty() { - buffer.append(0, &level_str, Style::Level(*level)); + if *level != Level::FailureNote { + buffer.append(0, level.to_str(), Style::Level(*level)); } // only render error codes, not lint codes if let Some(DiagnosticId::Error(ref code)) = *code { @@ -1246,7 +1242,7 @@ impl EmitterWriter { buffer.append(0, &code, Style::Level(*level)); buffer.append(0, "]", Style::Level(*level)); } - if *level != Level::FailureNote && !level_str.is_empty() { + if *level != Level::FailureNote { buffer.append(0, ": ", header_style); } for &(ref text, _) in msg.iter() { @@ -1548,11 +1544,9 @@ impl EmitterWriter { let mut buffer = StyledBuffer::new(); // Render the suggestion message - let level_str = level.to_string(); - if !level_str.is_empty() { - buffer.append(0, &level_str, Style::Level(*level)); - buffer.append(0, ": ", Style::HeaderMsg); - } + buffer.append(0, level.to_str(), Style::Level(*level)); + buffer.append(0, ": ", Style::HeaderMsg); + self.msg_to_buffer( &mut buffer, &[(suggestion.msg.to_owned(), Style::NoStyle)], From 363aff0a9d0b85285b7501cb04dd8263d29d273a Mon Sep 17 00:00:00 2001 From: Mark Rousskov Date: Thu, 17 Sep 2020 16:03:42 -0400 Subject: [PATCH 20/44] Add test for x.py build cross-compilation --- src/bootstrap/builder/tests.rs | 48 ++++++++++++++++++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/src/bootstrap/builder/tests.rs b/src/bootstrap/builder/tests.rs index f96925f927086..77b39cbb87ed9 100644 --- a/src/bootstrap/builder/tests.rs +++ b/src/bootstrap/builder/tests.rs @@ -90,6 +90,54 @@ mod defaults { assert!(builder.cache.all::().is_empty()); } + #[test] + fn build_cross_compile() { + let config = Config { stage: 1, ..configure("build", &["B"], &["B"]) }; + let build = Build::new(config); + let mut builder = Builder::new(&build); + builder.run_step_descriptions(&Builder::get_step_descriptions(Kind::Build), &[]); + + let a = TargetSelection::from_user("A"); + let b = TargetSelection::from_user("B"); + + // Ideally, this build wouldn't actually have `target: a` + // rustdoc/rustcc/std here (the user only requested a host=B build, so + // there's not really a need for us to build for target A in this case + // (since we're producing stage 1 libraries/binaries). But currently + // rustbuild is just a bit buggy here; this should be fixed though. + assert_eq!( + first(builder.cache.all::()), + &[ + compile::Std { compiler: Compiler { host: a, stage: 0 }, target: a }, + compile::Std { compiler: Compiler { host: a, stage: 1 }, target: a }, + compile::Std { compiler: Compiler { host: a, stage: 0 }, target: b }, + compile::Std { compiler: Compiler { host: a, stage: 1 }, target: b }, + ] + ); + assert_eq!( + first(builder.cache.all::()), + &[ + compile::Assemble { target_compiler: Compiler { host: a, stage: 0 } }, + compile::Assemble { target_compiler: Compiler { host: a, stage: 1 } }, + compile::Assemble { target_compiler: Compiler { host: b, stage: 1 } }, + ] + ); + assert_eq!( + first(builder.cache.all::()), + &[ + tool::Rustdoc { compiler: Compiler { host: a, stage: 1 } }, + tool::Rustdoc { compiler: Compiler { host: b, stage: 1 } }, + ], + ); + assert_eq!( + first(builder.cache.all::()), + &[ + compile::Rustc { compiler: Compiler { host: a, stage: 0 }, target: a }, + compile::Rustc { compiler: Compiler { host: a, stage: 0 }, target: b }, + ] + ); + } + #[test] fn doc_default() { let mut config = configure("doc", &[], &[]); From 28cfa9730eec41314dee99323f7d20aaadde9e0e Mon Sep 17 00:00:00 2001 From: Juan Aguilar Santillana Date: Fri, 18 Sep 2020 05:57:01 +0000 Subject: [PATCH 21/44] Simplify panic_if_treat_err_as_bug avoiding allocations --- compiler/rustc_errors/src/lib.rs | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/compiler/rustc_errors/src/lib.rs b/compiler/rustc_errors/src/lib.rs index 2abd20869aecf..b16fe5603c100 100644 --- a/compiler/rustc_errors/src/lib.rs +++ b/compiler/rustc_errors/src/lib.rs @@ -973,16 +973,14 @@ impl HandlerInner { fn panic_if_treat_err_as_bug(&self) { if self.treat_err_as_bug() { - let s = match (self.err_count(), self.flags.treat_err_as_bug.unwrap_or(0)) { - (0, _) => return, - (1, 1) => "aborting due to `-Z treat-err-as-bug=1`".to_string(), - (1, _) => return, - (count, as_bug) => format!( + match (self.err_count(), self.flags.treat_err_as_bug.unwrap_or(0)) { + (1, 1) => panic!("aborting due to `-Z treat-err-as-bug=1`"), + (0, _) | (1, _) => {} + (count, as_bug) => panic!( "aborting after {} errors due to `-Z treat-err-as-bug={}`", count, as_bug, ), - }; - panic!(s); + } } } } From 4675a3104b3ace025560337c5d164e330f6b9d68 Mon Sep 17 00:00:00 2001 From: Alexis Bourget Date: Fri, 18 Sep 2020 09:51:26 +0200 Subject: [PATCH 22/44] Use intra-doc links in core/src/iter when possible --- library/core/src/iter/sources.rs | 76 +++++++------------- library/core/src/iter/traits/accum.rs | 46 ++++++------ library/core/src/iter/traits/collect.rs | 20 ++---- library/core/src/iter/traits/double_ended.rs | 39 +++++----- library/core/src/iter/traits/exact_size.rs | 23 +++--- library/core/src/iter/traits/iterator.rs | 1 - library/core/src/iter/traits/marker.rs | 18 ++--- 7 files changed, 91 insertions(+), 132 deletions(-) diff --git a/library/core/src/iter/sources.rs b/library/core/src/iter/sources.rs index d76fa89bd012c..0348d5a10d984 100644 --- a/library/core/src/iter/sources.rs +++ b/library/core/src/iter/sources.rs @@ -5,9 +5,7 @@ use super::{FusedIterator, TrustedLen}; /// An iterator that repeats an element endlessly. /// -/// This `struct` is created by the [`repeat`] function. See its documentation for more. -/// -/// [`repeat`]: fn.repeat.html +/// This `struct` is created by the [`repeat()`] function. See its documentation for more. #[derive(Clone, Debug)] #[stable(feature = "rust1", since = "1.0.0")] pub struct Repeat { @@ -47,15 +45,11 @@ unsafe impl TrustedLen for Repeat {} /// The `repeat()` function repeats a single value over and over again. /// /// Infinite iterators like `repeat()` are often used with adapters like -/// [`take`], in order to make them finite. -/// -/// [`take`]: trait.Iterator.html#method.take +/// [`Iterator::take()`], in order to make them finite. /// /// If the element type of the iterator you need does not implement `Clone`, /// or if you do not want to keep the repeated element in memory, you can -/// instead use the [`repeat_with`] function. -/// -/// [`repeat_with`]: fn.repeat_with.html +/// instead use the [`repeat_with()`] function. /// /// # Examples /// @@ -77,7 +71,7 @@ unsafe impl TrustedLen for Repeat {} /// assert_eq!(Some(4), fours.next()); /// ``` /// -/// Going finite with [`take`]: +/// Going finite with [`Iterator::take()`]: /// /// ``` /// use std::iter; @@ -102,10 +96,8 @@ pub fn repeat(elt: T) -> Repeat { /// An iterator that repeats elements of type `A` endlessly by /// applying the provided closure `F: FnMut() -> A`. /// -/// This `struct` is created by the [`repeat_with`] function. +/// This `struct` is created by the [`repeat_with()`] function. /// See its documentation for more. -/// -/// [`repeat_with`]: fn.repeat_with.html #[derive(Copy, Clone, Debug)] #[stable(feature = "iterator_repeat_with", since = "1.28.0")] pub struct RepeatWith { @@ -139,20 +131,18 @@ unsafe impl A> TrustedLen for RepeatWith {} /// The `repeat_with()` function calls the repeater over and over again. /// /// Infinite iterators like `repeat_with()` are often used with adapters like -/// [`take`], in order to make them finite. +/// [`Iterator::take()`], in order to make them finite. /// -/// [`take`]: trait.Iterator.html#method.take -/// -/// If the element type of the iterator you need implements `Clone`, and +/// If the element type of the iterator you need implements [`Clone`], and /// it is OK to keep the source element in memory, you should instead use -/// the [`repeat`] function. -/// -/// [`repeat`]: fn.repeat.html +/// the [`repeat()`] function. /// -/// An iterator produced by `repeat_with()` is not a `DoubleEndedIterator`. -/// If you need `repeat_with()` to return a `DoubleEndedIterator`, +/// An iterator produced by `repeat_with()` is not a [`DoubleEndedIterator`]. +/// If you need `repeat_with()` to return a [`DoubleEndedIterator`], /// please open a GitHub issue explaining your use case. /// +/// [`DoubleEndedIterator`]: crate::iter::DoubleEndedIterator +/// /// # Examples /// /// Basic usage: @@ -201,9 +191,7 @@ pub fn repeat_with A>(repeater: F) -> RepeatWith { /// An iterator that yields nothing. /// -/// This `struct` is created by the [`empty`] function. See its documentation for more. -/// -/// [`empty`]: fn.empty.html +/// This `struct` is created by the [`empty()`] function. See its documentation for more. #[stable(feature = "iter_empty", since = "1.2.0")] pub struct Empty(marker::PhantomData); @@ -292,9 +280,7 @@ pub const fn empty() -> Empty { /// An iterator that yields an element exactly once. /// -/// This `struct` is created by the [`once`] function. See its documentation for more. -/// -/// [`once`]: fn.once.html +/// This `struct` is created by the [`once()`] function. See its documentation for more. #[derive(Clone, Debug)] #[stable(feature = "iter_once", since = "1.2.0")] pub struct Once { @@ -336,12 +322,12 @@ impl FusedIterator for Once {} /// Creates an iterator that yields an element exactly once. /// -/// This is commonly used to adapt a single value into a [`chain`] of other +/// This is commonly used to adapt a single value into a [`chain()`] of other /// kinds of iteration. Maybe you have an iterator that covers almost /// everything, but you need an extra special case. Maybe you have a function /// which works on iterators, but you only need to process one value. /// -/// [`chain`]: trait.Iterator.html#method.chain +/// [`chain()`]: Iterator::chain /// /// # Examples /// @@ -393,10 +379,8 @@ pub fn once(value: T) -> Once { /// An iterator that yields a single element of type `A` by /// applying the provided closure `F: FnOnce() -> A`. /// -/// This `struct` is created by the [`once_with`] function. +/// This `struct` is created by the [`once_with()`] function. /// See its documentation for more. -/// -/// [`once_with`]: fn.once_with.html #[derive(Clone, Debug)] #[stable(feature = "iter_once_with", since = "1.43.0")] pub struct OnceWith { @@ -442,15 +426,14 @@ unsafe impl A> TrustedLen for OnceWith {} /// Creates an iterator that lazily generates a value exactly once by invoking /// the provided closure. /// -/// This is commonly used to adapt a single value generator into a [`chain`] of +/// This is commonly used to adapt a single value generator into a [`chain()`] of /// other kinds of iteration. Maybe you have an iterator that covers almost /// everything, but you need an extra special case. Maybe you have a function /// which works on iterators, but you only need to process one value. /// -/// Unlike [`once`], this function will lazily generate the value on request. +/// Unlike [`once()`], this function will lazily generate the value on request. /// -/// [`once`]: fn.once.html -/// [`chain`]: trait.Iterator.html#method.chain +/// [`chain()`]: Iterator::chain /// /// # Examples /// @@ -505,17 +488,16 @@ pub fn once_with A>(gen: F) -> OnceWith { /// /// This allows creating a custom iterator with any behavior /// without using the more verbose syntax of creating a dedicated type -/// and implementing the `Iterator` trait for it. +/// and implementing the [`Iterator`] trait for it. /// /// Note that the `FromFn` iterator doesn’t make assumptions about the behavior of the closure, /// and therefore conservatively does not implement [`FusedIterator`], -/// or override [`Iterator::size_hint`] from its default `(0, None)`. -/// -/// [`FusedIterator`]: trait.FusedIterator.html -/// [`Iterator::size_hint`]: trait.Iterator.html#method.size_hint +/// or override [`Iterator::size_hint()`] from its default `(0, None)`. /// /// The closure can use captures and its environment to track state across iterations. Depending on -/// how the iterator is used, this may require specifying the `move` keyword on the closure. +/// how the iterator is used, this may require specifying the [`move`] keyword on the closure. +/// +/// [`move`]: ../../../std/keyword.move.html /// /// # Examples /// @@ -549,10 +531,8 @@ where /// An iterator where each iteration calls the provided closure `F: FnMut() -> Option`. /// -/// This `struct` is created by the [`iter::from_fn`] function. +/// This `struct` is created by the [`from_fn()`] function. /// See its documentation for more. -/// -/// [`iter::from_fn`]: fn.from_fn.html #[derive(Clone)] #[stable(feature = "iter_from_fn", since = "1.34.0")] pub struct FromFn(F); @@ -601,10 +581,8 @@ where /// An new iterator where each successive item is computed based on the preceding one. /// -/// This `struct` is created by the [`successors`] function. +/// This `struct` is created by the [`successors()`] function. /// See its documentation for more. -/// -/// [`successors`]: fn.successors.html #[derive(Clone)] #[stable(feature = "iter_successors", since = "1.34.0")] pub struct Successors { diff --git a/library/core/src/iter/traits/accum.rs b/library/core/src/iter/traits/accum.rs index 494c75174ff83..dc0d8087ffbff 100644 --- a/library/core/src/iter/traits/accum.rs +++ b/library/core/src/iter/traits/accum.rs @@ -4,14 +4,13 @@ use crate::ops::{Add, Mul}; /// Trait to represent types that can be created by summing up an iterator. /// -/// This trait is used to implement the [`sum`] method on iterators. Types which -/// implement the trait can be generated by the [`sum`] method. Like +/// This trait is used to implement the [`sum()`] method on iterators. Types which +/// implement the trait can be generated by the [`sum()`] method. Like /// [`FromIterator`] this trait should rarely be called directly and instead -/// interacted with through [`Iterator::sum`]. +/// interacted with through [`Iterator::sum()`]. /// -/// [`sum`]: #tymethod.sum -/// [`FromIterator`]: crate::iter::FromIterator -/// [`Iterator::sum`]: crate::iter::Iterator::sum +/// [`sum()`]: Sum::sum +/// [`FromIterator`]: iter::FromIterator #[stable(feature = "iter_arith_traits", since = "1.12.0")] pub trait Sum: Sized { /// Method which takes an iterator and generates `Self` from the elements by @@ -23,14 +22,13 @@ pub trait Sum: Sized { /// Trait to represent types that can be created by multiplying elements of an /// iterator. /// -/// This trait is used to implement the [`product`] method on iterators. Types -/// which implement the trait can be generated by the [`product`] method. Like +/// This trait is used to implement the [`product()`] method on iterators. Types +/// which implement the trait can be generated by the [`product()`] method. Like /// [`FromIterator`] this trait should rarely be called directly and instead -/// interacted with through [`Iterator::product`]. +/// interacted with through [`Iterator::product()`]. /// -/// [`product`]: #tymethod.product -/// [`FromIterator`]: crate::iter::FromIterator -/// [`Iterator::product`]: crate::iter::Iterator::product +/// [`product()`]: Product::product +/// [`FromIterator`]: iter::FromIterator #[stable(feature = "iter_arith_traits", since = "1.12.0")] pub trait Product: Sized { /// Method which takes an iterator and generates `Self` from the elements by @@ -120,9 +118,9 @@ impl Sum> for Result where T: Sum, { - /// Takes each element in the `Iterator`: if it is an `Err`, no further - /// elements are taken, and the `Err` is returned. Should no `Err` occur, - /// the sum of all elements is returned. + /// Takes each element in the [`Iterator`]: if it is an [`Err`], no further + /// elements are taken, and the [`Err`] is returned. Should no [`Err`] + /// occur, the sum of all elements is returned. /// /// # Examples /// @@ -150,9 +148,9 @@ impl Product> for Result where T: Product, { - /// Takes each element in the `Iterator`: if it is an `Err`, no further - /// elements are taken, and the `Err` is returned. Should no `Err` occur, - /// the product of all elements is returned. + /// Takes each element in the [`Iterator`]: if it is an [`Err`], no further + /// elements are taken, and the [`Err`] is returned. Should no [`Err`] + /// occur, the product of all elements is returned. fn product(iter: I) -> Result where I: Iterator>, @@ -166,9 +164,9 @@ impl Sum> for Option where T: Sum, { - /// Takes each element in the `Iterator`: if it is a `None`, no further - /// elements are taken, and the `None` is returned. Should no `None` occur, - /// the sum of all elements is returned. + /// Takes each element in the [`Iterator`]: if it is a [`None`], no further + /// elements are taken, and the [`None`] is returned. Should no [`None`] + /// occur, the sum of all elements is returned. /// /// # Examples /// @@ -193,9 +191,9 @@ impl Product> for Option where T: Product, { - /// Takes each element in the `Iterator`: if it is a `None`, no further - /// elements are taken, and the `None` is returned. Should no `None` occur, - /// the product of all elements is returned. + /// Takes each element in the [`Iterator`]: if it is a [`None`], no further + /// elements are taken, and the [`None`] is returned. Should no [`None`] + /// occur, the product of all elements is returned. fn product(iter: I) -> Option where I: Iterator>, diff --git a/library/core/src/iter/traits/collect.rs b/library/core/src/iter/traits/collect.rs index 75827d785e10e..41a503c4abb4f 100644 --- a/library/core/src/iter/traits/collect.rs +++ b/library/core/src/iter/traits/collect.rs @@ -1,21 +1,15 @@ -/// Conversion from an `Iterator`. +/// Conversion from an [`Iterator`]. /// /// By implementing `FromIterator` for a type, you define how it will be /// created from an iterator. This is common for types which describe a /// collection of some kind. /// -/// `FromIterator`'s [`from_iter`] is rarely called explicitly, and is instead -/// used through [`Iterator`]'s [`collect`] method. See [`collect`]'s +/// [`FromIterator::from_iter()`] is rarely called explicitly, and is instead +/// used through [`Iterator::collect()`] method. See [`Iterator::collect()`]'s /// documentation for more examples. /// -/// [`from_iter`]: #tymethod.from_iter -/// [`Iterator`]: trait.Iterator.html -/// [`collect`]: trait.Iterator.html#method.collect -/// /// See also: [`IntoIterator`]. /// -/// [`IntoIterator`]: trait.IntoIterator.html -/// /// # Examples /// /// Basic usage: @@ -30,7 +24,7 @@ /// assert_eq!(v, vec![5, 5, 5, 5, 5]); /// ``` /// -/// Using [`collect`] to implicitly use `FromIterator`: +/// Using [`Iterator::collect()`] to implicitly use `FromIterator`: /// /// ``` /// let five_fives = std::iter::repeat(5).take(5); @@ -119,7 +113,7 @@ pub trait FromIterator: Sized { fn from_iter>(iter: T) -> Self; } -/// Conversion into an `Iterator`. +/// Conversion into an [`Iterator`]. /// /// By implementing `IntoIterator` for a type, you define how it will be /// converted to an iterator. This is common for types which describe a @@ -130,8 +124,6 @@ pub trait FromIterator: Sized { /// /// See also: [`FromIterator`]. /// -/// [`FromIterator`]: trait.FromIterator.html -/// /// # Examples /// /// Basic usage: @@ -326,7 +318,7 @@ pub trait Extend { /// As this is the only required method for this trait, the [trait-level] docs /// contain more details. /// - /// [trait-level]: trait.Extend.html + /// [trait-level]: Extend /// /// # Examples /// diff --git a/library/core/src/iter/traits/double_ended.rs b/library/core/src/iter/traits/double_ended.rs index a025bc8b56049..bc03c143d6afb 100644 --- a/library/core/src/iter/traits/double_ended.rs +++ b/library/core/src/iter/traits/double_ended.rs @@ -10,11 +10,12 @@ use crate::ops::{ControlFlow, Try}; /// and do not cross: iteration is over when they meet in the middle. /// /// In a similar fashion to the [`Iterator`] protocol, once a -/// `DoubleEndedIterator` returns `None` from a `next_back()`, calling it again -/// may or may not ever return `Some` again. `next()` and `next_back()` are -/// interchangeable for this purpose. +/// `DoubleEndedIterator` returns [`None`] from a [`next_back()`], calling it +/// again may or may not ever return [`Some`] again. [`next()`] and +/// [`next_back()`] are interchangeable for this purpose. /// -/// [`Iterator`]: trait.Iterator.html +/// [`next_back()`]: DoubleEndedIterator::next_back +/// [`next()`]: Iterator::next /// /// # Examples /// @@ -42,7 +43,7 @@ pub trait DoubleEndedIterator: Iterator { /// /// The [trait-level] docs contain more details. /// - /// [trait-level]: trait.DoubleEndedIterator.html + /// [trait-level]: DoubleEndedIterator /// /// # Examples /// @@ -66,7 +67,7 @@ pub trait DoubleEndedIterator: Iterator { /// # Remarks /// /// The elements yielded by `DoubleEndedIterator`'s methods may differ from - /// the ones yielded by `Iterator`'s methods: + /// the ones yielded by [`Iterator`]'s methods: /// /// ``` /// let vec = vec![(1, 'a'), (1, 'b'), (1, 'c'), (2, 'a'), (2, 'b')]; @@ -87,25 +88,23 @@ pub trait DoubleEndedIterator: Iterator { /// vec![(2, 'b'), (1, 'c')] /// ); /// ``` - /// #[stable(feature = "rust1", since = "1.0.0")] fn next_back(&mut self) -> Option; /// Returns the `n`th element from the end of the iterator. /// - /// This is essentially the reversed version of [`nth`]. Although like most indexing - /// operations, the count starts from zero, so `nth_back(0)` returns the first value from - /// the end, `nth_back(1)` the second, and so on. + /// This is essentially the reversed version of [`Iterator::nth()`]. + /// Although like most indexing operations, the count starts from zero, so + /// `nth_back(0)` returns the first value from the end, `nth_back(1)` the + /// second, and so on. /// /// Note that all elements between the end and the returned element will be /// consumed, including the returned element. This also means that calling /// `nth_back(0)` multiple times on the same iterator will return different /// elements. /// - /// `nth_back()` will return [`None`] if `n` is greater than or equal to the length of the - /// iterator. - /// - /// [`nth`]: crate::iter::Iterator::nth + /// `nth_back()` will return [`None`] if `n` is greater than or equal to the + /// length of the iterator. /// /// # Examples /// @@ -145,10 +144,8 @@ pub trait DoubleEndedIterator: Iterator { None } - /// This is the reverse version of [`try_fold()`]: it takes elements - /// starting from the back of the iterator. - /// - /// [`try_fold()`]: Iterator::try_fold + /// This is the reverse version of [`Iterator::try_fold()`]: it takes + /// elements starting from the back of the iterator. /// /// # Examples /// @@ -195,8 +192,8 @@ pub trait DoubleEndedIterator: Iterator { /// An iterator method that reduces the iterator's elements to a single, /// final value, starting from the back. /// - /// This is the reverse version of [`fold()`]: it takes elements starting from - /// the back of the iterator. + /// This is the reverse version of [`Iterator::fold()`]: it takes elements + /// starting from the back of the iterator. /// /// `rfold()` takes two arguments: an initial value, and a closure with two /// arguments: an 'accumulator', and an element. The closure returns the value that @@ -213,8 +210,6 @@ pub trait DoubleEndedIterator: Iterator { /// Folding is useful whenever you have a collection of something, and want /// to produce a single value from it. /// - /// [`fold()`]: Iterator::fold - /// /// # Examples /// /// Basic usage: diff --git a/library/core/src/iter/traits/exact_size.rs b/library/core/src/iter/traits/exact_size.rs index ad87d09588e3a..33ace60a27419 100644 --- a/library/core/src/iter/traits/exact_size.rs +++ b/library/core/src/iter/traits/exact_size.rs @@ -6,17 +6,14 @@ /// backwards, a good start is to know where the end is. /// /// When implementing an `ExactSizeIterator`, you must also implement -/// [`Iterator`]. When doing so, the implementation of [`size_hint`] *must* -/// return the exact size of the iterator. -/// -/// [`Iterator`]: trait.Iterator.html -/// [`size_hint`]: trait.Iterator.html#method.size_hint +/// [`Iterator`]. When doing so, the implementation of [`Iterator::size_hint`] +/// *must* return the exact size of the iterator. /// /// The [`len`] method has a default implementation, so you usually shouldn't /// implement it. However, you may be able to provide a more performant /// implementation than the default, so overriding it in this case makes sense. /// -/// [`len`]: #method.len +/// [`len`]: ExactSizeIterator::len /// /// # Examples /// @@ -72,17 +69,17 @@ pub trait ExactSizeIterator: Iterator { /// Returns the exact length of the iterator. /// /// The implementation ensures that the iterator will return exactly `len()` - /// more times a `Some(T)` value, before returning `None`. + /// more times a [`Some(T)`] value, before returning [`None`]. /// This method has a default implementation, so you usually should not /// implement it directly. However, if you can provide a more efficient /// implementation, you can do so. See the [trait-level] docs for an /// example. /// - /// This function has the same safety guarantees as the [`size_hint`] - /// function. + /// This function has the same safety guarantees as the + /// [`Iterator::size_hint`] function. /// - /// [trait-level]: trait.ExactSizeIterator.html - /// [`size_hint`]: trait.Iterator.html#method.size_hint + /// [trait-level]: ExactSizeIterator + /// [`Some(T)`]: Some /// /// # Examples /// @@ -108,8 +105,8 @@ pub trait ExactSizeIterator: Iterator { /// Returns `true` if the iterator is empty. /// - /// This method has a default implementation using `self.len()`, so you - /// don't need to implement it yourself. + /// This method has a default implementation using + /// [`ExactSizeIterator::len()`], so you don't need to implement it yourself. /// /// # Examples /// diff --git a/library/core/src/iter/traits/iterator.rs b/library/core/src/iter/traits/iterator.rs index b8a09f822b6da..f70e92f0ffafe 100644 --- a/library/core/src/iter/traits/iterator.rs +++ b/library/core/src/iter/traits/iterator.rs @@ -2203,7 +2203,6 @@ pub trait Iterator { /// /// `iter.find_map(f)` is equivalent to `iter.filter_map(f).next()`. /// - /// /// # Examples /// /// ``` diff --git a/library/core/src/iter/traits/marker.rs b/library/core/src/iter/traits/marker.rs index f287196da03ef..0900676146c0d 100644 --- a/library/core/src/iter/traits/marker.rs +++ b/library/core/src/iter/traits/marker.rs @@ -2,14 +2,13 @@ /// /// Calling next on a fused iterator that has returned `None` once is guaranteed /// to return [`None`] again. This trait should be implemented by all iterators -/// that behave this way because it allows optimizing [`Iterator::fuse`]. +/// that behave this way because it allows optimizing [`Iterator::fuse()`]. /// /// Note: In general, you should not use `FusedIterator` in generic bounds if -/// you need a fused iterator. Instead, you should just call [`Iterator::fuse`] +/// you need a fused iterator. Instead, you should just call [`Iterator::fuse()`] /// on the iterator. If the iterator is already fused, the additional [`Fuse`] /// wrapper will be a no-op with no performance penalty. /// -/// [`Iterator::fuse`]: crate::iter::Iterator::fuse /// [`Fuse`]: crate::iter::Fuse #[stable(feature = "fused", since = "1.26.0")] #[rustc_unsafe_specialization_marker] @@ -24,18 +23,18 @@ impl FusedIterator for &mut I {} /// (lower bound is equal to upper bound), or the upper bound is [`None`]. /// The upper bound must only be [`None`] if the actual iterator length is /// larger than [`usize::MAX`]. In that case, the lower bound must be -/// [`usize::MAX`], resulting in a [`.size_hint`] of `(usize::MAX, None)`. +/// [`usize::MAX`], resulting in a [`Iterator::size_hint()`] of +/// `(usize::MAX, None)`. /// /// The iterator must produce exactly the number of elements it reported /// or diverge before reaching the end. /// /// # Safety /// -/// This trait must only be implemented when the contract is upheld. -/// Consumers of this trait must inspect [`.size_hint`]’s upper bound. +/// This trait must only be implemented when the contract is upheld. Consumers +/// of this trait must inspect [`Iterator::size_hint()`]’s upper bound. /// /// [`usize::MAX`]: crate::usize::MAX -/// [`.size_hint`]: crate::iter::Iterator::size_hint #[unstable(feature = "trusted_len", issue = "37572")] #[rustc_unsafe_specialization_marker] pub unsafe trait TrustedLen: Iterator {} @@ -46,11 +45,12 @@ unsafe impl TrustedLen for &mut I {} /// An iterator that when yielding an item will have taken at least one element /// from its underlying [`SourceIter`]. /// -/// Calling next() guarantees that at least one value of the iterator's underlying source +/// Calling [`next()`] guarantees that at least one value of the iterator's underlying source /// has been moved out and the result of the iterator chain could be inserted in its place, /// assuming structural constraints of the source allow such an insertion. /// In other words this trait indicates that an iterator pipeline can be collected in place. /// -/// [`SourceIter`]: ../../std/iter/trait.SourceIter.html +/// [`SourceIter`]: crate::iter::SourceIter +/// [`next()`]: Iterator::next #[unstable(issue = "none", feature = "inplace_iteration")] pub unsafe trait InPlaceIterable: Iterator {} From bffd2111f7b033902e60889da5d3fa7a033a7d5e Mon Sep 17 00:00:00 2001 From: Alexis Bourget Date: Fri, 18 Sep 2020 11:09:36 +0200 Subject: [PATCH 23/44] Finish moving to intra doc links for std::sync --- library/std/src/sync/barrier.rs | 23 +++++------- library/std/src/sync/once.rs | 62 ++++++++++++++------------------- 2 files changed, 34 insertions(+), 51 deletions(-) diff --git a/library/std/src/sync/barrier.rs b/library/std/src/sync/barrier.rs index 204d7f3084f03..b8b4baf14b4c1 100644 --- a/library/std/src/sync/barrier.rs +++ b/library/std/src/sync/barrier.rs @@ -43,11 +43,8 @@ struct BarrierState { generation_id: usize, } -/// A `BarrierWaitResult` is returned by [`wait`] when all threads in the [`Barrier`] -/// have rendezvoused. -/// -/// [`wait`]: struct.Barrier.html#method.wait -/// [`Barrier`]: struct.Barrier.html +/// A `BarrierWaitResult` is returned by [`Barrier::wait()`] when all threads +/// in the [`Barrier`] have rendezvoused. /// /// # Examples /// @@ -70,10 +67,10 @@ impl fmt::Debug for Barrier { impl Barrier { /// Creates a new barrier that can block a given number of threads. /// - /// A barrier will block `n`-1 threads which call [`wait`] and then wake up - /// all threads at once when the `n`th thread calls [`wait`]. + /// A barrier will block `n`-1 threads which call [`wait()`] and then wake + /// up all threads at once when the `n`th thread calls [`wait()`]. /// - /// [`wait`]: #method.wait + /// [`wait()`]: Barrier::wait /// /// # Examples /// @@ -99,10 +96,7 @@ impl Barrier { /// A single (arbitrary) thread will receive a [`BarrierWaitResult`] that /// returns `true` from [`is_leader`] when returning from this function, and /// all other threads will receive a result that will return `false` from - /// [`is_leader`]. - /// - /// [`BarrierWaitResult`]: struct.BarrierWaitResult.html - /// [`is_leader`]: struct.BarrierWaitResult.html#method.is_leader + /// [`BarrierWaitResult::is_leader`]. /// /// # Examples /// @@ -156,13 +150,12 @@ impl fmt::Debug for BarrierWaitResult { } impl BarrierWaitResult { - /// Returns `true` if this thread from [`wait`] is the "leader thread". + /// Returns `true` if this thread from [`Barrier::wait()`] is the + /// "leader thread". /// /// Only one thread will have `true` returned from their result, all other /// threads will have `false` returned. /// - /// [`wait`]: struct.Barrier.html#method.wait - /// /// # Examples /// /// ``` diff --git a/library/std/src/sync/once.rs b/library/std/src/sync/once.rs index 29ae338cb2ec7..ee8902bf764bf 100644 --- a/library/std/src/sync/once.rs +++ b/library/std/src/sync/once.rs @@ -95,11 +95,9 @@ use crate::thread::{self, Thread}; /// A synchronization primitive which can be used to run a one-time global /// initialization. Useful for one-time initialization for FFI or related -/// functionality. This type can only be constructed with the [`Once::new`] +/// functionality. This type can only be constructed with the [`Once::new()`] /// constructor. /// -/// [`Once::new`]: struct.Once.html#method.new -/// /// # Examples /// /// ``` @@ -126,11 +124,8 @@ unsafe impl Sync for Once {} #[stable(feature = "rust1", since = "1.0.0")] unsafe impl Send for Once {} -/// State yielded to [`call_once_force`]’s closure parameter. The state can be -/// used to query the poison status of the [`Once`]. -/// -/// [`call_once_force`]: struct.Once.html#method.call_once_force -/// [`Once`]: struct.Once.html +/// State yielded to [`Once::call_once_force()`]’s closure parameter. The state +/// can be used to query the poison status of the [`Once`]. #[unstable(feature = "once_poison", issue = "33577")] #[derive(Debug)] pub struct OnceState { @@ -140,8 +135,6 @@ pub struct OnceState { /// Initialization value for static [`Once`] values. /// -/// [`Once`]: struct.Once.html -/// /// # Examples /// /// ``` @@ -212,7 +205,7 @@ impl Once { /// happens-before relation between the closure and code executing after the /// return). /// - /// If the given closure recursively invokes `call_once` on the same `Once` + /// If the given closure recursively invokes `call_once` on the same [`Once`] /// instance the exact behavior is not specified, allowed outcomes are /// a panic or a deadlock. /// @@ -249,7 +242,7 @@ impl Once { /// /// The closure `f` will only be executed once if this is called /// concurrently amongst many threads. If that closure panics, however, then - /// it will *poison* this `Once` instance, causing all future invocations of + /// it will *poison* this [`Once`] instance, causing all future invocations of /// `call_once` to also panic. /// /// This is similar to [poisoning with mutexes][poison]. @@ -269,21 +262,21 @@ impl Once { self.call_inner(false, &mut |_| f.take().unwrap()()); } - /// Performs the same function as [`call_once`] except ignores poisoning. + /// Performs the same function as [`call_once()`] except ignores poisoning. /// - /// Unlike [`call_once`], if this `Once` has been poisoned (i.e., a previous - /// call to `call_once` or `call_once_force` caused a panic), calling - /// `call_once_force` will still invoke the closure `f` and will _not_ - /// result in an immediate panic. If `f` panics, the `Once` will remain - /// in a poison state. If `f` does _not_ panic, the `Once` will no - /// longer be in a poison state and all future calls to `call_once` or - /// `call_once_force` will be no-ops. + /// Unlike [`call_once()`], if this [`Once`] has been poisoned (i.e., a previous + /// call to [`call_once()`] or [`call_once_force()`] caused a panic), calling + /// [`call_once_force()`] will still invoke the closure `f` and will _not_ + /// result in an immediate panic. If `f` panics, the [`Once`] will remain + /// in a poison state. If `f` does _not_ panic, the [`Once`] will no + /// longer be in a poison state and all future calls to [`call_once()`] or + /// [`call_once_force()`] will be no-ops. /// /// The closure `f` is yielded a [`OnceState`] structure which can be used - /// to query the poison status of the `Once`. + /// to query the poison status of the [`Once`]. /// - /// [`call_once`]: struct.Once.html#method.call_once - /// [`OnceState`]: struct.OnceState.html + /// [`call_once()`]: Once::call_once + /// [`call_once_force()`]: Once::call_once_force /// /// # Examples /// @@ -329,18 +322,20 @@ impl Once { self.call_inner(true, &mut |p| f.take().unwrap()(p)); } - /// Returns `true` if some `call_once` call has completed + /// Returns `true` if some [`call_once()`] call has completed /// successfully. Specifically, `is_completed` will return false in /// the following situations: - /// * `call_once` was not called at all, - /// * `call_once` was called, but has not yet completed, - /// * the `Once` instance is poisoned + /// * [`call_once()`] was not called at all, + /// * [`call_once()`] was called, but has not yet completed, + /// * the [`Once`] instance is poisoned /// - /// This function returning `false` does not mean that `Once` has not been + /// This function returning `false` does not mean that [`Once`] has not been /// executed. For example, it may have been executed in the time between /// when `is_completed` starts executing and when it returns, in which case /// the `false` return value would be stale (but still permissible). /// + /// [`call_once()`]: Once::call_once + /// /// # Examples /// /// ``` @@ -519,14 +514,11 @@ impl Drop for WaiterQueue<'_> { impl OnceState { /// Returns `true` if the associated [`Once`] was poisoned prior to the - /// invocation of the closure passed to [`call_once_force`]. - /// - /// [`call_once_force`]: struct.Once.html#method.call_once_force - /// [`Once`]: struct.Once.html + /// invocation of the closure passed to [`Once::call_once_force()`]. /// /// # Examples /// - /// A poisoned `Once`: + /// A poisoned [`Once`]: /// /// ``` /// #![feature(once_poison)] @@ -547,7 +539,7 @@ impl OnceState { /// }); /// ``` /// - /// An unpoisoned `Once`: + /// An unpoisoned [`Once`]: /// /// ``` /// #![feature(once_poison)] @@ -565,8 +557,6 @@ impl OnceState { } /// Poison the associated [`Once`] without explicitly panicking. - /// - /// [`Once`]: struct.Once.html // NOTE: This is currently only exposed for the `lazy` module pub(crate) fn poison(&self) { self.set_state_on_drop_to.set(POISONED); From 982ec0d0c9ed93d806340502d48de190e1558a64 Mon Sep 17 00:00:00 2001 From: Alexis Bourget Date: Fri, 18 Sep 2020 11:14:36 +0200 Subject: [PATCH 24/44] Fix broken link --- library/core/src/iter/sources.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/core/src/iter/sources.rs b/library/core/src/iter/sources.rs index 0348d5a10d984..c28538ef027c0 100644 --- a/library/core/src/iter/sources.rs +++ b/library/core/src/iter/sources.rs @@ -497,7 +497,7 @@ pub fn once_with A>(gen: F) -> OnceWith { /// The closure can use captures and its environment to track state across iterations. Depending on /// how the iterator is used, this may require specifying the [`move`] keyword on the closure. /// -/// [`move`]: ../../../std/keyword.move.html +/// [`move`]: ../../std/keyword.move.html /// /// # Examples /// From b534d9f6e1e2b3e77842c5ededa62f6bcfb2ea58 Mon Sep 17 00:00:00 2001 From: Alexis Bourget Date: Fri, 18 Sep 2020 12:04:49 +0200 Subject: [PATCH 25/44] Fix broken link --- library/std/src/sync/barrier.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/library/std/src/sync/barrier.rs b/library/std/src/sync/barrier.rs index b8b4baf14b4c1..8d3e30dbd4545 100644 --- a/library/std/src/sync/barrier.rs +++ b/library/std/src/sync/barrier.rs @@ -94,9 +94,9 @@ impl Barrier { /// be used continuously. /// /// A single (arbitrary) thread will receive a [`BarrierWaitResult`] that - /// returns `true` from [`is_leader`] when returning from this function, and - /// all other threads will receive a result that will return `false` from - /// [`BarrierWaitResult::is_leader`]. + /// returns `true` from [`BarrierWaitResult::is_leader()`] when returning + /// from this function, and all other threads will receive a result that + /// will return `false` from [`BarrierWaitResult::is_leader()`]. /// /// # Examples /// From e3c6e46168758642f0bab64da374f93ed21b1cd0 Mon Sep 17 00:00:00 2001 From: Christiaan Dirkx Date: Fri, 18 Sep 2020 19:23:50 +0200 Subject: [PATCH 26/44] Make some methods of `Pin<&mut T>` unstable const Make the following methods unstable const under the `const_pin` feature: - `into_ref` - `get_mut` - `get_unchecked_mut` --- library/core/src/pin.rs | 13 ++++++++----- library/core/tests/lib.rs | 1 + library/core/tests/pin.rs | 12 +++++++++++- 3 files changed, 20 insertions(+), 6 deletions(-) diff --git a/library/core/src/pin.rs b/library/core/src/pin.rs index fa5b37edc36e6..9f0284d5d9542 100644 --- a/library/core/src/pin.rs +++ b/library/core/src/pin.rs @@ -708,8 +708,9 @@ impl<'a, T: ?Sized> Pin<&'a T> { impl<'a, T: ?Sized> Pin<&'a mut T> { /// Converts this `Pin<&mut T>` into a `Pin<&T>` with the same lifetime. #[inline(always)] + #[rustc_const_unstable(feature = "const_pin", issue = "76654")] #[stable(feature = "pin", since = "1.33.0")] - pub fn into_ref(self) -> Pin<&'a T> { + pub const fn into_ref(self) -> Pin<&'a T> { Pin { pointer: self.pointer } } @@ -722,9 +723,10 @@ impl<'a, T: ?Sized> Pin<&'a mut T> { /// that lives for as long as the borrow of the `Pin`, not the lifetime of /// the `Pin` itself. This method allows turning the `Pin` into a reference /// with the same lifetime as the original `Pin`. - #[stable(feature = "pin", since = "1.33.0")] #[inline(always)] - pub fn get_mut(self) -> &'a mut T + #[stable(feature = "pin", since = "1.33.0")] + #[rustc_const_unstable(feature = "const_pin", issue = "76654")] + pub const fn get_mut(self) -> &'a mut T where T: Unpin, { @@ -741,9 +743,10 @@ impl<'a, T: ?Sized> Pin<&'a mut T> { /// /// If the underlying data is `Unpin`, `Pin::get_mut` should be used /// instead. - #[stable(feature = "pin", since = "1.33.0")] #[inline(always)] - pub unsafe fn get_unchecked_mut(self) -> &'a mut T { + #[stable(feature = "pin", since = "1.33.0")] + #[rustc_const_unstable(feature = "const_pin", issue = "76654")] + pub const unsafe fn get_unchecked_mut(self) -> &'a mut T { self.pointer } diff --git a/library/core/tests/lib.rs b/library/core/tests/lib.rs index b8d67d7266543..490f016ab8b2e 100644 --- a/library/core/tests/lib.rs +++ b/library/core/tests/lib.rs @@ -39,6 +39,7 @@ #![feature(iter_order_by)] #![feature(cmp_min_max_by)] #![feature(iter_map_while)] +#![feature(const_mut_refs)] #![feature(const_pin)] #![feature(const_slice_from_raw_parts)] #![feature(const_raw_ptr_deref)] diff --git a/library/core/tests/pin.rs b/library/core/tests/pin.rs index 1363353163829..6f617c8d0c297 100644 --- a/library/core/tests/pin.rs +++ b/library/core/tests/pin.rs @@ -17,5 +17,15 @@ fn pin_const() { assert_eq!(INNER_UNCHECKED, POINTER); const REF: &'static usize = PINNED.get_ref(); - assert_eq!(REF, POINTER) + assert_eq!(REF, POINTER); + + // Note: `pin_mut_const` tests that the methods of `Pin<&mut T>` are usable in a const context. + // A const fn is used because `&mut` is not (yet) usable in constants. + const fn pin_mut_const() { + let _ = Pin::new(&mut 2).into_ref(); + let _ = Pin::new(&mut 2).get_mut(); + let _ = unsafe { Pin::new(&mut 2).get_unchecked_mut() }; + } + + pin_mut_const(); } From 673935fc0c8cbb4925e4fa0c936e4340aa818be0 Mon Sep 17 00:00:00 2001 From: Simon Vandel Sillesen Date: Sat, 19 Sep 2020 13:52:55 +0200 Subject: [PATCH 27/44] Get LocalDefId from source instead of passing in --- compiler/rustc_mir/src/transform/mod.rs | 2 +- .../rustc_mir/src/transform/remove_unneeded_drops.rs | 12 ++---------- 2 files changed, 3 insertions(+), 11 deletions(-) diff --git a/compiler/rustc_mir/src/transform/mod.rs b/compiler/rustc_mir/src/transform/mod.rs index 3e831a48c2577..685a56ad5de9c 100644 --- a/compiler/rustc_mir/src/transform/mod.rs +++ b/compiler/rustc_mir/src/transform/mod.rs @@ -456,7 +456,7 @@ fn run_optimization_passes<'tcx>( // The main optimizations that we do on MIR. let optimizations: &[&dyn MirPass<'tcx>] = &[ - &remove_unneeded_drops::RemoveUnneededDrops::new(def_id), + &remove_unneeded_drops::RemoveUnneededDrops, &match_branches::MatchBranchSimplification, // inst combine is after MatchBranchSimplification to clean up Ne(_1, false) &instcombine::InstCombine, diff --git a/compiler/rustc_mir/src/transform/remove_unneeded_drops.rs b/compiler/rustc_mir/src/transform/remove_unneeded_drops.rs index 393b0f923b910..505133219b863 100644 --- a/compiler/rustc_mir/src/transform/remove_unneeded_drops.rs +++ b/compiler/rustc_mir/src/transform/remove_unneeded_drops.rs @@ -6,15 +6,7 @@ use rustc_middle::mir::visit::Visitor; use rustc_middle::mir::*; use rustc_middle::ty::TyCtxt; -pub struct RemoveUnneededDrops { - def_id: LocalDefId, -} - -impl RemoveUnneededDrops { - pub fn new(def_id: LocalDefId) -> Self { - Self { def_id } - } -} +pub struct RemoveUnneededDrops; impl<'tcx> MirPass<'tcx> for RemoveUnneededDrops { fn run_pass(&self, tcx: TyCtxt<'tcx>, source: MirSource<'tcx>, body: &mut Body<'tcx>) { @@ -23,7 +15,7 @@ impl<'tcx> MirPass<'tcx> for RemoveUnneededDrops { tcx, body, optimizations: vec![], - def_id: self.def_id, + def_id: source.def_id().expect_local(), }; opt_finder.visit_body(body); for (loc, target) in opt_finder.optimizations { From 30dd6cf9ba472d4ff12c9a28d03e3a61d54e34da Mon Sep 17 00:00:00 2001 From: Simon Vandel Sillesen Date: Sat, 19 Sep 2020 14:25:53 +0200 Subject: [PATCH 28/44] The optimization should also apply for DropAndReplace --- compiler/rustc_mir/src/transform/remove_unneeded_drops.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/compiler/rustc_mir/src/transform/remove_unneeded_drops.rs b/compiler/rustc_mir/src/transform/remove_unneeded_drops.rs index 505133219b863..b8ad816f75831 100644 --- a/compiler/rustc_mir/src/transform/remove_unneeded_drops.rs +++ b/compiler/rustc_mir/src/transform/remove_unneeded_drops.rs @@ -29,7 +29,8 @@ impl<'tcx> MirPass<'tcx> for RemoveUnneededDrops { impl<'a, 'tcx> Visitor<'tcx> for RemoveUnneededDropsOptimizationFinder<'a, 'tcx> { fn visit_terminator(&mut self, terminator: &Terminator<'tcx>, location: Location) { match terminator.kind { - TerminatorKind::Drop { place, target, .. } => { + TerminatorKind::Drop { place, target, .. } + | TerminatorKind::DropAndReplace { place, target, .. } => { let ty = place.ty(self.body, self.tcx); let needs_drop = ty.ty.needs_drop(self.tcx, self.tcx.param_env(self.def_id)); if !needs_drop { From 804f673762d732198e5023be6fa8f1e305e2c2ad Mon Sep 17 00:00:00 2001 From: Simon Vandel Sillesen Date: Sat, 19 Sep 2020 15:21:39 +0200 Subject: [PATCH 29/44] cleanup cfg after optimization --- .../rustc_mir/src/transform/remove_unneeded_drops.rs | 9 +++++++++ .../remove_unneeded_drops.opt.RemoveUnneededDrops.diff | 7 +++---- ...eeded_drops.opt_generic_copy.RemoveUnneededDrops.diff | 7 +++---- 3 files changed, 15 insertions(+), 8 deletions(-) diff --git a/compiler/rustc_mir/src/transform/remove_unneeded_drops.rs b/compiler/rustc_mir/src/transform/remove_unneeded_drops.rs index b8ad816f75831..f027f5e33a187 100644 --- a/compiler/rustc_mir/src/transform/remove_unneeded_drops.rs +++ b/compiler/rustc_mir/src/transform/remove_unneeded_drops.rs @@ -6,6 +6,8 @@ use rustc_middle::mir::visit::Visitor; use rustc_middle::mir::*; use rustc_middle::ty::TyCtxt; +use super::simplify::simplify_cfg; + pub struct RemoveUnneededDrops; impl<'tcx> MirPass<'tcx> for RemoveUnneededDrops { @@ -18,11 +20,18 @@ impl<'tcx> MirPass<'tcx> for RemoveUnneededDrops { def_id: source.def_id().expect_local(), }; opt_finder.visit_body(body); + let should_simplify = !opt_finder.optimizations.is_empty(); for (loc, target) in opt_finder.optimizations { let terminator = body.basic_blocks_mut()[loc.block].terminator_mut(); debug!("SUCCESS: replacing `drop` with goto({:?})", target); terminator.kind = TerminatorKind::Goto { target }; } + + // if we applied optimizations, we potentially have some cfg to cleanup to + // make it easier for further passes + if should_simplify { + simplify_cfg(body); + } } } diff --git a/src/test/mir-opt/remove_unneeded_drops.opt.RemoveUnneededDrops.diff b/src/test/mir-opt/remove_unneeded_drops.opt.RemoveUnneededDrops.diff index 34193ccc5c7e7..eba839cf0a48b 100644 --- a/src/test/mir-opt/remove_unneeded_drops.opt.RemoveUnneededDrops.diff +++ b/src/test/mir-opt/remove_unneeded_drops.opt.RemoveUnneededDrops.diff @@ -16,10 +16,9 @@ _3 = _1; // scope 0 at $DIR/remove_unneeded_drops.rs:3:10: 3:11 _2 = const (); // scope 1 at $SRC_DIR/core/src/mem/mod.rs:LL:COL - drop(_3) -> bb1; // scope 1 at $SRC_DIR/core/src/mem/mod.rs:LL:COL -+ goto -> bb1; // scope 1 at $SRC_DIR/core/src/mem/mod.rs:LL:COL - } - - bb1: { +- } +- +- bb1: { StorageDead(_3); // scope 0 at $DIR/remove_unneeded_drops.rs:3:11: 3:12 StorageDead(_2); // scope 0 at $DIR/remove_unneeded_drops.rs:3:12: 3:13 _0 = const (); // scope 0 at $DIR/remove_unneeded_drops.rs:2:17: 4:2 diff --git a/src/test/mir-opt/remove_unneeded_drops.opt_generic_copy.RemoveUnneededDrops.diff b/src/test/mir-opt/remove_unneeded_drops.opt_generic_copy.RemoveUnneededDrops.diff index 2e6c8c8a78a72..840b1ba30fb9b 100644 --- a/src/test/mir-opt/remove_unneeded_drops.opt_generic_copy.RemoveUnneededDrops.diff +++ b/src/test/mir-opt/remove_unneeded_drops.opt_generic_copy.RemoveUnneededDrops.diff @@ -16,10 +16,9 @@ _3 = _1; // scope 0 at $DIR/remove_unneeded_drops.rs:13:10: 13:11 _2 = const (); // scope 1 at $SRC_DIR/core/src/mem/mod.rs:LL:COL - drop(_3) -> bb1; // scope 1 at $SRC_DIR/core/src/mem/mod.rs:LL:COL -+ goto -> bb1; // scope 1 at $SRC_DIR/core/src/mem/mod.rs:LL:COL - } - - bb1: { +- } +- +- bb1: { StorageDead(_3); // scope 0 at $DIR/remove_unneeded_drops.rs:13:11: 13:12 StorageDead(_2); // scope 0 at $DIR/remove_unneeded_drops.rs:13:12: 13:13 _0 = const (); // scope 0 at $DIR/remove_unneeded_drops.rs:12:36: 14:2 From 924cd135b6ab0fe48dae26d9ae30eaadebcd066d Mon Sep 17 00:00:00 2001 From: Giacomo Stevanato Date: Sat, 5 Sep 2020 23:16:56 +0200 Subject: [PATCH 30/44] Added benchmarks for BinaryHeap --- library/alloc/benches/binary_heap.rs | 91 ++++++++++++++++++++++++++++ library/alloc/benches/lib.rs | 1 + 2 files changed, 92 insertions(+) create mode 100644 library/alloc/benches/binary_heap.rs diff --git a/library/alloc/benches/binary_heap.rs b/library/alloc/benches/binary_heap.rs new file mode 100644 index 0000000000000..5b6538ea6c6b3 --- /dev/null +++ b/library/alloc/benches/binary_heap.rs @@ -0,0 +1,91 @@ +use std::collections::BinaryHeap; + +use rand::{seq::SliceRandom, thread_rng}; +use test::{black_box, Bencher}; + +#[bench] +fn bench_find_smallest_1000(b: &mut Bencher) { + let mut rng = thread_rng(); + let mut vec: Vec = (0..100_000).collect(); + vec.shuffle(&mut rng); + + b.iter(|| { + let mut iter = vec.iter().copied(); + let mut heap: BinaryHeap<_> = iter.by_ref().take(1000).collect(); + + for x in iter { + let mut max = heap.peek_mut().unwrap(); + // This comparison should be true only 1% of the time. + // Unnecessary `sift_down`s will degrade performance + if x < *max { + *max = x; + } + } + + heap + }) +} + +#[bench] +fn bench_peek_mut_deref_mut(b: &mut Bencher) { + let mut bheap = BinaryHeap::from(vec![42]); + let vec: Vec = (0..1_000_000).collect(); + + b.iter(|| { + let vec = black_box(&vec); + let mut peek_mut = bheap.peek_mut().unwrap(); + // The compiler shouldn't be able to optimize away the `sift_down` + // assignment in `PeekMut`'s `DerefMut` implementation since + // the loop may not run. + for &i in vec.iter() { + *peek_mut = i; + } + // Remove the already minimal overhead of the sift_down + std::mem::forget(peek_mut); + }) +} + +#[bench] +fn bench_from_vec(b: &mut Bencher) { + let mut rng = thread_rng(); + let mut vec: Vec = (0..100_000).collect(); + vec.shuffle(&mut rng); + + b.iter(|| BinaryHeap::from(vec.clone())) +} + +#[bench] +fn bench_into_sorted_vec(b: &mut Bencher) { + let bheap: BinaryHeap = (0..10_000).collect(); + + b.iter(|| bheap.clone().into_sorted_vec()) +} + +#[bench] +fn bench_push(b: &mut Bencher) { + let mut bheap = BinaryHeap::with_capacity(50_000); + let mut rng = thread_rng(); + let mut vec: Vec = (0..50_000).collect(); + vec.shuffle(&mut rng); + + b.iter(|| { + for &i in vec.iter() { + bheap.push(i); + } + black_box(&mut bheap); + bheap.clear(); + }) +} + +#[bench] +fn bench_pop(b: &mut Bencher) { + let mut bheap = BinaryHeap::with_capacity(10_000); + + b.iter(|| { + bheap.extend((0..10_000).rev()); + black_box(&mut bheap); + while let Some(elem) = bheap.pop() { + black_box(elem); + } + }) +} diff --git a/library/alloc/benches/lib.rs b/library/alloc/benches/lib.rs index 608eafc88d2a6..32edb86d10119 100644 --- a/library/alloc/benches/lib.rs +++ b/library/alloc/benches/lib.rs @@ -8,6 +8,7 @@ extern crate test; +mod binary_heap; mod btree; mod linked_list; mod slice; From af1e3633f734930a50000cc424fbcdc6629885c3 Mon Sep 17 00:00:00 2001 From: Giacomo Stevanato Date: Wed, 26 Aug 2020 23:52:20 +0200 Subject: [PATCH 31/44] Set sift=true only when PeekMut yields a mutable reference --- library/alloc/src/collections/binary_heap.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/library/alloc/src/collections/binary_heap.rs b/library/alloc/src/collections/binary_heap.rs index 477a598ff5b00..e3b738a70c888 100644 --- a/library/alloc/src/collections/binary_heap.rs +++ b/library/alloc/src/collections/binary_heap.rs @@ -293,6 +293,7 @@ impl Deref for PeekMut<'_, T> { impl DerefMut for PeekMut<'_, T> { fn deref_mut(&mut self) -> &mut T { debug_assert!(!self.heap.is_empty()); + self.sift = true; // SAFE: PeekMut is only instantiated for non-empty heaps unsafe { self.heap.data.get_unchecked_mut(0) } } @@ -401,7 +402,7 @@ impl BinaryHeap { /// Cost is *O*(1) in the worst case. #[stable(feature = "binary_heap_peek_mut", since = "1.12.0")] pub fn peek_mut(&mut self) -> Option> { - if self.is_empty() { None } else { Some(PeekMut { heap: self, sift: true }) } + if self.is_empty() { None } else { Some(PeekMut { heap: self, sift: false }) } } /// Removes the greatest item from the binary heap and returns it, or `None` if it From ca15e9d8a11e7c25eb85930d1da3009647c1680e Mon Sep 17 00:00:00 2001 From: Giacomo Stevanato Date: Sun, 20 Sep 2020 01:07:34 +0200 Subject: [PATCH 32/44] Fix time complexity in BinaryHeap::peek_mut docs --- library/alloc/src/collections/binary_heap.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/library/alloc/src/collections/binary_heap.rs b/library/alloc/src/collections/binary_heap.rs index e3b738a70c888..06e5ac037ca79 100644 --- a/library/alloc/src/collections/binary_heap.rs +++ b/library/alloc/src/collections/binary_heap.rs @@ -399,7 +399,8 @@ impl BinaryHeap { /// /// # Time complexity /// - /// Cost is *O*(1) in the worst case. + /// If the item is modified then the worst case time complexity is *O*(log(*n*)), + /// otherwise it's *O*(1). #[stable(feature = "binary_heap_peek_mut", since = "1.12.0")] pub fn peek_mut(&mut self) -> Option> { if self.is_empty() { None } else { Some(PeekMut { heap: self, sift: false }) } From 2a00dda90258576e3adf5ecae0437a8fe6fadbcf Mon Sep 17 00:00:00 2001 From: Bastian Kauschke Date: Thu, 10 Sep 2020 19:43:53 +0200 Subject: [PATCH 33/44] miri: correctly deal with `ConstKind::Bound` --- compiler/rustc_mir/src/interpret/operand.rs | 6 ++-- .../ui/const-generics/issues/issue-73260.rs | 20 +++++++++++++ .../const-generics/issues/issue-73260.stderr | 29 +++++++++++++++++++ .../ui/const-generics/issues/issue-74634.rs | 27 +++++++++++++++++ .../const-generics/issues/issue-74634.stderr | 10 +++++++ 5 files changed, 88 insertions(+), 4 deletions(-) create mode 100644 src/test/ui/const-generics/issues/issue-73260.rs create mode 100644 src/test/ui/const-generics/issues/issue-73260.stderr create mode 100644 src/test/ui/const-generics/issues/issue-74634.rs create mode 100644 src/test/ui/const-generics/issues/issue-74634.stderr diff --git a/compiler/rustc_mir/src/interpret/operand.rs b/compiler/rustc_mir/src/interpret/operand.rs index 57245696e576e..136a2699d20b2 100644 --- a/compiler/rustc_mir/src/interpret/operand.rs +++ b/compiler/rustc_mir/src/interpret/operand.rs @@ -549,7 +549,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { }; // Early-return cases. let val_val = match val.val { - ty::ConstKind::Param(_) => throw_inval!(TooGeneric), + ty::ConstKind::Param(_) | ty::ConstKind::Bound(..) => throw_inval!(TooGeneric), ty::ConstKind::Error(_) => throw_inval!(TypeckError(ErrorReported)), ty::ConstKind::Unevaluated(def, substs, promoted) => { let instance = self.resolve(def.did, substs)?; @@ -561,9 +561,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { // happening. return Ok(self.const_eval(GlobalId { instance, promoted }, val.ty)?); } - ty::ConstKind::Infer(..) - | ty::ConstKind::Bound(..) - | ty::ConstKind::Placeholder(..) => { + ty::ConstKind::Infer(..) | ty::ConstKind::Placeholder(..) => { span_bug!(self.cur_span(), "const_to_op: Unexpected ConstKind {:?}", val) } ty::ConstKind::Value(val_val) => val_val, diff --git a/src/test/ui/const-generics/issues/issue-73260.rs b/src/test/ui/const-generics/issues/issue-73260.rs new file mode 100644 index 0000000000000..351d6849af5db --- /dev/null +++ b/src/test/ui/const-generics/issues/issue-73260.rs @@ -0,0 +1,20 @@ +// compile-flags: -Zsave-analysis + +#![feature(const_generics)] +#![allow(incomplete_features)] +struct Arr +where Assert::<{N < usize::max_value() / 2}>: IsTrue, //~ ERROR constant expression +{ +} + +enum Assert {} + +trait IsTrue {} + +impl IsTrue for Assert {} + +fn main() { + let x: Arr<{usize::max_value()}> = Arr {}; + //~^ ERROR mismatched types + //~| ERROR mismatched types +} diff --git a/src/test/ui/const-generics/issues/issue-73260.stderr b/src/test/ui/const-generics/issues/issue-73260.stderr new file mode 100644 index 0000000000000..e22612ed5ea63 --- /dev/null +++ b/src/test/ui/const-generics/issues/issue-73260.stderr @@ -0,0 +1,29 @@ +error: constant expression depends on a generic parameter + --> $DIR/issue-73260.rs:6:47 + | +LL | where Assert::<{N < usize::max_value() / 2}>: IsTrue, + | ^^^^^^ + | + = note: this may fail depending on what value the parameter takes + +error[E0308]: mismatched types + --> $DIR/issue-73260.rs:17:12 + | +LL | let x: Arr<{usize::max_value()}> = Arr {}; + | ^^^^^^^^^^^^^^^^^^^^^^^^^ expected `false`, found `true` + | + = note: expected type `false` + found type `true` + +error[E0308]: mismatched types + --> $DIR/issue-73260.rs:17:40 + | +LL | let x: Arr<{usize::max_value()}> = Arr {}; + | ^^^ expected `false`, found `true` + | + = note: expected type `false` + found type `true` + +error: aborting due to 3 previous errors + +For more information about this error, try `rustc --explain E0308`. diff --git a/src/test/ui/const-generics/issues/issue-74634.rs b/src/test/ui/const-generics/issues/issue-74634.rs new file mode 100644 index 0000000000000..0f23fa92c3679 --- /dev/null +++ b/src/test/ui/const-generics/issues/issue-74634.rs @@ -0,0 +1,27 @@ +#![feature(const_generics)] +#![allow(incomplete_features)] + +trait If {} +impl If for () {} + +trait IsZero { + type Answer; +} + +struct True; +struct False; + +impl IsZero for () +where (): If<{N == 0}> { //~ERROR constant expression + type Answer = True; +} + +trait Foobar {} + +impl Foobar for () +where (): IsZero {} + +impl Foobar for () +where (): IsZero {} + +fn main() {} diff --git a/src/test/ui/const-generics/issues/issue-74634.stderr b/src/test/ui/const-generics/issues/issue-74634.stderr new file mode 100644 index 0000000000000..091a1ac7b9981 --- /dev/null +++ b/src/test/ui/const-generics/issues/issue-74634.stderr @@ -0,0 +1,10 @@ +error: constant expression depends on a generic parameter + --> $DIR/issue-74634.rs:15:11 + | +LL | where (): If<{N == 0}> { + | ^^^^^^^^^^^^ + | + = note: this may fail depending on what value the parameter takes + +error: aborting due to previous error + From 67342304253d8af47fe4453fbe2396b627620431 Mon Sep 17 00:00:00 2001 From: Bastian Kauschke Date: Fri, 11 Sep 2020 18:39:26 +0200 Subject: [PATCH 34/44] do not ICE on `ty::Bound` in Layout::compute --- compiler/rustc_middle/src/ty/layout.rs | 4 ++-- .../ui/const-generics/issues/issue-76595.rs | 18 ++++++++++++++++ .../const-generics/issues/issue-76595.stderr | 21 +++++++++++++++++++ 3 files changed, 41 insertions(+), 2 deletions(-) create mode 100644 src/test/ui/const-generics/issues/issue-76595.rs create mode 100644 src/test/ui/const-generics/issues/issue-76595.stderr diff --git a/compiler/rustc_middle/src/ty/layout.rs b/compiler/rustc_middle/src/ty/layout.rs index b0a1413a9d62f..cb79b089d94a0 100644 --- a/compiler/rustc_middle/src/ty/layout.rs +++ b/compiler/rustc_middle/src/ty/layout.rs @@ -1259,11 +1259,11 @@ impl<'tcx> LayoutCx<'tcx, TyCtxt<'tcx>> { tcx.layout_raw(param_env.and(normalized))? } - ty::Bound(..) | ty::Placeholder(..) | ty::GeneratorWitness(..) | ty::Infer(_) => { + ty::Placeholder(..) | ty::GeneratorWitness(..) | ty::Infer(_) => { bug!("Layout::compute: unexpected type `{}`", ty) } - ty::Param(_) | ty::Error(_) => { + ty::Bound(..) | ty::Param(_) | ty::Error(_) => { return Err(LayoutError::Unknown(ty)); } }) diff --git a/src/test/ui/const-generics/issues/issue-76595.rs b/src/test/ui/const-generics/issues/issue-76595.rs new file mode 100644 index 0000000000000..0a16ca181f557 --- /dev/null +++ b/src/test/ui/const-generics/issues/issue-76595.rs @@ -0,0 +1,18 @@ +#![feature(const_generics, const_evaluatable_checked)] +#![allow(incomplete_features)] + +struct Bool; + +trait True {} + +impl True for Bool {} + +fn test() where Bool<{core::mem::size_of::() > 4}>: True { + todo!() +} + +fn main() { + test::<2>(); + //~^ ERROR wrong number of type + //~| ERROR constant expression depends +} diff --git a/src/test/ui/const-generics/issues/issue-76595.stderr b/src/test/ui/const-generics/issues/issue-76595.stderr new file mode 100644 index 0000000000000..4235bdc9b893c --- /dev/null +++ b/src/test/ui/const-generics/issues/issue-76595.stderr @@ -0,0 +1,21 @@ +error[E0107]: wrong number of type arguments: expected 1, found 0 + --> $DIR/issue-76595.rs:15:5 + | +LL | test::<2>(); + | ^^^^^^^^^ expected 1 type argument + +error: constant expression depends on a generic parameter + --> $DIR/issue-76595.rs:15:5 + | +LL | fn test() where Bool<{core::mem::size_of::() > 4}>: True { + | ---- required by a bound in this +... +LL | test::<2>(); + | ^^^^^^^^^ + | + = note: this may fail depending on what value the parameter takes + = note: required by this bound in `test` + +error: aborting due to 2 previous errors + +For more information about this error, try `rustc --explain E0107`. From 65b3419ca04fdf921309e7fc03010d9d2cc9b8f0 Mon Sep 17 00:00:00 2001 From: Bastian Kauschke Date: Sun, 20 Sep 2020 09:32:59 +0200 Subject: [PATCH 35/44] update stderr file --- src/test/ui/const-generics/issues/issue-76595.stderr | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/test/ui/const-generics/issues/issue-76595.stderr b/src/test/ui/const-generics/issues/issue-76595.stderr index 4235bdc9b893c..2e457595393ca 100644 --- a/src/test/ui/const-generics/issues/issue-76595.stderr +++ b/src/test/ui/const-generics/issues/issue-76595.stderr @@ -8,13 +8,12 @@ error: constant expression depends on a generic parameter --> $DIR/issue-76595.rs:15:5 | LL | fn test() where Bool<{core::mem::size_of::() > 4}>: True { - | ---- required by a bound in this + | ---- required by this bound in `test` ... LL | test::<2>(); | ^^^^^^^^^ | = note: this may fail depending on what value the parameter takes - = note: required by this bound in `test` error: aborting due to 2 previous errors From cebbd9fcd35a63569b8fb5c836b5a26089861c41 Mon Sep 17 00:00:00 2001 From: est31 Date: Sun, 20 Sep 2020 10:12:57 +0200 Subject: [PATCH 36/44] Use as_nanos in bench.rs and base.rs --- compiler/rustc_codegen_llvm/src/base.rs | 2 +- library/test/src/bench.rs | 6 +----- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/compiler/rustc_codegen_llvm/src/base.rs b/compiler/rustc_codegen_llvm/src/base.rs index 6a1b373ef0711..f35708b1d0965 100644 --- a/compiler/rustc_codegen_llvm/src/base.rs +++ b/compiler/rustc_codegen_llvm/src/base.rs @@ -108,7 +108,7 @@ pub fn compile_codegen_unit( // We assume that the cost to run LLVM on a CGU is proportional to // the time we needed for codegenning it. - let cost = time_to_codegen.as_secs() * 1_000_000_000 + time_to_codegen.subsec_nanos() as u64; + let cost = time_to_codegen.as_nanos() as u64; fn module_codegen(tcx: TyCtxt<'_>, cgu_name: Symbol) -> ModuleCodegen { let cgu = tcx.codegen_unit(cgu_name); diff --git a/library/test/src/bench.rs b/library/test/src/bench.rs index e92e5b9829ec2..b709c76329637 100644 --- a/library/test/src/bench.rs +++ b/library/test/src/bench.rs @@ -98,10 +98,6 @@ fn fmt_thousands_sep(mut n: usize, sep: char) -> String { output } -fn ns_from_dur(dur: Duration) -> u64 { - dur.as_secs() * 1_000_000_000 + (dur.subsec_nanos() as u64) -} - fn ns_iter_inner(inner: &mut F, k: u64) -> u64 where F: FnMut() -> T, @@ -110,7 +106,7 @@ where for _ in 0..k { black_box(inner()); } - ns_from_dur(start.elapsed()) + start.elapsed().as_nanos() as u64 } pub fn iter(inner: &mut F) -> stats::Summary From 43193dcb882466163436057e50c96bb74d9bf50f Mon Sep 17 00:00:00 2001 From: est31 Date: Sun, 20 Sep 2020 10:27:14 +0200 Subject: [PATCH 37/44] Use as_secs_f64 in profiling.rs --- compiler/rustc_data_structures/src/profiling.rs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/compiler/rustc_data_structures/src/profiling.rs b/compiler/rustc_data_structures/src/profiling.rs index 07d16c6483ec7..363879cbb1d19 100644 --- a/compiler/rustc_data_structures/src/profiling.rs +++ b/compiler/rustc_data_structures/src/profiling.rs @@ -600,10 +600,7 @@ pub fn print_time_passes_entry(do_it: bool, what: &str, dur: Duration) { // Hack up our own formatting for the duration to make it easier for scripts // to parse (always use the same number of decimal places and the same unit). pub fn duration_to_secs_str(dur: std::time::Duration) -> String { - const NANOS_PER_SEC: f64 = 1_000_000_000.0; - let secs = dur.as_secs() as f64 + dur.subsec_nanos() as f64 / NANOS_PER_SEC; - - format!("{:.3}", secs) + format!("{:.3}", dur.as_secs_f64()) } // Memory reporting From 4bc0e55ac4280be80d8cd0f9bc26bd0949f75494 Mon Sep 17 00:00:00 2001 From: est31 Date: Sun, 20 Sep 2020 10:35:23 +0200 Subject: [PATCH 38/44] Replace write_fmt with write! Latter is simpler --- library/test/src/bench.rs | 20 ++++++++++---------- src/tools/unstable-book-gen/src/main.rs | 4 ++-- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/library/test/src/bench.rs b/library/test/src/bench.rs index e92e5b9829ec2..83be1cce63d52 100644 --- a/library/test/src/bench.rs +++ b/library/test/src/bench.rs @@ -61,15 +61,15 @@ pub fn fmt_bench_samples(bs: &BenchSamples) -> String { let median = bs.ns_iter_summ.median as usize; let deviation = (bs.ns_iter_summ.max - bs.ns_iter_summ.min) as usize; - output - .write_fmt(format_args!( - "{:>11} ns/iter (+/- {})", - fmt_thousands_sep(median, ','), - fmt_thousands_sep(deviation, ',') - )) - .unwrap(); + write!( + output, + "{:>11} ns/iter (+/- {})", + fmt_thousands_sep(median, ','), + fmt_thousands_sep(deviation, ',') + ) + .unwrap(); if bs.mb_s != 0 { - output.write_fmt(format_args!(" = {} MB/s", bs.mb_s)).unwrap(); + write!(output, " = {} MB/s", bs.mb_s).unwrap(); } output } @@ -83,9 +83,9 @@ fn fmt_thousands_sep(mut n: usize, sep: char) -> String { let base = 10_usize.pow(pow); if pow == 0 || trailing || n / base != 0 { if !trailing { - output.write_fmt(format_args!("{}", n / base)).unwrap(); + write!(output, "{}", n / base).unwrap(); } else { - output.write_fmt(format_args!("{:03}", n / base)).unwrap(); + write!(output, "{:03}", n / base).unwrap(); } if pow != 0 { output.push(sep); diff --git a/src/tools/unstable-book-gen/src/main.rs b/src/tools/unstable-book-gen/src/main.rs index 387b2acd1069e..e10f72a47b2c4 100644 --- a/src/tools/unstable-book-gen/src/main.rs +++ b/src/tools/unstable-book-gen/src/main.rs @@ -27,12 +27,12 @@ macro_rules! t { fn generate_stub_issue(path: &Path, name: &str, issue: u32) { let mut file = t!(File::create(path)); - t!(file.write_fmt(format_args!(include_str!("stub-issue.md"), name = name, issue = issue))); + t!(write!(file, include_str!("stub-issue.md"), name = name, issue = issue)); } fn generate_stub_no_issue(path: &Path, name: &str) { let mut file = t!(File::create(path)); - t!(file.write_fmt(format_args!(include_str!("stub-no-issue.md"), name = name))); + t!(write!(file, include_str!("stub-no-issue.md"), name = name)); } fn set_to_summary_str(set: &BTreeSet, dir: &str) -> String { From 0e56b5231947c794d891f52abdcd86f31fccf63a Mon Sep 17 00:00:00 2001 From: Alexis Bourget Date: Sun, 20 Sep 2020 17:31:55 +0200 Subject: [PATCH 39/44] Fix accordingly to review --- library/std/src/thread/local.rs | 42 ++++++++++++++++----------------- 1 file changed, 20 insertions(+), 22 deletions(-) diff --git a/library/std/src/thread/local.rs b/library/std/src/thread/local.rs index 4ac1e523a97ea..cba3f1b23fe45 100644 --- a/library/std/src/thread/local.rs +++ b/library/std/src/thread/local.rs @@ -284,13 +284,15 @@ mod lazy { } pub unsafe fn get(&self) -> Option<&'static T> { - // SAFETY: No reference is ever handed out to the inner cell nor - // mutable reference to the Option inside said cell. This make it - // safe to hand a reference, though the lifetime of 'static - // is itself unsafe, making the get method unsafe. + // SAFETY: The caller must ensure no reference is ever handed out to + // the inner cell nor mutable reference to the Option inside said + // cell. This make it safe to hand a reference, though the lifetime + // of 'static is itself unsafe, making the get method unsafe. unsafe { (*self.inner.get()).as_ref() } } + /// The caller must ensure that no reference is active: this method + /// needs unique access. pub unsafe fn initialize T>(&self, init: F) -> &'static T { // Execute the initialization up front, *then* move it into our slot, // just in case initialization fails. @@ -312,20 +314,13 @@ mod lazy { // destructor" flag we just use `mem::replace` which should sequence the // operations a little differently and make this safe to call. // - // `ptr` can be dereferenced safely since it was obtained from - // `UnsafeCell::get`, which should not return a non-aligned or NUL pointer. - // What's more a `LazyKeyInner` can only be created with `new`, which ensures - // `inner` is correctly initialized and all calls to methods on `LazyKeyInner` - // will leave `inner` initialized too. + // The precondition also ensures that we are the only one accessing + // `self` at the moment so replacing is fine. unsafe { let _ = mem::replace(&mut *ptr, Some(value)); } - // SAFETY: the `*ptr` operation is made safe by the `mem::replace` - // call above combined with `ptr` being correct from the beginning - // (see previous SAFETY: comment above). - // - // Plus, with the call to `mem::replace` it is guaranteed there is + // SAFETY: With the call to `mem::replace` it is guaranteed there is // a `Some` behind `ptr`, not a `None` so `unreachable_unchecked` // will never be reached. unsafe { @@ -341,11 +336,12 @@ mod lazy { } } + /// The other methods hand out references while taking &self. + /// As such, callers of this method must ensure no `&` and `&mut` are + /// available and used at the same time. #[allow(unused)] pub unsafe fn take(&mut self) -> Option { - // SAFETY: The other methods hand out references while taking &self. - // As such, callers of this method must ensure no `&` and `&mut` are - // available and used at the same time. + // SAFETY: See doc comment for this method. unsafe { (*self.inner.get()).take() } } } @@ -438,9 +434,10 @@ pub mod fast { // SAFETY: See the definitions of `LazyKeyInner::get` and // `try_initialize` for more informations. // - // The call to `get` is made safe because no mutable references are - // ever handed out and the `try_initialize` is dependant on the - // passed `init` function. + // The caller must ensure no mutable references are ever active to + // the inner cell or the inner T when this is called. + // The `try_initialize` is dependant on the passed `init` function + // for this. unsafe { match self.inner.get() { Some(val) => Some(val), @@ -546,9 +543,10 @@ pub mod os { Key { os: OsStaticKey::new(Some(destroy_value::)), marker: marker::PhantomData } } + /// It is a requirement for the caller to ensure that no mutable + /// reference is active when this method is called. pub unsafe fn get(&'static self, init: fn() -> T) -> Option<&'static T> { - // SAFETY: No mutable references are ever handed out meaning getting - // the value is ok. + // SAFETY: See the documentation for this method. let ptr = unsafe { self.os.get() as *mut Value }; if ptr as usize > 1 { // SAFETY: the check ensured the pointer is safe (its destructor From 08b85a6fc88b4c7b5e02742b5df825b62e168f81 Mon Sep 17 00:00:00 2001 From: Alexis Bourget Date: Sun, 20 Sep 2020 18:04:12 +0200 Subject: [PATCH 40/44] use iter:: before free functions --- library/core/src/iter/sources.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/library/core/src/iter/sources.rs b/library/core/src/iter/sources.rs index c28538ef027c0..97562cf73b869 100644 --- a/library/core/src/iter/sources.rs +++ b/library/core/src/iter/sources.rs @@ -531,8 +531,10 @@ where /// An iterator where each iteration calls the provided closure `F: FnMut() -> Option`. /// -/// This `struct` is created by the [`from_fn()`] function. +/// This `struct` is created by the [`iter::from_fn()`] function. /// See its documentation for more. +/// +/// [`iter::from_fn()`]: from_fn #[derive(Clone)] #[stable(feature = "iter_from_fn", since = "1.34.0")] pub struct FromFn(F); @@ -581,8 +583,10 @@ where /// An new iterator where each successive item is computed based on the preceding one. /// -/// This `struct` is created by the [`successors()`] function. +/// This `struct` is created by the [`iter::successors()`] function. /// See its documentation for more. +/// +/// [`iter::successors()`]: successors #[derive(Clone)] #[stable(feature = "iter_successors", since = "1.34.0")] pub struct Successors { From 81699895073162cd6731413711a4357dde67d661 Mon Sep 17 00:00:00 2001 From: Daniel Henry-Mantilla Date: Sat, 19 Sep 2020 21:32:33 +0200 Subject: [PATCH 41/44] Add non-`unsafe` `.get_mut()` for `UnsafeCell` Update the tracking issue number Updated the documentation for `UnsafeCell` Address review comments Address more review comments + minor changes --- library/core/src/cell.rs | 93 ++++++++++++++++++++++++++++++++++++---- 1 file changed, 84 insertions(+), 9 deletions(-) diff --git a/library/core/src/cell.rs b/library/core/src/cell.rs index cbbfcb4611321..44b863b220051 100644 --- a/library/core/src/cell.rs +++ b/library/core/src/cell.rs @@ -1543,8 +1543,11 @@ impl fmt::Display for RefMut<'_, T> { /// allow internal mutability, such as `Cell` and `RefCell`, use `UnsafeCell` to wrap their /// internal data. There is *no* legal way to obtain aliasing `&mut`, not even with `UnsafeCell`. /// -/// The `UnsafeCell` API itself is technically very simple: it gives you a raw pointer `*mut T` to -/// its contents. It is up to _you_ as the abstraction designer to use that raw pointer correctly. +/// The `UnsafeCell` API itself is technically very simple: [`.get()`] gives you a raw pointer +/// `*mut T` to its contents. It is up to _you_ as the abstraction designer to use that raw pointer +/// correctly. +/// +/// [`.get()`]: `UnsafeCell::get` /// /// The precise Rust aliasing rules are somewhat in flux, but the main points are not contentious: /// @@ -1571,21 +1574,70 @@ impl fmt::Display for RefMut<'_, T> { /// 2. A `&mut T` reference may be released to safe code provided neither other `&mut T` nor `&T` /// co-exist with it. A `&mut T` must always be unique. /// -/// Note that while mutating or mutably aliasing the contents of an `&UnsafeCell` is -/// ok (provided you enforce the invariants some other way), it is still undefined behavior -/// to have multiple `&mut UnsafeCell` aliases. +/// Note that whilst mutating the contents of an `&UnsafeCell` (even while other +/// `&UnsafeCell` references alias the cell) is +/// ok (provided you enforce the above invariants some other way), it is still undefined behavior +/// to have multiple `&mut UnsafeCell` aliases. That is, `UnsafeCell` is a wrapper +/// designed to have a special interaction with _shared_ accesses (_i.e._, through an +/// `&UnsafeCell<_>` reference); there is no magic whatsoever when dealing with _exclusive_ +/// accesses (_e.g._, through an `&mut UnsafeCell<_>`): neither the cell nor the wrapped value +/// may be aliased for the duration of that `&mut` borrow. +/// This is showcased by the [`.get_mut()`] accessor, which is a non-`unsafe` getter that yields +/// a `&mut T`. +/// +/// [`.get_mut()`]: `UnsafeCell::get_mut` /// /// # Examples /// +/// Here is an example showcasing how to soundly mutate the contents of an `UnsafeCell<_>` despite +/// there being multiple references aliasing the cell: +/// /// ``` /// use std::cell::UnsafeCell; /// -/// # #[allow(dead_code)] -/// struct NotThreadSafe { -/// value: UnsafeCell, +/// let x: UnsafeCell = 42.into(); +/// // Get multiple / concurrent / shared references to the same `x`. +/// let (p1, p2): (&UnsafeCell, &UnsafeCell) = (&x, &x); +/// +/// unsafe { +/// // SAFETY: within this scope there are no other references to `x`'s contents, +/// // so ours is effectively unique. +/// let p1_exclusive: &mut i32 = &mut *p1.get(); // -- borrow --+ +/// *p1_exclusive += 27; // | +/// } // <---------- cannot go beyond this point -------------------+ +/// +/// unsafe { +/// // SAFETY: within this scope nobody expects to have exclusive access to `x`'s contents, +/// // so we can have multiple shared accesses concurrently. +/// let p2_shared: &i32 = &*p2.get(); +/// assert_eq!(*p2_shared, 42 + 27); +/// let p1_shared: &i32 = &*p1.get(); +/// assert_eq!(*p1_shared, *p2_shared); /// } +/// ``` +/// +/// The following example showcases the fact that exclusive access to an `UnsafeCell` +/// implies exclusive access to its `T`: /// -/// unsafe impl Sync for NotThreadSafe {} +/// ```rust +/// #![feature(unsafe_cell_get_mut)] +/// #![forbid(unsafe_code)] // with exclusive accesses, +/// // `UnsafeCell` is a transparent no-op wrapper, +/// // so no need for `unsafe` here. +/// use std::cell::UnsafeCell; +/// +/// let mut x: UnsafeCell = 42.into(); +/// +/// // Get a compile-time-checked unique reference to `x`. +/// let p_unique: &mut UnsafeCell = &mut x; +/// // With an exclusive reference, we can mutate the contents for free. +/// *p_unique.get_mut() = 0; +/// // Or, equivalently: +/// x = UnsafeCell::new(0); +/// +/// // When we own the value, we can extract the contents for free. +/// let contents: i32 = x.into_inner(); +/// assert_eq!(contents, 0); /// ``` #[lang = "unsafe_cell"] #[stable(feature = "rust1", since = "1.0.0")] @@ -1663,6 +1715,29 @@ impl UnsafeCell { self as *const UnsafeCell as *const T as *mut T } + /// Returns a mutable reference to the underlying data. + /// + /// This call borrows the `UnsafeCell` mutably (at compile-time) which + /// guarantees that we possess the only reference. + /// + /// # Examples + /// + /// ``` + /// #![feature(unsafe_cell_get_mut)] + /// use std::cell::UnsafeCell; + /// + /// let mut c = UnsafeCell::new(5); + /// *c.get_mut() += 1; + /// + /// assert_eq!(*c.get_mut(), 6); + /// ``` + #[inline] + #[unstable(feature = "unsafe_cell_get_mut", issue = "76943")] + pub fn get_mut(&mut self) -> &mut T { + // SAFETY: (outer) `&mut` guarantees unique access. + unsafe { &mut *self.get() } + } + /// Gets a mutable pointer to the wrapped value. /// The difference to [`get`] is that this function accepts a raw pointer, /// which is useful to avoid the creation of temporary references. From 5886c38112c8bb347b1cbd46c28b1ca6f8bac88d Mon Sep 17 00:00:00 2001 From: Daniel Henry-Mantilla Date: Sat, 19 Sep 2020 21:33:40 +0200 Subject: [PATCH 42/44] Replace unneeded `unsafe` calls to `.get()` with calls to `.get_mut()` --- library/core/src/cell.rs | 8 ++------ library/core/src/sync/atomic.rs | 6 ++---- library/std/src/lib.rs | 1 + library/std/src/sync/mutex.rs | 4 +--- library/std/src/sync/rwlock.rs | 4 +--- 5 files changed, 7 insertions(+), 16 deletions(-) diff --git a/library/core/src/cell.rs b/library/core/src/cell.rs index 44b863b220051..f60aa2d24c5ca 100644 --- a/library/core/src/cell.rs +++ b/library/core/src/cell.rs @@ -496,10 +496,7 @@ impl Cell { #[inline] #[stable(feature = "cell_get_mut", since = "1.11.0")] pub fn get_mut(&mut self) -> &mut T { - // SAFETY: This can cause data races if called from a separate thread, - // but `Cell` is `!Sync` so this won't happen, and `&mut` guarantees - // unique access. - unsafe { &mut *self.value.get() } + self.value.get_mut() } /// Returns a `&Cell` from a `&mut T` @@ -945,8 +942,7 @@ impl RefCell { #[inline] #[stable(feature = "cell_get_mut", since = "1.11.0")] pub fn get_mut(&mut self) -> &mut T { - // SAFETY: `&mut` guarantees unique access. - unsafe { &mut *self.value.get() } + self.value.get_mut() } /// Undo the effect of leaked guards on the borrow state of the `RefCell`. diff --git a/library/core/src/sync/atomic.rs b/library/core/src/sync/atomic.rs index 9d74f537491b1..c67d6422c01ec 100644 --- a/library/core/src/sync/atomic.rs +++ b/library/core/src/sync/atomic.rs @@ -838,8 +838,7 @@ impl AtomicPtr { #[inline] #[stable(feature = "atomic_access", since = "1.15.0")] pub fn get_mut(&mut self) -> &mut *mut T { - // SAFETY: the mutable reference guarantees unique ownership. - unsafe { &mut *self.p.get() } + self.p.get_mut() } /// Get atomic access to a pointer. @@ -1275,8 +1274,7 @@ assert_eq!(some_var.load(Ordering::SeqCst), 5); #[inline] #[$stable_access] pub fn get_mut(&mut self) -> &mut $int_type { - // SAFETY: the mutable reference guarantees unique ownership. - unsafe { &mut *self.v.get() } + self.v.get_mut() } } diff --git a/library/std/src/lib.rs b/library/std/src/lib.rs index 5333d75ec1bc5..71b29cf5af99b 100644 --- a/library/std/src/lib.rs +++ b/library/std/src/lib.rs @@ -315,6 +315,7 @@ #![feature(try_reserve)] #![feature(unboxed_closures)] #![feature(unsafe_block_in_unsafe_fn)] +#![feature(unsafe_cell_get_mut)] #![feature(unsafe_cell_raw_get)] #![feature(untagged_unions)] #![feature(unwind_attributes)] diff --git a/library/std/src/sync/mutex.rs b/library/std/src/sync/mutex.rs index 240155b06b411..a1703c731d44d 100644 --- a/library/std/src/sync/mutex.rs +++ b/library/std/src/sync/mutex.rs @@ -406,9 +406,7 @@ impl Mutex { /// ``` #[stable(feature = "mutex_get_mut", since = "1.6.0")] pub fn get_mut(&mut self) -> LockResult<&mut T> { - // We know statically that there are no other references to `self`, so - // there's no need to lock the inner mutex. - let data = unsafe { &mut *self.data.get() }; + let data = self.data.get_mut(); poison::map_result(self.poison.borrow(), |_| data) } } diff --git a/library/std/src/sync/rwlock.rs b/library/std/src/sync/rwlock.rs index f38d6101da0d3..d967779ce361d 100644 --- a/library/std/src/sync/rwlock.rs +++ b/library/std/src/sync/rwlock.rs @@ -404,9 +404,7 @@ impl RwLock { /// ``` #[stable(feature = "rwlock_get_mut", since = "1.6.0")] pub fn get_mut(&mut self) -> LockResult<&mut T> { - // We know statically that there are no other references to `self`, so - // there's no need to lock the inner lock. - let data = unsafe { &mut *self.data.get() }; + let data = self.data.get_mut(); poison::map_result(self.poison.borrow(), |_| data) } } From aaddcdb0d097de1fee14be16479aeaeea41e8810 Mon Sep 17 00:00:00 2001 From: Alexis Bourget Date: Sun, 20 Sep 2020 18:37:05 +0200 Subject: [PATCH 43/44] Fix nits --- library/std/src/sync/barrier.rs | 4 ++-- library/std/src/sync/once.rs | 3 +-- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/library/std/src/sync/barrier.rs b/library/std/src/sync/barrier.rs index 8d3e30dbd4545..eab26b6c7150c 100644 --- a/library/std/src/sync/barrier.rs +++ b/library/std/src/sync/barrier.rs @@ -150,8 +150,8 @@ impl fmt::Debug for BarrierWaitResult { } impl BarrierWaitResult { - /// Returns `true` if this thread from [`Barrier::wait()`] is the - /// "leader thread". + /// Returns `true` if this thread is the "leader thread" for the call to + /// [`Barrier::wait()`]. /// /// Only one thread will have `true` returned from their result, all other /// threads will have `false` returned. diff --git a/library/std/src/sync/once.rs b/library/std/src/sync/once.rs index ee8902bf764bf..de5ddf1daf27b 100644 --- a/library/std/src/sync/once.rs +++ b/library/std/src/sync/once.rs @@ -95,8 +95,7 @@ use crate::thread::{self, Thread}; /// A synchronization primitive which can be used to run a one-time global /// initialization. Useful for one-time initialization for FFI or related -/// functionality. This type can only be constructed with the [`Once::new()`] -/// constructor. +/// functionality. This type can only be constructed with [`Once::new()`]. /// /// # Examples /// From c9c8fb88cf1be7e0a6bd6fd049d8d28fb5d86135 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Sat, 12 Sep 2020 00:42:52 -0400 Subject: [PATCH 44/44] Add sample defaults for config.toml - Allow including defaults in `src/bootstrap/defaults` using `profile = "..."` - Add default config files - Combine config files using the merge dependency. - Add comments to default config files - Add a README asking to open an issue if the defaults are bad - Give a loud error if trying to merge `.target`, since it's not currently supported - Use an exhaustive match - Use `` in config.toml.example to avoid confusion - Fix bugs in `Merge` derives Previously, it would completely ignore the profile defaults if there were any settings in `config.toml`. I sent an email to the `merge` maintainer asking them to make the behavior in this commit the default. This introduces a new dependency on `merge` that hasn't yet been vetted. I want to improve the output when `include = "x"` isn't found: ``` thread 'main' panicked at 'fs::read_to_string(&file) failed with No such file or directory (os error 2) ("configuration file did not exist")', src/bootstrap/config.rs:522:28 note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace failed to run: /home/joshua/rustc/build/bootstrap/debug/bootstrap test tidy Build completed unsuccessfully in 0:00:00 ``` However that seems like it could be fixed in a follow-up. --- Cargo.lock | 23 +++++++ config.toml.example | 10 +++ src/bootstrap/Cargo.toml | 1 + src/bootstrap/config.rs | 76 ++++++++++++++------- src/bootstrap/defaults/README.md | 11 +++ src/bootstrap/defaults/config.toml.codegen | 13 ++++ src/bootstrap/defaults/config.toml.compiler | 8 +++ src/bootstrap/defaults/config.toml.library | 10 +++ src/bootstrap/defaults/config.toml.user | 9 +++ 9 files changed, 136 insertions(+), 25 deletions(-) create mode 100644 src/bootstrap/defaults/README.md create mode 100644 src/bootstrap/defaults/config.toml.codegen create mode 100644 src/bootstrap/defaults/config.toml.compiler create mode 100644 src/bootstrap/defaults/config.toml.library create mode 100644 src/bootstrap/defaults/config.toml.user diff --git a/Cargo.lock b/Cargo.lock index d3f777bc663dd..4c55fea30e0e4 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -207,6 +207,7 @@ dependencies = [ "ignore", "lazy_static", "libc", + "merge", "num_cpus", "opener", "pretty_assertions", @@ -1900,6 +1901,28 @@ dependencies = [ "autocfg", ] +[[package]] +name = "merge" +version = "0.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "10bbef93abb1da61525bbc45eeaff6473a41907d19f8f9aa5168d214e10693e9" +dependencies = [ + "merge_derive", + "num-traits", +] + +[[package]] +name = "merge_derive" +version = "0.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "209d075476da2e63b4b29e72a2ef627b840589588e71400a25e3565c4f849d07" +dependencies = [ + "proc-macro-error", + "proc-macro2", + "quote", + "syn", +] + [[package]] name = "minifier" version = "0.0.33" diff --git a/config.toml.example b/config.toml.example index 99e6f9dceb41c..8bf1b48ce830e 100644 --- a/config.toml.example +++ b/config.toml.example @@ -9,6 +9,16 @@ # a custom configuration file can also be specified with `--config` to the build # system. +# ============================================================================= +# Global Settings +# ============================================================================= + +# Use different pre-set defaults than the global defaults. +# +# See `src/bootstrap/defaults` for more information. +# Note that this has no default value (x.py uses the defaults in `config.toml.example`). +#profile = + # ============================================================================= # Tweaking how LLVM is compiled # ============================================================================= diff --git a/src/bootstrap/Cargo.toml b/src/bootstrap/Cargo.toml index faec2c53742ec..0177a9dab97e3 100644 --- a/src/bootstrap/Cargo.toml +++ b/src/bootstrap/Cargo.toml @@ -48,6 +48,7 @@ lazy_static = "1.3.0" time = "0.1" ignore = "0.4.10" opener = "0.4" +merge = "0.1.0" [target.'cfg(windows)'.dependencies.winapi] version = "0.3" diff --git a/src/bootstrap/config.rs b/src/bootstrap/config.rs index 7e2cb7721865e..d925af19a8426 100644 --- a/src/bootstrap/config.rs +++ b/src/bootstrap/config.rs @@ -16,6 +16,7 @@ use crate::flags::Flags; pub use crate::flags::Subcommand; use crate::util::exe; use build_helper::t; +use merge::Merge; use serde::Deserialize; macro_rules! check_ci_llvm { @@ -278,10 +279,31 @@ struct TomlConfig { rust: Option, target: Option>, dist: Option, + profile: Option, +} + +impl Merge for TomlConfig { + fn merge(&mut self, TomlConfig { build, install, llvm, rust, dist, target, profile: _ }: Self) { + fn do_merge(x: &mut Option, y: Option) { + if let Some(new) = y { + if let Some(original) = x { + original.merge(new); + } else { + *x = Some(new); + } + } + }; + do_merge(&mut self.build, build); + do_merge(&mut self.install, install); + do_merge(&mut self.llvm, llvm); + do_merge(&mut self.rust, rust); + do_merge(&mut self.dist, dist); + assert!(target.is_none(), "merging target-specific config is not currently supported"); + } } /// TOML representation of various global build decisions. -#[derive(Deserialize, Default, Clone)] +#[derive(Deserialize, Default, Clone, Merge)] #[serde(deny_unknown_fields, rename_all = "kebab-case")] struct Build { build: Option, @@ -321,7 +343,7 @@ struct Build { } /// TOML representation of various global install decisions. -#[derive(Deserialize, Default, Clone)] +#[derive(Deserialize, Default, Clone, Merge)] #[serde(deny_unknown_fields, rename_all = "kebab-case")] struct Install { prefix: Option, @@ -338,7 +360,7 @@ struct Install { } /// TOML representation of how the LLVM build is configured. -#[derive(Deserialize, Default)] +#[derive(Deserialize, Default, Merge)] #[serde(deny_unknown_fields, rename_all = "kebab-case")] struct Llvm { skip_rebuild: Option, @@ -365,7 +387,7 @@ struct Llvm { download_ci_llvm: Option, } -#[derive(Deserialize, Default, Clone)] +#[derive(Deserialize, Default, Clone, Merge)] #[serde(deny_unknown_fields, rename_all = "kebab-case")] struct Dist { sign_folder: Option, @@ -389,7 +411,7 @@ impl Default for StringOrBool { } /// TOML representation of how the Rust build is configured. -#[derive(Deserialize, Default)] +#[derive(Deserialize, Default, Merge)] #[serde(deny_unknown_fields, rename_all = "kebab-case")] struct Rust { optimize: Option, @@ -434,7 +456,7 @@ struct Rust { } /// TOML representation of how each build target is configured. -#[derive(Deserialize, Default)] +#[derive(Deserialize, Default, Merge)] #[serde(deny_unknown_fields, rename_all = "kebab-case")] struct TomlTarget { cc: Option, @@ -523,27 +545,31 @@ impl Config { } #[cfg(test)] - let toml = TomlConfig::default(); + let get_toml = |_| TomlConfig::default(); #[cfg(not(test))] - let toml = flags - .config - .map(|file| { - use std::process; - - let contents = t!(fs::read_to_string(&file)); - match toml::from_str(&contents) { - Ok(table) => table, - Err(err) => { - println!( - "failed to parse TOML configuration '{}': {}", - file.display(), - err - ); - process::exit(2); - } + let get_toml = |file: PathBuf| { + use std::process; + + let contents = t!(fs::read_to_string(&file), "configuration file did not exist"); + match toml::from_str(&contents) { + Ok(table) => table, + Err(err) => { + println!("failed to parse TOML configuration '{}': {}", file.display(), err); + process::exit(2); } - }) - .unwrap_or_else(TomlConfig::default); + } + }; + + let mut toml = flags.config.map(get_toml).unwrap_or_else(TomlConfig::default); + if let Some(include) = &toml.profile { + let mut include_path = config.src.clone(); + include_path.push("src"); + include_path.push("bootstrap"); + include_path.push("defaults"); + include_path.push(format!("config.toml.{}", include)); + let included_toml = get_toml(include_path); + toml.merge(included_toml); + } let build = toml.build.unwrap_or_default(); diff --git a/src/bootstrap/defaults/README.md b/src/bootstrap/defaults/README.md new file mode 100644 index 0000000000000..a91fc3538eb55 --- /dev/null +++ b/src/bootstrap/defaults/README.md @@ -0,0 +1,11 @@ +# About bootstrap defaults + +These defaults are intended to be a good starting point for working with x.py, +with the understanding that no one set of defaults make sense for everyone. + +They are still experimental, and we'd appreciate your help improving them! +If you use a setting that's not in these defaults that you think others would benefit from, please [file an issue] or make a PR with the changes. +Similarly, if one of these defaults doesn't match what you use personally, +please open an issue to get it changed. + +[file an issue]: https://github.com/rust-lang/rust/issues/new/choose diff --git a/src/bootstrap/defaults/config.toml.codegen b/src/bootstrap/defaults/config.toml.codegen new file mode 100644 index 0000000000000..a9505922ca7fc --- /dev/null +++ b/src/bootstrap/defaults/config.toml.codegen @@ -0,0 +1,13 @@ +# These defaults are meant for contributors to the compiler who modify codegen or LLVM +[llvm] +# This enables debug-assertions in LLVM, +# catching logic errors in codegen much earlier in the process. +assertions = true + +[rust] +# This enables `RUSTC_LOG=debug`, avoiding confusing situations +# where adding `debug!()` appears to do nothing. +# However, it makes running the compiler slightly slower. +debug-logging = true +# This greatly increases the speed of rebuilds, especially when there are only minor changes. However, it makes the initial build slightly slower. +incremental = true diff --git a/src/bootstrap/defaults/config.toml.compiler b/src/bootstrap/defaults/config.toml.compiler new file mode 100644 index 0000000000000..4772de8a2cb22 --- /dev/null +++ b/src/bootstrap/defaults/config.toml.compiler @@ -0,0 +1,8 @@ +# These defaults are meant for contributors to the compiler who do not modify codegen or LLVM +[rust] +# This enables `RUSTC_LOG=debug`, avoiding confusing situations +# where adding `debug!()` appears to do nothing. +# However, it makes running the compiler slightly slower. +debug-logging = true +# This greatly increases the speed of rebuilds, especially when there are only minor changes. However, it makes the initial build slightly slower. +incremental = true diff --git a/src/bootstrap/defaults/config.toml.library b/src/bootstrap/defaults/config.toml.library new file mode 100644 index 0000000000000..e4316f4d86440 --- /dev/null +++ b/src/bootstrap/defaults/config.toml.library @@ -0,0 +1,10 @@ +# These defaults are meant for contributors to the standard library and documentation. +[build] +# When building the standard library, you almost never want to build the compiler itself. +build-stage = 0 +test-stage = 0 +bench-stage = 0 + +[rust] +# This greatly increases the speed of rebuilds, especially when there are only minor changes. However, it makes the initial build slightly slower. +incremental = true diff --git a/src/bootstrap/defaults/config.toml.user b/src/bootstrap/defaults/config.toml.user new file mode 100644 index 0000000000000..6647061d88fcb --- /dev/null +++ b/src/bootstrap/defaults/config.toml.user @@ -0,0 +1,9 @@ +# These defaults are meant for users and distro maintainers building from source, without intending to make multiple changes. +[build] +# When compiling from source, you almost always want a full stage 2 build, +# which has all the latest optimizations from nightly. +build-stage = 2 +test-stage = 2 +doc-stage = 2 +# When compiling from source, you usually want all tools. +extended = true