From 78245286dcde9f2cf99a370ec0cd4383fa684404 Mon Sep 17 00:00:00 2001 From: joboet Date: Sat, 31 Dec 2022 11:00:54 +0100 Subject: [PATCH 01/21] std: use id-based thread parking on SOLID --- library/std/src/sys/itron/thread_parking.rs | 37 +++++++ library/std/src/sys/itron/wait_flag.rs | 72 ------------- library/std/src/sys/solid/mod.rs | 4 +- .../std/src/sys_common/thread_parking/mod.rs | 4 +- .../sys_common/thread_parking/wait_flag.rs | 102 ------------------ 5 files changed, 40 insertions(+), 179 deletions(-) create mode 100644 library/std/src/sys/itron/thread_parking.rs delete mode 100644 library/std/src/sys/itron/wait_flag.rs delete mode 100644 library/std/src/sys_common/thread_parking/wait_flag.rs diff --git a/library/std/src/sys/itron/thread_parking.rs b/library/std/src/sys/itron/thread_parking.rs new file mode 100644 index 0000000000000..fe9934439d152 --- /dev/null +++ b/library/std/src/sys/itron/thread_parking.rs @@ -0,0 +1,37 @@ +use super::abi; +use super::error::expect_success_aborting; +use super::time::with_tmos; +use crate::time::Duration; + +pub type ThreadId = abi::ID; + +pub use super::task::current_task_id_aborting as current; + +pub fn park(_hint: usize) { + match unsafe { abi::slp_tsk() } { + abi::E_OK | abi::E_RLWAI => {} + err => { + expect_success_aborting(err, &"slp_tsk"); + } + } +} + +pub fn park_timeout(dur: Duration, _hint: usize) { + match with_tmos(dur, |tmo| unsafe { abi::tslp_tsk(tmo) }) { + abi::E_OK | abi::E_RLWAI | abi::E_TMOUT => {} + err => { + expect_success_aborting(err, &"tslp_tsk"); + } + } +} + +pub fn unpark(id: ThreadId, _hint: usize) { + match unsafe { abi::wup_tsk(id) } { + // It is allowed to try to wake up a destroyed or unrelated task, so we ignore all + // errors that could result from that situation. + abi::E_OK | abi::E_NOEXS | abi::E_OBJ | abi::E_QOVR => {} + err => { + expect_success_aborting(err, &"wup_tsk"); + } + } +} diff --git a/library/std/src/sys/itron/wait_flag.rs b/library/std/src/sys/itron/wait_flag.rs deleted file mode 100644 index e432edd207754..0000000000000 --- a/library/std/src/sys/itron/wait_flag.rs +++ /dev/null @@ -1,72 +0,0 @@ -use crate::mem::MaybeUninit; -use crate::time::Duration; - -use super::{ - abi, - error::{expect_success, fail}, - time::with_tmos, -}; - -const CLEAR: abi::FLGPTN = 0; -const RAISED: abi::FLGPTN = 1; - -/// A thread parking primitive that is not susceptible to race conditions, -/// but provides no atomic ordering guarantees and allows only one `raise` per wait. -pub struct WaitFlag { - flag: abi::ID, -} - -impl WaitFlag { - /// Creates a new wait flag. - pub fn new() -> WaitFlag { - let flag = expect_success( - unsafe { - abi::acre_flg(&abi::T_CFLG { - flgatr: abi::TA_FIFO | abi::TA_WSGL | abi::TA_CLR, - iflgptn: CLEAR, - }) - }, - &"acre_flg", - ); - - WaitFlag { flag } - } - - /// Wait for the wait flag to be raised. - pub fn wait(&self) { - let mut token = MaybeUninit::uninit(); - expect_success( - unsafe { abi::wai_flg(self.flag, RAISED, abi::TWF_ORW, token.as_mut_ptr()) }, - &"wai_flg", - ); - } - - /// Wait for the wait flag to be raised or the timeout to occur. - /// - /// Returns whether the flag was raised (`true`) or the operation timed out (`false`). - pub fn wait_timeout(&self, dur: Duration) -> bool { - let mut token = MaybeUninit::uninit(); - let res = with_tmos(dur, |tmout| unsafe { - abi::twai_flg(self.flag, RAISED, abi::TWF_ORW, token.as_mut_ptr(), tmout) - }); - - match res { - abi::E_OK => true, - abi::E_TMOUT => false, - error => fail(error, &"twai_flg"), - } - } - - /// Raise the wait flag. - /// - /// Calls to this function should be balanced with the number of successful waits. - pub fn raise(&self) { - expect_success(unsafe { abi::set_flg(self.flag, RAISED) }, &"set_flg"); - } -} - -impl Drop for WaitFlag { - fn drop(&mut self) { - expect_success(unsafe { abi::del_flg(self.flag) }, &"del_flg"); - } -} diff --git a/library/std/src/sys/solid/mod.rs b/library/std/src/sys/solid/mod.rs index 5867979a2a729..923d27fd9369d 100644 --- a/library/std/src/sys/solid/mod.rs +++ b/library/std/src/sys/solid/mod.rs @@ -13,9 +13,9 @@ mod itron { pub(super) mod spin; pub(super) mod task; pub mod thread; + pub mod thread_parking; pub(super) mod time; use super::unsupported; - pub mod wait_flag; } pub mod alloc; @@ -43,8 +43,8 @@ pub use self::itron::thread; pub mod memchr; pub mod thread_local_dtor; pub mod thread_local_key; +pub use self::itron::thread_parking; pub mod time; -pub use self::itron::wait_flag; mod rwlock; diff --git a/library/std/src/sys_common/thread_parking/mod.rs b/library/std/src/sys_common/thread_parking/mod.rs index 0ead6633c3501..e8e028bb3308f 100644 --- a/library/std/src/sys_common/thread_parking/mod.rs +++ b/library/std/src/sys_common/thread_parking/mod.rs @@ -14,12 +14,10 @@ cfg_if::cfg_if! { } else if #[cfg(any( target_os = "netbsd", all(target_vendor = "fortanix", target_env = "sgx"), + target_os = "solid_asp3", ))] { mod id; pub use id::Parker; - } else if #[cfg(target_os = "solid_asp3")] { - mod wait_flag; - pub use wait_flag::Parker; } else if #[cfg(any(windows, target_family = "unix"))] { pub use crate::sys::thread_parking::Parker; } else { diff --git a/library/std/src/sys_common/thread_parking/wait_flag.rs b/library/std/src/sys_common/thread_parking/wait_flag.rs deleted file mode 100644 index d0f8899a94eb8..0000000000000 --- a/library/std/src/sys_common/thread_parking/wait_flag.rs +++ /dev/null @@ -1,102 +0,0 @@ -//! A wait-flag-based thread parker. -//! -//! Some operating systems provide low-level parking primitives like wait counts, -//! event flags or semaphores which are not susceptible to race conditions (meaning -//! the wakeup can occur before the wait operation). To implement the `std` thread -//! parker on top of these primitives, we only have to ensure that parking is fast -//! when the thread token is available, the atomic ordering guarantees are maintained -//! and spurious wakeups are minimized. -//! -//! To achieve this, this parker uses an atomic variable with three states: `EMPTY`, -//! `PARKED` and `NOTIFIED`: -//! * `EMPTY` means the token has not been made available, but the thread is not -//! currently waiting on it. -//! * `PARKED` means the token is not available and the thread is parked. -//! * `NOTIFIED` means the token is available. -//! -//! `park` and `park_timeout` change the state from `EMPTY` to `PARKED` and from -//! `NOTIFIED` to `EMPTY`. If the state was `NOTIFIED`, the thread was unparked and -//! execution can continue without calling into the OS. If the state was `EMPTY`, -//! the token is not available and the thread waits on the primitive (here called -//! "wait flag"). -//! -//! `unpark` changes the state to `NOTIFIED`. If the state was `PARKED`, the thread -//! is or will be sleeping on the wait flag, so we raise it. - -use crate::pin::Pin; -use crate::sync::atomic::AtomicI8; -use crate::sync::atomic::Ordering::{Acquire, Relaxed, Release}; -use crate::sys::wait_flag::WaitFlag; -use crate::time::Duration; - -const EMPTY: i8 = 0; -const PARKED: i8 = -1; -const NOTIFIED: i8 = 1; - -pub struct Parker { - state: AtomicI8, - wait_flag: WaitFlag, -} - -impl Parker { - /// Construct a parker for the current thread. The UNIX parker - /// implementation requires this to happen in-place. - pub unsafe fn new_in_place(parker: *mut Parker) { - parker.write(Parker { state: AtomicI8::new(EMPTY), wait_flag: WaitFlag::new() }) - } - - // This implementation doesn't require `unsafe` and `Pin`, but other implementations do. - pub unsafe fn park(self: Pin<&Self>) { - match self.state.fetch_sub(1, Acquire) { - // NOTIFIED => EMPTY - NOTIFIED => return, - // EMPTY => PARKED - EMPTY => (), - _ => panic!("inconsistent park state"), - } - - // Avoid waking up from spurious wakeups (these are quite likely, see below). - loop { - self.wait_flag.wait(); - - match self.state.compare_exchange(NOTIFIED, EMPTY, Acquire, Relaxed) { - Ok(_) => return, - Err(PARKED) => (), - Err(_) => panic!("inconsistent park state"), - } - } - } - - // This implementation doesn't require `unsafe` and `Pin`, but other implementations do. - pub unsafe fn park_timeout(self: Pin<&Self>, dur: Duration) { - match self.state.fetch_sub(1, Acquire) { - NOTIFIED => return, - EMPTY => (), - _ => panic!("inconsistent park state"), - } - - self.wait_flag.wait_timeout(dur); - - // Either a wakeup or a timeout occurred. Wakeups may be spurious, as there can be - // a race condition when `unpark` is performed between receiving the timeout and - // resetting the state, resulting in the eventflag being set unnecessarily. `park` - // is protected against this by looping until the token is actually given, but - // here we cannot easily tell. - - // Use `swap` to provide acquire ordering. - match self.state.swap(EMPTY, Acquire) { - NOTIFIED => (), - PARKED => (), - _ => panic!("inconsistent park state"), - } - } - - // This implementation doesn't require `Pin`, but other implementations do. - pub fn unpark(self: Pin<&Self>) { - let state = self.state.swap(NOTIFIED, Release); - - if state == PARKED { - self.wait_flag.raise(); - } - } -} From 837c1aff0546edca18b43fbfc65b8dacf0b0b699 Mon Sep 17 00:00:00 2001 From: Ali MJ Al-Nasrawy Date: Mon, 16 Jan 2023 01:31:58 +0300 Subject: [PATCH 02/21] rework min_choice algorithm of member constraints See the inline comments for the description of the new algorithm. --- .../rustc_borrowck/src/region_infer/mod.rs | 35 ++++++--- ...mber-constraints-min-choice-issue-63033.rs | 10 +++ .../min-choice-reject-ambiguous.rs | 43 +++++++++++ .../min-choice-reject-ambiguous.stderr | 40 ++++++++++ tests/ui/nll/member-constraints/min-choice.rs | 34 +++++++++ .../nested-impl-trait-fail.rs | 33 ++++++++ .../nested-impl-trait-fail.stderr | 75 +++++++++++++++++++ .../nested-impl-trait-pass.rs | 29 +++++++ 8 files changed, 288 insertions(+), 11 deletions(-) create mode 100644 tests/ui/async-await/multiple-lifetimes/member-constraints-min-choice-issue-63033.rs create mode 100644 tests/ui/nll/member-constraints/min-choice-reject-ambiguous.rs create mode 100644 tests/ui/nll/member-constraints/min-choice-reject-ambiguous.stderr create mode 100644 tests/ui/nll/member-constraints/min-choice.rs create mode 100644 tests/ui/nll/member-constraints/nested-impl-trait-fail.rs create mode 100644 tests/ui/nll/member-constraints/nested-impl-trait-fail.stderr create mode 100644 tests/ui/nll/member-constraints/nested-impl-trait-pass.rs diff --git a/compiler/rustc_borrowck/src/region_infer/mod.rs b/compiler/rustc_borrowck/src/region_infer/mod.rs index 308f6e19a73e8..9dd5a6c760716 100644 --- a/compiler/rustc_borrowck/src/region_infer/mod.rs +++ b/compiler/rustc_borrowck/src/region_infer/mod.rs @@ -739,20 +739,33 @@ impl<'tcx> RegionInferenceContext<'tcx> { } debug!(?choice_regions, "after ub"); - // If we ruled everything out, we're done. - if choice_regions.is_empty() { - return false; - } - - // Otherwise, we need to find the minimum remaining choice, if - // any, and take that. - debug!("choice_regions remaining are {:#?}", choice_regions); - let Some(&min_choice) = choice_regions.iter().find(|&r1| { + // At this point we can pick any member of `choice_regions`, but to avoid potential + // non-determinism we will pick the *unique minimum* choice. + // + // Because universal regions are only partially ordered (i.e, not every two regions are + // comparable), we will ignore any region that doesn't compare to all others when picking + // the minimum choice. + // For example, consider `choice_regions = ['static, 'a, 'b, 'c, 'd, 'e]`, where + // `'static: 'a, 'static: 'b, 'a: 'c, 'b: 'c, 'c: 'd, 'c: 'e`. + // `['d, 'e]` are ignored because they do not compare - the same goes for `['a, 'b]`. + let totally_ordered_subset = choice_regions.iter().copied().filter(|&r1| { choice_regions.iter().all(|&r2| { - self.universal_region_relations.outlives(r2, *r1) + self.universal_region_relations.outlives(r1, r2) + || self.universal_region_relations.outlives(r2, r1) }) + }); + // Now we're left with `['static, 'c]`. Pick `'c` as the minimum! + let Some(min_choice) = totally_ordered_subset.reduce(|r1, r2| { + let r1_outlives_r2 = self.universal_region_relations.outlives(r1, r2); + let r2_outlives_r1 = self.universal_region_relations.outlives(r2, r1); + match (r1_outlives_r2, r2_outlives_r1) { + (true, true) => r1.min(r2), + (true, false) => r2, + (false, true) => r1, + (false, false) => bug!("incomparable regions in total order"), + } }) else { - debug!("no choice region outlived by all others"); + debug!("no unique minimum choice"); return false; }; diff --git a/tests/ui/async-await/multiple-lifetimes/member-constraints-min-choice-issue-63033.rs b/tests/ui/async-await/multiple-lifetimes/member-constraints-min-choice-issue-63033.rs new file mode 100644 index 0000000000000..614f189729126 --- /dev/null +++ b/tests/ui/async-await/multiple-lifetimes/member-constraints-min-choice-issue-63033.rs @@ -0,0 +1,10 @@ +// Regression test for #63033. + +// check-pass +// edition: 2018 + +async fn test1(_: &'static u8, _: &'_ u8, _: &'_ u8) {} + +async fn test2<'s>(_: &'s u8, _: &'_ &'s u8, _: &'_ &'s u8) {} + +fn main() {} diff --git a/tests/ui/nll/member-constraints/min-choice-reject-ambiguous.rs b/tests/ui/nll/member-constraints/min-choice-reject-ambiguous.rs new file mode 100644 index 0000000000000..52ea0f28d69f3 --- /dev/null +++ b/tests/ui/nll/member-constraints/min-choice-reject-ambiguous.rs @@ -0,0 +1,43 @@ +// ... continued from ./min-choice.rs + +// check-fail + +trait Cap<'a> {} +impl Cap<'_> for T {} + +fn type_test<'a, T: 'a>() -> &'a u8 { &0 } + +// Make sure we don't pick `'b`. +fn test_b<'a, 'b, 'c, T>() -> impl Cap<'a> + Cap<'b> + Cap<'c> +where + 'a: 'b, + 'a: 'c, + T: 'b, +{ + type_test::<'_, T>() // This should pass if we pick 'b. + //~^ ERROR the parameter type `T` may not live long enough +} + +// Make sure we don't pick `'c`. +fn test_c<'a, 'b, 'c, T>() -> impl Cap<'a> + Cap<'b> + Cap<'c> +where + 'a: 'b, + 'a: 'c, + T: 'c, +{ + type_test::<'_, T>() // This should pass if we pick 'c. + //~^ ERROR the parameter type `T` may not live long enough +} + +// We need to pick min_choice from `['b, 'c]`, but it's ambiguous which one to pick because +// they're incomparable. +fn test_ambiguous<'a, 'b, 'c>(s: &'a u8) -> impl Cap<'b> + Cap<'c> +where + 'a: 'b, + 'a: 'c, +{ + s + //~^ ERROR captures lifetime that does not appear in bounds +} + +fn main() {} diff --git a/tests/ui/nll/member-constraints/min-choice-reject-ambiguous.stderr b/tests/ui/nll/member-constraints/min-choice-reject-ambiguous.stderr new file mode 100644 index 0000000000000..1e6ef614dee24 --- /dev/null +++ b/tests/ui/nll/member-constraints/min-choice-reject-ambiguous.stderr @@ -0,0 +1,40 @@ +error[E0309]: the parameter type `T` may not live long enough + --> $DIR/min-choice-reject-ambiguous.rs:17:5 + | +LL | type_test::<'_, T>() // This should pass if we pick 'b. + | ^^^^^^^^^^^^^^^^^^ ...so that the type `T` will meet its required lifetime bounds + | +help: consider adding an explicit lifetime bound... + | +LL | T: 'b + 'a, + | ++++ + +error[E0309]: the parameter type `T` may not live long enough + --> $DIR/min-choice-reject-ambiguous.rs:28:5 + | +LL | type_test::<'_, T>() // This should pass if we pick 'c. + | ^^^^^^^^^^^^^^^^^^ ...so that the type `T` will meet its required lifetime bounds + | +help: consider adding an explicit lifetime bound... + | +LL | T: 'c + 'a, + | ++++ + +error[E0700]: hidden type for `impl Cap<'b> + Cap<'c>` captures lifetime that does not appear in bounds + --> $DIR/min-choice-reject-ambiguous.rs:39:5 + | +LL | fn test_ambiguous<'a, 'b, 'c>(s: &'a u8) -> impl Cap<'b> + Cap<'c> + | -- hidden type `&'a u8` captures the lifetime `'a` as defined here +... +LL | s + | ^ + | +help: to declare that `impl Cap<'b> + Cap<'c>` captures `'a`, you can add an explicit `'a` lifetime bound + | +LL | fn test_ambiguous<'a, 'b, 'c>(s: &'a u8) -> impl Cap<'b> + Cap<'c> + 'a + | ++++ + +error: aborting due to 3 previous errors + +Some errors have detailed explanations: E0309, E0700. +For more information about an error, try `rustc --explain E0309`. diff --git a/tests/ui/nll/member-constraints/min-choice.rs b/tests/ui/nll/member-constraints/min-choice.rs new file mode 100644 index 0000000000000..14b4dae7abfde --- /dev/null +++ b/tests/ui/nll/member-constraints/min-choice.rs @@ -0,0 +1,34 @@ +// Assuming that the hidden type in these tests is `&'_#15r u8`, +// we have a member constraint: `'_#15r member ['static, 'a, 'b, 'c]`. +// +// Make sure we pick up the minimum non-ambiguous region among them. +// We will have to exclude `['b, 'c]` because they're incomparable, +// and then we should pick `'a` because we know `'static: 'a`. + +// check-pass + +trait Cap<'a> {} +impl Cap<'_> for T {} + +fn type_test<'a, T: 'a>() -> &'a u8 { &0 } + +// Basic test: make sure we don't bail out because 'b and 'c are incomparable. +fn basic<'a, 'b, 'c>() -> impl Cap<'a> + Cap<'b> + Cap<'c> +where + 'a: 'b, + 'a: 'c, +{ + &0 +} + +// Make sure we don't pick `'static`. +fn test_static<'a, 'b, 'c, T>() -> impl Cap<'a> + Cap<'b> + Cap<'c> +where + 'a: 'b, + 'a: 'c, + T: 'a, +{ + type_test::<'_, T>() // This will fail if we pick 'static +} + +fn main() {} diff --git a/tests/ui/nll/member-constraints/nested-impl-trait-fail.rs b/tests/ui/nll/member-constraints/nested-impl-trait-fail.rs new file mode 100644 index 0000000000000..66ff828a84f7c --- /dev/null +++ b/tests/ui/nll/member-constraints/nested-impl-trait-fail.rs @@ -0,0 +1,33 @@ +// Nested impl-traits can impose different member constraints on the same region variable. + +// check-fail + +trait Cap<'a> {} +impl Cap<'_> for T {} + +// Assuming the hidden type is `[&'_#15r u8; 1]`, we have two distinct member constraints: +// - '_#15r member ['static, 'a, 'b] // from outer impl-trait +// - '_#15r member ['static, 'a, 'b] // from inner impl-trait +// To satisfy both we can choose 'a or 'b, so it's a failure due to ambiguity. +fn fail_early_bound<'s, 'a, 'b>(a: &'s u8) -> impl IntoIterator + Cap<'b>> +where + 's: 'a, + 's: 'b, +{ + [a] + //~^ E0700 + //~| E0700 +} + +// Same as the above but with late-bound regions. +fn fail_late_bound<'s, 'a, 'b>( + a: &'s u8, + _: &'a &'s u8, + _: &'b &'s u8, +) -> impl IntoIterator + Cap<'b>> { + [a] + //~^ E0700 + //~| E0700 +} + +fn main() {} diff --git a/tests/ui/nll/member-constraints/nested-impl-trait-fail.stderr b/tests/ui/nll/member-constraints/nested-impl-trait-fail.stderr new file mode 100644 index 0000000000000..6824e27ead028 --- /dev/null +++ b/tests/ui/nll/member-constraints/nested-impl-trait-fail.stderr @@ -0,0 +1,75 @@ +error[E0700]: hidden type for `impl IntoIterator + Cap<'b>>` captures lifetime that does not appear in bounds + --> $DIR/nested-impl-trait-fail.rs:17:5 + | +LL | fn fail_early_bound<'s, 'a, 'b>(a: &'s u8) -> impl IntoIterator + Cap<'b>> + | -- hidden type `[&'s u8; 1]` captures the lifetime `'s` as defined here +... +LL | [a] + | ^^^ + | +help: to declare that `impl IntoIterator + Cap<'b>>` captures `'s`, you can add an explicit `'s` lifetime bound + | +LL | fn fail_early_bound<'s, 'a, 'b>(a: &'s u8) -> impl IntoIterator + Cap<'b>> + 's + | ++++ +help: to declare that `impl Cap<'a> + Cap<'b>` captures `'s`, you can add an explicit `'s` lifetime bound + | +LL | fn fail_early_bound<'s, 'a, 'b>(a: &'s u8) -> impl IntoIterator + Cap<'b> + 's> + | ++++ + +error[E0700]: hidden type for `impl Cap<'a> + Cap<'b>` captures lifetime that does not appear in bounds + --> $DIR/nested-impl-trait-fail.rs:17:5 + | +LL | fn fail_early_bound<'s, 'a, 'b>(a: &'s u8) -> impl IntoIterator + Cap<'b>> + | -- hidden type `&'s u8` captures the lifetime `'s` as defined here +... +LL | [a] + | ^^^ + | +help: to declare that `impl IntoIterator + Cap<'b>>` captures `'s`, you can add an explicit `'s` lifetime bound + | +LL | fn fail_early_bound<'s, 'a, 'b>(a: &'s u8) -> impl IntoIterator + Cap<'b>> + 's + | ++++ +help: to declare that `impl Cap<'a> + Cap<'b>` captures `'s`, you can add an explicit `'s` lifetime bound + | +LL | fn fail_early_bound<'s, 'a, 'b>(a: &'s u8) -> impl IntoIterator + Cap<'b> + 's> + | ++++ + +error[E0700]: hidden type for `impl IntoIterator + Cap<'b>>` captures lifetime that does not appear in bounds + --> $DIR/nested-impl-trait-fail.rs:28:5 + | +LL | fn fail_late_bound<'s, 'a, 'b>( + | -- hidden type `[&'s u8; 1]` captures the lifetime `'s` as defined here +... +LL | [a] + | ^^^ + | +help: to declare that `impl IntoIterator + Cap<'b>>` captures `'s`, you can add an explicit `'s` lifetime bound + | +LL | ) -> impl IntoIterator + Cap<'b>> + 's { + | ++++ +help: to declare that `impl Cap<'a> + Cap<'b>` captures `'s`, you can add an explicit `'s` lifetime bound + | +LL | ) -> impl IntoIterator + Cap<'b> + 's> { + | ++++ + +error[E0700]: hidden type for `impl Cap<'a> + Cap<'b>` captures lifetime that does not appear in bounds + --> $DIR/nested-impl-trait-fail.rs:28:5 + | +LL | fn fail_late_bound<'s, 'a, 'b>( + | -- hidden type `&'s u8` captures the lifetime `'s` as defined here +... +LL | [a] + | ^^^ + | +help: to declare that `impl IntoIterator + Cap<'b>>` captures `'s`, you can add an explicit `'s` lifetime bound + | +LL | ) -> impl IntoIterator + Cap<'b>> + 's { + | ++++ +help: to declare that `impl Cap<'a> + Cap<'b>` captures `'s`, you can add an explicit `'s` lifetime bound + | +LL | ) -> impl IntoIterator + Cap<'b> + 's> { + | ++++ + +error: aborting due to 4 previous errors + +For more information about this error, try `rustc --explain E0700`. diff --git a/tests/ui/nll/member-constraints/nested-impl-trait-pass.rs b/tests/ui/nll/member-constraints/nested-impl-trait-pass.rs new file mode 100644 index 0000000000000..15540cb460e7a --- /dev/null +++ b/tests/ui/nll/member-constraints/nested-impl-trait-pass.rs @@ -0,0 +1,29 @@ +// Nested impl-traits can impose different member constraints on the same region variable. + +// check-pass + +trait Cap<'a> {} +impl Cap<'_> for T {} + +// Assuming the hidden type is `[&'_#15r u8; 1]`, we have two distinct member constraints: +// - '_#15r member ['static, 'a, 'b] // from outer impl-trait +// - '_#15r member ['static, 'a] // from inner impl-trait +// To satisfy both we can only choose 'a. +fn pass_early_bound<'s, 'a, 'b>(a: &'s u8) -> impl IntoIterator> + Cap<'b> +where + 's: 'a, + 's: 'b, +{ + [a] +} + +// Same as the above but with late-bound regions. +fn pass_late_bound<'s, 'a, 'b>( + a: &'s u8, + _: &'a &'s u8, + _: &'b &'s u8, +) -> impl IntoIterator> + Cap<'b> { + [a] +} + +fn main() {} From 8df27d07aed4e12ce6e767f8675599aa0a8e46b9 Mon Sep 17 00:00:00 2001 From: Michael Benfield Date: Fri, 20 Jan 2023 20:56:16 -0800 Subject: [PATCH 03/21] Remove some superfluous type parameters from layout.rs. Specifically remove V, which can always be VariantIdx, and F, which can always be Layout. --- compiler/rustc_abi/src/layout.rs | 146 +++++++++++------------ compiler/rustc_abi/src/lib.rs | 71 +++++++++-- compiler/rustc_middle/src/arena.rs | 2 +- compiler/rustc_middle/src/ty/context.rs | 4 +- compiler/rustc_target/src/abi/mod.rs | 48 +------- compiler/rustc_ty_utils/src/layout.rs | 22 ++-- src/librustdoc/html/render/print_item.rs | 4 +- 7 files changed, 148 insertions(+), 149 deletions(-) diff --git a/compiler/rustc_abi/src/layout.rs b/compiler/rustc_abi/src/layout.rs index 9c2cf58efed4a..54858b52008f9 100644 --- a/compiler/rustc_abi/src/layout.rs +++ b/compiler/rustc_abi/src/layout.rs @@ -1,11 +1,5 @@ use super::*; -use std::{ - borrow::Borrow, - cmp, - fmt::Debug, - iter, - ops::{Bound, Deref}, -}; +use std::{borrow::Borrow, cmp, iter, ops::Bound}; #[cfg(feature = "randomize")] use rand::{seq::SliceRandom, SeedableRng}; @@ -33,7 +27,7 @@ pub trait LayoutCalculator { fn delay_bug(&self, txt: &str); fn current_data_layout(&self) -> Self::TargetDataLayoutRef; - fn scalar_pair(&self, a: Scalar, b: Scalar) -> LayoutS { + fn scalar_pair(&self, a: Scalar, b: Scalar) -> LayoutS { let dl = self.current_data_layout(); let dl = dl.borrow(); let b_align = b.align(dl); @@ -49,7 +43,7 @@ pub trait LayoutCalculator { .max_by_key(|niche| niche.available(dl)); LayoutS { - variants: Variants::Single { index: V::new(0) }, + variants: Variants::Single { index: VariantIdx::new(0) }, fields: FieldsShape::Arbitrary { offsets: vec![Size::ZERO, b_offset], memory_index: vec![0, 1], @@ -61,13 +55,13 @@ pub trait LayoutCalculator { } } - fn univariant<'a, V: Idx, F: Deref> + Debug>( + fn univariant( &self, dl: &TargetDataLayout, - fields: &[F], + fields: &[Layout<'_>], repr: &ReprOptions, kind: StructKind, - ) -> Option> { + ) -> Option { let pack = repr.pack; let mut align = if pack.is_some() { dl.i8_align } else { dl.aggregate_align }; let mut inverse_memory_index: Vec = (0..fields.len() as u32).collect(); @@ -76,17 +70,17 @@ pub trait LayoutCalculator { let end = if let StructKind::MaybeUnsized = kind { fields.len() - 1 } else { fields.len() }; let optimizing = &mut inverse_memory_index[..end]; - let effective_field_align = |f: &F| { + let effective_field_align = |layout: Layout<'_>| { if let Some(pack) = pack { // return the packed alignment in bytes - f.align.abi.min(pack).bytes() + layout.align().abi.min(pack).bytes() } else { // returns log2(effective-align). // This is ok since `pack` applies to all fields equally. // The calculation assumes that size is an integer multiple of align, except for ZSTs. // // group [u8; 4] with align-4 or [u8; 6] with align-2 fields - f.align.abi.bytes().max(f.size.bytes()).trailing_zeros() as u64 + layout.align().abi.bytes().max(layout.size().bytes()).trailing_zeros() as u64 } }; @@ -111,9 +105,9 @@ pub trait LayoutCalculator { // Place ZSTs first to avoid "interesting offsets", // especially with only one or two non-ZST fields. // Then place largest alignments first, largest niches within an alignment group last - let f = &fields[x as usize]; - let niche_size = f.largest_niche.map_or(0, |n| n.available(dl)); - (!f.is_zst(), cmp::Reverse(effective_field_align(f)), niche_size) + let f = fields[x as usize]; + let niche_size = f.largest_niche().map_or(0, |n| n.available(dl)); + (!f.0.is_zst(), cmp::Reverse(effective_field_align(f)), niche_size) }); } @@ -123,8 +117,8 @@ pub trait LayoutCalculator { // And put the largest niche in an alignment group at the end // so it can be used as discriminant in jagged enums optimizing.sort_by_key(|&x| { - let f = &fields[x as usize]; - let niche_size = f.largest_niche.map_or(0, |n| n.available(dl)); + let f = fields[x as usize]; + let niche_size = f.largest_niche().map_or(0, |n| n.available(dl)); (effective_field_align(f), niche_size) }); } @@ -160,15 +154,15 @@ pub trait LayoutCalculator { )); } - if field.is_unsized() { + if field.0.is_unsized() { sized = false; } // Invariant: offset < dl.obj_size_bound() <= 1<<61 let field_align = if let Some(pack) = pack { - field.align.min(AbiAndPrefAlign::new(pack)) + field.align().min(AbiAndPrefAlign::new(pack)) } else { - field.align + field.align() }; offset = offset.align_to(field_align.abi); align = align.max(field_align); @@ -176,7 +170,7 @@ pub trait LayoutCalculator { debug!("univariant offset: {:?} field: {:#?}", offset, field); offsets[i as usize] = offset; - if let Some(mut niche) = field.largest_niche { + if let Some(mut niche) = field.largest_niche() { let available = niche.available(dl); if available > largest_niche_available { largest_niche_available = available; @@ -185,7 +179,7 @@ pub trait LayoutCalculator { } } - offset = offset.checked_add(field.size, dl)?; + offset = offset.checked_add(field.size(), dl)?; } if let Some(repr_align) = repr.align { align = align.max(AbiAndPrefAlign::new(repr_align)); @@ -205,24 +199,26 @@ pub trait LayoutCalculator { // Unpack newtype ABIs and find scalar pairs. if sized && size.bytes() > 0 { // All other fields must be ZSTs. - let mut non_zst_fields = fields.iter().enumerate().filter(|&(_, f)| !f.is_zst()); + let mut non_zst_fields = fields.iter().enumerate().filter(|&(_, f)| !f.0.is_zst()); match (non_zst_fields.next(), non_zst_fields.next(), non_zst_fields.next()) { // We have exactly one non-ZST field. (Some((i, field)), None, None) => { // Field fills the struct and it has a scalar or scalar pair ABI. - if offsets[i].bytes() == 0 && align.abi == field.align.abi && size == field.size + if offsets[i].bytes() == 0 + && align.abi == field.align().abi + && size == field.size() { - match field.abi { + match field.abi() { // For plain scalars, or vectors of them, we can't unpack // newtypes for `#[repr(C)]`, as that affects C ABIs. Abi::Scalar(_) | Abi::Vector { .. } if optimize => { - abi = field.abi; + abi = field.abi(); } // But scalar pairs are Rust-specific and get // treated as aggregates by C ABIs anyway. Abi::ScalarPair(..) => { - abi = field.abi; + abi = field.abi(); } _ => {} } @@ -231,7 +227,7 @@ pub trait LayoutCalculator { // Two non-ZST fields, and they're both scalars. (Some((i, a)), Some((j, b)), None) => { - match (a.abi, b.abi) { + match (a.abi(), b.abi()) { (Abi::Scalar(a), Abi::Scalar(b)) => { // Order by the memory placement, not source order. let ((i, a), (j, b)) = if offsets[i] < offsets[j] { @@ -239,7 +235,7 @@ pub trait LayoutCalculator { } else { ((j, b), (i, a)) }; - let pair = self.scalar_pair::(a, b); + let pair = self.scalar_pair(a, b); let pair_offsets = match pair.fields { FieldsShape::Arbitrary { ref offsets, ref memory_index } => { assert_eq!(memory_index, &[0, 1]); @@ -264,11 +260,11 @@ pub trait LayoutCalculator { _ => {} } } - if fields.iter().any(|f| f.abi.is_uninhabited()) { + if fields.iter().any(|f| f.abi().is_uninhabited()) { abi = Abi::Uninhabited; } Some(LayoutS { - variants: Variants::Single { index: V::new(0) }, + variants: Variants::Single { index: VariantIdx::new(0) }, fields: FieldsShape::Arbitrary { offsets, memory_index }, abi, largest_niche, @@ -277,11 +273,11 @@ pub trait LayoutCalculator { }) } - fn layout_of_never_type(&self) -> LayoutS { + fn layout_of_never_type(&self) -> LayoutS { let dl = self.current_data_layout(); let dl = dl.borrow(); LayoutS { - variants: Variants::Single { index: V::new(0) }, + variants: Variants::Single { index: VariantIdx::new(0) }, fields: FieldsShape::Primitive, abi: Abi::Uninhabited, largest_niche: None, @@ -290,18 +286,18 @@ pub trait LayoutCalculator { } } - fn layout_of_struct_or_enum<'a, V: Idx, F: Deref> + Debug>( + fn layout_of_struct_or_enum( &self, repr: &ReprOptions, - variants: &IndexVec>, + variants: &IndexVec>>, is_enum: bool, is_unsafe_cell: bool, scalar_valid_range: (Bound, Bound), discr_range_of_repr: impl Fn(i128, i128) -> (Integer, bool), - discriminants: impl Iterator, + discriminants: impl Iterator, niche_optimize_enum: bool, always_sized: bool, - ) -> Option> { + ) -> Option { let dl = self.current_data_layout(); let dl = dl.borrow(); @@ -316,9 +312,9 @@ pub trait LayoutCalculator { // but *not* an encoding of the discriminant (e.g., a tag value). // See issue #49298 for more details on the need to leave space // for non-ZST uninhabited data (mostly partial initialization). - let absent = |fields: &[F]| { - let uninhabited = fields.iter().any(|f| f.abi.is_uninhabited()); - let is_zst = fields.iter().all(|f| f.is_zst()); + let absent = |fields: &[Layout<'_>]| { + let uninhabited = fields.iter().any(|f| f.abi().is_uninhabited()); + let is_zst = fields.iter().all(|f| f.0.is_zst()); uninhabited && is_zst }; let (present_first, present_second) = { @@ -335,7 +331,7 @@ pub trait LayoutCalculator { } // If it's a struct, still compute a layout so that we can still compute the // field offsets. - None => V::new(0), + None => VariantIdx::new(0), }; let is_struct = !is_enum || @@ -439,12 +435,12 @@ pub trait LayoutCalculator { // variant layouts, so we can't store them in the // overall LayoutS. Store the overall LayoutS // and the variant LayoutSs here until then. - struct TmpLayout { - layout: LayoutS, - variants: IndexVec>, + struct TmpLayout { + layout: LayoutS, + variants: IndexVec, } - let calculate_niche_filling_layout = || -> Option> { + let calculate_niche_filling_layout = || -> Option { if niche_optimize_enum { return None; } @@ -464,15 +460,16 @@ pub trait LayoutCalculator { Some(st) }) - .collect::>>()?; + .collect::>>()?; let largest_variant_index = variant_layouts .iter_enumerated() .max_by_key(|(_i, layout)| layout.size.bytes()) .map(|(i, _layout)| i)?; - let all_indices = (0..=variants.len() - 1).map(V::new); - let needs_disc = |index: V| index != largest_variant_index && !absent(&variants[index]); + let all_indices = (0..=variants.len() - 1).map(VariantIdx::new); + let needs_disc = + |index: VariantIdx| index != largest_variant_index && !absent(&variants[index]); let niche_variants = all_indices.clone().find(|v| needs_disc(*v)).unwrap().index() ..=all_indices.rev().find(|v| needs_disc(*v)).unwrap().index(); @@ -482,7 +479,7 @@ pub trait LayoutCalculator { let (field_index, niche, (niche_start, niche_scalar)) = variants[largest_variant_index] .iter() .enumerate() - .filter_map(|(j, field)| Some((j, field.largest_niche?))) + .filter_map(|(j, field)| Some((j, field.largest_niche()?))) .max_by_key(|(_, niche)| niche.available(dl)) .and_then(|(j, niche)| Some((j, niche, niche.reserve(dl, count)?)))?; let niche_offset = @@ -514,7 +511,7 @@ pub trait LayoutCalculator { match layout.fields { FieldsShape::Arbitrary { ref mut offsets, .. } => { for (j, offset) in offsets.iter_mut().enumerate() { - if !variants[i][j].is_zst() { + if !variants[i][j].0.is_zst() { *offset += this_offset; } } @@ -572,8 +569,8 @@ pub trait LayoutCalculator { tag: niche_scalar, tag_encoding: TagEncoding::Niche { untagged_variant: largest_variant_index, - niche_variants: (V::new(*niche_variants.start()) - ..=V::new(*niche_variants.end())), + niche_variants: (VariantIdx::new(*niche_variants.start()) + ..=VariantIdx::new(*niche_variants.end())), niche_start, }, tag_field: 0, @@ -598,7 +595,7 @@ pub trait LayoutCalculator { let discr_type = repr.discr_type(); let bits = Integer::from_attr(dl, discr_type).size().bits(); for (i, mut val) in discriminants { - if variants[i].iter().any(|f| f.abi.is_uninhabited()) { + if variants[i].iter().any(|f| f.abi().is_uninhabited()) { continue; } if discr_type.is_signed() { @@ -636,7 +633,7 @@ pub trait LayoutCalculator { if repr.c() { for fields in variants { for field in fields { - prefix_align = prefix_align.max(field.align.abi); + prefix_align = prefix_align.max(field.align().abi); } } } @@ -655,8 +652,8 @@ pub trait LayoutCalculator { // Find the first field we can't move later // to make room for a larger discriminant. for field in st.fields.index_by_increasing_offset().map(|j| &field_layouts[j]) { - if !field.is_zst() || field.align.abi.bytes() != 1 { - start_align = start_align.min(field.align.abi); + if !field.0.is_zst() || field.align().abi.bytes() != 1 { + start_align = start_align.min(field.align().abi); break; } } @@ -664,7 +661,7 @@ pub trait LayoutCalculator { align = align.max(st.align); Some(st) }) - .collect::>>()?; + .collect::>>()?; // Align the maximum variant size to the largest alignment. size = size.align_to(align.abi); @@ -759,7 +756,7 @@ pub trait LayoutCalculator { let FieldsShape::Arbitrary { ref offsets, .. } = layout_variant.fields else { panic!(); }; - let mut fields = iter::zip(field_layouts, offsets).filter(|p| !p.0.is_zst()); + let mut fields = iter::zip(field_layouts, offsets).filter(|p| !p.0.0.is_zst()); let (field, offset) = match (fields.next(), fields.next()) { (None, None) => { common_prim_initialized_in_all_variants = false; @@ -771,7 +768,7 @@ pub trait LayoutCalculator { break; } }; - let prim = match field.abi { + let prim = match field.abi() { Abi::Scalar(scalar) => { common_prim_initialized_in_all_variants &= matches!(scalar, Scalar::Initialized { .. }); @@ -802,7 +799,7 @@ pub trait LayoutCalculator { // Common prim might be uninit. Scalar::Union { value: prim } }; - let pair = self.scalar_pair::(tag, prim_scalar); + let pair = self.scalar_pair(tag, prim_scalar); let pair_offsets = match pair.fields { FieldsShape::Arbitrary { ref offsets, ref memory_index } => { assert_eq!(memory_index, &[0, 1]); @@ -862,9 +859,8 @@ pub trait LayoutCalculator { // pick the layout with the larger niche; otherwise, // pick tagged as it has simpler codegen. use cmp::Ordering::*; - let niche_size = |tmp_l: &TmpLayout| { - tmp_l.layout.largest_niche.map_or(0, |n| n.available(dl)) - }; + let niche_size = + |tmp_l: &TmpLayout| tmp_l.layout.largest_niche.map_or(0, |n| n.available(dl)); match (tl.layout.size.cmp(&nl.layout.size), niche_size(&tl).cmp(&niche_size(&nl))) { (Greater, _) => nl, (Equal, Less) => nl, @@ -884,11 +880,11 @@ pub trait LayoutCalculator { Some(best_layout.layout) } - fn layout_of_union<'a, V: Idx, F: Deref> + Debug>( + fn layout_of_union( &self, repr: &ReprOptions, - variants: &IndexVec>, - ) -> Option> { + variants: &IndexVec>>, + ) -> Option { let dl = self.current_data_layout(); let dl = dl.borrow(); let mut align = if repr.pack.is_some() { dl.i8_align } else { dl.aggregate_align }; @@ -900,15 +896,15 @@ pub trait LayoutCalculator { let optimize = !repr.inhibit_union_abi_opt(); let mut size = Size::ZERO; let mut abi = Abi::Aggregate { sized: true }; - let index = V::new(0); + let index = VariantIdx::new(0); for field in &variants[index] { - assert!(field.is_sized()); - align = align.max(field.align); + assert!(field.0.is_sized()); + align = align.max(field.align()); // If all non-ZST fields have the same ABI, forward this ABI - if optimize && !field.is_zst() { + if optimize && !field.0.is_zst() { // Discard valid range information and allow undef - let field_abi = match field.abi { + let field_abi = match field.abi() { Abi::Scalar(x) => Abi::Scalar(x.to_union()), Abi::ScalarPair(x, y) => Abi::ScalarPair(x.to_union(), y.to_union()), Abi::Vector { element: x, count } => { @@ -926,7 +922,7 @@ pub trait LayoutCalculator { } } - size = cmp::max(size, field.size); + size = cmp::max(size, field.size()); } if let Some(pack) = repr.pack { diff --git a/compiler/rustc_abi/src/lib.rs b/compiler/rustc_abi/src/lib.rs index f4cb459f32fdd..5cd0aff2d5b71 100644 --- a/compiler/rustc_abi/src/lib.rs +++ b/compiler/rustc_abi/src/lib.rs @@ -8,6 +8,7 @@ use std::ops::{Add, AddAssign, Mul, RangeInclusive, Sub}; use std::str::FromStr; use bitflags::bitflags; +use rustc_data_structures::intern::Interned; #[cfg(feature = "nightly")] use rustc_data_structures::stable_hasher::StableOrd; use rustc_index::vec::{Idx, IndexVec}; @@ -1257,9 +1258,9 @@ impl Abi { #[derive(PartialEq, Eq, Hash, Clone, Debug)] #[cfg_attr(feature = "nightly", derive(HashStable_Generic))] -pub enum Variants { +pub enum Variants { /// Single enum variants, structs/tuples, unions, and all non-ADTs. - Single { index: V }, + Single { index: VariantIdx }, /// Enum-likes with more than one inhabited variant: each variant comes with /// a *discriminant* (usually the same as the variant index but the user can @@ -1269,15 +1270,15 @@ pub enum Variants { /// For enums, the tag is the sole field of the layout. Multiple { tag: Scalar, - tag_encoding: TagEncoding, + tag_encoding: TagEncoding, tag_field: usize, - variants: IndexVec>, + variants: IndexVec, }, } #[derive(PartialEq, Eq, Hash, Clone, Debug)] #[cfg_attr(feature = "nightly", derive(HashStable_Generic))] -pub enum TagEncoding { +pub enum TagEncoding { /// The tag directly stores the discriminant, but possibly with a smaller layout /// (so converting the tag to the discriminant can require sign extension). Direct, @@ -1292,7 +1293,11 @@ pub enum TagEncoding { /// For example, `Option<(usize, &T)>` is represented such that /// `None` has a null pointer for the second tuple field, and /// `Some` is the identity function (with a non-null reference). - Niche { untagged_variant: V, niche_variants: RangeInclusive, niche_start: u128 }, + Niche { + untagged_variant: VariantIdx, + niche_variants: RangeInclusive, + niche_start: u128, + }, } #[derive(Clone, Copy, PartialEq, Eq, Hash, Debug)] @@ -1379,9 +1384,14 @@ impl Niche { } } +rustc_index::newtype_index! { + #[derive(HashStable_Generic)] + pub struct VariantIdx {} +} + #[derive(PartialEq, Eq, Hash, Clone)] #[cfg_attr(feature = "nightly", derive(HashStable_Generic))] -pub struct LayoutS { +pub struct LayoutS { /// Says where the fields are located within the layout. pub fields: FieldsShape, @@ -1392,7 +1402,7 @@ pub struct LayoutS { /// /// To access all fields of this layout, both `fields` and the fields of the active variant /// must be taken into account. - pub variants: Variants, + pub variants: Variants, /// The `abi` defines how this data is passed between functions, and it defines /// value restrictions via `valid_range`. @@ -1411,13 +1421,13 @@ pub struct LayoutS { pub size: Size, } -impl LayoutS { +impl LayoutS { pub fn scalar(cx: &C, scalar: Scalar) -> Self { let largest_niche = Niche::from_scalar(cx, Size::ZERO, scalar); let size = scalar.size(cx); let align = scalar.align(cx); LayoutS { - variants: Variants::Single { index: V::new(0) }, + variants: Variants::Single { index: VariantIdx::new(0) }, fields: FieldsShape::Primitive, abi: Abi::Scalar(scalar), largest_niche, @@ -1427,7 +1437,7 @@ impl LayoutS { } } -impl fmt::Debug for LayoutS { +impl fmt::Debug for LayoutS { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { // This is how `Layout` used to print before it become // `Interned`. We print it like this to avoid having to update @@ -1444,6 +1454,43 @@ impl fmt::Debug for LayoutS { } } +#[derive(Copy, Clone, PartialEq, Eq, Hash, HashStable_Generic)] +#[rustc_pass_by_value] +pub struct Layout<'a>(pub Interned<'a, LayoutS>); + +impl<'a> fmt::Debug for Layout<'a> { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + // See comment on `::fmt` above. + self.0.0.fmt(f) + } +} + +impl<'a> Layout<'a> { + pub fn fields(self) -> &'a FieldsShape { + &self.0.0.fields + } + + pub fn variants(self) -> &'a Variants { + &self.0.0.variants + } + + pub fn abi(self) -> Abi { + self.0.0.abi + } + + pub fn largest_niche(self) -> Option { + self.0.0.largest_niche + } + + pub fn align(self) -> AbiAndPrefAlign { + self.0.0.align + } + + pub fn size(self) -> Size { + self.0.0.size + } +} + #[derive(Copy, Clone, PartialEq, Eq, Debug)] pub enum PointerKind { /// Most general case, we know no restrictions to tell LLVM. @@ -1479,7 +1526,7 @@ pub enum InitKind { UninitMitigated0x01Fill, } -impl LayoutS { +impl LayoutS { /// Returns `true` if the layout corresponds to an unsized type. pub fn is_unsized(&self) -> bool { self.abi.is_unsized() diff --git a/compiler/rustc_middle/src/arena.rs b/compiler/rustc_middle/src/arena.rs index f816d614500a0..f0a63c243a5e1 100644 --- a/compiler/rustc_middle/src/arena.rs +++ b/compiler/rustc_middle/src/arena.rs @@ -8,7 +8,7 @@ macro_rules! arena_types { ($macro:path) => ( $macro!([ - [] layout: rustc_target::abi::LayoutS, + [] layout: rustc_target::abi::LayoutS, [] fn_abi: rustc_target::abi::call::FnAbi<'tcx, rustc_middle::ty::Ty<'tcx>>, // AdtDef are interned and compared by address [decode] adt_def: rustc_middle::ty::AdtDefData, diff --git a/compiler/rustc_middle/src/ty/context.rs b/compiler/rustc_middle/src/ty/context.rs index ce04d8d21f4cd..c207d639f4a1f 100644 --- a/compiler/rustc_middle/src/ty/context.rs +++ b/compiler/rustc_middle/src/ty/context.rs @@ -142,7 +142,7 @@ pub struct CtxtInterners<'tcx> { const_: InternedSet<'tcx, ConstData<'tcx>>, const_allocation: InternedSet<'tcx, Allocation>, bound_variable_kinds: InternedSet<'tcx, List>, - layout: InternedSet<'tcx, LayoutS>, + layout: InternedSet<'tcx, LayoutS>, adt_def: InternedSet<'tcx, AdtDefData>, } @@ -1586,7 +1586,7 @@ direct_interners! { region: mk_region(RegionKind<'tcx>): Region -> Region<'tcx>, const_: mk_const_internal(ConstData<'tcx>): Const -> Const<'tcx>, const_allocation: intern_const_alloc(Allocation): ConstAllocation -> ConstAllocation<'tcx>, - layout: intern_layout(LayoutS): Layout -> Layout<'tcx>, + layout: intern_layout(LayoutS): Layout -> Layout<'tcx>, adt_def: intern_adt_def(AdtDefData): AdtDef -> AdtDef<'tcx>, } diff --git a/compiler/rustc_target/src/abi/mod.rs b/compiler/rustc_target/src/abi/mod.rs index 88a0a1f8ecfde..653f99fb11a10 100644 --- a/compiler/rustc_target/src/abi/mod.rs +++ b/compiler/rustc_target/src/abi/mod.rs @@ -3,10 +3,8 @@ pub use Primitive::*; use crate::json::{Json, ToJson}; -use std::fmt; use std::ops::Deref; -use rustc_data_structures::intern::Interned; use rustc_macros::HashStable_Generic; pub mod call; @@ -19,48 +17,6 @@ impl ToJson for Endian { } } -rustc_index::newtype_index! { - #[derive(HashStable_Generic)] - pub struct VariantIdx {} -} - -#[derive(Copy, Clone, PartialEq, Eq, Hash, HashStable_Generic)] -#[rustc_pass_by_value] -pub struct Layout<'a>(pub Interned<'a, LayoutS>); - -impl<'a> fmt::Debug for Layout<'a> { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - // See comment on `::fmt` above. - self.0.0.fmt(f) - } -} - -impl<'a> Layout<'a> { - pub fn fields(self) -> &'a FieldsShape { - &self.0.0.fields - } - - pub fn variants(self) -> &'a Variants { - &self.0.0.variants - } - - pub fn abi(self) -> Abi { - self.0.0.abi - } - - pub fn largest_niche(self) -> Option { - self.0.0.largest_niche - } - - pub fn align(self) -> AbiAndPrefAlign { - self.0.0.align - } - - pub fn size(self) -> Size { - self.0.0.size - } -} - /// The layout of a type, alongside the type itself. /// Provides various type traversal APIs (e.g., recursing into fields). /// @@ -75,8 +31,8 @@ pub struct TyAndLayout<'a, Ty> { } impl<'a, Ty> Deref for TyAndLayout<'a, Ty> { - type Target = &'a LayoutS; - fn deref(&self) -> &&'a LayoutS { + type Target = &'a LayoutS; + fn deref(&self) -> &&'a LayoutS { &self.layout.0.0 } } diff --git a/compiler/rustc_ty_utils/src/layout.rs b/compiler/rustc_ty_utils/src/layout.rs index 6aa016133ca59..68312f8b11c63 100644 --- a/compiler/rustc_ty_utils/src/layout.rs +++ b/compiler/rustc_ty_utils/src/layout.rs @@ -78,10 +78,10 @@ fn invert_mapping(map: &[u32]) -> Vec { fn univariant_uninterned<'tcx>( cx: &LayoutCx<'tcx, TyCtxt<'tcx>>, ty: Ty<'tcx>, - fields: &[TyAndLayout<'_>], + fields: &[Layout<'_>], repr: &ReprOptions, kind: StructKind, -) -> Result, LayoutError<'tcx>> { +) -> Result> { let dl = cx.data_layout(); let pack = repr.pack; if pack.is_some() && repr.align.is_some() { @@ -106,7 +106,7 @@ fn layout_of_uncached<'tcx>( }; let scalar = |value: Primitive| tcx.intern_layout(LayoutS::scalar(cx, scalar_unit(value))); - let univariant = |fields: &[TyAndLayout<'_>], repr: &ReprOptions, kind| { + let univariant = |fields: &[Layout<'_>], repr: &ReprOptions, kind| { Ok(tcx.intern_layout(univariant_uninterned(cx, ty, fields, repr, kind)?)) }; debug_assert!(!ty.has_non_region_infer()); @@ -272,7 +272,7 @@ fn layout_of_uncached<'tcx>( ty::Closure(_, ref substs) => { let tys = substs.as_closure().upvar_tys(); univariant( - &tys.map(|ty| cx.layout_of(ty)).collect::, _>>()?, + &tys.map(|ty| Ok(cx.layout_of(ty)?.layout)).collect::, _>>()?, &ReprOptions::default(), StructKind::AlwaysSized, )? @@ -283,7 +283,7 @@ fn layout_of_uncached<'tcx>( if tys.len() == 0 { StructKind::AlwaysSized } else { StructKind::MaybeUnsized }; univariant( - &tys.iter().map(|k| cx.layout_of(k)).collect::, _>>()?, + &tys.iter().map(|k| Ok(cx.layout_of(k)?.layout)).collect::, _>>()?, &ReprOptions::default(), kind, )? @@ -412,7 +412,7 @@ fn layout_of_uncached<'tcx>( .map(|v| { v.fields .iter() - .map(|field| cx.layout_of(field.ty(tcx, substs))) + .map(|field| Ok(cx.layout_of(field.ty(tcx, substs))?.layout)) .collect::, _>>() }) .collect::, _>>()?; @@ -630,23 +630,21 @@ fn generator_layout<'tcx>( // `info.variant_fields` already accounts for the reserved variants, so no need to add them. let max_discr = (info.variant_fields.len() - 1) as u128; let discr_int = Integer::fit_unsigned(max_discr); - let discr_int_ty = discr_int.to_ty(tcx, false); let tag = Scalar::Initialized { value: Primitive::Int(discr_int, false), valid_range: WrappingRange { start: 0, end: max_discr }, }; let tag_layout = cx.tcx.intern_layout(LayoutS::scalar(cx, tag)); - let tag_layout = TyAndLayout { ty: discr_int_ty, layout: tag_layout }; let promoted_layouts = ineligible_locals .iter() .map(|local| subst_field(info.field_tys[local])) .map(|ty| tcx.mk_maybe_uninit(ty)) - .map(|ty| cx.layout_of(ty)); + .map(|ty| Ok(cx.layout_of(ty)?.layout)); let prefix_layouts = substs .as_generator() .prefix_tys() - .map(|ty| cx.layout_of(ty)) + .map(|ty| Ok(cx.layout_of(ty)?.layout)) .chain(iter::once(Ok(tag_layout))) .chain(promoted_layouts) .collect::, _>>()?; @@ -715,7 +713,9 @@ fn generator_layout<'tcx>( let mut variant = univariant_uninterned( cx, ty, - &variant_only_tys.map(|ty| cx.layout_of(ty)).collect::, _>>()?, + &variant_only_tys + .map(|ty| Ok(cx.layout_of(ty)?.layout)) + .collect::, _>>()?, &ReprOptions::default(), StructKind::Prefixed(prefix_size, prefix_align.abi), )?; diff --git a/src/librustdoc/html/render/print_item.rs b/src/librustdoc/html/render/print_item.rs index f824c9e3ad2bd..e57f087642702 100644 --- a/src/librustdoc/html/render/print_item.rs +++ b/src/librustdoc/html/render/print_item.rs @@ -10,7 +10,7 @@ use rustc_middle::ty::layout::LayoutError; use rustc_middle::ty::{self, Adt, TyCtxt}; use rustc_span::hygiene::MacroKind; use rustc_span::symbol::{kw, sym, Symbol}; -use rustc_target::abi::{LayoutS, Primitive, TagEncoding, VariantIdx, Variants}; +use rustc_target::abi::{LayoutS, Primitive, TagEncoding, Variants}; use std::cmp::Ordering; use std::fmt; use std::rc::Rc; @@ -1887,7 +1887,7 @@ fn document_non_exhaustive(w: &mut Buffer, item: &clean::Item) { } fn document_type_layout(w: &mut Buffer, cx: &Context<'_>, ty_def_id: DefId) { - fn write_size_of_layout(w: &mut Buffer, layout: &LayoutS, tag_size: u64) { + fn write_size_of_layout(w: &mut Buffer, layout: &LayoutS, tag_size: u64) { if layout.abi.is_unsized() { write!(w, "(unsized)"); } else { From e813132e4f8b0c469c9959c2efa1b0629067b3b8 Mon Sep 17 00:00:00 2001 From: SpanishPear Date: Mon, 24 Oct 2022 00:52:59 +1100 Subject: [PATCH 04/21] --wip-- [skip ci] --wip-- [skip ci] get the generic text and put it int he suggestion, but suggestion not working on derive subdiagnostic refactor away from derives and use span_suggestion() instead. Show's the correct(?) generic contents, but overwrites the fn name :( x fmt drop commented code and s/todo/fixme get the correct diagnostic for functions, at least x fmt remove some debugs remove format remove debugs remove useless change remove useless change remove legacy approach correct lookahead + error message contains the ident name fmt refactor code tests add tests remoev debug remove comment --- .../rustc_parse/src/parser/diagnostics.rs | 57 ++++++++++++++++++- .../suggest_misplaced_generics/enum.fixed | 10 ++++ .../parser/suggest_misplaced_generics/enum.rs | 10 ++++ .../suggest_misplaced_generics/enum.stderr | 13 +++++ .../existing_generics.rs | 9 +++ .../existing_generics.stderr | 10 ++++ .../fn-complex-generics.fixed | 10 ++++ .../fn-complex-generics.rs | 10 ++++ .../fn-complex-generics.stderr | 13 +++++ .../fn-invalid-generics.rs | 8 +++ .../fn-invalid-generics.stderr | 8 +++ .../fn-simple.fixed | 10 ++++ .../suggest_misplaced_generics/fn-simple.rs | 10 ++++ .../fn-simple.stderr | 13 +++++ .../suggest_misplaced_generics/struct.fixed | 10 ++++ .../suggest_misplaced_generics/struct.rs | 10 ++++ .../suggest_misplaced_generics/struct.stderr | 13 +++++ .../suggest_misplaced_generics/trait.fixed | 12 ++++ .../suggest_misplaced_generics/trait.rs | 12 ++++ .../suggest_misplaced_generics/trait.stderr | 13 +++++ .../suggest_misplaced_generics/type.fixed | 10 ++++ .../parser/suggest_misplaced_generics/type.rs | 10 ++++ .../suggest_misplaced_generics/type.stderr | 13 +++++ 23 files changed, 292 insertions(+), 2 deletions(-) create mode 100644 src/test/ui/parser/suggest_misplaced_generics/enum.fixed create mode 100644 src/test/ui/parser/suggest_misplaced_generics/enum.rs create mode 100644 src/test/ui/parser/suggest_misplaced_generics/enum.stderr create mode 100644 src/test/ui/parser/suggest_misplaced_generics/existing_generics.rs create mode 100644 src/test/ui/parser/suggest_misplaced_generics/existing_generics.stderr create mode 100644 src/test/ui/parser/suggest_misplaced_generics/fn-complex-generics.fixed create mode 100644 src/test/ui/parser/suggest_misplaced_generics/fn-complex-generics.rs create mode 100644 src/test/ui/parser/suggest_misplaced_generics/fn-complex-generics.stderr create mode 100644 src/test/ui/parser/suggest_misplaced_generics/fn-invalid-generics.rs create mode 100644 src/test/ui/parser/suggest_misplaced_generics/fn-invalid-generics.stderr create mode 100644 src/test/ui/parser/suggest_misplaced_generics/fn-simple.fixed create mode 100644 src/test/ui/parser/suggest_misplaced_generics/fn-simple.rs create mode 100644 src/test/ui/parser/suggest_misplaced_generics/fn-simple.stderr create mode 100644 src/test/ui/parser/suggest_misplaced_generics/struct.fixed create mode 100644 src/test/ui/parser/suggest_misplaced_generics/struct.rs create mode 100644 src/test/ui/parser/suggest_misplaced_generics/struct.stderr create mode 100644 src/test/ui/parser/suggest_misplaced_generics/trait.fixed create mode 100644 src/test/ui/parser/suggest_misplaced_generics/trait.rs create mode 100644 src/test/ui/parser/suggest_misplaced_generics/trait.stderr create mode 100644 src/test/ui/parser/suggest_misplaced_generics/type.fixed create mode 100644 src/test/ui/parser/suggest_misplaced_generics/type.rs create mode 100644 src/test/ui/parser/suggest_misplaced_generics/type.stderr diff --git a/compiler/rustc_parse/src/parser/diagnostics.rs b/compiler/rustc_parse/src/parser/diagnostics.rs index 4c918c6702ed9..6df9cfd3ff4e3 100644 --- a/compiler/rustc_parse/src/parser/diagnostics.rs +++ b/compiler/rustc_parse/src/parser/diagnostics.rs @@ -284,7 +284,7 @@ impl<'a> Parser<'a> { self.sess.source_map().span_to_snippet(span) } - pub(super) fn expected_ident_found(&self) -> DiagnosticBuilder<'a, ErrorGuaranteed> { + pub(super) fn expected_ident_found(&mut self) -> DiagnosticBuilder<'a, ErrorGuaranteed> { let valid_follow = &[ TokenKind::Eq, TokenKind::Colon, @@ -324,7 +324,60 @@ impl<'a> Parser<'a> { suggest_raw, suggest_remove_comma, }; - err.into_diagnostic(&self.sess.span_diagnostic) + let mut err = err.into_diagnostic(&self.sess.span_diagnostic); + + // if the token we have is a `<` + // it *might* be a misplaced generic + if self.token == token::Lt { + // all keywords that could have generic applied + let valid_prev_keywords = + [kw::Fn, kw::Type, kw::Struct, kw::Enum, kw::Union, kw::Trait]; + + // If we've expected an identifier, + // and the current token is a '<' + // if the previous token is a valid keyword + // that might use a generic, then suggest a correct + // generic placement (later on) + let maybe_keyword = self.prev_token.clone(); + if valid_prev_keywords.into_iter().any(|x| maybe_keyword.is_keyword(x)) { + // if we have a valid keyword, attempt to parse generics + // also obtain the keywords symbol + match self.parse_generics() { + Ok(generic) => { + if let TokenKind::Ident(symbol, _) = maybe_keyword.kind { + let ident_name = symbol.to_string(); + // at this point, we've found something like + // `fn id` + // and current token should be Ident with the item name (i.e. the function name) + // if there is a `<` after the fn name, then don't show a suggestion, show help + + if !self.look_ahead(1, |t| *t == token::Lt) && + let Ok(snippet) = self.sess.source_map().span_to_snippet(generic.span) && + let Ok(ident) = self.sess.source_map().span_to_snippet(self.token.span) { + err.span_suggestion_verbose( + generic.span.to(self.token.span), + format!("place the generic parameter name after the {ident_name} name"), + format!(" {ident}{snippet}"), + Applicability::MachineApplicable, + ); + } else { + err.help(format!( + "place the generic parameter name after the {ident_name} name" + )); + } + } + } + Err(err) => { + // if there's an error parsing the generics, + // then don't do a misplaced generics suggestion + // and emit the expected ident error instead; + err.cancel(); + } + } + } + } + + err } pub(super) fn expected_one_of_not_found( diff --git a/src/test/ui/parser/suggest_misplaced_generics/enum.fixed b/src/test/ui/parser/suggest_misplaced_generics/enum.fixed new file mode 100644 index 0000000000000..a9d3e9f86d09c --- /dev/null +++ b/src/test/ui/parser/suggest_misplaced_generics/enum.fixed @@ -0,0 +1,10 @@ +// Issue: 103366 , Suggest fix for misplaced generic params +// run-rustfix + +#[allow(unused)] +enum Foo { Variant(T) } +//~^ ERROR expected identifier, found `<` +//~| HELP place the generic parameter name after the enum name +//~| SUGGESTION Foo + +fn main() {} diff --git a/src/test/ui/parser/suggest_misplaced_generics/enum.rs b/src/test/ui/parser/suggest_misplaced_generics/enum.rs new file mode 100644 index 0000000000000..2d216ba53cc72 --- /dev/null +++ b/src/test/ui/parser/suggest_misplaced_generics/enum.rs @@ -0,0 +1,10 @@ +// Issue: 103366 , Suggest fix for misplaced generic params +// run-rustfix + +#[allow(unused)] +enum Foo { Variant(T) } +//~^ ERROR expected identifier, found `<` +//~| HELP place the generic parameter name after the enum name +//~| SUGGESTION Foo + +fn main() {} diff --git a/src/test/ui/parser/suggest_misplaced_generics/enum.stderr b/src/test/ui/parser/suggest_misplaced_generics/enum.stderr new file mode 100644 index 0000000000000..521cee4f72898 --- /dev/null +++ b/src/test/ui/parser/suggest_misplaced_generics/enum.stderr @@ -0,0 +1,13 @@ +error: expected identifier, found `<` + --> $DIR/enum.rs:5:5 + | +LL | enum Foo { Variant(T) } + | ^ expected identifier + | +help: place the generic parameter name after the enum name + | +LL | enum Foo { Variant(T) } + | ~~~~~~ + +error: aborting due to previous error + diff --git a/src/test/ui/parser/suggest_misplaced_generics/existing_generics.rs b/src/test/ui/parser/suggest_misplaced_generics/existing_generics.rs new file mode 100644 index 0000000000000..1dc182398d80a --- /dev/null +++ b/src/test/ui/parser/suggest_misplaced_generics/existing_generics.rs @@ -0,0 +1,9 @@ +// Issue: 103366 +// there is already an existing generic on f, so don't show a suggestion + +#[allow(unused)] +fn<'a, B: 'a + std::ops::Add> f(_x: B) { } +//~^ ERROR expected identifier, found `<` +//~| HELP place the generic parameter name after the fn name + +fn main() {} diff --git a/src/test/ui/parser/suggest_misplaced_generics/existing_generics.stderr b/src/test/ui/parser/suggest_misplaced_generics/existing_generics.stderr new file mode 100644 index 0000000000000..89716e6f1ed0a --- /dev/null +++ b/src/test/ui/parser/suggest_misplaced_generics/existing_generics.stderr @@ -0,0 +1,10 @@ +error: expected identifier, found `<` + --> $DIR/existing_generics.rs:5:3 + | +LL | fn<'a, B: 'a + std::ops::Add> f(_x: B) { } + | ^ expected identifier + | + = help: place the generic parameter name after the fn name + +error: aborting due to previous error + diff --git a/src/test/ui/parser/suggest_misplaced_generics/fn-complex-generics.fixed b/src/test/ui/parser/suggest_misplaced_generics/fn-complex-generics.fixed new file mode 100644 index 0000000000000..06947e098ee6a --- /dev/null +++ b/src/test/ui/parser/suggest_misplaced_generics/fn-complex-generics.fixed @@ -0,0 +1,10 @@ +// Issue: 103366 , Suggest fix for misplaced generic params +// run-rustfix + +#[allow(unused)] +fn f<'a, B: 'a + std::ops::Add>(_x: B) { } +//~^ ERROR expected identifier, found `<` +//~| HELP place the generic parameter name after the fn name +//~| SUGGESTION f<'a, B: 'a + std::ops::Add> + +fn main() {} diff --git a/src/test/ui/parser/suggest_misplaced_generics/fn-complex-generics.rs b/src/test/ui/parser/suggest_misplaced_generics/fn-complex-generics.rs new file mode 100644 index 0000000000000..cefce8d08806d --- /dev/null +++ b/src/test/ui/parser/suggest_misplaced_generics/fn-complex-generics.rs @@ -0,0 +1,10 @@ +// Issue: 103366 , Suggest fix for misplaced generic params +// run-rustfix + +#[allow(unused)] +fn<'a, B: 'a + std::ops::Add> f(_x: B) { } +//~^ ERROR expected identifier, found `<` +//~| HELP place the generic parameter name after the fn name +//~| SUGGESTION f<'a, B: 'a + std::ops::Add> + +fn main() {} diff --git a/src/test/ui/parser/suggest_misplaced_generics/fn-complex-generics.stderr b/src/test/ui/parser/suggest_misplaced_generics/fn-complex-generics.stderr new file mode 100644 index 0000000000000..7d1b44c44944c --- /dev/null +++ b/src/test/ui/parser/suggest_misplaced_generics/fn-complex-generics.stderr @@ -0,0 +1,13 @@ +error: expected identifier, found `<` + --> $DIR/fn-complex-generics.rs:5:3 + | +LL | fn<'a, B: 'a + std::ops::Add> f(_x: B) { } + | ^ expected identifier + | +help: place the generic parameter name after the fn name + | +LL | fn f<'a, B: 'a + std::ops::Add>(_x: B) { } + | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +error: aborting due to previous error + diff --git a/src/test/ui/parser/suggest_misplaced_generics/fn-invalid-generics.rs b/src/test/ui/parser/suggest_misplaced_generics/fn-invalid-generics.rs new file mode 100644 index 0000000000000..7fcb6a82ce451 --- /dev/null +++ b/src/test/ui/parser/suggest_misplaced_generics/fn-invalid-generics.rs @@ -0,0 +1,8 @@ +// Issue: 103366 , Suggest fix for misplaced generic params +// The generics fail to parse here, so don't make any suggestions/help + +#[allow(unused)] +fn<~>()> id(x: T) -> T { x } +//~^ ERROR expected identifier, found `<` + +fn main() {} diff --git a/src/test/ui/parser/suggest_misplaced_generics/fn-invalid-generics.stderr b/src/test/ui/parser/suggest_misplaced_generics/fn-invalid-generics.stderr new file mode 100644 index 0000000000000..47e12016938d8 --- /dev/null +++ b/src/test/ui/parser/suggest_misplaced_generics/fn-invalid-generics.stderr @@ -0,0 +1,8 @@ +error: expected identifier, found `<` + --> $DIR/fn-invalid-generics.rs:5:3 + | +LL | fn<~>()> id(x: T) -> T { x } + | ^ expected identifier + +error: aborting due to previous error + diff --git a/src/test/ui/parser/suggest_misplaced_generics/fn-simple.fixed b/src/test/ui/parser/suggest_misplaced_generics/fn-simple.fixed new file mode 100644 index 0000000000000..31c5429b16b05 --- /dev/null +++ b/src/test/ui/parser/suggest_misplaced_generics/fn-simple.fixed @@ -0,0 +1,10 @@ +// Issue: 103366 , Suggest fix for misplaced generic params +// run-rustfix + +#[allow(unused)] +fn id(x: T) -> T { x } +//~^ ERROR expected identifier, found `<` +//~| HELP place the generic parameter name after the fn name +//~| SUGGESTION id + +fn main() {} diff --git a/src/test/ui/parser/suggest_misplaced_generics/fn-simple.rs b/src/test/ui/parser/suggest_misplaced_generics/fn-simple.rs new file mode 100644 index 0000000000000..0a466184e996f --- /dev/null +++ b/src/test/ui/parser/suggest_misplaced_generics/fn-simple.rs @@ -0,0 +1,10 @@ +// Issue: 103366 , Suggest fix for misplaced generic params +// run-rustfix + +#[allow(unused)] +fn id(x: T) -> T { x } +//~^ ERROR expected identifier, found `<` +//~| HELP place the generic parameter name after the fn name +//~| SUGGESTION id + +fn main() {} diff --git a/src/test/ui/parser/suggest_misplaced_generics/fn-simple.stderr b/src/test/ui/parser/suggest_misplaced_generics/fn-simple.stderr new file mode 100644 index 0000000000000..40c4581e513ad --- /dev/null +++ b/src/test/ui/parser/suggest_misplaced_generics/fn-simple.stderr @@ -0,0 +1,13 @@ +error: expected identifier, found `<` + --> $DIR/fn-simple.rs:5:3 + | +LL | fn id(x: T) -> T { x } + | ^ expected identifier + | +help: place the generic parameter name after the fn name + | +LL | fn id(x: T) -> T { x } + | ~~~~~ + +error: aborting due to previous error + diff --git a/src/test/ui/parser/suggest_misplaced_generics/struct.fixed b/src/test/ui/parser/suggest_misplaced_generics/struct.fixed new file mode 100644 index 0000000000000..8627699a83084 --- /dev/null +++ b/src/test/ui/parser/suggest_misplaced_generics/struct.fixed @@ -0,0 +1,10 @@ +// Issue: 103366 , Suggest fix for misplaced generic params +// run-rustfix + +#[allow(unused)] +struct Foo { x: T } +//~^ ERROR expected identifier, found `<` +//~| HELP place the generic parameter name after the struct name +//~| SUGGESTION Foo + +fn main() {} diff --git a/src/test/ui/parser/suggest_misplaced_generics/struct.rs b/src/test/ui/parser/suggest_misplaced_generics/struct.rs new file mode 100644 index 0000000000000..15646b06cfc62 --- /dev/null +++ b/src/test/ui/parser/suggest_misplaced_generics/struct.rs @@ -0,0 +1,10 @@ +// Issue: 103366 , Suggest fix for misplaced generic params +// run-rustfix + +#[allow(unused)] +struct Foo { x: T } +//~^ ERROR expected identifier, found `<` +//~| HELP place the generic parameter name after the struct name +//~| SUGGESTION Foo + +fn main() {} diff --git a/src/test/ui/parser/suggest_misplaced_generics/struct.stderr b/src/test/ui/parser/suggest_misplaced_generics/struct.stderr new file mode 100644 index 0000000000000..ab17ee57e0bcd --- /dev/null +++ b/src/test/ui/parser/suggest_misplaced_generics/struct.stderr @@ -0,0 +1,13 @@ +error: expected identifier, found `<` + --> $DIR/struct.rs:5:7 + | +LL | struct Foo { x: T } + | ^ expected identifier + | +help: place the generic parameter name after the struct name + | +LL | struct Foo { x: T } + | ~~~~~~ + +error: aborting due to previous error + diff --git a/src/test/ui/parser/suggest_misplaced_generics/trait.fixed b/src/test/ui/parser/suggest_misplaced_generics/trait.fixed new file mode 100644 index 0000000000000..31ebf1f088fc7 --- /dev/null +++ b/src/test/ui/parser/suggest_misplaced_generics/trait.fixed @@ -0,0 +1,12 @@ +// Issue: 103366 , Suggest fix for misplaced generic params +// run-rustfix + +#[allow(unused)] +trait Foo { + //~^ ERROR expected identifier, found `<` + //~| HELP place the generic parameter name after the trait name + //~| SUGGESTION Foo +} + + +fn main() {} diff --git a/src/test/ui/parser/suggest_misplaced_generics/trait.rs b/src/test/ui/parser/suggest_misplaced_generics/trait.rs new file mode 100644 index 0000000000000..81b6abbd66163 --- /dev/null +++ b/src/test/ui/parser/suggest_misplaced_generics/trait.rs @@ -0,0 +1,12 @@ +// Issue: 103366 , Suggest fix for misplaced generic params +// run-rustfix + +#[allow(unused)] +trait Foo { + //~^ ERROR expected identifier, found `<` + //~| HELP place the generic parameter name after the trait name + //~| SUGGESTION Foo +} + + +fn main() {} diff --git a/src/test/ui/parser/suggest_misplaced_generics/trait.stderr b/src/test/ui/parser/suggest_misplaced_generics/trait.stderr new file mode 100644 index 0000000000000..069683bda1be3 --- /dev/null +++ b/src/test/ui/parser/suggest_misplaced_generics/trait.stderr @@ -0,0 +1,13 @@ +error: expected identifier, found `<` + --> $DIR/trait.rs:5:6 + | +LL | trait Foo { + | ^ expected identifier + | +help: place the generic parameter name after the trait name + | +LL | trait Foo { + | ~~~~~~ + +error: aborting due to previous error + diff --git a/src/test/ui/parser/suggest_misplaced_generics/type.fixed b/src/test/ui/parser/suggest_misplaced_generics/type.fixed new file mode 100644 index 0000000000000..b04003b803d1c --- /dev/null +++ b/src/test/ui/parser/suggest_misplaced_generics/type.fixed @@ -0,0 +1,10 @@ +// Issue: 103366 , Suggest fix for misplaced generic params +// run-rustfix + +#[allow(unused)] +type Foo = T; +//~^ ERROR expected identifier, found `<` +//~| HELP place the generic parameter name after the type name +//~| SUGGESTION Foo + +fn main() {} diff --git a/src/test/ui/parser/suggest_misplaced_generics/type.rs b/src/test/ui/parser/suggest_misplaced_generics/type.rs new file mode 100644 index 0000000000000..2d759a8b1ab61 --- /dev/null +++ b/src/test/ui/parser/suggest_misplaced_generics/type.rs @@ -0,0 +1,10 @@ +// Issue: 103366 , Suggest fix for misplaced generic params +// run-rustfix + +#[allow(unused)] +type Foo = T; +//~^ ERROR expected identifier, found `<` +//~| HELP place the generic parameter name after the type name +//~| SUGGESTION Foo + +fn main() {} diff --git a/src/test/ui/parser/suggest_misplaced_generics/type.stderr b/src/test/ui/parser/suggest_misplaced_generics/type.stderr new file mode 100644 index 0000000000000..a2832965c6d0e --- /dev/null +++ b/src/test/ui/parser/suggest_misplaced_generics/type.stderr @@ -0,0 +1,13 @@ +error: expected identifier, found `<` + --> $DIR/type.rs:5:5 + | +LL | type Foo = T; + | ^ expected identifier + | +help: place the generic parameter name after the type name + | +LL | type Foo = T; + | ~~~~~~ + +error: aborting due to previous error + From 5287004aa4f9b0685197cc0c009237812fed7047 Mon Sep 17 00:00:00 2001 From: Shrey Sudhir Date: Fri, 2 Dec 2022 00:40:33 +1100 Subject: [PATCH 05/21] Apply automatic suggestions from code review Co-authored-by: Takayuki Maeda --- compiler/rustc_parse/src/parser/diagnostics.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/compiler/rustc_parse/src/parser/diagnostics.rs b/compiler/rustc_parse/src/parser/diagnostics.rs index 6df9cfd3ff4e3..1e1e804c0d30a 100644 --- a/compiler/rustc_parse/src/parser/diagnostics.rs +++ b/compiler/rustc_parse/src/parser/diagnostics.rs @@ -345,7 +345,7 @@ impl<'a> Parser<'a> { match self.parse_generics() { Ok(generic) => { if let TokenKind::Ident(symbol, _) = maybe_keyword.kind { - let ident_name = symbol.to_string(); + let ident_name = symbol; // at this point, we've found something like // `fn id` // and current token should be Ident with the item name (i.e. the function name) @@ -355,9 +355,9 @@ impl<'a> Parser<'a> { let Ok(snippet) = self.sess.source_map().span_to_snippet(generic.span) && let Ok(ident) = self.sess.source_map().span_to_snippet(self.token.span) { err.span_suggestion_verbose( - generic.span.to(self.token.span), + self.token.span.shrink_to_hi(), format!("place the generic parameter name after the {ident_name} name"), - format!(" {ident}{snippet}"), + snippet, Applicability::MachineApplicable, ); } else { From 655beb4ece8a116c664ae63f26811ba75aa9e0e7 Mon Sep 17 00:00:00 2001 From: Shrey Sudhir Date: Thu, 1 Dec 2022 14:12:33 +0000 Subject: [PATCH 06/21] Attempt to address review comments via github web... --- compiler/rustc_parse/src/parser/diagnostics.rs | 5 ++--- src/test/ui/parser/suggest_misplaced_generics/enum.stderr | 2 +- .../suggest_misplaced_generics/fn-complex-generics.stderr | 2 +- .../ui/parser/suggest_misplaced_generics/fn-simple.stderr | 2 +- src/test/ui/parser/suggest_misplaced_generics/struct.stderr | 2 +- src/test/ui/parser/suggest_misplaced_generics/trait.stderr | 2 +- src/test/ui/parser/suggest_misplaced_generics/type.stderr | 2 +- 7 files changed, 8 insertions(+), 9 deletions(-) diff --git a/compiler/rustc_parse/src/parser/diagnostics.rs b/compiler/rustc_parse/src/parser/diagnostics.rs index 1e1e804c0d30a..94bedc07ba155 100644 --- a/compiler/rustc_parse/src/parser/diagnostics.rs +++ b/compiler/rustc_parse/src/parser/diagnostics.rs @@ -352,13 +352,12 @@ impl<'a> Parser<'a> { // if there is a `<` after the fn name, then don't show a suggestion, show help if !self.look_ahead(1, |t| *t == token::Lt) && - let Ok(snippet) = self.sess.source_map().span_to_snippet(generic.span) && - let Ok(ident) = self.sess.source_map().span_to_snippet(self.token.span) { + let Ok(snippet) = self.sess.source_map().span_to_snippet(generic.span) { err.span_suggestion_verbose( self.token.span.shrink_to_hi(), format!("place the generic parameter name after the {ident_name} name"), snippet, - Applicability::MachineApplicable, + Applicability::MaybeIncorrect, ); } else { err.help(format!( diff --git a/src/test/ui/parser/suggest_misplaced_generics/enum.stderr b/src/test/ui/parser/suggest_misplaced_generics/enum.stderr index 521cee4f72898..8af94856a4a48 100644 --- a/src/test/ui/parser/suggest_misplaced_generics/enum.stderr +++ b/src/test/ui/parser/suggest_misplaced_generics/enum.stderr @@ -7,7 +7,7 @@ LL | enum Foo { Variant(T) } help: place the generic parameter name after the enum name | LL | enum Foo { Variant(T) } - | ~~~~~~ + | ~~~ error: aborting due to previous error diff --git a/src/test/ui/parser/suggest_misplaced_generics/fn-complex-generics.stderr b/src/test/ui/parser/suggest_misplaced_generics/fn-complex-generics.stderr index 7d1b44c44944c..196769cb2b569 100644 --- a/src/test/ui/parser/suggest_misplaced_generics/fn-complex-generics.stderr +++ b/src/test/ui/parser/suggest_misplaced_generics/fn-complex-generics.stderr @@ -7,7 +7,7 @@ LL | fn<'a, B: 'a + std::ops::Add> f(_x: B) { } help: place the generic parameter name after the fn name | LL | fn f<'a, B: 'a + std::ops::Add>(_x: B) { } - | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ error: aborting due to previous error diff --git a/src/test/ui/parser/suggest_misplaced_generics/fn-simple.stderr b/src/test/ui/parser/suggest_misplaced_generics/fn-simple.stderr index 40c4581e513ad..0d09d8967b849 100644 --- a/src/test/ui/parser/suggest_misplaced_generics/fn-simple.stderr +++ b/src/test/ui/parser/suggest_misplaced_generics/fn-simple.stderr @@ -7,7 +7,7 @@ LL | fn id(x: T) -> T { x } help: place the generic parameter name after the fn name | LL | fn id(x: T) -> T { x } - | ~~~~~ + | ~~~ error: aborting due to previous error diff --git a/src/test/ui/parser/suggest_misplaced_generics/struct.stderr b/src/test/ui/parser/suggest_misplaced_generics/struct.stderr index ab17ee57e0bcd..32ffdb5e9c308 100644 --- a/src/test/ui/parser/suggest_misplaced_generics/struct.stderr +++ b/src/test/ui/parser/suggest_misplaced_generics/struct.stderr @@ -7,7 +7,7 @@ LL | struct Foo { x: T } help: place the generic parameter name after the struct name | LL | struct Foo { x: T } - | ~~~~~~ + | ~~~ error: aborting due to previous error diff --git a/src/test/ui/parser/suggest_misplaced_generics/trait.stderr b/src/test/ui/parser/suggest_misplaced_generics/trait.stderr index 069683bda1be3..01a31b7a85a9d 100644 --- a/src/test/ui/parser/suggest_misplaced_generics/trait.stderr +++ b/src/test/ui/parser/suggest_misplaced_generics/trait.stderr @@ -7,7 +7,7 @@ LL | trait Foo { help: place the generic parameter name after the trait name | LL | trait Foo { - | ~~~~~~ + | ~~~ error: aborting due to previous error diff --git a/src/test/ui/parser/suggest_misplaced_generics/type.stderr b/src/test/ui/parser/suggest_misplaced_generics/type.stderr index a2832965c6d0e..1ae73fae7f926 100644 --- a/src/test/ui/parser/suggest_misplaced_generics/type.stderr +++ b/src/test/ui/parser/suggest_misplaced_generics/type.stderr @@ -7,7 +7,7 @@ LL | type Foo = T; help: place the generic parameter name after the type name | LL | type Foo = T; - | ~~~~~~ + | ~~~ error: aborting due to previous error From 4447949e400822a02cc9945fc39f06842e6b9439 Mon Sep 17 00:00:00 2001 From: SpanishPear Date: Sun, 22 Jan 2023 16:45:56 +1100 Subject: [PATCH 07/21] revert to previous span --- compiler/rustc_parse/src/parser/diagnostics.rs | 7 ++++--- src/test/ui/parser/suggest_misplaced_generics/enum.stderr | 2 +- .../suggest_misplaced_generics/fn-complex-generics.stderr | 2 +- .../ui/parser/suggest_misplaced_generics/fn-simple.stderr | 2 +- .../ui/parser/suggest_misplaced_generics/struct.stderr | 2 +- src/test/ui/parser/suggest_misplaced_generics/trait.stderr | 2 +- src/test/ui/parser/suggest_misplaced_generics/type.stderr | 2 +- 7 files changed, 10 insertions(+), 9 deletions(-) diff --git a/compiler/rustc_parse/src/parser/diagnostics.rs b/compiler/rustc_parse/src/parser/diagnostics.rs index 94bedc07ba155..9ac3bb946dc42 100644 --- a/compiler/rustc_parse/src/parser/diagnostics.rs +++ b/compiler/rustc_parse/src/parser/diagnostics.rs @@ -352,11 +352,12 @@ impl<'a> Parser<'a> { // if there is a `<` after the fn name, then don't show a suggestion, show help if !self.look_ahead(1, |t| *t == token::Lt) && - let Ok(snippet) = self.sess.source_map().span_to_snippet(generic.span) { + let Ok(snippet) = self.sess.source_map().span_to_snippet(generic.span) && + let Ok(ident) = self.sess.source_map().span_to_snippet(self.token.span) { err.span_suggestion_verbose( - self.token.span.shrink_to_hi(), + generic.span.to(self.token.span), format!("place the generic parameter name after the {ident_name} name"), - snippet, + format!(" {ident}{snippet}"), Applicability::MaybeIncorrect, ); } else { diff --git a/src/test/ui/parser/suggest_misplaced_generics/enum.stderr b/src/test/ui/parser/suggest_misplaced_generics/enum.stderr index 8af94856a4a48..521cee4f72898 100644 --- a/src/test/ui/parser/suggest_misplaced_generics/enum.stderr +++ b/src/test/ui/parser/suggest_misplaced_generics/enum.stderr @@ -7,7 +7,7 @@ LL | enum Foo { Variant(T) } help: place the generic parameter name after the enum name | LL | enum Foo { Variant(T) } - | ~~~ + | ~~~~~~ error: aborting due to previous error diff --git a/src/test/ui/parser/suggest_misplaced_generics/fn-complex-generics.stderr b/src/test/ui/parser/suggest_misplaced_generics/fn-complex-generics.stderr index 196769cb2b569..7d1b44c44944c 100644 --- a/src/test/ui/parser/suggest_misplaced_generics/fn-complex-generics.stderr +++ b/src/test/ui/parser/suggest_misplaced_generics/fn-complex-generics.stderr @@ -7,7 +7,7 @@ LL | fn<'a, B: 'a + std::ops::Add> f(_x: B) { } help: place the generic parameter name after the fn name | LL | fn f<'a, B: 'a + std::ops::Add>(_x: B) { } - | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ error: aborting due to previous error diff --git a/src/test/ui/parser/suggest_misplaced_generics/fn-simple.stderr b/src/test/ui/parser/suggest_misplaced_generics/fn-simple.stderr index 0d09d8967b849..40c4581e513ad 100644 --- a/src/test/ui/parser/suggest_misplaced_generics/fn-simple.stderr +++ b/src/test/ui/parser/suggest_misplaced_generics/fn-simple.stderr @@ -7,7 +7,7 @@ LL | fn id(x: T) -> T { x } help: place the generic parameter name after the fn name | LL | fn id(x: T) -> T { x } - | ~~~ + | ~~~~~ error: aborting due to previous error diff --git a/src/test/ui/parser/suggest_misplaced_generics/struct.stderr b/src/test/ui/parser/suggest_misplaced_generics/struct.stderr index 32ffdb5e9c308..ab17ee57e0bcd 100644 --- a/src/test/ui/parser/suggest_misplaced_generics/struct.stderr +++ b/src/test/ui/parser/suggest_misplaced_generics/struct.stderr @@ -7,7 +7,7 @@ LL | struct Foo { x: T } help: place the generic parameter name after the struct name | LL | struct Foo { x: T } - | ~~~ + | ~~~~~~ error: aborting due to previous error diff --git a/src/test/ui/parser/suggest_misplaced_generics/trait.stderr b/src/test/ui/parser/suggest_misplaced_generics/trait.stderr index 01a31b7a85a9d..069683bda1be3 100644 --- a/src/test/ui/parser/suggest_misplaced_generics/trait.stderr +++ b/src/test/ui/parser/suggest_misplaced_generics/trait.stderr @@ -7,7 +7,7 @@ LL | trait Foo { help: place the generic parameter name after the trait name | LL | trait Foo { - | ~~~ + | ~~~~~~ error: aborting due to previous error diff --git a/src/test/ui/parser/suggest_misplaced_generics/type.stderr b/src/test/ui/parser/suggest_misplaced_generics/type.stderr index 1ae73fae7f926..a2832965c6d0e 100644 --- a/src/test/ui/parser/suggest_misplaced_generics/type.stderr +++ b/src/test/ui/parser/suggest_misplaced_generics/type.stderr @@ -7,7 +7,7 @@ LL | type Foo = T; help: place the generic parameter name after the type name | LL | type Foo = T; - | ~~~ + | ~~~~~~ error: aborting due to previous error From 8292d07cc4c9a070a9de808620bb79bab6935f70 Mon Sep 17 00:00:00 2001 From: SpanishPear Date: Sun, 22 Jan 2023 17:16:39 +1100 Subject: [PATCH 08/21] move tests to new rust-lang location --- .../ui/parser/suggest_misplaced_generics/enum.fixed | 0 {src/test => tests}/ui/parser/suggest_misplaced_generics/enum.rs | 0 .../ui/parser/suggest_misplaced_generics/enum.stderr | 0 .../ui/parser/suggest_misplaced_generics/existing_generics.rs | 0 .../ui/parser/suggest_misplaced_generics/existing_generics.stderr | 0 .../parser/suggest_misplaced_generics/fn-complex-generics.fixed | 0 .../ui/parser/suggest_misplaced_generics/fn-complex-generics.rs | 0 .../parser/suggest_misplaced_generics/fn-complex-generics.stderr | 0 .../ui/parser/suggest_misplaced_generics/fn-invalid-generics.rs | 0 .../parser/suggest_misplaced_generics/fn-invalid-generics.stderr | 0 .../ui/parser/suggest_misplaced_generics/fn-simple.fixed | 0 .../ui/parser/suggest_misplaced_generics/fn-simple.rs | 0 .../ui/parser/suggest_misplaced_generics/fn-simple.stderr | 0 .../ui/parser/suggest_misplaced_generics/struct.fixed | 0 .../test => tests}/ui/parser/suggest_misplaced_generics/struct.rs | 0 .../ui/parser/suggest_misplaced_generics/struct.stderr | 0 .../ui/parser/suggest_misplaced_generics/trait.fixed | 0 {src/test => tests}/ui/parser/suggest_misplaced_generics/trait.rs | 0 .../ui/parser/suggest_misplaced_generics/trait.stderr | 0 .../ui/parser/suggest_misplaced_generics/type.fixed | 0 {src/test => tests}/ui/parser/suggest_misplaced_generics/type.rs | 0 .../ui/parser/suggest_misplaced_generics/type.stderr | 0 22 files changed, 0 insertions(+), 0 deletions(-) rename {src/test => tests}/ui/parser/suggest_misplaced_generics/enum.fixed (100%) rename {src/test => tests}/ui/parser/suggest_misplaced_generics/enum.rs (100%) rename {src/test => tests}/ui/parser/suggest_misplaced_generics/enum.stderr (100%) rename {src/test => tests}/ui/parser/suggest_misplaced_generics/existing_generics.rs (100%) rename {src/test => tests}/ui/parser/suggest_misplaced_generics/existing_generics.stderr (100%) rename {src/test => tests}/ui/parser/suggest_misplaced_generics/fn-complex-generics.fixed (100%) rename {src/test => tests}/ui/parser/suggest_misplaced_generics/fn-complex-generics.rs (100%) rename {src/test => tests}/ui/parser/suggest_misplaced_generics/fn-complex-generics.stderr (100%) rename {src/test => tests}/ui/parser/suggest_misplaced_generics/fn-invalid-generics.rs (100%) rename {src/test => tests}/ui/parser/suggest_misplaced_generics/fn-invalid-generics.stderr (100%) rename {src/test => tests}/ui/parser/suggest_misplaced_generics/fn-simple.fixed (100%) rename {src/test => tests}/ui/parser/suggest_misplaced_generics/fn-simple.rs (100%) rename {src/test => tests}/ui/parser/suggest_misplaced_generics/fn-simple.stderr (100%) rename {src/test => tests}/ui/parser/suggest_misplaced_generics/struct.fixed (100%) rename {src/test => tests}/ui/parser/suggest_misplaced_generics/struct.rs (100%) rename {src/test => tests}/ui/parser/suggest_misplaced_generics/struct.stderr (100%) rename {src/test => tests}/ui/parser/suggest_misplaced_generics/trait.fixed (100%) rename {src/test => tests}/ui/parser/suggest_misplaced_generics/trait.rs (100%) rename {src/test => tests}/ui/parser/suggest_misplaced_generics/trait.stderr (100%) rename {src/test => tests}/ui/parser/suggest_misplaced_generics/type.fixed (100%) rename {src/test => tests}/ui/parser/suggest_misplaced_generics/type.rs (100%) rename {src/test => tests}/ui/parser/suggest_misplaced_generics/type.stderr (100%) diff --git a/src/test/ui/parser/suggest_misplaced_generics/enum.fixed b/tests/ui/parser/suggest_misplaced_generics/enum.fixed similarity index 100% rename from src/test/ui/parser/suggest_misplaced_generics/enum.fixed rename to tests/ui/parser/suggest_misplaced_generics/enum.fixed diff --git a/src/test/ui/parser/suggest_misplaced_generics/enum.rs b/tests/ui/parser/suggest_misplaced_generics/enum.rs similarity index 100% rename from src/test/ui/parser/suggest_misplaced_generics/enum.rs rename to tests/ui/parser/suggest_misplaced_generics/enum.rs diff --git a/src/test/ui/parser/suggest_misplaced_generics/enum.stderr b/tests/ui/parser/suggest_misplaced_generics/enum.stderr similarity index 100% rename from src/test/ui/parser/suggest_misplaced_generics/enum.stderr rename to tests/ui/parser/suggest_misplaced_generics/enum.stderr diff --git a/src/test/ui/parser/suggest_misplaced_generics/existing_generics.rs b/tests/ui/parser/suggest_misplaced_generics/existing_generics.rs similarity index 100% rename from src/test/ui/parser/suggest_misplaced_generics/existing_generics.rs rename to tests/ui/parser/suggest_misplaced_generics/existing_generics.rs diff --git a/src/test/ui/parser/suggest_misplaced_generics/existing_generics.stderr b/tests/ui/parser/suggest_misplaced_generics/existing_generics.stderr similarity index 100% rename from src/test/ui/parser/suggest_misplaced_generics/existing_generics.stderr rename to tests/ui/parser/suggest_misplaced_generics/existing_generics.stderr diff --git a/src/test/ui/parser/suggest_misplaced_generics/fn-complex-generics.fixed b/tests/ui/parser/suggest_misplaced_generics/fn-complex-generics.fixed similarity index 100% rename from src/test/ui/parser/suggest_misplaced_generics/fn-complex-generics.fixed rename to tests/ui/parser/suggest_misplaced_generics/fn-complex-generics.fixed diff --git a/src/test/ui/parser/suggest_misplaced_generics/fn-complex-generics.rs b/tests/ui/parser/suggest_misplaced_generics/fn-complex-generics.rs similarity index 100% rename from src/test/ui/parser/suggest_misplaced_generics/fn-complex-generics.rs rename to tests/ui/parser/suggest_misplaced_generics/fn-complex-generics.rs diff --git a/src/test/ui/parser/suggest_misplaced_generics/fn-complex-generics.stderr b/tests/ui/parser/suggest_misplaced_generics/fn-complex-generics.stderr similarity index 100% rename from src/test/ui/parser/suggest_misplaced_generics/fn-complex-generics.stderr rename to tests/ui/parser/suggest_misplaced_generics/fn-complex-generics.stderr diff --git a/src/test/ui/parser/suggest_misplaced_generics/fn-invalid-generics.rs b/tests/ui/parser/suggest_misplaced_generics/fn-invalid-generics.rs similarity index 100% rename from src/test/ui/parser/suggest_misplaced_generics/fn-invalid-generics.rs rename to tests/ui/parser/suggest_misplaced_generics/fn-invalid-generics.rs diff --git a/src/test/ui/parser/suggest_misplaced_generics/fn-invalid-generics.stderr b/tests/ui/parser/suggest_misplaced_generics/fn-invalid-generics.stderr similarity index 100% rename from src/test/ui/parser/suggest_misplaced_generics/fn-invalid-generics.stderr rename to tests/ui/parser/suggest_misplaced_generics/fn-invalid-generics.stderr diff --git a/src/test/ui/parser/suggest_misplaced_generics/fn-simple.fixed b/tests/ui/parser/suggest_misplaced_generics/fn-simple.fixed similarity index 100% rename from src/test/ui/parser/suggest_misplaced_generics/fn-simple.fixed rename to tests/ui/parser/suggest_misplaced_generics/fn-simple.fixed diff --git a/src/test/ui/parser/suggest_misplaced_generics/fn-simple.rs b/tests/ui/parser/suggest_misplaced_generics/fn-simple.rs similarity index 100% rename from src/test/ui/parser/suggest_misplaced_generics/fn-simple.rs rename to tests/ui/parser/suggest_misplaced_generics/fn-simple.rs diff --git a/src/test/ui/parser/suggest_misplaced_generics/fn-simple.stderr b/tests/ui/parser/suggest_misplaced_generics/fn-simple.stderr similarity index 100% rename from src/test/ui/parser/suggest_misplaced_generics/fn-simple.stderr rename to tests/ui/parser/suggest_misplaced_generics/fn-simple.stderr diff --git a/src/test/ui/parser/suggest_misplaced_generics/struct.fixed b/tests/ui/parser/suggest_misplaced_generics/struct.fixed similarity index 100% rename from src/test/ui/parser/suggest_misplaced_generics/struct.fixed rename to tests/ui/parser/suggest_misplaced_generics/struct.fixed diff --git a/src/test/ui/parser/suggest_misplaced_generics/struct.rs b/tests/ui/parser/suggest_misplaced_generics/struct.rs similarity index 100% rename from src/test/ui/parser/suggest_misplaced_generics/struct.rs rename to tests/ui/parser/suggest_misplaced_generics/struct.rs diff --git a/src/test/ui/parser/suggest_misplaced_generics/struct.stderr b/tests/ui/parser/suggest_misplaced_generics/struct.stderr similarity index 100% rename from src/test/ui/parser/suggest_misplaced_generics/struct.stderr rename to tests/ui/parser/suggest_misplaced_generics/struct.stderr diff --git a/src/test/ui/parser/suggest_misplaced_generics/trait.fixed b/tests/ui/parser/suggest_misplaced_generics/trait.fixed similarity index 100% rename from src/test/ui/parser/suggest_misplaced_generics/trait.fixed rename to tests/ui/parser/suggest_misplaced_generics/trait.fixed diff --git a/src/test/ui/parser/suggest_misplaced_generics/trait.rs b/tests/ui/parser/suggest_misplaced_generics/trait.rs similarity index 100% rename from src/test/ui/parser/suggest_misplaced_generics/trait.rs rename to tests/ui/parser/suggest_misplaced_generics/trait.rs diff --git a/src/test/ui/parser/suggest_misplaced_generics/trait.stderr b/tests/ui/parser/suggest_misplaced_generics/trait.stderr similarity index 100% rename from src/test/ui/parser/suggest_misplaced_generics/trait.stderr rename to tests/ui/parser/suggest_misplaced_generics/trait.stderr diff --git a/src/test/ui/parser/suggest_misplaced_generics/type.fixed b/tests/ui/parser/suggest_misplaced_generics/type.fixed similarity index 100% rename from src/test/ui/parser/suggest_misplaced_generics/type.fixed rename to tests/ui/parser/suggest_misplaced_generics/type.fixed diff --git a/src/test/ui/parser/suggest_misplaced_generics/type.rs b/tests/ui/parser/suggest_misplaced_generics/type.rs similarity index 100% rename from src/test/ui/parser/suggest_misplaced_generics/type.rs rename to tests/ui/parser/suggest_misplaced_generics/type.rs diff --git a/src/test/ui/parser/suggest_misplaced_generics/type.stderr b/tests/ui/parser/suggest_misplaced_generics/type.stderr similarity index 100% rename from src/test/ui/parser/suggest_misplaced_generics/type.stderr rename to tests/ui/parser/suggest_misplaced_generics/type.stderr From 70bfcc2518dc431cf20cd7d088b954fa348f17d9 Mon Sep 17 00:00:00 2001 From: SpanishPear Date: Tue, 31 Jan 2023 21:44:11 +1100 Subject: [PATCH 09/21] move to multipart spans --- compiler/rustc_parse/src/parser/diagnostics.rs | 11 ++++++----- .../ui/parser/suggest_misplaced_generics/enum.stderr | 5 +++-- .../fn-complex-generics.stderr | 5 +++-- .../suggest_misplaced_generics/fn-simple.stderr | 5 +++-- .../parser/suggest_misplaced_generics/struct.stderr | 5 +++-- .../ui/parser/suggest_misplaced_generics/trait.stderr | 5 +++-- .../ui/parser/suggest_misplaced_generics/type.stderr | 5 +++-- 7 files changed, 24 insertions(+), 17 deletions(-) diff --git a/compiler/rustc_parse/src/parser/diagnostics.rs b/compiler/rustc_parse/src/parser/diagnostics.rs index 9ac3bb946dc42..1740f2c2c8455 100644 --- a/compiler/rustc_parse/src/parser/diagnostics.rs +++ b/compiler/rustc_parse/src/parser/diagnostics.rs @@ -352,12 +352,13 @@ impl<'a> Parser<'a> { // if there is a `<` after the fn name, then don't show a suggestion, show help if !self.look_ahead(1, |t| *t == token::Lt) && - let Ok(snippet) = self.sess.source_map().span_to_snippet(generic.span) && - let Ok(ident) = self.sess.source_map().span_to_snippet(self.token.span) { - err.span_suggestion_verbose( - generic.span.to(self.token.span), + let Ok(snippet) = self.sess.source_map().span_to_snippet(generic.span) { + err.multipart_suggestion_verbose( format!("place the generic parameter name after the {ident_name} name"), - format!(" {ident}{snippet}"), + vec![ + (self.token.span.shrink_to_hi(), snippet), + (generic.span, String::new()) + ], Applicability::MaybeIncorrect, ); } else { diff --git a/tests/ui/parser/suggest_misplaced_generics/enum.stderr b/tests/ui/parser/suggest_misplaced_generics/enum.stderr index 521cee4f72898..5f5947627ee5c 100644 --- a/tests/ui/parser/suggest_misplaced_generics/enum.stderr +++ b/tests/ui/parser/suggest_misplaced_generics/enum.stderr @@ -6,8 +6,9 @@ LL | enum Foo { Variant(T) } | help: place the generic parameter name after the enum name | -LL | enum Foo { Variant(T) } - | ~~~~~~ +LL - enum Foo { Variant(T) } +LL + enum Foo { Variant(T) } + | error: aborting due to previous error diff --git a/tests/ui/parser/suggest_misplaced_generics/fn-complex-generics.stderr b/tests/ui/parser/suggest_misplaced_generics/fn-complex-generics.stderr index 7d1b44c44944c..061d0910a742d 100644 --- a/tests/ui/parser/suggest_misplaced_generics/fn-complex-generics.stderr +++ b/tests/ui/parser/suggest_misplaced_generics/fn-complex-generics.stderr @@ -6,8 +6,9 @@ LL | fn<'a, B: 'a + std::ops::Add> f(_x: B) { } | help: place the generic parameter name after the fn name | -LL | fn f<'a, B: 'a + std::ops::Add>(_x: B) { } - | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +LL - fn<'a, B: 'a + std::ops::Add> f(_x: B) { } +LL + fn f<'a, B: 'a + std::ops::Add>(_x: B) { } + | error: aborting due to previous error diff --git a/tests/ui/parser/suggest_misplaced_generics/fn-simple.stderr b/tests/ui/parser/suggest_misplaced_generics/fn-simple.stderr index 40c4581e513ad..e749f1a0d00d6 100644 --- a/tests/ui/parser/suggest_misplaced_generics/fn-simple.stderr +++ b/tests/ui/parser/suggest_misplaced_generics/fn-simple.stderr @@ -6,8 +6,9 @@ LL | fn id(x: T) -> T { x } | help: place the generic parameter name after the fn name | -LL | fn id(x: T) -> T { x } - | ~~~~~ +LL - fn id(x: T) -> T { x } +LL + fn id(x: T) -> T { x } + | error: aborting due to previous error diff --git a/tests/ui/parser/suggest_misplaced_generics/struct.stderr b/tests/ui/parser/suggest_misplaced_generics/struct.stderr index ab17ee57e0bcd..2b650907092d1 100644 --- a/tests/ui/parser/suggest_misplaced_generics/struct.stderr +++ b/tests/ui/parser/suggest_misplaced_generics/struct.stderr @@ -6,8 +6,9 @@ LL | struct Foo { x: T } | help: place the generic parameter name after the struct name | -LL | struct Foo { x: T } - | ~~~~~~ +LL - struct Foo { x: T } +LL + struct Foo { x: T } + | error: aborting due to previous error diff --git a/tests/ui/parser/suggest_misplaced_generics/trait.stderr b/tests/ui/parser/suggest_misplaced_generics/trait.stderr index 069683bda1be3..ac86cfa469704 100644 --- a/tests/ui/parser/suggest_misplaced_generics/trait.stderr +++ b/tests/ui/parser/suggest_misplaced_generics/trait.stderr @@ -6,8 +6,9 @@ LL | trait Foo { | help: place the generic parameter name after the trait name | -LL | trait Foo { - | ~~~~~~ +LL - trait Foo { +LL + trait Foo { + | error: aborting due to previous error diff --git a/tests/ui/parser/suggest_misplaced_generics/type.stderr b/tests/ui/parser/suggest_misplaced_generics/type.stderr index a2832965c6d0e..22744f6cf37fb 100644 --- a/tests/ui/parser/suggest_misplaced_generics/type.stderr +++ b/tests/ui/parser/suggest_misplaced_generics/type.stderr @@ -6,8 +6,9 @@ LL | type Foo = T; | help: place the generic parameter name after the type name | -LL | type Foo = T; - | ~~~~~~ +LL - type Foo = T; +LL + type Foo = T; + | error: aborting due to previous error From a3d32bbbbe06ffe42edbc4905e964d394de5ee02 Mon Sep 17 00:00:00 2001 From: SpanishPear Date: Wed, 1 Feb 2023 18:11:37 +1100 Subject: [PATCH 10/21] fix formatting + test syntax --- compiler/rustc_parse/src/parser/diagnostics.rs | 2 +- tests/ui/parser/suggest_misplaced_generics/enum.fixed | 1 - tests/ui/parser/suggest_misplaced_generics/enum.rs | 1 - .../parser/suggest_misplaced_generics/fn-complex-generics.fixed | 1 - .../ui/parser/suggest_misplaced_generics/fn-complex-generics.rs | 1 - tests/ui/parser/suggest_misplaced_generics/fn-simple.fixed | 1 - tests/ui/parser/suggest_misplaced_generics/fn-simple.rs | 1 - tests/ui/parser/suggest_misplaced_generics/struct.fixed | 1 - tests/ui/parser/suggest_misplaced_generics/struct.rs | 1 - tests/ui/parser/suggest_misplaced_generics/trait.fixed | 1 - tests/ui/parser/suggest_misplaced_generics/trait.rs | 1 - tests/ui/parser/suggest_misplaced_generics/type.fixed | 1 - tests/ui/parser/suggest_misplaced_generics/type.rs | 1 - 13 files changed, 1 insertion(+), 13 deletions(-) diff --git a/compiler/rustc_parse/src/parser/diagnostics.rs b/compiler/rustc_parse/src/parser/diagnostics.rs index 1740f2c2c8455..2c6db485828bd 100644 --- a/compiler/rustc_parse/src/parser/diagnostics.rs +++ b/compiler/rustc_parse/src/parser/diagnostics.rs @@ -353,7 +353,7 @@ impl<'a> Parser<'a> { if !self.look_ahead(1, |t| *t == token::Lt) && let Ok(snippet) = self.sess.source_map().span_to_snippet(generic.span) { - err.multipart_suggestion_verbose( + err.multipart_suggestion_verbose( format!("place the generic parameter name after the {ident_name} name"), vec![ (self.token.span.shrink_to_hi(), snippet), diff --git a/tests/ui/parser/suggest_misplaced_generics/enum.fixed b/tests/ui/parser/suggest_misplaced_generics/enum.fixed index a9d3e9f86d09c..3332118a1e768 100644 --- a/tests/ui/parser/suggest_misplaced_generics/enum.fixed +++ b/tests/ui/parser/suggest_misplaced_generics/enum.fixed @@ -5,6 +5,5 @@ enum Foo { Variant(T) } //~^ ERROR expected identifier, found `<` //~| HELP place the generic parameter name after the enum name -//~| SUGGESTION Foo fn main() {} diff --git a/tests/ui/parser/suggest_misplaced_generics/enum.rs b/tests/ui/parser/suggest_misplaced_generics/enum.rs index 2d216ba53cc72..5a2289c5c5ae2 100644 --- a/tests/ui/parser/suggest_misplaced_generics/enum.rs +++ b/tests/ui/parser/suggest_misplaced_generics/enum.rs @@ -5,6 +5,5 @@ enum Foo { Variant(T) } //~^ ERROR expected identifier, found `<` //~| HELP place the generic parameter name after the enum name -//~| SUGGESTION Foo fn main() {} diff --git a/tests/ui/parser/suggest_misplaced_generics/fn-complex-generics.fixed b/tests/ui/parser/suggest_misplaced_generics/fn-complex-generics.fixed index 06947e098ee6a..84bf64bd63cf9 100644 --- a/tests/ui/parser/suggest_misplaced_generics/fn-complex-generics.fixed +++ b/tests/ui/parser/suggest_misplaced_generics/fn-complex-generics.fixed @@ -5,6 +5,5 @@ fn f<'a, B: 'a + std::ops::Add>(_x: B) { } //~^ ERROR expected identifier, found `<` //~| HELP place the generic parameter name after the fn name -//~| SUGGESTION f<'a, B: 'a + std::ops::Add> fn main() {} diff --git a/tests/ui/parser/suggest_misplaced_generics/fn-complex-generics.rs b/tests/ui/parser/suggest_misplaced_generics/fn-complex-generics.rs index cefce8d08806d..d0684397e744c 100644 --- a/tests/ui/parser/suggest_misplaced_generics/fn-complex-generics.rs +++ b/tests/ui/parser/suggest_misplaced_generics/fn-complex-generics.rs @@ -5,6 +5,5 @@ fn<'a, B: 'a + std::ops::Add> f(_x: B) { } //~^ ERROR expected identifier, found `<` //~| HELP place the generic parameter name after the fn name -//~| SUGGESTION f<'a, B: 'a + std::ops::Add> fn main() {} diff --git a/tests/ui/parser/suggest_misplaced_generics/fn-simple.fixed b/tests/ui/parser/suggest_misplaced_generics/fn-simple.fixed index 31c5429b16b05..cbfd5f2d39c08 100644 --- a/tests/ui/parser/suggest_misplaced_generics/fn-simple.fixed +++ b/tests/ui/parser/suggest_misplaced_generics/fn-simple.fixed @@ -5,6 +5,5 @@ fn id(x: T) -> T { x } //~^ ERROR expected identifier, found `<` //~| HELP place the generic parameter name after the fn name -//~| SUGGESTION id fn main() {} diff --git a/tests/ui/parser/suggest_misplaced_generics/fn-simple.rs b/tests/ui/parser/suggest_misplaced_generics/fn-simple.rs index 0a466184e996f..b207cf70d8584 100644 --- a/tests/ui/parser/suggest_misplaced_generics/fn-simple.rs +++ b/tests/ui/parser/suggest_misplaced_generics/fn-simple.rs @@ -5,6 +5,5 @@ fn id(x: T) -> T { x } //~^ ERROR expected identifier, found `<` //~| HELP place the generic parameter name after the fn name -//~| SUGGESTION id fn main() {} diff --git a/tests/ui/parser/suggest_misplaced_generics/struct.fixed b/tests/ui/parser/suggest_misplaced_generics/struct.fixed index 8627699a83084..fec05bdeca15c 100644 --- a/tests/ui/parser/suggest_misplaced_generics/struct.fixed +++ b/tests/ui/parser/suggest_misplaced_generics/struct.fixed @@ -5,6 +5,5 @@ struct Foo { x: T } //~^ ERROR expected identifier, found `<` //~| HELP place the generic parameter name after the struct name -//~| SUGGESTION Foo fn main() {} diff --git a/tests/ui/parser/suggest_misplaced_generics/struct.rs b/tests/ui/parser/suggest_misplaced_generics/struct.rs index 15646b06cfc62..6b80150d54656 100644 --- a/tests/ui/parser/suggest_misplaced_generics/struct.rs +++ b/tests/ui/parser/suggest_misplaced_generics/struct.rs @@ -5,6 +5,5 @@ struct Foo { x: T } //~^ ERROR expected identifier, found `<` //~| HELP place the generic parameter name after the struct name -//~| SUGGESTION Foo fn main() {} diff --git a/tests/ui/parser/suggest_misplaced_generics/trait.fixed b/tests/ui/parser/suggest_misplaced_generics/trait.fixed index 31ebf1f088fc7..a471a078af142 100644 --- a/tests/ui/parser/suggest_misplaced_generics/trait.fixed +++ b/tests/ui/parser/suggest_misplaced_generics/trait.fixed @@ -5,7 +5,6 @@ trait Foo { //~^ ERROR expected identifier, found `<` //~| HELP place the generic parameter name after the trait name - //~| SUGGESTION Foo } diff --git a/tests/ui/parser/suggest_misplaced_generics/trait.rs b/tests/ui/parser/suggest_misplaced_generics/trait.rs index 81b6abbd66163..55355f451f9fd 100644 --- a/tests/ui/parser/suggest_misplaced_generics/trait.rs +++ b/tests/ui/parser/suggest_misplaced_generics/trait.rs @@ -5,7 +5,6 @@ trait Foo { //~^ ERROR expected identifier, found `<` //~| HELP place the generic parameter name after the trait name - //~| SUGGESTION Foo } diff --git a/tests/ui/parser/suggest_misplaced_generics/type.fixed b/tests/ui/parser/suggest_misplaced_generics/type.fixed index b04003b803d1c..a97b9e66d0b2b 100644 --- a/tests/ui/parser/suggest_misplaced_generics/type.fixed +++ b/tests/ui/parser/suggest_misplaced_generics/type.fixed @@ -5,6 +5,5 @@ type Foo = T; //~^ ERROR expected identifier, found `<` //~| HELP place the generic parameter name after the type name -//~| SUGGESTION Foo fn main() {} diff --git a/tests/ui/parser/suggest_misplaced_generics/type.rs b/tests/ui/parser/suggest_misplaced_generics/type.rs index 2d759a8b1ab61..17e200536fa3e 100644 --- a/tests/ui/parser/suggest_misplaced_generics/type.rs +++ b/tests/ui/parser/suggest_misplaced_generics/type.rs @@ -5,6 +5,5 @@ type Foo = T; //~^ ERROR expected identifier, found `<` //~| HELP place the generic parameter name after the type name -//~| SUGGESTION Foo fn main() {} From 0d59b8c997e31095732c9f9864e10d76daaeb42e Mon Sep 17 00:00:00 2001 From: Camille GILLOT Date: Sat, 21 Jan 2023 21:52:40 +0000 Subject: [PATCH 11/21] Remove redundant test. --- compiler/rustc_mir_dataflow/src/value_analysis.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/compiler/rustc_mir_dataflow/src/value_analysis.rs b/compiler/rustc_mir_dataflow/src/value_analysis.rs index 8bf6493be4b01..45afd16c313e4 100644 --- a/compiler/rustc_mir_dataflow/src/value_analysis.rs +++ b/compiler/rustc_mir_dataflow/src/value_analysis.rs @@ -622,8 +622,7 @@ impl Map { ty: Ty<'tcx>, filter: &mut impl FnMut(Ty<'tcx>) -> bool, ) { - // Note: The framework supports only scalars for now. - if filter(ty) && ty.is_scalar() { + if filter(ty) { // We know that the projection only contains trackable elements. let place = self.make_place(local, projection).unwrap(); From cd3649b2a595da17dcff983b6d5f74a28a98dd00 Mon Sep 17 00:00:00 2001 From: Camille GILLOT Date: Sat, 21 Jan 2023 21:53:26 +0000 Subject: [PATCH 12/21] Only exclude locals if the place is not indirect. --- .../src/impls/borrowed_locals.rs | 4 +++- .../rustc_mir_dataflow/src/value_analysis.rs | 20 ++++++++++--------- compiler/rustc_mir_transform/src/sroa.rs | 18 +++++++++-------- 3 files changed, 24 insertions(+), 18 deletions(-) diff --git a/compiler/rustc_mir_dataflow/src/impls/borrowed_locals.rs b/compiler/rustc_mir_dataflow/src/impls/borrowed_locals.rs index 0f8e86d1d6679..6f4e7fd4682c1 100644 --- a/compiler/rustc_mir_dataflow/src/impls/borrowed_locals.rs +++ b/compiler/rustc_mir_dataflow/src/impls/borrowed_locals.rs @@ -121,7 +121,9 @@ where // for now. See discussion on [#61069]. // // [#61069]: https://github.com/rust-lang/rust/pull/61069 - self.trans.gen(dropped_place.local); + if !dropped_place.is_indirect() { + self.trans.gen(dropped_place.local); + } } TerminatorKind::Abort diff --git a/compiler/rustc_mir_dataflow/src/value_analysis.rs b/compiler/rustc_mir_dataflow/src/value_analysis.rs index 45afd16c313e4..8003da6bbd269 100644 --- a/compiler/rustc_mir_dataflow/src/value_analysis.rs +++ b/compiler/rustc_mir_dataflow/src/value_analysis.rs @@ -35,6 +35,7 @@ use std::fmt::{Debug, Formatter}; use rustc_data_structures::fx::FxHashMap; +use rustc_index::bit_set::BitSet; use rustc_index::vec::IndexVec; use rustc_middle::mir::visit::{MutatingUseContext, PlaceContext, Visitor}; use rustc_middle::mir::*; @@ -589,7 +590,7 @@ impl Map { ) -> Self { let mut map = Self::new(); let exclude = excluded_locals(body); - map.register_with_filter(tcx, body, filter, &exclude); + map.register_with_filter(tcx, body, filter, exclude); debug!("registered {} places ({} nodes in total)", map.value_count, map.places.len()); map } @@ -600,12 +601,12 @@ impl Map { tcx: TyCtxt<'tcx>, body: &Body<'tcx>, mut filter: impl FnMut(Ty<'tcx>) -> bool, - exclude: &IndexVec, + exclude: BitSet, ) { // We use this vector as stack, pushing and popping projections. let mut projection = Vec::new(); for (local, decl) in body.local_decls.iter_enumerated() { - if !exclude[local] { + if !exclude.contains(local) { self.register_with_filter_rec(tcx, local, &mut projection, decl.ty, &mut filter); } } @@ -823,26 +824,27 @@ pub fn iter_fields<'tcx>( } /// Returns all locals with projections that have their reference or address taken. -pub fn excluded_locals(body: &Body<'_>) -> IndexVec { +pub fn excluded_locals(body: &Body<'_>) -> BitSet { struct Collector { - result: IndexVec, + result: BitSet, } impl<'tcx> Visitor<'tcx> for Collector { fn visit_place(&mut self, place: &Place<'tcx>, context: PlaceContext, _location: Location) { - if context.is_borrow() + if (context.is_borrow() || context.is_address_of() || context.is_drop() - || context == PlaceContext::MutatingUse(MutatingUseContext::AsmOutput) + || context == PlaceContext::MutatingUse(MutatingUseContext::AsmOutput)) + && !place.is_indirect() { // A pointer to a place could be used to access other places with the same local, // hence we have to exclude the local completely. - self.result[place.local] = true; + self.result.insert(place.local); } } } - let mut collector = Collector { result: IndexVec::from_elem(false, &body.local_decls) }; + let mut collector = Collector { result: BitSet::new_empty(body.local_decls.len()) }; collector.visit_body(body); collector.result } diff --git a/compiler/rustc_mir_transform/src/sroa.rs b/compiler/rustc_mir_transform/src/sroa.rs index 26acd406ed8a9..3fb7836ed6867 100644 --- a/compiler/rustc_mir_transform/src/sroa.rs +++ b/compiler/rustc_mir_transform/src/sroa.rs @@ -1,5 +1,5 @@ use crate::MirPass; -use rustc_index::bit_set::BitSet; +use rustc_index::bit_set::{BitSet, GrowableBitSet}; use rustc_index::vec::IndexVec; use rustc_middle::mir::patch::MirPatch; use rustc_middle::mir::visit::*; @@ -26,10 +26,12 @@ impl<'tcx> MirPass<'tcx> for ScalarReplacementOfAggregates { debug!(?replacements); let all_dead_locals = replace_flattened_locals(tcx, body, replacements); if !all_dead_locals.is_empty() { - for local in excluded.indices() { - excluded[local] |= all_dead_locals.contains(local); - } - excluded.raw.resize(body.local_decls.len(), false); + excluded.union(&all_dead_locals); + excluded = { + let mut growable = GrowableBitSet::from(excluded); + growable.ensure(body.local_decls.len()); + growable.into() + }; } else { break; } @@ -44,11 +46,11 @@ impl<'tcx> MirPass<'tcx> for ScalarReplacementOfAggregates { /// - the locals is a union or an enum; /// - the local's address is taken, and thus the relative addresses of the fields are observable to /// client code. -fn escaping_locals(excluded: &IndexVec, body: &Body<'_>) -> BitSet { +fn escaping_locals(excluded: &BitSet, body: &Body<'_>) -> BitSet { let mut set = BitSet::new_empty(body.local_decls.len()); set.insert_range(RETURN_PLACE..=Local::from_usize(body.arg_count)); for (local, decl) in body.local_decls().iter_enumerated() { - if decl.ty.is_union() || decl.ty.is_enum() || excluded[local] { + if decl.ty.is_union() || decl.ty.is_enum() || excluded.contains(local) { set.insert(local); } } @@ -172,7 +174,7 @@ fn replace_flattened_locals<'tcx>( body: &mut Body<'tcx>, replacements: ReplacementMap<'tcx>, ) -> BitSet { - let mut all_dead_locals = BitSet::new_empty(body.local_decls.len()); + let mut all_dead_locals = BitSet::new_empty(replacements.fragments.len()); for (local, replacements) in replacements.fragments.iter_enumerated() { if replacements.is_some() { all_dead_locals.insert(local); From 9a6c04f5d0e6ec47bf150187cffcb7f737799db4 Mon Sep 17 00:00:00 2001 From: Camille GILLOT Date: Sat, 21 Jan 2023 22:28:54 +0000 Subject: [PATCH 13/21] Handle discriminants in dataflow-const-prop. --- compiler/rustc_mir_dataflow/src/lib.rs | 1 + .../rustc_mir_dataflow/src/value_analysis.rs | 173 ++++++++++++++---- .../src/dataflow_const_prop.rs | 103 +++++++++-- ...mutate_discriminant.DataflowConstProp.diff | 26 +++ tests/mir-opt/dataflow-const-prop/enum.rs | 45 ++++- ...iff => enum.simple.DataflowConstProp.diff} | 16 +- 6 files changed, 305 insertions(+), 59 deletions(-) create mode 100644 tests/mir-opt/dataflow-const-prop/enum.mutate_discriminant.DataflowConstProp.diff rename tests/mir-opt/dataflow-const-prop/{enum.main.DataflowConstProp.diff => enum.simple.DataflowConstProp.diff} (84%) diff --git a/compiler/rustc_mir_dataflow/src/lib.rs b/compiler/rustc_mir_dataflow/src/lib.rs index 7f40cfca32fff..3e382f500afbe 100644 --- a/compiler/rustc_mir_dataflow/src/lib.rs +++ b/compiler/rustc_mir_dataflow/src/lib.rs @@ -1,6 +1,7 @@ #![feature(associated_type_defaults)] #![feature(box_patterns)] #![feature(exact_size_is_empty)] +#![feature(let_chains)] #![feature(min_specialization)] #![feature(once_cell)] #![feature(stmt_expr_attributes)] diff --git a/compiler/rustc_mir_dataflow/src/value_analysis.rs b/compiler/rustc_mir_dataflow/src/value_analysis.rs index 8003da6bbd269..03b6c182062db 100644 --- a/compiler/rustc_mir_dataflow/src/value_analysis.rs +++ b/compiler/rustc_mir_dataflow/src/value_analysis.rs @@ -65,10 +65,8 @@ pub trait ValueAnalysis<'tcx> { StatementKind::Assign(box (place, rvalue)) => { self.handle_assign(*place, rvalue, state); } - StatementKind::SetDiscriminant { .. } => { - // Could treat this as writing a constant to a pseudo-place. - // But discriminants are currently not tracked, so we do nothing. - // Related: https://github.com/rust-lang/unsafe-code-guidelines/issues/84 + StatementKind::SetDiscriminant { box ref place, .. } => { + state.flood_discr(place.as_ref(), self.map()); } StatementKind::Intrinsic(box intrinsic) => { self.handle_intrinsic(intrinsic, state); @@ -447,26 +445,29 @@ impl State { } pub fn flood_with(&mut self, place: PlaceRef<'_>, map: &Map, value: V) { - if let Some(root) = map.find(place) { - self.flood_idx_with(root, map, value); - } + let StateData::Reachable(values) = &mut self.0 else { return }; + map.for_each_aliasing_place(place, None, &mut |place| { + if let Some(vi) = map.places[place].value_index { + values[vi] = value.clone(); + } + }); } pub fn flood(&mut self, place: PlaceRef<'_>, map: &Map) { self.flood_with(place, map, V::top()) } - pub fn flood_idx_with(&mut self, place: PlaceIndex, map: &Map, value: V) { + pub fn flood_discr_with(&mut self, place: PlaceRef<'_>, map: &Map, value: V) { let StateData::Reachable(values) = &mut self.0 else { return }; - map.preorder_invoke(place, &mut |place| { + map.for_each_aliasing_place(place, Some(TrackElem::Discriminant), &mut |place| { if let Some(vi) = map.places[place].value_index { values[vi] = value.clone(); } }); } - pub fn flood_idx(&mut self, place: PlaceIndex, map: &Map) { - self.flood_idx_with(place, map, V::top()) + pub fn flood_discr(&mut self, place: PlaceRef<'_>, map: &Map) { + self.flood_discr_with(place, map, V::top()) } /// Copies `source` to `target`, including all tracked places beneath. @@ -474,7 +475,9 @@ impl State { /// If `target` contains a place that is not contained in `source`, it will be overwritten with /// Top. Also, because this will copy all entries one after another, it may only be used for /// places that are non-overlapping or identical. - pub fn assign_place_idx(&mut self, target: PlaceIndex, source: PlaceIndex, map: &Map) { + /// + /// The target place must have been flooded before calling this method. + fn assign_place_idx(&mut self, target: PlaceIndex, source: PlaceIndex, map: &Map) { let StateData::Reachable(values) = &mut self.0 else { return }; // If both places are tracked, we copy the value to the target. If the target is tracked, @@ -492,26 +495,28 @@ impl State { let projection = map.places[target_child].proj_elem.unwrap(); if let Some(source_child) = map.projections.get(&(source, projection)) { self.assign_place_idx(target_child, *source_child, map); - } else { - self.flood_idx(target_child, map); } } } pub fn assign(&mut self, target: PlaceRef<'_>, result: ValueOrPlace, map: &Map) { + self.flood(target, map); if let Some(target) = map.find(target) { self.assign_idx(target, result, map); - } else { - // We don't track this place nor any projections, assignment can be ignored. } } + pub fn assign_discr(&mut self, target: PlaceRef<'_>, result: ValueOrPlace, map: &Map) { + self.flood_discr(target, map); + if let Some(target) = map.find_discr(target) { + self.assign_idx(target, result, map); + } + } + + /// The target place must have been flooded before calling this method. pub fn assign_idx(&mut self, target: PlaceIndex, result: ValueOrPlace, map: &Map) { match result { ValueOrPlace::Value(value) => { - // First flood the target place in case we also track any projections (although - // this scenario is currently not well-supported by the API). - self.flood_idx(target, map); let StateData::Reachable(values) = &mut self.0 else { return }; if let Some(value_index) = map.places[target].value_index { values[value_index] = value; @@ -526,6 +531,14 @@ impl State { map.find(place).map(|place| self.get_idx(place, map)).unwrap_or(V::top()) } + /// Retrieve the value stored for a place, or ⊤ if it is not tracked. + pub fn get_discr(&self, place: PlaceRef<'_>, map: &Map) -> V { + match map.find_discr(place) { + Some(place) => self.get_idx(place, map), + None => V::top(), + } + } + /// Retrieve the value stored for a place index, or ⊤ if it is not tracked. pub fn get_idx(&self, place: PlaceIndex, map: &Map) -> V { match &self.0 { @@ -582,7 +595,6 @@ impl Map { /// This is currently the only way to create a [`Map`]. The way in which the tracked places are /// chosen is an implementation detail and may not be relied upon (other than that their type /// passes the filter). - #[instrument(skip_all, level = "debug")] pub fn from_filter<'tcx>( tcx: TyCtxt<'tcx>, body: &Body<'tcx>, @@ -614,7 +626,7 @@ impl Map { /// Potentially register the (local, projection) place and its fields, recursively. /// - /// Invariant: The projection must only contain fields. + /// Invariant: The projection must only contain trackable elements. fn register_with_filter_rec<'tcx>( &mut self, tcx: TyCtxt<'tcx>, @@ -623,21 +635,46 @@ impl Map { ty: Ty<'tcx>, filter: &mut impl FnMut(Ty<'tcx>) -> bool, ) { - if filter(ty) { - // We know that the projection only contains trackable elements. - let place = self.make_place(local, projection).unwrap(); + // We know that the projection only contains trackable elements. + let place = self.make_place(local, projection).unwrap(); - // Allocate a value slot if it doesn't have one. - if self.places[place].value_index.is_none() { - self.places[place].value_index = Some(self.value_count.into()); - self.value_count += 1; + // Allocate a value slot if it doesn't have one, and the user requested one. + if self.places[place].value_index.is_none() && filter(ty) { + self.places[place].value_index = Some(self.value_count.into()); + self.value_count += 1; + } + + if ty.is_enum() { + let discr_ty = ty.discriminant_ty(tcx); + if filter(discr_ty) { + let discr = *self + .projections + .entry((place, TrackElem::Discriminant)) + .or_insert_with(|| { + // Prepend new child to the linked list. + let next = self.places.push(PlaceInfo::new(Some(TrackElem::Discriminant))); + self.places[next].next_sibling = self.places[place].first_child; + self.places[place].first_child = Some(next); + next + }); + + // Allocate a value slot if it doesn't have one. + if self.places[discr].value_index.is_none() { + self.places[discr].value_index = Some(self.value_count.into()); + self.value_count += 1; + } } } // Recurse with all fields of this place. iter_fields(ty, tcx, |variant, field, ty| { - if variant.is_some() { - // Downcasts are currently not supported. + if let Some(variant) = variant { + projection.push(PlaceElem::Downcast(None, variant)); + let _ = self.make_place(local, projection); + projection.push(PlaceElem::Field(field, ty)); + self.register_with_filter_rec(tcx, local, projection, ty, filter); + projection.pop(); + projection.pop(); return; } projection.push(PlaceElem::Field(field, ty)); @@ -694,13 +731,77 @@ impl Map { Some(index) } + /// Locates the given place, if it exists in the tree. + pub fn find_discr(&self, place: PlaceRef<'_>) -> Option { + let index = self.find(place)?; + self.apply(index, TrackElem::Discriminant) + } + /// Iterate over all direct children. pub fn children(&self, parent: PlaceIndex) -> impl Iterator + '_ { Children::new(self, parent) } + /// Invoke a function on the given place and all places that may alias it. + /// + /// In particular, when the given place has a variant downcast, we invoke the function on all + /// the other variants. + /// + /// `tail_elem` allows to support discriminants that are not a place in MIR, but that we track + /// as such. + fn for_each_aliasing_place( + &self, + place: PlaceRef<'_>, + tail_elem: Option, + f: &mut impl FnMut(PlaceIndex), + ) { + let Some(&Some(mut index)) = self.locals.get(place.local) else { + // The local is not tracked at all, nothing to invalidate. + return; + }; + let elems = place + .projection + .iter() + .map(|&elem| elem.try_into()) + .chain(tail_elem.map(Ok).into_iter()); + for elem in elems { + let Ok(elem) = elem else { return }; + let sub = self.apply(index, elem); + if let TrackElem::Variant(..) | TrackElem::Discriminant = elem { + // Writing to an enum variant field invalidates the other variants and the discriminant. + self.for_each_variant_sibling(index, sub, f); + } + if let Some(sub) = sub { + index = sub + } else { + return; + } + } + self.preorder_invoke(index, f); + } + + /// Invoke the given function on all the descendants of the given place, except one branch. + pub fn for_each_variant_sibling( + &self, + parent: PlaceIndex, + preserved_child: Option, + f: &mut impl FnMut(PlaceIndex), + ) { + for sibling in self.children(parent) { + let elem = self.places[sibling].proj_elem; + // Only invalidate variants and discriminant. Fields (for generators) are not + // invalidated by assignment to a variant. + if let Some(TrackElem::Variant(..) | TrackElem::Discriminant) = elem + // Only invalidate the other variants, the current one is fine. + && Some(sibling) != preserved_child + { + self.preorder_invoke(sibling, f); + } + } + } + /// Invoke a function on the given place and all descendants. - pub fn preorder_invoke(&self, root: PlaceIndex, f: &mut impl FnMut(PlaceIndex)) { + fn preorder_invoke(&self, root: PlaceIndex, f: &mut impl FnMut(PlaceIndex)) { f(root); for child in self.children(root) { self.preorder_invoke(child, f); @@ -759,6 +860,7 @@ impl<'a> Iterator for Children<'a> { } /// Used as the result of an operand or r-value. +#[derive(Debug)] pub enum ValueOrPlace { Value(V), Place(PlaceIndex), @@ -776,6 +878,8 @@ impl ValueOrPlace { #[derive(Copy, Clone, Debug, PartialEq, Eq, Hash)] pub enum TrackElem { Field(Field), + Variant(VariantIdx), + Discriminant, } impl TryFrom> for TrackElem { @@ -784,6 +888,7 @@ impl TryFrom> for TrackElem { fn try_from(value: ProjectionElem) -> Result { match value { ProjectionElem::Field(field, _) => Ok(TrackElem::Field(field)), + ProjectionElem::Downcast(_, idx) => Ok(TrackElem::Variant(idx)), _ => Err(()), } } @@ -900,6 +1005,12 @@ fn debug_with_context_rec( for child in map.children(place) { let info_elem = map.places[child].proj_elem.unwrap(); let child_place_str = match info_elem { + TrackElem::Discriminant => { + format!("discriminant({})", place_str) + } + TrackElem::Variant(idx) => { + format!("({} as {:?})", place_str, idx) + } TrackElem::Field(field) => { if place_str.starts_with('*') { format!("({}).{}", place_str, field.index()) diff --git a/compiler/rustc_mir_transform/src/dataflow_const_prop.rs b/compiler/rustc_mir_transform/src/dataflow_const_prop.rs index 949a59a97bfb6..f10f208f5de3c 100644 --- a/compiler/rustc_mir_transform/src/dataflow_const_prop.rs +++ b/compiler/rustc_mir_transform/src/dataflow_const_prop.rs @@ -13,6 +13,7 @@ use rustc_mir_dataflow::value_analysis::{Map, State, TrackElem, ValueAnalysis, V use rustc_mir_dataflow::{lattice::FlatSet, Analysis, ResultsVisitor, SwitchIntEdgeEffects}; use rustc_span::DUMMY_SP; use rustc_target::abi::Align; +use rustc_target::abi::VariantIdx; use crate::MirPass; @@ -30,6 +31,7 @@ impl<'tcx> MirPass<'tcx> for DataflowConstProp { #[instrument(skip_all level = "debug")] fn run_pass(&self, tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) { + debug!(def_id = ?body.source.def_id()); if tcx.sess.mir_opt_level() < 4 && body.basic_blocks.len() > BLOCK_LIMIT { debug!("aborted dataflow const prop due too many basic blocks"); return; @@ -63,14 +65,31 @@ impl<'tcx> MirPass<'tcx> for DataflowConstProp { } } -struct ConstAnalysis<'tcx> { +struct ConstAnalysis<'a, 'tcx> { map: Map, tcx: TyCtxt<'tcx>, + local_decls: &'a LocalDecls<'tcx>, ecx: InterpCx<'tcx, 'tcx, DummyMachine>, param_env: ty::ParamEnv<'tcx>, } -impl<'tcx> ValueAnalysis<'tcx> for ConstAnalysis<'tcx> { +impl<'tcx> ConstAnalysis<'_, 'tcx> { + fn eval_disciminant( + &self, + enum_ty: Ty<'tcx>, + variant_index: VariantIdx, + ) -> Option> { + if !enum_ty.is_enum() { + return None; + } + let discr = enum_ty.discriminant_for_variant(self.tcx, variant_index)?; + let discr_layout = self.tcx.layout_of(self.param_env.and(discr.ty)).ok()?; + let discr_value = Scalar::try_from_uint(discr.val, discr_layout.size)?; + Some(ScalarTy(discr_value, discr.ty)) + } +} + +impl<'tcx> ValueAnalysis<'tcx> for ConstAnalysis<'_, 'tcx> { type Value = FlatSet>; const NAME: &'static str = "ConstAnalysis"; @@ -79,6 +98,25 @@ impl<'tcx> ValueAnalysis<'tcx> for ConstAnalysis<'tcx> { &self.map } + fn handle_statement(&self, statement: &Statement<'tcx>, state: &mut State) { + match statement.kind { + StatementKind::SetDiscriminant { box ref place, variant_index } => { + state.flood_discr(place.as_ref(), &self.map); + if self.map.find_discr(place.as_ref()).is_some() { + let enum_ty = place.ty(self.local_decls, self.tcx).ty; + if let Some(discr) = self.eval_disciminant(enum_ty, variant_index) { + state.assign_discr( + place.as_ref(), + ValueOrPlace::Value(FlatSet::Elem(discr)), + &self.map, + ); + } + } + } + _ => self.super_statement(statement, state), + } + } + fn handle_assign( &self, target: Place<'tcx>, @@ -87,17 +125,22 @@ impl<'tcx> ValueAnalysis<'tcx> for ConstAnalysis<'tcx> { ) { match rvalue { Rvalue::Aggregate(kind, operands) => { - let target = self.map().find(target.as_ref()); - if let Some(target) = target { - state.flood_idx_with(target, self.map(), FlatSet::Bottom); - let field_based = match **kind { - AggregateKind::Tuple | AggregateKind::Closure(..) => true, - AggregateKind::Adt(def_id, ..) => { - matches!(self.tcx.def_kind(def_id), DefKind::Struct) + state.flood_with(target.as_ref(), self.map(), FlatSet::Bottom); + if let Some(target_idx) = self.map().find(target.as_ref()) { + let (variant_target, variant_index) = match **kind { + AggregateKind::Tuple | AggregateKind::Closure(..) => { + (Some(target_idx), None) + } + AggregateKind::Adt(def_id, variant_index, ..) => { + match self.tcx.def_kind(def_id) { + DefKind::Struct => (Some(target_idx), None), + DefKind::Enum => (Some(target_idx), Some(variant_index)), + _ => (None, None), + } } - _ => false, + _ => (None, None), }; - if field_based { + if let Some(target) = variant_target { for (field_index, operand) in operands.iter().enumerate() { if let Some(field) = self .map() @@ -108,15 +151,20 @@ impl<'tcx> ValueAnalysis<'tcx> for ConstAnalysis<'tcx> { } } } + if let Some(variant_index) = variant_index + && let Some(discr_idx) = self.map().apply(target_idx, TrackElem::Discriminant) + { + let enum_ty = target.ty(self.local_decls, self.tcx).ty; + if let Some(discr_val) = self.eval_disciminant(enum_ty, variant_index) { + state.assign_idx(discr_idx, ValueOrPlace::Value(FlatSet::Elem(discr_val)), &self.map); + } + } } } Rvalue::CheckedBinaryOp(op, box (left, right)) => { + state.flood(target.as_ref(), self.map()); + let target = self.map().find(target.as_ref()); - if let Some(target) = target { - // We should not track any projections other than - // what is overwritten below, but just in case... - state.flood_idx(target, self.map()); - } let value_target = target .and_then(|target| self.map().apply(target, TrackElem::Field(0_u32.into()))); @@ -195,6 +243,9 @@ impl<'tcx> ValueAnalysis<'tcx> for ConstAnalysis<'tcx> { FlatSet::Bottom => ValueOrPlace::Value(FlatSet::Bottom), FlatSet::Top => ValueOrPlace::Value(FlatSet::Top), }, + Rvalue::Discriminant(place) => { + ValueOrPlace::Value(state.get_discr(place.as_ref(), self.map())) + } _ => self.super_rvalue(rvalue, state), } } @@ -268,12 +319,13 @@ impl<'tcx> std::fmt::Debug for ScalarTy<'tcx> { } } -impl<'tcx> ConstAnalysis<'tcx> { - pub fn new(tcx: TyCtxt<'tcx>, body: &Body<'tcx>, map: Map) -> Self { +impl<'a, 'tcx> ConstAnalysis<'a, 'tcx> { + pub fn new(tcx: TyCtxt<'tcx>, body: &'a Body<'tcx>, map: Map) -> Self { let param_env = tcx.param_env(body.source.def_id()); Self { map, tcx, + local_decls: &body.local_decls, ecx: InterpCx::new(tcx, DUMMY_SP, param_env, DummyMachine), param_env: param_env, } @@ -466,6 +518,21 @@ impl<'tcx, 'map, 'a> Visitor<'tcx> for OperandCollector<'tcx, 'map, 'a> { _ => (), } } + + fn visit_rvalue(&mut self, rvalue: &Rvalue<'tcx>, location: Location) { + match rvalue { + Rvalue::Discriminant(place) => { + match self.state.get_discr(place.as_ref(), self.visitor.map) { + FlatSet::Top => (), + FlatSet::Elem(value) => { + self.visitor.before_effect.insert((location, *place), value); + } + FlatSet::Bottom => (), + } + } + _ => self.super_rvalue(rvalue, location), + } + } } struct DummyMachine; diff --git a/tests/mir-opt/dataflow-const-prop/enum.mutate_discriminant.DataflowConstProp.diff b/tests/mir-opt/dataflow-const-prop/enum.mutate_discriminant.DataflowConstProp.diff new file mode 100644 index 0000000000000..038e6c6bd9005 --- /dev/null +++ b/tests/mir-opt/dataflow-const-prop/enum.mutate_discriminant.DataflowConstProp.diff @@ -0,0 +1,26 @@ +- // MIR for `mutate_discriminant` before DataflowConstProp ++ // MIR for `mutate_discriminant` after DataflowConstProp + + fn mutate_discriminant() -> u8 { + let mut _0: u8; // return place in scope 0 at $DIR/enum.rs:+0:29: +0:31 + let mut _1: std::option::Option; // in scope 0 at $SRC_DIR/core/src/intrinsics/mir.rs:LL:COL + let mut _2: isize; // in scope 0 at $SRC_DIR/core/src/intrinsics/mir.rs:LL:COL + + bb0: { + discriminant(_1) = 1; // scope 0 at $DIR/enum.rs:+4:13: +4:34 + (((_1 as variant#1).0: NonZeroUsize).0: usize) = const 0_usize; // scope 0 at $DIR/enum.rs:+6:13: +6:64 + _2 = discriminant(_1); // scope 0 at $SRC_DIR/core/src/intrinsics/mir.rs:LL:COL + switchInt(_2) -> [0: bb1, otherwise: bb2]; // scope 0 at $DIR/enum.rs:+9:13: +12:14 + } + + bb1: { + _0 = const 1_u8; // scope 0 at $DIR/enum.rs:+15:13: +15:20 + return; // scope 0 at $DIR/enum.rs:+16:13: +16:21 + } + + bb2: { + _0 = const 2_u8; // scope 0 at $DIR/enum.rs:+19:13: +19:20 + unreachable; // scope 0 at $DIR/enum.rs:+20:13: +20:26 + } + } + diff --git a/tests/mir-opt/dataflow-const-prop/enum.rs b/tests/mir-opt/dataflow-const-prop/enum.rs index 13288577dea3f..7ea405bd9c408 100644 --- a/tests/mir-opt/dataflow-const-prop/enum.rs +++ b/tests/mir-opt/dataflow-const-prop/enum.rs @@ -1,13 +1,52 @@ // unit-test: DataflowConstProp -// Not trackable, because variants could be aliased. +#![feature(custom_mir, core_intrinsics, rustc_attrs)] + +use std::intrinsics::mir::*; + enum E { V1(i32), V2(i32) } -// EMIT_MIR enum.main.DataflowConstProp.diff -fn main() { +// EMIT_MIR enum.simple.DataflowConstProp.diff +fn simple() { let e = E::V1(0); let x = match e { E::V1(x) => x, E::V2(x) => x }; } + +#[rustc_layout_scalar_valid_range_start(1)] +#[rustc_nonnull_optimization_guaranteed] +struct NonZeroUsize(usize); + +// EMIT_MIR enum.mutate_discriminant.DataflowConstProp.diff +#[custom_mir(dialect = "runtime", phase = "post-cleanup")] +fn mutate_discriminant() -> u8 { + mir!( + let x: Option; + { + SetDiscriminant(x, 1); + // This assignment overwrites the niche in which the discriminant is stored. + place!(Field(Field(Variant(x, 1), 0), 0)) = 0_usize; + // So we cannot know the value of this discriminant. + let a = Discriminant(x); + match a { + 0 => bb1, + _ => bad, + } + } + bb1 = { + RET = 1; + Return() + } + bad = { + RET = 2; + Unreachable() + } + ) +} + +fn main() { + simple(); + mutate_discriminant(); +} diff --git a/tests/mir-opt/dataflow-const-prop/enum.main.DataflowConstProp.diff b/tests/mir-opt/dataflow-const-prop/enum.simple.DataflowConstProp.diff similarity index 84% rename from tests/mir-opt/dataflow-const-prop/enum.main.DataflowConstProp.diff rename to tests/mir-opt/dataflow-const-prop/enum.simple.DataflowConstProp.diff index d049c79d78def..1fb65e6584525 100644 --- a/tests/mir-opt/dataflow-const-prop/enum.main.DataflowConstProp.diff +++ b/tests/mir-opt/dataflow-const-prop/enum.simple.DataflowConstProp.diff @@ -1,8 +1,8 @@ -- // MIR for `main` before DataflowConstProp -+ // MIR for `main` after DataflowConstProp +- // MIR for `simple` before DataflowConstProp ++ // MIR for `simple` after DataflowConstProp - fn main() -> () { - let mut _0: (); // return place in scope 0 at $DIR/enum.rs:+0:11: +0:11 + fn simple() -> () { + let mut _0: (); // return place in scope 0 at $DIR/enum.rs:+0:13: +0:13 let _1: E; // in scope 0 at $DIR/enum.rs:+1:9: +1:10 let mut _3: isize; // in scope 0 at $DIR/enum.rs:+2:23: +2:31 scope 1 { @@ -25,8 +25,10 @@ StorageLive(_1); // scope 0 at $DIR/enum.rs:+1:9: +1:10 _1 = E::V1(const 0_i32); // scope 0 at $DIR/enum.rs:+1:13: +1:21 StorageLive(_2); // scope 1 at $DIR/enum.rs:+2:9: +2:10 - _3 = discriminant(_1); // scope 1 at $DIR/enum.rs:+2:19: +2:20 - switchInt(move _3) -> [0: bb3, 1: bb1, otherwise: bb2]; // scope 1 at $DIR/enum.rs:+2:13: +2:20 +- _3 = discriminant(_1); // scope 1 at $DIR/enum.rs:+2:19: +2:20 +- switchInt(move _3) -> [0: bb3, 1: bb1, otherwise: bb2]; // scope 1 at $DIR/enum.rs:+2:13: +2:20 ++ _3 = const 0_isize; // scope 1 at $DIR/enum.rs:+2:19: +2:20 ++ switchInt(const 0_isize) -> [0: bb3, 1: bb1, otherwise: bb2]; // scope 1 at $DIR/enum.rs:+2:13: +2:20 } bb1: { @@ -50,7 +52,7 @@ } bb4: { - _0 = const (); // scope 0 at $DIR/enum.rs:+0:11: +3:2 + _0 = const (); // scope 0 at $DIR/enum.rs:+0:13: +3:2 StorageDead(_2); // scope 1 at $DIR/enum.rs:+3:1: +3:2 StorageDead(_1); // scope 0 at $DIR/enum.rs:+3:1: +3:2 return; // scope 0 at $DIR/enum.rs:+3:2: +3:2 From c48756cdbfb1725251cbfa6fe760b2cb4e47b2d9 Mon Sep 17 00:00:00 2001 From: Camille GILLOT Date: Sat, 28 Jan 2023 11:23:18 +0000 Subject: [PATCH 14/21] Limit creation of tracked place directly. --- .../rustc_mir_dataflow/src/value_analysis.rs | 22 +++++++++++++++---- .../src/dataflow_const_prop.rs | 11 ++++------ 2 files changed, 22 insertions(+), 11 deletions(-) diff --git a/compiler/rustc_mir_dataflow/src/value_analysis.rs b/compiler/rustc_mir_dataflow/src/value_analysis.rs index 03b6c182062db..f587f17f12a1b 100644 --- a/compiler/rustc_mir_dataflow/src/value_analysis.rs +++ b/compiler/rustc_mir_dataflow/src/value_analysis.rs @@ -599,10 +599,11 @@ impl Map { tcx: TyCtxt<'tcx>, body: &Body<'tcx>, filter: impl FnMut(Ty<'tcx>) -> bool, + place_limit: Option, ) -> Self { let mut map = Self::new(); let exclude = excluded_locals(body); - map.register_with_filter(tcx, body, filter, exclude); + map.register_with_filter(tcx, body, filter, exclude, place_limit); debug!("registered {} places ({} nodes in total)", map.value_count, map.places.len()); map } @@ -614,12 +615,20 @@ impl Map { body: &Body<'tcx>, mut filter: impl FnMut(Ty<'tcx>) -> bool, exclude: BitSet, + place_limit: Option, ) { // We use this vector as stack, pushing and popping projections. let mut projection = Vec::new(); for (local, decl) in body.local_decls.iter_enumerated() { if !exclude.contains(local) { - self.register_with_filter_rec(tcx, local, &mut projection, decl.ty, &mut filter); + self.register_with_filter_rec( + tcx, + local, + &mut projection, + decl.ty, + &mut filter, + place_limit, + ); } } } @@ -634,7 +643,12 @@ impl Map { projection: &mut Vec>, ty: Ty<'tcx>, filter: &mut impl FnMut(Ty<'tcx>) -> bool, + place_limit: Option, ) { + if let Some(place_limit) = place_limit && self.value_count >= place_limit { + return + } + // We know that the projection only contains trackable elements. let place = self.make_place(local, projection).unwrap(); @@ -672,13 +686,13 @@ impl Map { projection.push(PlaceElem::Downcast(None, variant)); let _ = self.make_place(local, projection); projection.push(PlaceElem::Field(field, ty)); - self.register_with_filter_rec(tcx, local, projection, ty, filter); + self.register_with_filter_rec(tcx, local, projection, ty, filter, place_limit); projection.pop(); projection.pop(); return; } projection.push(PlaceElem::Field(field, ty)); - self.register_with_filter_rec(tcx, local, projection, ty, filter); + self.register_with_filter_rec(tcx, local, projection, ty, filter, place_limit); projection.pop(); }); } diff --git a/compiler/rustc_mir_transform/src/dataflow_const_prop.rs b/compiler/rustc_mir_transform/src/dataflow_const_prop.rs index f10f208f5de3c..bfb1eb8b5fb79 100644 --- a/compiler/rustc_mir_transform/src/dataflow_const_prop.rs +++ b/compiler/rustc_mir_transform/src/dataflow_const_prop.rs @@ -37,9 +37,6 @@ impl<'tcx> MirPass<'tcx> for DataflowConstProp { return; } - // Decide which places to track during the analysis. - let map = Map::from_filter(tcx, body, Ty::is_scalar); - // We want to have a somewhat linear runtime w.r.t. the number of statements/terminators. // Let's call this number `n`. Dataflow analysis has `O(h*n)` transfer function // applications, where `h` is the height of the lattice. Because the height of our lattice @@ -48,10 +45,10 @@ impl<'tcx> MirPass<'tcx> for DataflowConstProp { // `O(num_nodes * tracked_places * n)` in terms of time complexity. Since the number of // map nodes is strongly correlated to the number of tracked places, this becomes more or // less `O(n)` if we place a constant limit on the number of tracked places. - if tcx.sess.mir_opt_level() < 4 && map.tracked_places() > PLACE_LIMIT { - debug!("aborted dataflow const prop due to too many tracked places"); - return; - } + let place_limit = if tcx.sess.mir_opt_level() < 4 { Some(PLACE_LIMIT) } else { None }; + + // Decide which places to track during the analysis. + let map = Map::from_filter(tcx, body, Ty::is_scalar, place_limit); // Perform the actual dataflow analysis. let analysis = ConstAnalysis::new(tcx, body, map); From 9af191f86f2c81ec5613ae35ab1a3b2ac3edbdee Mon Sep 17 00:00:00 2001 From: Camille GILLOT Date: Sun, 29 Jan 2023 14:20:45 +0000 Subject: [PATCH 15/21] Improve value_analysis API. --- .../rustc_mir_dataflow/src/value_analysis.rs | 25 +++++++++++++------ 1 file changed, 18 insertions(+), 7 deletions(-) diff --git a/compiler/rustc_mir_dataflow/src/value_analysis.rs b/compiler/rustc_mir_dataflow/src/value_analysis.rs index f587f17f12a1b..353b8d801d57b 100644 --- a/compiler/rustc_mir_dataflow/src/value_analysis.rs +++ b/compiler/rustc_mir_dataflow/src/value_analysis.rs @@ -735,20 +735,31 @@ impl Map { } /// Locates the given place, if it exists in the tree. - pub fn find(&self, place: PlaceRef<'_>) -> Option { + pub fn find_extra( + &self, + place: PlaceRef<'_>, + extra: impl IntoIterator, + ) -> Option { let mut index = *self.locals.get(place.local)?.as_ref()?; for &elem in place.projection { index = self.apply(index, elem.try_into().ok()?)?; } + for elem in extra { + index = self.apply(index, elem)?; + } Some(index) } /// Locates the given place, if it exists in the tree. + pub fn find(&self, place: PlaceRef<'_>) -> Option { + self.find_extra(place, []) + } + + /// Locates the given place and applies `Discriminant`, if it exists in the tree. pub fn find_discr(&self, place: PlaceRef<'_>) -> Option { - let index = self.find(place)?; - self.apply(index, TrackElem::Discriminant) + self.find_extra(place, [TrackElem::Discriminant]) } /// Iterate over all direct children. @@ -763,14 +774,14 @@ impl Map { /// /// `tail_elem` allows to support discriminants that are not a place in MIR, but that we track /// as such. - fn for_each_aliasing_place( + pub fn for_each_aliasing_place( &self, place: PlaceRef<'_>, tail_elem: Option, f: &mut impl FnMut(PlaceIndex), ) { let Some(&Some(mut index)) = self.locals.get(place.local) else { - // The local is not tracked at all, nothing to invalidate. + // The local is not tracked at all, so it does not alias anything. return; }; let elems = place @@ -782,7 +793,7 @@ impl Map { let Ok(elem) = elem else { return }; let sub = self.apply(index, elem); if let TrackElem::Variant(..) | TrackElem::Discriminant = elem { - // Writing to an enum variant field invalidates the other variants and the discriminant. + // Enum variant fields and enum discriminants alias each another. self.for_each_variant_sibling(index, sub, f); } if let Some(sub) = sub { @@ -795,7 +806,7 @@ impl Map { } /// Invoke the given function on all the descendants of the given place, except one branch. - pub fn for_each_variant_sibling( + fn for_each_variant_sibling( &self, parent: PlaceIndex, preserved_child: Option, From 67a8c16fe285dc5dc3ca8a0c74fb1bcfa58ce8dc Mon Sep 17 00:00:00 2001 From: Camille GILLOT Date: Mon, 30 Jan 2023 17:37:56 +0000 Subject: [PATCH 16/21] Complete for_each_aliasing_place. --- compiler/rustc_middle/src/mir/mod.rs | 8 ++++++++ compiler/rustc_mir_dataflow/src/value_analysis.rs | 7 +++++++ 2 files changed, 15 insertions(+) diff --git a/compiler/rustc_middle/src/mir/mod.rs b/compiler/rustc_middle/src/mir/mod.rs index 10ac7e0d39af6..e272c90e0cdaf 100644 --- a/compiler/rustc_middle/src/mir/mod.rs +++ b/compiler/rustc_middle/src/mir/mod.rs @@ -1640,6 +1640,14 @@ impl<'tcx> PlaceRef<'tcx> { } } + /// Returns `true` if this `Place` contains a `Deref` projection. + /// + /// If `Place::is_indirect` returns false, the caller knows that the `Place` refers to the + /// same region of memory as its base. + pub fn is_indirect(&self) -> bool { + self.projection.iter().any(|elem| elem.is_indirect()) + } + /// If MirPhase >= Derefered and if projection contains Deref, /// It's guaranteed to be in the first place pub fn has_deref(&self) -> bool { diff --git a/compiler/rustc_mir_dataflow/src/value_analysis.rs b/compiler/rustc_mir_dataflow/src/value_analysis.rs index 353b8d801d57b..f24280e218716 100644 --- a/compiler/rustc_mir_dataflow/src/value_analysis.rs +++ b/compiler/rustc_mir_dataflow/src/value_analysis.rs @@ -780,6 +780,10 @@ impl Map { tail_elem: Option, f: &mut impl FnMut(PlaceIndex), ) { + if place.is_indirect() { + // We do not track indirect places. + return; + } let Some(&Some(mut index)) = self.locals.get(place.local) else { // The local is not tracked at all, so it does not alias anything. return; @@ -790,6 +794,9 @@ impl Map { .map(|&elem| elem.try_into()) .chain(tail_elem.map(Ok).into_iter()); for elem in elems { + // A field aliases the parent place. + f(index); + let Ok(elem) = elem else { return }; let sub = self.apply(index, elem); if let TrackElem::Variant(..) | TrackElem::Discriminant = elem { From df889c9821970020abb8dbb1ed9b0014e38c6137 Mon Sep 17 00:00:00 2001 From: Camille GILLOT Date: Fri, 3 Feb 2023 16:39:10 +0000 Subject: [PATCH 17/21] Rename assign_idx methods. --- .../rustc_mir_dataflow/src/value_analysis.rs | 55 +++++++++++-------- .../src/dataflow_const_prop.rs | 15 +++-- 2 files changed, 39 insertions(+), 31 deletions(-) diff --git a/compiler/rustc_mir_dataflow/src/value_analysis.rs b/compiler/rustc_mir_dataflow/src/value_analysis.rs index f24280e218716..2da7cdd02a7f8 100644 --- a/compiler/rustc_mir_dataflow/src/value_analysis.rs +++ b/compiler/rustc_mir_dataflow/src/value_analysis.rs @@ -24,7 +24,7 @@ //! - The bottom state denotes uninitialized memory. Because we are only doing a sound approximation //! of the actual execution, we can also use this state for places where access would be UB. //! -//! - The assignment logic in `State::assign_place_idx` assumes that the places are non-overlapping, +//! - The assignment logic in `State::insert_place_idx` assumes that the places are non-overlapping, //! or identical. Note that this refers to place expressions, not memory locations. //! //! - Currently, places that have their reference taken cannot be tracked. Although this would be @@ -470,6 +470,28 @@ impl State { self.flood_discr_with(place, map, V::top()) } + /// Low-level method that assigns to a place. + /// This does nothing if the place is not tracked. + /// + /// The target place must have been flooded before calling this method. + pub fn insert_idx(&mut self, target: PlaceIndex, result: ValueOrPlace, map: &Map) { + match result { + ValueOrPlace::Value(value) => self.insert_value_idx(target, value, map), + ValueOrPlace::Place(source) => self.insert_place_idx(target, source, map), + } + } + + /// Low-level method that assigns a value to a place. + /// This does nothing if the place is not tracked. + /// + /// The target place must have been flooded before calling this method. + pub fn insert_value_idx(&mut self, target: PlaceIndex, value: V, map: &Map) { + let StateData::Reachable(values) = &mut self.0 else { return }; + if let Some(value_index) = map.places[target].value_index { + values[value_index] = value; + } + } + /// Copies `source` to `target`, including all tracked places beneath. /// /// If `target` contains a place that is not contained in `source`, it will be overwritten with @@ -477,52 +499,39 @@ impl State { /// places that are non-overlapping or identical. /// /// The target place must have been flooded before calling this method. - fn assign_place_idx(&mut self, target: PlaceIndex, source: PlaceIndex, map: &Map) { + fn insert_place_idx(&mut self, target: PlaceIndex, source: PlaceIndex, map: &Map) { let StateData::Reachable(values) = &mut self.0 else { return }; - // If both places are tracked, we copy the value to the target. If the target is tracked, - // but the source is not, we have to invalidate the value in target. If the target is not - // tracked, then we don't have to do anything. + // If both places are tracked, we copy the value to the target. + // If the target is tracked, but the source is not, we do nothing, as invalidation has + // already been performed. if let Some(target_value) = map.places[target].value_index { if let Some(source_value) = map.places[source].value_index { values[target_value] = values[source_value].clone(); - } else { - values[target_value] = V::top(); } } for target_child in map.children(target) { // Try to find corresponding child and recurse. Reasoning is similar as above. let projection = map.places[target_child].proj_elem.unwrap(); if let Some(source_child) = map.projections.get(&(source, projection)) { - self.assign_place_idx(target_child, *source_child, map); + self.insert_place_idx(target_child, *source_child, map); } } } + /// Helper method to interpret `target = result`. pub fn assign(&mut self, target: PlaceRef<'_>, result: ValueOrPlace, map: &Map) { self.flood(target, map); if let Some(target) = map.find(target) { - self.assign_idx(target, result, map); + self.insert_idx(target, result, map); } } + /// Helper method for assignments to a discriminant. pub fn assign_discr(&mut self, target: PlaceRef<'_>, result: ValueOrPlace, map: &Map) { self.flood_discr(target, map); if let Some(target) = map.find_discr(target) { - self.assign_idx(target, result, map); - } - } - - /// The target place must have been flooded before calling this method. - pub fn assign_idx(&mut self, target: PlaceIndex, result: ValueOrPlace, map: &Map) { - match result { - ValueOrPlace::Value(value) => { - let StateData::Reachable(values) = &mut self.0 else { return }; - if let Some(value_index) = map.places[target].value_index { - values[value_index] = value; - } - } - ValueOrPlace::Place(source) => self.assign_place_idx(target, source, map), + self.insert_idx(target, result, map); } } diff --git a/compiler/rustc_mir_transform/src/dataflow_const_prop.rs b/compiler/rustc_mir_transform/src/dataflow_const_prop.rs index bfb1eb8b5fb79..d715e250ca4eb 100644 --- a/compiler/rustc_mir_transform/src/dataflow_const_prop.rs +++ b/compiler/rustc_mir_transform/src/dataflow_const_prop.rs @@ -144,7 +144,7 @@ impl<'tcx> ValueAnalysis<'tcx> for ConstAnalysis<'_, 'tcx> { .apply(target, TrackElem::Field(Field::from_usize(field_index))) { let result = self.handle_operand(operand, state); - state.assign_idx(field, result, self.map()); + state.insert_idx(field, result, self.map()); } } } @@ -153,12 +153,13 @@ impl<'tcx> ValueAnalysis<'tcx> for ConstAnalysis<'_, 'tcx> { { let enum_ty = target.ty(self.local_decls, self.tcx).ty; if let Some(discr_val) = self.eval_disciminant(enum_ty, variant_index) { - state.assign_idx(discr_idx, ValueOrPlace::Value(FlatSet::Elem(discr_val)), &self.map); + state.insert_value_idx(discr_idx, FlatSet::Elem(discr_val), &self.map); } } } } Rvalue::CheckedBinaryOp(op, box (left, right)) => { + // Flood everything now, so we can use `insert_value_idx` directly later. state.flood(target.as_ref(), self.map()); let target = self.map().find(target.as_ref()); @@ -172,7 +173,8 @@ impl<'tcx> ValueAnalysis<'tcx> for ConstAnalysis<'_, 'tcx> { let (val, overflow) = self.binary_op(state, *op, left, right); if let Some(value_target) = value_target { - state.assign_idx(value_target, ValueOrPlace::Value(val), self.map()); + // We have flooded `target` earlier. + state.insert_value_idx(value_target, val, self.map()); } if let Some(overflow_target) = overflow_target { let overflow = match overflow { @@ -187,11 +189,8 @@ impl<'tcx> ValueAnalysis<'tcx> for ConstAnalysis<'_, 'tcx> { } FlatSet::Bottom => FlatSet::Bottom, }; - state.assign_idx( - overflow_target, - ValueOrPlace::Value(overflow), - self.map(), - ); + // We have flooded `target` earlier. + state.insert_value_idx(overflow_target, overflow, self.map()); } } } From 5925400e47bce0142574056683caba11d794cf0d Mon Sep 17 00:00:00 2001 From: Ben Kimock Date: Sun, 12 Feb 2023 14:37:09 -0500 Subject: [PATCH 18/21] Fix unintentional UB in ui tests --- .../2229_closure_analysis/migrations/auto_traits.fixed | 4 ++-- .../closures/2229_closure_analysis/migrations/auto_traits.rs | 4 ++-- .../2229_closure_analysis/migrations/auto_traits.stderr | 4 ++-- .../2229_closure_analysis/migrations/multi_diagnostics.fixed | 2 +- .../2229_closure_analysis/migrations/multi_diagnostics.rs | 2 +- .../migrations/multi_diagnostics.stderr | 2 +- tests/ui/consts/const-eval/issue-91827-extern-types.rs | 5 ++++- tests/ui/unsized/unsized3-rpass.rs | 4 ++-- 8 files changed, 15 insertions(+), 12 deletions(-) diff --git a/tests/ui/closures/2229_closure_analysis/migrations/auto_traits.fixed b/tests/ui/closures/2229_closure_analysis/migrations/auto_traits.fixed index 26703fbf81193..b74b5e94e2b75 100644 --- a/tests/ui/closures/2229_closure_analysis/migrations/auto_traits.fixed +++ b/tests/ui/closures/2229_closure_analysis/migrations/auto_traits.fixed @@ -26,7 +26,7 @@ fn test_send_trait() { //~| HELP: add a dummy let to cause `fptr` to be fully captured *fptr.0 = 20; //~^ NOTE: in Rust 2018, this closure captures all of `fptr`, but in Rust 2021, it will only capture `fptr.0` - } }); + } }).join().unwrap(); } /* Test Sync Trait Migration */ @@ -47,7 +47,7 @@ fn test_sync_trait() { //~| HELP: add a dummy let to cause `fptr` to be fully captured *fptr.0.0 = 20; //~^ NOTE: in Rust 2018, this closure captures all of `fptr`, but in Rust 2021, it will only capture `fptr.0.0` - } }); + } }).join().unwrap(); } /* Test Clone Trait Migration */ diff --git a/tests/ui/closures/2229_closure_analysis/migrations/auto_traits.rs b/tests/ui/closures/2229_closure_analysis/migrations/auto_traits.rs index 932db51d43713..e4965e33cc16f 100644 --- a/tests/ui/closures/2229_closure_analysis/migrations/auto_traits.rs +++ b/tests/ui/closures/2229_closure_analysis/migrations/auto_traits.rs @@ -26,7 +26,7 @@ fn test_send_trait() { //~| HELP: add a dummy let to cause `fptr` to be fully captured *fptr.0 = 20; //~^ NOTE: in Rust 2018, this closure captures all of `fptr`, but in Rust 2021, it will only capture `fptr.0` - }); + }).join().unwrap(); } /* Test Sync Trait Migration */ @@ -47,7 +47,7 @@ fn test_sync_trait() { //~| HELP: add a dummy let to cause `fptr` to be fully captured *fptr.0.0 = 20; //~^ NOTE: in Rust 2018, this closure captures all of `fptr`, but in Rust 2021, it will only capture `fptr.0.0` - }); + }).join().unwrap(); } /* Test Clone Trait Migration */ diff --git a/tests/ui/closures/2229_closure_analysis/migrations/auto_traits.stderr b/tests/ui/closures/2229_closure_analysis/migrations/auto_traits.stderr index 3a42cc8b8439b..856ec4a5b9eb3 100644 --- a/tests/ui/closures/2229_closure_analysis/migrations/auto_traits.stderr +++ b/tests/ui/closures/2229_closure_analysis/migrations/auto_traits.stderr @@ -19,7 +19,7 @@ LL ~ thread::spawn(move || { let _ = &fptr; unsafe { LL | ... LL | -LL ~ } }); +LL ~ } }).join().unwrap(); | error: changes to closure capture in Rust 2021 will affect which traits the closure implements @@ -41,7 +41,7 @@ LL ~ thread::spawn(move || { let _ = &fptr; unsafe { LL | ... LL | -LL ~ } }); +LL ~ } }).join().unwrap(); | error: changes to closure capture in Rust 2021 will affect drop order and which traits the closure implements diff --git a/tests/ui/closures/2229_closure_analysis/migrations/multi_diagnostics.fixed b/tests/ui/closures/2229_closure_analysis/migrations/multi_diagnostics.fixed index 173dd2e2cff2e..bde8c7497310d 100644 --- a/tests/ui/closures/2229_closure_analysis/migrations/multi_diagnostics.fixed +++ b/tests/ui/closures/2229_closure_analysis/migrations/multi_diagnostics.fixed @@ -145,7 +145,7 @@ fn test_multi_traits_issues() { //~^ NOTE: in Rust 2018, this closure captures all of `fptr1`, but in Rust 2021, it will only capture `fptr1.0.0` *fptr2.0 = 20; //~^ NOTE: in Rust 2018, this closure captures all of `fptr2`, but in Rust 2021, it will only capture `fptr2.0` - } }); + } }).join().unwrap(); } fn main() { diff --git a/tests/ui/closures/2229_closure_analysis/migrations/multi_diagnostics.rs b/tests/ui/closures/2229_closure_analysis/migrations/multi_diagnostics.rs index cfc4555ca03a0..584c52ea13430 100644 --- a/tests/ui/closures/2229_closure_analysis/migrations/multi_diagnostics.rs +++ b/tests/ui/closures/2229_closure_analysis/migrations/multi_diagnostics.rs @@ -141,7 +141,7 @@ fn test_multi_traits_issues() { //~^ NOTE: in Rust 2018, this closure captures all of `fptr1`, but in Rust 2021, it will only capture `fptr1.0.0` *fptr2.0 = 20; //~^ NOTE: in Rust 2018, this closure captures all of `fptr2`, but in Rust 2021, it will only capture `fptr2.0` - }); + }).join().unwrap(); } fn main() { diff --git a/tests/ui/closures/2229_closure_analysis/migrations/multi_diagnostics.stderr b/tests/ui/closures/2229_closure_analysis/migrations/multi_diagnostics.stderr index efb264447f68d..344bc662ee73f 100644 --- a/tests/ui/closures/2229_closure_analysis/migrations/multi_diagnostics.stderr +++ b/tests/ui/closures/2229_closure_analysis/migrations/multi_diagnostics.stderr @@ -111,7 +111,7 @@ LL ~ thread::spawn(move || { let _ = (&fptr1, &fptr2); unsafe { LL | ... LL | -LL ~ } }); +LL ~ } }).join().unwrap(); | error: aborting due to 5 previous errors diff --git a/tests/ui/consts/const-eval/issue-91827-extern-types.rs b/tests/ui/consts/const-eval/issue-91827-extern-types.rs index 43c99799f7704..b1e813f8a39f8 100644 --- a/tests/ui/consts/const-eval/issue-91827-extern-types.rs +++ b/tests/ui/consts/const-eval/issue-91827-extern-types.rs @@ -28,7 +28,10 @@ pub struct ListImpl { impl List { const fn as_slice(&self) -> &[T] { - unsafe { std::slice::from_raw_parts(self.data.as_ptr(), self.len) } + unsafe { + let ptr = (self as *const List).cast::().add(1).cast::(); + std::slice::from_raw_parts(ptr, self.len) + } } } diff --git a/tests/ui/unsized/unsized3-rpass.rs b/tests/ui/unsized/unsized3-rpass.rs index 4d5e89575bef6..a3f92be6cf61f 100644 --- a/tests/ui/unsized/unsized3-rpass.rs +++ b/tests/ui/unsized/unsized3-rpass.rs @@ -59,7 +59,7 @@ pub fn main() { } let data: Box> = Box::new(Foo_ { f: [1, 2, 3] }); - let x: &Foo = mem::transmute(slice::from_raw_parts(&*data, 3)); + let x: &Foo = mem::transmute(ptr::slice_from_raw_parts(&*data, 3)); assert_eq!(x.f.len(), 3); assert_eq!(x.f[0], 1); @@ -70,7 +70,7 @@ pub fn main() { let data: Box<_> = Box::new(Baz_ { f1: 42, f2: ['a' as u8, 'b' as u8, 'c' as u8, 'd' as u8, 'e' as u8] }); - let x: &Baz = mem::transmute(slice::from_raw_parts(&*data, 5)); + let x: &Baz = mem::transmute(ptr::slice_from_raw_parts(&*data, 5)); assert_eq!(x.f1, 42); let chs: Vec = x.f2.chars().collect(); assert_eq!(chs.len(), 5); From 09797a463cd1bc70bc439aaf0c94b7b5a80f5bfb Mon Sep 17 00:00:00 2001 From: Camille GILLOT Date: Mon, 13 Feb 2023 17:01:03 +0000 Subject: [PATCH 19/21] Typo. --- compiler/rustc_mir_transform/src/dataflow_const_prop.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/compiler/rustc_mir_transform/src/dataflow_const_prop.rs b/compiler/rustc_mir_transform/src/dataflow_const_prop.rs index d715e250ca4eb..f3ca2337e59ca 100644 --- a/compiler/rustc_mir_transform/src/dataflow_const_prop.rs +++ b/compiler/rustc_mir_transform/src/dataflow_const_prop.rs @@ -71,7 +71,7 @@ struct ConstAnalysis<'a, 'tcx> { } impl<'tcx> ConstAnalysis<'_, 'tcx> { - fn eval_disciminant( + fn eval_discriminant( &self, enum_ty: Ty<'tcx>, variant_index: VariantIdx, @@ -101,7 +101,7 @@ impl<'tcx> ValueAnalysis<'tcx> for ConstAnalysis<'_, 'tcx> { state.flood_discr(place.as_ref(), &self.map); if self.map.find_discr(place.as_ref()).is_some() { let enum_ty = place.ty(self.local_decls, self.tcx).ty; - if let Some(discr) = self.eval_disciminant(enum_ty, variant_index) { + if let Some(discr) = self.eval_discriminant(enum_ty, variant_index) { state.assign_discr( place.as_ref(), ValueOrPlace::Value(FlatSet::Elem(discr)), @@ -152,7 +152,7 @@ impl<'tcx> ValueAnalysis<'tcx> for ConstAnalysis<'_, 'tcx> { && let Some(discr_idx) = self.map().apply(target_idx, TrackElem::Discriminant) { let enum_ty = target.ty(self.local_decls, self.tcx).ty; - if let Some(discr_val) = self.eval_disciminant(enum_ty, variant_index) { + if let Some(discr_val) = self.eval_discriminant(enum_ty, variant_index) { state.insert_value_idx(discr_idx, FlatSet::Elem(discr_val), &self.map); } } From 3180f1c828636a247777d277f10c9695d7141d20 Mon Sep 17 00:00:00 2001 From: yukang Date: Mon, 13 Feb 2023 17:20:38 +0000 Subject: [PATCH 20/21] Fix #107998, avoid ICE when the generic_span is empty --- compiler/rustc_lint/src/context.rs | 10 ++++++- tests/ui/single-use-lifetime/issue-107998.rs | 9 ++++++ .../single-use-lifetime/issue-107998.stderr | 30 +++++++++++++++++++ 3 files changed, 48 insertions(+), 1 deletion(-) create mode 100644 tests/ui/single-use-lifetime/issue-107998.rs create mode 100644 tests/ui/single-use-lifetime/issue-107998.stderr diff --git a/compiler/rustc_lint/src/context.rs b/compiler/rustc_lint/src/context.rs index d1d4bb375282f..972240f42cf46 100644 --- a/compiler/rustc_lint/src/context.rs +++ b/compiler/rustc_lint/src/context.rs @@ -837,9 +837,17 @@ pub trait LintContext: Sized { (use_span, "'_".to_owned()) }; debug!(?deletion_span, ?use_span); + + // issue 107998 for the case such as a wrong function pointer type + // `deletion_span` is empty and there is no need to report lifetime uses here + let suggestions = if deletion_span.is_empty() { + vec![(use_span, replace_lt)] + } else { + vec![(deletion_span, String::new()), (use_span, replace_lt)] + }; db.multipart_suggestion( msg, - vec![(deletion_span, String::new()), (use_span, replace_lt)], + suggestions, Applicability::MachineApplicable, ); } diff --git a/tests/ui/single-use-lifetime/issue-107998.rs b/tests/ui/single-use-lifetime/issue-107998.rs new file mode 100644 index 0000000000000..f32688d205813 --- /dev/null +++ b/tests/ui/single-use-lifetime/issue-107998.rs @@ -0,0 +1,9 @@ +#![deny(single_use_lifetimes)] + +fn with(f: &fn<'a>(x: &'a i32) -> R) -> R { + //~^ ERROR function pointer types may not have generic parameters + //~| ERROR lifetime parameter `'a` only used once + f(&3) +} + +fn main() {} diff --git a/tests/ui/single-use-lifetime/issue-107998.stderr b/tests/ui/single-use-lifetime/issue-107998.stderr new file mode 100644 index 0000000000000..e870351de9eae --- /dev/null +++ b/tests/ui/single-use-lifetime/issue-107998.stderr @@ -0,0 +1,30 @@ +error: function pointer types may not have generic parameters + --> $DIR/issue-107998.rs:3:18 + | +LL | fn with(f: &fn<'a>(x: &'a i32) -> R) -> R { + | ^^^^ + | +help: consider moving the lifetime parameter to a `for` parameter list + | +LL - fn with(f: &fn<'a>(x: &'a i32) -> R) -> R { +LL + fn with(f: &for<'a> fn(x: &'a i32) -> R) -> R { + | + +error: lifetime parameter `'a` only used once + --> $DIR/issue-107998.rs:3:19 + | +LL | fn with(f: &fn<'a>(x: &'a i32) -> R) -> R { + | ^^ --- + | | | + | | ...is used only here + | | help: elide the single-use lifetime + | this lifetime... + | +note: the lint level is defined here + --> $DIR/issue-107998.rs:1:9 + | +LL | #![deny(single_use_lifetimes)] + | ^^^^^^^^^^^^^^^^^^^^ + +error: aborting due to 2 previous errors + From 826abcc72803fce26bcce574ace5ee6c4bbc936f Mon Sep 17 00:00:00 2001 From: kadmin Date: Tue, 14 Feb 2023 05:01:24 +0000 Subject: [PATCH 21/21] Shrink size of array benchmarks --- library/core/benches/array.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/library/core/benches/array.rs b/library/core/benches/array.rs index 845c60762949b..d8cc44d05c4ba 100644 --- a/library/core/benches/array.rs +++ b/library/core/benches/array.rs @@ -11,9 +11,9 @@ macro_rules! map_array { }; } -map_array!(map_8byte_8byte_8, 0u64, 1u64, 800); -map_array!(map_8byte_8byte_64, 0u64, 1u64, 6400); -map_array!(map_8byte_8byte_256, 0u64, 1u64, 25600); +map_array!(map_8byte_8byte_8, 0u64, 1u64, 80); +map_array!(map_8byte_8byte_64, 0u64, 1u64, 640); +map_array!(map_8byte_8byte_256, 0u64, 1u64, 2560); -map_array!(map_8byte_256byte_256, 0u64, [0u64; 4], 25600); -map_array!(map_256byte_8byte_256, [0u64; 4], 0u64, 25600); +map_array!(map_8byte_256byte_256, 0u64, [0u64; 4], 2560); +map_array!(map_256byte_8byte_256, [0u64; 4], 0u64, 2560);