From 34de2573f5a66f6f000d71f6933aa190dcfb17a1 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Fri, 25 Nov 2022 20:48:53 +0100 Subject: [PATCH 01/13] PERs are homogeneous --- library/core/src/cmp.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/library/core/src/cmp.rs b/library/core/src/cmp.rs index 5db5cbfc3dfdd..949896e574806 100644 --- a/library/core/src/cmp.rs +++ b/library/core/src/cmp.rs @@ -29,8 +29,7 @@ use crate::marker::StructuralPartialEq; use self::Ordering::*; -/// Trait for equality comparisons which are [partial equivalence -/// relations](https://en.wikipedia.org/wiki/Partial_equivalence_relation). +/// Trait for equality comparisons. /// /// `x.eq(y)` can also be written `x == y`, and `x.ne(y)` can be written `x != y`. /// We use the easier-to-read infix notation in the remainder of this documentation. @@ -38,6 +37,8 @@ use self::Ordering::*; /// This trait allows for partial equality, for types that do not have a full /// equivalence relation. For example, in floating point numbers `NaN != NaN`, /// so floating point types implement `PartialEq` but not [`trait@Eq`]. +/// Formally speaking, when `Rhs == Self`, this trait corresponds to a [partial equivalence +/// relation](https://en.wikipedia.org/wiki/Partial_equivalence_relation). /// /// Implementations must ensure that `eq` and `ne` are consistent with each other: /// From 71fd3abc73471bf1b68666c980d73c4385e5e9a7 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Sat, 26 Nov 2022 15:00:39 -0500 Subject: [PATCH 02/13] Don't update submodules for `x setup` Before, the submodule handling was very jank and would update *between two interactive prompts*: ``` ; x setup Building rustbuild Finished dev [unoptimized] target(s) in 0.05s Welcome to the Rust project! What do you want to do with x.py? a) library: Contribute to the standard library Please choose one (a/b/c/d/e): a Updating submodule library/backtrace Submodule 'library/backtrace' (https://github.com/rust-lang/backtrace-rs.git) registered for path 'library/backtrace' error: you asked `x.py` to setup a new config file, but one already exists at `config.toml` Build completed unsuccessfully in 0:00:02 ``` That's not a great user experience because you need to wait a long time between prompts. It would be possible to move the submodule handling either before or after the prompt, but it seems better to just not require submodules to be checked out at all, to minimize the time spend waiting just to create a new configuration. --- src/bootstrap/flags.rs | 9 +++++---- src/bootstrap/lib.rs | 44 ++++++++++++++++++++++++------------------ src/bootstrap/setup.rs | 3 ++- 3 files changed, 32 insertions(+), 24 deletions(-) diff --git a/src/bootstrap/flags.rs b/src/bootstrap/flags.rs index 2001e29bd2ead..37a8eb884efb0 100644 --- a/src/bootstrap/flags.rs +++ b/src/bootstrap/flags.rs @@ -143,7 +143,7 @@ pub enum Subcommand { args: Vec, }, Setup { - profile: Profile, + profile: Option, }, } @@ -628,14 +628,15 @@ Arguments: |path| format!("{} is not a valid UTF8 string", path.to_string_lossy()) )); - profile_string.parse().unwrap_or_else(|err| { + let profile = profile_string.parse().unwrap_or_else(|err| { eprintln!("error: {}", err); eprintln!("help: the available profiles are:"); eprint!("{}", Profile::all_for_help("- ")); crate::detail_exit(1); - }) + }); + Some(profile) } else { - t!(crate::setup::interactive_path()) + None }; Subcommand::Setup { profile } } diff --git a/src/bootstrap/lib.rs b/src/bootstrap/lib.rs index f4fa556b97450..60ce431cb0c39 100644 --- a/src/bootstrap/lib.rs +++ b/src/bootstrap/lib.rs @@ -542,16 +542,6 @@ impl Build { metrics: metrics::BuildMetrics::init(), }; - build.verbose("finding compilers"); - cc_detect::find(&mut build); - // When running `setup`, the profile is about to change, so any requirements we have now may - // be different on the next invocation. Don't check for them until the next time x.py is - // run. This is ok because `setup` never runs any build commands, so it won't fail if commands are missing. - if !matches!(build.config.cmd, Subcommand::Setup { .. }) { - build.verbose("running sanity check"); - sanity::check(&mut build); - } - // If local-rust is the same major.minor as the current version, then force a // local-rebuild let local_version_verbose = @@ -567,16 +557,32 @@ impl Build { build.local_rebuild = true; } - // Make sure we update these before gathering metadata so we don't get an error about missing - // Cargo.toml files. - let rust_submodules = - ["src/tools/rust-installer", "src/tools/cargo", "library/backtrace", "library/stdarch"]; - for s in rust_submodules { - build.update_submodule(Path::new(s)); - } + build.verbose("finding compilers"); + cc_detect::find(&mut build); + // When running `setup`, the profile is about to change, so any requirements we have now may + // be different on the next invocation. Don't check for them until the next time x.py is + // run. This is ok because `setup` never runs any build commands, so it won't fail if commands are missing. + // + // Similarly, for `setup` we don't actually need submodules or cargo metadata. + if !matches!(build.config.cmd, Subcommand::Setup { .. }) { + build.verbose("running sanity check"); + sanity::check(&mut build); - build.verbose("learning about cargo"); - metadata::build(&mut build); + // Make sure we update these before gathering metadata so we don't get an error about missing + // Cargo.toml files. + let rust_submodules = [ + "src/tools/rust-installer", + "src/tools/cargo", + "library/backtrace", + "library/stdarch", + ]; + for s in rust_submodules { + build.update_submodule(Path::new(s)); + } + + build.verbose("learning about cargo"); + metadata::build(&mut build); + } build } diff --git a/src/bootstrap/setup.rs b/src/bootstrap/setup.rs index 04480277fe047..2507b7a29b06f 100644 --- a/src/bootstrap/setup.rs +++ b/src/bootstrap/setup.rs @@ -81,8 +81,9 @@ impl fmt::Display for Profile { } } -pub fn setup(config: &Config, profile: Profile) { +pub fn setup(config: &Config, profile: Option) { let path = &config.config.clone().unwrap_or(PathBuf::from("config.toml")); + let profile = profile.unwrap_or_else(|| t!(interactive_path())); if path.exists() { eprintln!( From 86251dabac6252ebd3c2a90bd4695563d202b919 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Sat, 26 Nov 2022 15:08:41 -0500 Subject: [PATCH 03/13] Refactor `setup_config_toml` into a function --- src/bootstrap/setup.rs | 62 +++++++++++++++++++++--------------------- 1 file changed, 31 insertions(+), 31 deletions(-) diff --git a/src/bootstrap/setup.rs b/src/bootstrap/setup.rs index 2507b7a29b06f..bad6accc78472 100644 --- a/src/bootstrap/setup.rs +++ b/src/bootstrap/setup.rs @@ -1,15 +1,13 @@ +use crate::Config; use crate::{t, VERSION}; -use crate::{Config, TargetSelection}; use std::env::consts::EXE_SUFFIX; use std::fmt::Write as _; use std::fs::File; +use std::io::Write; use std::path::{Path, PathBuf, MAIN_SEPARATOR}; use std::process::Command; use std::str::FromStr; -use std::{ - env, fmt, fs, - io::{self, Write}, -}; +use std::{fmt, fs, io}; #[derive(Clone, Copy, Debug, Eq, PartialEq)] pub enum Profile { @@ -84,34 +82,10 @@ impl fmt::Display for Profile { pub fn setup(config: &Config, profile: Option) { let path = &config.config.clone().unwrap_or(PathBuf::from("config.toml")); let profile = profile.unwrap_or_else(|| t!(interactive_path())); + setup_config_toml(path, profile, config); - if path.exists() { - eprintln!( - "error: you asked `x.py` to setup a new config file, but one already exists at `{}`", - path.display() - ); - eprintln!("help: try adding `profile = \"{}\"` at the top of {}", profile, path.display()); - eprintln!( - "note: this will use the configuration in {}", - profile.include_path(&config.src).display() - ); - crate::detail_exit(1); - } - - let settings = format!( - "# Includes one of the default files in src/bootstrap/defaults\n\ - profile = \"{}\"\n\ - changelog-seen = {}\n", - profile, VERSION - ); - t!(fs::write(path, settings)); - - let include_path = profile.include_path(&config.src); - println!("`x.py` will now use the configuration at {}", include_path.display()); - - let build = TargetSelection::from_user(&env!("BUILD_TRIPLE")); let stage_path = - ["build", build.rustc_target_arg(), "stage1"].join(&MAIN_SEPARATOR.to_string()); + ["build", config.build.rustc_target_arg(), "stage1"].join(&MAIN_SEPARATOR.to_string()); println!(); @@ -153,6 +127,32 @@ pub fn setup(config: &Config, profile: Option) { } } +fn setup_config_toml(path: &PathBuf, profile: Profile, config: &Config) { + if path.exists() { + eprintln!( + "error: you asked `x.py` to setup a new config file, but one already exists at `{}`", + path.display() + ); + eprintln!("help: try adding `profile = \"{}\"` at the top of {}", profile, path.display()); + eprintln!( + "note: this will use the configuration in {}", + profile.include_path(&config.src).display() + ); + crate::detail_exit(1); + } + + let settings = format!( + "# Includes one of the default files in src/bootstrap/defaults\n\ + profile = \"{}\"\n\ + changelog-seen = {}\n", + profile, VERSION + ); + t!(fs::write(path, settings)); + + let include_path = profile.include_path(&config.src); + println!("`x.py` will now use the configuration at {}", include_path.display()); +} + fn rustup_installed() -> bool { Command::new("rustup") .arg("--version") From b771d901f735aa6d4078aed92b03e30eba35bf5a Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Sat, 26 Nov 2022 15:43:10 -0500 Subject: [PATCH 04/13] Revamp the order `setup` executes - Create `config.toml` last. It's the most likely to error, and used to stop later steps from executing - Don't print an error message + exit if the git hook already exists; that's expected --- src/bootstrap/setup.rs | 28 +++++++++++++++------------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/src/bootstrap/setup.rs b/src/bootstrap/setup.rs index bad6accc78472..c7f98a7d0d149 100644 --- a/src/bootstrap/setup.rs +++ b/src/bootstrap/setup.rs @@ -80,15 +80,10 @@ impl fmt::Display for Profile { } pub fn setup(config: &Config, profile: Option) { - let path = &config.config.clone().unwrap_or(PathBuf::from("config.toml")); let profile = profile.unwrap_or_else(|| t!(interactive_path())); - setup_config_toml(path, profile, config); - let stage_path = ["build", config.build.rustc_target_arg(), "stage1"].join(&MAIN_SEPARATOR.to_string()); - println!(); - if !rustup_installed() && profile != Profile::User { eprintln!("`rustup` is not installed; cannot link `stage1` toolchain"); } else if stage_dir_exists(&stage_path[..]) { @@ -109,8 +104,6 @@ pub fn setup(config: &Config, profile: Option) { Profile::User => &["dist", "build"], }; - println!(); - t!(install_git_hook_maybe(&config)); println!(); @@ -125,10 +118,14 @@ pub fn setup(config: &Config, profile: Option) { "For more suggestions, see https://rustc-dev-guide.rust-lang.org/building/suggested.html" ); } + + let path = &config.config.clone().unwrap_or(PathBuf::from("config.toml")); + setup_config_toml(path, profile, config); } fn setup_config_toml(path: &PathBuf, profile: Profile, config: &Config) { if path.exists() { + eprintln!(); eprintln!( "error: you asked `x.py` to setup a new config file, but one already exists at `{}`", path.display() @@ -304,7 +301,18 @@ pub fn interactive_path() -> io::Result { // install a git hook to automatically run tidy --bless, if they want fn install_git_hook_maybe(config: &Config) -> io::Result<()> { + let git = t!(config.git().args(&["rev-parse", "--git-common-dir"]).output().map(|output| { + assert!(output.status.success(), "failed to run `git`"); + PathBuf::from(t!(String::from_utf8(output.stdout)).trim()) + })); + let dst = git.join("hooks").join("pre-push"); + if dst.exists() { + // The git hook has already been set up, or the user already has a custom hook. + return Ok(()); + } + let mut input = String::new(); + println!(); println!( "Rust's CI will automatically fail if it doesn't pass `tidy`, the internal tool for ensuring code quality. If you'd like, x.py can install a git hook for you that will automatically run `tidy --bless` before @@ -330,12 +338,6 @@ undesirable, simply delete the `pre-push` file from .git/hooks." if should_install { let src = config.src.join("src").join("etc").join("pre-push.sh"); - let git = - t!(config.git().args(&["rev-parse", "--git-common-dir"]).output().map(|output| { - assert!(output.status.success(), "failed to run `git`"); - PathBuf::from(t!(String::from_utf8(output.stdout)).trim()) - })); - let dst = git.join("hooks").join("pre-push"); match fs::hard_link(src, &dst) { Err(e) => eprintln!( "error: could not create hook {}: do you already have the git hook installed?\n{}", From ab89c171bdb29194baca274decf86acfecd2bb9d Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Sat, 26 Nov 2022 16:11:07 -0500 Subject: [PATCH 05/13] Ensure required submodules at the same time as updating existing submodules In practice, this would always happen at the same time, but putting them next to each other makes that more obvious and ensures it doesn't change in the future. It also avoids the difference avoiding `cargo metadata` somehow. --- src/bootstrap/lib.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/bootstrap/lib.rs b/src/bootstrap/lib.rs index 60ce431cb0c39..86b195fb89e02 100644 --- a/src/bootstrap/lib.rs +++ b/src/bootstrap/lib.rs @@ -579,6 +579,8 @@ impl Build { for s in rust_submodules { build.update_submodule(Path::new(s)); } + // Now, update all existing submodules. + build.update_existing_submodules(); build.verbose("learning about cargo"); metadata::build(&mut build); @@ -660,7 +662,7 @@ impl Build { /// If any submodule has been initialized already, sync it unconditionally. /// This avoids contributors checking in a submodule change by accident. - pub fn maybe_update_submodules(&self) { + pub fn update_existing_submodules(&self) { // Avoid running git when there isn't a git checkout. if !self.config.submodules(&self.rust_info()) { return; @@ -689,8 +691,6 @@ impl Build { job::setup(self); } - self.maybe_update_submodules(); - if let Subcommand::Format { check, paths } = &self.config.cmd { return format::format(&builder::Builder::new(&self), *check, &paths); } From 51ac2af99ffd46cf83760ceef2404dad0ce238a9 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 3 Dec 2022 13:33:00 +0100 Subject: [PATCH 06/13] interpret: clobber return place when calling function --- compiler/rustc_const_eval/src/interpret/eval_context.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/compiler/rustc_const_eval/src/interpret/eval_context.rs b/compiler/rustc_const_eval/src/interpret/eval_context.rs index 79450fccfc4d8..e17d3e516a64f 100644 --- a/compiler/rustc_const_eval/src/interpret/eval_context.rs +++ b/compiler/rustc_const_eval/src/interpret/eval_context.rs @@ -676,6 +676,10 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { return_to_block: StackPopCleanup, ) -> InterpResult<'tcx> { trace!("body: {:#?}", body); + // Clobber previous return place contents, nobody is supposed to be able to see them any more + // This also checks dereferenceable, but not align. We rely on all constructed places being + // sufficiently aligned (in particular we rely on `deref_operand` checking alignment). + self.write_uninit(return_place)?; // first push a stack frame so we have access to the local substs let pre_frame = Frame { body, From 3b4cbe9095d83d13feb13060cc66f3e50bd75350 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 3 Dec 2022 18:38:04 +0100 Subject: [PATCH 07/13] add test for self-referential future --- .../future-self-referential.rs | 102 ++++++++++++++++++ 1 file changed, 102 insertions(+) create mode 100644 src/tools/miri/tests/pass/stacked-borrows/future-self-referential.rs diff --git a/src/tools/miri/tests/pass/stacked-borrows/future-self-referential.rs b/src/tools/miri/tests/pass/stacked-borrows/future-self-referential.rs new file mode 100644 index 0000000000000..3ba21552fd362 --- /dev/null +++ b/src/tools/miri/tests/pass/stacked-borrows/future-self-referential.rs @@ -0,0 +1,102 @@ +#![feature(pin_macro)] + +use std::future::*; +use std::marker::PhantomPinned; +use std::pin::*; +use std::ptr; +use std::task::*; + +struct Delay { + delay: usize, +} + +impl Delay { + fn new(delay: usize) -> Self { + Delay { delay } + } +} + +impl Future for Delay { + type Output = (); + fn poll(mut self: Pin<&mut Self>, _cx: &mut Context<'_>) -> Poll<()> { + if self.delay > 0 { + self.delay -= 1; + Poll::Pending + } else { + Poll::Ready(()) + } + } +} + +async fn do_stuff() { + (&mut Delay::new(1)).await; +} + +// Same thing implemented by hand +struct DoStuff { + state: usize, + delay: Delay, + delay_ref: *mut Delay, + _marker: PhantomPinned, +} + +impl DoStuff { + fn new() -> Self { + DoStuff { + state: 0, + delay: Delay::new(1), + delay_ref: ptr::null_mut(), + _marker: PhantomPinned, + } + } +} + +impl Future for DoStuff { + type Output = (); + fn poll(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll<()> { + unsafe { + let this = self.get_unchecked_mut(); + match this.state { + 0 => { + // Set up self-ref. + this.delay_ref = &mut this.delay; + // Move to next state. + this.state = 1; + Poll::Pending + } + 1 => { + let delay = &mut *this.delay_ref; + Pin::new_unchecked(delay).poll(cx) + } + _ => unreachable!(), + } + } + } +} + +fn run_fut(fut: impl Future) -> T { + use std::sync::Arc; + + struct MyWaker; + impl Wake for MyWaker { + fn wake(self: Arc) { + unimplemented!() + } + } + + let waker = Waker::from(Arc::new(MyWaker)); + let mut context = Context::from_waker(&waker); + + let mut pinned = pin!(fut); + loop { + match pinned.as_mut().poll(&mut context) { + Poll::Pending => continue, + Poll::Ready(v) => return v, + } + } +} + +fn main() { + run_fut(do_stuff()); + run_fut(DoStuff::new()); +} From 3fa692c8b22cfcd4d13e0568056b680235d0cef0 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 3 Dec 2022 19:05:46 +0100 Subject: [PATCH 08/13] for now, do not do fake reads on non-Unpin mutable references --- .../src/borrow_tracker/stacked_borrows/mod.rs | 23 +++++++++------ .../notunpin_dereferenceable_fakeread.rs | 17 ----------- .../notunpin_dereferenceable_fakeread.stderr | 28 ------------------- 3 files changed, 14 insertions(+), 54 deletions(-) delete mode 100644 src/tools/miri/tests/fail/stacked_borrows/notunpin_dereferenceable_fakeread.rs delete mode 100644 src/tools/miri/tests/fail/stacked_borrows/notunpin_dereferenceable_fakeread.stderr diff --git a/src/tools/miri/src/borrow_tracker/stacked_borrows/mod.rs b/src/tools/miri/src/borrow_tracker/stacked_borrows/mod.rs index 50c2ad75ca71e..bcac873251f58 100644 --- a/src/tools/miri/src/borrow_tracker/stacked_borrows/mod.rs +++ b/src/tools/miri/src/borrow_tracker/stacked_borrows/mod.rs @@ -45,7 +45,9 @@ pub struct Stacks { /// new pointer. #[derive(Copy, Clone, Hash, PartialEq, Eq)] enum RefKind { - /// `&mut` and `Box`. + /// `Box`. + Box, + /// `&mut`. Unique { two_phase: bool }, /// `&` with or without interior mutability. Shared, @@ -56,6 +58,7 @@ enum RefKind { impl fmt::Display for RefKind { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match self { + RefKind::Box => write!(f, "Box"), RefKind::Unique { two_phase: false } => write!(f, "unique reference"), RefKind::Unique { two_phase: true } => write!(f, "unique reference (two-phase)"), RefKind::Shared => write!(f, "shared reference"), @@ -654,15 +657,17 @@ trait EvalContextPrivExt<'mir: 'ecx, 'tcx: 'mir, 'ecx>: crate::MiriInterpCxExt<' let (perm, access) = match kind { RefKind::Unique { two_phase } => { // Permission is Unique only if the type is `Unpin` and this is not twophase - let perm = if !two_phase && place.layout.ty.is_unpin(*this.tcx, this.param_env()) { - Permission::Unique + if !two_phase && place.layout.ty.is_unpin(*this.tcx, this.param_env()) { + (Permission::Unique, Some(AccessKind::Write)) } else { - Permission::SharedReadWrite - }; - // We do an access for all full borrows, even if `!Unpin`. - let access = if !two_phase { Some(AccessKind::Write) } else { None }; - (perm, access) + // FIXME: We emit `dereferenceable` for `!Unpin` mutable references, so we + // should do fake accesses here. But then we run into + // , so for now + // we don't do that. + (Permission::SharedReadWrite, None) + } } + RefKind::Box => (Permission::Unique, Some(AccessKind::Write)), RefKind::Raw { mutable: true } => { // Creating a raw ptr does not count as an access (Permission::SharedReadWrite, None) @@ -853,7 +858,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { // Boxes get a weak protectors, since they may be deallocated. self.retag_place( place, - RefKind::Unique { two_phase: false }, + RefKind::Box, self.retag_cause, /*protector*/ (self.kind == RetagKind::FnEntry).then_some(ProtectorKind::WeakProtector), diff --git a/src/tools/miri/tests/fail/stacked_borrows/notunpin_dereferenceable_fakeread.rs b/src/tools/miri/tests/fail/stacked_borrows/notunpin_dereferenceable_fakeread.rs deleted file mode 100644 index d660921bfe6e2..0000000000000 --- a/src/tools/miri/tests/fail/stacked_borrows/notunpin_dereferenceable_fakeread.rs +++ /dev/null @@ -1,17 +0,0 @@ -//! Reborrowing a `&mut !Unpin` must still act like a (fake) read. -use std::marker::PhantomPinned; - -struct NotUnpin(i32, PhantomPinned); - -fn main() { - unsafe { - let mut x = NotUnpin(0, PhantomPinned); - // Mutable borrow of `Unpin` field (with lifetime laundering) - let fieldref = &mut *(&mut x.0 as *mut i32); - // Mutable reborrow of the entire `x`, which is `!Unpin` but should - // still count as a read since we would add `dereferenceable`. - let _xref = &mut x; - // That read should have invalidated `fieldref`. - *fieldref = 0; //~ ERROR: /write access .* tag does not exist in the borrow stack/ - } -} diff --git a/src/tools/miri/tests/fail/stacked_borrows/notunpin_dereferenceable_fakeread.stderr b/src/tools/miri/tests/fail/stacked_borrows/notunpin_dereferenceable_fakeread.stderr deleted file mode 100644 index 3ef8a8e0e9c6a..0000000000000 --- a/src/tools/miri/tests/fail/stacked_borrows/notunpin_dereferenceable_fakeread.stderr +++ /dev/null @@ -1,28 +0,0 @@ -error: Undefined Behavior: attempting a write access using at ALLOC[0x0], but that tag does not exist in the borrow stack for this location - --> $DIR/notunpin_dereferenceable_fakeread.rs:LL:CC - | -LL | *fieldref = 0; - | ^^^^^^^^^^^^^ - | | - | attempting a write access using at ALLOC[0x0], but that tag does not exist in the borrow stack for this location - | this error occurs as part of an access at ALLOC[0x0..0x4] - | - = help: this indicates a potential bug in the program: it performed an invalid operation, but the Stacked Borrows rules it violated are still experimental - = help: see https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/stacked-borrows.md for further information -help: was created by a Unique retag at offsets [0x0..0x4] - --> $DIR/notunpin_dereferenceable_fakeread.rs:LL:CC - | -LL | let fieldref = &mut *(&mut x.0 as *mut i32); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -help: was later invalidated at offsets [0x0..0x4] by a SharedReadWrite retag - --> $DIR/notunpin_dereferenceable_fakeread.rs:LL:CC - | -LL | let _xref = &mut x; - | ^^^^^^ - = note: BACKTRACE: - = note: inside `main` at $DIR/notunpin_dereferenceable_fakeread.rs:LL:CC - -note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace - -error: aborting due to previous error - From 35c00a9731809292959e98b1e7da3c6e2673f3cc Mon Sep 17 00:00:00 2001 From: Lukas Markeffsky <@> Date: Sat, 3 Dec 2022 20:02:39 +0100 Subject: [PATCH 09/13] suggest parenthesis around ExprWithBlock BinOp ExprWithBlock --- .../src/fn_ctxt/suggestions.rs | 4 +- compiler/rustc_parse/src/parser/expr.rs | 14 +----- src/test/ui/parser/expr-as-stmt.fixed | 12 ++++++ src/test/ui/parser/expr-as-stmt.rs | 12 ++++++ src/test/ui/parser/expr-as-stmt.stderr | 43 ++++++++++++++++++- 5 files changed, 71 insertions(+), 14 deletions(-) diff --git a/compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs b/compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs index b9a8d16311c93..56b889fdcdda6 100644 --- a/compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs +++ b/compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs @@ -32,11 +32,13 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { } pub(in super::super) fn suggest_semicolon_at_end(&self, span: Span, err: &mut Diagnostic) { + // This suggestion is incorrect for + // fn foo() -> bool { match () { () => true } || match () { () => true } } err.span_suggestion_short( span.shrink_to_hi(), "consider using a semicolon here", ";", - Applicability::MachineApplicable, + Applicability::MaybeIncorrect, ); } diff --git a/compiler/rustc_parse/src/parser/expr.rs b/compiler/rustc_parse/src/parser/expr.rs index e0443a697b504..377638ab96870 100644 --- a/compiler/rustc_parse/src/parser/expr.rs +++ b/compiler/rustc_parse/src/parser/expr.rs @@ -390,20 +390,10 @@ impl<'a> Parser<'a> { // want to keep their span info to improve diagnostics in these cases in a later stage. (true, Some(AssocOp::Multiply)) | // `{ 42 } *foo = bar;` or `{ 42 } * 3` (true, Some(AssocOp::Subtract)) | // `{ 42 } -5` - (true, Some(AssocOp::Add)) // `{ 42 } + 42 - // If the next token is a keyword, then the tokens above *are* unambiguously incorrect: - // `if x { a } else { b } && if y { c } else { d }` - if !self.look_ahead(1, |t| t.is_used_keyword()) => { - // These cases are ambiguous and can't be identified in the parser alone. - let sp = self.sess.source_map().start_point(self.token.span); - self.sess.ambiguous_block_expr_parse.borrow_mut().insert(sp, lhs.span); - false - } - (true, Some(AssocOp::LAnd)) | + (true, Some(AssocOp::Add)) | // `{ 42 } + 42 + (true, Some(AssocOp::LAnd)) | // `{ 42 } &&x` (#61475) or `{ 42 } && if x { 1 } else { 0 }` (true, Some(AssocOp::LOr)) | (true, Some(AssocOp::BitOr)) => { - // `{ 42 } &&x` (#61475) or `{ 42 } && if x { 1 } else { 0 }`. Separated from the - // above due to #74233. // These cases are ambiguous and can't be identified in the parser alone. // // Bitwise AND is left out because guessing intent is hard. We can make diff --git a/src/test/ui/parser/expr-as-stmt.fixed b/src/test/ui/parser/expr-as-stmt.fixed index 36709eea17c21..b06f62794c4e3 100644 --- a/src/test/ui/parser/expr-as-stmt.fixed +++ b/src/test/ui/parser/expr-as-stmt.fixed @@ -64,4 +64,16 @@ fn asteroids() -> impl FnOnce() -> bool { { foo(); } || { true } //~ ERROR E0308 } +// https://github.com/rust-lang/rust/issues/105179 +fn r#match() -> i32 { + (match () { () => 1 }) + match () { () => 1 } //~ ERROR expected expression, found `+` + //~^ ERROR mismatched types +} + +// https://github.com/rust-lang/rust/issues/102171 +fn r#unsafe() -> i32 { + (unsafe { 1 }) + unsafe { 1 } //~ ERROR expected expression, found `+` + //~^ ERROR mismatched types +} + fn main() {} diff --git a/src/test/ui/parser/expr-as-stmt.rs b/src/test/ui/parser/expr-as-stmt.rs index 92bb972b24020..b39d2b88647c0 100644 --- a/src/test/ui/parser/expr-as-stmt.rs +++ b/src/test/ui/parser/expr-as-stmt.rs @@ -64,4 +64,16 @@ fn asteroids() -> impl FnOnce() -> bool { { foo() } || { true } //~ ERROR E0308 } +// https://github.com/rust-lang/rust/issues/105179 +fn r#match() -> i32 { + match () { () => 1 } + match () { () => 1 } //~ ERROR expected expression, found `+` + //~^ ERROR mismatched types +} + +// https://github.com/rust-lang/rust/issues/102171 +fn r#unsafe() -> i32 { + unsafe { 1 } + unsafe { 1 } //~ ERROR expected expression, found `+` + //~^ ERROR mismatched types +} + fn main() {} diff --git a/src/test/ui/parser/expr-as-stmt.stderr b/src/test/ui/parser/expr-as-stmt.stderr index 6da4ac34067e9..18c8b0b7c506b 100644 --- a/src/test/ui/parser/expr-as-stmt.stderr +++ b/src/test/ui/parser/expr-as-stmt.stderr @@ -55,6 +55,28 @@ help: parentheses are required to parse this as an expression LL | ({ true }) | { true } | + + +error: expected expression, found `+` + --> $DIR/expr-as-stmt.rs:69:26 + | +LL | match () { () => 1 } + match () { () => 1 } + | ^ expected expression + | +help: parentheses are required to parse this as an expression + | +LL | (match () { () => 1 }) + match () { () => 1 } + | + + + +error: expected expression, found `+` + --> $DIR/expr-as-stmt.rs:75:18 + | +LL | unsafe { 1 } + unsafe { 1 } + | ^ expected expression + | +help: parentheses are required to parse this as an expression + | +LL | (unsafe { 1 }) + unsafe { 1 } + | + + + error[E0308]: mismatched types --> $DIR/expr-as-stmt.rs:64:7 | @@ -201,7 +223,26 @@ help: parentheses are required to parse this as an expression LL | ({ true }) || { true } | + + -error: aborting due to 18 previous errors +error[E0308]: mismatched types + --> $DIR/expr-as-stmt.rs:69:5 + | +LL | match () { () => 1 } + match () { () => 1 } + | ^^^^^^^^^^^^^^^^^^^^- help: consider using a semicolon here + | | + | expected `()`, found integer + +error[E0308]: mismatched types + --> $DIR/expr-as-stmt.rs:75:14 + | +LL | unsafe { 1 } + unsafe { 1 } + | ^ expected `()`, found integer + | +help: you might have meant to return this value + | +LL | unsafe { return 1; } + unsafe { 1 } + | ++++++ + + +error: aborting due to 22 previous errors Some errors have detailed explanations: E0308, E0600, E0614. For more information about an error, try `rustc --explain E0308`. From c808d0b289c1f8e0f26885bf44cb2ba1873ba10e Mon Sep 17 00:00:00 2001 From: Lukas Markeffsky <@> Date: Sat, 3 Dec 2022 23:54:06 +0100 Subject: [PATCH 10/13] more comments --- compiler/rustc_parse/src/parser/expr.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/compiler/rustc_parse/src/parser/expr.rs b/compiler/rustc_parse/src/parser/expr.rs index 377638ab96870..0a948a2810a4e 100644 --- a/compiler/rustc_parse/src/parser/expr.rs +++ b/compiler/rustc_parse/src/parser/expr.rs @@ -390,10 +390,11 @@ impl<'a> Parser<'a> { // want to keep their span info to improve diagnostics in these cases in a later stage. (true, Some(AssocOp::Multiply)) | // `{ 42 } *foo = bar;` or `{ 42 } * 3` (true, Some(AssocOp::Subtract)) | // `{ 42 } -5` - (true, Some(AssocOp::Add)) | // `{ 42 } + 42 + (true, Some(AssocOp::Add)) | // `{ 42 } + 42` (unary plus) (true, Some(AssocOp::LAnd)) | // `{ 42 } &&x` (#61475) or `{ 42 } && if x { 1 } else { 0 }` - (true, Some(AssocOp::LOr)) | - (true, Some(AssocOp::BitOr)) => { + (true, Some(AssocOp::LOr)) | // `{ 42 } || 42` ("logical or" or closure) + (true, Some(AssocOp::BitOr)) // `{ 42 } | 42` or `{ 42 } |x| 42` + => { // These cases are ambiguous and can't be identified in the parser alone. // // Bitwise AND is left out because guessing intent is hard. We can make From 6cc86db06957194191028c9c7582d920f6aeb6d0 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Sun, 4 Dec 2022 15:21:05 +0100 Subject: [PATCH 11/13] Add small comment explaining what `method-margins.goml` test is about --- src/test/rustdoc-gui/method-margins.goml | 1 + 1 file changed, 1 insertion(+) diff --git a/src/test/rustdoc-gui/method-margins.goml b/src/test/rustdoc-gui/method-margins.goml index 397bcd40b36c4..ed36bcdec17d2 100644 --- a/src/test/rustdoc-gui/method-margins.goml +++ b/src/test/rustdoc-gui/method-margins.goml @@ -1,3 +1,4 @@ +// This test ensures that the margins on methods are coherent inside an impl block. goto: "file://" + |DOC_PATH| + "/test_docs/trait_members/struct.HasTrait.html#impl-TraitMembers-for-HasTrait" assert-count: ("#trait-implementations-list > .rustdoc-toggle", 1) From 58110572fb23e0ad1944bdf3d8cdb3f84cb91aa2 Mon Sep 17 00:00:00 2001 From: Rageking8 Date: Mon, 5 Dec 2022 16:42:36 +0800 Subject: [PATCH 12/13] fix dupe word typos --- compiler/rustc_const_eval/src/const_eval/machine.rs | 2 +- compiler/rustc_mir_dataflow/src/value_analysis.rs | 2 +- compiler/rustc_resolve/src/late.rs | 2 +- library/core/src/iter/sources/repeat_n.rs | 2 +- library/core/src/str/pattern.rs | 2 +- library/std/src/sync/mpmc/array.rs | 2 +- library/std/src/thread/scoped.rs | 2 +- .../match-requires-both-partialeq-and-eq.rs | 2 +- 8 files changed, 8 insertions(+), 8 deletions(-) diff --git a/compiler/rustc_const_eval/src/const_eval/machine.rs b/compiler/rustc_const_eval/src/const_eval/machine.rs index 04e68b9645525..3dfded2d930a0 100644 --- a/compiler/rustc_const_eval/src/const_eval/machine.rs +++ b/compiler/rustc_const_eval/src/const_eval/machine.rs @@ -240,7 +240,7 @@ impl<'mir, 'tcx: 'mir> CompileTimeEvalContext<'mir, 'tcx> { let align = ImmTy::from_uint(target_align, args[1].layout).into(); let fn_abi = self.fn_abi_of_instance(instance, ty::List::empty())?; - // We replace the entire entire function call with a "tail call". + // We replace the entire function call with a "tail call". // Note that this happens before the frame of the original function // is pushed on the stack. self.eval_fn_call( diff --git a/compiler/rustc_mir_dataflow/src/value_analysis.rs b/compiler/rustc_mir_dataflow/src/value_analysis.rs index cc69a1bb02db1..7df0114226418 100644 --- a/compiler/rustc_mir_dataflow/src/value_analysis.rs +++ b/compiler/rustc_mir_dataflow/src/value_analysis.rs @@ -406,7 +406,7 @@ impl Clone for StateData { /// The dataflow state for an instance of [`ValueAnalysis`]. /// /// Every instance specifies a lattice that represents the possible values of a single tracked -/// place. If we call this lattice `V` and set set of tracked places `P`, then a [`State`] is an +/// place. If we call this lattice `V` and set of tracked places `P`, then a [`State`] is an /// element of `{unreachable} ∪ (P -> V)`. This again forms a lattice, where the bottom element is /// `unreachable` and the top element is the mapping `p ↦ ⊤`. Note that the mapping `p ↦ ⊥` is not /// the bottom element (because joining an unreachable and any other reachable state yields a diff --git a/compiler/rustc_resolve/src/late.rs b/compiler/rustc_resolve/src/late.rs index 51a53f7b37cc2..cf241b8922d61 100644 --- a/compiler/rustc_resolve/src/late.rs +++ b/compiler/rustc_resolve/src/late.rs @@ -1927,7 +1927,7 @@ impl<'a: 'ast, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> { // We have a single lifetime => success. elision_lifetime = Elision::Param(res) } else { - // We have have multiple lifetimes => error. + // We have multiple lifetimes => error. elision_lifetime = Elision::Err; } } diff --git a/library/core/src/iter/sources/repeat_n.rs b/library/core/src/iter/sources/repeat_n.rs index dc69bf4df595e..fd8d25ce1a55e 100644 --- a/library/core/src/iter/sources/repeat_n.rs +++ b/library/core/src/iter/sources/repeat_n.rs @@ -20,7 +20,7 @@ use crate::mem::ManuallyDrop; /// #![feature(iter_repeat_n)] /// use std::iter; /// -/// // four of the the number four: +/// // four of the number four: /// let mut four_fours = iter::repeat_n(4, 4); /// /// assert_eq!(Some(4), four_fours.next()); diff --git a/library/core/src/str/pattern.rs b/library/core/src/str/pattern.rs index d76d6f8b2a2d9..19da6d2fbecbc 100644 --- a/library/core/src/str/pattern.rs +++ b/library/core/src/str/pattern.rs @@ -1894,7 +1894,7 @@ unsafe fn small_slice_eq(x: &[u8], y: &[u8]) -> bool { // Thus, derefencing both `px` and `py` in the loop below is safe. // // Moreover, we set `pxend` and `pyend` to be 4 bytes before the actual - // end of of `px` and `py`. Thus, the final dereference outside of the + // end of `px` and `py`. Thus, the final dereference outside of the // loop is guaranteed to be valid. (The final comparison will overlap with // the last comparison done in the loop for lengths that aren't multiples // of four.) diff --git a/library/std/src/sync/mpmc/array.rs b/library/std/src/sync/mpmc/array.rs index 4db7b4990b97d..f71edc6c525a2 100644 --- a/library/std/src/sync/mpmc/array.rs +++ b/library/std/src/sync/mpmc/array.rs @@ -225,7 +225,7 @@ impl Channel { let slot = unsafe { self.buffer.get_unchecked(index) }; let stamp = slot.stamp.load(Ordering::Acquire); - // If the the stamp is ahead of the head by 1, we may attempt to pop. + // If the stamp is ahead of the head by 1, we may attempt to pop. if head + 1 == stamp { let new = if index + 1 < self.cap { // Same lap, incremented index. diff --git a/library/std/src/thread/scoped.rs b/library/std/src/thread/scoped.rs index e6dbf35bd0286..ada69aa8269f6 100644 --- a/library/std/src/thread/scoped.rs +++ b/library/std/src/thread/scoped.rs @@ -46,7 +46,7 @@ impl ScopeData { // We check for 'overflow' with usize::MAX / 2, to make sure there's no // chance it overflows to 0, which would result in unsoundness. if self.num_running_threads.fetch_add(1, Ordering::Relaxed) > usize::MAX / 2 { - // This can only reasonably happen by mem::forget()'ing many many ScopedJoinHandles. + // This can only reasonably happen by mem::forget()'ing a lot of ScopedJoinHandles. self.decrement_num_running_threads(false); panic!("too many running threads in thread scope"); } diff --git a/src/test/ui/rfc-1445-restrict-constants-in-patterns/match-requires-both-partialeq-and-eq.rs b/src/test/ui/rfc-1445-restrict-constants-in-patterns/match-requires-both-partialeq-and-eq.rs index 6b7d94603b567..a8deb8a7550bc 100644 --- a/src/test/ui/rfc-1445-restrict-constants-in-patterns/match-requires-both-partialeq-and-eq.rs +++ b/src/test/ui/rfc-1445-restrict-constants-in-patterns/match-requires-both-partialeq-and-eq.rs @@ -5,7 +5,7 @@ struct Foo { impl PartialEq for Foo { fn eq(&self, _: &Foo) -> bool { - false // ha ha sucker! + false // ha ha! } } From 16a9fdf66372a6aeeb86f6cd19e00bdfb0a13912 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 5 Dec 2022 13:51:41 +0100 Subject: [PATCH 13/13] Preparing for merge from rustc --- src/tools/miri/rust-version | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tools/miri/rust-version b/src/tools/miri/rust-version index 0a6b9417cc2e2..8dd18ae98e6d6 100644 --- a/src/tools/miri/rust-version +++ b/src/tools/miri/rust-version @@ -1 +1 @@ -cef44f53034eac46be3a0e3eec7b2b3d4ef5140b +203c8765ea33c65d888febe0e8219c4bb11b0d89