From 3615cb476b30d4058f397f45016e9b1823b4c870 Mon Sep 17 00:00:00 2001 From: Lukas Lueg Date: Sun, 17 Apr 2022 20:58:36 +0200 Subject: [PATCH 1/6] Expand core::hint::unreachable_unchecked() docs Fixes #95865 --- library/core/src/hint.rs | 72 ++++++++++++++++++++++++++++++++-------- 1 file changed, 58 insertions(+), 14 deletions(-) diff --git a/library/core/src/hint.rs b/library/core/src/hint.rs index 58ea109c735b6..2473fbd18661a 100644 --- a/library/core/src/hint.rs +++ b/library/core/src/hint.rs @@ -5,27 +5,71 @@ use crate::intrinsics; -/// Informs the compiler that this point in the code is not reachable, enabling -/// further optimizations. +/// Informs the compiler that the site which is calling this function is not +/// reachable, possibly enabling further optimizations. /// /// # Safety /// -/// Reaching this function is completely *undefined behavior* (UB). In -/// particular, the compiler assumes that all UB must never happen, and -/// therefore will eliminate all branches that reach to a call to +/// Reaching this function is *Undefined Behavior* (UB). In particular, as the +/// compiler assumes that all forms of Undefined Behavior can never happen, it +/// will eliminate all branches which themselves reach a call to /// `unreachable_unchecked()`. /// -/// Like all instances of UB, if this assumption turns out to be wrong, i.e., the -/// `unreachable_unchecked()` call is actually reachable among all possible -/// control flow, the compiler will apply the wrong optimization strategy, and -/// may sometimes even corrupt seemingly unrelated code, causing -/// difficult-to-debug problems. +/// If the assumptions embedded in using this function turn out to be wrong - +/// that is, if the site which is calling `unreachable_unchecked()` is actually +/// reachable at runtime - the compiler may have generated nonsensical machine +/// instructions for this situation, including in seemingly unrelated code, +/// causing difficult-to-debug problems. /// -/// Use this function only when you can prove that the code will never call it. -/// Otherwise, consider using the [`unreachable!`] macro, which does not allow -/// optimizations but will panic when executed. +/// Use this function sparingly. Consider using the [`unreachable!`] macro, +/// which may prevent some optimizations but will safely panic in case it is +/// actually reached at runtime. Benchmark your code to find out if using +/// `unreachable_unchecked()` comes with a performance benefit. /// -/// # Example +/// # Examples +/// +/// `unreachable_unchecked()` can be used in situations where the compiler +/// can't prove invariants that were previously established. Such situations +/// have a higher chance of occuring if those invariants are upheld by +/// external code that the compiler can't analyze. +/// ``` +/// fn prepare_inputs(divisors: &mut Vec) { +/// // Note to future-self when making changes: The invariant established +/// // here is NOT checked in `do_computation()`; if this changes, you HAVE +/// // to change `do_computation()`. +/// divisors.retain(|divisor| *divisor != 0) +/// } +/// +/// fn do_computation(i: u32, divisors: &[u32]) -> u32 { +/// divisors.iter().fold(i, |acc, divisor| { +/// // Convince the compiler that a division by zero can't happen here +/// // and a check is not needed below. +/// if *divisor == 0 { +/// // SAFETY: `divisor` can't be zero because of `prepare_inputs`, +/// // but the compiler does not know about this. We *promise* +/// // that we always call `prepare_inputs`. +/// unsafe { std::hint::unreachable_unchecked() } +/// } +/// // The compiler would normally introduce a check here that prevents +/// // a division by zero. However, if `divisor` was zero, the branch +/// // above would reach what we explicitly marked as unreachable. +/// // The compiler concludes that `divisor` can't be zero at this point +/// // and removes the - now proven useless - check. +/// acc / divisor +/// }) +/// } +/// +/// let mut divisors = vec![2, 0, 4]; +/// prepare_inputs(&mut divisors); +/// assert_eq!(do_computation(100, &divisors), 12); +/// +/// ``` +/// +/// While using `unreachable_unchecked()` is perfectly safe in the following +/// example, the compiler is able to prove that a division by zero is not +/// possible. Benchmarking reveals that `unreachable_unchecked()` provides +/// no benefit over using [`unreachable!`], while the latter does not introduce +/// the possibility of Undefined Behavior. /// /// ``` /// fn div_1(a: u32, b: u32) -> u32 { From 5db03162f0ea4bc9b2a0f23d57e87b641e807e02 Mon Sep 17 00:00:00 2001 From: Yuki Okushi Date: Mon, 2 May 2022 06:51:02 +0900 Subject: [PATCH 2/6] Add a regression test for #54779 --- .../nll/issue-54779-anon-static-lifetime.rs | 51 +++++++++++++++++++ .../issue-54779-anon-static-lifetime.stderr | 11 ++++ 2 files changed, 62 insertions(+) create mode 100644 src/test/ui/nll/issue-54779-anon-static-lifetime.rs create mode 100644 src/test/ui/nll/issue-54779-anon-static-lifetime.stderr diff --git a/src/test/ui/nll/issue-54779-anon-static-lifetime.rs b/src/test/ui/nll/issue-54779-anon-static-lifetime.rs new file mode 100644 index 0000000000000..4bb983dd3581d --- /dev/null +++ b/src/test/ui/nll/issue-54779-anon-static-lifetime.rs @@ -0,0 +1,51 @@ +// Regression test for #54779, checks if the diagnostics are confusing. + +#![feature(nll)] + +trait DebugWith { + fn debug_with<'me>(&'me self, cx: &'me Cx) -> DebugCxPair<'me, Self, Cx> { + DebugCxPair { value: self, cx } + } + + fn fmt_with(&self, cx: &Cx, fmt: &mut std::fmt::Formatter<'_>) -> std::fmt::Result; +} + +struct DebugCxPair<'me, Value: ?Sized, Cx: ?Sized> +where + Value: DebugWith, +{ + value: &'me Value, + cx: &'me Cx, +} + +trait DebugContext {} + +struct Foo { + bar: Bar, +} + +impl DebugWith for Foo { + fn fmt_with( + &self, + cx: &dyn DebugContext, + fmt: &mut std::fmt::Formatter<'_>, + ) -> std::fmt::Result { + let Foo { bar } = self; + bar.debug_with(cx); //~ ERROR: lifetime may not live long enough + Ok(()) + } +} + +struct Bar {} + +impl DebugWith for Bar { + fn fmt_with( + &self, + cx: &dyn DebugContext, + fmt: &mut std::fmt::Formatter<'_>, + ) -> std::fmt::Result { + Ok(()) + } +} + +fn main() {} diff --git a/src/test/ui/nll/issue-54779-anon-static-lifetime.stderr b/src/test/ui/nll/issue-54779-anon-static-lifetime.stderr new file mode 100644 index 0000000000000..9dc9bdbab8d09 --- /dev/null +++ b/src/test/ui/nll/issue-54779-anon-static-lifetime.stderr @@ -0,0 +1,11 @@ +error: lifetime may not live long enough + --> $DIR/issue-54779-anon-static-lifetime.rs:34:24 + | +LL | cx: &dyn DebugContext, + | - let's call the lifetime of this reference `'1` +... +LL | bar.debug_with(cx); + | ^^ cast requires that `'1` must outlive `'static` + +error: aborting due to previous error + From cd1746b2b4275a3437581992e2cb38f104b74b6c Mon Sep 17 00:00:00 2001 From: Lukas Lueg Date: Fri, 6 May 2022 09:34:41 +0200 Subject: [PATCH 3/6] Clarify unreachable_unchecked docs --- library/core/src/hint.rs | 25 ++++++++++++++++--------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/library/core/src/hint.rs b/library/core/src/hint.rs index 2473fbd18661a..7ae1bfd4f351a 100644 --- a/library/core/src/hint.rs +++ b/library/core/src/hint.rs @@ -10,10 +10,11 @@ use crate::intrinsics; /// /// # Safety /// -/// Reaching this function is *Undefined Behavior* (UB). In particular, as the -/// compiler assumes that all forms of Undefined Behavior can never happen, it -/// will eliminate all branches which themselves reach a call to -/// `unreachable_unchecked()`. +/// Reaching this function is *Undefined Behavior*. +/// +/// As the compiler assumes that all forms of Undefined Behavior can never +/// happen, it will eliminate all branches in the surrounding code that it can +/// determine will invariably lead to a call to `unreachable_unchecked()`. /// /// If the assumptions embedded in using this function turn out to be wrong - /// that is, if the site which is calling `unreachable_unchecked()` is actually @@ -40,15 +41,17 @@ use crate::intrinsics; /// divisors.retain(|divisor| *divisor != 0) /// } /// -/// fn do_computation(i: u32, divisors: &[u32]) -> u32 { +/// /// # Safety +/// /// All elements of `divisor` must be non-zero. +/// unsafe fn do_computation(i: u32, divisors: &[u32]) -> u32 { /// divisors.iter().fold(i, |acc, divisor| { /// // Convince the compiler that a division by zero can't happen here /// // and a check is not needed below. /// if *divisor == 0 { -/// // SAFETY: `divisor` can't be zero because of `prepare_inputs`, +/// // Safety: `divisor` can't be zero because of `prepare_inputs`, /// // but the compiler does not know about this. We *promise* /// // that we always call `prepare_inputs`. -/// unsafe { std::hint::unreachable_unchecked() } +/// std::hint::unreachable_unchecked() /// } /// // The compiler would normally introduce a check here that prevents /// // a division by zero. However, if `divisor` was zero, the branch @@ -61,11 +64,15 @@ use crate::intrinsics; /// /// let mut divisors = vec![2, 0, 4]; /// prepare_inputs(&mut divisors); -/// assert_eq!(do_computation(100, &divisors), 12); +/// let result = unsafe { +/// // Safety: prepare_inputs() guarantees that divisors is non-zero +/// do_computation(100, &divisors) +/// }; +/// assert_eq!(result, 12); /// /// ``` /// -/// While using `unreachable_unchecked()` is perfectly safe in the following +/// While using `unreachable_unchecked()` is perfectly sound in the following /// example, the compiler is able to prove that a division by zero is not /// possible. Benchmarking reveals that `unreachable_unchecked()` provides /// no benefit over using [`unreachable!`], while the latter does not introduce From 7f318256c908be1df494ae16c2d2dad94e9f66a7 Mon Sep 17 00:00:00 2001 From: klensy Date: Thu, 12 May 2022 19:26:52 +0300 Subject: [PATCH 4/6] fix clippy expect_fun_call --- compiler/rustc_const_eval/src/const_eval/valtrees.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/compiler/rustc_const_eval/src/const_eval/valtrees.rs b/compiler/rustc_const_eval/src/const_eval/valtrees.rs index 374179d0cc24d..f57f25c19f90f 100644 --- a/compiler/rustc_const_eval/src/const_eval/valtrees.rs +++ b/compiler/rustc_const_eval/src/const_eval/valtrees.rs @@ -58,7 +58,9 @@ fn slice_branches<'tcx>( ecx: &CompileTimeEvalContext<'tcx, 'tcx>, place: &MPlaceTy<'tcx>, ) -> Option> { - let n = place.len(&ecx.tcx.tcx).expect(&format!("expected to use len of place {:?}", place)); + let n = place + .len(&ecx.tcx.tcx) + .unwrap_or_else(|_| panic!("expected to use len of place {:?}", place)); let branches = (0..n).map(|i| { let place_elem = ecx.mplace_index(place, i).unwrap(); const_to_valtree_inner(ecx, &place_elem) From fd01fbc05884178122471a1a44b715f0dc87d187 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Fri, 13 May 2022 16:01:18 +1000 Subject: [PATCH 5/6] Remove some unnecessary `rustc_allow_const_fn_unstable` attributes. --- library/alloc/src/raw_vec.rs | 1 - library/core/src/num/nonzero.rs | 1 - library/core/src/option.rs | 1 - library/core/src/task/wake.rs | 1 - library/proc_macro/src/bridge/client.rs | 5 ----- library/proc_macro/src/bridge/scoped_cell.rs | 1 - 6 files changed, 10 deletions(-) diff --git a/library/alloc/src/raw_vec.rs b/library/alloc/src/raw_vec.rs index 9dbac3c36ffb2..4be5f6cf9ca51 100644 --- a/library/alloc/src/raw_vec.rs +++ b/library/alloc/src/raw_vec.rs @@ -118,7 +118,6 @@ impl RawVec { /// Like `new`, but parameterized over the choice of allocator for /// the returned `RawVec`. - #[rustc_allow_const_fn_unstable(const_fn)] pub const fn new_in(alloc: A) -> Self { // `cap: 0` means "unallocated". zero-sized types are ignored. Self { ptr: Unique::dangling(), cap: 0, alloc } diff --git a/library/core/src/num/nonzero.rs b/library/core/src/num/nonzero.rs index a98b5d787f10a..70969edd6eaf7 100644 --- a/library/core/src/num/nonzero.rs +++ b/library/core/src/num/nonzero.rs @@ -52,7 +52,6 @@ macro_rules! nonzero_integers { #[$const_new_unchecked_stability] #[must_use] #[inline] - #[rustc_allow_const_fn_unstable(const_fn_fn_ptr_basics)] // required by assert_unsafe_precondition pub const unsafe fn new_unchecked(n: $Int) -> Self { // SAFETY: this is guaranteed to be safe by the caller. unsafe { diff --git a/library/core/src/option.rs b/library/core/src/option.rs index f339b076dd7d0..d0746698f4013 100644 --- a/library/core/src/option.rs +++ b/library/core/src/option.rs @@ -1486,7 +1486,6 @@ impl Option { where T: ~const Default, { - #[rustc_allow_const_fn_unstable(const_fn_trait_bound)] const fn default() -> T { T::default() } diff --git a/library/core/src/task/wake.rs b/library/core/src/task/wake.rs index 27af227a1f27f..413fe7e6cc40f 100644 --- a/library/core/src/task/wake.rs +++ b/library/core/src/task/wake.rs @@ -147,7 +147,6 @@ impl RawWakerVTable { #[rustc_promotable] #[stable(feature = "futures_api", since = "1.36.0")] #[rustc_const_stable(feature = "futures_api", since = "1.36.0")] - #[rustc_allow_const_fn_unstable(const_fn_fn_ptr_basics)] pub const fn new( clone: unsafe fn(*const ()) -> RawWaker, wake: unsafe fn(*const ()), diff --git a/library/proc_macro/src/bridge/client.rs b/library/proc_macro/src/bridge/client.rs index cf51d8da16db5..cdb2bac260757 100644 --- a/library/proc_macro/src/bridge/client.rs +++ b/library/proc_macro/src/bridge/client.rs @@ -416,7 +416,6 @@ fn run_client DecodeMut<'a, 's, ()>, R: Encode<()>>( } impl Client crate::TokenStream> { - #[rustc_allow_const_fn_unstable(const_fn)] pub const fn expand1(f: fn(crate::TokenStream) -> crate::TokenStream) -> Self { extern "C" fn run( bridge: Bridge<'_>, @@ -429,7 +428,6 @@ impl Client crate::TokenStream> { } impl Client crate::TokenStream> { - #[rustc_allow_const_fn_unstable(const_fn)] pub const fn expand2( f: fn(crate::TokenStream, crate::TokenStream) -> crate::TokenStream, ) -> Self { @@ -474,7 +472,6 @@ impl ProcMacro { } } - #[rustc_allow_const_fn_unstable(const_fn)] pub const fn custom_derive( trait_name: &'static str, attributes: &'static [&'static str], @@ -483,7 +480,6 @@ impl ProcMacro { ProcMacro::CustomDerive { trait_name, attributes, client: Client::expand1(expand) } } - #[rustc_allow_const_fn_unstable(const_fn)] pub const fn attr( name: &'static str, expand: fn(crate::TokenStream, crate::TokenStream) -> crate::TokenStream, @@ -491,7 +487,6 @@ impl ProcMacro { ProcMacro::Attr { name, client: Client::expand2(expand) } } - #[rustc_allow_const_fn_unstable(const_fn)] pub const fn bang( name: &'static str, expand: fn(crate::TokenStream) -> crate::TokenStream, diff --git a/library/proc_macro/src/bridge/scoped_cell.rs b/library/proc_macro/src/bridge/scoped_cell.rs index e1307856175b1..2cde1f65adf9c 100644 --- a/library/proc_macro/src/bridge/scoped_cell.rs +++ b/library/proc_macro/src/bridge/scoped_cell.rs @@ -35,7 +35,6 @@ impl<'a, 'b, T: LambdaL> DerefMut for RefMutL<'a, 'b, T> { pub struct ScopedCell(Cell<>::Out>); impl ScopedCell { - #[rustc_allow_const_fn_unstable(const_fn)] pub const fn new(value: >::Out) -> Self { ScopedCell(Cell::new(value)) } From 9ff4d349232f9be3c779aae74332af41715494b7 Mon Sep 17 00:00:00 2001 From: Yuki Okushi Date: Fri, 13 May 2022 22:04:47 +0900 Subject: [PATCH 6/6] Add regression test for #28935 --- src/test/ui/inference/issue-28935.rs | 9 +++++++++ 1 file changed, 9 insertions(+) create mode 100644 src/test/ui/inference/issue-28935.rs diff --git a/src/test/ui/inference/issue-28935.rs b/src/test/ui/inference/issue-28935.rs new file mode 100644 index 0000000000000..872822dbd0f66 --- /dev/null +++ b/src/test/ui/inference/issue-28935.rs @@ -0,0 +1,9 @@ +// check-pass + +use std::cell::RefCell; + +pub fn f(v: Vec>) { + let _t = &mut *v[0].borrow_mut(); +} + +fn main() {}