From 395ea03ec51e91e91107d47c04a9aabad55e5a39 Mon Sep 17 00:00:00 2001 From: Ayush Singh Date: Wed, 27 Mar 2024 01:07:39 +0530 Subject: [PATCH 01/21] 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 48b74df138439..27b7049ac54dd 100644 --- a/library/std/src/sys/pal/uefi/mod.rs +++ b/library/std/src/sys/pal/uefi/mod.rs @@ -24,7 +24,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 ed2dca207e455386fa96bd72c610d392fae5387a Mon Sep 17 00:00:00 2001 From: Ayush Singh Date: Fri, 29 Mar 2024 17:13:16 +0530 Subject: [PATCH 02/21] 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 3c478805206a55e93dfa7222907d3c0178095231 Mon Sep 17 00:00:00 2001 From: Ayush Singh Date: Fri, 29 Mar 2024 17:20:50 +0530 Subject: [PATCH 03/21] 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 8ee24d371424c098ce82a918f9ed8227ebbcb521 Mon Sep 17 00:00:00 2001 From: Ayush Singh Date: Fri, 29 Mar 2024 18:36:52 +0530 Subject: [PATCH 04/21] 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 588881b8251903400110d8b40d383496a10db122 Mon Sep 17 00:00:00 2001 From: Ayush Singh Date: Fri, 29 Mar 2024 19:07:46 +0530 Subject: [PATCH 05/21] 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 6ce936aa76d9a5b728b7607c5e41f41c97f69a65 Mon Sep 17 00:00:00 2001 From: Ayush Singh Date: Fri, 29 Mar 2024 19:44:37 +0530 Subject: [PATCH 06/21] 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 c05a9b1e02f33bd723ff7ae9f1bd71a5a077da74 Mon Sep 17 00:00:00 2001 From: Ayush Singh Date: Fri, 29 Mar 2024 20:31:56 +0530 Subject: [PATCH 07/21] 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 a1e344fc26605b149ebe247c49bb977fd59855aa Mon Sep 17 00:00:00 2001 From: Ayush Singh Date: Fri, 29 Mar 2024 20:51:48 +0530 Subject: [PATCH 08/21] 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 4a868f41520c20553869299b7735025b1ddd7e71 Mon Sep 17 00:00:00 2001 From: Ayush Singh Date: Sun, 31 Mar 2024 01:06:33 +0530 Subject: [PATCH 09/21] 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 --- Cargo.lock | 4 +- 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 +++++++++++++----------- 5 files changed, 234 insertions(+), 154 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index c0d1337ef93a2..166da5e09ed97 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3052,9 +3052,9 @@ dependencies = [ [[package]] name = "r-efi" -version = "4.4.0" +version = "4.5.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c47196f636c4cc0634b73b0405323d177753c2e15e866952c64ea22902567a34" +checksum = "e9e935efc5854715dfc0a4c9ef18dc69dee0ec3bf9cc3ab740db831c0fdd86a3" dependencies = [ "compiler_builtins", "rustc-std-workspace-core", diff --git a/library/std/Cargo.toml b/library/std/Cargo.toml index 79a504c5a5e2c..7badf23c3f135 100644 --- a/library/std/Cargo.toml +++ b/library/std/Cargo.toml @@ -56,7 +56,7 @@ hermit-abi = { version = "0.3.9", 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 9d6576fa84117..bc6360d24f551 100644 --- a/library/std/src/lib.rs +++ b/library/std/src/lib.rs @@ -292,6 +292,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..a61673c39885b 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(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 5834772177540912b53dc4f19413a4ba3c804ea4 Mon Sep 17 00:00:00 2001 From: Veera Date: Thu, 4 Jul 2024 19:16:47 -0400 Subject: [PATCH 10/21] Update Tests --- ...ced-return-type-complex-type-issue-126311.rs | 5 +++++ ...return-type-complex-type-issue-126311.stderr | 14 ++++++++++++++ .../misplaced-return-type-issue-126311.rs | 5 +++++ .../misplaced-return-type-issue-126311.stderr | 14 ++++++++++++++ ...turn-type-where-in-next-line-issue-126311.rs | 11 +++++++++++ ...-type-where-in-next-line-issue-126311.stderr | 17 +++++++++++++++++ ...ced-return-type-without-type-issue-126311.rs | 6 ++++++ ...return-type-without-type-issue-126311.stderr | 8 ++++++++ ...ed-return-type-without-where-issue-126311.rs | 4 ++++ ...eturn-type-without-where-issue-126311.stderr | 8 ++++++++ 10 files changed, 92 insertions(+) create mode 100644 tests/ui/parser/issues/misplaced-return-type-complex-type-issue-126311.rs create mode 100644 tests/ui/parser/issues/misplaced-return-type-complex-type-issue-126311.stderr create mode 100644 tests/ui/parser/issues/misplaced-return-type-issue-126311.rs create mode 100644 tests/ui/parser/issues/misplaced-return-type-issue-126311.stderr create mode 100644 tests/ui/parser/issues/misplaced-return-type-where-in-next-line-issue-126311.rs create mode 100644 tests/ui/parser/issues/misplaced-return-type-where-in-next-line-issue-126311.stderr create mode 100644 tests/ui/parser/issues/misplaced-return-type-without-type-issue-126311.rs create mode 100644 tests/ui/parser/issues/misplaced-return-type-without-type-issue-126311.stderr create mode 100644 tests/ui/parser/issues/misplaced-return-type-without-where-issue-126311.rs create mode 100644 tests/ui/parser/issues/misplaced-return-type-without-where-issue-126311.stderr diff --git a/tests/ui/parser/issues/misplaced-return-type-complex-type-issue-126311.rs b/tests/ui/parser/issues/misplaced-return-type-complex-type-issue-126311.rs new file mode 100644 index 0000000000000..722303dd03482 --- /dev/null +++ b/tests/ui/parser/issues/misplaced-return-type-complex-type-issue-126311.rs @@ -0,0 +1,5 @@ +fn foo() where T: Default -> impl Default + 'static {} +//~^ ERROR return type should be specified after the function parameters +//~| HELP place the return type after the function parameters + +fn main() {} diff --git a/tests/ui/parser/issues/misplaced-return-type-complex-type-issue-126311.stderr b/tests/ui/parser/issues/misplaced-return-type-complex-type-issue-126311.stderr new file mode 100644 index 0000000000000..2ce3b78bb8ba9 --- /dev/null +++ b/tests/ui/parser/issues/misplaced-return-type-complex-type-issue-126311.stderr @@ -0,0 +1,14 @@ +error: return type should be specified after the function parameters + --> $DIR/misplaced-return-type-complex-type-issue-126311.rs:1:30 + | +LL | fn foo() where T: Default -> impl Default + 'static {} + | ^^ expected one of `(`, `+`, `,`, `::`, `<`, or `{` + | +help: place the return type after the function parameters + | +LL - fn foo() where T: Default -> impl Default + 'static {} +LL + fn foo() -> impl Default + 'static where T: Default {} + | + +error: aborting due to 1 previous error + diff --git a/tests/ui/parser/issues/misplaced-return-type-issue-126311.rs b/tests/ui/parser/issues/misplaced-return-type-issue-126311.rs new file mode 100644 index 0000000000000..ad164f77beea6 --- /dev/null +++ b/tests/ui/parser/issues/misplaced-return-type-issue-126311.rs @@ -0,0 +1,5 @@ +fn foo() where T: Default -> u8 {} +//~^ ERROR return type should be specified after the function parameters +//~| HELP place the return type after the function parameters + +fn main() {} diff --git a/tests/ui/parser/issues/misplaced-return-type-issue-126311.stderr b/tests/ui/parser/issues/misplaced-return-type-issue-126311.stderr new file mode 100644 index 0000000000000..e473b902ce3ef --- /dev/null +++ b/tests/ui/parser/issues/misplaced-return-type-issue-126311.stderr @@ -0,0 +1,14 @@ +error: return type should be specified after the function parameters + --> $DIR/misplaced-return-type-issue-126311.rs:1:30 + | +LL | fn foo() where T: Default -> u8 {} + | ^^ expected one of `(`, `+`, `,`, `::`, `<`, or `{` + | +help: place the return type after the function parameters + | +LL - fn foo() where T: Default -> u8 {} +LL + fn foo() -> u8 where T: Default {} + | + +error: aborting due to 1 previous error + diff --git a/tests/ui/parser/issues/misplaced-return-type-where-in-next-line-issue-126311.rs b/tests/ui/parser/issues/misplaced-return-type-where-in-next-line-issue-126311.rs new file mode 100644 index 0000000000000..782d7d5ee4905 --- /dev/null +++ b/tests/ui/parser/issues/misplaced-return-type-where-in-next-line-issue-126311.rs @@ -0,0 +1,11 @@ +fn foo() +//~^ HELP place the return type after the function parameters +where + T: Default, + K: Clone, -> Result +//~^ ERROR return type should be specified after the function parameters +{ + Ok(0) +} + +fn main() {} diff --git a/tests/ui/parser/issues/misplaced-return-type-where-in-next-line-issue-126311.stderr b/tests/ui/parser/issues/misplaced-return-type-where-in-next-line-issue-126311.stderr new file mode 100644 index 0000000000000..196a46d7ea54b --- /dev/null +++ b/tests/ui/parser/issues/misplaced-return-type-where-in-next-line-issue-126311.stderr @@ -0,0 +1,17 @@ +error: return type should be specified after the function parameters + --> $DIR/misplaced-return-type-where-in-next-line-issue-126311.rs:5:15 + | +LL | K: Clone, -> Result + | ^^ expected one of `{`, lifetime, or type + | +help: place the return type after the function parameters + | +LL ~ fn foo() -> Result +LL | +LL | where +LL | T: Default, +LL ~ K: Clone, + | + +error: aborting due to 1 previous error + diff --git a/tests/ui/parser/issues/misplaced-return-type-without-type-issue-126311.rs b/tests/ui/parser/issues/misplaced-return-type-without-type-issue-126311.rs new file mode 100644 index 0000000000000..2c09edbc7926c --- /dev/null +++ b/tests/ui/parser/issues/misplaced-return-type-without-type-issue-126311.rs @@ -0,0 +1,6 @@ +fn foo() where T: Default -> { +//~^ ERROR expected one of `(`, `+`, `,`, `::`, `<`, or `{`, found `->` + 0 +} + +fn main() {} diff --git a/tests/ui/parser/issues/misplaced-return-type-without-type-issue-126311.stderr b/tests/ui/parser/issues/misplaced-return-type-without-type-issue-126311.stderr new file mode 100644 index 0000000000000..0eb3bb7d8126e --- /dev/null +++ b/tests/ui/parser/issues/misplaced-return-type-without-type-issue-126311.stderr @@ -0,0 +1,8 @@ +error: expected one of `(`, `+`, `,`, `::`, `<`, or `{`, found `->` + --> $DIR/misplaced-return-type-without-type-issue-126311.rs:1:30 + | +LL | fn foo() where T: Default -> { + | ^^ expected one of `(`, `+`, `,`, `::`, `<`, or `{` + +error: aborting due to 1 previous error + diff --git a/tests/ui/parser/issues/misplaced-return-type-without-where-issue-126311.rs b/tests/ui/parser/issues/misplaced-return-type-without-where-issue-126311.rs new file mode 100644 index 0000000000000..672233674a058 --- /dev/null +++ b/tests/ui/parser/issues/misplaced-return-type-without-where-issue-126311.rs @@ -0,0 +1,4 @@ +fn bar() -> u8 -> u64 {} +//~^ ERROR expected one of `!`, `(`, `+`, `::`, `<`, `where`, or `{`, found `->` + +fn main() {} diff --git a/tests/ui/parser/issues/misplaced-return-type-without-where-issue-126311.stderr b/tests/ui/parser/issues/misplaced-return-type-without-where-issue-126311.stderr new file mode 100644 index 0000000000000..730904d3671f1 --- /dev/null +++ b/tests/ui/parser/issues/misplaced-return-type-without-where-issue-126311.stderr @@ -0,0 +1,8 @@ +error: expected one of `!`, `(`, `+`, `::`, `<`, `where`, or `{`, found `->` + --> $DIR/misplaced-return-type-without-where-issue-126311.rs:1:19 + | +LL | fn bar() -> u8 -> u64 {} + | ^^ expected one of 7 possible tokens + +error: aborting due to 1 previous error + From cf09cba20c317c1d2cfa78820a699ac5ee684bf2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Fri, 12 Jul 2024 18:52:52 +0000 Subject: [PATCH 11/21] When finding item gated behind a `cfg` flat, point at it Previously we would only mention that the item was gated out, and opportunisitically mention the feature flag name when possible. We now point to the place where the item was gated, which can be behind layers of macro indirection, or in different modules. ``` error[E0433]: failed to resolve: could not find `doesnt_exist` in `inner` --> $DIR/diagnostics-cross-crate.rs:18:23 | LL | cfged_out::inner::doesnt_exist::hello(); | ^^^^^^^^^^^^ could not find `doesnt_exist` in `inner` | note: found an item that was configured out --> $DIR/auxiliary/cfged_out.rs:6:13 | LL | pub mod doesnt_exist { | ^^^^^^^^^^^^ note: the item is gated here --> $DIR/auxiliary/cfged_out.rs:5:5 | LL | #[cfg(FALSE)] | ^^^^^^^^^^^^^ ``` --- compiler/rustc_resolve/messages.ftl | 2 + compiler/rustc_resolve/src/diagnostics.rs | 8 +++- compiler/rustc_resolve/src/errors.rs | 9 ++++ tests/ui/cfg/diagnostics-cross-crate.rs | 3 ++ tests/ui/cfg/diagnostics-cross-crate.stderr | 27 +++++++++-- tests/ui/cfg/diagnostics-reexport.rs | 8 ++-- tests/ui/cfg/diagnostics-reexport.stderr | 20 ++++++++ tests/ui/cfg/diagnostics-same-crate.rs | 9 ++-- tests/ui/cfg/diagnostics-same-crate.stderr | 48 ++++++++++++++----- tests/ui/macros/builtin-std-paths-fail.stderr | 2 + tests/ui/macros/macro-outer-attributes.stderr | 11 +++++ 11 files changed, 122 insertions(+), 25 deletions(-) diff --git a/compiler/rustc_resolve/messages.ftl b/compiler/rustc_resolve/messages.ftl index 4b9c36ad39fb5..73d1a2ea49a13 100644 --- a/compiler/rustc_resolve/messages.ftl +++ b/compiler/rustc_resolve/messages.ftl @@ -232,6 +232,8 @@ resolve_is_private = resolve_item_was_behind_feature = the item is gated behind the `{$feature}` feature +resolve_item_was_cfg_out = the item is gated here + resolve_items_in_traits_are_not_importable = items in traits are not importable diff --git a/compiler/rustc_resolve/src/diagnostics.rs b/compiler/rustc_resolve/src/diagnostics.rs index 09221041031a2..3ce500866bde2 100644 --- a/compiler/rustc_resolve/src/diagnostics.rs +++ b/compiler/rustc_resolve/src/diagnostics.rs @@ -2530,7 +2530,13 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> { && let NestedMetaItem::MetaItem(meta_item) = &nested[0] && let MetaItemKind::NameValue(feature_name) = &meta_item.kind { - let note = errors::ItemWasBehindFeature { feature: feature_name.symbol }; + let note = errors::ItemWasBehindFeature { + feature: feature_name.symbol, + span: meta_item.span, + }; + err.subdiagnostic(note); + } else { + let note = errors::ItemWasCfgOut { span: cfg.span }; err.subdiagnostic(note); } } diff --git a/compiler/rustc_resolve/src/errors.rs b/compiler/rustc_resolve/src/errors.rs index 097f4af05c305..0a68231c6fee4 100644 --- a/compiler/rustc_resolve/src/errors.rs +++ b/compiler/rustc_resolve/src/errors.rs @@ -1228,6 +1228,15 @@ pub(crate) struct FoundItemConfigureOut { #[note(resolve_item_was_behind_feature)] pub(crate) struct ItemWasBehindFeature { pub(crate) feature: Symbol, + #[primary_span] + pub(crate) span: Span, +} + +#[derive(Subdiagnostic)] +#[note(resolve_item_was_cfg_out)] +pub(crate) struct ItemWasCfgOut { + #[primary_span] + pub(crate) span: Span, } #[derive(Diagnostic)] diff --git a/tests/ui/cfg/diagnostics-cross-crate.rs b/tests/ui/cfg/diagnostics-cross-crate.rs index 77dd91d6c2823..00ac7e2fd080e 100644 --- a/tests/ui/cfg/diagnostics-cross-crate.rs +++ b/tests/ui/cfg/diagnostics-cross-crate.rs @@ -11,12 +11,14 @@ fn main() { cfged_out::inner::uwu(); //~ ERROR cannot find function //~^ NOTE found an item that was configured out //~| NOTE not found in `cfged_out::inner` + //~| NOTE the item is gated here // The module isn't found - we would like to get a diagnostic, but currently don't due to // the awkward way the resolver diagnostics are currently implemented. cfged_out::inner::doesnt_exist::hello(); //~ ERROR failed to resolve //~^ NOTE could not find `doesnt_exist` in `inner` //~| NOTE found an item that was configured out + //~| NOTE the item is gated here // It should find the one in the right module, not the wrong one. cfged_out::inner::right::meow(); //~ ERROR cannot find function @@ -28,4 +30,5 @@ fn main() { cfged_out::vanished(); //~ ERROR cannot find function //~^ NOTE found an item that was configured out //~| NOTE not found in `cfged_out` + //~| NOTE the item is gated here } diff --git a/tests/ui/cfg/diagnostics-cross-crate.stderr b/tests/ui/cfg/diagnostics-cross-crate.stderr index 8a238f3640445..07ad4e3272d1b 100644 --- a/tests/ui/cfg/diagnostics-cross-crate.stderr +++ b/tests/ui/cfg/diagnostics-cross-crate.stderr @@ -1,5 +1,5 @@ error[E0433]: failed to resolve: could not find `doesnt_exist` in `inner` - --> $DIR/diagnostics-cross-crate.rs:17:23 + --> $DIR/diagnostics-cross-crate.rs:18:23 | LL | cfged_out::inner::doesnt_exist::hello(); | ^^^^^^^^^^^^ could not find `doesnt_exist` in `inner` @@ -9,6 +9,11 @@ note: found an item that was configured out | LL | pub mod doesnt_exist { | ^^^^^^^^^^^^ +note: the item is gated here + --> $DIR/auxiliary/cfged_out.rs:5:5 + | +LL | #[cfg(FALSE)] + | ^^^^^^^^^^^^^ error[E0425]: cannot find function `uwu` in crate `cfged_out` --> $DIR/diagnostics-cross-crate.rs:7:16 @@ -27,9 +32,14 @@ note: found an item that was configured out | LL | pub fn uwu() {} | ^^^ +note: the item is gated here + --> $DIR/auxiliary/cfged_out.rs:2:5 + | +LL | #[cfg(FALSE)] + | ^^^^^^^^^^^^^ error[E0425]: cannot find function `meow` in module `cfged_out::inner::right` - --> $DIR/diagnostics-cross-crate.rs:22:30 + --> $DIR/diagnostics-cross-crate.rs:24:30 | LL | cfged_out::inner::right::meow(); | ^^^^ not found in `cfged_out::inner::right` @@ -39,10 +49,14 @@ note: found an item that was configured out | LL | pub fn meow() {} | ^^^^ - = note: the item is gated behind the `what-a-cool-feature` feature +note: the item is gated behind the `what-a-cool-feature` feature + --> $DIR/auxiliary/cfged_out.rs:16:15 + | +LL | #[cfg(feature = "what-a-cool-feature")] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error[E0425]: cannot find function `vanished` in crate `cfged_out` - --> $DIR/diagnostics-cross-crate.rs:28:16 + --> $DIR/diagnostics-cross-crate.rs:30:16 | LL | cfged_out::vanished(); | ^^^^^^^^ not found in `cfged_out` @@ -52,6 +66,11 @@ note: found an item that was configured out | LL | pub fn vanished() {} | ^^^^^^^^ +note: the item is gated here + --> $DIR/auxiliary/cfged_out.rs:21:1 + | +LL | #[cfg(i_dont_exist_and_you_can_do_nothing_about_it)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: aborting due to 5 previous errors diff --git a/tests/ui/cfg/diagnostics-reexport.rs b/tests/ui/cfg/diagnostics-reexport.rs index 9b3208cb87cf9..9ae7d931fcb8f 100644 --- a/tests/ui/cfg/diagnostics-reexport.rs +++ b/tests/ui/cfg/diagnostics-reexport.rs @@ -4,7 +4,7 @@ pub mod inner { pub fn uwu() {} } - #[cfg(FALSE)] + #[cfg(FALSE)] //~ NOTE the item is gated here pub use super::uwu; //~^ NOTE found an item that was configured out } @@ -14,7 +14,7 @@ pub use a::x; //~| NOTE no `x` in `a` mod a { - #[cfg(FALSE)] + #[cfg(FALSE)] //~ NOTE the item is gated here pub fn x() {} //~^ NOTE found an item that was configured out } @@ -25,10 +25,10 @@ pub use b::{x, y}; //~| NOTE no `y` in `b` mod b { - #[cfg(FALSE)] + #[cfg(FALSE)] //~ NOTE the item is gated here pub fn x() {} //~^ NOTE found an item that was configured out - #[cfg(FALSE)] + #[cfg(FALSE)] //~ NOTE the item is gated here pub fn y() {} //~^ NOTE found an item that was configured out } diff --git a/tests/ui/cfg/diagnostics-reexport.stderr b/tests/ui/cfg/diagnostics-reexport.stderr index e25b7cf86e210..737202fdf9ad9 100644 --- a/tests/ui/cfg/diagnostics-reexport.stderr +++ b/tests/ui/cfg/diagnostics-reexport.stderr @@ -9,6 +9,11 @@ note: found an item that was configured out | LL | pub fn x() {} | ^ +note: the item is gated here + --> $DIR/diagnostics-reexport.rs:17:5 + | +LL | #[cfg(FALSE)] + | ^^^^^^^^^^^^^ error[E0432]: unresolved imports `b::x`, `b::y` --> $DIR/diagnostics-reexport.rs:22:13 @@ -23,11 +28,21 @@ note: found an item that was configured out | LL | pub fn x() {} | ^ +note: the item is gated here + --> $DIR/diagnostics-reexport.rs:28:5 + | +LL | #[cfg(FALSE)] + | ^^^^^^^^^^^^^ note: found an item that was configured out --> $DIR/diagnostics-reexport.rs:32:12 | LL | pub fn y() {} | ^ +note: the item is gated here + --> $DIR/diagnostics-reexport.rs:31:5 + | +LL | #[cfg(FALSE)] + | ^^^^^^^^^^^^^ error[E0425]: cannot find function `uwu` in module `inner` --> $DIR/diagnostics-reexport.rs:38:12 @@ -40,6 +55,11 @@ note: found an item that was configured out | LL | pub use super::uwu; | ^^^ +note: the item is gated here + --> $DIR/diagnostics-reexport.rs:7:5 + | +LL | #[cfg(FALSE)] + | ^^^^^^^^^^^^^ error: aborting due to 3 previous errors diff --git a/tests/ui/cfg/diagnostics-same-crate.rs b/tests/ui/cfg/diagnostics-same-crate.rs index b2a0fb58dd6b5..d6f8dd21a9221 100644 --- a/tests/ui/cfg/diagnostics-same-crate.rs +++ b/tests/ui/cfg/diagnostics-same-crate.rs @@ -1,11 +1,13 @@ #![allow(unexpected_cfgs)] // since we want to recognize them as unexpected pub mod inner { - #[cfg(FALSE)] + #[cfg(FALSE)] //~ NOTE the item is gated here pub fn uwu() {} //~^ NOTE found an item that was configured out - #[cfg(FALSE)] + #[cfg(FALSE)] //~ NOTE the item is gated here + //~^ NOTE the item is gated here + //~| NOTE the item is gated here pub mod doesnt_exist { //~^ NOTE found an item that was configured out //~| NOTE found an item that was configured out @@ -20,7 +22,7 @@ pub mod inner { } pub mod right { - #[cfg(feature = "what-a-cool-feature")] + #[cfg(feature = "what-a-cool-feature")] //~ NOTE the item is gated behind the `what-a-cool-feature` feature pub fn meow() {} //~^ NOTE found an item that was configured out } @@ -55,7 +57,6 @@ fn main() { // It should find the one in the right module, not the wrong one. inner::right::meow(); //~ ERROR cannot find function //~| NOTE not found in `inner::right - //~| NOTE the item is gated behind the `what-a-cool-feature` feature // Exists in the crate root - we would generally want a diagnostic, // but currently don't have one. diff --git a/tests/ui/cfg/diagnostics-same-crate.stderr b/tests/ui/cfg/diagnostics-same-crate.stderr index 86421736b8c60..dd0d10c6567ea 100644 --- a/tests/ui/cfg/diagnostics-same-crate.stderr +++ b/tests/ui/cfg/diagnostics-same-crate.stderr @@ -1,41 +1,56 @@ error[E0432]: unresolved import `super::inner::doesnt_exist` - --> $DIR/diagnostics-same-crate.rs:30:9 + --> $DIR/diagnostics-same-crate.rs:32:9 | LL | use super::inner::doesnt_exist; | ^^^^^^^^^^^^^^^^^^^^^^^^^^ no `doesnt_exist` in `inner` | note: found an item that was configured out - --> $DIR/diagnostics-same-crate.rs:9:13 + --> $DIR/diagnostics-same-crate.rs:11:13 | LL | pub mod doesnt_exist { | ^^^^^^^^^^^^ +note: the item is gated here + --> $DIR/diagnostics-same-crate.rs:8:5 + | +LL | #[cfg(FALSE)] + | ^^^^^^^^^^^^^ error[E0432]: unresolved import `super::inner::doesnt_exist` - --> $DIR/diagnostics-same-crate.rs:33:23 + --> $DIR/diagnostics-same-crate.rs:35:23 | LL | use super::inner::doesnt_exist::hi; | ^^^^^^^^^^^^ could not find `doesnt_exist` in `inner` | note: found an item that was configured out - --> $DIR/diagnostics-same-crate.rs:9:13 + --> $DIR/diagnostics-same-crate.rs:11:13 | LL | pub mod doesnt_exist { | ^^^^^^^^^^^^ +note: the item is gated here + --> $DIR/diagnostics-same-crate.rs:8:5 + | +LL | #[cfg(FALSE)] + | ^^^^^^^^^^^^^ error[E0433]: failed to resolve: could not find `doesnt_exist` in `inner` - --> $DIR/diagnostics-same-crate.rs:52:12 + --> $DIR/diagnostics-same-crate.rs:54:12 | LL | inner::doesnt_exist::hello(); | ^^^^^^^^^^^^ could not find `doesnt_exist` in `inner` | note: found an item that was configured out - --> $DIR/diagnostics-same-crate.rs:9:13 + --> $DIR/diagnostics-same-crate.rs:11:13 | LL | pub mod doesnt_exist { | ^^^^^^^^^^^^ +note: the item is gated here + --> $DIR/diagnostics-same-crate.rs:8:5 + | +LL | #[cfg(FALSE)] + | ^^^^^^^^^^^^^ error[E0425]: cannot find function `uwu` in module `inner` - --> $DIR/diagnostics-same-crate.rs:47:12 + --> $DIR/diagnostics-same-crate.rs:49:12 | LL | inner::uwu(); | ^^^ not found in `inner` @@ -45,28 +60,37 @@ note: found an item that was configured out | LL | pub fn uwu() {} | ^^^ +note: the item is gated here + --> $DIR/diagnostics-same-crate.rs:4:5 + | +LL | #[cfg(FALSE)] + | ^^^^^^^^^^^^^ error[E0425]: cannot find function `meow` in module `inner::right` - --> $DIR/diagnostics-same-crate.rs:56:19 + --> $DIR/diagnostics-same-crate.rs:58:19 | LL | inner::right::meow(); | ^^^^ not found in `inner::right` | note: found an item that was configured out - --> $DIR/diagnostics-same-crate.rs:24:16 + --> $DIR/diagnostics-same-crate.rs:26:16 | LL | pub fn meow() {} | ^^^^ - = note: the item is gated behind the `what-a-cool-feature` feature +note: the item is gated behind the `what-a-cool-feature` feature + --> $DIR/diagnostics-same-crate.rs:25:15 + | +LL | #[cfg(feature = "what-a-cool-feature")] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error[E0425]: cannot find function `uwu` in this scope - --> $DIR/diagnostics-same-crate.rs:43:5 + --> $DIR/diagnostics-same-crate.rs:45:5 | LL | uwu(); | ^^^ not found in this scope error[E0425]: cannot find function `vanished` in this scope - --> $DIR/diagnostics-same-crate.rs:63:5 + --> $DIR/diagnostics-same-crate.rs:64:5 | LL | vanished(); | ^^^^^^^^ not found in this scope diff --git a/tests/ui/macros/builtin-std-paths-fail.stderr b/tests/ui/macros/builtin-std-paths-fail.stderr index 331943843c02a..49034c3987b24 100644 --- a/tests/ui/macros/builtin-std-paths-fail.stderr +++ b/tests/ui/macros/builtin-std-paths-fail.stderr @@ -104,6 +104,8 @@ LL | #[std::test] | note: found an item that was configured out --> $SRC_DIR/std/src/lib.rs:LL:COL +note: the item is gated here + --> $SRC_DIR/std/src/lib.rs:LL:COL error: aborting due to 16 previous errors diff --git a/tests/ui/macros/macro-outer-attributes.stderr b/tests/ui/macros/macro-outer-attributes.stderr index 87c0655a422d6..a8809f3fcff69 100644 --- a/tests/ui/macros/macro-outer-attributes.stderr +++ b/tests/ui/macros/macro-outer-attributes.stderr @@ -9,6 +9,17 @@ note: found an item that was configured out | LL | pub fn bar() { }); | ^^^ +note: the item is gated here + --> $DIR/macro-outer-attributes.rs:5:45 + | +LL | $i:item) => (mod $nm { #[$a] $i }); } + | ^^^^^ +LL | +LL | / test!(a, +LL | | #[cfg(FALSE)], +LL | | pub fn bar() { }); + | |_______________________- in this macro invocation + = note: this error originates in the macro `test` (in Nightly builds, run with -Z macro-backtrace for more info) help: consider importing this function | LL + use b::bar; From 548394541b4bbd9ebfe031d9c3a8c57912a535be Mon Sep 17 00:00:00 2001 From: Oneirical Date: Tue, 9 Jul 2024 11:40:02 -0400 Subject: [PATCH 12/21] rewrite dump-ice-to-disk to rmake --- .../tidy/src/allowed_run_make_makefiles.txt | 1 - tests/run-make/dump-ice-to-disk/Makefile | 10 --- tests/run-make/dump-ice-to-disk/check.sh | 64 ------------------- .../dump-ice-to-disk/{src => }/lib.rs | 0 tests/run-make/dump-ice-to-disk/rmake.rs | 64 +++++++++++++++++++ 5 files changed, 64 insertions(+), 75 deletions(-) delete mode 100644 tests/run-make/dump-ice-to-disk/Makefile delete mode 100644 tests/run-make/dump-ice-to-disk/check.sh rename tests/run-make/dump-ice-to-disk/{src => }/lib.rs (100%) create mode 100644 tests/run-make/dump-ice-to-disk/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 2e26f9344b899..4902656f07384 100644 --- a/src/tools/tidy/src/allowed_run_make_makefiles.txt +++ b/src/tools/tidy/src/allowed_run_make_makefiles.txt @@ -19,7 +19,6 @@ run-make/cross-lang-lto/Makefile run-make/dep-info-doesnt-run-much/Makefile run-make/dep-info-spaces/Makefile run-make/dep-info/Makefile -run-make/dump-ice-to-disk/Makefile run-make/emit-to-stdout/Makefile run-make/export-executable-symbols/Makefile run-make/extern-diff-internal-name/Makefile diff --git a/tests/run-make/dump-ice-to-disk/Makefile b/tests/run-make/dump-ice-to-disk/Makefile deleted file mode 100644 index 23006fc09e2f3..0000000000000 --- a/tests/run-make/dump-ice-to-disk/Makefile +++ /dev/null @@ -1,10 +0,0 @@ -include ../tools.mk - -# ignore-windows - -export RUSTC := $(RUSTC_ORIGINAL) -export LD_LIBRARY_PATH := $(HOST_RPATH_DIR) -export TMPDIR := $(TMPDIR) - -all: - bash check.sh diff --git a/tests/run-make/dump-ice-to-disk/check.sh b/tests/run-make/dump-ice-to-disk/check.sh deleted file mode 100644 index ff6e4be35af9d..0000000000000 --- a/tests/run-make/dump-ice-to-disk/check.sh +++ /dev/null @@ -1,64 +0,0 @@ -#!/bin/sh - -# Default nightly behavior (write ICE to current directory) -# FIXME(estebank): these are failing on CI, but passing locally. -# $RUSTC src/lib.rs -Z treat-err-as-bug=1 1>$TMPDIR/rust-test-default.log 2>&1 -# default=$(cat ./rustc-ice-*.txt | wc -l) -# rm ./rustc-ice-*.txt - -# Explicit directory set -export RUSTC_ICE=$TMPDIR -$RUSTC src/lib.rs -Z treat-err-as-bug=1 1>$TMPDIR/rust-test-default-set.log 2>&1 -default_set=$(cat $TMPDIR/rustc-ice-*.txt | wc -l) -content=$(cat $TMPDIR/rustc-ice-*.txt) -# Ensure that the ICE dump path doesn't contain `:` because they cause problems on Windows -windows_safe=$(echo rustc-ice-*.txt | grep ':') -if [ ! -z "$windows_safe" ]; then - exit 1 -fi - -rm $TMPDIR/rustc-ice-*.txt -RUST_BACKTRACE=short $RUSTC src/lib.rs -Z treat-err-as-bug=1 1>$TMPDIR/rust-test-short.log 2>&1 -short=$(cat $TMPDIR/rustc-ice-*.txt | wc -l) -rm $TMPDIR/rustc-ice-*.txt -RUST_BACKTRACE=full $RUSTC src/lib.rs -Z treat-err-as-bug=1 1>$TMPDIR/rust-test-full.log 2>&1 -full=$(cat $TMPDIR/rustc-ice-*.txt | wc -l) -rm $TMPDIR/rustc-ice-*.txt - -# Explicitly disabling ICE dump -export RUSTC_ICE=0 -$RUSTC src/lib.rs -Z treat-err-as-bug=1 1>$TMPDIR/rust-test-disabled.log 2>&1 -should_be_empty_tmp=$(ls -l $TMPDIR/rustc-ice-*.txt 2>/dev/null | wc -l) -should_be_empty_dot=$(ls -l ./rustc-ice-*.txt 2>/dev/null | wc -l) - -echo "#### ICE Dump content:" -echo $content -echo "#### default length:" -echo $default -echo "#### short length:" -echo $short -echo "#### default_set length:" -echo $default_set -echo "#### full length:" -echo $full -echo "#### should_be_empty_dot length:" -echo $should_be_empty_dot -echo "#### should_be_empty_tmp length:" -echo $should_be_empty_tmp - -## Verify that a the ICE dump file is created in the appropriate directories, that -## their lengths are the same regardless of other backtrace configuration options, -## that the file is not created when asked to (RUSTC_ICE=0) and that the file -## contains at least part of the expected content. -if [ $short -eq $default_set ] && - #[ $default -eq $short ] && - [ $default_set -eq $full ] && - [[ $content == *"thread 'rustc' panicked at "* ]] && - [[ $content == *"stack backtrace:"* ]] && - #[ $default -gt 0 ] && - [ $should_be_empty_dot -eq 0 ] && - [ $should_be_empty_tmp -eq 0 ]; then - exit 0 -else - exit 1 -fi diff --git a/tests/run-make/dump-ice-to-disk/src/lib.rs b/tests/run-make/dump-ice-to-disk/lib.rs similarity index 100% rename from tests/run-make/dump-ice-to-disk/src/lib.rs rename to tests/run-make/dump-ice-to-disk/lib.rs diff --git a/tests/run-make/dump-ice-to-disk/rmake.rs b/tests/run-make/dump-ice-to-disk/rmake.rs new file mode 100644 index 0000000000000..95f9223452c65 --- /dev/null +++ b/tests/run-make/dump-ice-to-disk/rmake.rs @@ -0,0 +1,64 @@ +// This test checks if internal compilation error (ICE) log files work as expected. +// - Get the number of lines from the log files without any configuration options, +// then check that the line count doesn't change if the backtrace gets configured to be short +// or full. +// - Check that disabling ICE logging results in zero files created. +// - Check that the ICE files contain some of the expected strings. +// See https://github.com/rust-lang/rust/pull/108714 + +// FIXME(Oneirical): try it on Windows! + +use run_make_support::{cwd, fs_wrapper, has_extension, has_prefix, rustc, shallow_find_files}; + +fn main() { + rustc().input("lib.rs").arg("-Ztreat-err-as-bug=1").run_fail(); + let ice_text = get_text_from_ice(); + let default_set = ice_text.lines().count(); + let content = ice_text; + // Ensure that the ICE files don't contain `:` in their filename because + // this causes problems on Windows. + for file in shallow_find_files(cwd(), |path| { + has_prefix(path, "rustc-ice") && has_extension(path, "txt") + }) { + assert!(!file.display().to_string().contains(":")); + } + + clear_ice_files(); + rustc().input("lib.rs").env("RUST_BACKTRACE", "short").arg("-Ztreat-err-as-bug=1").run_fail(); + let short = get_text_from_ice().lines().count(); + clear_ice_files(); + rustc().input("lib.rs").env("RUST_BACKTRACE", "full").arg("-Ztreat-err-as-bug=1").run_fail(); + let full = get_text_from_ice().lines().count(); + clear_ice_files(); + + // The ICE dump is explicitely disabled. Therefore, this should produce no files. + rustc().env("RUSTC_ICE", "0").input("lib.rs").arg("-Ztreat-err-as-bug=1").run_fail(); + assert!(get_text_from_ice().is_empty()); + + // The line count should not change. + assert_eq!(short, default_set); + assert_eq!(full, default_set); + // Some of the expected strings in an ICE file should appear. + assert!(content.contains("thread 'rustc' panicked at")); + assert!(content.contains("stack backtrace:")); +} + +fn clear_ice_files() { + let ice_files = shallow_find_files(cwd(), |path| { + has_prefix(path, "rustc-ice") && has_extension(path, "txt") + }); + for file in ice_files { + fs_wrapper::remove_file(file); + } +} + +fn get_text_from_ice() -> String { + let ice_files = shallow_find_files(cwd(), |path| { + has_prefix(path, "rustc-ice") && has_extension(path, "txt") + }); + let mut output = String::new(); + for file in ice_files { + output.push_str(&fs_wrapper::read_to_string(file)); + } + output +} From f54fa62887b24a9a9d0cae4251b0498fae1dd725 Mon Sep 17 00:00:00 2001 From: Oneirical Date: Tue, 9 Jul 2024 11:55:54 -0400 Subject: [PATCH 13/21] rewrite panic-abort-eh_frame to rmake --- .../tidy/src/allowed_run_make_makefiles.txt | 1 - tests/run-make/dump-ice-to-disk/rmake.rs | 67 ++++++++++++------- tests/run-make/panic-abort-eh_frame/Makefile | 10 --- tests/run-make/panic-abort-eh_frame/rmake.rs | 20 ++++++ 4 files changed, 62 insertions(+), 36 deletions(-) delete mode 100644 tests/run-make/panic-abort-eh_frame/Makefile create mode 100644 tests/run-make/panic-abort-eh_frame/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 4902656f07384..4313e6472603e 100644 --- a/src/tools/tidy/src/allowed_run_make_makefiles.txt +++ b/src/tools/tidy/src/allowed_run_make_makefiles.txt @@ -81,7 +81,6 @@ run-make/native-link-modifier-whole-archive/Makefile run-make/no-alloc-shim/Makefile run-make/no-builtins-attribute/Makefile run-make/no-duplicate-libs/Makefile -run-make/panic-abort-eh_frame/Makefile run-make/pass-non-c-like-enum-to-c/Makefile run-make/pdb-buildinfo-cl-cmd/Makefile run-make/pgo-gen-lto/Makefile diff --git a/tests/run-make/dump-ice-to-disk/rmake.rs b/tests/run-make/dump-ice-to-disk/rmake.rs index 95f9223452c65..2fb5c825064b0 100644 --- a/tests/run-make/dump-ice-to-disk/rmake.rs +++ b/tests/run-make/dump-ice-to-disk/rmake.rs @@ -6,38 +6,56 @@ // - Check that the ICE files contain some of the expected strings. // See https://github.com/rust-lang/rust/pull/108714 -// FIXME(Oneirical): try it on Windows! - -use run_make_support::{cwd, fs_wrapper, has_extension, has_prefix, rustc, shallow_find_files}; +use run_make_support::{cwd, has_extension, has_prefix, rfs, rustc, shallow_find_files}; fn main() { rustc().input("lib.rs").arg("-Ztreat-err-as-bug=1").run_fail(); - let ice_text = get_text_from_ice(); + let default = get_text_from_ice(".").lines().count(); + clear_ice_files(); + + rustc().env("RUSTC_ICE", cwd()).input("lib.rs").arg("-Ztreat-err-as-bug=1").run_fail(); + let ice_text = get_text_from_ice(cwd()); let default_set = ice_text.lines().count(); let content = ice_text; - // Ensure that the ICE files don't contain `:` in their filename because - // this causes problems on Windows. - for file in shallow_find_files(cwd(), |path| { + let ice_files = shallow_find_files(cwd(), |path| { has_prefix(path, "rustc-ice") && has_extension(path, "txt") - }) { - assert!(!file.display().to_string().contains(":")); - } + }); + assert_eq!(ice_files.len(), 1); // There should only be 1 ICE file. + let ice_file_name = + ice_files.first().and_then(|f| f.file_name()).and_then(|n| n.to_str()).unwrap(); + // Ensure that the ICE dump path doesn't contain `:`, because they cause problems on Windows. + assert!(!ice_file_name.contains(":"), "{ice_file_name}"); clear_ice_files(); - rustc().input("lib.rs").env("RUST_BACKTRACE", "short").arg("-Ztreat-err-as-bug=1").run_fail(); - let short = get_text_from_ice().lines().count(); + rustc() + .env("RUSTC_ICE", cwd()) + .input("lib.rs") + .env("RUST_BACKTRACE", "short") + .arg("-Ztreat-err-as-bug=1") + .run_fail(); + let short = get_text_from_ice(cwd()).lines().count(); clear_ice_files(); - rustc().input("lib.rs").env("RUST_BACKTRACE", "full").arg("-Ztreat-err-as-bug=1").run_fail(); - let full = get_text_from_ice().lines().count(); + rustc() + .env("RUSTC_ICE", cwd()) + .input("lib.rs") + .env("RUST_BACKTRACE", "full") + .arg("-Ztreat-err-as-bug=1") + .run_fail(); + let full = get_text_from_ice(cwd()).lines().count(); clear_ice_files(); - // The ICE dump is explicitely disabled. Therefore, this should produce no files. + // The ICE dump is explicitly disabled. Therefore, this should produce no files. rustc().env("RUSTC_ICE", "0").input("lib.rs").arg("-Ztreat-err-as-bug=1").run_fail(); - assert!(get_text_from_ice().is_empty()); + let ice_files = shallow_find_files(cwd(), |path| { + has_prefix(path, "rustc-ice") && has_extension(path, "txt") + }); + assert!(ice_files.is_empty()); // There should be 0 ICE files. // The line count should not change. assert_eq!(short, default_set); + assert_eq!(short, default); assert_eq!(full, default_set); + assert!(default > 0); // Some of the expected strings in an ICE file should appear. assert!(content.contains("thread 'rustc' panicked at")); assert!(content.contains("stack backtrace:")); @@ -48,17 +66,16 @@ fn clear_ice_files() { has_prefix(path, "rustc-ice") && has_extension(path, "txt") }); for file in ice_files { - fs_wrapper::remove_file(file); + rfs::remove_file(file); } } -fn get_text_from_ice() -> String { - let ice_files = shallow_find_files(cwd(), |path| { - has_prefix(path, "rustc-ice") && has_extension(path, "txt") - }); - let mut output = String::new(); - for file in ice_files { - output.push_str(&fs_wrapper::read_to_string(file)); - } +#[track_caller] +fn get_text_from_ice(dir: impl AsRef) -> String { + let ice_files = + shallow_find_files(dir, |path| has_prefix(path, "rustc-ice") && has_extension(path, "txt")); + assert_eq!(ice_files.len(), 1); // There should only be 1 ICE file. + let ice_file = ice_files.get(0).unwrap(); + let output = rfs::read_to_string(ice_file); output } diff --git a/tests/run-make/panic-abort-eh_frame/Makefile b/tests/run-make/panic-abort-eh_frame/Makefile deleted file mode 100644 index 7020455b742d0..0000000000000 --- a/tests/run-make/panic-abort-eh_frame/Makefile +++ /dev/null @@ -1,10 +0,0 @@ -# only-linux -# -# This test ensures that `panic=abort` code (without `C-unwind`, that is) should not have any -# unwinding related `.eh_frame` sections emitted. - -include ../tools.mk - -all: - $(RUSTC) foo.rs --crate-type=lib --emit=obj=$(TMPDIR)/foo.o -Cpanic=abort --edition 2021 -Z validate-mir - objdump --dwarf=frames $(TMPDIR)/foo.o | $(CGREP) -v 'DW_CFA' diff --git a/tests/run-make/panic-abort-eh_frame/rmake.rs b/tests/run-make/panic-abort-eh_frame/rmake.rs new file mode 100644 index 0000000000000..6bcf718235675 --- /dev/null +++ b/tests/run-make/panic-abort-eh_frame/rmake.rs @@ -0,0 +1,20 @@ +// An `.eh_frame` section in an object file is a symptom of an UnwindAction::Terminate +// being inserted, useful for determining whether or not unwinding is necessary. +// This is useless when panics would NEVER unwind due to -C panic=abort. This section should +// therefore never appear in the emit file of a -C panic=abort compilation, and this test +// checks that this is respected. +// See https://github.com/rust-lang/rust/pull/112403 + +use run_make_support::{llvm_objdump, rustc}; + +fn main() { + rustc() + .input("foo.rs") + .crate_type("lib") + .emit("obj=foo.o") + .panic("abort") + .edition("2021") + .arg("-Zvalidate-mir") + .run(); + llvm_objdump().arg("--dwarf=frames").input("foo.o").run().assert_stdout_not_contains("DW_CFA"); +} From 2e1e6274306bed67291c1eefc7332d0bf972e6ac Mon Sep 17 00:00:00 2001 From: Michael Howell Date: Thu, 18 Jul 2024 12:06:21 -0700 Subject: [PATCH 14/21] rustdoc: fix `current` class on sidebar modnav --- src/librustdoc/html/static/js/main.js | 8 +++++--- tests/rustdoc-gui/sidebar.goml | 4 ++++ 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/src/librustdoc/html/static/js/main.js b/src/librustdoc/html/static/js/main.js index 64c356607788c..116ce615d8c55 100644 --- a/src/librustdoc/html/static/js/main.js +++ b/src/librustdoc/html/static/js/main.js @@ -529,13 +529,15 @@ function preLoadCss(cssUrl) { } const link = document.createElement("a"); link.href = path; - if (path === current_page) { - link.className = "current"; - } link.textContent = name; const li = document.createElement("li"); li.appendChild(link); ul.appendChild(li); + // Don't "optimize" this to just use `path`. + // We want the browser to normalize this into an absolute URL. + if (link.href === current_page) { + li.classList.add("current"); + } } sidebar.appendChild(h3); sidebar.appendChild(ul); diff --git a/tests/rustdoc-gui/sidebar.goml b/tests/rustdoc-gui/sidebar.goml index 56453517a55a2..e499c159c6c76 100644 --- a/tests/rustdoc-gui/sidebar.goml +++ b/tests/rustdoc-gui/sidebar.goml @@ -72,6 +72,7 @@ click: "#structs + .item-table .item-name > a" assert-count: (".sidebar .sidebar-crate", 1) assert-count: (".sidebar .location", 1) assert-count: (".sidebar h2", 3) +assert-text: (".sidebar-elems ul.block > li.current > a", "Foo") // We check that there is no crate listed outside of the top level. assert-false: ".sidebar-elems > .crate" @@ -110,6 +111,7 @@ click: "#functions + .item-table .item-name > a" assert-text: (".sidebar > .sidebar-crate > h2 > a", "lib2") assert-count: (".sidebar .location", 0) assert-count: (".sidebar h2", 1) +assert-text: (".sidebar-elems ul.block > li.current > a", "foobar") // We check that we don't have the crate list. assert-false: ".sidebar-elems > .crate" @@ -118,6 +120,7 @@ assert-property: (".sidebar", {"clientWidth": "200"}) assert-text: (".sidebar > .sidebar-crate > h2 > a", "lib2") assert-text: (".sidebar > .location", "Module module") assert-count: (".sidebar .location", 1) +assert-text: (".sidebar-elems ul.block > li.current > a", "module") // Module page requires three headings: // - Presistent crate branding (name and version) // - Module name, followed by TOC for module headings @@ -138,6 +141,7 @@ assert-text: (".sidebar > .sidebar-elems > h2", "In lib2::module::sub_module") assert-property: (".sidebar > .sidebar-elems > h2 > a", { "href": "/module/sub_module/index.html", }, ENDS_WITH) +assert-text: (".sidebar-elems ul.block > li.current > a", "sub_sub_module") // We check that we don't have the crate list. assert-false: ".sidebar-elems .crate" assert-text: (".sidebar-elems > section ul > li:nth-child(1)", "Functions") From 4cad705017f34a6545deb1505d0ce85537c18df5 Mon Sep 17 00:00:00 2001 From: Veera Date: Thu, 4 Jul 2024 19:19:15 -0400 Subject: [PATCH 15/21] Parser: Suggest Placing the Return Type After Function Parameters --- compiler/rustc_parse/messages.ftl | 2 + compiler/rustc_parse/src/errors.rs | 22 +++- .../rustc_parse/src/parser/diagnostics.rs | 8 +- compiler/rustc_parse/src/parser/item.rs | 124 ++++++++++++++---- compiler/rustc_parse/src/parser/mod.rs | 1 + compiler/rustc_parse/src/parser/ty.rs | 5 + 6 files changed, 123 insertions(+), 39 deletions(-) diff --git a/compiler/rustc_parse/messages.ftl b/compiler/rustc_parse/messages.ftl index f08efe60d96b8..984d402390327 100644 --- a/compiler/rustc_parse/messages.ftl +++ b/compiler/rustc_parse/messages.ftl @@ -526,6 +526,8 @@ parse_mismatched_closing_delimiter = mismatched closing delimiter: `{$delimiter} .label_opening_candidate = closing delimiter possibly meant for this .label_unclosed = unclosed delimiter +parse_misplaced_return_type = place the return type after the function parameters + parse_missing_comma_after_match_arm = expected `,` following `match` arm .suggestion = missing a comma here to end this `match` arm diff --git a/compiler/rustc_parse/src/errors.rs b/compiler/rustc_parse/src/errors.rs index 8d49887f16441..05bc0686e8efe 100644 --- a/compiler/rustc_parse/src/errors.rs +++ b/compiler/rustc_parse/src/errors.rs @@ -1434,6 +1434,20 @@ pub(crate) struct FnPtrWithGenerics { pub sugg: Option, } +#[derive(Subdiagnostic)] +#[multipart_suggestion( + parse_misplaced_return_type, + style = "verbose", + applicability = "maybe-incorrect" +)] +pub(crate) struct MisplacedReturnType { + #[suggestion_part(code = " {snippet}")] + pub fn_params_end: Span, + pub snippet: String, + #[suggestion_part(code = "")] + pub ret_ty_span: Span, +} + #[derive(Subdiagnostic)] #[multipart_suggestion(parse_suggestion, applicability = "maybe-incorrect")] pub(crate) struct FnPtrWithGenericsSugg { @@ -1448,7 +1462,6 @@ pub(crate) struct FnPtrWithGenericsSugg { pub(crate) struct FnTraitMissingParen { pub span: Span, - pub machine_applicable: bool, } impl Subdiagnostic for FnTraitMissingParen { @@ -1458,16 +1471,11 @@ impl Subdiagnostic for FnTraitMissingParen { _: &F, ) { diag.span_label(self.span, crate::fluent_generated::parse_fn_trait_missing_paren); - let applicability = if self.machine_applicable { - Applicability::MachineApplicable - } else { - Applicability::MaybeIncorrect - }; diag.span_suggestion_short( self.span.shrink_to_hi(), crate::fluent_generated::parse_add_paren, "()", - applicability, + Applicability::MachineApplicable, ); } } diff --git a/compiler/rustc_parse/src/parser/diagnostics.rs b/compiler/rustc_parse/src/parser/diagnostics.rs index 81d5f0fca0ec9..aecedc7d1c879 100644 --- a/compiler/rustc_parse/src/parser/diagnostics.rs +++ b/compiler/rustc_parse/src/parser/diagnostics.rs @@ -430,7 +430,7 @@ impl<'a> Parser<'a> { &mut self, edible: &[TokenKind], inedible: &[TokenKind], - ) -> PResult<'a, Recovered> { + ) -> PResult<'a, ErrorGuaranteed> { debug!("expected_one_of_not_found(edible: {:?}, inedible: {:?})", edible, inedible); fn tokens_to_string(tokens: &[TokenType]) -> String { let mut i = tokens.iter(); @@ -533,7 +533,7 @@ impl<'a> Parser<'a> { sugg: ExpectedSemiSugg::ChangeToSemi(self.token.span), }); self.bump(); - return Ok(Recovered::Yes(guar)); + return Ok(guar); } else if self.look_ahead(0, |t| { t == &token::CloseDelim(Delimiter::Brace) || ((t.can_begin_expr() || t.can_begin_item()) @@ -557,7 +557,7 @@ impl<'a> Parser<'a> { unexpected_token_label: Some(self.token.span), sugg: ExpectedSemiSugg::AddSemi(span), }); - return Ok(Recovered::Yes(guar)); + return Ok(guar); } } @@ -712,7 +712,7 @@ impl<'a> Parser<'a> { if self.check_too_many_raw_str_terminators(&mut err) { if expected.contains(&TokenType::Token(token::Semi)) && self.eat(&token::Semi) { let guar = err.emit(); - return Ok(Recovered::Yes(guar)); + return Ok(guar); } else { return Err(err); } diff --git a/compiler/rustc_parse/src/parser/item.rs b/compiler/rustc_parse/src/parser/item.rs index abb6b51cebd68..077f498a2e02a 100644 --- a/compiler/rustc_parse/src/parser/item.rs +++ b/compiler/rustc_parse/src/parser/item.rs @@ -19,6 +19,7 @@ use rustc_span::edit_distance::edit_distance; use rustc_span::edition::Edition; use rustc_span::source_map; use rustc_span::symbol::{kw, sym, Ident, Symbol}; +use rustc_span::ErrorGuaranteed; use rustc_span::{Span, DUMMY_SP}; use std::fmt::Write; use std::mem; @@ -2332,14 +2333,106 @@ impl<'a> Parser<'a> { } } }; + + // Store the end of function parameters to give better diagnostics + // inside `parse_fn_body()`. + let fn_params_end = self.prev_token.span.shrink_to_hi(); + generics.where_clause = self.parse_where_clause()?; // `where T: Ord` + // `fn_params_end` is needed only when it's followed by a where clause. + let fn_params_end = + if generics.where_clause.has_where_token { Some(fn_params_end) } else { None }; + let mut sig_hi = self.prev_token.span; - let body = self.parse_fn_body(attrs, &ident, &mut sig_hi, fn_parse_mode.req_body)?; // `;` or `{ ... }`. + // Either `;` or `{ ... }`. + let body = + self.parse_fn_body(attrs, &ident, &mut sig_hi, fn_parse_mode.req_body, fn_params_end)?; let fn_sig_span = sig_lo.to(sig_hi); Ok((ident, FnSig { header, decl, span: fn_sig_span }, generics, body)) } + /// Provide diagnostics when function body is not found + fn error_fn_body_not_found( + &mut self, + ident_span: Span, + req_body: bool, + fn_params_end: Option, + ) -> PResult<'a, ErrorGuaranteed> { + let expected = if req_body { + &[token::OpenDelim(Delimiter::Brace)][..] + } else { + &[token::Semi, token::OpenDelim(Delimiter::Brace)] + }; + match self.expected_one_of_not_found(&[], expected) { + Ok(error_guaranteed) => Ok(error_guaranteed), + Err(mut err) => { + if self.token.kind == token::CloseDelim(Delimiter::Brace) { + // The enclosing `mod`, `trait` or `impl` is being closed, so keep the `fn` in + // the AST for typechecking. + err.span_label(ident_span, "while parsing this `fn`"); + Ok(err.emit()) + } else if self.token.kind == token::RArrow + && let Some(fn_params_end) = fn_params_end + { + // Instead of a function body, the parser has encountered a right arrow + // preceded by a where clause. + + // Find whether token behind the right arrow is a function trait and + // store its span. + let fn_trait_span = + [sym::FnOnce, sym::FnMut, sym::Fn].into_iter().find_map(|symbol| { + if self.prev_token.is_ident_named(symbol) { + Some(self.prev_token.span) + } else { + None + } + }); + + // Parse the return type (along with the right arrow) and store its span. + // If there's a parse error, cancel it and return the existing error + // as we are primarily concerned with the + // expected-function-body-but-found-something-else error here. + let arrow_span = self.token.span; + let ty_span = match self.parse_ret_ty( + AllowPlus::Yes, + RecoverQPath::Yes, + RecoverReturnSign::Yes, + ) { + Ok(ty_span) => ty_span.span().shrink_to_hi(), + Err(parse_error) => { + parse_error.cancel(); + return Err(err); + } + }; + let ret_ty_span = arrow_span.to(ty_span); + + if let Some(fn_trait_span) = fn_trait_span { + // Typo'd Fn* trait bounds such as + // fn foo() where F: FnOnce -> () {} + err.subdiagnostic(errors::FnTraitMissingParen { span: fn_trait_span }); + } else if let Ok(snippet) = self.psess.source_map().span_to_snippet(ret_ty_span) + { + // If token behind right arrow is not a Fn* trait, the programmer + // probably misplaced the return type after the where clause like + // `fn foo() where T: Default -> u8 {}` + err.primary_message( + "return type should be specified after the function parameters", + ); + err.subdiagnostic(errors::MisplacedReturnType { + fn_params_end, + snippet, + ret_ty_span, + }); + } + Err(err) + } else { + Err(err) + } + } + } + } + /// Parse the "body" of a function. /// This can either be `;` when there's no body, /// or e.g. a block when the function is a provided one. @@ -2349,6 +2442,7 @@ impl<'a> Parser<'a> { ident: &Ident, sig_hi: &mut Span, req_body: bool, + fn_params_end: Option, ) -> PResult<'a, Option>> { let has_semi = if req_body { self.token.kind == TokenKind::Semi @@ -2377,33 +2471,7 @@ impl<'a> Parser<'a> { }); (AttrVec::new(), Some(self.mk_block_err(span, guar))) } else { - let expected = if req_body { - &[token::OpenDelim(Delimiter::Brace)][..] - } else { - &[token::Semi, token::OpenDelim(Delimiter::Brace)] - }; - if let Err(mut err) = self.expected_one_of_not_found(&[], expected) { - if self.token.kind == token::CloseDelim(Delimiter::Brace) { - // The enclosing `mod`, `trait` or `impl` is being closed, so keep the `fn` in - // the AST for typechecking. - err.span_label(ident.span, "while parsing this `fn`"); - err.emit(); - } else { - // check for typo'd Fn* trait bounds such as - // fn foo() where F: FnOnce -> () {} - if self.token.kind == token::RArrow { - let machine_applicable = [sym::FnOnce, sym::FnMut, sym::Fn] - .into_iter() - .any(|s| self.prev_token.is_ident_named(s)); - - err.subdiagnostic(errors::FnTraitMissingParen { - span: self.prev_token.span, - machine_applicable, - }); - } - return Err(err); - } - } + self.error_fn_body_not_found(ident.span, req_body, fn_params_end)?; (AttrVec::new(), None) }; attrs.extend(inner_attrs); diff --git a/compiler/rustc_parse/src/parser/mod.rs b/compiler/rustc_parse/src/parser/mod.rs index cfd0a72c056d5..c3c35f8e6c680 100644 --- a/compiler/rustc_parse/src/parser/mod.rs +++ b/compiler/rustc_parse/src/parser/mod.rs @@ -506,6 +506,7 @@ impl<'a> Parser<'a> { FatalError.raise(); } else { self.expected_one_of_not_found(edible, inedible) + .map(|error_guaranteed| Recovered::Yes(error_guaranteed)) } } diff --git a/compiler/rustc_parse/src/parser/ty.rs b/compiler/rustc_parse/src/parser/ty.rs index 1e5b227aaa9be..17a0829c6d2f6 100644 --- a/compiler/rustc_parse/src/parser/ty.rs +++ b/compiler/rustc_parse/src/parser/ty.rs @@ -21,6 +21,11 @@ use rustc_span::symbol::{kw, sym, Ident}; use rustc_span::{ErrorGuaranteed, Span, Symbol}; use thin_vec::{thin_vec, ThinVec}; +/// Signals whether parsing a type should allow `+`. +/// +/// For example, let T be the type `impl Default + 'static` +/// With `AllowPlus::Yes`, T will be parsed successfully +/// With `AllowPlus::No`, parsing T will return a parse error #[derive(Copy, Clone, PartialEq)] pub(super) enum AllowPlus { Yes, From 2f5a84ea16f232f8a0709ea567bca0cfd90b44cf Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Thu, 18 Jul 2024 18:02:09 -0400 Subject: [PATCH 16/21] Don't allow unsafe statics outside of extern blocks --- compiler/rustc_ast_passes/messages.ftl | 3 +++ compiler/rustc_ast_passes/src/ast_validation.rs | 8 ++++++++ compiler/rustc_ast_passes/src/errors.rs | 7 +++++++ tests/ui/rust-2024/safe-outside-extern.gated.stderr | 8 +++++++- tests/ui/rust-2024/safe-outside-extern.rs | 3 +++ tests/ui/rust-2024/safe-outside-extern.ungated.stderr | 8 +++++++- 6 files changed, 35 insertions(+), 2 deletions(-) diff --git a/compiler/rustc_ast_passes/messages.ftl b/compiler/rustc_ast_passes/messages.ftl index 02bdff96aa6d0..8f7dd77420709 100644 --- a/compiler/rustc_ast_passes/messages.ftl +++ b/compiler/rustc_ast_passes/messages.ftl @@ -269,6 +269,9 @@ ast_passes_unsafe_negative_impl = negative impls cannot be unsafe .negative = negative because of this .unsafe = unsafe because of this +ast_passes_unsafe_static = + static items cannot be declared with `unsafe` safety qualifier outside of `extern` block + ast_passes_visibility_not_permitted = visibility qualifiers are not permitted here .enum_variant = enum variants and their fields always share the visibility of the enum they are in diff --git a/compiler/rustc_ast_passes/src/ast_validation.rs b/compiler/rustc_ast_passes/src/ast_validation.rs index 83249dea82a3e..34aac6e447304 100644 --- a/compiler/rustc_ast_passes/src/ast_validation.rs +++ b/compiler/rustc_ast_passes/src/ast_validation.rs @@ -438,6 +438,11 @@ impl<'a> AstValidator<'a> { } } + /// This ensures that items can only be `unsafe` (or unmarked) outside of extern + /// blocks. + /// + /// This additionally ensures that within extern blocks, items can only be + /// `safe`/`unsafe` inside of a `unsafe`-adorned extern block. fn check_item_safety(&self, span: Span, safety: Safety) { match self.extern_mod_safety { Some(extern_safety) => { @@ -1177,6 +1182,9 @@ impl<'a> Visitor<'a> for AstValidator<'a> { } ItemKind::Static(box StaticItem { expr, safety, .. }) => { self.check_item_safety(item.span, *safety); + if matches!(safety, Safety::Unsafe(_)) { + self.dcx().emit_err(errors::UnsafeStatic { span: item.span }); + } if expr.is_none() { self.dcx().emit_err(errors::StaticWithoutBody { diff --git a/compiler/rustc_ast_passes/src/errors.rs b/compiler/rustc_ast_passes/src/errors.rs index 460da254653c6..783bca6b6958d 100644 --- a/compiler/rustc_ast_passes/src/errors.rs +++ b/compiler/rustc_ast_passes/src/errors.rs @@ -224,6 +224,13 @@ pub struct InvalidSafetyOnBareFn { pub span: Span, } +#[derive(Diagnostic)] +#[diag(ast_passes_unsafe_static)] +pub struct UnsafeStatic { + #[primary_span] + pub span: Span, +} + #[derive(Diagnostic)] #[diag(ast_passes_bound_in_context)] pub struct BoundInContext<'a> { diff --git a/tests/ui/rust-2024/safe-outside-extern.gated.stderr b/tests/ui/rust-2024/safe-outside-extern.gated.stderr index 18a3361f35b8e..e0b218281f365 100644 --- a/tests/ui/rust-2024/safe-outside-extern.gated.stderr +++ b/tests/ui/rust-2024/safe-outside-extern.gated.stderr @@ -28,5 +28,11 @@ error: function pointers cannot be declared with `safe` safety qualifier LL | type FnPtr = safe fn(i32, i32) -> i32; | ^^^^^^^^^^^^^^^^^^^^^^^^ -error: aborting due to 5 previous errors +error: static items cannot be declared with `unsafe` safety qualifier outside of `extern` block + --> $DIR/safe-outside-extern.rs:28:1 + | +LL | unsafe static LOL: u8 = 0; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: aborting due to 6 previous errors diff --git a/tests/ui/rust-2024/safe-outside-extern.rs b/tests/ui/rust-2024/safe-outside-extern.rs index 9ec0c5c70e19e..6773df5ef03b7 100644 --- a/tests/ui/rust-2024/safe-outside-extern.rs +++ b/tests/ui/rust-2024/safe-outside-extern.rs @@ -25,4 +25,7 @@ type FnPtr = safe fn(i32, i32) -> i32; //~^ ERROR: function pointers cannot be declared with `safe` safety qualifier //[ungated]~| ERROR: unsafe extern {}` blocks and `safe` keyword are experimental [E0658] +unsafe static LOL: u8 = 0; +//~^ ERROR: static items cannot be declared with `unsafe` safety qualifier outside of `extern` block + fn main() {} diff --git a/tests/ui/rust-2024/safe-outside-extern.ungated.stderr b/tests/ui/rust-2024/safe-outside-extern.ungated.stderr index 9ea6d451e8c47..98a4c0eab921a 100644 --- a/tests/ui/rust-2024/safe-outside-extern.ungated.stderr +++ b/tests/ui/rust-2024/safe-outside-extern.ungated.stderr @@ -28,6 +28,12 @@ error: function pointers cannot be declared with `safe` safety qualifier LL | type FnPtr = safe fn(i32, i32) -> i32; | ^^^^^^^^^^^^^^^^^^^^^^^^ +error: static items cannot be declared with `unsafe` safety qualifier outside of `extern` block + --> $DIR/safe-outside-extern.rs:28:1 + | +LL | unsafe static LOL: u8 = 0; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ + error[E0658]: `unsafe extern {}` blocks and `safe` keyword are experimental --> $DIR/safe-outside-extern.rs:4:1 | @@ -78,6 +84,6 @@ LL | type FnPtr = safe fn(i32, i32) -> i32; = help: add `#![feature(unsafe_extern_blocks)]` to the crate attributes to enable = note: this compiler was built on YYYY-MM-DD; consider upgrading it if it is out of date -error: aborting due to 10 previous errors +error: aborting due to 11 previous errors For more information about this error, try `rustc --explain E0658`. From e69ff1c1065933b3737e662f8f237ffbee964755 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Wed, 17 Jul 2024 02:21:39 +1000 Subject: [PATCH 17/21] Remove an unnecessary `ForceCollect::Yes`. No need to collect tokens on this recovery path, because the parsed statement isn't even looked at. --- compiler/rustc_parse/src/parser/mod.rs | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/compiler/rustc_parse/src/parser/mod.rs b/compiler/rustc_parse/src/parser/mod.rs index c03527acb2cfe..77965921fdf78 100644 --- a/compiler/rustc_parse/src/parser/mod.rs +++ b/compiler/rustc_parse/src/parser/mod.rs @@ -942,11 +942,10 @@ impl<'a> Parser<'a> { let initial_semicolon = self.token.span; while self.eat(&TokenKind::Semi) { - let _ = - self.parse_stmt_without_recovery(false, ForceCollect::Yes).unwrap_or_else(|e| { - e.cancel(); - None - }); + let _ = self.parse_stmt_without_recovery(false, ForceCollect::No).unwrap_or_else(|e| { + e.cancel(); + None + }); } expect_err From 7d7e2a173aac29bd89435096347613f445a5202f Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Wed, 17 Jul 2024 02:27:01 +1000 Subject: [PATCH 18/21] Don't always force collect tokens in `recover_stmt_local_after_let`. Use a parameter to decide whether to force collect, as is done for the closely related `parse_local_mk` method. --- compiler/rustc_parse/src/parser/stmt.rs | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/compiler/rustc_parse/src/parser/stmt.rs b/compiler/rustc_parse/src/parser/stmt.rs index 3ec891b4eea34..69dd3a3d65aac 100644 --- a/compiler/rustc_parse/src/parser/stmt.rs +++ b/compiler/rustc_parse/src/parser/stmt.rs @@ -72,6 +72,7 @@ impl<'a> Parser<'a> { lo, attrs, errors::InvalidVariableDeclarationSub::MissingLet, + force_collect, )? } else if self.is_kw_followed_by_ident(kw::Auto) && self.may_recover() { self.bump(); // `auto` @@ -79,6 +80,7 @@ impl<'a> Parser<'a> { lo, attrs, errors::InvalidVariableDeclarationSub::UseLetNotAuto, + force_collect, )? } else if self.is_kw_followed_by_ident(sym::var) && self.may_recover() { self.bump(); // `var` @@ -86,6 +88,7 @@ impl<'a> Parser<'a> { lo, attrs, errors::InvalidVariableDeclarationSub::UseLetNotVar, + force_collect, )? } else if self.check_path() && !self.token.is_qpath_start() @@ -231,13 +234,13 @@ impl<'a> Parser<'a> { lo: Span, attrs: AttrWrapper, subdiagnostic: fn(Span) -> errors::InvalidVariableDeclarationSub, + force_collect: ForceCollect, ) -> PResult<'a, Stmt> { - let stmt = - self.collect_tokens_trailing_token(attrs, ForceCollect::Yes, |this, attrs| { - let local = this.parse_local(attrs)?; - // FIXME - maybe capture semicolon in recovery? - Ok((this.mk_stmt(lo.to(this.prev_token.span), StmtKind::Let(local)), false)) - })?; + let stmt = self.collect_tokens_trailing_token(attrs, force_collect, |this, attrs| { + let local = this.parse_local(attrs)?; + // FIXME - maybe capture semicolon in recovery? + Ok((this.mk_stmt(lo.to(this.prev_token.span), StmtKind::Let(local)), false)) + })?; self.dcx() .emit_err(errors::InvalidVariableDeclaration { span: lo, sub: subdiagnostic(lo) }); Ok(stmt) From 9d908a2877ec54101e3f4c4b7d5e13facba9d3e0 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Wed, 17 Jul 2024 13:14:35 +1000 Subject: [PATCH 19/21] Use `ForceCollect` in `parse_attr_item`. Instead of a `bool`. Because `ForceCollect` is used in this way everywhere else. --- .../rustc_builtin_macros/src/cmdline_attrs.rs | 16 +++++++++------- compiler/rustc_parse/src/parser/attr.rs | 11 +++++++---- compiler/rustc_parse/src/parser/nonterminal.rs | 2 +- 3 files changed, 17 insertions(+), 12 deletions(-) diff --git a/compiler/rustc_builtin_macros/src/cmdline_attrs.rs b/compiler/rustc_builtin_macros/src/cmdline_attrs.rs index 58928815e8930..bffd5672b9b0c 100644 --- a/compiler/rustc_builtin_macros/src/cmdline_attrs.rs +++ b/compiler/rustc_builtin_macros/src/cmdline_attrs.rs @@ -4,6 +4,7 @@ use crate::errors; use rustc_ast::attr::mk_attr; use rustc_ast::token; use rustc_ast::{self as ast, AttrItem, AttrStyle}; +use rustc_parse::parser::ForceCollect; use rustc_parse::{new_parser_from_source_str, unwrap_or_emit_fatal}; use rustc_session::parse::ParseSess; use rustc_span::FileName; @@ -17,13 +18,14 @@ pub fn inject(krate: &mut ast::Crate, psess: &ParseSess, attrs: &[String]) { )); let start_span = parser.token.span; - let AttrItem { unsafety, path, args, tokens: _ } = match parser.parse_attr_item(false) { - Ok(ai) => ai, - Err(err) => { - err.emit(); - continue; - } - }; + let AttrItem { unsafety, path, args, tokens: _ } = + match parser.parse_attr_item(ForceCollect::No) { + Ok(ai) => ai, + Err(err) => { + err.emit(); + continue; + } + }; let end_span = parser.token.span; if parser.token != token::Eof { psess.dcx().emit_err(errors::InvalidCrateAttr { span: start_span.to(end_span) }); diff --git a/compiler/rustc_parse/src/parser/attr.rs b/compiler/rustc_parse/src/parser/attr.rs index a8fe35f45b31e..347cef86d981d 100644 --- a/compiler/rustc_parse/src/parser/attr.rs +++ b/compiler/rustc_parse/src/parser/attr.rs @@ -124,7 +124,7 @@ impl<'a> Parser<'a> { if this.eat(&token::Not) { ast::AttrStyle::Inner } else { ast::AttrStyle::Outer }; this.expect(&token::OpenDelim(Delimiter::Bracket))?; - let item = this.parse_attr_item(false)?; + let item = this.parse_attr_item(ForceCollect::No)?; this.expect(&token::CloseDelim(Delimiter::Bracket))?; let attr_sp = lo.to(this.prev_token.span); @@ -248,7 +248,7 @@ impl<'a> Parser<'a> { /// PATH /// PATH `=` UNSUFFIXED_LIT /// The delimiters or `=` are still put into the resulting token stream. - pub fn parse_attr_item(&mut self, capture_tokens: bool) -> PResult<'a, ast::AttrItem> { + pub fn parse_attr_item(&mut self, force_collect: ForceCollect) -> PResult<'a, ast::AttrItem> { maybe_whole!(self, NtMeta, |attr| attr.into_inner()); let do_parse = |this: &mut Self| { @@ -271,7 +271,10 @@ impl<'a> Parser<'a> { Ok(ast::AttrItem { unsafety, path, args, tokens: None }) }; // Attr items don't have attributes - if capture_tokens { self.collect_tokens_no_attrs(do_parse) } else { do_parse(self) } + match force_collect { + ForceCollect::Yes => self.collect_tokens_no_attrs(do_parse), + ForceCollect::No => do_parse(self), + } } /// Parses attributes that appear after the opening of an item. These should @@ -344,7 +347,7 @@ impl<'a> Parser<'a> { let mut expanded_attrs = Vec::with_capacity(1); while self.token.kind != token::Eof { let lo = self.token.span; - let item = self.parse_attr_item(true)?; + let item = self.parse_attr_item(ForceCollect::Yes)?; expanded_attrs.push((item, lo.to(self.prev_token.span))); if !self.eat(&token::Comma) { break; diff --git a/compiler/rustc_parse/src/parser/nonterminal.rs b/compiler/rustc_parse/src/parser/nonterminal.rs index 41e31d76d62d5..886d6af173535 100644 --- a/compiler/rustc_parse/src/parser/nonterminal.rs +++ b/compiler/rustc_parse/src/parser/nonterminal.rs @@ -171,7 +171,7 @@ impl<'a> Parser<'a> { NonterminalKind::Path => { NtPath(P(self.collect_tokens_no_attrs(|this| this.parse_path(PathStyle::Type))?)) } - NonterminalKind::Meta => NtMeta(P(self.parse_attr_item(true)?)), + NonterminalKind::Meta => NtMeta(P(self.parse_attr_item(ForceCollect::Yes)?)), NonterminalKind::Vis => { NtVis(P(self .collect_tokens_no_attrs(|this| this.parse_visibility(FollowedByType::Yes))?)) From 4158a1c48fb19bfce31e4001593faa2353b7201b Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Wed, 17 Jul 2024 14:02:37 +1000 Subject: [PATCH 20/21] Only check `force_collect` in `collect_tokens_trailing_token`. There are three places where we currently check `force_collect` and call `collect_tokens_no_attrs` for `ForceCollect::Yes` and a vanilla parsing function for `ForceCollect::No`. But we can instead just pass in `force_collect` and let `collect_tokens_trailing_token` do the appropriate thing. --- compiler/rustc_parse/src/parser/attr.rs | 12 +++------ compiler/rustc_parse/src/parser/stmt.rs | 33 +++++++++++++------------ 2 files changed, 21 insertions(+), 24 deletions(-) diff --git a/compiler/rustc_parse/src/parser/attr.rs b/compiler/rustc_parse/src/parser/attr.rs index 347cef86d981d..0922150870a74 100644 --- a/compiler/rustc_parse/src/parser/attr.rs +++ b/compiler/rustc_parse/src/parser/attr.rs @@ -251,13 +251,12 @@ impl<'a> Parser<'a> { pub fn parse_attr_item(&mut self, force_collect: ForceCollect) -> PResult<'a, ast::AttrItem> { maybe_whole!(self, NtMeta, |attr| attr.into_inner()); - let do_parse = |this: &mut Self| { + let do_parse = |this: &mut Self, _empty_attrs| { let is_unsafe = this.eat_keyword(kw::Unsafe); let unsafety = if is_unsafe { let unsafe_span = this.prev_token.span; this.psess.gated_spans.gate(sym::unsafe_attributes, unsafe_span); this.expect(&token::OpenDelim(Delimiter::Parenthesis))?; - ast::Safety::Unsafe(unsafe_span) } else { ast::Safety::Default @@ -268,13 +267,10 @@ impl<'a> Parser<'a> { if is_unsafe { this.expect(&token::CloseDelim(Delimiter::Parenthesis))?; } - Ok(ast::AttrItem { unsafety, path, args, tokens: None }) + Ok((ast::AttrItem { unsafety, path, args, tokens: None }, false)) }; - // Attr items don't have attributes - match force_collect { - ForceCollect::Yes => self.collect_tokens_no_attrs(do_parse), - ForceCollect::No => do_parse(self), - } + // Attr items don't have attributes. + self.collect_tokens_trailing_token(AttrWrapper::empty(), force_collect, do_parse) } /// Parses attributes that appear after the opening of an item. These should diff --git a/compiler/rustc_parse/src/parser/stmt.rs b/compiler/rustc_parse/src/parser/stmt.rs index 69dd3a3d65aac..d8de7c1bfa1c5 100644 --- a/compiler/rustc_parse/src/parser/stmt.rs +++ b/compiler/rustc_parse/src/parser/stmt.rs @@ -99,17 +99,17 @@ impl<'a> Parser<'a> { // or `auto trait` items. We aim to parse an arbitrary path `a::b` but not something // that starts like a path (1 token), but it fact not a path. // Also, we avoid stealing syntax from `parse_item_`. - match force_collect { - ForceCollect::Yes => { - self.collect_tokens_no_attrs(|this| this.parse_stmt_path_start(lo, attrs))? + let stmt = self.collect_tokens_trailing_token( + AttrWrapper::empty(), + force_collect, + |this, _empty_attrs| Ok((this.parse_stmt_path_start(lo, attrs)?, false)), + ); + match stmt { + Ok(stmt) => stmt, + Err(mut err) => { + self.suggest_add_missing_let_for_stmt(&mut err); + return Err(err); } - ForceCollect::No => match self.parse_stmt_path_start(lo, attrs) { - Ok(stmt) => stmt, - Err(mut err) => { - self.suggest_add_missing_let_for_stmt(&mut err); - return Err(err); - } - }, } } else if let Some(item) = self.parse_item_common( attrs.clone(), @@ -126,12 +126,13 @@ impl<'a> Parser<'a> { self.mk_stmt(lo, StmtKind::Empty) } else if self.token != token::CloseDelim(Delimiter::Brace) { // Remainder are line-expr stmts. - let e = match force_collect { - ForceCollect::Yes => self.collect_tokens_no_attrs(|this| { - this.parse_expr_res(Restrictions::STMT_EXPR, attrs) - })?, - ForceCollect::No => self.parse_expr_res(Restrictions::STMT_EXPR, attrs)?, - }; + let e = self.collect_tokens_trailing_token( + AttrWrapper::empty(), + force_collect, + |this, _empty_attrs| { + Ok((this.parse_expr_res(Restrictions::STMT_EXPR, attrs)?, false)) + }, + )?; if matches!(e.kind, ExprKind::Assign(..)) && self.eat_keyword(kw::Else) { let bl = self.parse_block()?; // Destructuring assignment ... else. From 0c932b763d24a9f57ce3285927ea54308b7be902 Mon Sep 17 00:00:00 2001 From: Michael Howell Date: Thu, 18 Jul 2024 19:49:32 -0700 Subject: [PATCH 21/21] Rearrange sidebar modnav builder to more logical order --- src/librustdoc/html/static/js/main.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/librustdoc/html/static/js/main.js b/src/librustdoc/html/static/js/main.js index 116ce615d8c55..9506bc9ed220b 100644 --- a/src/librustdoc/html/static/js/main.js +++ b/src/librustdoc/html/static/js/main.js @@ -531,13 +531,13 @@ function preLoadCss(cssUrl) { link.href = path; link.textContent = name; const li = document.createElement("li"); - li.appendChild(link); - ul.appendChild(li); // Don't "optimize" this to just use `path`. // We want the browser to normalize this into an absolute URL. if (link.href === current_page) { li.classList.add("current"); } + li.appendChild(link); + ul.appendChild(li); } sidebar.appendChild(h3); sidebar.appendChild(ul);