From 83e1efb254d3de48c4c00eed67698a4d10173407 Mon Sep 17 00:00:00 2001 From: Zalathar Date: Wed, 10 Jul 2024 16:15:23 +1000 Subject: [PATCH 01/22] Replace a long inline "autoref" comment with method docs This comment has two problems: - It is very long, making the flow of the enclosing method hard to follow. - It starts by talking about an `autoref` flag that hasn't existed since #59114. This PR therefore replaces the long inline comment with a revised doc comment on `bind_matched_candidate_for_guard`, and some shorter inline comments. For readers who want more historical context, we also link to the PR that added the old comment, and the PR that removed the `autoref` flag. --- .../rustc_mir_build/src/build/matches/mod.rs | 165 +++++++++--------- 1 file changed, 84 insertions(+), 81 deletions(-) diff --git a/compiler/rustc_mir_build/src/build/matches/mod.rs b/compiler/rustc_mir_build/src/build/matches/mod.rs index 5695c881ecc22..7b92c0a1be8fd 100644 --- a/compiler/rustc_mir_build/src/build/matches/mod.rs +++ b/compiler/rustc_mir_build/src/build/matches/mod.rs @@ -2177,92 +2177,15 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { self.ascribe_types(block, ascriptions); - // rust-lang/rust#27282: The `autoref` business deserves some - // explanation here. - // - // The intent of the `autoref` flag is that when it is true, - // then any pattern bindings of type T will map to a `&T` - // within the context of the guard expression, but will - // continue to map to a `T` in the context of the arm body. To - // avoid surfacing this distinction in the user source code - // (which would be a severe change to the language and require - // far more revision to the compiler), when `autoref` is true, - // then any occurrence of the identifier in the guard - // expression will automatically get a deref op applied to it. - // - // So an input like: - // - // ``` - // let place = Foo::new(); - // match place { foo if inspect(foo) - // => feed(foo), ... } - // ``` - // - // will be treated as if it were really something like: - // - // ``` - // let place = Foo::new(); - // match place { Foo { .. } if { let tmp1 = &place; inspect(*tmp1) } - // => { let tmp2 = place; feed(tmp2) }, ... } - // ``` - // - // And an input like: - // - // ``` - // let place = Foo::new(); - // match place { ref mut foo if inspect(foo) - // => feed(foo), ... } - // ``` - // - // will be treated as if it were really something like: - // - // ``` - // let place = Foo::new(); - // match place { Foo { .. } if { let tmp1 = & &mut place; inspect(*tmp1) } - // => { let tmp2 = &mut place; feed(tmp2) }, ... } - // ``` - // - // In short, any pattern binding will always look like *some* - // kind of `&T` within the guard at least in terms of how the - // MIR-borrowck views it, and this will ensure that guard - // expressions cannot mutate their the match inputs via such - // bindings. (It also ensures that guard expressions can at - // most *copy* values from such bindings; non-Copy things - // cannot be moved via pattern bindings in guard expressions.) - // - // ---- - // - // Implementation notes (under assumption `autoref` is true). - // - // To encode the distinction above, we must inject the - // temporaries `tmp1` and `tmp2`. - // - // There are two cases of interest: binding by-value, and binding by-ref. - // - // 1. Binding by-value: Things are simple. - // - // * Establishing `tmp1` creates a reference into the - // matched place. This code is emitted by - // bind_matched_candidate_for_guard. - // - // * `tmp2` is only initialized "lazily", after we have - // checked the guard. Thus, the code that can trigger - // moves out of the candidate can only fire after the - // guard evaluated to true. This initialization code is - // emitted by bind_matched_candidate_for_arm. - // - // 2. Binding by-reference: Things are tricky. - // - // * Here, the guard expression wants a `&&` or `&&mut` - // into the original input. This means we need to borrow - // the reference that we create for the arm. - // * So we eagerly create the reference for the arm and then take a - // reference to that. + // Lower an instance of the arm guard (if present) for this candidate, + // and then perform bindings for the arm body. if let Some((arm, match_scope)) = arm_match_scope && let Some(guard) = arm.guard { let tcx = self.tcx; + // Bindings for guards require some extra handling to automatically + // insert implicit references/dereferences. self.bind_matched_candidate_for_guard(block, schedule_drops, bindings.clone()); let guard_frame = GuardFrame { locals: bindings.clone().map(|b| GuardFrameLocal::new(b.var_id)).collect(), @@ -2402,6 +2325,82 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { } } + /// Binding for guards is a bit different from binding for the arm body, + /// because an extra layer of implicit reference/dereference is added. + /// + /// The idea is that any pattern bindings of type T will map to a `&T` within + /// the context of the guard expression, but will continue to map to a `T` + /// in the context of the arm body. To avoid surfacing this distinction in + /// the user source code (which would be a severe change to the language and + /// require far more revision to the compiler), any occurrence of the + /// identifier in the guard expression will automatically get a deref op + /// applied to it. (See the caller of [`Self::is_bound_var_in_guard`].) + /// + /// So an input like: + /// + /// ```ignore (illustrative) + /// let place = Foo::new(); + /// match place { foo if inspect(foo) + /// => feed(foo), ... } + /// ``` + /// + /// will be treated as if it were really something like: + /// + /// ```ignore (illustrative) + /// let place = Foo::new(); + /// match place { Foo { .. } if { let tmp1 = &place; inspect(*tmp1) } + /// => { let tmp2 = place; feed(tmp2) }, ... } + /// ``` + /// + /// And an input like: + /// + /// ```ignore (illustrative) + /// let place = Foo::new(); + /// match place { ref mut foo if inspect(foo) + /// => feed(foo), ... } + /// ``` + /// + /// will be treated as if it were really something like: + /// + /// ```ignore (illustrative) + /// let place = Foo::new(); + /// match place { Foo { .. } if { let tmp1 = & &mut place; inspect(*tmp1) } + /// => { let tmp2 = &mut place; feed(tmp2) }, ... } + /// ``` + /// --- + /// + /// ## Implementation notes + /// + /// To encode the distinction above, we must inject the + /// temporaries `tmp1` and `tmp2`. + /// + /// There are two cases of interest: binding by-value, and binding by-ref. + /// + /// 1. Binding by-value: Things are simple. + /// + /// * Establishing `tmp1` creates a reference into the + /// matched place. This code is emitted by + /// [`Self::bind_matched_candidate_for_guard`]. + /// + /// * `tmp2` is only initialized "lazily", after we have + /// checked the guard. Thus, the code that can trigger + /// moves out of the candidate can only fire after the + /// guard evaluated to true. This initialization code is + /// emitted by [`Self::bind_matched_candidate_for_arm_body`]. + /// + /// 2. Binding by-reference: Things are tricky. + /// + /// * Here, the guard expression wants a `&&` or `&&mut` + /// into the original input. This means we need to borrow + /// the reference that we create for the arm. + /// * So we eagerly create the reference for the arm and then take a + /// reference to that. + /// + /// --- + /// + /// See these PRs for some historical context: + /// - (introduction of autoref) + /// - (always use autoref) fn bind_matched_candidate_for_guard<'b>( &mut self, block: BasicBlock, @@ -2433,10 +2432,13 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { ); match binding.binding_mode.0 { ByRef::No => { + // The arm binding will be by value, so for the guard binding + // just take a shared reference to the matched place. let rvalue = Rvalue::Ref(re_erased, BorrowKind::Shared, binding.source); self.cfg.push_assign(block, source_info, ref_for_guard, rvalue); } ByRef::Yes(mutbl) => { + // The arm binding will be by reference, so eagerly create it now. let value_for_arm = self.storage_live_binding( block, binding.var_id, @@ -2448,6 +2450,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { let rvalue = Rvalue::Ref(re_erased, util::ref_pat_borrow_kind(mutbl), binding.source); self.cfg.push_assign(block, source_info, value_for_arm, rvalue); + // For the guard binding, take a shared reference to that reference. let rvalue = Rvalue::Ref(re_erased, BorrowKind::Shared, value_for_arm); self.cfg.push_assign(block, source_info, ref_for_guard, rvalue); } From ec6e07b09d0c499f4f3233f04826ecf2ce77800b Mon Sep 17 00:00:00 2001 From: Jerry Wang Date: Sat, 13 Jul 2024 14:27:56 -0400 Subject: [PATCH 02/22] Migrate `crate-hash-rustc-version` to `rmake` --- .../tidy/src/allowed_run_make_makefiles.txt | 1 - .../crate-hash-rustc-version/Makefile | 38 --------------- .../crate-hash-rustc-version/rmake.rs | 47 +++++++++++++++++++ 3 files changed, 47 insertions(+), 39 deletions(-) delete mode 100644 tests/run-make/crate-hash-rustc-version/Makefile create mode 100644 tests/run-make/crate-hash-rustc-version/rmake.rs diff --git a/src/tools/tidy/src/allowed_run_make_makefiles.txt b/src/tools/tidy/src/allowed_run_make_makefiles.txt index f7ec7d0b3f6b9..b2c1ff3f5f418 100644 --- a/src/tools/tidy/src/allowed_run_make_makefiles.txt +++ b/src/tools/tidy/src/allowed_run_make_makefiles.txt @@ -12,7 +12,6 @@ run-make/cdylib-dylib-linkage/Makefile run-make/compiler-lookup-paths-2/Makefile run-make/compiler-lookup-paths/Makefile run-make/compiler-rt-works-on-mingw/Makefile -run-make/crate-hash-rustc-version/Makefile run-make/cross-lang-lto-clang/Makefile run-make/cross-lang-lto-pgo-smoketest/Makefile run-make/cross-lang-lto-upstream-rlibs/Makefile diff --git a/tests/run-make/crate-hash-rustc-version/Makefile b/tests/run-make/crate-hash-rustc-version/Makefile deleted file mode 100644 index 6bf504bf01be5..0000000000000 --- a/tests/run-make/crate-hash-rustc-version/Makefile +++ /dev/null @@ -1,38 +0,0 @@ -# ignore-cross-compile -include ../tools.mk - -# Ensure that crates compiled with different rustc versions cannot -# be dynamically linked. - -FLAGS := -Cprefer-dynamic -Csymbol-mangling-version=v0 -UNAME := $(shell uname) -ifeq ($(UNAME),Linux) - EXT=".so" - NM_CMD := nm -D -endif -ifeq ($(UNAME),Darwin) - EXT=".dylib" - NM_CMD := nm -endif - -ifndef NM_CMD -all: - exit 0 -else -all: - # a.rs is a dylib - $(RUSTC) a.rs --crate-type=dylib $(FLAGS) - # Write symbols to disk. - $(NM_CMD) $(call DYLIB,a) > $(TMPDIR)/symbolsbefore - # b.rs is a binary - $(RUSTC) b.rs --extern a=$(TMPDIR)/liba$(EXT) --crate-type=bin -Crpath $(FLAGS) - $(call RUN,b) - # Now re-compile a.rs with another rustc version - RUSTC_FORCE_RUSTC_VERSION=deadfeed $(RUSTC) a.rs --crate-type=dylib $(FLAGS) - # After compiling with a different rustc version, write symbols to disk again. - $(NM_CMD) $(call DYLIB,a) > $(TMPDIR)/symbolsafter - # As a sanity check, test if the symbols changed: - # If the symbols are identical, there's been an error. - if diff $(TMPDIR)/symbolsbefore $(TMPDIR)/symbolsafter; then exit 1; fi - $(call FAIL,b) -endif diff --git a/tests/run-make/crate-hash-rustc-version/rmake.rs b/tests/run-make/crate-hash-rustc-version/rmake.rs new file mode 100644 index 0000000000000..6418deb126bd9 --- /dev/null +++ b/tests/run-make/crate-hash-rustc-version/rmake.rs @@ -0,0 +1,47 @@ +// Ensure that crates compiled with different rustc versions cannot +// be dynamically linked. + +//@ ignore-cross-compile +//@ only-unix + +use run_make_support::{cmd, diff, dynamic_lib_name, is_darwin, run, run_fail, rustc}; + +fn main() { + let flags = ["-Cprefer-dynamic", "-Csymbol-mangling-version=v0"]; + let nm_flag = if is_darwin() { [].as_slice() } else { ["-D"].as_slice() }; + + // a.rs is compiled to a dylib + rustc().input("a.rs").crate_type("dylib").args(&flags).run(); + + // Store symbols + let symbols_before = cmd("nm").args(nm_flag).arg(dynamic_lib_name("a")).run().stdout_utf8(); + + // b.rs is compiled to a binary + rustc() + .input("b.rs") + .extern_("a", dynamic_lib_name("a")) + .crate_type("bin") + .arg("-Crpath") + .args(&flags) + .run(); + run("b"); + + // Now re-compile a.rs with another rustc version + rustc() + .env("RUSTC_FORCE_RUSTC_VERSION", "deadfeed") + .input("a.rs") + .crate_type("dylib") + .args(&flags) + .run(); + + // After compiling with a different rustc version, store symbols again. + let symbols_after = cmd("nm").args(nm_flag).arg(dynamic_lib_name("a")).run().stdout_utf8(); + + // As a sanity check, test if the symbols changed: + // If the symbols are identical, there's been an error. + diff() + .expected_text("symbols_before", symbols_before) + .actual_text("symbols_after", symbols_after) + .run_fail(); + run_fail("b"); +} From eea6502dcb91942785b1d5cb8fea74c41bcc3f32 Mon Sep 17 00:00:00 2001 From: Jerry Wang Date: Mon, 15 Jul 2024 19:52:22 -0400 Subject: [PATCH 03/22] Use `llvm-readobj` for `run-make/crate-hash-rustc-version` --- .../run-make/crate-hash-rustc-version/rmake.rs | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/tests/run-make/crate-hash-rustc-version/rmake.rs b/tests/run-make/crate-hash-rustc-version/rmake.rs index 6418deb126bd9..97b3dd3931e87 100644 --- a/tests/run-make/crate-hash-rustc-version/rmake.rs +++ b/tests/run-make/crate-hash-rustc-version/rmake.rs @@ -4,17 +4,27 @@ //@ ignore-cross-compile //@ only-unix -use run_make_support::{cmd, diff, dynamic_lib_name, is_darwin, run, run_fail, rustc}; +use run_make_support::llvm; +use run_make_support::{diff, dynamic_lib_name, is_darwin, run, run_fail, rustc}; + +fn llvm_readobj() -> llvm::LlvmReadobj { + let mut cmd = llvm::llvm_readobj(); + if is_darwin() { + cmd.symbols(); + } else { + cmd.dynamic_table(); + } + cmd +} fn main() { let flags = ["-Cprefer-dynamic", "-Csymbol-mangling-version=v0"]; - let nm_flag = if is_darwin() { [].as_slice() } else { ["-D"].as_slice() }; // a.rs is compiled to a dylib rustc().input("a.rs").crate_type("dylib").args(&flags).run(); // Store symbols - let symbols_before = cmd("nm").args(nm_flag).arg(dynamic_lib_name("a")).run().stdout_utf8(); + let symbols_before = llvm_readobj().arg(dynamic_lib_name("a")).run().stdout_utf8(); // b.rs is compiled to a binary rustc() @@ -35,7 +45,7 @@ fn main() { .run(); // After compiling with a different rustc version, store symbols again. - let symbols_after = cmd("nm").args(nm_flag).arg(dynamic_lib_name("a")).run().stdout_utf8(); + let symbols_after = llvm_readobj().arg(dynamic_lib_name("a")).run().stdout_utf8(); // As a sanity check, test if the symbols changed: // If the symbols are identical, there's been an error. From a605e2f498ce9383d2b9ee44a796e5316c7a5e6c Mon Sep 17 00:00:00 2001 From: Chris Denton Date: Thu, 18 Jul 2024 13:42:11 +0000 Subject: [PATCH 04/22] Safely enforce thread name requirements --- library/std/src/thread/mod.rs | 39 +++++++++++++++++++++++++---------- 1 file changed, 28 insertions(+), 11 deletions(-) diff --git a/library/std/src/thread/mod.rs b/library/std/src/thread/mod.rs index c8ee365392f85..87d5c2f742c41 100644 --- a/library/std/src/thread/mod.rs +++ b/library/std/src/thread/mod.rs @@ -161,7 +161,7 @@ mod tests; use crate::any::Any; use crate::cell::{OnceCell, UnsafeCell}; use crate::env; -use crate::ffi::{CStr, CString}; +use crate::ffi::CStr; use crate::fmt; use crate::io; use crate::marker::PhantomData; @@ -487,11 +487,7 @@ impl Builder { amt }); - let my_thread = name.map_or_else(Thread::new_unnamed, |name| unsafe { - Thread::new( - CString::new(name).expect("thread name may not contain interior null bytes"), - ) - }); + let my_thread = name.map_or_else(Thread::new_unnamed, |name| Thread::new(name.into())); let their_thread = my_thread.clone(); let my_packet: Arc> = Arc::new(Packet { @@ -1273,10 +1269,34 @@ impl ThreadId { /// The internal representation of a `Thread`'s name. enum ThreadName { Main, - Other(CString), + Other(ThreadNameString), Unnamed, } +// This module ensures private fields are kept private, which is necessary to enforce the safety requirements. +mod thread_name_string { + use crate::ffi::{CStr, CString}; + + /// Like a `String` it's guaranteed UTF-8 and like a `CString` it's null terminated. + pub(crate) struct ThreadNameString { + inner: CString, + } + impl core::ops::Deref for ThreadNameString { + type Target = CStr; + fn deref(&self) -> &CStr { + &self.inner + } + } + impl From for ThreadNameString { + fn from(s: String) -> Self { + Self { + inner: CString::new(s).expect("thread name may not contain interior null bytes"), + } + } + } +} +pub(crate) use thread_name_string::ThreadNameString; + /// The internal representation of a `Thread` handle struct Inner { name: ThreadName, // Guaranteed to be UTF-8 @@ -1316,10 +1336,7 @@ pub struct Thread { impl Thread { /// Used only internally to construct a thread object without spawning. - /// - /// # Safety - /// `name` must be valid UTF-8. - pub(crate) unsafe fn new(name: CString) -> Thread { + pub(crate) fn new(name: ThreadNameString) -> Thread { unsafe { Self::new_inner(ThreadName::Other(name)) } } From 939ee383043dce00c108c22f1e933f1e052f3f3d Mon Sep 17 00:00:00 2001 From: Chris Denton Date: Thu, 18 Jul 2024 17:33:52 +0000 Subject: [PATCH 05/22] Make `Thread::new_inner` a safe function --- library/std/src/thread/mod.rs | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/library/std/src/thread/mod.rs b/library/std/src/thread/mod.rs index 87d5c2f742c41..13b02cd8d3abe 100644 --- a/library/std/src/thread/mod.rs +++ b/library/std/src/thread/mod.rs @@ -1337,21 +1337,19 @@ pub struct Thread { impl Thread { /// Used only internally to construct a thread object without spawning. pub(crate) fn new(name: ThreadNameString) -> Thread { - unsafe { Self::new_inner(ThreadName::Other(name)) } + Self::new_inner(ThreadName::Other(name)) } pub(crate) fn new_unnamed() -> Thread { - unsafe { Self::new_inner(ThreadName::Unnamed) } + Self::new_inner(ThreadName::Unnamed) } // Used in runtime to construct main thread pub(crate) fn new_main() -> Thread { - unsafe { Self::new_inner(ThreadName::Main) } + Self::new_inner(ThreadName::Main) } - /// # Safety - /// If `name` is `ThreadName::Other(_)`, the contained string must be valid UTF-8. - unsafe fn new_inner(name: ThreadName) -> Thread { + fn new_inner(name: ThreadName) -> Thread { // We have to use `unsafe` here to construct the `Parker` in-place, // which is required for the UNIX implementation. // From 8e4a9205e9fc0a86695799042e6fb70756bbfea2 Mon Sep 17 00:00:00 2001 From: Chris Denton Date: Thu, 18 Jul 2024 18:10:36 +0000 Subject: [PATCH 06/22] Style change --- library/std/src/thread/mod.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/library/std/src/thread/mod.rs b/library/std/src/thread/mod.rs index 13b02cd8d3abe..f3d20026c27b1 100644 --- a/library/std/src/thread/mod.rs +++ b/library/std/src/thread/mod.rs @@ -487,7 +487,7 @@ impl Builder { amt }); - let my_thread = name.map_or_else(Thread::new_unnamed, |name| Thread::new(name.into())); + let my_thread = name.map_or_else(Thread::new_unnamed, Thread::new); let their_thread = my_thread.clone(); let my_packet: Arc> = Arc::new(Packet { @@ -1336,8 +1336,8 @@ pub struct Thread { impl Thread { /// Used only internally to construct a thread object without spawning. - pub(crate) fn new(name: ThreadNameString) -> Thread { - Self::new_inner(ThreadName::Other(name)) + pub(crate) fn new(name: String) -> Thread { + Self::new_inner(ThreadName::Other(name.into())) } pub(crate) fn new_unnamed() -> Thread { From 9432955a01b989d3f4122887875643ef8dbea589 Mon Sep 17 00:00:00 2001 From: Chris Denton Date: Thu, 18 Jul 2024 18:13:11 +0000 Subject: [PATCH 07/22] Move ThreadName conversions to &cstr/&str --- library/std/src/thread/mod.rs | 25 +++++++++++++++++++------ 1 file changed, 19 insertions(+), 6 deletions(-) diff --git a/library/std/src/thread/mod.rs b/library/std/src/thread/mod.rs index f3d20026c27b1..0c908a5adae4e 100644 --- a/library/std/src/thread/mod.rs +++ b/library/std/src/thread/mod.rs @@ -1275,7 +1275,9 @@ enum ThreadName { // This module ensures private fields are kept private, which is necessary to enforce the safety requirements. mod thread_name_string { + use super::ThreadName; use crate::ffi::{CStr, CString}; + use core::str; /// Like a `String` it's guaranteed UTF-8 and like a `CString` it's null terminated. pub(crate) struct ThreadNameString { @@ -1294,6 +1296,21 @@ mod thread_name_string { } } } + impl ThreadName { + pub fn as_cstr(&self) -> Option<&CStr> { + match self { + ThreadName::Main => Some(c"main"), + ThreadName::Other(other) => Some(other), + ThreadName::Unnamed => None, + } + } + + pub fn as_str(&self) -> Option<&str> { + // SAFETY: `as_cstr` can only return `Some` for a fixed CStr or a `ThreadNameString`, + // which is guaranteed to be UTF-8. + self.as_cstr().map(|s| unsafe { str::from_utf8_unchecked(s.to_bytes()) }) + } + } } pub(crate) use thread_name_string::ThreadNameString; @@ -1472,15 +1489,11 @@ impl Thread { #[stable(feature = "rust1", since = "1.0.0")] #[must_use] pub fn name(&self) -> Option<&str> { - self.cname().map(|s| unsafe { str::from_utf8_unchecked(s.to_bytes()) }) + self.inner.name.as_str() } fn cname(&self) -> Option<&CStr> { - match &self.inner.name { - ThreadName::Main => Some(c"main"), - ThreadName::Other(other) => Some(&other), - ThreadName::Unnamed => None, - } + self.inner.name.as_cstr() } } From 9747a2c3efc231b064c2d9eb3e0cf23718a7d1b9 Mon Sep 17 00:00:00 2001 From: surechen Date: Fri, 19 Jul 2024 09:34:32 +0800 Subject: [PATCH 08/22] fixes panic error fixes #127915 --- .../src/diagnostics/conflict_errors.rs | 13 +++++++--- ...-error-suggest-clone-panic-issue-127915.rs | 15 +++++++++++ ...or-suggest-clone-panic-issue-127915.stderr | 25 +++++++++++++++++++ 3 files changed, 49 insertions(+), 4 deletions(-) create mode 100644 tests/ui/borrowck/move-error-suggest-clone-panic-issue-127915.rs create mode 100644 tests/ui/borrowck/move-error-suggest-clone-panic-issue-127915.stderr diff --git a/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs b/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs index f7e4bba371220..a4fd00efe0506 100644 --- a/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs +++ b/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs @@ -456,10 +456,15 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, '_, 'infcx, 'tcx> { if let Some(def_id) = def_id && self.infcx.tcx.def_kind(def_id).is_fn_like() && let Some(pos) = args.iter().position(|arg| arg.hir_id == expr.hir_id) - && let ty::Param(_) = - self.infcx.tcx.fn_sig(def_id).skip_binder().skip_binder().inputs() - [pos + offset] - .kind() + && let Some(arg) = self + .infcx + .tcx + .fn_sig(def_id) + .skip_binder() + .skip_binder() + .inputs() + .get(pos + offset) + && let ty::Param(_) = arg.kind() { let place = &self.move_data.move_paths[mpi].place; let ty = place.ty(self.body, self.infcx.tcx).ty; diff --git a/tests/ui/borrowck/move-error-suggest-clone-panic-issue-127915.rs b/tests/ui/borrowck/move-error-suggest-clone-panic-issue-127915.rs new file mode 100644 index 0000000000000..e422026ca4a27 --- /dev/null +++ b/tests/ui/borrowck/move-error-suggest-clone-panic-issue-127915.rs @@ -0,0 +1,15 @@ +#![allow(dead_code)] + +extern "C" { + fn rust_interesting_average(_: i64, ...) -> f64; +} + +fn test(a: i64, b: i64, c: i64, d: i64, e: i64, f: T, g: U) -> i64 { + unsafe { + rust_interesting_average( + 6, a as f64, b, b as f64, f, c as f64, d, d as f64, e, e as f64, f, g, //~ ERROR use of moved value: `f` [E0382] + ) as i64 + } +} + +fn main() {} diff --git a/tests/ui/borrowck/move-error-suggest-clone-panic-issue-127915.stderr b/tests/ui/borrowck/move-error-suggest-clone-panic-issue-127915.stderr new file mode 100644 index 0000000000000..6997710ec89fa --- /dev/null +++ b/tests/ui/borrowck/move-error-suggest-clone-panic-issue-127915.stderr @@ -0,0 +1,25 @@ +error[E0382]: use of moved value: `f` + --> $DIR/move-error-suggest-clone-panic-issue-127915.rs:10:78 + | +LL | fn test(a: i64, b: i64, c: i64, d: i64, e: i64, f: T, g: U) -> i64 { + | - move occurs because `f` has type `T`, which does not implement the `Copy` trait +... +LL | 6, a as f64, b, b as f64, f, c as f64, d, d as f64, e, e as f64, f, g, + | - value moved here ^ value used here after move + | +help: if `T` implemented `Clone`, you could clone the value + --> $DIR/move-error-suggest-clone-panic-issue-127915.rs:7:9 + | +LL | fn test(a: i64, b: i64, c: i64, d: i64, e: i64, f: T, g: U) -> i64 { + | ^ consider constraining this type parameter with `Clone` +... +LL | 6, a as f64, b, b as f64, f, c as f64, d, d as f64, e, e as f64, f, g, + | - you could clone this value +help: consider restricting type parameter `T` + | +LL | fn test(a: i64, b: i64, c: i64, d: i64, e: i64, f: T, g: U) -> i64 { + | ++++++ + +error: aborting due to 1 previous error + +For more information about this error, try `rustc --explain E0382`. From a8d7121e4a02764962c85e29168bebed2116b392 Mon Sep 17 00:00:00 2001 From: Ayush Singh Date: Wed, 27 Mar 2024 01:07:39 +0530 Subject: [PATCH 09/22] uefi: Add process Signed-off-by: Ayush Singh --- library/std/src/sys/pal/uefi/helpers.rs | 72 +++++- library/std/src/sys/pal/uefi/mod.rs | 1 - library/std/src/sys/pal/uefi/process.rs | 328 ++++++++++++++++++++++++ 3 files changed, 399 insertions(+), 2 deletions(-) create mode 100644 library/std/src/sys/pal/uefi/process.rs diff --git a/library/std/src/sys/pal/uefi/helpers.rs b/library/std/src/sys/pal/uefi/helpers.rs index 23aa4da14a763..97dd90716bbb0 100644 --- a/library/std/src/sys/pal/uefi/helpers.rs +++ b/library/std/src/sys/pal/uefi/helpers.rs @@ -12,7 +12,7 @@ use r_efi::efi::{self, Guid}; use r_efi::protocols::{device_path, device_path_to_text}; -use crate::ffi::OsString; +use crate::ffi::{OsString, OsStr}; use crate::io::{self, const_io_error}; use crate::mem::{size_of, MaybeUninit}; use crate::os::uefi::{self, env::boot_services, ffi::OsStringExt}; @@ -221,3 +221,73 @@ pub(crate) fn runtime_services() -> Option> let runtime_services = unsafe { (*system_table.as_ptr()).runtime_services }; NonNull::new(runtime_services) } + +pub(crate) struct DevicePath(NonNull); + +impl DevicePath { + pub(crate) fn from_text(p: &OsStr) -> io::Result { + fn inner( + p: &OsStr, + protocol: NonNull, + ) -> io::Result { + let path_vec = p.encode_wide().chain(Some(0)).collect::>(); + let path = + unsafe { ((*protocol.as_ptr()).convert_text_to_device_path)(path_vec.as_ptr()) }; + + NonNull::new(path).map(DevicePath).ok_or_else(|| { + const_io_error!(io::ErrorKind::InvalidFilename, "Invalid Device Path") + }) + } + + static LAST_VALID_HANDLE: AtomicPtr = + AtomicPtr::new(crate::ptr::null_mut()); + + if let Some(handle) = NonNull::new(LAST_VALID_HANDLE.load(Ordering::Acquire)) { + if let Ok(protocol) = open_protocol::( + handle, + r_efi::protocols::device_path_from_text::PROTOCOL_GUID, + ) { + return inner(p, protocol); + } + } + + let handles = locate_handles(r_efi::protocols::device_path_from_text::PROTOCOL_GUID)?; + for handle in handles { + if let Ok(protocol) = open_protocol::( + handle, + r_efi::protocols::device_path_from_text::PROTOCOL_GUID, + ) { + LAST_VALID_HANDLE.store(handle.as_ptr(), Ordering::Release); + return inner(p, protocol); + } + } + + io::Result::Err(const_io_error!( + io::ErrorKind::NotFound, + "DevicePathFromText Protocol not found" + )) + } +} + +impl AsRef for DevicePath { + fn as_ref(&self) -> &r_efi::protocols::device_path::Protocol { + unsafe { self.0.as_ref() } + } +} + +impl AsMut for DevicePath { + fn as_mut(&mut self) -> &mut r_efi::protocols::device_path::Protocol { + unsafe { self.0.as_mut() } + } +} + +impl Drop for DevicePath { + fn drop(&mut self) { + if let Some(bt) = boot_services() { + let bt: NonNull = bt.cast(); + unsafe { + ((*bt.as_ptr()).free_pool)(self.0.as_ptr() as *mut crate::ffi::c_void); + } + } + } +} diff --git a/library/std/src/sys/pal/uefi/mod.rs b/library/std/src/sys/pal/uefi/mod.rs index 4d50d9e8c3d9c..c54e9477bfc1f 100644 --- a/library/std/src/sys/pal/uefi/mod.rs +++ b/library/std/src/sys/pal/uefi/mod.rs @@ -25,7 +25,6 @@ pub mod net; pub mod os; #[path = "../unsupported/pipe.rs"] pub mod pipe; -#[path = "../unsupported/process.rs"] pub mod process; pub mod stdio; pub mod thread; diff --git a/library/std/src/sys/pal/uefi/process.rs b/library/std/src/sys/pal/uefi/process.rs new file mode 100644 index 0000000000000..e85fc209c0e18 --- /dev/null +++ b/library/std/src/sys/pal/uefi/process.rs @@ -0,0 +1,328 @@ +use crate::ffi::OsStr; +use crate::ffi::OsString; +use crate::fmt; +use crate::io; +use crate::marker::PhantomData; +use crate::num::NonZero; +use crate::num::NonZeroI32; +use crate::path::Path; +use crate::sys::fs::File; +use crate::sys::pipe::AnonPipe; +use crate::sys::unsupported; +use crate::sys_common::process::{CommandEnv, CommandEnvs}; + +pub use crate::ffi::OsString as EnvKey; + +//////////////////////////////////////////////////////////////////////////////// +// Command +//////////////////////////////////////////////////////////////////////////////// + +pub struct Command { + prog: OsString, +} + +// passed back to std::process with the pipes connected to the child, if any +// were requested +pub struct StdioPipes { + pub stdin: Option, + pub stdout: Option, + pub stderr: Option, +} + +// FIXME: This should be a unit struct, so we can always construct it +// The value here should be never used, since we cannot spawn processes. +pub enum Stdio { + Inherit, + Null, + MakePipe, +} + +impl Command { + pub fn new(program: &OsStr) -> Command { + Command { prog: program.to_os_string() } + } + + pub fn arg(&mut self, _arg: &OsStr) { + panic!("unsupported") + } + + pub fn env_mut(&mut self) -> &mut CommandEnv { + panic!("unsupported") + } + + pub fn cwd(&mut self, _dir: &OsStr) { + panic!("unsupported") + } + + pub fn stdin(&mut self, _stdin: Stdio) { + panic!("unsupported") + } + + pub fn stdout(&mut self, _stdout: Stdio) { + panic!("unsupported") + } + + pub fn stderr(&mut self, _stderr: Stdio) { + panic!("unsupported") + } + + pub fn get_program(&self) -> &OsStr { + panic!("unsupported") + } + + pub fn get_args(&self) -> CommandArgs<'_> { + panic!("unsupported") + } + + pub fn get_envs(&self) -> CommandEnvs<'_> { + panic!("unsupported") + } + + pub fn get_current_dir(&self) -> Option<&Path> { + None + } + + pub fn spawn( + &mut self, + _default: Stdio, + _needs_stdin: bool, + ) -> io::Result<(Process, StdioPipes)> { + unsupported() + } + + pub fn output(&mut self) -> io::Result<(ExitStatus, Vec, Vec)> { + let cmd = uefi_command_internal::Command::load_image(&self.prog)?; + let stat = cmd.start_image()?; + Ok((ExitStatus(stat), Vec::new(), Vec::new())) + } +} + +impl From for Stdio { + fn from(pipe: AnonPipe) -> Stdio { + pipe.diverge() + } +} + +impl From for Stdio { + fn from(_: io::Stdout) -> Stdio { + // FIXME: This is wrong. + // Instead, the Stdio we have here should be a unit struct. + panic!("unsupported") + } +} + +impl From for Stdio { + fn from(_: io::Stderr) -> Stdio { + // FIXME: This is wrong. + // Instead, the Stdio we have here should be a unit struct. + panic!("unsupported") + } +} + +impl From for Stdio { + fn from(_file: File) -> Stdio { + // FIXME: This is wrong. + // Instead, the Stdio we have here should be a unit struct. + panic!("unsupported") + } +} + +impl fmt::Debug for Command { + fn fmt(&self, _f: &mut fmt::Formatter<'_>) -> fmt::Result { + Ok(()) + } +} + +#[derive(PartialEq, Eq, Clone, Copy, Debug)] +#[non_exhaustive] +pub struct ExitStatus(r_efi::efi::Status); + +impl ExitStatus { + pub fn exit_ok(&self) -> Result<(), ExitStatusError> { + if self.0 == r_efi::efi::Status::SUCCESS { Ok(()) } else { Err(ExitStatusError(self.0)) } + } + + pub fn code(&self) -> Option { + Some(self.0.as_usize() as i32) + } +} + +impl fmt::Display for ExitStatus { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + let err_str = super::os::error_string(self.0.as_usize()); + write!(f, "{}", err_str) + } +} + +impl Default for ExitStatus { + fn default() -> Self { + ExitStatus(r_efi::efi::Status::SUCCESS) + } +} + +#[derive(Clone, Copy, PartialEq, Eq)] +pub struct ExitStatusError(r_efi::efi::Status); + +impl fmt::Debug for ExitStatusError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + let err_str = super::os::error_string(self.0.as_usize()); + write!(f, "{}", err_str) + } +} + +impl Into for ExitStatusError { + fn into(self) -> ExitStatus { + ExitStatus(self.0) + } +} + +impl ExitStatusError { + pub fn code(self) -> Option> { + NonZeroI32::new(self.0.as_usize() as i32) + } +} + +#[derive(PartialEq, Eq, Clone, Copy, Debug)] +pub struct ExitCode(bool); + +impl ExitCode { + pub const SUCCESS: ExitCode = ExitCode(false); + pub const FAILURE: ExitCode = ExitCode(true); + + pub fn as_i32(&self) -> i32 { + self.0 as i32 + } +} + +impl From for ExitCode { + fn from(code: u8) -> Self { + match code { + 0 => Self::SUCCESS, + 1..=255 => Self::FAILURE, + } + } +} + +pub struct Process(!); + +impl Process { + pub fn id(&self) -> u32 { + self.0 + } + + pub fn kill(&mut self) -> io::Result<()> { + self.0 + } + + pub fn wait(&mut self) -> io::Result { + self.0 + } + + pub fn try_wait(&mut self) -> io::Result> { + self.0 + } +} + +pub struct CommandArgs<'a> { + _p: PhantomData<&'a ()>, +} + +impl<'a> Iterator for CommandArgs<'a> { + type Item = &'a OsStr; + fn next(&mut self) -> Option<&'a OsStr> { + None + } + fn size_hint(&self) -> (usize, Option) { + (0, Some(0)) + } +} + +impl<'a> ExactSizeIterator for CommandArgs<'a> {} + +impl<'a> fmt::Debug for CommandArgs<'a> { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.debug_list().finish() + } +} + +mod uefi_command_internal { + use super::super::helpers; + use crate::ffi::OsStr; + use crate::io::{self, const_io_error}; + use crate::mem::MaybeUninit; + use crate::os::uefi::env::{boot_services, image_handle}; + use crate::ptr::NonNull; + + pub struct Command { + handle: NonNull, + } + + impl Command { + const fn new(handle: NonNull) -> Self { + Self { handle } + } + + pub fn load_image(p: &OsStr) -> io::Result { + let mut path = helpers::DevicePath::from_text(p)?; + let boot_services: NonNull = boot_services() + .ok_or_else(|| const_io_error!(io::ErrorKind::NotFound, "Boot Services not found"))? + .cast(); + let mut child_handle: MaybeUninit = MaybeUninit::uninit(); + let image_handle = image_handle(); + + let r = unsafe { + ((*boot_services.as_ptr()).load_image)( + r_efi::efi::Boolean::FALSE, + image_handle.as_ptr(), + path.as_mut(), + crate::ptr::null_mut(), + 0, + child_handle.as_mut_ptr(), + ) + }; + + if r.is_error() { + Err(io::Error::from_raw_os_error(r.as_usize())) + } else { + let child_handle = unsafe { child_handle.assume_init() }; + let child_handle = NonNull::new(child_handle).unwrap(); + Ok(Self::new(child_handle)) + } + } + + pub fn start_image(&self) -> io::Result { + let boot_services: NonNull = boot_services() + .ok_or_else(|| const_io_error!(io::ErrorKind::NotFound, "Boot Services not found"))? + .cast(); + let mut exit_data_size: MaybeUninit = MaybeUninit::uninit(); + let mut exit_data: MaybeUninit<*mut u16> = MaybeUninit::uninit(); + + let r = unsafe { + ((*boot_services.as_ptr()).start_image)( + self.handle.as_ptr(), + exit_data_size.as_mut_ptr(), + exit_data.as_mut_ptr(), + ) + }; + + // Drop exitdata + unsafe { + exit_data_size.assume_init_drop(); + exit_data.assume_init_drop(); + } + + Ok(r) + } + } + + impl Drop for Command { + fn drop(&mut self) { + if let Some(bt) = boot_services() { + let bt: NonNull = bt.cast(); + unsafe { + ((*bt.as_ptr()).unload_image)(self.handle.as_ptr()); + } + } + } + } +} From 6737a02a50b047deb4d50bd03899683ab6c37a1f Mon Sep 17 00:00:00 2001 From: Ayush Singh Date: Fri, 29 Mar 2024 17:13:16 +0530 Subject: [PATCH 10/22] uefi: process: Add support to capture stdout Use a custom simple_text_output protocol to capture output. Signed-off-by: Ayush Singh --- library/std/src/sys/pal/uefi/helpers.rs | 76 +++++++++- library/std/src/sys/pal/uefi/process.rs | 190 +++++++++++++++++++++++- 2 files changed, 258 insertions(+), 8 deletions(-) diff --git a/library/std/src/sys/pal/uefi/helpers.rs b/library/std/src/sys/pal/uefi/helpers.rs index 97dd90716bbb0..c2419bbb58573 100644 --- a/library/std/src/sys/pal/uefi/helpers.rs +++ b/library/std/src/sys/pal/uefi/helpers.rs @@ -12,10 +12,10 @@ use r_efi::efi::{self, Guid}; use r_efi::protocols::{device_path, device_path_to_text}; -use crate::ffi::{OsString, OsStr}; +use crate::ffi::{OsStr, OsString}; use crate::io::{self, const_io_error}; use crate::mem::{size_of, MaybeUninit}; -use crate::os::uefi::{self, env::boot_services, ffi::OsStringExt}; +use crate::os::uefi::{self, env::boot_services, ffi::OsStrExt, ffi::OsStringExt}; use crate::ptr::NonNull; use crate::slice; use crate::sync::atomic::{AtomicPtr, Ordering}; @@ -291,3 +291,75 @@ impl Drop for DevicePath { } } } + +pub(crate) struct Protocol { + guid: r_efi::efi::Guid, + handle: NonNull, + protocol: Box, +} + +impl Protocol { + const fn new( + guid: r_efi::efi::Guid, + handle: NonNull, + protocol: Box, + ) -> Self { + Self { guid, handle, protocol } + } + + pub(crate) fn create(protocol: T, mut guid: r_efi::efi::Guid) -> io::Result { + let boot_services: NonNull = + boot_services().ok_or(BOOT_SERVICES_UNAVAILABLE)?.cast(); + let mut protocol = Box::new(protocol); + let mut handle: r_efi::efi::Handle = crate::ptr::null_mut(); + + let r = unsafe { + ((*boot_services.as_ptr()).install_protocol_interface)( + &mut handle, + &mut guid, + r_efi::efi::NATIVE_INTERFACE, + protocol.as_mut() as *mut T as *mut crate::ffi::c_void, + ) + }; + + if r.is_error() { + return Err(crate::io::Error::from_raw_os_error(r.as_usize())); + }; + + let handle = NonNull::new(handle) + .ok_or(io::const_io_error!(io::ErrorKind::Uncategorized, "found null handle"))?; + + Ok(Self::new(guid, handle, protocol)) + } + + pub(crate) fn handle(&self) -> NonNull { + self.handle + } +} + +impl Drop for Protocol { + fn drop(&mut self) { + if let Some(bt) = boot_services() { + let bt: NonNull = bt.cast(); + unsafe { + ((*bt.as_ptr()).uninstall_protocol_interface)( + self.handle.as_ptr(), + &mut self.guid, + self.protocol.as_mut() as *mut T as *mut crate::ffi::c_void, + ) + }; + } + } +} + +impl AsRef for Protocol { + fn as_ref(&self) -> &T { + &self.protocol + } +} + +impl AsMut for Protocol { + fn as_mut(&mut self) -> &mut T { + &mut self.protocol + } +} diff --git a/library/std/src/sys/pal/uefi/process.rs b/library/std/src/sys/pal/uefi/process.rs index e85fc209c0e18..7abacc7c12e96 100644 --- a/library/std/src/sys/pal/uefi/process.rs +++ b/library/std/src/sys/pal/uefi/process.rs @@ -91,9 +91,11 @@ impl Command { } pub fn output(&mut self) -> io::Result<(ExitStatus, Vec, Vec)> { - let cmd = uefi_command_internal::Command::load_image(&self.prog)?; + let mut cmd = uefi_command_internal::Command::load_image(&self.prog)?; + cmd.stdout_init()?; let stat = cmd.start_image()?; - Ok((ExitStatus(stat), Vec::new(), Vec::new())) + let stdout = cmd.stdout()?; + Ok((ExitStatus(stat), stdout, Vec::new())) } } @@ -246,20 +248,30 @@ impl<'a> fmt::Debug for CommandArgs<'a> { } mod uefi_command_internal { + use r_efi::protocols::{loaded_image, simple_text_output}; + use super::super::helpers; - use crate::ffi::OsStr; + use crate::ffi::{OsStr, OsString}; use crate::io::{self, const_io_error}; use crate::mem::MaybeUninit; use crate::os::uefi::env::{boot_services, image_handle}; + use crate::os::uefi::ffi::OsStringExt; use crate::ptr::NonNull; + use crate::slice; + use crate::sys_common::wstr::WStrUnits; pub struct Command { handle: NonNull, + stdout: Option>, + st: Box, } impl Command { - const fn new(handle: NonNull) -> Self { - Self { handle } + const fn new( + handle: NonNull, + st: Box, + ) -> Self { + Self { handle, stdout: None, st } } pub fn load_image(p: &OsStr) -> io::Result { @@ -286,7 +298,17 @@ mod uefi_command_internal { } else { let child_handle = unsafe { child_handle.assume_init() }; let child_handle = NonNull::new(child_handle).unwrap(); - Ok(Self::new(child_handle)) + + let loaded_image: NonNull = + helpers::open_protocol(child_handle, loaded_image::PROTOCOL_GUID).unwrap(); + let mut st: Box = + Box::new(unsafe { crate::ptr::read((*loaded_image.as_ptr()).system_table) }); + + unsafe { + (*loaded_image.as_ptr()).system_table = st.as_mut(); + } + + Ok(Self::new(child_handle, st)) } } @@ -313,6 +335,32 @@ mod uefi_command_internal { Ok(r) } + + pub fn stdout_init(&mut self) -> io::Result<()> { + let mut protocol = + helpers::Protocol::create(PipeProtocol::new(), simple_text_output::PROTOCOL_GUID)?; + + self.st.console_out_handle = protocol.handle().as_ptr(); + self.st.con_out = + protocol.as_mut() as *mut PipeProtocol as *mut simple_text_output::Protocol; + + self.stdout = Some(protocol); + + Ok(()) + } + + pub fn stdout(&self) -> io::Result> { + if let Some(stdout) = &self.stdout { + stdout + .as_ref() + .utf8() + .into_string() + .map_err(|_| const_io_error!(io::ErrorKind::Other, "utf8 conversion failed")) + .map(Into::into) + } else { + Err(const_io_error!(io::ErrorKind::NotFound, "stdout not found")) + } + } } impl Drop for Command { @@ -325,4 +373,134 @@ mod uefi_command_internal { } } } + + #[repr(C)] + struct PipeProtocol { + reset: simple_text_output::ProtocolReset, + output_string: simple_text_output::ProtocolOutputString, + test_string: simple_text_output::ProtocolTestString, + query_mode: simple_text_output::ProtocolQueryMode, + set_mode: simple_text_output::ProtocolSetMode, + set_attribute: simple_text_output::ProtocolSetAttribute, + clear_screen: simple_text_output::ProtocolClearScreen, + set_cursor_position: simple_text_output::ProtocolSetCursorPosition, + enable_cursor: simple_text_output::ProtocolEnableCursor, + mode: *mut simple_text_output::Mode, + _mode: Box, + _buffer: Vec, + } + + impl PipeProtocol { + fn new() -> Self { + let mut mode = Box::new(simple_text_output::Mode { + max_mode: 0, + mode: 0, + attribute: 0, + cursor_column: 0, + cursor_row: 0, + cursor_visible: r_efi::efi::Boolean::FALSE, + }); + Self { + reset: Self::reset, + output_string: Self::output_string, + test_string: Self::test_string, + query_mode: Self::query_mode, + set_mode: Self::set_mode, + set_attribute: Self::set_attribute, + clear_screen: Self::clear_screen, + set_cursor_position: Self::set_cursor_position, + enable_cursor: Self::enable_cursor, + mode: mode.as_mut(), + _mode: mode, + _buffer: Vec::new(), + } + } + + fn utf8(&self) -> OsString { + OsString::from_wide(&self._buffer) + } + + extern "efiapi" fn reset( + proto: *mut simple_text_output::Protocol, + _: r_efi::efi::Boolean, + ) -> r_efi::efi::Status { + let proto: *mut PipeProtocol = proto.cast(); + unsafe { + (*proto)._buffer.clear(); + } + r_efi::efi::Status::SUCCESS + } + + extern "efiapi" fn output_string( + proto: *mut simple_text_output::Protocol, + buf: *mut r_efi::efi::Char16, + ) -> r_efi::efi::Status { + let proto: *mut PipeProtocol = proto.cast(); + let buf_len = unsafe { + if let Some(x) = WStrUnits::new(buf) { + x.count() + } else { + return r_efi::efi::Status::INVALID_PARAMETER; + } + }; + let buf_slice = unsafe { slice::from_raw_parts(buf, buf_len) }; + + unsafe { + (*proto)._buffer.extend_from_slice(buf_slice); + }; + + r_efi::efi::Status::SUCCESS + } + + extern "efiapi" fn test_string( + _: *mut simple_text_output::Protocol, + _: *mut r_efi::efi::Char16, + ) -> r_efi::efi::Status { + r_efi::efi::Status::SUCCESS + } + + extern "efiapi" fn query_mode( + _: *mut simple_text_output::Protocol, + _: usize, + _: *mut usize, + _: *mut usize, + ) -> r_efi::efi::Status { + r_efi::efi::Status::UNSUPPORTED + } + + extern "efiapi" fn set_mode( + _: *mut simple_text_output::Protocol, + _: usize, + ) -> r_efi::efi::Status { + r_efi::efi::Status::UNSUPPORTED + } + + extern "efiapi" fn set_attribute( + _: *mut simple_text_output::Protocol, + _: usize, + ) -> r_efi::efi::Status { + r_efi::efi::Status::UNSUPPORTED + } + + extern "efiapi" fn clear_screen( + _: *mut simple_text_output::Protocol, + ) -> r_efi::efi::Status { + r_efi::efi::Status::UNSUPPORTED + } + + extern "efiapi" fn set_cursor_position( + _: *mut simple_text_output::Protocol, + _: usize, + _: usize, + ) -> r_efi::efi::Status { + r_efi::efi::Status::UNSUPPORTED + } + + extern "efiapi" fn enable_cursor( + _: *mut simple_text_output::Protocol, + _: r_efi::efi::Boolean, + ) -> r_efi::efi::Status { + r_efi::efi::Status::UNSUPPORTED + } + } } From 87d7a07f5057c33df152af06d92a1a9bbae8767d Mon Sep 17 00:00:00 2001 From: Ayush Singh Date: Fri, 29 Mar 2024 17:20:50 +0530 Subject: [PATCH 11/22] uefi: process: Add stderr support Implement stderr support in similar fashion. Signed-off-by: Ayush Singh --- library/std/src/sys/pal/uefi/process.rs | 36 +++++++++++++++++++++++-- 1 file changed, 34 insertions(+), 2 deletions(-) diff --git a/library/std/src/sys/pal/uefi/process.rs b/library/std/src/sys/pal/uefi/process.rs index 7abacc7c12e96..06ce542b0be1f 100644 --- a/library/std/src/sys/pal/uefi/process.rs +++ b/library/std/src/sys/pal/uefi/process.rs @@ -92,10 +92,15 @@ impl Command { pub fn output(&mut self) -> io::Result<(ExitStatus, Vec, Vec)> { let mut cmd = uefi_command_internal::Command::load_image(&self.prog)?; + cmd.stdout_init()?; + cmd.stderr_init()?; + let stat = cmd.start_image()?; let stdout = cmd.stdout()?; - Ok((ExitStatus(stat), stdout, Vec::new())) + let stderr = cmd.stderr()?; + + Ok((ExitStatus(stat), stdout, stderr)) } } @@ -263,6 +268,7 @@ mod uefi_command_internal { pub struct Command { handle: NonNull, stdout: Option>, + stderr: Option>, st: Box, } @@ -271,7 +277,7 @@ mod uefi_command_internal { handle: NonNull, st: Box, ) -> Self { - Self { handle, stdout: None, st } + Self { handle, stdout: None, stderr: None, st } } pub fn load_image(p: &OsStr) -> io::Result { @@ -349,6 +355,19 @@ mod uefi_command_internal { Ok(()) } + pub fn stderr_init(&mut self) -> io::Result<()> { + let mut protocol = + helpers::Protocol::create(PipeProtocol::new(), simple_text_output::PROTOCOL_GUID)?; + + self.st.standard_error_handle = protocol.handle().as_ptr(); + self.st.std_err = + protocol.as_mut() as *mut PipeProtocol as *mut simple_text_output::Protocol; + + self.stderr = Some(protocol); + + Ok(()) + } + pub fn stdout(&self) -> io::Result> { if let Some(stdout) = &self.stdout { stdout @@ -361,6 +380,19 @@ mod uefi_command_internal { Err(const_io_error!(io::ErrorKind::NotFound, "stdout not found")) } } + + pub fn stderr(&self) -> io::Result> { + if let Some(stderr) = &self.stderr { + stderr + .as_ref() + .utf8() + .into_string() + .map_err(|_| const_io_error!(io::ErrorKind::Other, "utf8 conversion failed")) + .map(Into::into) + } else { + Err(const_io_error!(io::ErrorKind::NotFound, "stdout not found")) + } + } } impl Drop for Command { From 725376567ae0e2b75574bbda68d191ebd3a4ce68 Mon Sep 17 00:00:00 2001 From: Ayush Singh Date: Fri, 29 Mar 2024 18:36:52 +0530 Subject: [PATCH 12/22] uefi: process: Add null protocol Signed-off-by: Ayush Singh --- library/std/src/sys/pal/uefi/process.rs | 138 +++++++++++++++++------- 1 file changed, 100 insertions(+), 38 deletions(-) diff --git a/library/std/src/sys/pal/uefi/process.rs b/library/std/src/sys/pal/uefi/process.rs index 06ce542b0be1f..fc96f382650e5 100644 --- a/library/std/src/sys/pal/uefi/process.rs +++ b/library/std/src/sys/pal/uefi/process.rs @@ -1,3 +1,5 @@ +use r_efi::protocols::simple_text_output; + use crate::ffi::OsStr; use crate::ffi::OsString; use crate::fmt; @@ -13,12 +15,16 @@ use crate::sys_common::process::{CommandEnv, CommandEnvs}; pub use crate::ffi::OsString as EnvKey; +use super::helpers; + //////////////////////////////////////////////////////////////////////////////// // Command //////////////////////////////////////////////////////////////////////////////// pub struct Command { prog: OsString, + stdout: Option, + stderr: Option, } // passed back to std::process with the pipes connected to the child, if any @@ -39,7 +45,7 @@ pub enum Stdio { impl Command { pub fn new(program: &OsStr) -> Command { - Command { prog: program.to_os_string() } + Command { prog: program.to_os_string(), stdout: None, stderr: None } } pub fn arg(&mut self, _arg: &OsStr) { @@ -58,12 +64,20 @@ impl Command { panic!("unsupported") } - pub fn stdout(&mut self, _stdout: Stdio) { - panic!("unsupported") + pub fn stdout(&mut self, stdout: Stdio) { + self.stdout = match stdout { + Stdio::MakePipe => Some(uefi_command_internal::PipeProtocol::new()), + Stdio::Null => Some(uefi_command_internal::PipeProtocol::null()), + _ => None, + }; } - pub fn stderr(&mut self, _stderr: Stdio) { - panic!("unsupported") + pub fn stderr(&mut self, stderr: Stdio) { + self.stderr = match stderr { + Stdio::MakePipe => Some(uefi_command_internal::PipeProtocol::new()), + Stdio::Null => Some(uefi_command_internal::PipeProtocol::null()), + _ => None, + }; } pub fn get_program(&self) -> &OsStr { @@ -93,8 +107,26 @@ impl Command { pub fn output(&mut self) -> io::Result<(ExitStatus, Vec, Vec)> { let mut cmd = uefi_command_internal::Command::load_image(&self.prog)?; - cmd.stdout_init()?; - cmd.stderr_init()?; + let stdout: helpers::Protocol = + match self.stdout.take() { + Some(s) => helpers::Protocol::create(s, simple_text_output::PROTOCOL_GUID), + None => helpers::Protocol::create( + uefi_command_internal::PipeProtocol::new(), + simple_text_output::PROTOCOL_GUID, + ), + }?; + + let stderr: helpers::Protocol = + match self.stderr.take() { + Some(s) => helpers::Protocol::create(s, simple_text_output::PROTOCOL_GUID), + None => helpers::Protocol::create( + uefi_command_internal::PipeProtocol::new(), + simple_text_output::PROTOCOL_GUID, + ), + }?; + + cmd.stdout_init(stdout)?; + cmd.stderr_init(stderr)?; let stat = cmd.start_image()?; let stdout = cmd.stdout()?; @@ -342,10 +374,10 @@ mod uefi_command_internal { Ok(r) } - pub fn stdout_init(&mut self) -> io::Result<()> { - let mut protocol = - helpers::Protocol::create(PipeProtocol::new(), simple_text_output::PROTOCOL_GUID)?; - + pub fn stdout_init( + &mut self, + mut protocol: helpers::Protocol, + ) -> io::Result<()> { self.st.console_out_handle = protocol.handle().as_ptr(); self.st.con_out = protocol.as_mut() as *mut PipeProtocol as *mut simple_text_output::Protocol; @@ -355,10 +387,10 @@ mod uefi_command_internal { Ok(()) } - pub fn stderr_init(&mut self) -> io::Result<()> { - let mut protocol = - helpers::Protocol::create(PipeProtocol::new(), simple_text_output::PROTOCOL_GUID)?; - + pub fn stderr_init( + &mut self, + mut protocol: helpers::Protocol, + ) -> io::Result<()> { self.st.standard_error_handle = protocol.handle().as_ptr(); self.st.std_err = protocol.as_mut() as *mut PipeProtocol as *mut simple_text_output::Protocol; @@ -368,29 +400,17 @@ mod uefi_command_internal { Ok(()) } - pub fn stdout(&self) -> io::Result> { - if let Some(stdout) = &self.stdout { - stdout - .as_ref() - .utf8() - .into_string() - .map_err(|_| const_io_error!(io::ErrorKind::Other, "utf8 conversion failed")) - .map(Into::into) - } else { - Err(const_io_error!(io::ErrorKind::NotFound, "stdout not found")) + pub fn stderr(&self) -> io::Result> { + match &self.stderr { + Some(stderr) => stderr.as_ref().utf8(), + None => Ok(Vec::new()), } } - pub fn stderr(&self) -> io::Result> { - if let Some(stderr) = &self.stderr { - stderr - .as_ref() - .utf8() - .into_string() - .map_err(|_| const_io_error!(io::ErrorKind::Other, "utf8 conversion failed")) - .map(Into::into) - } else { - Err(const_io_error!(io::ErrorKind::NotFound, "stdout not found")) + pub fn stdout(&self) -> io::Result> { + match &self.stdout { + Some(stdout) => stdout.as_ref().utf8(), + None => Ok(Vec::new()), } } } @@ -407,7 +427,7 @@ mod uefi_command_internal { } #[repr(C)] - struct PipeProtocol { + pub struct PipeProtocol { reset: simple_text_output::ProtocolReset, output_string: simple_text_output::ProtocolOutputString, test_string: simple_text_output::ProtocolTestString, @@ -423,7 +443,7 @@ mod uefi_command_internal { } impl PipeProtocol { - fn new() -> Self { + pub fn new() -> Self { let mut mode = Box::new(simple_text_output::Mode { max_mode: 0, mode: 0, @@ -448,8 +468,36 @@ mod uefi_command_internal { } } - fn utf8(&self) -> OsString { + pub fn null() -> Self { + let mut mode = Box::new(simple_text_output::Mode { + max_mode: 0, + mode: 0, + attribute: 0, + cursor_column: 0, + cursor_row: 0, + cursor_visible: r_efi::efi::Boolean::FALSE, + }); + Self { + reset: Self::reset_null, + output_string: Self::output_string_null, + test_string: Self::test_string, + query_mode: Self::query_mode, + set_mode: Self::set_mode, + set_attribute: Self::set_attribute, + clear_screen: Self::clear_screen, + set_cursor_position: Self::set_cursor_position, + enable_cursor: Self::enable_cursor, + mode: mode.as_mut(), + _mode: mode, + _buffer: Vec::new(), + } + } + + pub fn utf8(&self) -> io::Result> { OsString::from_wide(&self._buffer) + .into_string() + .map(Into::into) + .map_err(|_| const_io_error!(io::ErrorKind::Other, "utf8 conversion failed")) } extern "efiapi" fn reset( @@ -463,6 +511,13 @@ mod uefi_command_internal { r_efi::efi::Status::SUCCESS } + extern "efiapi" fn reset_null( + _: *mut simple_text_output::Protocol, + _: r_efi::efi::Boolean, + ) -> r_efi::efi::Status { + r_efi::efi::Status::SUCCESS + } + extern "efiapi" fn output_string( proto: *mut simple_text_output::Protocol, buf: *mut r_efi::efi::Char16, @@ -484,6 +539,13 @@ mod uefi_command_internal { r_efi::efi::Status::SUCCESS } + extern "efiapi" fn output_string_null( + _: *mut simple_text_output::Protocol, + _: *mut r_efi::efi::Char16, + ) -> r_efi::efi::Status { + r_efi::efi::Status::SUCCESS + } + extern "efiapi" fn test_string( _: *mut simple_text_output::Protocol, _: *mut r_efi::efi::Char16, From d44b3fb120a05d4cd9d663b434bea56e65ef2df2 Mon Sep 17 00:00:00 2001 From: Ayush Singh Date: Fri, 29 Mar 2024 19:07:46 +0530 Subject: [PATCH 13/22] uefi: process Implement inherit Only tested in 2 levels right now. Need args support for 3 levels Signed-off-by: Ayush Singh --- library/std/src/sys/pal/uefi/process.rs | 57 ++++++++++++++++--------- 1 file changed, 37 insertions(+), 20 deletions(-) diff --git a/library/std/src/sys/pal/uefi/process.rs b/library/std/src/sys/pal/uefi/process.rs index fc96f382650e5..ed25ad81bbdc9 100644 --- a/library/std/src/sys/pal/uefi/process.rs +++ b/library/std/src/sys/pal/uefi/process.rs @@ -23,8 +23,8 @@ use super::helpers; pub struct Command { prog: OsString, - stdout: Option, - stderr: Option, + stdout: Option, + stderr: Option, } // passed back to std::process with the pipes connected to the child, if any @@ -65,19 +65,11 @@ impl Command { } pub fn stdout(&mut self, stdout: Stdio) { - self.stdout = match stdout { - Stdio::MakePipe => Some(uefi_command_internal::PipeProtocol::new()), - Stdio::Null => Some(uefi_command_internal::PipeProtocol::null()), - _ => None, - }; + self.stdout = Some(stdout); } pub fn stderr(&mut self, stderr: Stdio) { - self.stderr = match stderr { - Stdio::MakePipe => Some(uefi_command_internal::PipeProtocol::new()), - Stdio::Null => Some(uefi_command_internal::PipeProtocol::null()), - _ => None, - }; + self.stderr = Some(stderr); } pub fn get_program(&self) -> &OsStr { @@ -104,31 +96,56 @@ impl Command { unsupported() } + fn create_pipe( + s: Stdio, + ) -> io::Result>> { + match s { + Stdio::MakePipe => helpers::Protocol::create( + uefi_command_internal::PipeProtocol::new(), + simple_text_output::PROTOCOL_GUID, + ) + .map(Some), + Stdio::Null => helpers::Protocol::create( + uefi_command_internal::PipeProtocol::null(), + simple_text_output::PROTOCOL_GUID, + ) + .map(Some), + Stdio::Inherit => Ok(None), + } + } + pub fn output(&mut self) -> io::Result<(ExitStatus, Vec, Vec)> { let mut cmd = uefi_command_internal::Command::load_image(&self.prog)?; - let stdout: helpers::Protocol = + let stdout: Option> = match self.stdout.take() { - Some(s) => helpers::Protocol::create(s, simple_text_output::PROTOCOL_GUID), + Some(s) => Self::create_pipe(s), None => helpers::Protocol::create( uefi_command_internal::PipeProtocol::new(), simple_text_output::PROTOCOL_GUID, - ), + ) + .map(Some), }?; - let stderr: helpers::Protocol = + let stderr: Option> = match self.stderr.take() { - Some(s) => helpers::Protocol::create(s, simple_text_output::PROTOCOL_GUID), + Some(s) => Self::create_pipe(s), None => helpers::Protocol::create( uefi_command_internal::PipeProtocol::new(), simple_text_output::PROTOCOL_GUID, - ), + ) + .map(Some), }?; - cmd.stdout_init(stdout)?; - cmd.stderr_init(stderr)?; + if let Some(stdout) = stdout { + cmd.stdout_init(stdout)?; + } + if let Some(stderr) = stderr { + cmd.stderr_init(stderr)?; + } let stat = cmd.start_image()?; + let stdout = cmd.stdout()?; let stderr = cmd.stderr()?; From 29c198c85f1b1268e6a60664f79a87d8c362e6fb Mon Sep 17 00:00:00 2001 From: Ayush Singh Date: Fri, 29 Mar 2024 19:44:37 +0530 Subject: [PATCH 14/22] uefi: process: Add support for args Also fix stdio inherit Signed-off-by: Ayush Singh --- library/std/src/sys/pal/uefi/process.rs | 76 ++++++++++++++++++------- 1 file changed, 55 insertions(+), 21 deletions(-) diff --git a/library/std/src/sys/pal/uefi/process.rs b/library/std/src/sys/pal/uefi/process.rs index ed25ad81bbdc9..6b431553f8aa8 100644 --- a/library/std/src/sys/pal/uefi/process.rs +++ b/library/std/src/sys/pal/uefi/process.rs @@ -23,6 +23,7 @@ use super::helpers; pub struct Command { prog: OsString, + args: OsString, stdout: Option, stderr: Option, } @@ -45,11 +46,17 @@ pub enum Stdio { impl Command { pub fn new(program: &OsStr) -> Command { - Command { prog: program.to_os_string(), stdout: None, stderr: None } + Command { + prog: program.to_os_string(), + args: program.to_os_string(), + stdout: None, + stderr: None, + } } - pub fn arg(&mut self, _arg: &OsStr) { - panic!("unsupported") + pub fn arg(&mut self, arg: &OsStr) { + self.args.push(" "); + self.args.push(arg); } pub fn env_mut(&mut self) -> &mut CommandEnv { @@ -137,11 +144,17 @@ impl Command { .map(Some), }?; - if let Some(stdout) = stdout { - cmd.stdout_init(stdout)?; - } - if let Some(stderr) = stderr { - cmd.stderr_init(stderr)?; + match stdout { + Some(stdout) => cmd.stdout_init(stdout), + None => cmd.stdout_inherit(), + }; + match stderr { + Some(stderr) => cmd.stderr_init(stderr), + None => cmd.stderr_inherit(), + }; + + if !self.args.is_empty() { + cmd.set_args(&self.args); } let stat = cmd.start_image()?; @@ -308,8 +321,8 @@ mod uefi_command_internal { use crate::ffi::{OsStr, OsString}; use crate::io::{self, const_io_error}; use crate::mem::MaybeUninit; - use crate::os::uefi::env::{boot_services, image_handle}; - use crate::os::uefi::ffi::OsStringExt; + use crate::os::uefi::env::{boot_services, image_handle, system_table}; + use crate::os::uefi::ffi::{OsStrExt, OsStringExt}; use crate::ptr::NonNull; use crate::slice; use crate::sys_common::wstr::WStrUnits; @@ -319,6 +332,7 @@ mod uefi_command_internal { stdout: Option>, stderr: Option>, st: Box, + args: Option>, } impl Command { @@ -326,7 +340,7 @@ mod uefi_command_internal { handle: NonNull, st: Box, ) -> Self { - Self { handle, stdout: None, stderr: None, st } + Self { handle, stdout: None, stderr: None, st, args: None } } pub fn load_image(p: &OsStr) -> io::Result { @@ -391,30 +405,34 @@ mod uefi_command_internal { Ok(r) } - pub fn stdout_init( - &mut self, - mut protocol: helpers::Protocol, - ) -> io::Result<()> { + pub fn stdout_init(&mut self, mut protocol: helpers::Protocol) { self.st.console_out_handle = protocol.handle().as_ptr(); self.st.con_out = protocol.as_mut() as *mut PipeProtocol as *mut simple_text_output::Protocol; self.stdout = Some(protocol); + } - Ok(()) + pub fn stdout_inherit(&mut self) { + let st: NonNull = system_table().cast(); + + self.st.console_out_handle = unsafe { (*st.as_ptr()).console_out_handle }; + self.st.con_out = unsafe { (*st.as_ptr()).con_out }; } - pub fn stderr_init( - &mut self, - mut protocol: helpers::Protocol, - ) -> io::Result<()> { + pub fn stderr_init(&mut self, mut protocol: helpers::Protocol) { self.st.standard_error_handle = protocol.handle().as_ptr(); self.st.std_err = protocol.as_mut() as *mut PipeProtocol as *mut simple_text_output::Protocol; self.stderr = Some(protocol); + } + + pub fn stderr_inherit(&mut self) { + let st: NonNull = system_table().cast(); - Ok(()) + self.st.standard_error_handle = unsafe { (*st.as_ptr()).standard_error_handle }; + self.st.std_err = unsafe { (*st.as_ptr()).std_err }; } pub fn stderr(&self) -> io::Result> { @@ -430,6 +448,22 @@ mod uefi_command_internal { None => Ok(Vec::new()), } } + + pub fn set_args(&mut self, args: &OsStr) { + let loaded_image: NonNull = + helpers::open_protocol(self.handle, loaded_image::PROTOCOL_GUID).unwrap(); + + let mut args = args.encode_wide().collect::>(); + let args_size = (crate::mem::size_of::() * args.len()) as u32; + + unsafe { + (*loaded_image.as_ptr()).load_options = + args.as_mut_ptr() as *mut crate::ffi::c_void; + (*loaded_image.as_ptr()).load_options_size = args_size; + } + + self.args = Some(args); + } } impl Drop for Command { From c899e05457c95832bce6e997714f0c608f8cd572 Mon Sep 17 00:00:00 2001 From: Ayush Singh Date: Fri, 29 Mar 2024 20:31:56 +0530 Subject: [PATCH 15/22] uefi: process: Add CommandArgs support Signed-off-by: Ayush Singh --- library/std/src/sys/pal/uefi/process.rs | 40 ++++++++++++++++--------- 1 file changed, 26 insertions(+), 14 deletions(-) diff --git a/library/std/src/sys/pal/uefi/process.rs b/library/std/src/sys/pal/uefi/process.rs index 6b431553f8aa8..2a32341bf6372 100644 --- a/library/std/src/sys/pal/uefi/process.rs +++ b/library/std/src/sys/pal/uefi/process.rs @@ -4,7 +4,6 @@ use crate::ffi::OsStr; use crate::ffi::OsString; use crate::fmt; use crate::io; -use crate::marker::PhantomData; use crate::num::NonZero; use crate::num::NonZeroI32; use crate::path::Path; @@ -23,7 +22,7 @@ use super::helpers; pub struct Command { prog: OsString, - args: OsString, + args: Vec, stdout: Option, stderr: Option, } @@ -48,15 +47,14 @@ impl Command { pub fn new(program: &OsStr) -> Command { Command { prog: program.to_os_string(), - args: program.to_os_string(), + args: Vec::from([program.to_os_string()]), stdout: None, stderr: None, } } pub fn arg(&mut self, arg: &OsStr) { - self.args.push(" "); - self.args.push(arg); + self.args.push(arg.to_os_string()); } pub fn env_mut(&mut self) -> &mut CommandEnv { @@ -80,11 +78,11 @@ impl Command { } pub fn get_program(&self) -> &OsStr { - panic!("unsupported") + self.prog.as_ref() } pub fn get_args(&self) -> CommandArgs<'_> { - panic!("unsupported") + CommandArgs { iter: self.args.iter() } } pub fn get_envs(&self) -> CommandEnvs<'_> { @@ -153,8 +151,15 @@ impl Command { None => cmd.stderr_inherit(), }; - if !self.args.is_empty() { - cmd.set_args(&self.args); + if self.args.len() > 1 { + let args = self.args.iter().fold(OsString::new(), |mut acc, arg| { + if !acc.is_empty() { + acc.push(" "); + } + acc.push(arg); + acc + }); + cmd.set_args(&args); } let stat = cmd.start_image()?; @@ -293,24 +298,31 @@ impl Process { } pub struct CommandArgs<'a> { - _p: PhantomData<&'a ()>, + iter: crate::slice::Iter<'a, OsString>, } impl<'a> Iterator for CommandArgs<'a> { type Item = &'a OsStr; fn next(&mut self) -> Option<&'a OsStr> { - None + self.iter.next().map(|x| x.as_ref()) } fn size_hint(&self) -> (usize, Option) { - (0, Some(0)) + self.iter.size_hint() } } -impl<'a> ExactSizeIterator for CommandArgs<'a> {} +impl<'a> ExactSizeIterator for CommandArgs<'a> { + fn len(&self) -> usize { + self.iter.len() + } + fn is_empty(&self) -> bool { + self.iter.is_empty() + } +} impl<'a> fmt::Debug for CommandArgs<'a> { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - f.debug_list().finish() + f.debug_list().entries(self.iter.clone()).finish() } } From 56e2a575053d314a9b43ac9a9ae7b22c4842145a Mon Sep 17 00:00:00 2001 From: Ayush Singh Date: Fri, 29 Mar 2024 20:51:48 +0530 Subject: [PATCH 16/22] uefi: process: Final Touchups Signed-off-by: Ayush Singh --- library/std/src/sys/pal/uefi/process.rs | 38 ++++++++++++------------- 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/library/std/src/sys/pal/uefi/process.rs b/library/std/src/sys/pal/uefi/process.rs index 2a32341bf6372..7075af186f9bd 100644 --- a/library/std/src/sys/pal/uefi/process.rs +++ b/library/std/src/sys/pal/uefi/process.rs @@ -35,8 +35,6 @@ pub struct StdioPipes { pub stderr: Option, } -// FIXME: This should be a unit struct, so we can always construct it -// The value here should be never used, since we cannot spawn processes. pub enum Stdio { Inherit, Null, @@ -45,12 +43,7 @@ pub enum Stdio { impl Command { pub fn new(program: &OsStr) -> Command { - Command { - prog: program.to_os_string(), - args: Vec::from([program.to_os_string()]), - stdout: None, - stderr: None, - } + Command { prog: program.to_os_string(), args: Vec::new(), stdout: None, stderr: None } } pub fn arg(&mut self, arg: &OsStr) { @@ -122,6 +115,7 @@ impl Command { pub fn output(&mut self) -> io::Result<(ExitStatus, Vec, Vec)> { let mut cmd = uefi_command_internal::Command::load_image(&self.prog)?; + /* Setup Stdout */ let stdout: Option> = match self.stdout.take() { Some(s) => Self::create_pipe(s), @@ -131,7 +125,12 @@ impl Command { ) .map(Some), }?; + match stdout { + Some(stdout) => cmd.stdout_init(stdout), + None => cmd.stdout_inherit(), + }; + /* Setup Stderr */ let stderr: Option> = match self.stderr.take() { Some(s) => Self::create_pipe(s), @@ -141,21 +140,15 @@ impl Command { ) .map(Some), }?; - - match stdout { - Some(stdout) => cmd.stdout_init(stdout), - None => cmd.stdout_inherit(), - }; match stderr { Some(stderr) => cmd.stderr_init(stderr), None => cmd.stderr_inherit(), }; - if self.args.len() > 1 { - let args = self.args.iter().fold(OsString::new(), |mut acc, arg| { - if !acc.is_empty() { - acc.push(" "); - } + /* No reason to set args if only program name is preset */ + if !self.args.is_empty() { + let args = self.args.iter().fold(OsString::from(&self.prog), |mut acc, arg| { + acc.push(" "); acc.push(arg); acc }); @@ -202,7 +195,11 @@ impl From for Stdio { } impl fmt::Debug for Command { - fn fmt(&self, _f: &mut fmt::Formatter<'_>) -> fmt::Result { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + self.prog.fmt(f)?; + for arg in &self.args { + arg.fmt(f)?; + } Ok(()) } } @@ -303,9 +300,11 @@ pub struct CommandArgs<'a> { impl<'a> Iterator for CommandArgs<'a> { type Item = &'a OsStr; + fn next(&mut self) -> Option<&'a OsStr> { self.iter.next().map(|x| x.as_ref()) } + fn size_hint(&self) -> (usize, Option) { self.iter.size_hint() } @@ -315,6 +314,7 @@ impl<'a> ExactSizeIterator for CommandArgs<'a> { fn len(&self) -> usize { self.iter.len() } + fn is_empty(&self) -> bool { self.iter.is_empty() } From e2903989da89b43722efca49bcc21749014739ff Mon Sep 17 00:00:00 2001 From: Ayush Singh Date: Sun, 31 Mar 2024 01:06:33 +0530 Subject: [PATCH 17/22] uefi: process: Fixes from PR - Update system table crc32 - Fix unsound use of Box - Free exit data - Code improvements - Introduce OwnedTable - Update r-efi to latest version - Use extended_varargs_abi_support for install_multiple_protocol_interfaces and uninstall_multiple_protocol_interfaces - Fix comments - Stub out args implementation Signed-off-by: Ayush Singh --- library/std/Cargo.toml | 2 +- library/std/src/lib.rs | 1 + library/std/src/sys/pal/uefi/helpers.rs | 131 +++++++++---- library/std/src/sys/pal/uefi/process.rs | 250 +++++++++++++----------- 4 files changed, 232 insertions(+), 152 deletions(-) diff --git a/library/std/Cargo.toml b/library/std/Cargo.toml index b991b1cf22dd8..3db2f1138605d 100644 --- a/library/std/Cargo.toml +++ b/library/std/Cargo.toml @@ -56,7 +56,7 @@ hermit-abi = { version = "0.4.0", features = ['rustc-dep-of-std'], public = true wasi = { version = "0.11.0", features = ['rustc-dep-of-std'], default-features = false } [target.'cfg(target_os = "uefi")'.dependencies] -r-efi = { version = "4.2.0", features = ['rustc-dep-of-std'] } +r-efi = { version = "4.5.0", features = ['rustc-dep-of-std'] } r-efi-alloc = { version = "1.0.0", features = ['rustc-dep-of-std'] } [features] diff --git a/library/std/src/lib.rs b/library/std/src/lib.rs index f0a73a308a4a4..eb30a3355e5b6 100644 --- a/library/std/src/lib.rs +++ b/library/std/src/lib.rs @@ -293,6 +293,7 @@ #![feature(doc_masked)] #![feature(doc_notable_trait)] #![feature(dropck_eyepatch)] +#![feature(extended_varargs_abi_support)] #![feature(f128)] #![feature(f16)] #![feature(if_let_guard)] diff --git a/library/std/src/sys/pal/uefi/helpers.rs b/library/std/src/sys/pal/uefi/helpers.rs index c2419bbb58573..29984a915b894 100644 --- a/library/std/src/sys/pal/uefi/helpers.rs +++ b/library/std/src/sys/pal/uefi/helpers.rs @@ -21,6 +21,12 @@ use crate::slice; use crate::sync::atomic::{AtomicPtr, Ordering}; use crate::sys_common::wstr::WStrUnits; +type BootInstallMultipleProtocolInterfaces = + unsafe extern "efiapi" fn(_: *mut r_efi::efi::Handle, _: ...) -> r_efi::efi::Status; + +type BootUninstallMultipleProtocolInterfaces = + unsafe extern "efiapi" fn(_: r_efi::efi::Handle, _: ...) -> r_efi::efi::Status; + const BOOT_SERVICES_UNAVAILABLE: io::Error = const_io_error!(io::ErrorKind::Other, "Boot Services are no longer available"); @@ -231,6 +237,13 @@ impl DevicePath { protocol: NonNull, ) -> io::Result { let path_vec = p.encode_wide().chain(Some(0)).collect::>(); + if path_vec[..path_vec.len() - 1].contains(&0) { + return Err(const_io_error!( + io::ErrorKind::InvalidInput, + "strings passed to UEFI cannot contain NULs", + )); + } + let path = unsafe { ((*protocol.as_ptr()).convert_text_to_device_path)(path_vec.as_ptr()) }; @@ -267,17 +280,9 @@ impl DevicePath { "DevicePathFromText Protocol not found" )) } -} - -impl AsRef for DevicePath { - fn as_ref(&self) -> &r_efi::protocols::device_path::Protocol { - unsafe { self.0.as_ref() } - } -} -impl AsMut for DevicePath { - fn as_mut(&mut self) -> &mut r_efi::protocols::device_path::Protocol { - unsafe { self.0.as_mut() } + pub(crate) fn as_ptr(&self) -> *mut r_efi::protocols::device_path::Protocol { + self.0.as_ptr() } } @@ -292,44 +297,42 @@ impl Drop for DevicePath { } } -pub(crate) struct Protocol { +pub(crate) struct OwnedProtocol { guid: r_efi::efi::Guid, handle: NonNull, - protocol: Box, + protocol: *mut T, } -impl Protocol { - const fn new( - guid: r_efi::efi::Guid, - handle: NonNull, - protocol: Box, - ) -> Self { - Self { guid, handle, protocol } - } - - pub(crate) fn create(protocol: T, mut guid: r_efi::efi::Guid) -> io::Result { - let boot_services: NonNull = +impl OwnedProtocol { + // FIXME: Consider using unsafe trait for matching protocol with guid + pub(crate) unsafe fn create(protocol: T, mut guid: r_efi::efi::Guid) -> io::Result { + let bt: NonNull = boot_services().ok_or(BOOT_SERVICES_UNAVAILABLE)?.cast(); - let mut protocol = Box::new(protocol); + let protocol: *mut T = Box::into_raw(Box::new(protocol)); let mut handle: r_efi::efi::Handle = crate::ptr::null_mut(); + // FIXME: Move into r-efi once extended_varargs_abi_support is stablized + let func: BootInstallMultipleProtocolInterfaces = + unsafe { crate::mem::transmute((*bt.as_ptr()).install_multiple_protocol_interfaces) }; + let r = unsafe { - ((*boot_services.as_ptr()).install_protocol_interface)( + func( &mut handle, - &mut guid, - r_efi::efi::NATIVE_INTERFACE, - protocol.as_mut() as *mut T as *mut crate::ffi::c_void, + &mut guid as *mut _ as *mut crate::ffi::c_void, + protocol as *mut crate::ffi::c_void, + crate::ptr::null_mut() as *mut crate::ffi::c_void, ) }; if r.is_error() { + drop(unsafe { Box::from_raw(protocol) }); return Err(crate::io::Error::from_raw_os_error(r.as_usize())); }; let handle = NonNull::new(handle) .ok_or(io::const_io_error!(io::ErrorKind::Uncategorized, "found null handle"))?; - Ok(Self::new(guid, handle, protocol)) + Ok(Self { guid, handle, protocol }) } pub(crate) fn handle(&self) -> NonNull { @@ -337,29 +340,79 @@ impl Protocol { } } -impl Drop for Protocol { +impl Drop for OwnedProtocol { fn drop(&mut self) { + // Do not deallocate a runtime protocol if let Some(bt) = boot_services() { let bt: NonNull = bt.cast(); - unsafe { - ((*bt.as_ptr()).uninstall_protocol_interface)( + // FIXME: Move into r-efi once extended_varargs_abi_support is stablized + let func: BootUninstallMultipleProtocolInterfaces = unsafe { + crate::mem::transmute((*bt.as_ptr()).uninstall_multiple_protocol_interfaces) + }; + let status = unsafe { + func( self.handle.as_ptr(), - &mut self.guid, - self.protocol.as_mut() as *mut T as *mut crate::ffi::c_void, + &mut self.guid as *mut _ as *mut crate::ffi::c_void, + self.protocol as *mut crate::ffi::c_void, + crate::ptr::null_mut() as *mut crate::ffi::c_void, ) }; + + // Leak the protocol in case uninstall fails + if status == r_efi::efi::Status::SUCCESS { + let _ = unsafe { Box::from_raw(self.protocol) }; + } } } } -impl AsRef for Protocol { +impl AsRef for OwnedProtocol { fn as_ref(&self) -> &T { - &self.protocol + unsafe { self.protocol.as_ref().unwrap() } + } +} + +pub(crate) struct OwnedTable { + layout: crate::alloc::Layout, + ptr: *mut T, +} + +impl OwnedTable { + pub(crate) fn from_table_header(hdr: &r_efi::efi::TableHeader) -> Self { + let header_size = hdr.header_size as usize; + let layout = crate::alloc::Layout::from_size_align(header_size, 8).unwrap(); + let ptr = unsafe { crate::alloc::alloc(layout) as *mut T }; + Self { layout, ptr } + } + + pub(crate) const fn as_ptr(&self) -> *const T { + self.ptr + } + + pub(crate) const fn as_mut_ptr(&self) -> *mut T { + self.ptr } } -impl AsMut for Protocol { - fn as_mut(&mut self) -> &mut T { - &mut self.protocol +impl OwnedTable { + pub(crate) fn from_table(tbl: *const r_efi::efi::SystemTable) -> Self { + let hdr = unsafe { (*tbl).hdr }; + + let owned_tbl = Self::from_table_header(&hdr); + unsafe { + crate::ptr::copy_nonoverlapping( + tbl as *const u8, + owned_tbl.as_mut_ptr() as *mut u8, + hdr.header_size as usize, + ) + }; + + owned_tbl + } +} + +impl Drop for OwnedTable { + fn drop(&mut self) { + unsafe { crate::alloc::dealloc(self.ptr as *mut u8, self.layout) }; } } diff --git a/library/std/src/sys/pal/uefi/process.rs b/library/std/src/sys/pal/uefi/process.rs index 7075af186f9bd..5c7c8415ee295 100644 --- a/library/std/src/sys/pal/uefi/process.rs +++ b/library/std/src/sys/pal/uefi/process.rs @@ -20,9 +20,9 @@ use super::helpers; // Command //////////////////////////////////////////////////////////////////////////////// +#[derive(Debug)] pub struct Command { prog: OsString, - args: Vec, stdout: Option, stderr: Option, } @@ -35,6 +35,7 @@ pub struct StdioPipes { pub stderr: Option, } +#[derive(Copy, Clone, Debug)] pub enum Stdio { Inherit, Null, @@ -43,11 +44,12 @@ pub enum Stdio { impl Command { pub fn new(program: &OsStr) -> Command { - Command { prog: program.to_os_string(), args: Vec::new(), stdout: None, stderr: None } + Command { prog: program.to_os_string(), stdout: None, stderr: None } } - pub fn arg(&mut self, arg: &OsStr) { - self.args.push(arg.to_os_string()); + // FIXME: Implement arguments as reverse of parsing algorithm + pub fn arg(&mut self, _arg: &OsStr) { + panic!("unsupported") } pub fn env_mut(&mut self) -> &mut CommandEnv { @@ -75,7 +77,7 @@ impl Command { } pub fn get_args(&self) -> CommandArgs<'_> { - CommandArgs { iter: self.args.iter() } + panic!("unsupported") } pub fn get_envs(&self) -> CommandEnvs<'_> { @@ -96,65 +98,47 @@ impl Command { fn create_pipe( s: Stdio, - ) -> io::Result>> { + ) -> io::Result>> { match s { - Stdio::MakePipe => helpers::Protocol::create( - uefi_command_internal::PipeProtocol::new(), - simple_text_output::PROTOCOL_GUID, - ) + Stdio::MakePipe => unsafe { + helpers::OwnedProtocol::create( + uefi_command_internal::PipeProtocol::new(), + simple_text_output::PROTOCOL_GUID, + ) + } .map(Some), - Stdio::Null => helpers::Protocol::create( - uefi_command_internal::PipeProtocol::null(), - simple_text_output::PROTOCOL_GUID, - ) + Stdio::Null => unsafe { + helpers::OwnedProtocol::create( + uefi_command_internal::PipeProtocol::null(), + simple_text_output::PROTOCOL_GUID, + ) + } .map(Some), Stdio::Inherit => Ok(None), } } pub fn output(&mut self) -> io::Result<(ExitStatus, Vec, Vec)> { - let mut cmd = uefi_command_internal::Command::load_image(&self.prog)?; - - /* Setup Stdout */ - let stdout: Option> = - match self.stdout.take() { - Some(s) => Self::create_pipe(s), - None => helpers::Protocol::create( - uefi_command_internal::PipeProtocol::new(), - simple_text_output::PROTOCOL_GUID, - ) - .map(Some), - }?; - match stdout { - Some(stdout) => cmd.stdout_init(stdout), - None => cmd.stdout_inherit(), + let mut cmd = uefi_command_internal::Image::load_image(&self.prog)?; + + // Setup Stdout + let stdout = self.stdout.unwrap_or(Stdio::MakePipe); + let stdout = Self::create_pipe(stdout)?; + if let Some(con) = stdout { + cmd.stdout_init(con) + } else { + cmd.stdout_inherit() }; - /* Setup Stderr */ - let stderr: Option> = - match self.stderr.take() { - Some(s) => Self::create_pipe(s), - None => helpers::Protocol::create( - uefi_command_internal::PipeProtocol::new(), - simple_text_output::PROTOCOL_GUID, - ) - .map(Some), - }?; - match stderr { - Some(stderr) => cmd.stderr_init(stderr), - None => cmd.stderr_inherit(), + // Setup Stderr + let stderr = self.stderr.unwrap_or(Stdio::MakePipe); + let stderr = Self::create_pipe(stderr)?; + if let Some(con) = stderr { + cmd.stderr_init(con) + } else { + cmd.stderr_inherit() }; - /* No reason to set args if only program name is preset */ - if !self.args.is_empty() { - let args = self.args.iter().fold(OsString::from(&self.prog), |mut acc, arg| { - acc.push(" "); - acc.push(arg); - acc - }); - cmd.set_args(&args); - } - let stat = cmd.start_image()?; let stdout = cmd.stdout()?; @@ -194,16 +178,6 @@ impl From for Stdio { } } -impl fmt::Debug for Command { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - self.prog.fmt(f)?; - for arg in &self.args { - arg.fmt(f)?; - } - Ok(()) - } -} - #[derive(PartialEq, Eq, Clone, Copy, Debug)] #[non_exhaustive] pub struct ExitStatus(r_efi::efi::Status); @@ -326,6 +300,7 @@ impl<'a> fmt::Debug for CommandArgs<'a> { } } +#[allow(dead_code)] mod uefi_command_internal { use r_efi::protocols::{loaded_image, simple_text_output}; @@ -337,26 +312,20 @@ mod uefi_command_internal { use crate::os::uefi::ffi::{OsStrExt, OsStringExt}; use crate::ptr::NonNull; use crate::slice; + use crate::sys::pal::uefi::helpers::OwnedTable; use crate::sys_common::wstr::WStrUnits; - pub struct Command { + pub struct Image { handle: NonNull, - stdout: Option>, - stderr: Option>, - st: Box, + stdout: Option>, + stderr: Option>, + st: OwnedTable, args: Option>, } - impl Command { - const fn new( - handle: NonNull, - st: Box, - ) -> Self { - Self { handle, stdout: None, stderr: None, st, args: None } - } - + impl Image { pub fn load_image(p: &OsStr) -> io::Result { - let mut path = helpers::DevicePath::from_text(p)?; + let path = helpers::DevicePath::from_text(p)?; let boot_services: NonNull = boot_services() .ok_or_else(|| const_io_error!(io::ErrorKind::NotFound, "Boot Services not found"))? .cast(); @@ -367,7 +336,7 @@ mod uefi_command_internal { ((*boot_services.as_ptr()).load_image)( r_efi::efi::Boolean::FALSE, image_handle.as_ptr(), - path.as_mut(), + path.as_ptr(), crate::ptr::null_mut(), 0, child_handle.as_mut_ptr(), @@ -382,69 +351,93 @@ mod uefi_command_internal { let loaded_image: NonNull = helpers::open_protocol(child_handle, loaded_image::PROTOCOL_GUID).unwrap(); - let mut st: Box = - Box::new(unsafe { crate::ptr::read((*loaded_image.as_ptr()).system_table) }); + let st = OwnedTable::from_table(unsafe { (*loaded_image.as_ptr()).system_table }); - unsafe { - (*loaded_image.as_ptr()).system_table = st.as_mut(); - } - - Ok(Self::new(child_handle, st)) + Ok(Self { handle: child_handle, stdout: None, stderr: None, st, args: None }) } } - pub fn start_image(&self) -> io::Result { + pub fn start_image(&mut self) -> io::Result { + self.update_st_crc32()?; + + // Use our system table instead of the default one + let loaded_image: NonNull = + helpers::open_protocol(self.handle, loaded_image::PROTOCOL_GUID).unwrap(); + unsafe { + (*loaded_image.as_ptr()).system_table = self.st.as_mut_ptr(); + } + let boot_services: NonNull = boot_services() .ok_or_else(|| const_io_error!(io::ErrorKind::NotFound, "Boot Services not found"))? .cast(); - let mut exit_data_size: MaybeUninit = MaybeUninit::uninit(); + let mut exit_data_size: usize = 0; let mut exit_data: MaybeUninit<*mut u16> = MaybeUninit::uninit(); let r = unsafe { ((*boot_services.as_ptr()).start_image)( self.handle.as_ptr(), - exit_data_size.as_mut_ptr(), + &mut exit_data_size, exit_data.as_mut_ptr(), ) }; // Drop exitdata - unsafe { - exit_data_size.assume_init_drop(); - exit_data.assume_init_drop(); + if exit_data_size != 0 { + unsafe { + let exit_data = exit_data.assume_init(); + ((*boot_services.as_ptr()).free_pool)(exit_data as *mut crate::ffi::c_void); + } } Ok(r) } - pub fn stdout_init(&mut self, mut protocol: helpers::Protocol) { - self.st.console_out_handle = protocol.handle().as_ptr(); - self.st.con_out = - protocol.as_mut() as *mut PipeProtocol as *mut simple_text_output::Protocol; + fn set_stdout( + &mut self, + handle: r_efi::efi::Handle, + protocol: *mut simple_text_output::Protocol, + ) { + unsafe { + (*self.st.as_mut_ptr()).console_out_handle = handle; + (*self.st.as_mut_ptr()).con_out = protocol; + } + } + + fn set_stderr( + &mut self, + handle: r_efi::efi::Handle, + protocol: *mut simple_text_output::Protocol, + ) { + unsafe { + (*self.st.as_mut_ptr()).standard_error_handle = handle; + (*self.st.as_mut_ptr()).std_err = protocol; + } + } + pub fn stdout_init(&mut self, protocol: helpers::OwnedProtocol) { + self.set_stdout( + protocol.handle().as_ptr(), + protocol.as_ref() as *const PipeProtocol as *mut simple_text_output::Protocol, + ); self.stdout = Some(protocol); } pub fn stdout_inherit(&mut self) { let st: NonNull = system_table().cast(); - - self.st.console_out_handle = unsafe { (*st.as_ptr()).console_out_handle }; - self.st.con_out = unsafe { (*st.as_ptr()).con_out }; + unsafe { self.set_stdout((*st.as_ptr()).console_out_handle, (*st.as_ptr()).con_out) } } - pub fn stderr_init(&mut self, mut protocol: helpers::Protocol) { - self.st.standard_error_handle = protocol.handle().as_ptr(); - self.st.std_err = - protocol.as_mut() as *mut PipeProtocol as *mut simple_text_output::Protocol; - + pub fn stderr_init(&mut self, protocol: helpers::OwnedProtocol) { + self.set_stderr( + protocol.handle().as_ptr(), + protocol.as_ref() as *const PipeProtocol as *mut simple_text_output::Protocol, + ); self.stderr = Some(protocol); } pub fn stderr_inherit(&mut self) { let st: NonNull = system_table().cast(); - - self.st.standard_error_handle = unsafe { (*st.as_ptr()).standard_error_handle }; - self.st.std_err = unsafe { (*st.as_ptr()).std_err }; + unsafe { self.set_stderr((*st.as_ptr()).standard_error_handle, (*st.as_ptr()).std_err) } } pub fn stderr(&self) -> io::Result> { @@ -476,9 +469,37 @@ mod uefi_command_internal { self.args = Some(args); } + + fn update_st_crc32(&mut self) -> io::Result<()> { + let bt: NonNull = boot_services().unwrap().cast(); + let st_size = unsafe { (*self.st.as_ptr()).hdr.header_size as usize }; + let mut crc32: u32 = 0; + + // Set crc to 0 before calcuation + unsafe { + (*self.st.as_mut_ptr()).hdr.crc32 = 0; + } + + let r = unsafe { + ((*bt.as_ptr()).calculate_crc32)( + self.st.as_mut_ptr() as *mut crate::ffi::c_void, + st_size, + &mut crc32, + ) + }; + + if r.is_error() { + Err(io::Error::from_raw_os_error(r.as_usize())) + } else { + unsafe { + (*self.st.as_mut_ptr()).hdr.crc32 = crc32; + } + Ok(()) + } + } } - impl Drop for Command { + impl Drop for Image { fn drop(&mut self) { if let Some(bt) = boot_services() { let bt: NonNull = bt.cast(); @@ -501,13 +522,12 @@ mod uefi_command_internal { set_cursor_position: simple_text_output::ProtocolSetCursorPosition, enable_cursor: simple_text_output::ProtocolEnableCursor, mode: *mut simple_text_output::Mode, - _mode: Box, _buffer: Vec, } impl PipeProtocol { pub fn new() -> Self { - let mut mode = Box::new(simple_text_output::Mode { + let mode = Box::new(simple_text_output::Mode { max_mode: 0, mode: 0, attribute: 0, @@ -525,14 +545,13 @@ mod uefi_command_internal { clear_screen: Self::clear_screen, set_cursor_position: Self::set_cursor_position, enable_cursor: Self::enable_cursor, - mode: mode.as_mut(), - _mode: mode, + mode: Box::into_raw(mode), _buffer: Vec::new(), } } pub fn null() -> Self { - let mut mode = Box::new(simple_text_output::Mode { + let mode = Box::new(simple_text_output::Mode { max_mode: 0, mode: 0, attribute: 0, @@ -550,8 +569,7 @@ mod uefi_command_internal { clear_screen: Self::clear_screen, set_cursor_position: Self::set_cursor_position, enable_cursor: Self::enable_cursor, - mode: mode.as_mut(), - _mode: mode, + mode: Box::into_raw(mode), _buffer: Vec::new(), } } @@ -660,4 +678,12 @@ mod uefi_command_internal { r_efi::efi::Status::UNSUPPORTED } } + + impl Drop for PipeProtocol { + fn drop(&mut self) { + unsafe { + let _ = Box::from_raw(self.mode); + } + } + } } From ae82726a44e17e18f59cff6bcdcf31b00a672f86 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Wed, 17 Jul 2024 07:46:09 -0700 Subject: [PATCH 18/22] Conditionally build `wasm-component-ld` This commit updates the support for the `wasm-component-ld` tool from #126967 to conditionally build it rather than unconditionally building it when LLD is enabled. This support is disabled by default and can be enabled by one of two means: * the `extended` field in `config.toml` which dist builders use to build a complete set of tools for each host platform. * a `"wasm-component-ld"` entry in the `tools` section of `config.toml`. Neither of these are enabled by default meaning that most local builds will likely not have this new tool built. Dist builders should still, however, build the tool. --- config.example.toml | 1 + src/bootstrap/src/core/build_steps/compile.rs | 12 +++++++----- src/bootstrap/src/lib.rs | 13 +++++++++++++ 3 files changed, 21 insertions(+), 5 deletions(-) diff --git a/config.example.toml b/config.example.toml index a2c2fa1c2bd58..26687bcfb370f 100644 --- a/config.example.toml +++ b/config.example.toml @@ -333,6 +333,7 @@ # "rust-analyzer-proc-macro-srv", # "analysis", # "src", +# "wasm-component-ld", #] # Verbosity level: 0 == not verbose, 1 == verbose, 2 == very verbose, 3 == print environment variables on each rustc invocation diff --git a/src/bootstrap/src/core/build_steps/compile.rs b/src/bootstrap/src/core/build_steps/compile.rs index d4dd3e546ec44..c3ce66d5e3abc 100644 --- a/src/bootstrap/src/core/build_steps/compile.rs +++ b/src/bootstrap/src/core/build_steps/compile.rs @@ -1820,12 +1820,14 @@ impl Step for Assemble { &self_contained_lld_dir.join(exe(name, target_compiler.host)), ); } + } - // In addition to `rust-lld` also install `wasm-component-ld` when - // LLD is enabled. This is a relatively small binary that primarily - // delegates to the `rust-lld` binary for linking and then runs - // logic to create the final binary. This is used by the - // `wasm32-wasip2` target of Rust. + // In addition to `rust-lld` also install `wasm-component-ld` when + // LLD is enabled. This is a relatively small binary that primarily + // delegates to the `rust-lld` binary for linking and then runs + // logic to create the final binary. This is used by the + // `wasm32-wasip2` target of Rust. + if builder.build_wasm_component_ld() { let wasm_component_ld_exe = builder.ensure(crate::core::build_steps::tool::WasmComponentLd { compiler: build_compiler, diff --git a/src/bootstrap/src/lib.rs b/src/bootstrap/src/lib.rs index a77c20067e6bc..d2910f8edc6f8 100644 --- a/src/bootstrap/src/lib.rs +++ b/src/bootstrap/src/lib.rs @@ -1414,6 +1414,19 @@ Executed at: {executed_at}"#, None } + /// Returns whether it's requested that `wasm-component-ld` is built as part + /// of the sysroot. This is done either with the `extended` key in + /// `config.toml` or with the `tools` set. + fn build_wasm_component_ld(&self) -> bool { + if self.config.extended { + return true; + } + match &self.config.tools { + Some(set) => set.contains("wasm-component-ld"), + None => false, + } + } + /// Returns the root of the "rootfs" image that this target will be using, /// if one was configured. /// From f0a2b5b0d92a23cacb0acbae9ede9282b65c5dc1 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Thu, 18 Jul 2024 07:38:45 -0700 Subject: [PATCH 19/22] Add a change tracker entry --- src/bootstrap/src/utils/change_tracker.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/bootstrap/src/utils/change_tracker.rs b/src/bootstrap/src/utils/change_tracker.rs index 6a7c5c0f9b779..083418ed066b3 100644 --- a/src/bootstrap/src/utils/change_tracker.rs +++ b/src/bootstrap/src/utils/change_tracker.rs @@ -205,4 +205,9 @@ pub const CONFIG_CHANGE_HISTORY: &[ChangeInfo] = &[ severity: ChangeSeverity::Warning, summary: "`debug-logging` option has been removed from the default `tools` profile.", }, + ChangeInfo { + change_id: 127866, + severity: ChangeSeverity::Info, + summary: "the `wasm-component-ld` tool is now built as part of `build.extended` and can be a member of `build.tools`", + }, ]; From aef0e346de744faeaa522b9c08562bb3e1814adc Mon Sep 17 00:00:00 2001 From: Yuri Astrakhan Date: Fri, 19 Jul 2024 11:51:21 -0400 Subject: [PATCH 20/22] Avoid ref when using format! in compiler Clean up a few minor refs in `format!` macro, as it has a performance cost. Apparently the compiler is unable to inline `format!("{}", &variable)`, and does a run-time double-reference instead (format macro already does one level referencing). Inlining format args prevents accidental `&` misuse. --- .../rustc_codegen_cranelift/src/abi/mod.rs | 2 +- compiler/rustc_codegen_ssa/src/back/link.rs | 12 ++++---- .../rustc_codegen_ssa/src/codegen_attrs.rs | 4 +-- compiler/rustc_codegen_ssa/src/mono_item.rs | 2 +- compiler/rustc_fluent_macro/src/fluent.rs | 2 +- .../errors/wrong_number_of_generic_args.rs | 2 +- .../src/outlives/implicit_infer.rs | 6 ++-- .../rustc_hir_typeck/src/method/suggest.rs | 4 +-- compiler/rustc_middle/src/middle/stability.rs | 2 +- compiler/rustc_middle/src/ty/print/pretty.rs | 2 +- .../rustc_middle/src/util/find_self_call.rs | 2 +- .../src/impls/initialized.rs | 2 +- .../src/dead_store_elimination.rs | 2 +- .../src/early_otherwise_branch.rs | 4 +-- .../rustc_pattern_analysis/src/constructor.rs | 2 +- compiler/rustc_resolve/src/diagnostics.rs | 6 ++-- compiler/rustc_resolve/src/imports.rs | 2 +- compiler/rustc_resolve/src/rustdoc.rs | 2 +- .../src/cfi/typeid/itanium_cxx_abi/encode.rs | 28 +++++++++---------- compiler/rustc_type_ir/src/const_kind.rs | 10 +++---- compiler/rustc_type_ir/src/region_kind.rs | 2 +- compiler/rustc_type_ir/src/ty_kind.rs | 16 +++++------ compiler/stable_mir/src/mir/pretty.rs | 10 +++---- 23 files changed, 61 insertions(+), 65 deletions(-) diff --git a/compiler/rustc_codegen_cranelift/src/abi/mod.rs b/compiler/rustc_codegen_cranelift/src/abi/mod.rs index fa0de6f9de5ea..ac5aaea561cca 100644 --- a/compiler/rustc_codegen_cranelift/src/abi/mod.rs +++ b/compiler/rustc_codegen_cranelift/src/abi/mod.rs @@ -505,7 +505,7 @@ pub(crate) fn codegen_terminator_call<'tcx>( let nop_inst = fx.bcx.ins().nop(); fx.add_comment( nop_inst, - format!("virtual call; self arg pass mode: {:?}", &fn_abi.args[0]), + format!("virtual call; self arg pass mode: {:?}", fn_abi.args[0]), ); } diff --git a/compiler/rustc_codegen_ssa/src/back/link.rs b/compiler/rustc_codegen_ssa/src/back/link.rs index 8e07d128dbd64..8c582fac0d824 100644 --- a/compiler/rustc_codegen_ssa/src/back/link.rs +++ b/compiler/rustc_codegen_ssa/src/back/link.rs @@ -759,7 +759,7 @@ fn link_natively( sess.dcx().abort_if_errors(); // Invoke the system linker - info!("{:?}", &cmd); + info!("{cmd:?}"); let retry_on_segfault = env::var("RUSTC_RETRY_LINKER_ON_SEGFAULT").is_ok(); let unknown_arg_regex = Regex::new(r"(unknown|unrecognized) (command line )?(option|argument)").unwrap(); @@ -796,7 +796,7 @@ fn link_natively( cmd.arg(arg); } } - info!("{:?}", &cmd); + info!("{cmd:?}"); continue; } @@ -817,7 +817,7 @@ fn link_natively( cmd.arg(arg); } } - info!("{:?}", &cmd); + info!("{cmd:?}"); continue; } @@ -878,7 +878,7 @@ fn link_natively( cmd.arg(arg); } } - info!("{:?}", &cmd); + info!("{cmd:?}"); continue; } @@ -996,7 +996,7 @@ fn link_natively( sess.dcx().emit_err(errors::UnableToExeLinker { linker_path, error: e, - command_formatted: format!("{:?}", &cmd), + command_formatted: format!("{cmd:?}"), }); } @@ -1567,7 +1567,7 @@ fn print_native_static_libs( sess.dcx().emit_note(errors::StaticLibraryNativeArtifacts); // Prefix for greppability // Note: This must not be translated as tools are allowed to depend on this exact string. - sess.dcx().note(format!("native-static-libs: {}", &lib_args.join(" "))); + sess.dcx().note(format!("native-static-libs: {}", lib_args.join(" "))); } } } diff --git a/compiler/rustc_codegen_ssa/src/codegen_attrs.rs b/compiler/rustc_codegen_ssa/src/codegen_attrs.rs index 56a893738df60..bfa4c683d56ed 100644 --- a/compiler/rustc_codegen_ssa/src/codegen_attrs.rs +++ b/compiler/rustc_codegen_ssa/src/codegen_attrs.rs @@ -328,7 +328,7 @@ fn codegen_fn_attrs(tcx: TyCtxt<'_>, did: LocalDefId) -> CodegenFnAttrs { sym::link_section => { if let Some(val) = attr.value_str() { if val.as_str().bytes().any(|b| b == 0) { - let msg = format!("illegal null byte in link_section value: `{}`", &val); + let msg = format!("illegal null byte in link_section value: `{val}`"); tcx.dcx().span_err(attr.span, msg); } else { codegen_fn_attrs.link_section = Some(val); @@ -726,7 +726,7 @@ fn check_link_ordinal(tcx: TyCtxt<'_>, attr: &ast::Attribute) -> Option { if *ordinal <= u16::MAX as u128 { Some(ordinal.get() as u16) } else { - let msg = format!("ordinal value in `link_ordinal` is too large: `{}`", &ordinal); + let msg = format!("ordinal value in `link_ordinal` is too large: `{ordinal}`"); tcx.dcx() .struct_span_err(attr.span, msg) .with_note("the value may not exceed `u16::MAX`") diff --git a/compiler/rustc_codegen_ssa/src/mono_item.rs b/compiler/rustc_codegen_ssa/src/mono_item.rs index 0fbcb938d1a74..559ec400577f8 100644 --- a/compiler/rustc_codegen_ssa/src/mono_item.rs +++ b/compiler/rustc_codegen_ssa/src/mono_item.rs @@ -130,7 +130,7 @@ impl<'a, 'tcx: 'a> MonoItemExt<'a, 'tcx> for MonoItem<'tcx> { let symbol_name = self.symbol_name(cx.tcx()).name; - debug!("symbol {}", &symbol_name); + debug!("symbol {symbol_name}"); match *self { MonoItem::Static(def_id) => { diff --git a/compiler/rustc_fluent_macro/src/fluent.rs b/compiler/rustc_fluent_macro/src/fluent.rs index 214b6587af3b0..68fdabd3529a4 100644 --- a/compiler/rustc_fluent_macro/src/fluent.rs +++ b/compiler/rustc_fluent_macro/src/fluent.rs @@ -253,7 +253,7 @@ pub(crate) fn fluent_messages(input: proc_macro::TokenStream) -> proc_macro::Tok for Attribute { id: Identifier { name: attr_name }, .. } in attributes { let snake_name = Ident::new( - &format!("{}{}", &crate_prefix, &attr_name.replace('-', "_")), + &format!("{crate_prefix}{}", attr_name.replace('-', "_")), resource_str.span(), ); if !previous_attrs.insert(snake_name.clone()) { diff --git a/compiler/rustc_hir_analysis/src/errors/wrong_number_of_generic_args.rs b/compiler/rustc_hir_analysis/src/errors/wrong_number_of_generic_args.rs index 6426ad9dc184f..10be69a9fbb85 100644 --- a/compiler/rustc_hir_analysis/src/errors/wrong_number_of_generic_args.rs +++ b/compiler/rustc_hir_analysis/src/errors/wrong_number_of_generic_args.rs @@ -651,7 +651,7 @@ impl<'a, 'tcx> WrongNumberOfGenericArgs<'a, 'tcx> { self.path_segment.hir_id, num_params_to_take, ); - debug!("suggested_args: {:?}", &suggested_args); + debug!("suggested_args: {suggested_args:?}"); match self.angle_brackets { AngleBrackets::Missing => { diff --git a/compiler/rustc_hir_analysis/src/outlives/implicit_infer.rs b/compiler/rustc_hir_analysis/src/outlives/implicit_infer.rs index af08f50f65596..d953736c28c21 100644 --- a/compiler/rustc_hir_analysis/src/outlives/implicit_infer.rs +++ b/compiler/rustc_hir_analysis/src/outlives/implicit_infer.rs @@ -249,7 +249,7 @@ fn check_explicit_predicates<'tcx>( let explicit_predicates = explicit_map.explicit_predicates_of(tcx, def_id); for (outlives_predicate, &span) in explicit_predicates.as_ref().skip_binder() { - debug!("outlives_predicate = {:?}", &outlives_predicate); + debug!("outlives_predicate = {outlives_predicate:?}"); // Careful: If we are inferring the effects of a `dyn Trait<..>` // type, then when we look up the predicates for `Trait`, @@ -289,12 +289,12 @@ fn check_explicit_predicates<'tcx>( && let GenericArgKind::Type(ty) = outlives_predicate.0.unpack() && ty.walk().any(|arg| arg == self_ty.into()) { - debug!("skipping self ty = {:?}", &ty); + debug!("skipping self ty = {ty:?}"); continue; } let predicate = explicit_predicates.rebind(*outlives_predicate).instantiate(tcx, args); - debug!("predicate = {:?}", &predicate); + debug!("predicate = {predicate:?}"); insert_outlives_predicate(tcx, predicate.0, predicate.1, span, required_predicates); } } diff --git a/compiler/rustc_hir_typeck/src/method/suggest.rs b/compiler/rustc_hir_typeck/src/method/suggest.rs index 1cc7cf67ee31e..9bb30780a6e26 100644 --- a/compiler/rustc_hir_typeck/src/method/suggest.rs +++ b/compiler/rustc_hir_typeck/src/method/suggest.rs @@ -1265,9 +1265,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { } ( match parent_pred { - None => format!("`{}`", &p), + None => format!("`{p}`"), Some(parent_pred) => match format_pred(*parent_pred) { - None => format!("`{}`", &p), + None => format!("`{p}`"), Some((parent_p, _)) => { if !suggested && !suggested_bounds.contains(pred) diff --git a/compiler/rustc_middle/src/middle/stability.rs b/compiler/rustc_middle/src/middle/stability.rs index d1ccd158cf938..b113e81bd2d79 100644 --- a/compiler/rustc_middle/src/middle/stability.rs +++ b/compiler/rustc_middle/src/middle/stability.rs @@ -112,7 +112,7 @@ pub fn report_unstable( ) { let msg = match reason { Some(r) => format!("use of unstable library feature '{feature}': {r}"), - None => format!("use of unstable library feature '{}'", &feature), + None => format!("use of unstable library feature '{feature}'"), }; if is_soft { diff --git a/compiler/rustc_middle/src/ty/print/pretty.rs b/compiler/rustc_middle/src/ty/print/pretty.rs index 57cd2dc73c41b..0e241663e184b 100644 --- a/compiler/rustc_middle/src/ty/print/pretty.rs +++ b/compiler/rustc_middle/src/ty/print/pretty.rs @@ -2627,7 +2627,7 @@ impl<'tcx> FmtPrinter<'_, 'tcx> { self.prepare_region_info(value); } - debug!("self.used_region_names: {:?}", &self.used_region_names); + debug!("self.used_region_names: {:?}", self.used_region_names); let mut empty = true; let mut start_or_continue = |cx: &mut Self, start: &str, cont: &str| { diff --git a/compiler/rustc_middle/src/util/find_self_call.rs b/compiler/rustc_middle/src/util/find_self_call.rs index 027e2703e98d6..831853b0b48c3 100644 --- a/compiler/rustc_middle/src/util/find_self_call.rs +++ b/compiler/rustc_middle/src/util/find_self_call.rs @@ -14,7 +14,7 @@ pub fn find_self_call<'tcx>( local: Local, block: BasicBlock, ) -> Option<(DefId, GenericArgsRef<'tcx>)> { - debug!("find_self_call(local={:?}): terminator={:?}", local, &body[block].terminator); + debug!("find_self_call(local={:?}): terminator={:?}", local, body[block].terminator); if let Some(Terminator { kind: TerminatorKind::Call { func, args, .. }, .. }) = &body[block].terminator { diff --git a/compiler/rustc_mir_dataflow/src/impls/initialized.rs b/compiler/rustc_mir_dataflow/src/impls/initialized.rs index a9bceeccdce2f..d44da42416dbb 100644 --- a/compiler/rustc_mir_dataflow/src/impls/initialized.rs +++ b/compiler/rustc_mir_dataflow/src/impls/initialized.rs @@ -688,7 +688,7 @@ impl<'tcx> GenKillAnalysis<'tcx> for EverInitializedPlaces<'_, '_, 'tcx> { let init_loc_map = &move_data.init_loc_map; let rev_lookup = &move_data.rev_lookup; - debug!("initializes move_indexes {:?}", &init_loc_map[location]); + debug!("initializes move_indexes {:?}", init_loc_map[location]); trans.gen_all(init_loc_map[location].iter().copied()); if let mir::StatementKind::StorageDead(local) = stmt.kind { diff --git a/compiler/rustc_mir_transform/src/dead_store_elimination.rs b/compiler/rustc_mir_transform/src/dead_store_elimination.rs index 08dba1de500cf..60230bea02e29 100644 --- a/compiler/rustc_mir_transform/src/dead_store_elimination.rs +++ b/compiler/rustc_mir_transform/src/dead_store_elimination.rs @@ -102,7 +102,7 @@ pub fn eliminate<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) { | StatementKind::Nop => (), StatementKind::FakeRead(_) | StatementKind::AscribeUserType(_, _) => { - bug!("{:?} not found in this MIR phase!", &statement.kind) + bug!("{:?} not found in this MIR phase!", statement.kind) } } } diff --git a/compiler/rustc_mir_transform/src/early_otherwise_branch.rs b/compiler/rustc_mir_transform/src/early_otherwise_branch.rs index 9edb8bcee6e48..40c0c723d255a 100644 --- a/compiler/rustc_mir_transform/src/early_otherwise_branch.rs +++ b/compiler/rustc_mir_transform/src/early_otherwise_branch.rs @@ -106,11 +106,11 @@ impl<'tcx> MirPass<'tcx> for EarlyOtherwiseBranch { let parent = BasicBlock::from_usize(i); let Some(opt_data) = evaluate_candidate(tcx, body, parent) else { continue }; - if !tcx.consider_optimizing(|| format!("EarlyOtherwiseBranch {:?}", &opt_data)) { + if !tcx.consider_optimizing(|| format!("EarlyOtherwiseBranch {opt_data:?}")) { break; } - trace!("SUCCESS: found optimization possibility to apply: {:?}", &opt_data); + trace!("SUCCESS: found optimization possibility to apply: {opt_data:?}"); should_cleanup = true; diff --git a/compiler/rustc_pattern_analysis/src/constructor.rs b/compiler/rustc_pattern_analysis/src/constructor.rs index fb10370547597..f3e8e547066c8 100644 --- a/compiler/rustc_pattern_analysis/src/constructor.rs +++ b/compiler/rustc_pattern_analysis/src/constructor.rs @@ -904,7 +904,7 @@ impl Constructor { // be careful to detect strings here. However a string literal pattern will never // be reported as a non-exhaustiveness witness, so we can ignore this issue. Ref => { - write!(f, "&{:?}", &fields.next().unwrap())?; + write!(f, "&{:?}", fields.next().unwrap())?; } Slice(slice) => { write!(f, "[")?; diff --git a/compiler/rustc_resolve/src/diagnostics.rs b/compiler/rustc_resolve/src/diagnostics.rs index 7c0405c87e0c3..bf7972e392c9a 100644 --- a/compiler/rustc_resolve/src/diagnostics.rs +++ b/compiler/rustc_resolve/src/diagnostics.rs @@ -149,7 +149,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> { BuiltinLintDiag::AmbiguousGlobImports { diag }, ); } else { - let mut err = struct_span_code_err!(self.dcx(), diag.span, E0659, "{}", &diag.msg); + let mut err = struct_span_code_err!(self.dcx(), diag.span, E0659, "{}", diag.msg); report_ambiguity_error(&mut err, diag); err.emit(); } @@ -798,7 +798,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> { } ResolutionError::FailedToResolve { segment, label, suggestion, module } => { let mut err = - struct_span_code_err!(self.dcx(), span, E0433, "failed to resolve: {}", &label); + struct_span_code_err!(self.dcx(), span, E0433, "failed to resolve: {label}"); err.span_label(span, label); if let Some((suggestions, msg, applicability)) = suggestion { @@ -2893,7 +2893,7 @@ fn show_candidates( "" }; candidate.0 = - format!("{add_use}{}{append}{trailing}{additional_newline}", &candidate.0); + format!("{add_use}{}{append}{trailing}{additional_newline}", candidate.0); } match mode { diff --git a/compiler/rustc_resolve/src/imports.rs b/compiler/rustc_resolve/src/imports.rs index 3896fe4c4fa64..d05326ee311d4 100644 --- a/compiler/rustc_resolve/src/imports.rs +++ b/compiler/rustc_resolve/src/imports.rs @@ -694,7 +694,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> { .collect::>(); let msg = format!("unresolved import{} {}", pluralize!(paths.len()), paths.join(", "),); - let mut diag = struct_span_code_err!(self.dcx(), span, E0432, "{}", &msg); + let mut diag = struct_span_code_err!(self.dcx(), span, E0432, "{msg}"); if let Some((_, UnresolvedImportError { note: Some(note), .. })) = errors.iter().last() { diag.note(note.clone()); diff --git a/compiler/rustc_resolve/src/rustdoc.rs b/compiler/rustc_resolve/src/rustdoc.rs index 594608153211d..4c49c15c47258 100644 --- a/compiler/rustc_resolve/src/rustdoc.rs +++ b/compiler/rustc_resolve/src/rustdoc.rs @@ -339,7 +339,7 @@ pub fn strip_generics_from_path(path_str: &str) -> Result, MalformedGen } } - debug!("path_str: {:?}\nstripped segments: {:?}", path_str, &stripped_segments); + debug!("path_str: {path_str:?}\nstripped segments: {stripped_segments:?}"); let stripped_path = stripped_segments.join("::"); diff --git a/compiler/rustc_sanitizers/src/cfi/typeid/itanium_cxx_abi/encode.rs b/compiler/rustc_sanitizers/src/cfi/typeid/itanium_cxx_abi/encode.rs index f998fd599b7c0..190ea44318925 100644 --- a/compiler/rustc_sanitizers/src/cfi/typeid/itanium_cxx_abi/encode.rs +++ b/compiler/rustc_sanitizers/src/cfi/typeid/itanium_cxx_abi/encode.rs @@ -238,12 +238,12 @@ fn encode_predicate<'tcx>( match predicate.as_ref().skip_binder() { ty::ExistentialPredicate::Trait(trait_ref) => { let name = encode_ty_name(tcx, trait_ref.def_id); - let _ = write!(s, "u{}{}", name.len(), &name); + let _ = write!(s, "u{}{}", name.len(), name); s.push_str(&encode_args(tcx, trait_ref.args, trait_ref.def_id, true, dict, options)); } ty::ExistentialPredicate::Projection(projection) => { let name = encode_ty_name(tcx, projection.def_id); - let _ = write!(s, "u{}{}", name.len(), &name); + let _ = write!(s, "u{}{}", name.len(), name); s.push_str(&encode_args(tcx, projection.args, projection.def_id, true, dict, options)); match projection.term.unpack() { TermKind::Ty(ty) => s.push_str(&encode_ty(tcx, ty, dict, options)), @@ -258,7 +258,7 @@ fn encode_predicate<'tcx>( } ty::ExistentialPredicate::AutoTrait(def_id) => { let name = encode_ty_name(tcx, *def_id); - let _ = write!(s, "u{}{}", name.len(), &name); + let _ = write!(s, "u{}{}", name.len(), name); } }; compress(dict, DictKey::Predicate(*predicate.as_ref().skip_binder()), &mut s); @@ -416,7 +416,7 @@ pub fn encode_ty<'tcx>( // A let len = len.eval_target_usize(tcx, ty::ParamEnv::reveal_all()); let mut s = String::from("A"); - let _ = write!(s, "{}", &len); + let _ = write!(s, "{len}"); s.push_str(&encode_ty(tcx, *ty0, dict, options)); compress(dict, DictKey::Ty(ty, TyQ::None), &mut s); typeid.push_str(&s); @@ -492,13 +492,13 @@ pub fn encode_ty<'tcx>( // calling convention (or extern types [i.e., ty::Foreign]) as , where // is . let name = tcx.item_name(def_id).to_string(); - let _ = write!(s, "{}{}", name.len(), &name); + let _ = write!(s, "{}{}", name.len(), name); compress(dict, DictKey::Ty(ty, TyQ::None), &mut s); } else { // u[IE], where is // , as vendor extended type. let name = encode_ty_name(tcx, def_id); - let _ = write!(s, "u{}{}", name.len(), &name); + let _ = write!(s, "u{}{}", name.len(), name); s.push_str(&encode_args(tcx, args, def_id, false, dict, options)); compress(dict, DictKey::Ty(ty, TyQ::None), &mut s); } @@ -530,7 +530,7 @@ pub fn encode_ty<'tcx>( } } else { let name = tcx.item_name(*def_id).to_string(); - let _ = write!(s, "{}{}", name.len(), &name); + let _ = write!(s, "{}{}", name.len(), name); } compress(dict, DictKey::Ty(ty, TyQ::None), &mut s); typeid.push_str(&s); @@ -542,7 +542,7 @@ pub fn encode_ty<'tcx>( // as vendor extended type. let mut s = String::new(); let name = encode_ty_name(tcx, *def_id); - let _ = write!(s, "u{}{}", name.len(), &name); + let _ = write!(s, "u{}{}", name.len(), name); s.push_str(&encode_args(tcx, args, *def_id, false, dict, options)); compress(dict, DictKey::Ty(ty, TyQ::None), &mut s); typeid.push_str(&s); @@ -553,7 +553,7 @@ pub fn encode_ty<'tcx>( // as vendor extended type. let mut s = String::new(); let name = encode_ty_name(tcx, *def_id); - let _ = write!(s, "u{}{}", name.len(), &name); + let _ = write!(s, "u{}{}", name.len(), name); let parent_args = tcx.mk_args(args.as_coroutine_closure().parent_args()); s.push_str(&encode_args(tcx, parent_args, *def_id, false, dict, options)); compress(dict, DictKey::Ty(ty, TyQ::None), &mut s); @@ -565,7 +565,7 @@ pub fn encode_ty<'tcx>( // as vendor extended type. let mut s = String::new(); let name = encode_ty_name(tcx, *def_id); - let _ = write!(s, "u{}{}", name.len(), &name); + let _ = write!(s, "u{}{}", name.len(), name); // Encode parent args only s.push_str(&encode_args( tcx, @@ -588,7 +588,7 @@ pub fn encode_ty<'tcx>( s.push('E'); compress(dict, DictKey::Ty(Ty::new_imm_ref(tcx, *region, *ty0), TyQ::None), &mut s); if ty.is_mutable_ptr() { - s = format!("{}{}", "U3mut", &s); + s = format!("{}{}", "U3mut", s); compress(dict, DictKey::Ty(ty, TyQ::Mut), &mut s); } typeid.push_str(&s); @@ -600,10 +600,10 @@ pub fn encode_ty<'tcx>( let mut s = String::new(); s.push_str(&encode_ty(tcx, *ptr_ty, dict, options)); if !ty.is_mutable_ptr() { - s = format!("{}{}", "K", &s); + s = format!("{}{}", "K", s); compress(dict, DictKey::Ty(*ptr_ty, TyQ::Const), &mut s); }; - s = format!("{}{}", "P", &s); + s = format!("{}{}", "P", s); compress(dict, DictKey::Ty(ty, TyQ::None), &mut s); typeid.push_str(&s); } @@ -722,7 +722,7 @@ fn encode_ty_name(tcx: TyCtxt<'_>, def_id: DefId) -> String { s.push('C'); s.push_str(&to_disambiguator(tcx.stable_crate_id(def_path.krate).as_u64())); let crate_name = tcx.crate_name(def_path.krate).to_string(); - let _ = write!(s, "{}{}", crate_name.len(), &crate_name); + let _ = write!(s, "{}{}", crate_name.len(), crate_name); // Disambiguators and names def_path.data.reverse(); diff --git a/compiler/rustc_type_ir/src/const_kind.rs b/compiler/rustc_type_ir/src/const_kind.rs index f1683f5449f5c..1a51c95ecdff6 100644 --- a/compiler/rustc_type_ir/src/const_kind.rs +++ b/compiler/rustc_type_ir/src/const_kind.rs @@ -65,15 +65,13 @@ impl fmt::Debug for ConstKind { match self { Param(param) => write!(f, "{param:?}"), - Infer(var) => write!(f, "{:?}", &var), + Infer(var) => write!(f, "{var:?}"), Bound(debruijn, var) => crate::debug_bound_var(f, *debruijn, var), Placeholder(placeholder) => write!(f, "{placeholder:?}"), - Unevaluated(uv) => { - write!(f, "{:?}", &uv) - } - Value(ty, valtree) => write!(f, "({valtree:?}: {:?})", &ty), + Unevaluated(uv) => write!(f, "{uv:?}"), + Value(ty, valtree) => write!(f, "({valtree:?}: {ty:?})"), Error(_) => write!(f, "{{const error}}"), - Expr(expr) => write!(f, "{:?}", &expr), + Expr(expr) => write!(f, "{expr:?}"), } } } diff --git a/compiler/rustc_type_ir/src/region_kind.rs b/compiler/rustc_type_ir/src/region_kind.rs index 7abcc370c886c..140c89af147d0 100644 --- a/compiler/rustc_type_ir/src/region_kind.rs +++ b/compiler/rustc_type_ir/src/region_kind.rs @@ -232,7 +232,7 @@ impl fmt::Debug for RegionKind { ReStatic => f.write_str("'static"), - ReVar(vid) => write!(f, "{:?}", &vid), + ReVar(vid) => write!(f, "{vid:?}"), RePlaceholder(placeholder) => write!(f, "{placeholder:?}"), diff --git a/compiler/rustc_type_ir/src/ty_kind.rs b/compiler/rustc_type_ir/src/ty_kind.rs index 4ffebef9f1f9b..9896425a34110 100644 --- a/compiler/rustc_type_ir/src/ty_kind.rs +++ b/compiler/rustc_type_ir/src/ty_kind.rs @@ -367,18 +367,16 @@ impl fmt::Debug for TyKind { } Foreign(d) => f.debug_tuple("Foreign").field(d).finish(), Str => write!(f, "str"), - Array(t, c) => write!(f, "[{:?}; {:?}]", &t, &c), - Pat(t, p) => write!(f, "pattern_type!({:?} is {:?})", &t, &p), + Array(t, c) => write!(f, "[{t:?}; {c:?}]"), + Pat(t, p) => write!(f, "pattern_type!({t:?} is {p:?})"), Slice(t) => write!(f, "[{:?}]", &t), RawPtr(ty, mutbl) => write!(f, "*{} {:?}", mutbl.ptr_str(), ty), Ref(r, t, m) => write!(f, "&{:?} {}{:?}", r, m.prefix_str(), t), FnDef(d, s) => f.debug_tuple("FnDef").field(d).field(&s).finish(), - FnPtr(s) => write!(f, "{:?}", &s), + FnPtr(s) => write!(f, "{s:?}"), Dynamic(p, r, repr) => match repr { - DynKind::Dyn => write!(f, "dyn {:?} + {:?}", &p, &r), - DynKind::DynStar => { - write!(f, "dyn* {:?} + {:?}", &p, &r) - } + DynKind::Dyn => write!(f, "dyn {p:?} + {r:?}"), + DynKind::DynStar => write!(f, "dyn* {p:?} + {r:?}"), }, Closure(d, s) => f.debug_tuple("Closure").field(d).field(&s).finish(), CoroutineClosure(d, s) => f.debug_tuple("CoroutineClosure").field(d).field(&s).finish(), @@ -392,7 +390,7 @@ impl fmt::Debug for TyKind { if count > 0 { write!(f, ", ")?; } - write!(f, "{:?}", &ty)?; + write!(f, "{ty:?}")?; count += 1; } // unary tuples need a trailing comma @@ -1050,7 +1048,7 @@ impl fmt::Debug for FnSig { if i > 0 { write!(f, ", ")?; } - write!(f, "{:?}", &ty)?; + write!(f, "{ty:?}")?; } if *c_variadic { if inputs.is_empty() { diff --git a/compiler/stable_mir/src/mir/pretty.rs b/compiler/stable_mir/src/mir/pretty.rs index 83734a0d13826..18ecccb453663 100644 --- a/compiler/stable_mir/src/mir/pretty.rs +++ b/compiler/stable_mir/src/mir/pretty.rs @@ -179,7 +179,7 @@ fn pretty_terminator_head(writer: &mut W, terminator: &TerminatorKind) if !expected { write!(writer, "!")?; } - write!(writer, "{}, ", &pretty_operand(cond))?; + write!(writer, "{}, ", pretty_operand(cond))?; pretty_assert_message(writer, msg)?; write!(writer, ")") } @@ -325,7 +325,7 @@ fn pretty_ty_const(ct: &TyConst) -> String { fn pretty_rvalue(writer: &mut W, rval: &Rvalue) -> io::Result<()> { match rval { Rvalue::AddressOf(mutability, place) => { - write!(writer, "&raw {}(*{:?})", &pretty_mut(*mutability), place) + write!(writer, "&raw {}(*{:?})", pretty_mut(*mutability), place) } Rvalue::Aggregate(aggregate_kind, operands) => { // FIXME: Add pretty_aggregate function that returns a pretty string @@ -336,13 +336,13 @@ fn pretty_rvalue(writer: &mut W, rval: &Rvalue) -> io::Result<()> { write!(writer, ")") } Rvalue::BinaryOp(bin, op1, op2) => { - write!(writer, "{:?}({}, {})", bin, &pretty_operand(op1), pretty_operand(op2)) + write!(writer, "{:?}({}, {})", bin, pretty_operand(op1), pretty_operand(op2)) } Rvalue::Cast(_, op, ty) => { write!(writer, "{} as {}", pretty_operand(op), ty) } Rvalue::CheckedBinaryOp(bin, op1, op2) => { - write!(writer, "Checked{:?}({}, {})", bin, &pretty_operand(op1), pretty_operand(op2)) + write!(writer, "Checked{:?}({}, {})", bin, pretty_operand(op1), pretty_operand(op2)) } Rvalue::CopyForDeref(deref) => { write!(writer, "CopyForDeref({:?})", deref) @@ -363,7 +363,7 @@ fn pretty_rvalue(writer: &mut W, rval: &Rvalue) -> io::Result<()> { write!(writer, "{kind}{:?}", place) } Rvalue::Repeat(op, cnst) => { - write!(writer, "{} \" \" {}", &pretty_operand(op), &pretty_ty_const(cnst)) + write!(writer, "{} \" \" {}", pretty_operand(op), pretty_ty_const(cnst)) } Rvalue::ShallowInitBox(_, _) => Ok(()), Rvalue::ThreadLocalRef(item) => { From 3ff758877fd994ebeb7a3a0a65fae4de3f66b8d7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Fri, 19 Jul 2024 19:39:37 +0000 Subject: [PATCH 21/22] More accurate suggestion for `-> Box` or `-> impl Trait` When encountering `-> Trait`, suggest `-> Box` (instead of `-> Box`. If there's a single returned type within the `fn`, suggest `-> impl Trait`. --- .../src/error_reporting/traits/suggestions.rs | 48 +++++++++++++------ tests/ui/error-codes/E0746.stderr | 9 ++-- ...n-trait-return-should-be-impl-trait.stderr | 38 ++++++--------- ...-trait-in-return-position-dyn-trait.stderr | 5 +- ...type-err-cause-on-impl-trait-return.stderr | 13 ++--- tests/ui/issues/issue-18107.stderr | 4 +- tests/ui/unsized/box-instead-of-dyn-fn.stderr | 4 +- tests/ui/unsized/issue-91801.stderr | 10 ++-- tests/ui/unsized/issue-91803.stderr | 4 +- 9 files changed, 71 insertions(+), 64 deletions(-) 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 fa2acdd4a54ca..ffc8839435e89 100644 --- a/compiler/rustc_trait_selection/src/error_reporting/traits/suggestions.rs +++ b/compiler/rustc_trait_selection/src/error_reporting/traits/suggestions.rs @@ -1793,25 +1793,42 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> { err.children.clear(); let span = obligation.cause.span; - if let Ok(snip) = self.tcx.sess.source_map().span_to_snippet(span) + let body = self.tcx.hir().body_owned_by(obligation.cause.body_id); + + let mut visitor = ReturnsVisitor::default(); + visitor.visit_body(&body); + + let (pre, impl_span) = if let Ok(snip) = self.tcx.sess.source_map().span_to_snippet(span) && snip.starts_with("dyn ") { - err.span_suggestion( - span.with_hi(span.lo() + BytePos(4)), - "return an `impl Trait` instead of a `dyn Trait`, \ - if all returned values are the same type", + ("", span.with_hi(span.lo() + BytePos(4))) + } else { + ("dyn ", span.shrink_to_lo()) + }; + let alternatively = if visitor + .returns + .iter() + .map(|expr| self.typeck_results.as_ref().unwrap().expr_ty_adjusted_opt(expr)) + .collect::>() + .len() + <= 1 + { + err.span_suggestion_verbose( + impl_span, + "consider returning an `impl Trait` instead of a `dyn Trait`", "impl ", Applicability::MaybeIncorrect, ); - } - - let body = self.tcx.hir().body_owned_by(obligation.cause.body_id); - - let mut visitor = ReturnsVisitor::default(); - visitor.visit_body(&body); + "alternatively, " + } else { + err.help("if there were a single returned type, you could use `impl Trait` instead"); + "" + }; - let mut sugg = - vec![(span.shrink_to_lo(), "Box<".to_string()), (span.shrink_to_hi(), ">".to_string())]; + let mut sugg = vec![ + (span.shrink_to_lo(), format!("Box<{pre}")), + (span.shrink_to_hi(), ">".to_string()), + ]; sugg.extend(visitor.returns.into_iter().flat_map(|expr| { let span = expr.span.find_ancestor_in_same_ctxt(obligation.cause.span).unwrap_or(expr.span); @@ -1837,7 +1854,10 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> { })); err.multipart_suggestion( - "box the return type, and wrap all of the returned values in `Box::new`", + format!( + "{alternatively}box the return type, and wrap all of the returned values in \ + `Box::new`", + ), sugg, Applicability::MaybeIncorrect, ); diff --git a/tests/ui/error-codes/E0746.stderr b/tests/ui/error-codes/E0746.stderr index 9fe90ab7bec7f..cfc747cb1e2c4 100644 --- a/tests/ui/error-codes/E0746.stderr +++ b/tests/ui/error-codes/E0746.stderr @@ -4,11 +4,11 @@ error[E0746]: return type cannot have an unboxed trait object LL | fn foo() -> dyn Trait { Struct } | ^^^^^^^^^ doesn't have a size known at compile-time | -help: return an `impl Trait` instead of a `dyn Trait`, if all returned values are the same type +help: consider returning an `impl Trait` instead of a `dyn Trait` | LL | fn foo() -> impl Trait { Struct } | ~~~~ -help: box the return type, and wrap all of the returned values in `Box::new` +help: alternatively, box the return type, and wrap all of the returned values in `Box::new` | LL | fn foo() -> Box { Box::new(Struct) } | ++++ + +++++++++ + @@ -19,10 +19,7 @@ error[E0746]: return type cannot have an unboxed trait object LL | fn bar() -> dyn Trait { | ^^^^^^^^^ doesn't have a size known at compile-time | -help: return an `impl Trait` instead of a `dyn Trait`, if all returned values are the same type - | -LL | fn bar() -> impl Trait { - | ~~~~ + = help: if there were a single returned type, you could use `impl Trait` instead help: box the return type, and wrap all of the returned values in `Box::new` | LL ~ fn bar() -> Box { diff --git a/tests/ui/impl-trait/dyn-trait-return-should-be-impl-trait.stderr b/tests/ui/impl-trait/dyn-trait-return-should-be-impl-trait.stderr index f25269ca03207..9c54665956451 100644 --- a/tests/ui/impl-trait/dyn-trait-return-should-be-impl-trait.stderr +++ b/tests/ui/impl-trait/dyn-trait-return-should-be-impl-trait.stderr @@ -48,10 +48,14 @@ error[E0746]: return type cannot have an unboxed trait object LL | fn bap() -> Trait { Struct } | ^^^^^ doesn't have a size known at compile-time | -help: box the return type, and wrap all of the returned values in `Box::new` +help: consider returning an `impl Trait` instead of a `dyn Trait` + | +LL | fn bap() -> impl Trait { Struct } + | ++++ +help: alternatively, box the return type, and wrap all of the returned values in `Box::new` | -LL | fn bap() -> Box { Box::new(Struct) } - | ++++ + +++++++++ + +LL | fn bap() -> Box { Box::new(Struct) } + | +++++++ + +++++++++ + error[E0746]: return type cannot have an unboxed trait object --> $DIR/dyn-trait-return-should-be-impl-trait.rs:15:13 @@ -59,11 +63,11 @@ error[E0746]: return type cannot have an unboxed trait object LL | fn ban() -> dyn Trait { Struct } | ^^^^^^^^^ doesn't have a size known at compile-time | -help: return an `impl Trait` instead of a `dyn Trait`, if all returned values are the same type +help: consider returning an `impl Trait` instead of a `dyn Trait` | LL | fn ban() -> impl Trait { Struct } | ~~~~ -help: box the return type, and wrap all of the returned values in `Box::new` +help: alternatively, box the return type, and wrap all of the returned values in `Box::new` | LL | fn ban() -> Box { Box::new(Struct) } | ++++ + +++++++++ + @@ -74,11 +78,11 @@ error[E0746]: return type cannot have an unboxed trait object LL | fn bak() -> dyn Trait { unimplemented!() } | ^^^^^^^^^ doesn't have a size known at compile-time | -help: return an `impl Trait` instead of a `dyn Trait`, if all returned values are the same type +help: consider returning an `impl Trait` instead of a `dyn Trait` | LL | fn bak() -> impl Trait { unimplemented!() } | ~~~~ -help: box the return type, and wrap all of the returned values in `Box::new` +help: alternatively, box the return type, and wrap all of the returned values in `Box::new` | LL | fn bak() -> Box { Box::new(unimplemented!()) } | ++++ + +++++++++ + @@ -89,10 +93,7 @@ error[E0746]: return type cannot have an unboxed trait object LL | fn bal() -> dyn Trait { | ^^^^^^^^^ doesn't have a size known at compile-time | -help: return an `impl Trait` instead of a `dyn Trait`, if all returned values are the same type - | -LL | fn bal() -> impl Trait { - | ~~~~ + = help: if there were a single returned type, you could use `impl Trait` instead help: box the return type, and wrap all of the returned values in `Box::new` | LL ~ fn bal() -> Box { @@ -108,10 +109,7 @@ error[E0746]: return type cannot have an unboxed trait object LL | fn bax() -> dyn Trait { | ^^^^^^^^^ doesn't have a size known at compile-time | -help: return an `impl Trait` instead of a `dyn Trait`, if all returned values are the same type - | -LL | fn bax() -> impl Trait { - | ~~~~ + = help: if there were a single returned type, you could use `impl Trait` instead help: box the return type, and wrap all of the returned values in `Box::new` | LL ~ fn bax() -> Box { @@ -263,10 +261,7 @@ error[E0746]: return type cannot have an unboxed trait object LL | fn bat() -> dyn Trait { | ^^^^^^^^^ doesn't have a size known at compile-time | -help: return an `impl Trait` instead of a `dyn Trait`, if all returned values are the same type - | -LL | fn bat() -> impl Trait { - | ~~~~ + = help: if there were a single returned type, you could use `impl Trait` instead help: box the return type, and wrap all of the returned values in `Box::new` | LL ~ fn bat() -> Box { @@ -282,10 +277,7 @@ error[E0746]: return type cannot have an unboxed trait object LL | fn bay() -> dyn Trait { | ^^^^^^^^^ doesn't have a size known at compile-time | -help: return an `impl Trait` instead of a `dyn Trait`, if all returned values are the same type - | -LL | fn bay() -> impl Trait { - | ~~~~ + = help: if there were a single returned type, you could use `impl Trait` instead help: box the return type, and wrap all of the returned values in `Box::new` | LL ~ fn bay() -> Box { diff --git a/tests/ui/impl-trait/object-unsafe-trait-in-return-position-dyn-trait.stderr b/tests/ui/impl-trait/object-unsafe-trait-in-return-position-dyn-trait.stderr index fc9c30abf1336..09a689e6396e9 100644 --- a/tests/ui/impl-trait/object-unsafe-trait-in-return-position-dyn-trait.stderr +++ b/tests/ui/impl-trait/object-unsafe-trait-in-return-position-dyn-trait.stderr @@ -54,10 +54,7 @@ error[E0746]: return type cannot have an unboxed trait object LL | fn car() -> dyn NotObjectSafe { | ^^^^^^^^^^^^^^^^^ doesn't have a size known at compile-time | -help: return an `impl Trait` instead of a `dyn Trait`, if all returned values are the same type - | -LL | fn car() -> impl NotObjectSafe { - | ~~~~ + = help: if there were a single returned type, you could use `impl Trait` instead help: box the return type, and wrap all of the returned values in `Box::new` | LL ~ fn car() -> Box { diff --git a/tests/ui/impl-trait/point-to-type-err-cause-on-impl-trait-return.stderr b/tests/ui/impl-trait/point-to-type-err-cause-on-impl-trait-return.stderr index 9205d74504f6f..54849c112f5a0 100644 --- a/tests/ui/impl-trait/point-to-type-err-cause-on-impl-trait-return.stderr +++ b/tests/ui/impl-trait/point-to-type-err-cause-on-impl-trait-return.stderr @@ -171,11 +171,11 @@ error[E0746]: return type cannot have an unboxed trait object LL | fn hat() -> dyn std::fmt::Display { | ^^^^^^^^^^^^^^^^^^^^^ doesn't have a size known at compile-time | -help: return an `impl Trait` instead of a `dyn Trait`, if all returned values are the same type +help: consider returning an `impl Trait` instead of a `dyn Trait` | LL | fn hat() -> impl std::fmt::Display { | ~~~~ -help: box the return type, and wrap all of the returned values in `Box::new` +help: alternatively, box the return type, and wrap all of the returned values in `Box::new` | LL ~ fn hat() -> Box { LL | match 13 { @@ -192,11 +192,11 @@ error[E0746]: return type cannot have an unboxed trait object LL | fn pug() -> dyn std::fmt::Display { | ^^^^^^^^^^^^^^^^^^^^^ doesn't have a size known at compile-time | -help: return an `impl Trait` instead of a `dyn Trait`, if all returned values are the same type +help: consider returning an `impl Trait` instead of a `dyn Trait` | LL | fn pug() -> impl std::fmt::Display { | ~~~~ -help: box the return type, and wrap all of the returned values in `Box::new` +help: alternatively, box the return type, and wrap all of the returned values in `Box::new` | LL ~ fn pug() -> Box { LL | match 13 { @@ -211,10 +211,7 @@ error[E0746]: return type cannot have an unboxed trait object LL | fn man() -> dyn std::fmt::Display { | ^^^^^^^^^^^^^^^^^^^^^ doesn't have a size known at compile-time | -help: return an `impl Trait` instead of a `dyn Trait`, if all returned values are the same type - | -LL | fn man() -> impl std::fmt::Display { - | ~~~~ + = help: if there were a single returned type, you could use `impl Trait` instead help: box the return type, and wrap all of the returned values in `Box::new` | LL ~ fn man() -> Box { diff --git a/tests/ui/issues/issue-18107.stderr b/tests/ui/issues/issue-18107.stderr index 702207c30f7cd..705f7d0df12ae 100644 --- a/tests/ui/issues/issue-18107.stderr +++ b/tests/ui/issues/issue-18107.stderr @@ -4,11 +4,11 @@ error[E0746]: return type cannot have an unboxed trait object LL | dyn AbstractRenderer | ^^^^^^^^^^^^^^^^^^^^ doesn't have a size known at compile-time | -help: return an `impl Trait` instead of a `dyn Trait`, if all returned values are the same type +help: consider returning an `impl Trait` instead of a `dyn Trait` | LL | impl AbstractRenderer | ~~~~ -help: box the return type, and wrap all of the returned values in `Box::new` +help: alternatively, box the return type, and wrap all of the returned values in `Box::new` | LL ~ Box LL | diff --git a/tests/ui/unsized/box-instead-of-dyn-fn.stderr b/tests/ui/unsized/box-instead-of-dyn-fn.stderr index f2828b384b254..1f1845569ef17 100644 --- a/tests/ui/unsized/box-instead-of-dyn-fn.stderr +++ b/tests/ui/unsized/box-instead-of-dyn-fn.stderr @@ -4,11 +4,11 @@ error[E0746]: return type cannot have an unboxed trait object LL | fn print_on_or_the_other<'a>(a: i32, b: &'a String) -> dyn Fn() + 'a { | ^^^^^^^^^^^^^ doesn't have a size known at compile-time | -help: return an `impl Trait` instead of a `dyn Trait`, if all returned values are the same type +help: consider returning an `impl Trait` instead of a `dyn Trait` | LL | fn print_on_or_the_other<'a>(a: i32, b: &'a String) -> impl Fn() + 'a { | ~~~~ -help: box the return type, and wrap all of the returned values in `Box::new` +help: alternatively, box the return type, and wrap all of the returned values in `Box::new` | LL ~ fn print_on_or_the_other<'a>(a: i32, b: &'a String) -> Box { LL | diff --git a/tests/ui/unsized/issue-91801.stderr b/tests/ui/unsized/issue-91801.stderr index d1d652d1860f9..e13cabbb81d6c 100644 --- a/tests/ui/unsized/issue-91801.stderr +++ b/tests/ui/unsized/issue-91801.stderr @@ -4,10 +4,14 @@ error[E0746]: return type cannot have an unboxed trait object LL | fn or<'a>(first: &'static Validator<'a>, second: &'static Validator<'a>) -> Validator<'a> { | ^^^^^^^^^^^^^ doesn't have a size known at compile-time | -help: box the return type, and wrap all of the returned values in `Box::new` +help: consider returning an `impl Trait` instead of a `dyn Trait` | -LL | fn or<'a>(first: &'static Validator<'a>, second: &'static Validator<'a>) -> Box> { - | ++++ + +LL | fn or<'a>(first: &'static Validator<'a>, second: &'static Validator<'a>) -> impl Validator<'a> { + | ++++ +help: alternatively, box the return type, and wrap all of the returned values in `Box::new` + | +LL | fn or<'a>(first: &'static Validator<'a>, second: &'static Validator<'a>) -> Box> { + | +++++++ + error: aborting due to 1 previous error diff --git a/tests/ui/unsized/issue-91803.stderr b/tests/ui/unsized/issue-91803.stderr index 632af02b4b6ce..3b89066499dfe 100644 --- a/tests/ui/unsized/issue-91803.stderr +++ b/tests/ui/unsized/issue-91803.stderr @@ -4,11 +4,11 @@ error[E0746]: return type cannot have an unboxed trait object LL | fn or<'a>(first: &'static dyn Foo<'a>) -> dyn Foo<'a> { | ^^^^^^^^^^^ doesn't have a size known at compile-time | -help: return an `impl Trait` instead of a `dyn Trait`, if all returned values are the same type +help: consider returning an `impl Trait` instead of a `dyn Trait` | LL | fn or<'a>(first: &'static dyn Foo<'a>) -> impl Foo<'a> { | ~~~~ -help: box the return type, and wrap all of the returned values in `Box::new` +help: alternatively, box the return type, and wrap all of the returned values in `Box::new` | LL | fn or<'a>(first: &'static dyn Foo<'a>) -> Box> { | ++++ + From 8bcf0b4a374116c5f9cba9b02deb19b3ea694380 Mon Sep 17 00:00:00 2001 From: Yuri Astrakhan Date: Fri, 19 Jul 2024 11:51:21 -0400 Subject: [PATCH 22/22] Avoid ref when using format! in compiler MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Clean up a few minor refs in `format!` macro, as it has a performance cost. Apparently the compiler is unable to inline `format!("{}", &variable)`, and does a run-time double-reference instead (format macro already does one level referencing).  Inlining format args prevents accidental `&` misuse. --- src/bootstrap/src/core/build_steps/compile.rs | 2 +- src/bootstrap/src/core/build_steps/llvm.rs | 2 +- src/bootstrap/src/utils/cc_detect.rs | 10 +++++----- src/bootstrap/src/utils/tarball.rs | 2 +- src/librustdoc/html/format.rs | 2 +- 5 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/bootstrap/src/core/build_steps/compile.rs b/src/bootstrap/src/core/build_steps/compile.rs index d4dd3e546ec44..d2e22b0087247 100644 --- a/src/bootstrap/src/core/build_steps/compile.rs +++ b/src/bootstrap/src/core/build_steps/compile.rs @@ -695,7 +695,7 @@ 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(builder, &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(builder, &dst); diff --git a/src/bootstrap/src/core/build_steps/llvm.rs b/src/bootstrap/src/core/build_steps/llvm.rs index 888290a0479ba..4479030184118 100644 --- a/src/bootstrap/src/core/build_steps/llvm.rs +++ b/src/bootstrap/src/core/build_steps/llvm.rs @@ -1411,7 +1411,7 @@ impl Step for Libunwind { } } } - assert_eq!(cpp_len, count, "Can't get object files from {:?}", &out_dir); + assert_eq!(cpp_len, count, "Can't get object files from {out_dir:?}"); cc_cfg.compile("unwind"); out_dir diff --git a/src/bootstrap/src/utils/cc_detect.rs b/src/bootstrap/src/utils/cc_detect.rs index d6fa7fc0bbebd..20d79e490ec48 100644 --- a/src/bootstrap/src/utils/cc_detect.rs +++ b/src/bootstrap/src/utils/cc_detect.rs @@ -142,15 +142,15 @@ pub fn find_target(build: &Build, target: TargetSelection) { build.cxx.borrow_mut().insert(target, compiler); } - build.verbose(|| println!("CC_{} = {:?}", &target.triple, build.cc(target))); - build.verbose(|| println!("CFLAGS_{} = {:?}", &target.triple, cflags)); + build.verbose(|| println!("CC_{} = {:?}", target.triple, build.cc(target))); + build.verbose(|| println!("CFLAGS_{} = {cflags:?}", target.triple)); if let Ok(cxx) = build.cxx(target) { let cxxflags = build.cflags(target, GitRepo::Rustc, CLang::Cxx); - build.verbose(|| println!("CXX_{} = {:?}", &target.triple, cxx)); - build.verbose(|| println!("CXXFLAGS_{} = {:?}", &target.triple, cxxflags)); + build.verbose(|| println!("CXX_{} = {cxx:?}", target.triple)); + build.verbose(|| println!("CXXFLAGS_{} = {cxxflags:?}", target.triple)); } if let Some(ar) = ar { - build.verbose(|| println!("AR_{} = {:?}", &target.triple, ar)); + build.verbose(|| println!("AR_{} = {ar:?}", target.triple)); build.ar.borrow_mut().insert(target, ar); } diff --git a/src/bootstrap/src/utils/tarball.rs b/src/bootstrap/src/utils/tarball.rs index 9378c35127f26..28a295e3e2a5a 100644 --- a/src/bootstrap/src/utils/tarball.rs +++ b/src/bootstrap/src/utils/tarball.rs @@ -244,7 +244,7 @@ impl<'a> Tarball<'a> { cmd.arg("generate") .arg("--image-dir") .arg(&this.image_dir) - .arg(format!("--component-name={}", &component_name)); + .arg(format!("--component-name={component_name}")); if let Some((dir, dirs)) = this.bulk_dirs.split_first() { let mut arg = dir.as_os_str().to_os_string(); diff --git a/src/librustdoc/html/format.rs b/src/librustdoc/html/format.rs index 9b0b2571ec115..055781f7fed72 100644 --- a/src/librustdoc/html/format.rs +++ b/src/librustdoc/html/format.rs @@ -850,7 +850,7 @@ fn resolved_path<'cx>( } } if w.alternate() { - write!(w, "{}{:#}", &last.name, last.args.print(cx))?; + write!(w, "{}{:#}", last.name, last.args.print(cx))?; } else { let path = if use_absolute { if let Ok((_, _, fqp)) = href(did, cx) {