From dab77c50daeb563a05b20b31527f731b683cded5 Mon Sep 17 00:00:00 2001 From: Trevor Spiteri Date: Thu, 27 Jun 2024 18:31:29 +0200 Subject: [PATCH 1/9] fix least significant digits of f128 associated constants While the numbers are parsed to the correct value, the decimal numbers in the source were rounded to zero instead of to the nearest, making the literals different from the values shown in the documentation. --- library/core/src/num/f128.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/library/core/src/num/f128.rs b/library/core/src/num/f128.rs index 58ed98c888cc6..048e3cca93d21 100644 --- a/library/core/src/num/f128.rs +++ b/library/core/src/num/f128.rs @@ -167,7 +167,7 @@ impl f128 { /// [Machine epsilon]: https://en.wikipedia.org/wiki/Machine_epsilon /// [`MANTISSA_DIGITS`]: f128::MANTISSA_DIGITS #[unstable(feature = "f128", issue = "116909")] - pub const EPSILON: f128 = 1.92592994438723585305597794258492731e-34_f128; + pub const EPSILON: f128 = 1.92592994438723585305597794258492732e-34_f128; /// Smallest finite `f128` value. /// @@ -175,7 +175,7 @@ impl f128 { /// /// [`MAX`]: f128::MAX #[unstable(feature = "f128", issue = "116909")] - pub const MIN: f128 = -1.18973149535723176508575932662800701e+4932_f128; + pub const MIN: f128 = -1.18973149535723176508575932662800702e+4932_f128; /// Smallest positive normal `f128` value. /// /// Equal to 2[`MIN_EXP`] − 1. @@ -191,7 +191,7 @@ impl f128 { /// [`MANTISSA_DIGITS`]: f128::MANTISSA_DIGITS /// [`MAX_EXP`]: f128::MAX_EXP #[unstable(feature = "f128", issue = "116909")] - pub const MAX: f128 = 1.18973149535723176508575932662800701e+4932_f128; + pub const MAX: f128 = 1.18973149535723176508575932662800702e+4932_f128; /// One greater than the minimum possible normal power of 2 exponent. /// From e31709345c82542e14eae079303c1b09a42a811f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= Date: Thu, 4 Jul 2024 20:12:19 +0200 Subject: [PATCH 2/9] Port all remaining simple command usages in bootstrap to `BootstrapCommand` --- src/bootstrap/src/core/build_steps/compile.rs | 26 ++++------- src/bootstrap/src/core/build_steps/llvm.rs | 2 +- src/bootstrap/src/core/build_steps/setup.rs | 46 +++++++++---------- src/bootstrap/src/core/build_steps/test.rs | 14 ++---- src/bootstrap/src/utils/helpers.rs | 6 +-- 5 files changed, 40 insertions(+), 54 deletions(-) diff --git a/src/bootstrap/src/core/build_steps/compile.rs b/src/bootstrap/src/core/build_steps/compile.rs index 039a7b2fc276d..e6c15f27ea913 100644 --- a/src/bootstrap/src/core/build_steps/compile.rs +++ b/src/bootstrap/src/core/build_steps/compile.rs @@ -14,7 +14,7 @@ use std::fs; use std::io::prelude::*; use std::io::BufReader; use std::path::{Path, PathBuf}; -use std::process::{Command, Stdio}; +use std::process::Stdio; use std::str; use serde_derive::Deserialize; @@ -696,10 +696,10 @@ fn copy_sanitizers( || target == "x86_64-apple-ios" { // Update the library’s install name to reflect that it has been renamed. - apple_darwin_update_library_name(&dst, &format!("@rpath/{}", &runtime.name)); + apple_darwin_update_library_name(builder, &dst, &format!("@rpath/{}", &runtime.name)); // Upon renaming the install name, the code signature of the file will invalidate, // so we will sign it again. - apple_darwin_sign_file(&dst); + apple_darwin_sign_file(builder, &dst); } target_deps.push(dst); @@ -708,25 +708,17 @@ fn copy_sanitizers( target_deps } -fn apple_darwin_update_library_name(library_path: &Path, new_name: &str) { - let status = Command::new("install_name_tool") - .arg("-id") - .arg(new_name) - .arg(library_path) - .status() - .expect("failed to execute `install_name_tool`"); - assert!(status.success()); +fn apple_darwin_update_library_name(builder: &Builder<'_>, library_path: &Path, new_name: &str) { + command("install_name_tool").arg("-id").arg(new_name).arg(library_path).run(builder); } -fn apple_darwin_sign_file(file_path: &Path) { - let status = Command::new("codesign") +fn apple_darwin_sign_file(builder: &Builder<'_>, file_path: &Path) { + command("codesign") .arg("-f") // Force to rewrite the existing signature .arg("-s") .arg("-") .arg(file_path) - .status() - .expect("failed to execute `codesign`"); - assert!(status.success()); + .run(builder); } #[derive(Debug, Clone, PartialEq, Eq, Hash)] @@ -1172,7 +1164,7 @@ fn rustc_llvm_env(builder: &Builder<'_>, cargo: &mut Cargo, target: TargetSelect if builder.config.llvm_profile_generate && target.is_msvc() { if let Some(ref clang_cl_path) = builder.config.llvm_clang_cl { // Add clang's runtime library directory to the search path - let clang_rt_dir = get_clang_cl_resource_dir(clang_cl_path); + let clang_rt_dir = get_clang_cl_resource_dir(builder, clang_cl_path); llvm_linker_flags.push_str(&format!("-L{}", clang_rt_dir.display())); } } diff --git a/src/bootstrap/src/core/build_steps/llvm.rs b/src/bootstrap/src/core/build_steps/llvm.rs index 41dff2123f13d..876d73dce3ce3 100644 --- a/src/bootstrap/src/core/build_steps/llvm.rs +++ b/src/bootstrap/src/core/build_steps/llvm.rs @@ -912,7 +912,7 @@ impl Step for Lld { if let Some(clang_cl_path) = builder.config.llvm_clang_cl.as_ref() { // Find clang's runtime library directory and push that as a search path to the // cmake linker flags. - let clang_rt_dir = get_clang_cl_resource_dir(clang_cl_path); + let clang_rt_dir = get_clang_cl_resource_dir(builder, clang_cl_path); ldflags.push_all(format!("/libpath:{}", clang_rt_dir.display())); } } diff --git a/src/bootstrap/src/core/build_steps/setup.rs b/src/bootstrap/src/core/build_steps/setup.rs index 29cc5e00637e4..e155b30257834 100644 --- a/src/bootstrap/src/core/build_steps/setup.rs +++ b/src/bootstrap/src/core/build_steps/setup.rs @@ -8,6 +8,7 @@ use crate::core::builder::{Builder, RunConfig, ShouldRun, Step}; use crate::t; use crate::utils::change_tracker::CONFIG_CHANGE_HISTORY; +use crate::utils::exec::command; use crate::utils::helpers::{self, hex_encode}; use crate::Config; use sha2::Digest; @@ -16,7 +17,6 @@ use std::fmt::Write as _; use std::fs::File; use std::io::Write; use std::path::{Path, PathBuf, MAIN_SEPARATOR_STR}; -use std::process::Command; use std::str::FromStr; use std::{fmt, fs, io}; @@ -266,20 +266,16 @@ impl Step for Link { } let stage_path = ["build", config.build.rustc_target_arg(), "stage1"].join(MAIN_SEPARATOR_STR); - if !rustup_installed() { + if !rustup_installed(builder) { eprintln!("`rustup` is not installed; cannot link `stage1` toolchain"); } else if stage_dir_exists(&stage_path[..]) && !config.dry_run() { - attempt_toolchain_link(&stage_path[..]); + attempt_toolchain_link(builder, &stage_path[..]); } } } -fn rustup_installed() -> bool { - Command::new("rustup") - .arg("--version") - .stdout(std::process::Stdio::null()) - .output() - .map_or(false, |output| output.status.success()) +fn rustup_installed(builder: &Builder<'_>) -> bool { + command("rustup").capture_stdout().arg("--version").run(builder).is_success() } fn stage_dir_exists(stage_path: &str) -> bool { @@ -289,8 +285,8 @@ fn stage_dir_exists(stage_path: &str) -> bool { } } -fn attempt_toolchain_link(stage_path: &str) { - if toolchain_is_linked() { +fn attempt_toolchain_link(builder: &Builder<'_>, stage_path: &str) { + if toolchain_is_linked(builder) { return; } @@ -301,7 +297,7 @@ fn attempt_toolchain_link(stage_path: &str) { return; } - if try_link_toolchain(stage_path) { + if try_link_toolchain(builder, stage_path) { println!( "Added `stage1` rustup toolchain; try `cargo +stage1 build` on a separate rust project to run a newly-built toolchain" ); @@ -315,14 +311,16 @@ fn attempt_toolchain_link(stage_path: &str) { } } -fn toolchain_is_linked() -> bool { - match Command::new("rustup") +fn toolchain_is_linked(builder: &Builder<'_>) -> bool { + match command("rustup") + .capture_stdout() + .allow_failure() .args(["toolchain", "list"]) - .stdout(std::process::Stdio::piped()) - .output() + .run(builder) + .stdout_if_ok() { - Ok(toolchain_list) => { - if !String::from_utf8_lossy(&toolchain_list.stdout).contains("stage1") { + Some(toolchain_list) => { + if !toolchain_list.contains("stage1") { return false; } // The toolchain has already been linked. @@ -330,7 +328,7 @@ fn toolchain_is_linked() -> bool { "`stage1` toolchain already linked; not attempting to link `stage1` toolchain" ); } - Err(_) => { + None => { // In this case, we don't know if the `stage1` toolchain has been linked; // but `rustup` failed, so let's not go any further. println!( @@ -341,12 +339,12 @@ fn toolchain_is_linked() -> bool { true } -fn try_link_toolchain(stage_path: &str) -> bool { - Command::new("rustup") - .stdout(std::process::Stdio::null()) +fn try_link_toolchain(builder: &Builder<'_>, stage_path: &str) -> bool { + command("rustup") + .capture_stdout() .args(["toolchain", "link", "stage1", stage_path]) - .output() - .map_or(false, |output| output.status.success()) + .run(builder) + .is_success() } fn ensure_stage1_toolchain_placeholder_exists(stage_path: &str) -> bool { diff --git a/src/bootstrap/src/core/build_steps/test.rs b/src/bootstrap/src/core/build_steps/test.rs index 4d69d6b523111..118b6b876ed1e 100644 --- a/src/bootstrap/src/core/build_steps/test.rs +++ b/src/bootstrap/src/core/build_steps/test.rs @@ -9,7 +9,6 @@ use std::ffi::OsString; use std::fs; use std::iter; use std::path::{Path, PathBuf}; -use std::process::{Command, Stdio}; use clap_complete::shells; @@ -169,12 +168,8 @@ You can skip linkcheck with --skip src/tools/linkchecker" } } -fn check_if_tidy_is_installed() -> bool { - Command::new("tidy") - .arg("--version") - .stdout(Stdio::null()) - .status() - .map_or(false, |status| status.success()) +fn check_if_tidy_is_installed(builder: &Builder<'_>) -> bool { + command("tidy").capture_stdout().allow_failure().arg("--version").run(builder).is_success() } #[derive(Debug, Clone, PartialEq, Eq, Hash)] @@ -188,8 +183,9 @@ impl Step for HtmlCheck { const ONLY_HOSTS: bool = true; fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> { + let builder = run.builder; let run = run.path("src/tools/html-checker"); - run.lazy_default_condition(Box::new(check_if_tidy_is_installed)) + run.lazy_default_condition(Box::new(|| check_if_tidy_is_installed(builder))) } fn make_run(run: RunConfig<'_>) { @@ -197,7 +193,7 @@ impl Step for HtmlCheck { } fn run(self, builder: &Builder<'_>) { - if !check_if_tidy_is_installed() { + if !check_if_tidy_is_installed(builder) { eprintln!("not running HTML-check tool because `tidy` is missing"); eprintln!( "You need the HTML tidy tool https://www.html-tidy.org/, this tool is *not* part of the rust project and needs to be installed separately, for example via your package manager." diff --git a/src/bootstrap/src/utils/helpers.rs b/src/bootstrap/src/utils/helpers.rs index 15133e441b491..3c82fa189be36 100644 --- a/src/bootstrap/src/utils/helpers.rs +++ b/src/bootstrap/src/utils/helpers.rs @@ -338,13 +338,13 @@ fn dir_up_to_date(src: &Path, threshold: SystemTime) -> bool { /// When `clang-cl` is used with instrumentation, we need to add clang's runtime library resource /// directory to the linker flags, otherwise there will be linker errors about the profiler runtime /// missing. This function returns the path to that directory. -pub fn get_clang_cl_resource_dir(clang_cl_path: &str) -> PathBuf { +pub fn get_clang_cl_resource_dir(builder: &Builder<'_>, clang_cl_path: &str) -> PathBuf { // Similar to how LLVM does it, to find clang's library runtime directory: // - we ask `clang-cl` to locate the `clang_rt.builtins` lib. - let mut builtins_locator = Command::new(clang_cl_path); + let mut builtins_locator = command(clang_cl_path); builtins_locator.args(["/clang:-print-libgcc-file-name", "/clang:--rtlib=compiler-rt"]); - let clang_rt_builtins = output(&mut builtins_locator); + let clang_rt_builtins = builtins_locator.capture_stdout().run(builder).stdout(); let clang_rt_builtins = Path::new(clang_rt_builtins.trim()); assert!( clang_rt_builtins.exists(), From 8cffb475fd490785ea44367d91161aba9b98a490 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= Date: Sun, 7 Jul 2024 17:10:58 +0200 Subject: [PATCH 3/9] Remove several usages of the `as_command_mut` method Passes `&Builder<'_>` to additional places, so that they could use the `BootstrapCommand` APIs directly, rather than going through `as_command_mut`. --- src/bootstrap/src/core/build_steps/llvm.rs | 1 + src/bootstrap/src/core/build_steps/setup.rs | 14 ++- .../src/core/build_steps/toolstate.rs | 100 ++++++------------ src/bootstrap/src/lib.rs | 64 ++++++----- src/bootstrap/src/utils/tarball.rs | 14 +-- 5 files changed, 78 insertions(+), 115 deletions(-) diff --git a/src/bootstrap/src/core/build_steps/llvm.rs b/src/bootstrap/src/core/build_steps/llvm.rs index 876d73dce3ce3..888290a0479ba 100644 --- a/src/bootstrap/src/core/build_steps/llvm.rs +++ b/src/bootstrap/src/core/build_steps/llvm.rs @@ -125,6 +125,7 @@ pub fn prebuilt_llvm_config(builder: &Builder<'_>, target: TargetSelection) -> L static STAMP_HASH_MEMO: OnceLock = OnceLock::new(); let smart_stamp_hash = STAMP_HASH_MEMO.get_or_init(|| { generate_smart_stamp_hash( + builder, &builder.config.src.join("src/llvm-project"), builder.in_tree_llvm_info.sha().unwrap_or_default(), ) diff --git a/src/bootstrap/src/core/build_steps/setup.rs b/src/bootstrap/src/core/build_steps/setup.rs index e155b30257834..226b3729c10ce 100644 --- a/src/bootstrap/src/core/build_steps/setup.rs +++ b/src/bootstrap/src/core/build_steps/setup.rs @@ -474,20 +474,18 @@ impl Step for Hook { if config.dry_run() { return; } - t!(install_git_hook_maybe(config)); + t!(install_git_hook_maybe(builder, config)); } } // install a git hook to automatically run tidy, if they want -fn install_git_hook_maybe(config: &Config) -> io::Result<()> { +fn install_git_hook_maybe(builder: &Builder<'_>, config: &Config) -> io::Result<()> { let git = helpers::git(Some(&config.src)) + .capture() .args(["rev-parse", "--git-common-dir"]) - .as_command_mut() - .output() - .map(|output| { - assert!(output.status.success(), "failed to run `git`"); - PathBuf::from(t!(String::from_utf8(output.stdout)).trim()) - })?; + .run(builder) + .stdout(); + let git = PathBuf::from(git.trim()); let hooks_dir = git.join("hooks"); let dst = hooks_dir.join("pre-push"); if dst.exists() { diff --git a/src/bootstrap/src/core/build_steps/toolstate.rs b/src/bootstrap/src/core/build_steps/toolstate.rs index 9ddd68da59d16..2ab0ce7454b17 100644 --- a/src/bootstrap/src/core/build_steps/toolstate.rs +++ b/src/bootstrap/src/core/build_steps/toolstate.rs @@ -99,24 +99,16 @@ fn print_error(tool: &str, submodule: &str) { crate::exit!(3); } -fn check_changed_files(toolstates: &HashMap, ToolState>) { +fn check_changed_files(builder: &Builder<'_>, toolstates: &HashMap, ToolState>) { // Changed files let output = helpers::git(None) + .capture() .arg("diff") .arg("--name-status") .arg("HEAD") .arg("HEAD^") - .as_command_mut() - .output(); - let output = match output { - Ok(o) => o, - Err(e) => { - eprintln!("Failed to get changed files: {e:?}"); - crate::exit!(1); - } - }; - - let output = t!(String::from_utf8(output.stdout)); + .run(builder) + .stdout(); for (tool, submodule) in STABLE_TOOLS.iter().chain(NIGHTLY_TOOLS.iter()) { let changed = output.lines().any(|l| l.starts_with('M') && l.ends_with(submodule)); @@ -186,8 +178,8 @@ impl Step for ToolStateCheck { crate::exit!(1); } - check_changed_files(&toolstates); - checkout_toolstate_repo(); + check_changed_files(builder, &toolstates); + checkout_toolstate_repo(builder); let old_toolstate = read_old_toolstate(); for (tool, _) in STABLE_TOOLS.iter() { @@ -231,7 +223,7 @@ impl Step for ToolStateCheck { } if builder.config.channel == "nightly" && env::var_os("TOOLSTATE_PUBLISH").is_some() { - commit_toolstate_change(&toolstates); + commit_toolstate_change(builder, &toolstates); } } @@ -315,55 +307,34 @@ fn toolstate_repo() -> String { const TOOLSTATE_DIR: &str = "rust-toolstate"; /// Checks out the toolstate repo into `TOOLSTATE_DIR`. -fn checkout_toolstate_repo() { +fn checkout_toolstate_repo(builder: &Builder<'_>) { if let Ok(token) = env::var("TOOLSTATE_REPO_ACCESS_TOKEN") { - prepare_toolstate_config(&token); + prepare_toolstate_config(builder, &token); } if Path::new(TOOLSTATE_DIR).exists() { eprintln!("Cleaning old toolstate directory..."); t!(fs::remove_dir_all(TOOLSTATE_DIR)); } - let status = helpers::git(None) + helpers::git(None) .arg("clone") .arg("--depth=1") .arg(toolstate_repo()) .arg(TOOLSTATE_DIR) - .as_command_mut() - .status(); - let success = match status { - Ok(s) => s.success(), - Err(_) => false, - }; - if !success { - panic!("git clone unsuccessful (status: {status:?})"); - } + .run(builder); } /// Sets up config and authentication for modifying the toolstate repo. -fn prepare_toolstate_config(token: &str) { - fn git_config(key: &str, value: &str) { - let status = helpers::git(None) - .arg("config") - .arg("--global") - .arg(key) - .arg(value) - .as_command_mut() - .status(); - let success = match status { - Ok(s) => s.success(), - Err(_) => false, - }; - if !success { - panic!("git config key={key} value={value} failed (status: {status:?})"); - } +fn prepare_toolstate_config(builder: &Builder<'_>, token: &str) { + fn git_config(builder: &Builder<'_>, key: &str, value: &str) { + helpers::git(None).arg("config").arg("--global").arg(key).arg(value).run(builder); } // If changing anything here, then please check that `src/ci/publish_toolstate.sh` is up to date // as well. - git_config("user.email", "7378925+rust-toolstate-update@users.noreply.github.com"); - git_config("user.name", "Rust Toolstate Update"); - git_config("credential.helper", "store"); + git_config(builder, "user.email", "7378925+rust-toolstate-update@users.noreply.github.com"); + git_config(builder, "user.name", "Rust Toolstate Update"); + git_config(builder, "credential.helper", "store"); let credential = format!("https://{token}:x-oauth-basic@github.com\n",); let git_credential_path = PathBuf::from(t!(env::var("HOME"))).join(".git-credentials"); @@ -403,55 +374,51 @@ fn read_old_toolstate() -> Vec { /// /// * See /// if a private email by GitHub is wanted. -fn commit_toolstate_change(current_toolstate: &ToolstateData) { +fn commit_toolstate_change(builder: &Builder<'_>, current_toolstate: &ToolstateData) { let message = format!("({} CI update)", OS.expect("linux/windows only")); let mut success = false; for _ in 1..=5 { // Upload the test results (the new commit-to-toolstate mapping) to the toolstate repo. // This does *not* change the "current toolstate"; that only happens post-landing // via `src/ci/docker/publish_toolstate.sh`. - publish_test_results(current_toolstate); + publish_test_results(builder, current_toolstate); // `git commit` failing means nothing to commit. - let status = t!(helpers::git(Some(Path::new(TOOLSTATE_DIR))) + let status = helpers::git(Some(Path::new(TOOLSTATE_DIR))) + .allow_failure() .arg("commit") .arg("-a") .arg("-m") .arg(&message) - .as_command_mut() - .status()); - if !status.success() { + .run(builder); + if !status.is_success() { success = true; break; } - let status = t!(helpers::git(Some(Path::new(TOOLSTATE_DIR))) + let status = helpers::git(Some(Path::new(TOOLSTATE_DIR))) + .allow_failure() .arg("push") .arg("origin") .arg("master") - .as_command_mut() - .status()); + .run(builder); // If we successfully push, exit. - if status.success() { + if status.is_success() { success = true; break; } eprintln!("Sleeping for 3 seconds before retrying push"); std::thread::sleep(std::time::Duration::from_secs(3)); - let status = t!(helpers::git(Some(Path::new(TOOLSTATE_DIR))) + helpers::git(Some(Path::new(TOOLSTATE_DIR))) .arg("fetch") .arg("origin") .arg("master") - .as_command_mut() - .status()); - assert!(status.success()); - let status = t!(helpers::git(Some(Path::new(TOOLSTATE_DIR))) + .run(builder); + helpers::git(Some(Path::new(TOOLSTATE_DIR))) .arg("reset") .arg("--hard") .arg("origin/master") - .as_command_mut() - .status()); - assert!(status.success()); + .run(builder); } if !success { @@ -464,9 +431,8 @@ fn commit_toolstate_change(current_toolstate: &ToolstateData) { /// These results will later be promoted to `latest.json` by the /// `publish_toolstate.py` script if the PR passes all tests and is merged to /// master. -fn publish_test_results(current_toolstate: &ToolstateData) { - let commit = t!(helpers::git(None).arg("rev-parse").arg("HEAD").as_command_mut().output()); - let commit = t!(String::from_utf8(commit.stdout)); +fn publish_test_results(builder: &Builder<'_>, current_toolstate: &ToolstateData) { + let commit = helpers::git(None).capture().arg("rev-parse").arg("HEAD").run(builder).stdout(); let toolstate_serialized = t!(serde_json::to_string(¤t_toolstate)); diff --git a/src/bootstrap/src/lib.rs b/src/bootstrap/src/lib.rs index daa916ce0a070..52e28fbd0598d 100644 --- a/src/bootstrap/src/lib.rs +++ b/src/bootstrap/src/lib.rs @@ -23,7 +23,7 @@ use std::fmt::Display; use std::fs::{self, File}; use std::io; use std::path::{Path, PathBuf}; -use std::process::{Command, Stdio}; +use std::process::Command; use std::str; use std::sync::OnceLock; use std::time::SystemTime; @@ -37,7 +37,7 @@ use utils::channel::GitInfo; use utils::helpers::hex_encode; use crate::core::builder; -use crate::core::builder::Kind; +use crate::core::builder::{Builder, Kind}; use crate::core::config::{flags, LldMode}; use crate::core::config::{DryRun, Target}; use crate::core::config::{LlvmLibunwind, TargetSelection}; @@ -490,18 +490,18 @@ impl Build { return; } - let submodule_git = || helpers::git(Some(&absolute_path)); + let submodule_git = || helpers::git(Some(&absolute_path)).capture_stdout(); // Determine commit checked out in submodule. - let checked_out_hash = output(submodule_git().args(["rev-parse", "HEAD"]).as_command_mut()); + let checked_out_hash = submodule_git().args(["rev-parse", "HEAD"]).run(self).stdout(); let checked_out_hash = checked_out_hash.trim_end(); // Determine commit that the submodule *should* have. - let recorded = output( - helpers::git(Some(&self.src)) - .args(["ls-tree", "HEAD"]) - .arg(relative_path) - .as_command_mut(), - ); + let recorded = helpers::git(Some(&self.src)) + .capture_stdout() + .args(["ls-tree", "HEAD"]) + .arg(relative_path) + .run(self) + .stdout(); let actual_hash = recorded .split_whitespace() .nth(2) @@ -522,21 +522,14 @@ impl Build { let update = |progress: bool| { // Git is buggy and will try to fetch submodules from the tracking branch for *this* repository, // even though that has no relation to the upstream for the submodule. - let current_branch = { - let output = helpers::git(Some(&self.src)) - .args(["symbolic-ref", "--short", "HEAD"]) - .as_command_mut() - .stderr(Stdio::inherit()) - .output(); - let output = t!(output); - if output.status.success() { - Some(String::from_utf8(output.stdout).unwrap().trim().to_owned()) - } else { - None - } - }; + let current_branch = helpers::git(Some(&self.src)) + .capture_stdout() + .args(["symbolic-ref", "--short", "HEAD"]) + .run(self) + .stdout_if_ok() + .map(|s| s.trim().to_owned()); - let mut git = helpers::git(Some(&self.src)); + let mut git = helpers::git(Some(&self.src)).allow_failure(); if let Some(branch) = current_branch { // If there is a tag named after the current branch, git will try to disambiguate by prepending `heads/` to the branch name. // This syntax isn't accepted by `branch.{branch}`. Strip it. @@ -550,8 +543,7 @@ impl Build { git.arg(relative_path); git }; - // NOTE: doesn't use `try_run` because this shouldn't print an error if it fails. - if !update(true).as_command_mut().status().map_or(false, |status| status.success()) { + if !update(true).run(self).is_success() { update(false).run(self); } @@ -1943,22 +1935,28 @@ fn envify(s: &str) -> String { /// /// In case of errors during `git` command execution (e.g., in tarball sources), default values /// are used to prevent panics. -pub fn generate_smart_stamp_hash(dir: &Path, additional_input: &str) -> String { +pub fn generate_smart_stamp_hash( + builder: &Builder<'_>, + dir: &Path, + additional_input: &str, +) -> String { let diff = helpers::git(Some(dir)) + .capture_stdout() + .allow_failure() .arg("diff") - .as_command_mut() - .output() - .map(|o| String::from_utf8(o.stdout).unwrap_or_default()) + .run(builder) + .stdout_if_ok() .unwrap_or_default(); let status = helpers::git(Some(dir)) + .capture_stdout() + .allow_failure() .arg("status") .arg("--porcelain") .arg("-z") .arg("--untracked-files=normal") - .as_command_mut() - .output() - .map(|o| String::from_utf8(o.stdout).unwrap_or_default()) + .run(builder) + .stdout_if_ok() .unwrap_or_default(); let mut hasher = sha2::Sha256::new(); diff --git a/src/bootstrap/src/utils/tarball.rs b/src/bootstrap/src/utils/tarball.rs index f5fc94273c9ae..9378c35127f26 100644 --- a/src/bootstrap/src/utils/tarball.rs +++ b/src/bootstrap/src/utils/tarball.rs @@ -369,13 +369,13 @@ impl<'a> Tarball<'a> { // gets the same timestamp. if self.builder.rust_info().is_managed_git_subrepository() { // %ct means committer date - let timestamp = helpers::output( - helpers::git(Some(&self.builder.src)) - .arg("log") - .arg("-1") - .arg("--format=%ct") - .as_command_mut(), - ); + let timestamp = helpers::git(Some(&self.builder.src)) + .capture_stdout() + .arg("log") + .arg("-1") + .arg("--format=%ct") + .run(self.builder) + .stdout(); cmd.args(["--override-file-mtime", timestamp.trim()]); } From ee86e2d624a61b634c14dd931b7ae2532a6634ff Mon Sep 17 00:00:00 2001 From: rustbot <47979223+rustbot@users.noreply.github.com> Date: Mon, 15 Jul 2024 13:00:42 -0400 Subject: [PATCH 4/9] Update books --- src/doc/book | 2 +- src/doc/edition-guide | 2 +- src/doc/embedded-book | 2 +- src/doc/reference | 2 +- src/doc/rust-by-example | 2 +- src/doc/rustc-dev-guide | 2 +- 6 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/doc/book b/src/doc/book index f1e49bf7a8ea6..67fa536768013 160000 --- a/src/doc/book +++ b/src/doc/book @@ -1 +1 @@ -Subproject commit f1e49bf7a8ea6c31ce016a52b8a4f6e1ffcfbc64 +Subproject commit 67fa536768013d9d5a13f3a06790521d511ef711 diff --git a/src/doc/edition-guide b/src/doc/edition-guide index 941db8b3df45f..5454de3d12b9c 160000 --- a/src/doc/edition-guide +++ b/src/doc/edition-guide @@ -1 +1 @@ -Subproject commit 941db8b3df45fd46cd87b50a5c86714b91dcde9c +Subproject commit 5454de3d12b9ccc6375b629cf7ccda8264640aac diff --git a/src/doc/embedded-book b/src/doc/embedded-book index b10c6acaf0f43..019f3928d8b93 160000 --- a/src/doc/embedded-book +++ b/src/doc/embedded-book @@ -1 +1 @@ -Subproject commit b10c6acaf0f43481f6600e95d4b5013446e29f7a +Subproject commit 019f3928d8b939ec71b63722dcc2e46330156441 diff --git a/src/doc/reference b/src/doc/reference index 1ae3deebc3ac1..2a7931476643e 160000 --- a/src/doc/reference +++ b/src/doc/reference @@ -1 +1 @@ -Subproject commit 1ae3deebc3ac16e276b6558e01420f8e605def08 +Subproject commit 2a7931476643e2a07bf9796e14bbeee3c9e589b8 diff --git a/src/doc/rust-by-example b/src/doc/rust-by-example index 658c6c27cb975..89aecb6951b77 160000 --- a/src/doc/rust-by-example +++ b/src/doc/rust-by-example @@ -1 +1 @@ -Subproject commit 658c6c27cb975b92227936024816986c2d3716fb +Subproject commit 89aecb6951b77bc746da73df8c9f2b2ceaad494a diff --git a/src/doc/rustc-dev-guide b/src/doc/rustc-dev-guide index d6e3a32a557db..0c4d55cb59fe4 160000 --- a/src/doc/rustc-dev-guide +++ b/src/doc/rustc-dev-guide @@ -1 +1 @@ -Subproject commit d6e3a32a557db5902e714604def8015d6bb7e0f7 +Subproject commit 0c4d55cb59fe440d1a630e4e5774d043968edb3f From 3051436a3c5e7717c840bf04f6f147223739566b Mon Sep 17 00:00:00 2001 From: Eric Huss Date: Mon, 15 Jul 2024 10:58:58 -0700 Subject: [PATCH 5/9] Update reference to fix toolstate --- src/doc/reference | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/doc/reference b/src/doc/reference index 2a7931476643e..e2f0bdc403186 160000 --- a/src/doc/reference +++ b/src/doc/reference @@ -1 +1 @@ -Subproject commit 2a7931476643e2a07bf9796e14bbeee3c9e589b8 +Subproject commit e2f0bdc4031866734661dcdb548184bde1450baf From ff9c4883449ff895942398be656e260891755e4a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= Date: Sun, 7 Jul 2024 17:16:44 +0200 Subject: [PATCH 6/9] Move `force_coloring_in_ci` from `builder_helper` to `bootstrap` It was only used in bootstrap. This move allows us to modify the function to work with `BootstrapCommand`, rather than `Command`. --- src/bootstrap/src/core/build_steps/test.rs | 4 +--- src/bootstrap/src/core/builder.rs | 2 +- src/bootstrap/src/utils/exec.rs | 13 +++++++++++++ src/tools/build_helper/src/ci.rs | 14 -------------- 4 files changed, 15 insertions(+), 18 deletions(-) diff --git a/src/bootstrap/src/core/build_steps/test.rs b/src/bootstrap/src/core/build_steps/test.rs index 118b6b876ed1e..3f0cbde64e394 100644 --- a/src/bootstrap/src/core/build_steps/test.rs +++ b/src/bootstrap/src/core/build_steps/test.rs @@ -2095,9 +2095,7 @@ NOTE: if you're sure you want to do this, please open an issue as to why. In the let git_config = builder.config.git_config(); cmd.arg("--git-repository").arg(git_config.git_repository); cmd.arg("--nightly-branch").arg(git_config.nightly_branch); - - // FIXME: Move CiEnv back to bootstrap, it is only used here anyway - builder.ci_env.force_coloring_in_ci(cmd.as_command_mut()); + cmd.force_coloring_in_ci(builder.ci_env); #[cfg(feature = "build-metrics")] builder.metrics.begin_test_suite( diff --git a/src/bootstrap/src/core/builder.rs b/src/bootstrap/src/core/builder.rs index aeb3474360801..08308dbbf73fb 100644 --- a/src/bootstrap/src/core/builder.rs +++ b/src/bootstrap/src/core/builder.rs @@ -2105,7 +2105,7 @@ impl<'a> Builder<'a> { // Try to use a sysroot-relative bindir, in case it was configured absolutely. cargo.env("RUSTC_INSTALL_BINDIR", self.config.bindir_relative()); - self.ci_env.force_coloring_in_ci(cargo.as_command_mut()); + cargo.force_coloring_in_ci(self.ci_env); // When we build Rust dylibs they're all intended for intermediate // usage, so make sure we pass the -Cprefer-dynamic flag instead of diff --git a/src/bootstrap/src/utils/exec.rs b/src/bootstrap/src/utils/exec.rs index b053016499730..a60c0084f3d7f 100644 --- a/src/bootstrap/src/utils/exec.rs +++ b/src/bootstrap/src/utils/exec.rs @@ -1,4 +1,5 @@ use crate::Build; +use build_helper::ci::CiEnv; use build_helper::drop_bomb::DropBomb; use std::ffi::OsStr; use std::fmt::{Debug, Formatter}; @@ -171,6 +172,18 @@ impl BootstrapCommand { pub fn get_created_location(&self) -> std::panic::Location<'static> { self.drop_bomb.get_created_location() } + + /// If in a CI environment, forces the command to run with colors. + pub fn force_coloring_in_ci(&mut self, ci_env: CiEnv) { + if ci_env != CiEnv::None { + // Due to use of stamp/docker, the output stream of bootstrap is not + // a TTY in CI, so coloring is by-default turned off. + // The explicit `TERM=xterm` environment is needed for + // `--color always` to actually work. This env var was lost when + // compiling through the Makefile. Very strange. + self.env("TERM", "xterm").args(["--color", "always"]); + } + } } impl Debug for BootstrapCommand { diff --git a/src/tools/build_helper/src/ci.rs b/src/tools/build_helper/src/ci.rs index 233fed4151c19..6d79c7c83ad8b 100644 --- a/src/tools/build_helper/src/ci.rs +++ b/src/tools/build_helper/src/ci.rs @@ -1,5 +1,3 @@ -use std::process::Command; - #[derive(Copy, Clone, PartialEq, Eq, Debug)] pub enum CiEnv { /// Not a CI environment. @@ -21,18 +19,6 @@ impl CiEnv { pub fn is_ci() -> bool { Self::current() != CiEnv::None } - - /// If in a CI environment, forces the command to run with colors. - pub fn force_coloring_in_ci(self, cmd: &mut Command) { - if self != CiEnv::None { - // Due to use of stamp/docker, the output stream of bootstrap is not - // a TTY in CI, so coloring is by-default turned off. - // The explicit `TERM=xterm` environment is needed for - // `--color always` to actually work. This env var was lost when - // compiling through the Makefile. Very strange. - cmd.env("TERM", "xterm").args(&["--color", "always"]); - } - } } pub mod gha { From 7a5411757124b3cdc4aff5ea86cbe91f5e58a618 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= Date: Sun, 14 Jul 2024 10:58:57 +0200 Subject: [PATCH 7/9] Make sure to run git submodule checkout in dry run mode --- src/bootstrap/src/lib.rs | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/src/bootstrap/src/lib.rs b/src/bootstrap/src/lib.rs index 52e28fbd0598d..c2367375708aa 100644 --- a/src/bootstrap/src/lib.rs +++ b/src/bootstrap/src/lib.rs @@ -490,7 +490,17 @@ impl Build { return; } - let submodule_git = || helpers::git(Some(&absolute_path)).capture_stdout(); + // Submodule updating actually happens during in the dry run mode. We need to make sure that + // all the git commands below are actually executed, because some follow-up code + // in bootstrap might depend on the submodules being checked out. Furthermore, not all + // the command executions below work with an empty output (produced during dry run). + // Therefore, all commands below are marked with `run_always()`, so that they also run in + // dry run mode. + let submodule_git = || { + let mut cmd = helpers::git(Some(&absolute_path)).capture_stdout(); + cmd.run_always(); + cmd + }; // Determine commit checked out in submodule. let checked_out_hash = submodule_git().args(["rev-parse", "HEAD"]).run(self).stdout(); @@ -498,6 +508,7 @@ impl Build { // Determine commit that the submodule *should* have. let recorded = helpers::git(Some(&self.src)) .capture_stdout() + .run_always() .args(["ls-tree", "HEAD"]) .arg(relative_path) .run(self) @@ -514,6 +525,7 @@ impl Build { println!("Updating submodule {}", relative_path.display()); helpers::git(Some(&self.src)) + .run_always() .args(["submodule", "-q", "sync"]) .arg(relative_path) .run(self); @@ -524,12 +536,14 @@ impl Build { // even though that has no relation to the upstream for the submodule. let current_branch = helpers::git(Some(&self.src)) .capture_stdout() + .run_always() .args(["symbolic-ref", "--short", "HEAD"]) .run(self) .stdout_if_ok() .map(|s| s.trim().to_owned()); let mut git = helpers::git(Some(&self.src)).allow_failure(); + git.run_always(); if let Some(branch) = current_branch { // If there is a tag named after the current branch, git will try to disambiguate by prepending `heads/` to the branch name. // This syntax isn't accepted by `branch.{branch}`. Strip it. From 317952677ad6a80b3751a075f42ffd2a8d142a84 Mon Sep 17 00:00:00 2001 From: yukang Date: Tue, 13 Feb 2024 05:24:32 +0800 Subject: [PATCH 8/9] Suggest a borrow when using dbg --- .../src/diagnostics/conflict_errors.rs | 65 +++++++++- tests/ui/borrowck/dbg-issue-120327.rs | 68 ++++++++++ tests/ui/borrowck/dbg-issue-120327.stderr | 117 ++++++++++++++++++ .../dbg-macro-move-semantics.stderr | 4 + 4 files changed, 252 insertions(+), 2 deletions(-) create mode 100644 tests/ui/borrowck/dbg-issue-120327.rs create mode 100644 tests/ui/borrowck/dbg-issue-120327.stderr diff --git a/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs b/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs index 1cb74849017ab..c7f6840e401c6 100644 --- a/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs +++ b/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs @@ -4,7 +4,7 @@ #![allow(rustc::untranslatable_diagnostic)] use either::Either; -use hir::ClosureKind; +use hir::{ClosureKind, Path}; use rustc_data_structures::captures::Captures; use rustc_data_structures::fx::FxIndexSet; use rustc_errors::{codes::*, struct_span_code_err, Applicability, Diag, MultiSpan}; @@ -16,6 +16,7 @@ use rustc_hir::{CoroutineKind, CoroutineSource, LangItem}; use rustc_middle::bug; use rustc_middle::hir::nested_filter::OnlyBodies; use rustc_middle::mir::tcx::PlaceTy; +use rustc_middle::mir::VarDebugInfoContents; use rustc_middle::mir::{ self, AggregateKind, BindingForm, BorrowKind, CallSource, ClearCrossCrate, ConstraintCategory, FakeBorrowKind, FakeReadCause, LocalDecl, LocalInfo, LocalKind, Location, MutBorrowKind, @@ -546,7 +547,14 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, '_, 'infcx, 'tcx> { self.suggest_cloning(err, ty, expr, None, Some(move_spans)); } } - if let Some(pat) = finder.pat { + + self.suggest_ref_for_dbg_args(expr, place, move_span, err); + + // it's useless to suggest inserting `ref` when the span don't comes from local code + if let Some(pat) = finder.pat + && !move_span.is_dummy() + && !self.infcx.tcx.sess.source_map().is_imported(move_span) + { *in_pattern = true; let mut sugg = vec![(pat.span.shrink_to_lo(), "ref ".to_string())]; if let Some(pat) = finder.parent_pat { @@ -561,6 +569,59 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, '_, 'infcx, 'tcx> { } } + // for dbg!(x) which may take ownership, suggest dbg!(&x) instead + // but here we actually do not check whether the macro name is `dbg!` + // so that we may extend the scope a bit larger to cover more cases + fn suggest_ref_for_dbg_args( + &self, + body: &hir::Expr<'_>, + place: &Place<'tcx>, + move_span: Span, + err: &mut Diag<'infcx>, + ) { + let var_info = self.body.var_debug_info.iter().find(|info| match info.value { + VarDebugInfoContents::Place(ref p) => p == place, + _ => false, + }); + let arg_name = if let Some(var_info) = var_info { + var_info.name + } else { + return; + }; + struct MatchArgFinder { + expr_span: Span, + match_arg_span: Option, + arg_name: Symbol, + } + impl Visitor<'_> for MatchArgFinder { + fn visit_expr(&mut self, e: &hir::Expr<'_>) { + // dbg! is expanded into a match pattern, we need to find the right argument span + if let hir::ExprKind::Match(expr, ..) = &e.kind + && let hir::ExprKind::Path(hir::QPath::Resolved( + _, + path @ Path { segments: [seg], .. }, + )) = &expr.kind + && seg.ident.name == self.arg_name + && self.expr_span.source_callsite().contains(expr.span) + { + self.match_arg_span = Some(path.span); + } + hir::intravisit::walk_expr(self, e); + } + } + + let mut finder = MatchArgFinder { expr_span: move_span, match_arg_span: None, arg_name }; + finder.visit_expr(body); + if let Some(macro_arg_span) = finder.match_arg_span { + err.span_suggestion_verbose( + macro_arg_span.shrink_to_lo(), + "consider borrowing instead of transferring ownership", + "&", + Applicability::MachineApplicable, + ); + } + } + fn report_use_of_uninitialized( &self, mpi: MovePathIndex, diff --git a/tests/ui/borrowck/dbg-issue-120327.rs b/tests/ui/borrowck/dbg-issue-120327.rs new file mode 100644 index 0000000000000..2de43f634877a --- /dev/null +++ b/tests/ui/borrowck/dbg-issue-120327.rs @@ -0,0 +1,68 @@ +fn s() -> String { + let a = String::new(); + dbg!(a); + return a; //~ ERROR use of moved value: +} + +fn m() -> String { + let a = String::new(); + dbg!(1, 2, a, 1, 2); + return a; //~ ERROR use of moved value: +} + +fn t(a: String) -> String { + let b: String = "".to_string(); + dbg!(a, b); + return b; //~ ERROR use of moved value: +} + +fn x(a: String) -> String { + let b: String = "".to_string(); + dbg!(a, b); + return a; //~ ERROR use of moved value: +} + +macro_rules! my_dbg { + () => { + eprintln!("[{}:{}:{}]", file!(), line!(), column!()) + }; + ($val:expr $(,)?) => { + match $val { + tmp => { + eprintln!("[{}:{}:{}] {} = {:#?}", + file!(), line!(), column!(), stringify!($val), &tmp); + tmp + } + } + }; + ($($val:expr),+ $(,)?) => { + ($(my_dbg!($val)),+,) + }; +} + +fn test_my_dbg() -> String { + let b: String = "".to_string(); + my_dbg!(b, 1); + return b; //~ ERROR use of moved value: +} + +fn test_not_macro() -> String { + let a = String::new(); + let _b = match a { + tmp => { + eprintln!("dbg: {}", tmp); + tmp + } + }; + return a; //~ ERROR use of moved value: +} + +fn get_expr(_s: String) {} + +fn test() { + let a: String = "".to_string(); + let _res = get_expr(dbg!(a)); + let _l = a.len(); //~ ERROR borrow of moved value +} + +fn main() {} diff --git a/tests/ui/borrowck/dbg-issue-120327.stderr b/tests/ui/borrowck/dbg-issue-120327.stderr new file mode 100644 index 0000000000000..efacc0c3f1341 --- /dev/null +++ b/tests/ui/borrowck/dbg-issue-120327.stderr @@ -0,0 +1,117 @@ +error[E0382]: use of moved value: `a` + --> $DIR/dbg-issue-120327.rs:4:12 + | +LL | let a = String::new(); + | - move occurs because `a` has type `String`, which does not implement the `Copy` trait +LL | dbg!(a); + | ------- value moved here +LL | return a; + | ^ value used here after move + | +help: consider borrowing instead of transferring ownership + | +LL | dbg!(&a); + | + + +error[E0382]: use of moved value: `a` + --> $DIR/dbg-issue-120327.rs:10:12 + | +LL | let a = String::new(); + | - move occurs because `a` has type `String`, which does not implement the `Copy` trait +LL | dbg!(1, 2, a, 1, 2); + | ------------------- value moved here +LL | return a; + | ^ value used here after move + | +help: consider borrowing instead of transferring ownership + | +LL | dbg!(1, 2, &a, 1, 2); + | + + +error[E0382]: use of moved value: `b` + --> $DIR/dbg-issue-120327.rs:16:12 + | +LL | let b: String = "".to_string(); + | - move occurs because `b` has type `String`, which does not implement the `Copy` trait +LL | dbg!(a, b); + | ---------- value moved here +LL | return b; + | ^ value used here after move + | +help: consider borrowing instead of transferring ownership + | +LL | dbg!(a, &b); + | + + +error[E0382]: use of moved value: `a` + --> $DIR/dbg-issue-120327.rs:22:12 + | +LL | fn x(a: String) -> String { + | - move occurs because `a` has type `String`, which does not implement the `Copy` trait +LL | let b: String = "".to_string(); +LL | dbg!(a, b); + | ---------- value moved here +LL | return a; + | ^ value used here after move + | +help: consider borrowing instead of transferring ownership + | +LL | dbg!(&a, b); + | + + +error[E0382]: use of moved value: `b` + --> $DIR/dbg-issue-120327.rs:46:12 + | +LL | tmp => { + | --- value moved here +... +LL | let b: String = "".to_string(); + | - move occurs because `b` has type `String`, which does not implement the `Copy` trait +LL | my_dbg!(b, 1); +LL | return b; + | ^ value used here after move + | +help: consider borrowing instead of transferring ownership + | +LL | my_dbg!(&b, 1); + | + +help: borrow this binding in the pattern to avoid moving the value + | +LL | ref tmp => { + | +++ + +error[E0382]: use of moved value: `a` + --> $DIR/dbg-issue-120327.rs:57:12 + | +LL | let a = String::new(); + | - move occurs because `a` has type `String`, which does not implement the `Copy` trait +LL | let _b = match a { +LL | tmp => { + | --- value moved here +... +LL | return a; + | ^ value used here after move + | +help: borrow this binding in the pattern to avoid moving the value + | +LL | ref tmp => { + | +++ + +error[E0382]: borrow of moved value: `a` + --> $DIR/dbg-issue-120327.rs:65:14 + | +LL | let a: String = "".to_string(); + | - move occurs because `a` has type `String`, which does not implement the `Copy` trait +LL | let _res = get_expr(dbg!(a)); + | ------- value moved here +LL | let _l = a.len(); + | ^ value borrowed here after move + | +help: consider borrowing instead of transferring ownership + | +LL | let _res = get_expr(dbg!(&a)); + | + + +error: aborting due to 7 previous errors + +For more information about this error, try `rustc --explain E0382`. diff --git a/tests/ui/rfcs/rfc-2361-dbg-macro/dbg-macro-move-semantics.stderr b/tests/ui/rfcs/rfc-2361-dbg-macro/dbg-macro-move-semantics.stderr index c2b9899e20db3..f515cb62c7cde 100644 --- a/tests/ui/rfcs/rfc-2361-dbg-macro/dbg-macro-move-semantics.stderr +++ b/tests/ui/rfcs/rfc-2361-dbg-macro/dbg-macro-move-semantics.stderr @@ -9,6 +9,10 @@ LL | let _ = dbg!(a); | ^^^^^^^ value used here after move | = note: this error originates in the macro `dbg` (in Nightly builds, run with -Z macro-backtrace for more info) +help: consider borrowing instead of transferring ownership + | +LL | let _ = dbg!(&a); + | + error: aborting due to 1 previous error From 841b30f63ea1592f40a906ed1400c1999b89b4ee Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Mon, 15 Jul 2024 16:43:18 -0400 Subject: [PATCH 9/9] Make sure trait def ids match before zipping args in note_function_argument_obligation --- .../src/error_reporting/traits/suggestions.rs | 1 + tests/crashes/126416.rs | 20 ----- tests/ui/methods/filter-relevant-fn-bounds.rs | 23 ++++++ .../methods/filter-relevant-fn-bounds.stderr | 74 +++++++++++++++++++ 4 files changed, 98 insertions(+), 20 deletions(-) delete mode 100644 tests/crashes/126416.rs create mode 100644 tests/ui/methods/filter-relevant-fn-bounds.rs create mode 100644 tests/ui/methods/filter-relevant-fn-bounds.stderr diff --git a/compiler/rustc_trait_selection/src/error_reporting/traits/suggestions.rs b/compiler/rustc_trait_selection/src/error_reporting/traits/suggestions.rs index 2cf808f962f08..f8843b892db3d 100644 --- a/compiler/rustc_trait_selection/src/error_reporting/traits/suggestions.rs +++ b/compiler/rustc_trait_selection/src/error_reporting/traits/suggestions.rs @@ -3810,6 +3810,7 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> { { if let Some(where_pred) = where_pred.as_trait_clause() && let Some(failed_pred) = failed_pred.as_trait_clause() + && where_pred.def_id() == failed_pred.def_id() { self.enter_forall(where_pred, |where_pred| { let failed_pred = self.instantiate_binder_with_fresh_vars( diff --git a/tests/crashes/126416.rs b/tests/crashes/126416.rs deleted file mode 100644 index 9b6c5169d444e..0000000000000 --- a/tests/crashes/126416.rs +++ /dev/null @@ -1,20 +0,0 @@ -//@ known-bug: rust-lang/rust#126416 - -trait Output<'a, T: 'a> { - type Type; -} - -struct Wrapper; - -impl Wrapper { - fn do_something_wrapper(&mut self, _: F) - where - F: for<'a> FnOnce(>::Type), - { - } -} - -fn main() { - let mut wrapper = Wrapper; - wrapper.do_something_wrapper(|value| ()); -} diff --git a/tests/ui/methods/filter-relevant-fn-bounds.rs b/tests/ui/methods/filter-relevant-fn-bounds.rs new file mode 100644 index 0000000000000..76ececf7baa71 --- /dev/null +++ b/tests/ui/methods/filter-relevant-fn-bounds.rs @@ -0,0 +1,23 @@ +trait Output<'a> { + type Type; +} + +struct Wrapper; + +impl Wrapper { + fn do_something_wrapper(self, _: F) + //~^ ERROR the trait bound `for<'a> F: Output<'a>` is not satisfied + //~| ERROR the trait bound `for<'a> F: Output<'a>` is not satisfied + where + F: for<'a> FnOnce(>::Type), + //~^ ERROR the trait bound `F: Output<'_>` is not satisfied + //~| ERROR the trait bound `F: Output<'_>` is not satisfied + { + } +} + +fn main() { + let mut wrapper = Wrapper; + wrapper.do_something_wrapper(|value| ()); + //~^ ERROR expected a `FnOnce +} diff --git a/tests/ui/methods/filter-relevant-fn-bounds.stderr b/tests/ui/methods/filter-relevant-fn-bounds.stderr new file mode 100644 index 0000000000000..b737c0ab11fd4 --- /dev/null +++ b/tests/ui/methods/filter-relevant-fn-bounds.stderr @@ -0,0 +1,74 @@ +error[E0277]: the trait bound `for<'a> F: Output<'a>` is not satisfied + --> $DIR/filter-relevant-fn-bounds.rs:8:5 + | +LL | / fn do_something_wrapper(self, _: F) +LL | | +LL | | +LL | | where +LL | | F: for<'a> FnOnce(>::Type), + | |___________________________________________________^ the trait `for<'a> Output<'a>` is not implemented for `F` + | +help: consider further restricting this bound + | +LL | F: for<'a> FnOnce(>::Type) + for<'a> Output<'a>, + | ++++++++++++++++++++ + +error[E0277]: the trait bound `for<'a> F: Output<'a>` is not satisfied + --> $DIR/filter-relevant-fn-bounds.rs:8:8 + | +LL | fn do_something_wrapper(self, _: F) + | ^^^^^^^^^^^^^^^^^^^^ the trait `for<'a> Output<'a>` is not implemented for `F` + | +help: consider further restricting this bound + | +LL | F: for<'a> FnOnce(>::Type) + for<'a> Output<'a>, + | ++++++++++++++++++++ + +error[E0277]: the trait bound `F: Output<'_>` is not satisfied + --> $DIR/filter-relevant-fn-bounds.rs:12:12 + | +LL | F: for<'a> FnOnce(>::Type), + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `Output<'_>` is not implemented for `F` + | +help: consider further restricting this bound + | +LL | F: for<'a> FnOnce(>::Type) + Output<'_>, + | ++++++++++++ + +error[E0277]: the trait bound `F: Output<'_>` is not satisfied + --> $DIR/filter-relevant-fn-bounds.rs:12:20 + | +LL | F: for<'a> FnOnce(>::Type), + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `Output<'_>` is not implemented for `F` + | +help: consider further restricting this bound + | +LL | F: for<'a> FnOnce(>::Type) + Output<'_>, + | ++++++++++++ + +error[E0277]: expected a `FnOnce(<{closure@$DIR/filter-relevant-fn-bounds.rs:21:34: 21:41} as Output<'a>>::Type)` closure, found `{closure@$DIR/filter-relevant-fn-bounds.rs:21:34: 21:41}` + --> $DIR/filter-relevant-fn-bounds.rs:21:34 + | +LL | wrapper.do_something_wrapper(|value| ()); + | -------------------- ^^^^^^^^^^ expected an `FnOnce(<{closure@$DIR/filter-relevant-fn-bounds.rs:21:34: 21:41} as Output<'a>>::Type)` closure, found `{closure@$DIR/filter-relevant-fn-bounds.rs:21:34: 21:41}` + | | + | required by a bound introduced by this call + | + = help: the trait `for<'a> Output<'a>` is not implemented for closure `{closure@$DIR/filter-relevant-fn-bounds.rs:21:34: 21:41}` +help: this trait has no implementations, consider adding one + --> $DIR/filter-relevant-fn-bounds.rs:1:1 + | +LL | trait Output<'a> { + | ^^^^^^^^^^^^^^^^ +note: required by a bound in `Wrapper::do_something_wrapper` + --> $DIR/filter-relevant-fn-bounds.rs:12:12 + | +LL | fn do_something_wrapper(self, _: F) + | -------------------- required by a bound in this associated function +... +LL | F: for<'a> FnOnce(>::Type), + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `Wrapper::do_something_wrapper` + +error: aborting due to 5 previous errors + +For more information about this error, try `rustc --explain E0277`.