From f371952cdeee32ce8fb3e859e5a1fd05a1e750aa Mon Sep 17 00:00:00 2001 From: Michael van Straten Date: Sun, 7 Apr 2024 21:08:37 +0200 Subject: [PATCH 01/28] Abstract `ProcThreadAttributeList` into its own struct --- library/std/src/os/windows/process.rs | 328 +++++++++++++++++---- library/std/src/process/tests.rs | 14 +- library/std/src/sys/pal/windows/process.rs | 103 +------ 3 files changed, 294 insertions(+), 151 deletions(-) diff --git a/library/std/src/os/windows/process.rs b/library/std/src/os/windows/process.rs index c2830d2eb61d1..0277b79b8b69c 100644 --- a/library/std/src/os/windows/process.rs +++ b/library/std/src/os/windows/process.rs @@ -4,13 +4,14 @@ #![stable(feature = "process_extensions", since = "1.2.0")] -use crate::ffi::OsStr; +use crate::ffi::{OsStr, c_void}; +use crate::mem::MaybeUninit; use crate::os::windows::io::{ AsHandle, AsRawHandle, BorrowedHandle, FromRawHandle, IntoRawHandle, OwnedHandle, RawHandle, }; use crate::sealed::Sealed; use crate::sys_common::{AsInner, AsInnerMut, FromInner, IntoInner}; -use crate::{process, sys}; +use crate::{io, marker, process, ptr, sys}; #[stable(feature = "process_extensions", since = "1.2.0")] impl FromRawHandle for process::Stdio { @@ -295,41 +296,25 @@ pub trait CommandExt: Sealed { #[unstable(feature = "windows_process_extensions_async_pipes", issue = "98289")] fn async_pipes(&mut self, always_async: bool) -> &mut process::Command; - /// Set a raw attribute on the command, providing extended configuration options for Windows - /// processes. + /// Executes the command as a child process with the given + /// [`ProcThreadAttributeList`], returning a handle to it. /// - /// This method allows you to specify custom attributes for a child process on Windows systems - /// using raw attribute values. Raw attributes provide extended configurability for process - /// creation, but their usage can be complex and potentially unsafe. - /// - /// The `attribute` parameter specifies the raw attribute to be set, while the `value` - /// parameter holds the value associated with that attribute. Please refer to the - /// [`windows-rs` documentation] or the [Win32 API documentation] for detailed information - /// about available attributes and their meanings. - /// - /// [`windows-rs` documentation]: https://microsoft.github.io/windows-docs-rs/doc/windows/ - /// [Win32 API documentation]: https://learn.microsoft.com/en-us/windows/win32/api/processthreadsapi/nf-processthreadsapi-updateprocthreadattribute + /// This method enables the customization of attributes for the spawned + /// child process on Windows systems. + /// Attributes offer extended configurability for process creation, + /// but their usage can be intricate and potentially unsafe. /// /// # Note /// - /// The maximum number of raw attributes is the value of [`u32::MAX`]. - /// If this limit is exceeded, the call to [`process::Command::spawn`] will return an `Error` - /// indicating that the maximum number of attributes has been exceeded. - /// - /// # Safety - /// - /// The usage of raw attributes is potentially unsafe and should be done with caution. - /// Incorrect attribute values or improper configuration can lead to unexpected behavior or - /// errors. + /// By default, stdin, stdout, and stderr are inherited from the parent + /// process. /// /// # Example /// - /// The following example demonstrates how to create a child process with a specific parent - /// process ID using a raw attribute. - /// - /// ```rust + /// ``` /// #![feature(windows_process_extensions_raw_attribute)] - /// use std::os::windows::{process::CommandExt, io::AsRawHandle}; + /// use std::os::windows::io::AsRawHandle; + /// use std::os::windows::process::{CommandExt, ProcThreadAttributeList}; /// use std::process::Command; /// /// # struct ProcessDropGuard(std::process::Child); @@ -338,36 +323,27 @@ pub trait CommandExt: Sealed { /// # let _ = self.0.kill(); /// # } /// # } - /// + /// # /// let parent = Command::new("cmd").spawn()?; - /// - /// let mut child_cmd = Command::new("cmd"); + /// let parent_process_handle = parent.as_raw_handle(); + /// # let parent = ProcessDropGuard(parent); /// /// const PROC_THREAD_ATTRIBUTE_PARENT_PROCESS: usize = 0x00020000; + /// let mut attribute_list = ProcThreadAttributeList::build() + /// .attribute(PROC_THREAD_ATTRIBUTE_PARENT_PROCESS, &parent_process_handle) + /// .finish() + /// .unwrap(); /// - /// unsafe { - /// child_cmd.raw_attribute(PROC_THREAD_ATTRIBUTE_PARENT_PROCESS, parent.as_raw_handle() as isize); - /// } + /// let mut child = Command::new("cmd").spawn_with_attributes(&attribute_list)?; /// # - /// # let parent = ProcessDropGuard(parent); - /// - /// let mut child = child_cmd.spawn()?; - /// /// # child.kill()?; /// # Ok::<(), std::io::Error>(()) /// ``` - /// - /// # Safety Note - /// - /// Remember that improper use of raw attributes can lead to undefined behavior or security - /// vulnerabilities. Always consult the documentation and ensure proper attribute values are - /// used. #[unstable(feature = "windows_process_extensions_raw_attribute", issue = "114854")] - unsafe fn raw_attribute( + fn spawn_with_attributes( &mut self, - attribute: usize, - value: T, - ) -> &mut process::Command; + attribute_list: &ProcThreadAttributeList<'_>, + ) -> io::Result; } #[stable(feature = "windows_process_extensions", since = "1.16.0")] @@ -401,13 +377,13 @@ impl CommandExt for process::Command { self } - unsafe fn raw_attribute( + fn spawn_with_attributes( &mut self, - attribute: usize, - value: T, - ) -> &mut process::Command { - unsafe { self.as_inner_mut().raw_attribute(attribute, value) }; - self + attribute_list: &ProcThreadAttributeList<'_>, + ) -> io::Result { + self.as_inner_mut() + .spawn_with_attributes(sys::process::Stdio::Inherit, true, Some(attribute_list)) + .map(process::Child::from_inner) } } @@ -447,3 +423,245 @@ impl ExitCodeExt for process::ExitCode { process::ExitCode::from_inner(From::from(raw)) } } + +/// A wrapper around windows [`ProcThreadAttributeList`][1]. +/// +/// [1]: +#[derive(Debug)] +#[unstable(feature = "windows_process_extensions_raw_attribute", issue = "114854")] +pub struct ProcThreadAttributeList<'a> { + attribute_list: Box<[MaybeUninit]>, + _lifetime_marker: marker::PhantomData<&'a ()>, +} + +#[unstable(feature = "windows_process_extensions_raw_attribute", issue = "114854")] +impl<'a> ProcThreadAttributeList<'a> { + /// Creates a new builder for constructing a [`ProcThreadAttributeList`]. + pub fn build() -> ProcThreadAttributeListBuilder<'a> { + ProcThreadAttributeListBuilder::new() + } + + /// Returns a pointer to the underling attribute list. + #[doc(hidden)] + pub fn as_ptr(&self) -> *const MaybeUninit { + self.attribute_list.as_ptr() + } +} + +#[unstable(feature = "windows_process_extensions_raw_attribute", issue = "114854")] +impl<'a> Drop for ProcThreadAttributeList<'a> { + /// Deletes the attribute list. + /// + /// This method calls [`DeleteProcThreadAttributeList`][1] to delete the + /// underlying attribute list. + /// + /// [1]: + fn drop(&mut self) { + let lp_attribute_list = self.attribute_list.as_mut_ptr().cast::(); + unsafe { sys::c::DeleteProcThreadAttributeList(lp_attribute_list) } + } +} + +/// Builder for constructing a [`ProcThreadAttributeList`]. +#[derive(Clone, Debug)] +#[unstable(feature = "windows_process_extensions_raw_attribute", issue = "114854")] +pub struct ProcThreadAttributeListBuilder<'a> { + attributes: alloc::collections::BTreeMap, + _lifetime_marker: marker::PhantomData<&'a ()>, +} + +#[unstable(feature = "windows_process_extensions_raw_attribute", issue = "114854")] +impl<'a> ProcThreadAttributeListBuilder<'a> { + fn new() -> Self { + ProcThreadAttributeListBuilder { + attributes: alloc::collections::BTreeMap::new(), + _lifetime_marker: marker::PhantomData, + } + } + + /// Sets an attribute on the attribute list. + /// + /// The `attribute` parameter specifies the raw attribute to be set, while + /// the `value` parameter holds the value associated with that attribute. + /// Please refer to the [Windows documentation][1] for a list of valid attributes. + /// + /// # Note + /// + /// The maximum number of attributes is the value of [`u32::MAX`]. If this + /// limit is exceeded, the call to [`Self::finish`] will return an `Error` + /// indicating that the maximum number of attributes has been exceeded. + /// + /// # Safety Note + /// + /// Remember that improper use of attributes can lead to undefined behavior + /// or security vulnerabilities. Always consult the documentation and ensure + /// proper attribute values are used. + /// + /// [1]: + pub fn attribute(self, attribute: usize, value: &'a T) -> Self { + unsafe { + self.raw_attribute( + attribute, + ptr::addr_of!(*value).cast::(), + crate::mem::size_of::(), + ) + } + } + + /// Sets a raw attribute on the attribute list. + /// + /// This function is useful for setting attributes with pointers or sizes + /// that cannot be derived directly from their values. + /// + /// # Safety + /// + /// This function is marked as `unsafe` because it deals with raw pointers + /// and sizes. It is the responsibility of the caller to ensure the value + /// lives longer than the resulting [`ProcThreadAttributeList`] as well as + /// the validity of the size parameter. + /// + /// # Example + /// + /// ``` + /// #![feature(windows_process_extensions_raw_attribute)] + /// use std::ffi::c_void; + /// use std::os::windows::process::{CommandExt, ProcThreadAttributeList}; + /// use std::os::windows::raw::HANDLE; + /// use std::process::Command; + /// + /// #[repr(C)] + /// pub struct COORD { + /// pub X: i16, + /// pub Y: i16, + /// } + /// + /// extern "system" { + /// fn CreatePipe( + /// hreadpipe: *mut HANDLE, + /// hwritepipe: *mut HANDLE, + /// lppipeattributes: *const c_void, + /// nsize: u32, + /// ) -> i32; + /// fn CreatePseudoConsole( + /// size: COORD, + /// hinput: HANDLE, + /// houtput: HANDLE, + /// dwflags: u32, + /// phpc: *mut isize, + /// ) -> i32; + /// fn CloseHandle(hobject: HANDLE) -> i32; + /// } + /// + /// let [mut input_read_side, mut output_write_side, mut output_read_side, mut input_write_side] = + /// [unsafe { std::mem::zeroed::() }; 4]; + /// + /// unsafe { + /// CreatePipe(&mut input_read_side, &mut input_write_side, std::ptr::null(), 0); + /// CreatePipe(&mut output_read_side, &mut output_write_side, std::ptr::null(), 0); + /// } + /// + /// let size = COORD { X: 60, Y: 40 }; + /// let mut h_pc = unsafe { std::mem::zeroed() }; + /// unsafe { CreatePseudoConsole(size, input_read_side, output_write_side, 0, &mut h_pc) }; + /// + /// unsafe { CloseHandle(input_read_side) }; + /// unsafe { CloseHandle(output_write_side) }; + /// + /// const PROC_THREAD_ATTRIBUTE_PSEUDOCONSOLE: usize = 131094; + /// + /// let attribute_list = unsafe { + /// ProcThreadAttributeList::build() + /// .raw_attribute( + /// PROC_THREAD_ATTRIBUTE_PSEUDOCONSOLE, + /// h_pc as *const c_void, + /// std::mem::size_of::(), + /// ) + /// .finish()? + /// }; + /// + /// let mut child = Command::new("cmd").spawn_with_attributes(&attribute_list)?; + /// # + /// # child.kill()?; + /// # Ok::<(), std::io::Error>(()) + /// ``` + pub unsafe fn raw_attribute( + mut self, + attribute: usize, + value_ptr: *const T, + value_size: usize, + ) -> Self { + self.attributes.insert(attribute, ProcThreadAttributeValue { + ptr: value_ptr.cast::(), + size: value_size, + }); + self + } + + /// Finalizes the construction of the `ProcThreadAttributeList`. + /// + /// # Errors + /// + /// Returns an error if the maximum number of attributes is exceeded + /// or if there is an I/O error during initialization. + pub fn finish(&self) -> io::Result> { + // To initialize our ProcThreadAttributeList, we need to determine + // how many bytes to allocate for it. The Windows API simplifies this + // process by allowing us to call `InitializeProcThreadAttributeList` + // with a null pointer to retrieve the required size. + let mut required_size = 0; + let Ok(attribute_count) = self.attributes.len().try_into() else { + return Err(io::const_error!( + io::ErrorKind::InvalidInput, + "maximum number of ProcThreadAttributes exceeded", + )); + }; + unsafe { + sys::c::InitializeProcThreadAttributeList( + ptr::null_mut(), + attribute_count, + 0, + &mut required_size, + ) + }; + + let mut attribute_list = vec![MaybeUninit::uninit(); required_size].into_boxed_slice(); + + // Once we've allocated the necessary memory, it's safe to invoke + // `InitializeProcThreadAttributeList` to properly initialize the list. + sys::cvt(unsafe { + sys::c::InitializeProcThreadAttributeList( + attribute_list.as_mut_ptr().cast::(), + attribute_count, + 0, + &mut required_size, + ) + })?; + + // # Add our attributes to the buffer. + // It's theoretically possible for the attribute count to exceed a u32 + // value. Therefore, we ensure that we don't add more attributes than + // the buffer was initialized for. + for (&attribute, value) in self.attributes.iter().take(attribute_count as usize) { + sys::cvt(unsafe { + sys::c::UpdateProcThreadAttribute( + attribute_list.as_mut_ptr().cast::(), + 0, + attribute, + value.ptr, + value.size, + ptr::null_mut(), + ptr::null_mut(), + ) + })?; + } + + Ok(ProcThreadAttributeList { attribute_list, _lifetime_marker: marker::PhantomData }) + } +} + +/// Wrapper around the value data to be used as a Process Thread Attribute. +#[derive(Clone, Debug)] +struct ProcThreadAttributeValue { + ptr: *const c_void, + size: usize, +} diff --git a/library/std/src/process/tests.rs b/library/std/src/process/tests.rs index fb0b495961c36..e8cbfe337bccf 100644 --- a/library/std/src/process/tests.rs +++ b/library/std/src/process/tests.rs @@ -450,7 +450,7 @@ fn test_creation_flags() { fn test_proc_thread_attributes() { use crate::mem; use crate::os::windows::io::AsRawHandle; - use crate::os::windows::process::CommandExt; + use crate::os::windows::process::{CommandExt, ProcThreadAttributeList}; use crate::sys::c::{BOOL, CloseHandle, HANDLE}; use crate::sys::cvt; @@ -490,12 +490,14 @@ fn test_proc_thread_attributes() { let mut child_cmd = Command::new("cmd"); - unsafe { - child_cmd - .raw_attribute(PROC_THREAD_ATTRIBUTE_PARENT_PROCESS, parent.0.as_raw_handle() as isize); - } + let parent_process_handle = parent.0.as_raw_handle(); + + let mut attribute_list = ProcThreadAttributeList::build() + .attribute(PROC_THREAD_ATTRIBUTE_PARENT_PROCESS, &parent_process_handle) + .finish() + .unwrap(); - let child = ProcessDropGuard(child_cmd.spawn().unwrap()); + let child = ProcessDropGuard(child_cmd.spawn_with_attributes(&mut attribute_list).unwrap()); let h_snapshot = unsafe { CreateToolhelp32Snapshot(TH32CS_SNAPPROCESS, 0) }; diff --git a/library/std/src/sys/pal/windows/process.rs b/library/std/src/sys/pal/windows/process.rs index da0daacd1dde3..2ca20a21dfe51 100644 --- a/library/std/src/sys/pal/windows/process.rs +++ b/library/std/src/sys/pal/windows/process.rs @@ -10,10 +10,10 @@ use crate::collections::BTreeMap; use crate::env::consts::{EXE_EXTENSION, EXE_SUFFIX}; use crate::ffi::{OsStr, OsString}; use crate::io::{self, Error, ErrorKind}; -use crate::mem::MaybeUninit; use crate::num::NonZero; use crate::os::windows::ffi::{OsStrExt, OsStringExt}; use crate::os::windows::io::{AsHandle, AsRawHandle, BorrowedHandle, FromRawHandle, IntoRawHandle}; +use crate::os::windows::process::ProcThreadAttributeList; use crate::path::{Path, PathBuf}; use crate::sync::Mutex; use crate::sys::args::{self, Arg}; @@ -162,7 +162,6 @@ pub struct Command { stdout: Option, stderr: Option, force_quotes_enabled: bool, - proc_thread_attributes: BTreeMap, } pub enum Stdio { @@ -194,7 +193,6 @@ impl Command { stdout: None, stderr: None, force_quotes_enabled: false, - proc_thread_attributes: Default::default(), } } @@ -248,21 +246,19 @@ impl Command { self.cwd.as_ref().map(Path::new) } - pub unsafe fn raw_attribute( + pub fn spawn( &mut self, - attribute: usize, - value: T, - ) { - self.proc_thread_attributes.insert(attribute, ProcThreadAttributeValue { - size: mem::size_of::(), - data: Box::new(value), - }); + default: Stdio, + needs_stdin: bool, + ) -> io::Result<(Process, StdioPipes)> { + self.spawn_with_attributes(default, needs_stdin, None) } - pub fn spawn( + pub fn spawn_with_attributes( &mut self, default: Stdio, needs_stdin: bool, + proc_thread_attribute_list: Option<&ProcThreadAttributeList<'_>>, ) -> io::Result<(Process, StdioPipes)> { let maybe_env = self.env.capture_if_changed(); @@ -355,18 +351,18 @@ impl Command { let si_ptr: *mut c::STARTUPINFOW; - let mut proc_thread_attribute_list; let mut si_ex; - if !self.proc_thread_attributes.is_empty() { + if let Some(proc_thread_attribute_list) = proc_thread_attribute_list { si.cb = mem::size_of::() as u32; flags |= c::EXTENDED_STARTUPINFO_PRESENT; - proc_thread_attribute_list = - make_proc_thread_attribute_list(&self.proc_thread_attributes)?; si_ex = c::STARTUPINFOEXW { StartupInfo: si, - lpAttributeList: proc_thread_attribute_list.0.as_mut_ptr() as _, + // SAFETY: Casting this `*const` pointer to a `*mut` pointer is "safe" + // here because windows does not internally mutate the attribute list. + // Ideally this should be reflected in the interface of the `windows-sys` crate. + lpAttributeList: proc_thread_attribute_list.as_ptr().cast::().cast_mut(), }; si_ptr = (&raw mut si_ex) as _; } else { @@ -896,79 +892,6 @@ fn make_dirp(d: Option<&OsString>) -> io::Result<(*const u16, Vec)> { } } -struct ProcThreadAttributeList(Box<[MaybeUninit]>); - -impl Drop for ProcThreadAttributeList { - fn drop(&mut self) { - let lp_attribute_list = self.0.as_mut_ptr() as _; - unsafe { c::DeleteProcThreadAttributeList(lp_attribute_list) } - } -} - -/// Wrapper around the value data to be used as a Process Thread Attribute. -struct ProcThreadAttributeValue { - data: Box, - size: usize, -} - -fn make_proc_thread_attribute_list( - attributes: &BTreeMap, -) -> io::Result { - // To initialize our ProcThreadAttributeList, we need to determine - // how many bytes to allocate for it. The Windows API simplifies this process - // by allowing us to call `InitializeProcThreadAttributeList` with - // a null pointer to retrieve the required size. - let mut required_size = 0; - let Ok(attribute_count) = attributes.len().try_into() else { - return Err(io::const_error!( - ErrorKind::InvalidInput, - "maximum number of ProcThreadAttributes exceeded", - )); - }; - unsafe { - c::InitializeProcThreadAttributeList( - ptr::null_mut(), - attribute_count, - 0, - &mut required_size, - ) - }; - - let mut proc_thread_attribute_list = - ProcThreadAttributeList(vec![MaybeUninit::uninit(); required_size].into_boxed_slice()); - - // Once we've allocated the necessary memory, it's safe to invoke - // `InitializeProcThreadAttributeList` to properly initialize the list. - cvt(unsafe { - c::InitializeProcThreadAttributeList( - proc_thread_attribute_list.0.as_mut_ptr() as *mut _, - attribute_count, - 0, - &mut required_size, - ) - })?; - - // # Add our attributes to the buffer. - // It's theoretically possible for the attribute count to exceed a u32 value. - // Therefore, we ensure that we don't add more attributes than the buffer was initialized for. - for (&attribute, value) in attributes.iter().take(attribute_count as usize) { - let value_ptr = (&raw const *value.data) as _; - cvt(unsafe { - c::UpdateProcThreadAttribute( - proc_thread_attribute_list.0.as_mut_ptr() as _, - 0, - attribute, - value_ptr, - value.size, - ptr::null_mut(), - ptr::null_mut(), - ) - })?; - } - - Ok(proc_thread_attribute_list) -} - pub struct CommandArgs<'a> { iter: crate::slice::Iter<'a, Arg>, } From f3ac64ac342fca3040e067a9e3c3c8aba9186860 Mon Sep 17 00:00:00 2001 From: David Tolnay Date: Mon, 2 Dec 2024 17:23:12 -0800 Subject: [PATCH 02/28] Add test of closure vs jump precedence --- tests/ui-fulldeps/pprust-parenthesis-insertion.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/ui-fulldeps/pprust-parenthesis-insertion.rs b/tests/ui-fulldeps/pprust-parenthesis-insertion.rs index fd6644d73c161..d545c017fdac5 100644 --- a/tests/ui-fulldeps/pprust-parenthesis-insertion.rs +++ b/tests/ui-fulldeps/pprust-parenthesis-insertion.rs @@ -70,6 +70,9 @@ static EXPRS: &[&str] = &[ // These mean different things. "return - 2", "(return) - 2", + // Closures and jumps have equal precedence. + "|| return break 2", + "return break (|| 2)", // FIXME: no parenthesis needed. // These mean different things. "if let _ = true && false {}", "if let _ = (true && false) {}", From 193d82797cfa88eb49d53c16745423de6bc54851 Mon Sep 17 00:00:00 2001 From: David Tolnay Date: Mon, 2 Dec 2024 17:17:37 -0800 Subject: [PATCH 03/28] Squash closures and jumps into a single precedence level --- compiler/rustc_ast/src/ast.rs | 3 +-- compiler/rustc_ast/src/util/parser.rs | 3 +-- compiler/rustc_hir/src/hir.rs | 3 +-- tests/ui-fulldeps/pprust-parenthesis-insertion.rs | 2 +- 4 files changed, 4 insertions(+), 7 deletions(-) diff --git a/compiler/rustc_ast/src/ast.rs b/compiler/rustc_ast/src/ast.rs index 14205f66491c5..2c9e55e007f9b 100644 --- a/compiler/rustc_ast/src/ast.rs +++ b/compiler/rustc_ast/src/ast.rs @@ -1317,9 +1317,8 @@ impl Expr { pub fn precedence(&self) -> ExprPrecedence { match self.kind { - ExprKind::Closure(..) => ExprPrecedence::Closure, - ExprKind::Break(..) + | ExprKind::Closure(..) | ExprKind::Continue(..) | ExprKind::Ret(..) | ExprKind::Yield(..) diff --git a/compiler/rustc_ast/src/util/parser.rs b/compiler/rustc_ast/src/util/parser.rs index e88bf27021aff..a4f152b46878f 100644 --- a/compiler/rustc_ast/src/util/parser.rs +++ b/compiler/rustc_ast/src/util/parser.rs @@ -231,8 +231,7 @@ impl AssocOp { #[derive(Clone, Copy, PartialEq, PartialOrd)] pub enum ExprPrecedence { - Closure, - // return, break, yield + // return, break, yield, closures Jump, // = += -= *= /= %= &= |= ^= <<= >>= Assign, diff --git a/compiler/rustc_hir/src/hir.rs b/compiler/rustc_hir/src/hir.rs index a9696627f11b1..4524e1cbdf1fc 100644 --- a/compiler/rustc_hir/src/hir.rs +++ b/compiler/rustc_hir/src/hir.rs @@ -1697,9 +1697,8 @@ pub struct Expr<'hir> { impl Expr<'_> { pub fn precedence(&self) -> ExprPrecedence { match self.kind { - ExprKind::Closure { .. } => ExprPrecedence::Closure, - ExprKind::Break(..) + | ExprKind::Closure { .. } | ExprKind::Continue(..) | ExprKind::Ret(..) | ExprKind::Yield(..) diff --git a/tests/ui-fulldeps/pprust-parenthesis-insertion.rs b/tests/ui-fulldeps/pprust-parenthesis-insertion.rs index d545c017fdac5..f535db06879f3 100644 --- a/tests/ui-fulldeps/pprust-parenthesis-insertion.rs +++ b/tests/ui-fulldeps/pprust-parenthesis-insertion.rs @@ -72,7 +72,7 @@ static EXPRS: &[&str] = &[ "(return) - 2", // Closures and jumps have equal precedence. "|| return break 2", - "return break (|| 2)", // FIXME: no parenthesis needed. + "return break || 2", // These mean different things. "if let _ = true && false {}", "if let _ = (true && false) {}", From 4df47a09a42620f018bdaed733fdbc4049a65e78 Mon Sep 17 00:00:00 2001 From: David Tolnay Date: Mon, 2 Dec 2024 17:37:03 -0800 Subject: [PATCH 04/28] Add test of closure precedence with return type --- tests/ui-fulldeps/pprust-parenthesis-insertion.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/ui-fulldeps/pprust-parenthesis-insertion.rs b/tests/ui-fulldeps/pprust-parenthesis-insertion.rs index f535db06879f3..6535367927ef0 100644 --- a/tests/ui-fulldeps/pprust-parenthesis-insertion.rs +++ b/tests/ui-fulldeps/pprust-parenthesis-insertion.rs @@ -73,6 +73,9 @@ static EXPRS: &[&str] = &[ // Closures and jumps have equal precedence. "|| return break 2", "return break || 2", + // Closures with a return type have especially high precedence. + "(|| -> T { x }) + 1", // FIXME: no parenthesis needed. + "(|| { x }) + 1", // These mean different things. "if let _ = true && false {}", "if let _ = (true && false) {}", From 72ac961616f23401c54f08436006250fd1fcdb9e Mon Sep 17 00:00:00 2001 From: David Tolnay Date: Mon, 2 Dec 2024 17:41:47 -0800 Subject: [PATCH 05/28] Raise precedence of closure that has explicit return type --- compiler/rustc_ast/src/ast.rs | 10 ++++++++-- compiler/rustc_hir/src/hir.rs | 12 +++++++++--- tests/ui-fulldeps/pprust-parenthesis-insertion.rs | 2 +- 3 files changed, 18 insertions(+), 6 deletions(-) diff --git a/compiler/rustc_ast/src/ast.rs b/compiler/rustc_ast/src/ast.rs index 2c9e55e007f9b..482efa132ab53 100644 --- a/compiler/rustc_ast/src/ast.rs +++ b/compiler/rustc_ast/src/ast.rs @@ -1316,9 +1316,15 @@ impl Expr { } pub fn precedence(&self) -> ExprPrecedence { - match self.kind { + match &self.kind { + ExprKind::Closure(closure) => { + match closure.fn_decl.output { + FnRetTy::Default(_) => ExprPrecedence::Jump, + FnRetTy::Ty(_) => ExprPrecedence::Unambiguous, + } + } + ExprKind::Break(..) - | ExprKind::Closure(..) | ExprKind::Continue(..) | ExprKind::Ret(..) | ExprKind::Yield(..) diff --git a/compiler/rustc_hir/src/hir.rs b/compiler/rustc_hir/src/hir.rs index 4524e1cbdf1fc..21fa11fbc0a66 100644 --- a/compiler/rustc_hir/src/hir.rs +++ b/compiler/rustc_hir/src/hir.rs @@ -1696,9 +1696,15 @@ pub struct Expr<'hir> { impl Expr<'_> { pub fn precedence(&self) -> ExprPrecedence { - match self.kind { + match &self.kind { + ExprKind::Closure(closure) => { + match closure.fn_decl.output { + FnRetTy::DefaultReturn(_) => ExprPrecedence::Jump, + FnRetTy::Return(_) => ExprPrecedence::Unambiguous, + } + } + ExprKind::Break(..) - | ExprKind::Closure { .. } | ExprKind::Continue(..) | ExprKind::Ret(..) | ExprKind::Yield(..) @@ -1741,7 +1747,7 @@ impl Expr<'_> { | ExprKind::Type(..) | ExprKind::Err(_) => ExprPrecedence::Unambiguous, - ExprKind::DropTemps(ref expr, ..) => expr.precedence(), + ExprKind::DropTemps(expr, ..) => expr.precedence(), } } diff --git a/tests/ui-fulldeps/pprust-parenthesis-insertion.rs b/tests/ui-fulldeps/pprust-parenthesis-insertion.rs index 6535367927ef0..1f4e98d483d06 100644 --- a/tests/ui-fulldeps/pprust-parenthesis-insertion.rs +++ b/tests/ui-fulldeps/pprust-parenthesis-insertion.rs @@ -74,7 +74,7 @@ static EXPRS: &[&str] = &[ "|| return break 2", "return break || 2", // Closures with a return type have especially high precedence. - "(|| -> T { x }) + 1", // FIXME: no parenthesis needed. + "|| -> T { x } + 1", "(|| { x }) + 1", // These mean different things. "if let _ = true && false {}", From fe06c5dce1db564d42678ea8e4f4a8ae451fe4a3 Mon Sep 17 00:00:00 2001 From: David Tolnay Date: Mon, 2 Dec 2024 17:50:12 -0800 Subject: [PATCH 06/28] Never parenthesize `continue` --- compiler/rustc_ast/src/ast.rs | 2 +- compiler/rustc_hir/src/hir.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/compiler/rustc_ast/src/ast.rs b/compiler/rustc_ast/src/ast.rs index 482efa132ab53..d7d4a821ef6a2 100644 --- a/compiler/rustc_ast/src/ast.rs +++ b/compiler/rustc_ast/src/ast.rs @@ -1325,7 +1325,6 @@ impl Expr { } ExprKind::Break(..) - | ExprKind::Continue(..) | ExprKind::Ret(..) | ExprKind::Yield(..) | ExprKind::Yeet(..) @@ -1359,6 +1358,7 @@ impl Expr { | ExprKind::Block(..) | ExprKind::Call(..) | ExprKind::ConstBlock(_) + | ExprKind::Continue(..) | ExprKind::Field(..) | ExprKind::ForLoop { .. } | ExprKind::FormatArgs(..) diff --git a/compiler/rustc_hir/src/hir.rs b/compiler/rustc_hir/src/hir.rs index 21fa11fbc0a66..2a7df6827e4dc 100644 --- a/compiler/rustc_hir/src/hir.rs +++ b/compiler/rustc_hir/src/hir.rs @@ -1705,7 +1705,6 @@ impl Expr<'_> { } ExprKind::Break(..) - | ExprKind::Continue(..) | ExprKind::Ret(..) | ExprKind::Yield(..) | ExprKind::Become(..) => ExprPrecedence::Jump, @@ -1731,6 +1730,7 @@ impl Expr<'_> { | ExprKind::Block(..) | ExprKind::Call(..) | ExprKind::ConstBlock(_) + | ExprKind::Continue(..) | ExprKind::Field(..) | ExprKind::If(..) | ExprKind::Index(..) From 1e33dd17115ca948c6e5ebf319695ed64490b2bf Mon Sep 17 00:00:00 2001 From: Harrison Kaiser Date: Sun, 15 Dec 2024 20:06:02 -0500 Subject: [PATCH 07/28] Fix logical error with what text is considered whitespace. There is a logical issue around what counts as leading white-space. There is code which does a subtraction assuming that no errors will be reported inside the leading whitespace. However we compute the length of that whitespace with std::char::is_whitespace and not rustc_lexer::is_whitespace. The former will include a no-break space while later will excluded it. We can only safely make the assumption that no errors will be reported in whitespace if it is all "Rust Standard" whitespace. Indeed an error does occur in unicode whitespace if it contains a no-break space. --- Cargo.lock | 1 + compiler/rustc_errors/Cargo.toml | 1 + compiler/rustc_errors/src/emitter.rs | 10 ++++++++-- tests/ui/errors/emitter-overflow-bad-whitespace.rs | 11 +++++++++++ .../errors/emitter-overflow-bad-whitespace.stderr | 13 +++++++++++++ 5 files changed, 34 insertions(+), 2 deletions(-) create mode 100644 tests/ui/errors/emitter-overflow-bad-whitespace.rs create mode 100644 tests/ui/errors/emitter-overflow-bad-whitespace.stderr diff --git a/Cargo.lock b/Cargo.lock index 5eeb855aacb87..b53d89ba88640 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3703,6 +3703,7 @@ dependencies = [ "rustc_fluent_macro", "rustc_hir", "rustc_index", + "rustc_lexer", "rustc_lint_defs", "rustc_macros", "rustc_serialize", diff --git a/compiler/rustc_errors/Cargo.toml b/compiler/rustc_errors/Cargo.toml index 06bae57638f3c..66b9adbead0b6 100644 --- a/compiler/rustc_errors/Cargo.toml +++ b/compiler/rustc_errors/Cargo.toml @@ -16,6 +16,7 @@ rustc_error_messages = { path = "../rustc_error_messages" } rustc_fluent_macro = { path = "../rustc_fluent_macro" } rustc_hir = { path = "../rustc_hir" } rustc_index = { path = "../rustc_index" } +rustc_lexer = { path = "../rustc_lexer" } rustc_lint_defs = { path = "../rustc_lint_defs" } rustc_macros = { path = "../rustc_macros" } rustc_serialize = { path = "../rustc_serialize" } diff --git a/compiler/rustc_errors/src/emitter.rs b/compiler/rustc_errors/src/emitter.rs index ac2f91cdeb3fe..17908bca26afc 100644 --- a/compiler/rustc_errors/src/emitter.rs +++ b/compiler/rustc_errors/src/emitter.rs @@ -19,6 +19,7 @@ use derive_setters::Setters; use rustc_data_structures::fx::{FxHashMap, FxIndexMap, FxIndexSet}; use rustc_data_structures::sync::{DynSend, IntoDynSyncSend, Lrc}; use rustc_error_messages::{FluentArgs, SpanLabel}; +use rustc_lexer; use rustc_lint_defs::pluralize; use rustc_span::hygiene::{ExpnKind, MacroKind}; use rustc_span::source_map::SourceMap; @@ -1698,9 +1699,14 @@ impl HumanEmitter { if let Some(source_string) = line.line_index.checked_sub(1).and_then(|l| file.get_line(l)) { + // Whitespace can only be removed (aka considered leading) + // if the lexer considers it whitespace. + // non-rustc_lexer::is_whitespace() chars are reported as an + // error (ex. no-break-spaces \u{a0}), and thus can't be considered + // for removal during error reporting. let leading_whitespace = source_string .chars() - .take_while(|c| c.is_whitespace()) + .take_while(|c| rustc_lexer::is_whitespace(*c)) .map(|c| { match c { // Tabs are displayed as 4 spaces @@ -1709,7 +1715,7 @@ impl HumanEmitter { } }) .sum(); - if source_string.chars().any(|c| !c.is_whitespace()) { + if source_string.chars().any(|c| !rustc_lexer::is_whitespace(c)) { whitespace_margin = min(whitespace_margin, leading_whitespace); } } diff --git a/tests/ui/errors/emitter-overflow-bad-whitespace.rs b/tests/ui/errors/emitter-overflow-bad-whitespace.rs new file mode 100644 index 0000000000000..d024d1f3921b2 --- /dev/null +++ b/tests/ui/errors/emitter-overflow-bad-whitespace.rs @@ -0,0 +1,11 @@ +// Invalid whitespace (not listed here: https://doc.rust-lang.org/reference/whitespace.html +// e.g. \u{a0}) before any other syntax on the line should not cause any integer overflow +// in the emitter, even when the terminal width causes the line to be truncated. +// +// issue #132918 + +//@ check-fail +//@ needs-rustc-debug-assertions +//@ compile-flags: --diagnostic-width=1 +                                        fn main() { return; } +//~^ ERROR unknown start of token: \u{a0} diff --git a/tests/ui/errors/emitter-overflow-bad-whitespace.stderr b/tests/ui/errors/emitter-overflow-bad-whitespace.stderr new file mode 100644 index 0000000000000..8c0c097bcb9c4 --- /dev/null +++ b/tests/ui/errors/emitter-overflow-bad-whitespace.stderr @@ -0,0 +1,13 @@ +error: unknown start of token: \u{a0} + --> $DIR/emitter-overflow-bad-whitespace.rs:10:1 + | +LL |     ... + | ^ + | +help: Unicode character ' ' (No-Break Space) looks like ' ' (Space), but it is not + | +LL |                                       fn main() { return; } + | + + +error: aborting due to 1 previous error + From cb88030b286dab999d835aeebd21a2cc56c96b8c Mon Sep 17 00:00:00 2001 From: Adrian Taylor Date: Thu, 19 Dec 2024 11:12:35 +0000 Subject: [PATCH 08/28] Arbitrary self types v2: niche deshadowing test Arbitrary self types v2 attempts to detect cases where methods in an "outer" type (e.g. a smart pointer) might "shadow" methods in the referent. There are a couple of cases where the current code makes no attempt to detect such shadowing. Both of these cases only apply if other unstable features are enabled. Add a test, mostly for illustrative purposes, so we can see the shadowing cases that can occur. --- .../arbitrary_self_types_niche_deshadowing.rs | 63 +++++++++++++++++++ 1 file changed, 63 insertions(+) create mode 100644 tests/ui/self/arbitrary_self_types_niche_deshadowing.rs diff --git a/tests/ui/self/arbitrary_self_types_niche_deshadowing.rs b/tests/ui/self/arbitrary_self_types_niche_deshadowing.rs new file mode 100644 index 0000000000000..9326eca1f5353 --- /dev/null +++ b/tests/ui/self/arbitrary_self_types_niche_deshadowing.rs @@ -0,0 +1,63 @@ +//@ run-pass + +#![allow(dead_code)] +#![allow(incomplete_features)] + +#![feature(arbitrary_self_types)] +#![feature(arbitrary_self_types_pointers)] +#![feature(pin_ergonomics)] + +use std::pin::Pin; +use std::pin::pin; +use std::marker::PhantomData; + +struct A; + +impl A { + fn m(self: *const SmartPtr) -> usize { 2 } + fn n(self: *const SmartPtr) -> usize { 2 } + + fn o(self: Pin<&SmartPtr2>) -> usize { 2 } + fn p(self: Pin<&SmartPtr2>) -> usize { 2 } +} + +struct SmartPtr(T); + +impl core::ops::Receiver for SmartPtr { + type Target = *mut T; +} + +impl SmartPtr { + // In general we try to detect cases where a method in a smart pointer + // "shadows" a method in the referent (in this test, A). + // This method "shadows" the 'n' method in the inner type A + // We do not attempt to produce an error in these shadowing cases + // since the type signature of this method and the corresponding + // method in A are pretty unlikely to occur in practice, + // and because it shows up conflicts between *const::cast and *mut::cast. + fn n(self: *mut Self) -> usize { 1 } +} + +struct SmartPtr2<'a, T>(T, PhantomData<&'a T>); + +impl<'a, T> core::ops::Receiver for SmartPtr2<'a, T> { + type Target = Pin<&'a mut T>; +} + +impl SmartPtr2<'_, T> { + // Similarly, this method shadows the method in A + // Can only happen with the unstable feature pin_ergonomics + fn p(self: Pin<&mut Self>) -> usize { 1 } +} + +fn main() { + let mut sm = SmartPtr(A); + let smp: *mut SmartPtr = &mut sm as *mut SmartPtr; + assert_eq!(smp.m(), 2); + assert_eq!(smp.n(), 1); + + let smp: Pin<&mut SmartPtr2> = pin!(SmartPtr2(A, PhantomData)); + assert_eq!(smp.o(), 2); + let smp: Pin<&mut SmartPtr2> = pin!(SmartPtr2(A, PhantomData)); + assert_eq!(smp.p(), 1); +} From b731c36faa2ecbd1043062ec7a226f17d289e00f Mon Sep 17 00:00:00 2001 From: MarcoIeni <11428655+MarcoIeni@users.noreply.github.com> Date: Thu, 19 Dec 2024 16:02:05 +0100 Subject: [PATCH 09/28] ci: use ubuntu `24` instead of `latest` --- .github/workflows/ci.yml | 4 ++-- .github/workflows/dependencies.yml | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 553ef676154bc..f0e151d25778c 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -46,7 +46,7 @@ jobs: # If you want to modify CI jobs, take a look at src/ci/github-actions/jobs.yml. calculate_matrix: name: Calculate job matrix - runs-on: ubuntu-latest + runs-on: ubuntu-24.04 outputs: jobs: ${{ steps.jobs.outputs.jobs }} run_type: ${{ steps.jobs.outputs.run_type }} @@ -243,7 +243,7 @@ jobs: # when a workflow is successful listening to webhooks only in our current bors implementation (homu). outcome: name: bors build finished - runs-on: ubuntu-latest + runs-on: ubuntu-24.04 needs: [ calculate_matrix, job ] # !cancelled() executes the job regardless of whether the previous jobs passed or failed if: ${{ !cancelled() && contains(fromJSON('["auto", "try"]'), needs.calculate_matrix.outputs.run_type) }} diff --git a/.github/workflows/dependencies.yml b/.github/workflows/dependencies.yml index b7b5a03bd41f3..98d8c14f7d185 100644 --- a/.github/workflows/dependencies.yml +++ b/.github/workflows/dependencies.yml @@ -27,7 +27,7 @@ jobs: not-waiting-on-bors: if: github.repository_owner == 'rust-lang' name: skip if S-waiting-on-bors - runs-on: ubuntu-latest + runs-on: ubuntu-24.04 steps: - env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} @@ -47,7 +47,7 @@ jobs: if: github.repository_owner == 'rust-lang' name: update dependencies needs: not-waiting-on-bors - runs-on: ubuntu-latest + runs-on: ubuntu-24.04 steps: - name: checkout the source code uses: actions/checkout@v4 @@ -94,7 +94,7 @@ jobs: if: github.repository_owner == 'rust-lang' name: amend PR needs: update - runs-on: ubuntu-latest + runs-on: ubuntu-24.04 permissions: contents: write pull-requests: write From 38bc902b1596f47b7e8c80760fb7250e0a4ea697 Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Thu, 12 Dec 2024 12:25:41 +0000 Subject: [PATCH 10/28] Minor cleanup --- compiler/rustc_interface/src/passes.rs | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/compiler/rustc_interface/src/passes.rs b/compiler/rustc_interface/src/passes.rs index 02905e632ab87..aff66e48fbbee 100644 --- a/compiler/rustc_interface/src/passes.rs +++ b/compiler/rustc_interface/src/passes.rs @@ -19,7 +19,7 @@ use rustc_incremental::setup_dep_graph; use rustc_lint::{BufferedEarlyLint, EarlyCheckNode, LintStore, unerased_lint_store}; use rustc_metadata::creader::CStore; use rustc_middle::arena::Arena; -use rustc_middle::ty::{self, GlobalCtxt, RegisteredTools, TyCtxt}; +use rustc_middle::ty::{self, CurrentGcx, GlobalCtxt, RegisteredTools, TyCtxt}; use rustc_middle::util::Providers; use rustc_parse::{ new_parser_from_file, new_parser_from_source_str, unwrap_or_emit_fatal, validate_attr, @@ -770,15 +770,14 @@ pub fn create_and_enter_global_ctxt FnOnce(TyCtxt<'tcx>) -> T>( // subtyping for GlobalCtxt::enter to be allowed. let inner: Box< dyn for<'tcx> FnOnce( - &'tcx Compiler, + &'tcx Session, + CurrentGcx, &'tcx OnceLock>, &'tcx WorkerLocal>, &'tcx WorkerLocal>, F, ) -> T, - > = Box::new(move |compiler, gcx_cell, arena, hir_arena, f| { - let sess = &compiler.sess; - + > = Box::new(move |sess, current_gcx, gcx_cell, arena, hir_arena, f| { TyCtxt::create_global_ctxt( gcx_cell, sess, @@ -796,7 +795,7 @@ pub fn create_and_enter_global_ctxt FnOnce(TyCtxt<'tcx>) -> T>( incremental, ), providers.hooks, - compiler.current_gcx.clone(), + current_gcx, |tcx| { let feed = tcx.create_crate_num(stable_crate_id).unwrap(); assert_eq!(feed.key(), LOCAL_CRATE); @@ -804,7 +803,7 @@ pub fn create_and_enter_global_ctxt FnOnce(TyCtxt<'tcx>) -> T>( let feed = tcx.feed_unit_query(); feed.features_query(tcx.arena.alloc(rustc_expand::config::features( - sess, + tcx.sess, &pre_configured_attrs, crate_name, ))); @@ -819,7 +818,7 @@ pub fn create_and_enter_global_ctxt FnOnce(TyCtxt<'tcx>) -> T>( ) }); - inner(compiler, &gcx_cell, &arena, &hir_arena, f) + inner(&compiler.sess, compiler.current_gcx.clone(), &gcx_cell, &arena, &hir_arena, f) } /// Runs all analyses that we guarantee to run, even if errors were reported in earlier analyses. From 7e6be136472a49c511a6861b9cbd9b6522c11762 Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Thu, 12 Dec 2024 14:55:36 +0000 Subject: [PATCH 11/28] Make DependencyList an IndexVec --- .../rustc_codegen_cranelift/src/driver/jit.rs | 2 +- compiler/rustc_codegen_ssa/src/back/link.rs | 10 +-- compiler/rustc_codegen_ssa/src/back/linker.rs | 7 +- .../rustc_metadata/src/dependency_format.rs | 89 +++++++++++-------- compiler/rustc_metadata/src/rmeta/decoder.rs | 2 +- compiler/rustc_metadata/src/rmeta/encoder.rs | 12 +-- .../src/middle/dependency_format.rs | 6 +- src/tools/miri/src/helpers.rs | 14 +-- 8 files changed, 76 insertions(+), 66 deletions(-) diff --git a/compiler/rustc_codegen_cranelift/src/driver/jit.rs b/compiler/rustc_codegen_cranelift/src/driver/jit.rs index 4be4291021dfa..eaab3362c7e83 100644 --- a/compiler/rustc_codegen_cranelift/src/driver/jit.rs +++ b/compiler/rustc_codegen_cranelift/src/driver/jit.rs @@ -294,7 +294,7 @@ fn dep_symbol_lookup_fn( // search path. for &cnum in crate_info.used_crates.iter().rev() { let src = &crate_info.used_crate_source[&cnum]; - match data[cnum.as_usize() - 1] { + match data[cnum] { Linkage::NotLinked | Linkage::IncludedFromDylib => {} Linkage::Static => { let name = crate_info.crate_name[&cnum]; diff --git a/compiler/rustc_codegen_ssa/src/back/link.rs b/compiler/rustc_codegen_ssa/src/back/link.rs index f4f6161ebbccd..66258790c1ebd 100644 --- a/compiler/rustc_codegen_ssa/src/back/link.rs +++ b/compiler/rustc_codegen_ssa/src/back/link.rs @@ -262,7 +262,7 @@ pub fn each_linked_rlib( }; for &cnum in crates { - match fmts.get(cnum.as_usize() - 1) { + match fmts.get(cnum) { Some(&Linkage::NotLinked | &Linkage::Dynamic | &Linkage::IncludedFromDylib) => continue, Some(_) => {} None => return Err(errors::LinkRlibError::MissingFormat), @@ -624,7 +624,7 @@ fn link_staticlib( let mut all_rust_dylibs = vec![]; for &cnum in crates { - match fmts.get(cnum.as_usize() - 1) { + match fmts.get(cnum) { Some(&Linkage::Dynamic) => {} _ => continue, } @@ -2361,8 +2361,8 @@ fn linker_with_args( .crate_info .native_libraries .iter() - .filter_map(|(cnum, libraries)| { - (dependency_linkage[cnum.as_usize() - 1] != Linkage::Static).then_some(libraries) + .filter_map(|(&cnum, libraries)| { + (dependency_linkage[cnum] != Linkage::Static).then_some(libraries) }) .flatten() .collect::>(); @@ -2754,7 +2754,7 @@ fn add_upstream_rust_crates( // (e.g. `libstd` when `-C prefer-dynamic` is used). // FIXME: `dependency_formats` can report `profiler_builtins` as `NotLinked` for some // reason, it shouldn't do that because `profiler_builtins` should indeed be linked. - let linkage = data[cnum.as_usize() - 1]; + let linkage = data[cnum]; let link_static_crate = linkage == Linkage::Static || (linkage == Linkage::IncludedFromDylib || linkage == Linkage::NotLinked) && (codegen_results.crate_info.compiler_builtins == Some(cnum) diff --git a/compiler/rustc_codegen_ssa/src/back/linker.rs b/compiler/rustc_codegen_ssa/src/back/linker.rs index 8a2f3d73bc15c..3c6513ca26bb1 100644 --- a/compiler/rustc_codegen_ssa/src/back/linker.rs +++ b/compiler/rustc_codegen_ssa/src/back/linker.rs @@ -1744,15 +1744,10 @@ fn for_each_exported_symbols_include_dep<'tcx>( crate_type: CrateType, mut callback: impl FnMut(ExportedSymbol<'tcx>, SymbolExportInfo, CrateNum), ) { - for &(symbol, info) in tcx.exported_symbols(LOCAL_CRATE).iter() { - callback(symbol, info, LOCAL_CRATE); - } - let formats = tcx.dependency_formats(()); let deps = &formats[&crate_type]; - for (index, dep_format) in deps.iter().enumerate() { - let cnum = CrateNum::new(index + 1); + for (cnum, dep_format) in deps.iter_enumerated() { // For each dependency that we are linking to statically ... if *dep_format == Linkage::Static { for &(symbol, info) in tcx.exported_symbols(cnum).iter() { diff --git a/compiler/rustc_metadata/src/dependency_format.rs b/compiler/rustc_metadata/src/dependency_format.rs index e8de0acb7c9fe..1d4083a07d30b 100644 --- a/compiler/rustc_metadata/src/dependency_format.rs +++ b/compiler/rustc_metadata/src/dependency_format.rs @@ -52,7 +52,8 @@ //! than finding a number of solutions (there are normally quite a few). use rustc_data_structures::fx::{FxHashMap, FxHashSet}; -use rustc_hir::def_id::CrateNum; +use rustc_hir::def_id::{CrateNum, LOCAL_CRATE}; +use rustc_index::IndexVec; use rustc_middle::bug; use rustc_middle::middle::dependency_format::{Dependencies, DependencyList, Linkage}; use rustc_middle::ty::TyCtxt; @@ -84,7 +85,7 @@ fn calculate_type(tcx: TyCtxt<'_>, ty: CrateType) -> DependencyList { let sess = &tcx.sess; if !sess.opts.output_types.should_codegen() { - return Vec::new(); + return IndexVec::new(); } let preferred_linkage = match ty { @@ -131,7 +132,7 @@ fn calculate_type(tcx: TyCtxt<'_>, ty: CrateType) -> DependencyList { match preferred_linkage { // If the crate is not linked, there are no link-time dependencies. - Linkage::NotLinked => return Vec::new(), + Linkage::NotLinked => return IndexVec::new(), Linkage::Static => { // Attempt static linkage first. For dylibs and executables, we may be // able to retry below with dynamic linkage. @@ -156,7 +157,7 @@ fn calculate_type(tcx: TyCtxt<'_>, ty: CrateType) -> DependencyList { } sess.dcx().emit_err(RlibRequired { crate_name: tcx.crate_name(cnum) }); } - return Vec::new(); + return IndexVec::new(); } } Linkage::Dynamic | Linkage::IncludedFromDylib => {} @@ -210,13 +211,19 @@ fn calculate_type(tcx: TyCtxt<'_>, ty: CrateType) -> DependencyList { // Collect what we've got so far in the return vector. let last_crate = tcx.crates(()).len(); - let mut ret = (1..last_crate + 1) - .map(|cnum| match formats.get(&CrateNum::new(cnum)) { - Some(&RequireDynamic) => Linkage::Dynamic, - Some(&RequireStatic) => Linkage::IncludedFromDylib, - None => Linkage::NotLinked, - }) - .collect::>(); + let mut ret = IndexVec::new(); + assert_eq!(ret.push(Linkage::Static), LOCAL_CRATE); + for cnum in 1..last_crate + 1 { + let cnum = CrateNum::new(cnum); + assert_eq!( + ret.push(match formats.get(&cnum) { + Some(&RequireDynamic) => Linkage::Dynamic, + Some(&RequireStatic) => Linkage::IncludedFromDylib, + None => Linkage::NotLinked, + }), + cnum + ); + } // Run through the dependency list again, and add any missing libraries as // static libraries. @@ -232,7 +239,7 @@ fn calculate_type(tcx: TyCtxt<'_>, ty: CrateType) -> DependencyList { assert!(src.rlib.is_some() || src.rmeta.is_some()); info!("adding staticlib: {}", tcx.crate_name(cnum)); add_library(tcx, cnum, RequireStatic, &mut formats, &mut unavailable_as_static); - ret[cnum.as_usize() - 1] = Linkage::Static; + ret[cnum] = Linkage::Static; } } @@ -252,8 +259,10 @@ fn calculate_type(tcx: TyCtxt<'_>, ty: CrateType) -> DependencyList { // // For situations like this, we perform one last pass over the dependencies, // making sure that everything is available in the requested format. - for (cnum, kind) in ret.iter().enumerate() { - let cnum = CrateNum::new(cnum + 1); + for (cnum, kind) in ret.iter_enumerated() { + if cnum == LOCAL_CRATE { + continue; + } let src = tcx.used_crate_source(cnum); match *kind { Linkage::NotLinked | Linkage::IncludedFromDylib => {} @@ -334,14 +343,17 @@ fn attempt_static(tcx: TyCtxt<'_>, unavailable: &mut Vec) -> Option Linkage::Static, - CrateDepKind::MacrosOnly | CrateDepKind::Implicit => Linkage::NotLinked, - }) - .collect::>(); + let mut ret = IndexVec::new(); + assert_eq!(ret.push(Linkage::Static), LOCAL_CRATE); + for &cnum in tcx.crates(()) { + assert_eq!( + ret.push(match tcx.dep_kind(cnum) { + CrateDepKind::Explicit => Linkage::Static, + CrateDepKind::MacrosOnly | CrateDepKind::Implicit => Linkage::NotLinked, + }), + cnum + ); + } // Our allocator/panic runtime may not have been linked above if it wasn't // explicitly linked, which is the case for any injected dependency. Handle @@ -367,8 +379,7 @@ fn activate_injected_dep( list: &mut DependencyList, replaces_injected: &dyn Fn(CrateNum) -> bool, ) { - for (i, slot) in list.iter().enumerate() { - let cnum = CrateNum::new(i + 1); + for (cnum, slot) in list.iter_enumerated() { if !replaces_injected(cnum) { continue; } @@ -377,25 +388,23 @@ fn activate_injected_dep( } } if let Some(injected) = injected { - let idx = injected.as_usize() - 1; - assert_eq!(list[idx], Linkage::NotLinked); - list[idx] = Linkage::Static; + assert_eq!(list[injected], Linkage::NotLinked); + list[injected] = Linkage::Static; } } // After the linkage for a crate has been determined we need to verify that // there's only going to be one allocator in the output. -fn verify_ok(tcx: TyCtxt<'_>, list: &[Linkage]) { +fn verify_ok(tcx: TyCtxt<'_>, list: &DependencyList) { let sess = &tcx.sess; if list.is_empty() { return; } let mut panic_runtime = None; - for (i, linkage) in list.iter().enumerate() { + for (cnum, linkage) in list.iter_enumerated() { if let Linkage::NotLinked = *linkage { continue; } - let cnum = CrateNum::new(i + 1); if tcx.is_panic_runtime(cnum) { if let Some((prev, _)) = panic_runtime { @@ -431,11 +440,10 @@ fn verify_ok(tcx: TyCtxt<'_>, list: &[Linkage]) { // strategy. If the dep isn't linked, we ignore it, and if our strategy // is abort then it's compatible with everything. Otherwise all crates' // panic strategy must match our own. - for (i, linkage) in list.iter().enumerate() { + for (cnum, linkage) in list.iter_enumerated() { if let Linkage::NotLinked = *linkage { continue; } - let cnum = CrateNum::new(i + 1); if cnum == runtime_cnum || tcx.is_compiler_builtins(cnum) { continue; } @@ -450,13 +458,16 @@ fn verify_ok(tcx: TyCtxt<'_>, list: &[Linkage]) { }); } - let found_drop_strategy = tcx.panic_in_drop_strategy(cnum); - if tcx.sess.opts.unstable_opts.panic_in_drop != found_drop_strategy { - sess.dcx().emit_err(IncompatiblePanicInDropStrategy { - crate_name: tcx.crate_name(cnum), - found_strategy: found_drop_strategy, - desired_strategy: tcx.sess.opts.unstable_opts.panic_in_drop, - }); + // panic_in_drop_strategy isn't allowed for LOCAL_CRATE + if cnum != LOCAL_CRATE { + let found_drop_strategy = tcx.panic_in_drop_strategy(cnum); + if tcx.sess.opts.unstable_opts.panic_in_drop != found_drop_strategy { + sess.dcx().emit_err(IncompatiblePanicInDropStrategy { + crate_name: tcx.crate_name(cnum), + found_strategy: found_drop_strategy, + desired_strategy: tcx.sess.opts.unstable_opts.panic_in_drop, + }); + } } } } diff --git a/compiler/rustc_metadata/src/rmeta/decoder.rs b/compiler/rustc_metadata/src/rmeta/decoder.rs index 90b1d2952c599..c2b5e318bda72 100644 --- a/compiler/rustc_metadata/src/rmeta/decoder.rs +++ b/compiler/rustc_metadata/src/rmeta/decoder.rs @@ -1474,7 +1474,7 @@ impl<'a> CrateMetadataRef<'a> { ) -> &'tcx [(CrateNum, LinkagePreference)] { tcx.arena.alloc_from_iter( self.root.dylib_dependency_formats.decode(self).enumerate().flat_map(|(i, link)| { - let cnum = CrateNum::new(i + 1); + let cnum = CrateNum::new(i + 1); // We skipped LOCAL_CRATE when encoding link.map(|link| (self.cnum_map[cnum], link)) }), ) diff --git a/compiler/rustc_metadata/src/rmeta/encoder.rs b/compiler/rustc_metadata/src/rmeta/encoder.rs index fff6f3f804fc3..c538ab99fb54b 100644 --- a/compiler/rustc_metadata/src/rmeta/encoder.rs +++ b/compiler/rustc_metadata/src/rmeta/encoder.rs @@ -2165,12 +2165,14 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> { empty_proc_macro!(self); let formats = self.tcx.dependency_formats(()); if let Some(arr) = formats.get(&CrateType::Dylib) { - return self.lazy_array(arr.iter().map(|slot| match *slot { - Linkage::NotLinked | Linkage::IncludedFromDylib => None, + return self.lazy_array(arr.iter().skip(1 /* skip LOCAL_CRATE */).map( + |slot| match *slot { + Linkage::NotLinked | Linkage::IncludedFromDylib => None, - Linkage::Dynamic => Some(LinkagePreference::RequireDynamic), - Linkage::Static => Some(LinkagePreference::RequireStatic), - })); + Linkage::Dynamic => Some(LinkagePreference::RequireDynamic), + Linkage::Static => Some(LinkagePreference::RequireStatic), + }, + )); } LazyArray::default() } diff --git a/compiler/rustc_middle/src/middle/dependency_format.rs b/compiler/rustc_middle/src/middle/dependency_format.rs index e3b40b6415784..4f613e97631d0 100644 --- a/compiler/rustc_middle/src/middle/dependency_format.rs +++ b/compiler/rustc_middle/src/middle/dependency_format.rs @@ -8,13 +8,13 @@ // this will introduce circular dependency between rustc_metadata and rustc_middle use rustc_data_structures::fx::FxIndexMap; +use rustc_hir::def_id::CrateNum; +use rustc_index::IndexVec; use rustc_macros::{Decodable, Encodable, HashStable}; use rustc_session::config::CrateType; /// A list of dependencies for a certain crate type. -/// -/// The length of this vector is the same as the number of external crates used. -pub type DependencyList = Vec; +pub type DependencyList = IndexVec; /// A mapping of all required dependencies for a particular flavor of output. /// diff --git a/src/tools/miri/src/helpers.rs b/src/tools/miri/src/helpers.rs index ef4543dcee862..cb0040fcaf1d0 100644 --- a/src/tools/miri/src/helpers.rs +++ b/src/tools/miri/src/helpers.rs @@ -151,12 +151,14 @@ pub fn iter_exported_symbols<'tcx>( let dependency_format = dependency_formats .get(&CrateType::Executable) .expect("interpreting a non-executable crate"); - for cnum in dependency_format.iter().enumerate().filter_map(|(num, &linkage)| { - // We add 1 to the number because that's what rustc also does everywhere it - // calls `CrateNum::new`... - #[expect(clippy::arithmetic_side_effects)] - (linkage != Linkage::NotLinked).then_some(CrateNum::new(num + 1)) - }) { + for cnum in dependency_format + .iter_enumerated() + .filter_map(|(num, &linkage)| (linkage != Linkage::NotLinked).then_some(num)) + { + if cnum == LOCAL_CRATE { + continue; // Already handled above + } + // We can ignore `_export_info` here: we are a Rust crate, and everything is exported // from a Rust crate. for &(symbol, _export_info) in tcx.exported_symbols(cnum) { From 943f6a8ca9e9a52eb34ae927ecbd45d2d829d85d Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Fri, 13 Dec 2024 16:27:07 +0000 Subject: [PATCH 12/28] Update comments --- compiler/rustc_metadata/src/dependency_format.rs | 15 +++++++-------- compiler/rustc_session/src/cstore.rs | 2 +- 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/compiler/rustc_metadata/src/dependency_format.rs b/compiler/rustc_metadata/src/dependency_format.rs index 1d4083a07d30b..6c5e59e49f7ad 100644 --- a/compiler/rustc_metadata/src/dependency_format.rs +++ b/compiler/rustc_metadata/src/dependency_format.rs @@ -229,7 +229,7 @@ fn calculate_type(tcx: TyCtxt<'_>, ty: CrateType) -> DependencyList { // static libraries. // // If the crate hasn't been included yet and it's not actually required - // (e.g., it's an allocator) then we skip it here as well. + // (e.g., it's a panic runtime) then we skip it here as well. for &cnum in tcx.crates(()).iter() { let src = tcx.used_crate_source(cnum); if src.dylib.is_none() @@ -247,8 +247,7 @@ fn calculate_type(tcx: TyCtxt<'_>, ty: CrateType) -> DependencyList { // artifact which means that we may need to inject dependencies of some // form. // - // Things like allocators and panic runtimes may not have been activated - // quite yet, so do so here. + // Things like panic runtimes may not have been activated quite yet, so do so here. activate_injected_dep(CStore::from_tcx(tcx).injected_panic_runtime(), &mut ret, &|cnum| { tcx.is_panic_runtime(cnum) }); @@ -355,9 +354,9 @@ fn attempt_static(tcx: TyCtxt<'_>, unavailable: &mut Vec) -> Option, list: &DependencyList) { let sess = &tcx.sess; if list.is_empty() { diff --git a/compiler/rustc_session/src/cstore.rs b/compiler/rustc_session/src/cstore.rs index beae9dc278c15..c8a5c22ad1230 100644 --- a/compiler/rustc_session/src/cstore.rs +++ b/compiler/rustc_session/src/cstore.rs @@ -42,7 +42,7 @@ pub enum CrateDepKind { /// A dependency that is only used for its macros. MacrosOnly, /// A dependency that is always injected into the dependency list and so - /// doesn't need to be linked to an rlib, e.g., the injected allocator. + /// doesn't need to be linked to an rlib, e.g., the injected panic runtime. Implicit, /// A dependency that is required by an rlib version of this crate. /// Ordinary `extern crate`s result in `Explicit` dependencies. From 544809e48a3de2de20c15bc5488b889cd375215e Mon Sep 17 00:00:00 2001 From: Zalathar Date: Wed, 18 Dec 2024 13:53:23 +1100 Subject: [PATCH 13/28] coverage: Rename `basic_coverage_blocks` to just `graph` During coverage instrumentation, this variable always holds the coverage graph, which is a simplified view of the MIR control-flow graph. The new name is clearer in context, and also shorter. --- .../src/coverage/mappings.rs | 20 +++-- .../rustc_mir_transform/src/coverage/mod.rs | 40 ++++----- .../rustc_mir_transform/src/coverage/spans.rs | 9 +-- .../src/coverage/spans/from_mir.rs | 4 +- .../rustc_mir_transform/src/coverage/tests.rs | 81 +++++++------------ 5 files changed, 61 insertions(+), 93 deletions(-) diff --git a/compiler/rustc_mir_transform/src/coverage/mappings.rs b/compiler/rustc_mir_transform/src/coverage/mappings.rs index 2db7c6cf1d6fd..4185b3f4d4d85 100644 --- a/compiler/rustc_mir_transform/src/coverage/mappings.rs +++ b/compiler/rustc_mir_transform/src/coverage/mappings.rs @@ -80,7 +80,7 @@ pub(super) fn extract_all_mapping_info_from_mir<'tcx>( tcx: TyCtxt<'tcx>, mir_body: &mir::Body<'tcx>, hir_info: &ExtractedHirInfo, - basic_coverage_blocks: &CoverageGraph, + graph: &CoverageGraph, ) -> ExtractedMappings { let mut code_mappings = vec![]; let mut branch_pairs = vec![]; @@ -102,23 +102,23 @@ pub(super) fn extract_all_mapping_info_from_mir<'tcx>( } } else { // Extract coverage spans from MIR statements/terminators as normal. - extract_refined_covspans(mir_body, hir_info, basic_coverage_blocks, &mut code_mappings); + extract_refined_covspans(mir_body, hir_info, graph, &mut code_mappings); } - branch_pairs.extend(extract_branch_pairs(mir_body, hir_info, basic_coverage_blocks)); + branch_pairs.extend(extract_branch_pairs(mir_body, hir_info, graph)); extract_mcdc_mappings( mir_body, tcx, hir_info.body_span, - basic_coverage_blocks, + graph, &mut mcdc_bitmap_bits, &mut mcdc_degraded_branches, &mut mcdc_mappings, ); ExtractedMappings { - num_bcbs: basic_coverage_blocks.num_nodes(), + num_bcbs: graph.num_nodes(), code_mappings, branch_pairs, mcdc_bitmap_bits, @@ -211,7 +211,7 @@ fn resolve_block_markers( pub(super) fn extract_branch_pairs( mir_body: &mir::Body<'_>, hir_info: &ExtractedHirInfo, - basic_coverage_blocks: &CoverageGraph, + graph: &CoverageGraph, ) -> Vec { let Some(coverage_info_hi) = mir_body.coverage_info_hi.as_deref() else { return vec![] }; @@ -228,8 +228,7 @@ pub(super) fn extract_branch_pairs( } let span = unexpand_into_body_span(raw_span, hir_info.body_span)?; - let bcb_from_marker = - |marker: BlockMarkerId| basic_coverage_blocks.bcb_from_bb(block_markers[marker]?); + let bcb_from_marker = |marker: BlockMarkerId| graph.bcb_from_bb(block_markers[marker]?); let true_bcb = bcb_from_marker(true_marker)?; let false_bcb = bcb_from_marker(false_marker)?; @@ -243,7 +242,7 @@ pub(super) fn extract_mcdc_mappings( mir_body: &mir::Body<'_>, tcx: TyCtxt<'_>, body_span: Span, - basic_coverage_blocks: &CoverageGraph, + graph: &CoverageGraph, mcdc_bitmap_bits: &mut usize, mcdc_degraded_branches: &mut impl Extend, mcdc_mappings: &mut impl Extend<(MCDCDecision, Vec)>, @@ -252,8 +251,7 @@ pub(super) fn extract_mcdc_mappings( let block_markers = resolve_block_markers(coverage_info_hi, mir_body); - let bcb_from_marker = - |marker: BlockMarkerId| basic_coverage_blocks.bcb_from_bb(block_markers[marker]?); + let bcb_from_marker = |marker: BlockMarkerId| graph.bcb_from_bb(block_markers[marker]?); let check_branch_bcb = |raw_span: Span, true_marker: BlockMarkerId, false_marker: BlockMarkerId| { diff --git a/compiler/rustc_mir_transform/src/coverage/mod.rs b/compiler/rustc_mir_transform/src/coverage/mod.rs index 83e7ff99639d1..57956448414b5 100644 --- a/compiler/rustc_mir_transform/src/coverage/mod.rs +++ b/compiler/rustc_mir_transform/src/coverage/mod.rs @@ -71,16 +71,15 @@ fn instrument_function_for_coverage<'tcx>(tcx: TyCtxt<'tcx>, mir_body: &mut mir: let _span = debug_span!("instrument_function_for_coverage", ?def_id).entered(); let hir_info = extract_hir_info(tcx, def_id.expect_local()); - let basic_coverage_blocks = CoverageGraph::from_mir(mir_body); + + // Build the coverage graph, which is a simplified view of the MIR control-flow + // graph that ignores some details not relevant to coverage instrumentation. + let graph = CoverageGraph::from_mir(mir_body); //////////////////////////////////////////////////// // Extract coverage spans and other mapping info from MIR. - let extracted_mappings = mappings::extract_all_mapping_info_from_mir( - tcx, - mir_body, - &hir_info, - &basic_coverage_blocks, - ); + let extracted_mappings = + mappings::extract_all_mapping_info_from_mir(tcx, mir_body, &hir_info, &graph); //////////////////////////////////////////////////// // Create an optimized mix of `Counter`s and `Expression`s for the `CoverageGraph`. Ensure @@ -94,7 +93,7 @@ fn instrument_function_for_coverage<'tcx>(tcx: TyCtxt<'tcx>, mir_body: &mut mir: } let coverage_counters = - CoverageCounters::make_bcb_counters(&basic_coverage_blocks, &bcbs_with_counter_mappings); + CoverageCounters::make_bcb_counters(&graph, &bcbs_with_counter_mappings); let mappings = create_mappings(&extracted_mappings, &coverage_counters); if mappings.is_empty() { @@ -103,14 +102,9 @@ fn instrument_function_for_coverage<'tcx>(tcx: TyCtxt<'tcx>, mir_body: &mut mir: return; } - inject_coverage_statements( - mir_body, - &basic_coverage_blocks, - &extracted_mappings, - &coverage_counters, - ); + inject_coverage_statements(mir_body, &graph, &extracted_mappings, &coverage_counters); - inject_mcdc_statements(mir_body, &basic_coverage_blocks, &extracted_mappings); + inject_mcdc_statements(mir_body, &graph, &extracted_mappings); let mcdc_num_condition_bitmaps = extracted_mappings .mcdc_mappings @@ -243,7 +237,7 @@ fn create_mappings( /// inject any necessary coverage statements into MIR. fn inject_coverage_statements<'tcx>( mir_body: &mut mir::Body<'tcx>, - basic_coverage_blocks: &CoverageGraph, + graph: &CoverageGraph, extracted_mappings: &ExtractedMappings, coverage_counters: &CoverageCounters, ) { @@ -253,12 +247,12 @@ fn inject_coverage_statements<'tcx>( // For BCB nodes this is just their first block, but for edges we need // to create a new block between the two BCBs, and inject into that. let target_bb = match site { - Site::Node { bcb } => basic_coverage_blocks[bcb].leader_bb(), + Site::Node { bcb } => graph[bcb].leader_bb(), Site::Edge { from_bcb, to_bcb } => { // Create a new block between the last block of `from_bcb` and // the first block of `to_bcb`. - let from_bb = basic_coverage_blocks[from_bcb].last_bb(); - let to_bb = basic_coverage_blocks[to_bcb].leader_bb(); + let from_bb = graph[from_bcb].last_bb(); + let to_bb = graph[to_bcb].leader_bb(); let new_bb = inject_edge_counter_basic_block(mir_body, from_bb, to_bb); debug!( @@ -291,7 +285,7 @@ fn inject_coverage_statements<'tcx>( inject_statement( mir_body, CoverageKind::ExpressionUsed { id: expression_id }, - basic_coverage_blocks[bcb].leader_bb(), + graph[bcb].leader_bb(), ); } } @@ -300,13 +294,13 @@ fn inject_coverage_statements<'tcx>( /// For each decision inject statements to update test vector bitmap after it has been evaluated. fn inject_mcdc_statements<'tcx>( mir_body: &mut mir::Body<'tcx>, - basic_coverage_blocks: &CoverageGraph, + graph: &CoverageGraph, extracted_mappings: &ExtractedMappings, ) { for (decision, conditions) in &extracted_mappings.mcdc_mappings { // Inject test vector update first because `inject_statement` always insert new statement at head. for &end in &decision.end_bcbs { - let end_bb = basic_coverage_blocks[end].leader_bb(); + let end_bb = graph[end].leader_bb(); inject_statement( mir_body, CoverageKind::TestVectorBitmapUpdate { @@ -327,7 +321,7 @@ fn inject_mcdc_statements<'tcx>( } in conditions { for (index, bcb) in [(false_index, false_bcb), (true_index, true_bcb)] { - let bb = basic_coverage_blocks[bcb].leader_bb(); + let bb = graph[bcb].leader_bb(); inject_statement( mir_body, CoverageKind::CondBitmapUpdate { diff --git a/compiler/rustc_mir_transform/src/coverage/spans.rs b/compiler/rustc_mir_transform/src/coverage/spans.rs index 085c738f1f9fc..314a86ea52f0a 100644 --- a/compiler/rustc_mir_transform/src/coverage/spans.rs +++ b/compiler/rustc_mir_transform/src/coverage/spans.rs @@ -17,14 +17,13 @@ mod from_mir; pub(super) fn extract_refined_covspans( mir_body: &mir::Body<'_>, hir_info: &ExtractedHirInfo, - basic_coverage_blocks: &CoverageGraph, + graph: &CoverageGraph, code_mappings: &mut impl Extend, ) { - let ExtractedCovspans { mut covspans } = - extract_covspans_from_mir(mir_body, hir_info, basic_coverage_blocks); + let ExtractedCovspans { mut covspans } = extract_covspans_from_mir(mir_body, hir_info, graph); // First, perform the passes that need macro information. - covspans.sort_by(|a, b| basic_coverage_blocks.cmp_in_dominator_order(a.bcb, b.bcb)); + covspans.sort_by(|a, b| graph.cmp_in_dominator_order(a.bcb, b.bcb)); remove_unwanted_expansion_spans(&mut covspans); split_visible_macro_spans(&mut covspans); @@ -34,7 +33,7 @@ pub(super) fn extract_refined_covspans( let compare_covspans = |a: &Covspan, b: &Covspan| { compare_spans(a.span, b.span) // After deduplication, we want to keep only the most-dominated BCB. - .then_with(|| basic_coverage_blocks.cmp_in_dominator_order(a.bcb, b.bcb).reverse()) + .then_with(|| graph.cmp_in_dominator_order(a.bcb, b.bcb).reverse()) }; covspans.sort_by(compare_covspans); diff --git a/compiler/rustc_mir_transform/src/coverage/spans/from_mir.rs b/compiler/rustc_mir_transform/src/coverage/spans/from_mir.rs index 824d657e1fc2d..26ce743be3613 100644 --- a/compiler/rustc_mir_transform/src/coverage/spans/from_mir.rs +++ b/compiler/rustc_mir_transform/src/coverage/spans/from_mir.rs @@ -22,13 +22,13 @@ pub(crate) struct ExtractedCovspans { pub(crate) fn extract_covspans_from_mir( mir_body: &mir::Body<'_>, hir_info: &ExtractedHirInfo, - basic_coverage_blocks: &CoverageGraph, + graph: &CoverageGraph, ) -> ExtractedCovspans { let &ExtractedHirInfo { body_span, .. } = hir_info; let mut covspans = vec![]; - for (bcb, bcb_data) in basic_coverage_blocks.iter_enumerated() { + for (bcb, bcb_data) in graph.iter_enumerated() { bcb_to_initial_coverage_spans(mir_body, body_span, bcb, bcb_data, &mut covspans); } diff --git a/compiler/rustc_mir_transform/src/coverage/tests.rs b/compiler/rustc_mir_transform/src/coverage/tests.rs index 233ca9981c50c..b2ee50de50a23 100644 --- a/compiler/rustc_mir_transform/src/coverage/tests.rs +++ b/compiler/rustc_mir_transform/src/coverage/tests.rs @@ -223,16 +223,12 @@ fn print_mir_graphviz(name: &str, mir_body: &Body<'_>) { } } -fn print_coverage_graphviz( - name: &str, - mir_body: &Body<'_>, - basic_coverage_blocks: &graph::CoverageGraph, -) { +fn print_coverage_graphviz(name: &str, mir_body: &Body<'_>, graph: &graph::CoverageGraph) { if PRINT_GRAPHS { println!( "digraph {} {{\n{}\n}}", name, - basic_coverage_blocks + graph .iter_enumerated() .map(|(bcb, bcb_data)| { format!( @@ -240,7 +236,7 @@ fn print_coverage_graphviz( bcb, bcb, mir_body[bcb_data.last_bb()].terminator().kind.name(), - basic_coverage_blocks + graph .successors(bcb) .map(|successor| { format!(" {:?} -> {:?};", bcb, successor) }) .join("\n") @@ -300,11 +296,11 @@ fn goto_switchint<'a>() -> Body<'a> { #[track_caller] fn assert_successors( - basic_coverage_blocks: &graph::CoverageGraph, + graph: &graph::CoverageGraph, bcb: BasicCoverageBlock, expected_successors: &[BasicCoverageBlock], ) { - let mut successors = basic_coverage_blocks.successors[bcb].clone(); + let mut successors = graph.successors[bcb].clone(); successors.sort_unstable(); assert_eq!(successors, expected_successors); } @@ -315,8 +311,8 @@ fn test_covgraph_goto_switchint() { if false { eprintln!("basic_blocks = {}", debug_basic_blocks(&mir_body)); } - let basic_coverage_blocks = graph::CoverageGraph::from_mir(&mir_body); - print_coverage_graphviz("covgraph_goto_switchint ", &mir_body, &basic_coverage_blocks); + let graph = graph::CoverageGraph::from_mir(&mir_body); + print_coverage_graphviz("covgraph_goto_switchint ", &mir_body, &graph); /* ┌──────────────┐ ┌─────────────────┐ │ bcb2: Return │ ◀── │ bcb0: SwitchInt │ @@ -328,16 +324,11 @@ fn test_covgraph_goto_switchint() { │ bcb1: Return │ └─────────────────┘ */ - assert_eq!( - basic_coverage_blocks.num_nodes(), - 3, - "basic_coverage_blocks: {:?}", - basic_coverage_blocks.iter_enumerated().collect::>() - ); + assert_eq!(graph.num_nodes(), 3, "graph: {:?}", graph.iter_enumerated().collect::>()); - assert_successors(&basic_coverage_blocks, bcb(0), &[bcb(1), bcb(2)]); - assert_successors(&basic_coverage_blocks, bcb(1), &[]); - assert_successors(&basic_coverage_blocks, bcb(2), &[]); + assert_successors(&graph, bcb(0), &[bcb(1), bcb(2)]); + assert_successors(&graph, bcb(1), &[]); + assert_successors(&graph, bcb(2), &[]); } /// Create a mock `Body` with a loop. @@ -383,12 +374,8 @@ fn switchint_then_loop_else_return<'a>() -> Body<'a> { #[test] fn test_covgraph_switchint_then_loop_else_return() { let mir_body = switchint_then_loop_else_return(); - let basic_coverage_blocks = graph::CoverageGraph::from_mir(&mir_body); - print_coverage_graphviz( - "covgraph_switchint_then_loop_else_return", - &mir_body, - &basic_coverage_blocks, - ); + let graph = graph::CoverageGraph::from_mir(&mir_body); + print_coverage_graphviz("covgraph_switchint_then_loop_else_return", &mir_body, &graph); /* ┌─────────────────┐ │ bcb0: Call │ @@ -408,17 +395,12 @@ fn test_covgraph_switchint_then_loop_else_return() { │ │ └─────────────────────────────────────┘ */ - assert_eq!( - basic_coverage_blocks.num_nodes(), - 4, - "basic_coverage_blocks: {:?}", - basic_coverage_blocks.iter_enumerated().collect::>() - ); + assert_eq!(graph.num_nodes(), 4, "graph: {:?}", graph.iter_enumerated().collect::>()); - assert_successors(&basic_coverage_blocks, bcb(0), &[bcb(1)]); - assert_successors(&basic_coverage_blocks, bcb(1), &[bcb(2), bcb(3)]); - assert_successors(&basic_coverage_blocks, bcb(2), &[]); - assert_successors(&basic_coverage_blocks, bcb(3), &[bcb(1)]); + assert_successors(&graph, bcb(0), &[bcb(1)]); + assert_successors(&graph, bcb(1), &[bcb(2), bcb(3)]); + assert_successors(&graph, bcb(2), &[]); + assert_successors(&graph, bcb(3), &[bcb(1)]); } /// Create a mock `Body` with nested loops. @@ -494,11 +476,11 @@ fn switchint_loop_then_inner_loop_else_break<'a>() -> Body<'a> { #[test] fn test_covgraph_switchint_loop_then_inner_loop_else_break() { let mir_body = switchint_loop_then_inner_loop_else_break(); - let basic_coverage_blocks = graph::CoverageGraph::from_mir(&mir_body); + let graph = graph::CoverageGraph::from_mir(&mir_body); print_coverage_graphviz( "covgraph_switchint_loop_then_inner_loop_else_break", &mir_body, - &basic_coverage_blocks, + &graph, ); /* ┌─────────────────┐ @@ -531,18 +513,13 @@ fn test_covgraph_switchint_loop_then_inner_loop_else_break() { │ │ └────────────────────────────────────────────┘ */ - assert_eq!( - basic_coverage_blocks.num_nodes(), - 7, - "basic_coverage_blocks: {:?}", - basic_coverage_blocks.iter_enumerated().collect::>() - ); - - assert_successors(&basic_coverage_blocks, bcb(0), &[bcb(1)]); - assert_successors(&basic_coverage_blocks, bcb(1), &[bcb(2), bcb(3)]); - assert_successors(&basic_coverage_blocks, bcb(2), &[]); - assert_successors(&basic_coverage_blocks, bcb(3), &[bcb(4)]); - assert_successors(&basic_coverage_blocks, bcb(4), &[bcb(5), bcb(6)]); - assert_successors(&basic_coverage_blocks, bcb(5), &[bcb(1)]); - assert_successors(&basic_coverage_blocks, bcb(6), &[bcb(4)]); + assert_eq!(graph.num_nodes(), 7, "graph: {:?}", graph.iter_enumerated().collect::>()); + + assert_successors(&graph, bcb(0), &[bcb(1)]); + assert_successors(&graph, bcb(1), &[bcb(2), bcb(3)]); + assert_successors(&graph, bcb(2), &[]); + assert_successors(&graph, bcb(3), &[bcb(4)]); + assert_successors(&graph, bcb(4), &[bcb(5), bcb(6)]); + assert_successors(&graph, bcb(5), &[bcb(1)]); + assert_successors(&graph, bcb(6), &[bcb(4)]); } From 0daa921f0e95acef303f3c091a09d667f9b63405 Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Fri, 20 Dec 2024 08:35:02 +0000 Subject: [PATCH 14/28] Review comments --- compiler/rustc_codegen_ssa/src/back/link.rs | 5 ++--- compiler/rustc_metadata/src/dependency_format.rs | 7 +++++++ 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/compiler/rustc_codegen_ssa/src/back/link.rs b/compiler/rustc_codegen_ssa/src/back/link.rs index 66258790c1ebd..562af0777f52f 100644 --- a/compiler/rustc_codegen_ssa/src/back/link.rs +++ b/compiler/rustc_codegen_ssa/src/back/link.rs @@ -234,8 +234,6 @@ pub fn each_linked_rlib( crate_type: Option, f: &mut dyn FnMut(CrateNum, &Path), ) -> Result<(), errors::LinkRlibError> { - let crates = info.used_crates.iter(); - let fmts = if let Some(crate_type) = crate_type { let Some(fmts) = info.dependency_formats.get(&crate_type) else { return Err(errors::LinkRlibError::MissingFormat); @@ -261,7 +259,8 @@ pub fn each_linked_rlib( info.dependency_formats.first().unwrap().1 }; - for &cnum in crates { + let used_dep_crates = info.used_crates.iter(); + for &cnum in used_dep_crates { match fmts.get(cnum) { Some(&Linkage::NotLinked | &Linkage::Dynamic | &Linkage::IncludedFromDylib) => continue, Some(_) => {} diff --git a/compiler/rustc_metadata/src/dependency_format.rs b/compiler/rustc_metadata/src/dependency_format.rs index 6c5e59e49f7ad..582c2215d92e1 100644 --- a/compiler/rustc_metadata/src/dependency_format.rs +++ b/compiler/rustc_metadata/src/dependency_format.rs @@ -212,7 +212,14 @@ fn calculate_type(tcx: TyCtxt<'_>, ty: CrateType) -> DependencyList { // Collect what we've got so far in the return vector. let last_crate = tcx.crates(()).len(); let mut ret = IndexVec::new(); + + // We need to fill in something for LOCAL_CRATE as IndexVec is a dense map. + // Linkage::Static semantically the most correct thing to use as the local + // crate is always statically linked into the linker output, even when + // linking a dylib. Using Linkage::Static also allow avoiding special cases + // for LOCAL_CRATE in some places. assert_eq!(ret.push(Linkage::Static), LOCAL_CRATE); + for cnum in 1..last_crate + 1 { let cnum = CrateNum::new(cnum); assert_eq!( From adf549808e42fe6155f043aee34a806fdc3de4c1 Mon Sep 17 00:00:00 2001 From: lcnr Date: Fri, 20 Dec 2024 11:45:52 +0100 Subject: [PATCH 15/28] add comments --- compiler/rustc_borrowck/src/region_infer/mod.rs | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/compiler/rustc_borrowck/src/region_infer/mod.rs b/compiler/rustc_borrowck/src/region_infer/mod.rs index d39fbf32921ac..60f7770d3f792 100644 --- a/compiler/rustc_borrowck/src/region_infer/mod.rs +++ b/compiler/rustc_borrowck/src/region_infer/mod.rs @@ -795,7 +795,6 @@ impl<'tcx> RegionInferenceContext<'tcx> { // If the member region lives in a higher universe, we currently choose // the most conservative option by leaving it unchanged. - if !self.constraint_sccs().annotation(scc).min_universe().is_root() { return; } @@ -823,12 +822,14 @@ impl<'tcx> RegionInferenceContext<'tcx> { } debug!(?choice_regions, "after ub"); - // At this point we can pick any member of `choice_regions`, but to avoid potential - // non-determinism we will pick the *unique minimum* choice. + // At this point we can pick any member of `choice_regions` and would like to choose + // it to be a small as possible. To avoid potential non-determinism we will pick the + // smallest such choice. // // Because universal regions are only partially ordered (i.e, not every two regions are // comparable), we will ignore any region that doesn't compare to all others when picking // the minimum choice. + // // For example, consider `choice_regions = ['static, 'a, 'b, 'c, 'd, 'e]`, where // `'static: 'a, 'static: 'b, 'a: 'c, 'b: 'c, 'c: 'd, 'c: 'e`. // `['d, 'e]` are ignored because they do not compare - the same goes for `['a, 'b]`. @@ -853,6 +854,8 @@ impl<'tcx> RegionInferenceContext<'tcx> { return; }; + // As we require `'scc: 'min_choice`, we have definitely already computed + // its `scc_values` at this point. let min_choice_scc = self.constraint_sccs.scc(min_choice); debug!(?min_choice, ?min_choice_scc); if self.scc_values.add_region(scc, min_choice_scc) { From fae72074c69c2feea0ca7a1522d201a438f2f96b Mon Sep 17 00:00:00 2001 From: Adrian Taylor Date: Thu, 19 Dec 2024 17:00:58 +0000 Subject: [PATCH 16/28] Arbitrary self types v2: no deshadow pre feature. The arbitrary self types v2 work introduces a check for shadowed methods, whereby a method in some "outer" smart pointer type may called in preference to a method in the inner referent. This is bad if the outer pointer adds a method later, as it may change behavior, so we ensure we error in this circumstance. It was intended that this new shadowing detection system only comes into play for users who enable the `arbitrary_self_types` feature (or of course everyone later if it's stabilized). It was believed that the new deshadowing code couldn't be reached without building the custom smart pointers that `arbitrary_self_types` enables, and therefore there was no risk of this code impacting existing users. However, it turns out that cunning use of `Pin::get_ref` can cause this type of shadowing error to be emitted now. This commit adds a test for this case. --- compiler/rustc_hir_typeck/src/method/probe.rs | 9 +++++++ ...trary_self_types_pin_getref.feature.stderr | 16 ++++++++++++ .../self/arbitrary_self_types_pin_getref.rs | 25 +++++++++++++++++++ 3 files changed, 50 insertions(+) create mode 100644 tests/ui/self/arbitrary_self_types_pin_getref.feature.stderr create mode 100644 tests/ui/self/arbitrary_self_types_pin_getref.rs diff --git a/compiler/rustc_hir_typeck/src/method/probe.rs b/compiler/rustc_hir_typeck/src/method/probe.rs index 3b377076545da..d287a6a3a028f 100644 --- a/compiler/rustc_hir_typeck/src/method/probe.rs +++ b/compiler/rustc_hir_typeck/src/method/probe.rs @@ -1337,6 +1337,15 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> { mutbl: hir::Mutability, track_unstable_candidates: bool, ) -> Result<(), MethodError<'tcx>> { + // The errors emitted by this function are part of + // the arbitrary self types work, and should not impact + // other users. + if !self.tcx.features().arbitrary_self_types() + && !self.tcx.features().arbitrary_self_types_pointers() + { + return Ok(()); + } + // We don't want to remember any of the diagnostic hints from this // shadow search, but we do need to provide Some/None for the // unstable_candidates in order to reflect the behavior of the diff --git a/tests/ui/self/arbitrary_self_types_pin_getref.feature.stderr b/tests/ui/self/arbitrary_self_types_pin_getref.feature.stderr new file mode 100644 index 0000000000000..52cf69f33a54b --- /dev/null +++ b/tests/ui/self/arbitrary_self_types_pin_getref.feature.stderr @@ -0,0 +1,16 @@ +error[E0034]: multiple applicable items in scope + --> $DIR/arbitrary_self_types_pin_getref.rs:23:22 + | +LL | let _ = pinned_a.get_ref(); + | ^^^^^^^ multiple `get_ref` found + | +note: candidate #1 is defined in an impl for the type `A` + --> $DIR/arbitrary_self_types_pin_getref.rs:17:5 + | +LL | fn get_ref(self: &Pin<&A>) {} // note &Pin + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ + = note: candidate #2 is defined in an impl for the type `Pin<&'a T>` + +error: aborting due to 1 previous error + +For more information about this error, try `rustc --explain E0034`. diff --git a/tests/ui/self/arbitrary_self_types_pin_getref.rs b/tests/ui/self/arbitrary_self_types_pin_getref.rs new file mode 100644 index 0000000000000..29dd907f7ff00 --- /dev/null +++ b/tests/ui/self/arbitrary_self_types_pin_getref.rs @@ -0,0 +1,25 @@ +// Confirms that Pin::get_ref can no longer shadow methods in pointees +// once arbitrary_self_types is enabled. +// +//@ revisions: default feature +#![cfg_attr(feature, feature(arbitrary_self_types))] + +//@[default] check-pass + +#![allow(dead_code)] + +use std::pin::Pin; +use std::pin::pin; + +struct A; + +impl A { + fn get_ref(self: &Pin<&A>) {} // note &Pin +} + +fn main() { + let pinned_a: Pin<&mut A> = pin!(A); + let pinned_a: Pin<&A> = pinned_a.as_ref(); + let _ = pinned_a.get_ref(); + //[feature]~^ ERROR: multiple applicable items +} From 1abe485e8952d6b6254b5d06d677e9842e6a7158 Mon Sep 17 00:00:00 2001 From: The 8472 Date: Fri, 20 Dec 2024 13:37:45 +0100 Subject: [PATCH 17/28] fix missing ns units in bootstrap's benchmark rendering --- src/bootstrap/src/utils/render_tests.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/bootstrap/src/utils/render_tests.rs b/src/bootstrap/src/utils/render_tests.rs index eb2c8254dc0f4..46b250555f2d5 100644 --- a/src/bootstrap/src/utils/render_tests.rs +++ b/src/bootstrap/src/utils/render_tests.rs @@ -242,7 +242,7 @@ impl<'a> Renderer<'a> { for bench in &self.benches { rows.push(( &bench.name, - format!("{:.2?}/iter", bench.median), + format!("{:.2?}ns/iter", bench.median), format!("+/- {:.2?}", bench.deviation), )); } From 8b2b6359f97ee9ac8fb1b5c35e86211222d02574 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Fri, 20 Dec 2024 15:03:51 +0100 Subject: [PATCH 18/28] mri: add track_caller to thread spawning methods for better backtraces --- library/std/src/sys/pal/unix/thread.rs | 1 + library/std/src/sys/pal/windows/thread.rs | 1 + library/std/src/thread/mod.rs | 4 ++++ 3 files changed, 6 insertions(+) diff --git a/library/std/src/sys/pal/unix/thread.rs b/library/std/src/sys/pal/unix/thread.rs index 131a6e81b1e92..e360ba0f6d7d8 100644 --- a/library/std/src/sys/pal/unix/thread.rs +++ b/library/std/src/sys/pal/unix/thread.rs @@ -45,6 +45,7 @@ unsafe impl Sync for Thread {} impl Thread { // unsafe: see thread::Builder::spawn_unchecked for safety requirements + #[cfg_attr(miri, track_caller)] // even without panics, this helps for Miri backtraces pub unsafe fn new(stack: usize, p: Box) -> io::Result { let p = Box::into_raw(Box::new(p)); let mut native: libc::pthread_t = mem::zeroed(); diff --git a/library/std/src/sys/pal/windows/thread.rs b/library/std/src/sys/pal/windows/thread.rs index 2c8ce42f4148b..45e52cf4d047f 100644 --- a/library/std/src/sys/pal/windows/thread.rs +++ b/library/std/src/sys/pal/windows/thread.rs @@ -19,6 +19,7 @@ pub struct Thread { impl Thread { // unsafe: see thread::Builder::spawn_unchecked for safety requirements + #[cfg_attr(miri, track_caller)] // even without panics, this helps for Miri backtraces pub unsafe fn new(stack: usize, p: Box) -> io::Result { let p = Box::into_raw(Box::new(p)); diff --git a/library/std/src/thread/mod.rs b/library/std/src/thread/mod.rs index cfbf6548a380c..85ee369ca2b66 100644 --- a/library/std/src/thread/mod.rs +++ b/library/std/src/thread/mod.rs @@ -391,6 +391,7 @@ impl Builder { /// handler.join().unwrap(); /// ``` #[stable(feature = "rust1", since = "1.0.0")] + #[cfg_attr(miri, track_caller)] // even without panics, this helps for Miri backtraces pub fn spawn(self, f: F) -> io::Result> where F: FnOnce() -> T, @@ -458,6 +459,7 @@ impl Builder { /// /// [`io::Result`]: crate::io::Result #[stable(feature = "thread_spawn_unchecked", since = "1.82.0")] + #[cfg_attr(miri, track_caller)] // even without panics, this helps for Miri backtraces pub unsafe fn spawn_unchecked(self, f: F) -> io::Result> where F: FnOnce() -> T, @@ -467,6 +469,7 @@ impl Builder { Ok(JoinHandle(unsafe { self.spawn_unchecked_(f, None) }?)) } + #[cfg_attr(miri, track_caller)] // even without panics, this helps for Miri backtraces unsafe fn spawn_unchecked_<'scope, F, T>( self, f: F, @@ -721,6 +724,7 @@ impl Builder { /// [`join`]: JoinHandle::join /// [`Err`]: crate::result::Result::Err #[stable(feature = "rust1", since = "1.0.0")] +#[cfg_attr(miri, track_caller)] // even without panics, this helps for Miri backtraces pub fn spawn(f: F) -> JoinHandle where F: FnOnce() -> T, From 701e2f708b95d508f90ffd5a3b234335662ae521 Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Fri, 20 Dec 2024 14:08:24 +0000 Subject: [PATCH 19/28] Reduce the amount of explicit FatalError.raise() Instead use dcx.abort_if_error() or guar.raise_fatal() instead. These guarantee that an error actually happened previously and thus we don't silently abort. --- compiler/rustc_codegen_ssa/src/back/link.rs | 12 ++++++------ compiler/rustc_driver_impl/src/pretty.rs | 9 ++------- compiler/rustc_parse/src/parser/diagnostics.rs | 17 +++++------------ compiler/rustc_parse/src/parser/path.rs | 3 +-- compiler/rustc_session/src/output.rs | 15 ++++++--------- .../src/error_reporting/traits/overflow.rs | 7 ++----- 6 files changed, 22 insertions(+), 41 deletions(-) diff --git a/compiler/rustc_codegen_ssa/src/back/link.rs b/compiler/rustc_codegen_ssa/src/back/link.rs index f4f6161ebbccd..60e2d1978913f 100644 --- a/compiler/rustc_codegen_ssa/src/back/link.rs +++ b/compiler/rustc_codegen_ssa/src/back/link.rs @@ -15,7 +15,7 @@ use rustc_ast::CRATE_NODE_ID; use rustc_data_structures::fx::{FxIndexMap, FxIndexSet}; use rustc_data_structures::memmap::Mmap; use rustc_data_structures::temp_dir::MaybeTempDir; -use rustc_errors::{DiagCtxtHandle, FatalError}; +use rustc_errors::DiagCtxtHandle; use rustc_fs_util::{fix_windows_verbatim_for_gcc, try_canonicalize}; use rustc_hir::def_id::{CrateNum, LOCAL_CRATE}; use rustc_metadata::fs::{METADATA_FILENAME, copy_to_stdout, emit_wrapper_file}; @@ -1039,22 +1039,22 @@ fn link_natively( Err(e) => { let linker_not_found = e.kind() == io::ErrorKind::NotFound; - if linker_not_found { - sess.dcx().emit_err(errors::LinkerNotFound { linker_path, error: e }); + let err = if linker_not_found { + sess.dcx().emit_err(errors::LinkerNotFound { linker_path, error: e }) } else { sess.dcx().emit_err(errors::UnableToExeLinker { linker_path, error: e, command_formatted: format!("{cmd:?}"), - }); - } + }) + }; if sess.target.is_like_msvc && linker_not_found { sess.dcx().emit_note(errors::MsvcMissingLinker); sess.dcx().emit_note(errors::CheckInstalledVisualStudio); sess.dcx().emit_note(errors::InsufficientVSCodeProduct); } - FatalError.raise(); + err.raise_fatal(); } } diff --git a/compiler/rustc_driver_impl/src/pretty.rs b/compiler/rustc_driver_impl/src/pretty.rs index 5a1a873d4bde9..93f3d2ab911f8 100644 --- a/compiler/rustc_driver_impl/src/pretty.rs +++ b/compiler/rustc_driver_impl/src/pretty.rs @@ -4,7 +4,6 @@ use std::cell::Cell; use std::fmt::Write; use rustc_ast_pretty::pprust as pprust_ast; -use rustc_errors::FatalError; use rustc_middle::bug; use rustc_middle::mir::{write_mir_graphviz, write_mir_pretty}; use rustc_middle::ty::{self, TyCtxt}; @@ -311,9 +310,7 @@ pub fn print<'tcx>(sess: &Session, ppm: PpMode, ex: PrintExtra<'tcx>) { let tcx = ex.tcx(); let mut out = String::new(); rustc_hir_analysis::check_crate(tcx); - if tcx.dcx().has_errors().is_some() { - FatalError.raise(); - } + tcx.dcx().abort_if_errors(); debug!("pretty printing THIR tree"); for did in tcx.hir().body_owners() { let _ = writeln!(out, "{:?}:\n{}\n", did, tcx.thir_tree(did)); @@ -324,9 +321,7 @@ pub fn print<'tcx>(sess: &Session, ppm: PpMode, ex: PrintExtra<'tcx>) { let tcx = ex.tcx(); let mut out = String::new(); rustc_hir_analysis::check_crate(tcx); - if tcx.dcx().has_errors().is_some() { - FatalError.raise(); - } + tcx.dcx().abort_if_errors(); debug!("pretty printing THIR flat"); for did in tcx.hir().body_owners() { let _ = writeln!(out, "{:?}:\n{}\n", did, tcx.thir_flat(did)); diff --git a/compiler/rustc_parse/src/parser/diagnostics.rs b/compiler/rustc_parse/src/parser/diagnostics.rs index 7e9b9219e7ac0..aab4e1b1afc1b 100644 --- a/compiler/rustc_parse/src/parser/diagnostics.rs +++ b/compiler/rustc_parse/src/parser/diagnostics.rs @@ -16,8 +16,8 @@ use rustc_ast_pretty::pprust; use rustc_data_structures::fx::FxHashSet; use rustc_data_structures::sync::Lrc; use rustc_errors::{ - Applicability, Diag, DiagCtxtHandle, ErrorGuaranteed, FatalError, PResult, Subdiagnostic, - Suggestions, pluralize, + Applicability, Diag, DiagCtxtHandle, ErrorGuaranteed, PResult, Subdiagnostic, Suggestions, + pluralize, }; use rustc_session::errors::ExprParenthesesNeeded; use rustc_span::edit_distance::find_best_match_for_name; @@ -3023,17 +3023,10 @@ impl<'a> Parser<'a> { } pub(super) fn recover_vcs_conflict_marker(&mut self) { - if let Err(err) = self.err_vcs_conflict_marker() { - err.emit(); - FatalError.raise(); - } - } - - pub(crate) fn err_vcs_conflict_marker(&mut self) -> PResult<'a, ()> { // <<<<<<< let Some(start) = self.conflict_marker(&TokenKind::BinOp(token::Shl), &TokenKind::Lt) else { - return Ok(()); + return; }; let mut spans = Vec::with_capacity(3); spans.push(start); @@ -3063,7 +3056,7 @@ impl<'a> Parser<'a> { self.bump(); } - let mut err = self.dcx().struct_span_err(spans, "encountered diff marker"); + let mut err = self.dcx().struct_span_fatal(spans, "encountered diff marker"); match middlediff3 { // We're using diff3 Some(middlediff3) => { @@ -3106,7 +3099,7 @@ impl<'a> Parser<'a> { visit ", ); - Err(err) + err.emit(); } /// Parse and throw away a parenthesized comma separated diff --git a/compiler/rustc_parse/src/parser/path.rs b/compiler/rustc_parse/src/parser/path.rs index 73612d1da2954..39737b9e13798 100644 --- a/compiler/rustc_parse/src/parser/path.rs +++ b/compiler/rustc_parse/src/parser/path.rs @@ -597,8 +597,7 @@ impl<'a> Parser<'a> { // When encountering severely malformed code where there are several levels of // nested unclosed angle args (`f:: 0 => { self.angle_bracket_nesting -= 1; diff --git a/compiler/rustc_session/src/output.rs b/compiler/rustc_session/src/output.rs index bd103e86e2634..ff0419d06bff9 100644 --- a/compiler/rustc_session/src/output.rs +++ b/compiler/rustc_session/src/output.rs @@ -3,7 +3,6 @@ use std::path::Path; use rustc_ast::{self as ast, attr}; -use rustc_errors::FatalError; use rustc_span::{Span, Symbol, sym}; use crate::Session; @@ -90,11 +89,10 @@ pub fn find_crate_name(sess: &Session, attrs: &[ast::Attribute]) -> Symbol { } pub fn validate_crate_name(sess: &Session, s: Symbol, sp: Option) { - let mut err_count = 0; + let mut guar = None; { if s.is_empty() { - err_count += 1; - sess.dcx().emit_err(CrateNameEmpty { span: sp }); + guar = Some(sess.dcx().emit_err(CrateNameEmpty { span: sp })); } for c in s.as_str().chars() { if c.is_alphanumeric() { @@ -103,8 +101,7 @@ pub fn validate_crate_name(sess: &Session, s: Symbol, sp: Option) { if c == '_' { continue; } - err_count += 1; - sess.dcx().emit_err(InvalidCharacterInCrateName { + guar = Some(sess.dcx().emit_err(InvalidCharacterInCrateName { span: sp, character: c, crate_name: s, @@ -113,12 +110,12 @@ pub fn validate_crate_name(sess: &Session, s: Symbol, sp: Option) { } else { None }, - }); + })); } } - if err_count > 0 { - FatalError.raise(); + if let Some(guar) = guar { + guar.raise_fatal(); } } diff --git a/compiler/rustc_trait_selection/src/error_reporting/traits/overflow.rs b/compiler/rustc_trait_selection/src/error_reporting/traits/overflow.rs index c47c21696911b..d82acc4e0543f 100644 --- a/compiler/rustc_trait_selection/src/error_reporting/traits/overflow.rs +++ b/compiler/rustc_trait_selection/src/error_reporting/traits/overflow.rs @@ -1,8 +1,6 @@ use std::fmt; -use rustc_errors::{ - Diag, E0275, EmissionGuarantee, ErrorGuaranteed, FatalError, struct_span_code_err, -}; +use rustc_errors::{Diag, E0275, EmissionGuarantee, ErrorGuaranteed, struct_span_code_err}; use rustc_hir::def::Namespace; use rustc_hir::def_id::LOCAL_CRATE; use rustc_infer::traits::{Obligation, PredicateObligation}; @@ -52,8 +50,7 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> { ) -> ! { let mut err = self.build_overflow_error(cause, span, suggest_increasing_limit); mutate(&mut err); - err.emit(); - FatalError.raise(); + err.emit().raise_fatal(); } pub fn build_overflow_error( From 96edf411947afdc7867d9515cff2557357a2f9c4 Mon Sep 17 00:00:00 2001 From: Taiki Endo Date: Fri, 20 Dec 2024 23:19:12 +0900 Subject: [PATCH 20/28] tests/codegen/asm: Remove uses of rustc_attrs and lang_items features by using minicore --- tests/codegen/asm/aarch64-clobbers.rs | 12 ++++-------- tests/codegen/asm/avr-clobbers.rs | 12 ++++-------- tests/codegen/asm/hexagon-clobbers.rs | 12 ++++-------- tests/codegen/asm/msp430-clobbers.rs | 12 ++++-------- tests/codegen/asm/powerpc-clobbers.rs | 12 ++++-------- tests/codegen/asm/riscv-clobbers.rs | 12 ++++-------- tests/codegen/asm/s390x-clobbers.rs | 12 ++++-------- tests/codegen/asm/sanitize-llvm.rs | 14 ++++---------- tests/codegen/asm/sparc-clobbers.rs | 12 ++++-------- 9 files changed, 36 insertions(+), 74 deletions(-) diff --git a/tests/codegen/asm/aarch64-clobbers.rs b/tests/codegen/asm/aarch64-clobbers.rs index 900e6629fd2c2..dd3ba1510b5b1 100644 --- a/tests/codegen/asm/aarch64-clobbers.rs +++ b/tests/codegen/asm/aarch64-clobbers.rs @@ -1,3 +1,4 @@ +//@ add-core-stubs //@ revisions: aarch64 aarch64_fixed_x18 aarch64_no_x18 aarch64_reserve_x18 arm64ec //@[aarch64] compile-flags: --target aarch64-unknown-linux-gnu //@[aarch64] needs-llvm-components: aarch64 @@ -14,16 +15,11 @@ // ignore-tidy-linelength #![crate_type = "rlib"] -#![feature(no_core, rustc_attrs, lang_items)] +#![feature(no_core)] #![no_core] -#[lang = "sized"] -trait Sized {} - -#[rustc_builtin_macro] -macro_rules! asm { - () => {}; -} +extern crate minicore; +use minicore::*; // CHECK-LABEL: @cc_clobber // CHECK: call void asm sideeffect "", "~{cc}"() diff --git a/tests/codegen/asm/avr-clobbers.rs b/tests/codegen/asm/avr-clobbers.rs index 6e0c75368e23f..56218cd7bcffb 100644 --- a/tests/codegen/asm/avr-clobbers.rs +++ b/tests/codegen/asm/avr-clobbers.rs @@ -1,18 +1,14 @@ +//@ add-core-stubs //@ assembly-output: emit-asm //@ compile-flags: --target avr-unknown-gnu-atmega328 //@ needs-llvm-components: avr #![crate_type = "rlib"] -#![feature(no_core, rustc_attrs, lang_items, asm_experimental_arch)] +#![feature(no_core, asm_experimental_arch)] #![no_core] -#[lang = "sized"] -trait Sized {} - -#[rustc_builtin_macro] -macro_rules! asm { - () => {}; -} +extern crate minicore; +use minicore::*; // CHECK-LABEL: @sreg_is_clobbered // CHECK: void asm sideeffect "", "~{sreg}"() diff --git a/tests/codegen/asm/hexagon-clobbers.rs b/tests/codegen/asm/hexagon-clobbers.rs index 6bb662ead9940..800b8964669c0 100644 --- a/tests/codegen/asm/hexagon-clobbers.rs +++ b/tests/codegen/asm/hexagon-clobbers.rs @@ -1,19 +1,15 @@ +//@ add-core-stubs //@ revisions: hexagon //@[hexagon] compile-flags: --target hexagon-unknown-linux-musl //@[hexagon] needs-llvm-components: hexagon //@ compile-flags: -Zmerge-functions=disabled #![crate_type = "rlib"] -#![feature(no_core, rustc_attrs, lang_items, asm_experimental_arch)] +#![feature(no_core, asm_experimental_arch)] #![no_core] -#[lang = "sized"] -trait Sized {} - -#[rustc_builtin_macro] -macro_rules! asm { - () => {}; -} +extern crate minicore; +use minicore::*; // CHECK-LABEL: @flags_clobber // CHECK: call void asm sideeffect "", ""() diff --git a/tests/codegen/asm/msp430-clobbers.rs b/tests/codegen/asm/msp430-clobbers.rs index c00c04f3088cb..2c8d29cffc4be 100644 --- a/tests/codegen/asm/msp430-clobbers.rs +++ b/tests/codegen/asm/msp430-clobbers.rs @@ -1,18 +1,14 @@ +//@ add-core-stubs //@ assembly-output: emit-asm //@ compile-flags: --target msp430-none-elf //@ needs-llvm-components: msp430 #![crate_type = "rlib"] -#![feature(no_core, rustc_attrs, lang_items, asm_experimental_arch)] +#![feature(no_core, asm_experimental_arch)] #![no_core] -#[lang = "sized"] -trait Sized {} - -#[rustc_builtin_macro] -macro_rules! asm { - () => {}; -} +extern crate minicore; +use minicore::*; // CHECK-LABEL: @sr_clobber // CHECK: call void asm sideeffect "", "~{sr}"() diff --git a/tests/codegen/asm/powerpc-clobbers.rs b/tests/codegen/asm/powerpc-clobbers.rs index 2832377cef00d..f7fc7eea5d506 100644 --- a/tests/codegen/asm/powerpc-clobbers.rs +++ b/tests/codegen/asm/powerpc-clobbers.rs @@ -1,3 +1,4 @@ +//@ add-core-stubs //@ revisions: powerpc powerpc64 powerpc64le aix64 //@[powerpc] compile-flags: --target powerpc-unknown-linux-gnu //@[powerpc] needs-llvm-components: powerpc @@ -10,16 +11,11 @@ // ignore-tidy-linelength #![crate_type = "rlib"] -#![feature(no_core, rustc_attrs, lang_items, asm_experimental_arch)] +#![feature(no_core, asm_experimental_arch)] #![no_core] -#[lang = "sized"] -trait Sized {} - -#[rustc_builtin_macro] -macro_rules! asm { - () => {}; -} +extern crate minicore; +use minicore::*; // CHECK-LABEL: @cr_clobber // CHECK: call void asm sideeffect "", "~{cr}"() diff --git a/tests/codegen/asm/riscv-clobbers.rs b/tests/codegen/asm/riscv-clobbers.rs index 59b2705a4496a..e55b6731098e7 100644 --- a/tests/codegen/asm/riscv-clobbers.rs +++ b/tests/codegen/asm/riscv-clobbers.rs @@ -1,3 +1,4 @@ +//@ add-core-stubs //@ assembly-output: emit-asm //@ revisions: rv32i rv64i rv32e //@[rv32i] compile-flags: --target riscv32i-unknown-none-elf @@ -9,16 +10,11 @@ // ignore-tidy-linelength #![crate_type = "rlib"] -#![feature(no_core, rustc_attrs, lang_items)] +#![feature(no_core)] #![no_core] -#[lang = "sized"] -trait Sized {} - -#[rustc_builtin_macro] -macro_rules! asm { - () => {}; -} +extern crate minicore; +use minicore::*; // CHECK-LABEL: @flags_clobber // CHECK: call void asm sideeffect "", "~{vtype},~{vl},~{vxsat},~{vxrm}"() diff --git a/tests/codegen/asm/s390x-clobbers.rs b/tests/codegen/asm/s390x-clobbers.rs index 56d82b4b04400..cbb6630553cfa 100644 --- a/tests/codegen/asm/s390x-clobbers.rs +++ b/tests/codegen/asm/s390x-clobbers.rs @@ -1,18 +1,14 @@ +//@ add-core-stubs //@ revisions: s390x //@[s390x] compile-flags: --target s390x-unknown-linux-gnu //@[s390x] needs-llvm-components: systemz #![crate_type = "rlib"] -#![feature(no_core, rustc_attrs, lang_items)] +#![feature(no_core)] #![no_core] -#[lang = "sized"] -trait Sized {} - -#[rustc_builtin_macro] -macro_rules! asm { - () => {}; -} +extern crate minicore; +use minicore::*; // CHECK-LABEL: @cc_clobber // CHECK: call void asm sideeffect "", "~{cc}"() diff --git a/tests/codegen/asm/sanitize-llvm.rs b/tests/codegen/asm/sanitize-llvm.rs index fb332f9a0f3e6..97a7703328428 100644 --- a/tests/codegen/asm/sanitize-llvm.rs +++ b/tests/codegen/asm/sanitize-llvm.rs @@ -1,3 +1,4 @@ +//@ add-core-stubs // FIXME(nagisa): remove the flags below once all targets support `asm!`. //@ compile-flags: --target x86_64-unknown-linux-gnu -Copt-level=0 //@ needs-llvm-components: x86 @@ -5,19 +6,12 @@ // Verify we sanitize the special tokens for the LLVM inline-assembly, ensuring people won't // inadvertently rely on the LLVM-specific syntax and features. #![no_core] -#![feature(no_core, lang_items, rustc_attrs)] +#![feature(no_core)] #![crate_type = "rlib"] #![allow(named_asm_labels)] -#[rustc_builtin_macro] -macro_rules! asm { - () => {}; -} - -#[lang = "sized"] -trait Sized {} -#[lang = "copy"] -trait Copy {} +extern crate minicore; +use minicore::*; pub unsafe fn we_escape_dollar_signs() { // CHECK: call void asm sideeffect alignstack inteldialect "banana$$:" diff --git a/tests/codegen/asm/sparc-clobbers.rs b/tests/codegen/asm/sparc-clobbers.rs index 843abd553523e..a71715ed94d56 100644 --- a/tests/codegen/asm/sparc-clobbers.rs +++ b/tests/codegen/asm/sparc-clobbers.rs @@ -1,3 +1,4 @@ +//@ add-core-stubs //@ revisions: sparc sparcv8plus sparc64 //@[sparc] compile-flags: --target sparc-unknown-none-elf //@[sparc] needs-llvm-components: sparc @@ -7,16 +8,11 @@ //@[sparc64] needs-llvm-components: sparc #![crate_type = "rlib"] -#![feature(no_core, rustc_attrs, lang_items, asm_experimental_arch)] +#![feature(no_core, asm_experimental_arch)] #![no_core] -#[lang = "sized"] -trait Sized {} - -#[rustc_builtin_macro] -macro_rules! asm { - () => {}; -} +extern crate minicore; +use minicore::*; // CHECK-LABEL: @cc_clobber // CHECK: call void asm sideeffect "", "~{icc},~{fcc0},~{fcc1},~{fcc2},~{fcc3}"() From a5dce0e16d387cf13afce252fa42322d39a4cf97 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= Date: Fri, 20 Dec 2024 16:04:39 +0100 Subject: [PATCH 21/28] Revert "Auto merge of #133809 - mrkajetanp:ci-aarch64-dist, r=Kobzol" This reverts commit 023521e6825edfa6df01e392520d7cb120eab158, reversing changes made to c434b4b4b6cd20560c5b32e80b2b22618a4da3dd. --- .../dist-aarch64-linux/Dockerfile | 99 ------------------- .../host-x86_64/dist-aarch64-linux/Dockerfile | 32 ++++++ .../aarch64-linux-gnu.defconfig | 10 ++ .../host-x86_64/dist-i686-linux/Dockerfile | 8 +- .../dist-powerpc64le-linux/Dockerfile | 2 +- .../dist-powerpc64le-linux/shared.sh | 16 +++ .../host-x86_64/dist-x86_64-linux/Dockerfile | 9 +- .../dist-x86_64-linux}/build-clang.sh | 4 +- .../dist-x86_64-linux}/build-gcc.sh | 10 +- .../dist-x86_64-linux}/build-gccjit.sh | 0 .../dist-x86_64-linux}/build-zstd.sh | 0 .../host-x86_64/dist-x86_64-linux/shared.sh | 16 +++ .../host-x86_64/x86_64-gnu-llvm-18/Dockerfile | 4 +- .../host-x86_64/x86_64-gnu-llvm-19/Dockerfile | 4 +- .../host-x86_64/x86_64-gnu-tools/Dockerfile | 4 +- src/ci/github-actions/jobs.yml | 10 +- 16 files changed, 99 insertions(+), 129 deletions(-) delete mode 100644 src/ci/docker/host-aarch64/dist-aarch64-linux/Dockerfile create mode 100644 src/ci/docker/host-x86_64/dist-aarch64-linux/Dockerfile create mode 100644 src/ci/docker/host-x86_64/dist-aarch64-linux/aarch64-linux-gnu.defconfig create mode 100644 src/ci/docker/host-x86_64/dist-powerpc64le-linux/shared.sh rename src/ci/docker/{scripts => host-x86_64/dist-x86_64-linux}/build-clang.sh (95%) rename src/ci/docker/{scripts => host-x86_64/dist-x86_64-linux}/build-gcc.sh (87%) rename src/ci/docker/{scripts => host-x86_64/dist-x86_64-linux}/build-gccjit.sh (100%) rename src/ci/docker/{scripts => host-x86_64/dist-x86_64-linux}/build-zstd.sh (100%) create mode 100644 src/ci/docker/host-x86_64/dist-x86_64-linux/shared.sh diff --git a/src/ci/docker/host-aarch64/dist-aarch64-linux/Dockerfile b/src/ci/docker/host-aarch64/dist-aarch64-linux/Dockerfile deleted file mode 100644 index 4f4caa5fa50cf..0000000000000 --- a/src/ci/docker/host-aarch64/dist-aarch64-linux/Dockerfile +++ /dev/null @@ -1,99 +0,0 @@ -# We document platform support for minimum glibc 2.17 and kernel 3.2. -# CentOS 7 has headers for kernel 3.10, but that's fine as long as we don't -# actually use newer APIs in rustc or std without a fallback. It's more -# important that we match glibc for ELF symbol versioning. -FROM centos:7 - -WORKDIR /build - -# CentOS 7 EOL is June 30, 2024, but the repos remain in the vault. -RUN sed -i /etc/yum.repos.d/*.repo -e 's!^mirrorlist!#mirrorlist!' \ - -e 's!^#baseurl=http://mirror.centos.org/!baseurl=https://vault.centos.org/!' -RUN sed -i 's/enabled=1/enabled=0/' /etc/yum/pluginconf.d/fastestmirror.conf - -RUN yum upgrade -y && \ - yum install -y \ - automake \ - bzip2 \ - file \ - gcc \ - gcc-c++ \ - git \ - glibc-devel \ - libedit-devel \ - libstdc++-devel \ - make \ - ncurses-devel \ - openssl-devel \ - patch \ - perl \ - perl-core \ - pkgconfig \ - python3 \ - unzip \ - wget \ - xz \ - zlib-devel \ - && yum clean all - -RUN mkdir -p /rustroot/bin - -ENV PATH=/rustroot/bin:$PATH -ENV LD_LIBRARY_PATH=/rustroot/lib64:/rustroot/lib32:/rustroot/lib -ENV PKG_CONFIG_PATH=/rustroot/lib/pkgconfig -WORKDIR /tmp -RUN mkdir /home/user -COPY scripts/shared.sh /tmp/ - -# Need at least GCC 5.1 to compile LLVM -COPY scripts/build-gcc.sh /tmp/ -RUN ./build-gcc.sh && yum remove -y gcc gcc-c++ - -ENV CC=gcc CXX=g++ - -# LLVM 17 needs cmake 3.20 or higher. -COPY scripts/cmake.sh /tmp/ -RUN ./cmake.sh - -# Build LLVM+Clang -COPY scripts/build-clang.sh /tmp/ -ENV LLVM_BUILD_TARGETS=AArch64 -RUN ./build-clang.sh -ENV CC=clang CXX=clang++ - -# Build zstd to enable `llvm.libzstd`. -COPY scripts/build-zstd.sh /tmp/ -RUN ./build-zstd.sh - -COPY scripts/sccache.sh /scripts/ -RUN sh /scripts/sccache.sh - -ENV PGO_HOST=aarch64-unknown-linux-gnu -ENV HOSTS=aarch64-unknown-linux-gnu - -ENV CPATH=/usr/include/aarch64-linux-gnu/:$CPATH - -ENV RUST_CONFIGURE_ARGS \ - --build=aarch64-unknown-linux-gnu \ - --enable-full-tools \ - --enable-profiler \ - --enable-sanitizers \ - --enable-compiler-docs \ - --set target.aarch64-unknown-linux-gnu.linker=clang \ - --set target.aarch64-unknown-linux-gnu.ar=/rustroot/bin/llvm-ar \ - --set target.aarch64-unknown-linux-gnu.ranlib=/rustroot/bin/llvm-ranlib \ - --set llvm.link-shared=true \ - --set llvm.thin-lto=true \ - --set llvm.libzstd=true \ - --set llvm.ninja=false \ - --set rust.debug-assertions=false \ - --set rust.jemalloc \ - --set rust.use-lld=true \ - --set rust.codegen-units=1 - -ENV SCRIPT python3 ../x.py dist --host $HOSTS --target $HOSTS - -ENV CARGO_TARGET_AARCH64_UNKNOWN_LINUX_GNU_LINKER=clang -ENV DIST_SRC 1 -ENV LIBCURL_NO_PKG_CONFIG 1 -ENV DIST_REQUIRE_ALL_TOOLS 1 diff --git a/src/ci/docker/host-x86_64/dist-aarch64-linux/Dockerfile b/src/ci/docker/host-x86_64/dist-aarch64-linux/Dockerfile new file mode 100644 index 0000000000000..18972387e34d7 --- /dev/null +++ b/src/ci/docker/host-x86_64/dist-aarch64-linux/Dockerfile @@ -0,0 +1,32 @@ +FROM ubuntu:22.04 + +COPY scripts/cross-apt-packages.sh /scripts/ +RUN sh /scripts/cross-apt-packages.sh + +COPY scripts/crosstool-ng.sh /scripts/ +RUN sh /scripts/crosstool-ng.sh + +COPY scripts/rustbuild-setup.sh /scripts/ +RUN sh /scripts/rustbuild-setup.sh +WORKDIR /tmp + +COPY scripts/crosstool-ng-build.sh /scripts/ +COPY host-x86_64/dist-aarch64-linux/aarch64-linux-gnu.defconfig /tmp/crosstool.defconfig +RUN /scripts/crosstool-ng-build.sh + +COPY scripts/sccache.sh /scripts/ +RUN sh /scripts/sccache.sh + +ENV PATH=$PATH:/x-tools/aarch64-unknown-linux-gnu/bin + +ENV CC_aarch64_unknown_linux_gnu=aarch64-unknown-linux-gnu-gcc \ + AR_aarch64_unknown_linux_gnu=aarch64-unknown-linux-gnu-ar \ + CXX_aarch64_unknown_linux_gnu=aarch64-unknown-linux-gnu-g++ + +ENV HOSTS=aarch64-unknown-linux-gnu + +ENV RUST_CONFIGURE_ARGS \ + --enable-full-tools \ + --enable-profiler \ + --enable-sanitizers +ENV SCRIPT python3 ../x.py dist --host $HOSTS --target $HOSTS diff --git a/src/ci/docker/host-x86_64/dist-aarch64-linux/aarch64-linux-gnu.defconfig b/src/ci/docker/host-x86_64/dist-aarch64-linux/aarch64-linux-gnu.defconfig new file mode 100644 index 0000000000000..520b1667c8be1 --- /dev/null +++ b/src/ci/docker/host-x86_64/dist-aarch64-linux/aarch64-linux-gnu.defconfig @@ -0,0 +1,10 @@ +CT_CONFIG_VERSION="4" +CT_PREFIX_DIR="/x-tools/${CT_TARGET}" +CT_USE_MIRROR=y +CT_MIRROR_BASE_URL="https://ci-mirrors.rust-lang.org/rustc" +CT_ARCH_ARM=y +CT_ARCH_64=y +CT_KERNEL_LINUX=y +CT_LINUX_V_4_1=y +CT_GLIBC_V_2_17=y +CT_CC_LANG_CXX=y diff --git a/src/ci/docker/host-x86_64/dist-i686-linux/Dockerfile b/src/ci/docker/host-x86_64/dist-i686-linux/Dockerfile index 7cf1c80dfbc65..414bcc52484c9 100644 --- a/src/ci/docker/host-x86_64/dist-i686-linux/Dockerfile +++ b/src/ci/docker/host-x86_64/dist-i686-linux/Dockerfile @@ -46,11 +46,10 @@ ENV LD_LIBRARY_PATH=/rustroot/lib64:/rustroot/lib32:/rustroot/lib ENV PKG_CONFIG_PATH=/rustroot/lib/pkgconfig WORKDIR /tmp RUN mkdir /home/user -COPY scripts/shared.sh /tmp/ +COPY host-x86_64/dist-x86_64-linux/shared.sh /tmp/ # Need at least GCC 5.1 to compile LLVM nowadays -COPY scripts/build-gcc.sh /tmp/ -ENV GCC_BUILD_TARGET=i686 +COPY host-x86_64/dist-x86_64-linux/build-gcc.sh /tmp/ RUN ./build-gcc.sh && yum remove -y gcc gcc-c++ COPY scripts/cmake.sh /tmp/ @@ -58,8 +57,7 @@ RUN ./cmake.sh # Now build LLVM+Clang, afterwards configuring further compilations to use the # clang/clang++ compilers. -COPY scripts/build-clang.sh /tmp/ -ENV LLVM_BUILD_TARGETS=X86 +COPY host-x86_64/dist-x86_64-linux/build-clang.sh /tmp/ RUN ./build-clang.sh ENV CC=clang CXX=clang++ diff --git a/src/ci/docker/host-x86_64/dist-powerpc64le-linux/Dockerfile b/src/ci/docker/host-x86_64/dist-powerpc64le-linux/Dockerfile index 9d3be51d037d1..9ef391892497f 100644 --- a/src/ci/docker/host-x86_64/dist-powerpc64le-linux/Dockerfile +++ b/src/ci/docker/host-x86_64/dist-powerpc64le-linux/Dockerfile @@ -18,7 +18,7 @@ RUN /scripts/crosstool-ng-build.sh WORKDIR /build RUN apt-get install -y --no-install-recommends rpm2cpio cpio -COPY scripts/shared.sh host-x86_64/dist-powerpc64le-linux/build-powerpc64le-toolchain.sh /build/ +COPY host-x86_64/dist-powerpc64le-linux/shared.sh host-x86_64/dist-powerpc64le-linux/build-powerpc64le-toolchain.sh /build/ RUN ./build-powerpc64le-toolchain.sh COPY scripts/sccache.sh /scripts/ diff --git a/src/ci/docker/host-x86_64/dist-powerpc64le-linux/shared.sh b/src/ci/docker/host-x86_64/dist-powerpc64le-linux/shared.sh new file mode 100644 index 0000000000000..dc86dddd464f2 --- /dev/null +++ b/src/ci/docker/host-x86_64/dist-powerpc64le-linux/shared.sh @@ -0,0 +1,16 @@ +#!/bin/sh +hide_output() { + set +x + on_err=" +echo ERROR: An error was encountered with the build. +cat /tmp/build.log +exit 1 +" + trap "$on_err" ERR + bash -c "while true; do sleep 30; echo \$(date) - building ...; done" & + PING_LOOP_PID=$! + "$@" &> /tmp/build.log + trap - ERR + kill $PING_LOOP_PID + set -x +} diff --git a/src/ci/docker/host-x86_64/dist-x86_64-linux/Dockerfile b/src/ci/docker/host-x86_64/dist-x86_64-linux/Dockerfile index c13c340871c9a..e857f38e68a85 100644 --- a/src/ci/docker/host-x86_64/dist-x86_64-linux/Dockerfile +++ b/src/ci/docker/host-x86_64/dist-x86_64-linux/Dockerfile @@ -46,10 +46,10 @@ ENV LD_LIBRARY_PATH=/rustroot/lib64:/rustroot/lib32:/rustroot/lib ENV PKG_CONFIG_PATH=/rustroot/lib/pkgconfig WORKDIR /tmp RUN mkdir /home/user -COPY scripts/shared.sh /tmp/ +COPY host-x86_64/dist-x86_64-linux/shared.sh /tmp/ # Need at least GCC 5.1 to compile LLVM nowadays -COPY scripts/build-gcc.sh /tmp/ +COPY host-x86_64/dist-x86_64-linux/build-gcc.sh /tmp/ RUN ./build-gcc.sh && yum remove -y gcc gcc-c++ # LLVM 17 needs cmake 3.20 or higher. @@ -58,13 +58,12 @@ RUN ./cmake.sh # Now build LLVM+Clang, afterwards configuring further compilations to use the # clang/clang++ compilers. -COPY scripts/build-clang.sh /tmp/ -ENV LLVM_BUILD_TARGETS=X86 +COPY host-x86_64/dist-x86_64-linux/build-clang.sh /tmp/ RUN ./build-clang.sh ENV CC=clang CXX=clang++ # Build zstd to enable `llvm.libzstd`. -COPY scripts/build-zstd.sh /tmp/ +COPY host-x86_64/dist-x86_64-linux/build-zstd.sh /tmp/ RUN ./build-zstd.sh COPY scripts/sccache.sh /scripts/ diff --git a/src/ci/docker/scripts/build-clang.sh b/src/ci/docker/host-x86_64/dist-x86_64-linux/build-clang.sh similarity index 95% rename from src/ci/docker/scripts/build-clang.sh rename to src/ci/docker/host-x86_64/dist-x86_64-linux/build-clang.sh index 47bfcfbecab38..2e08c87f278c0 100755 --- a/src/ci/docker/scripts/build-clang.sh +++ b/src/ci/docker/host-x86_64/dist-x86_64-linux/build-clang.sh @@ -1,6 +1,6 @@ #!/usr/bin/env bash -set -exu +set -ex source shared.sh @@ -34,7 +34,7 @@ hide_output \ -DCOMPILER_RT_BUILD_XRAY=OFF \ -DCOMPILER_RT_BUILD_MEMPROF=OFF \ -DCOMPILER_RT_BUILD_CTX_PROFILE=OFF \ - -DLLVM_TARGETS_TO_BUILD=$LLVM_BUILD_TARGETS \ + -DLLVM_TARGETS_TO_BUILD=X86 \ -DLLVM_INCLUDE_BENCHMARKS=OFF \ -DLLVM_INCLUDE_TESTS=OFF \ -DLLVM_INCLUDE_EXAMPLES=OFF \ diff --git a/src/ci/docker/scripts/build-gcc.sh b/src/ci/docker/host-x86_64/dist-x86_64-linux/build-gcc.sh similarity index 87% rename from src/ci/docker/scripts/build-gcc.sh rename to src/ci/docker/host-x86_64/dist-x86_64-linux/build-gcc.sh index 78a038215e4b9..e939a5d7eac4d 100755 --- a/src/ci/docker/scripts/build-gcc.sh +++ b/src/ci/docker/host-x86_64/dist-x86_64-linux/build-gcc.sh @@ -50,9 +50,7 @@ cd .. rm -rf gcc-build rm -rf gcc-$GCC -if [[ $GCC_BUILD_TARGET == "i686" ]]; then - # FIXME: clang doesn't find 32-bit libraries in /rustroot/lib, - # but it does look all the way under /rustroot/lib/[...]/32, - # so we can link stuff there to help it out. - ln /rustroot/lib/*.{a,so} -rst /rustroot/lib/gcc/x86_64-pc-linux-gnu/$GCC/32/ -fi +# FIXME: clang doesn't find 32-bit libraries in /rustroot/lib, +# but it does look all the way under /rustroot/lib/[...]/32, +# so we can link stuff there to help it out. +ln /rustroot/lib/*.{a,so} -rst /rustroot/lib/gcc/x86_64-pc-linux-gnu/$GCC/32/ diff --git a/src/ci/docker/scripts/build-gccjit.sh b/src/ci/docker/host-x86_64/dist-x86_64-linux/build-gccjit.sh similarity index 100% rename from src/ci/docker/scripts/build-gccjit.sh rename to src/ci/docker/host-x86_64/dist-x86_64-linux/build-gccjit.sh diff --git a/src/ci/docker/scripts/build-zstd.sh b/src/ci/docker/host-x86_64/dist-x86_64-linux/build-zstd.sh similarity index 100% rename from src/ci/docker/scripts/build-zstd.sh rename to src/ci/docker/host-x86_64/dist-x86_64-linux/build-zstd.sh diff --git a/src/ci/docker/host-x86_64/dist-x86_64-linux/shared.sh b/src/ci/docker/host-x86_64/dist-x86_64-linux/shared.sh new file mode 100644 index 0000000000000..dc86dddd464f2 --- /dev/null +++ b/src/ci/docker/host-x86_64/dist-x86_64-linux/shared.sh @@ -0,0 +1,16 @@ +#!/bin/sh +hide_output() { + set +x + on_err=" +echo ERROR: An error was encountered with the build. +cat /tmp/build.log +exit 1 +" + trap "$on_err" ERR + bash -c "while true; do sleep 30; echo \$(date) - building ...; done" & + PING_LOOP_PID=$! + "$@" &> /tmp/build.log + trap - ERR + kill $PING_LOOP_PID + set -x +} diff --git a/src/ci/docker/host-x86_64/x86_64-gnu-llvm-18/Dockerfile b/src/ci/docker/host-x86_64/x86_64-gnu-llvm-18/Dockerfile index 0a58f337d9ddb..e157debfd09a8 100644 --- a/src/ci/docker/host-x86_64/x86_64-gnu-llvm-18/Dockerfile +++ b/src/ci/docker/host-x86_64/x86_64-gnu-llvm-18/Dockerfile @@ -54,8 +54,8 @@ ENV RUST_CONFIGURE_ARGS \ --set rust.randomize-layout=true \ --set rust.thin-lto-import-instr-limit=10 -COPY scripts/shared.sh /scripts/ -COPY scripts/build-gccjit.sh /scripts/ +COPY host-x86_64/dist-x86_64-linux/shared.sh /scripts/ +COPY host-x86_64/dist-x86_64-linux/build-gccjit.sh /scripts/ RUN /scripts/build-gccjit.sh /scripts diff --git a/src/ci/docker/host-x86_64/x86_64-gnu-llvm-19/Dockerfile b/src/ci/docker/host-x86_64/x86_64-gnu-llvm-19/Dockerfile index 092847cdfe04c..e7016e7d3c0d6 100644 --- a/src/ci/docker/host-x86_64/x86_64-gnu-llvm-19/Dockerfile +++ b/src/ci/docker/host-x86_64/x86_64-gnu-llvm-19/Dockerfile @@ -54,8 +54,8 @@ ENV RUST_CONFIGURE_ARGS \ --set rust.randomize-layout=true \ --set rust.thin-lto-import-instr-limit=10 -COPY scripts/shared.sh /scripts/ -COPY scripts/build-gccjit.sh /scripts/ +COPY host-x86_64/dist-x86_64-linux/shared.sh /scripts/ +COPY host-x86_64/dist-x86_64-linux/build-gccjit.sh /scripts/ RUN /scripts/build-gccjit.sh /scripts diff --git a/src/ci/docker/host-x86_64/x86_64-gnu-tools/Dockerfile b/src/ci/docker/host-x86_64/x86_64-gnu-tools/Dockerfile index ab749b3fdd5a6..2a09cd54b139a 100644 --- a/src/ci/docker/host-x86_64/x86_64-gnu-tools/Dockerfile +++ b/src/ci/docker/host-x86_64/x86_64-gnu-tools/Dockerfile @@ -89,8 +89,8 @@ ENV HOST_TARGET x86_64-unknown-linux-gnu # assertions enabled! Therefore, we cannot force download CI rustc. #ENV FORCE_CI_RUSTC 1 -COPY scripts/shared.sh /scripts/ -COPY scripts/build-gccjit.sh /scripts/ +COPY host-x86_64/dist-x86_64-linux/shared.sh /scripts/ +COPY host-x86_64/dist-x86_64-linux/build-gccjit.sh /scripts/ RUN /scripts/build-gccjit.sh /scripts diff --git a/src/ci/github-actions/jobs.yml b/src/ci/github-actions/jobs.yml index 43ef47641ba79..94033d79af922 100644 --- a/src/ci/github-actions/jobs.yml +++ b/src/ci/github-actions/jobs.yml @@ -129,17 +129,17 @@ auto: - image: aarch64-gnu-debug <<: *job-linux-8c-aarch64 - - image: dist-aarch64-linux - env: - CODEGEN_BACKENDS: llvm,cranelift - <<: *job-linux-8c-aarch64 - - image: arm-android <<: *job-linux-4c - image: armhf-gnu <<: *job-linux-4c + - image: dist-aarch64-linux + env: + CODEGEN_BACKENDS: llvm,cranelift + <<: *job-linux-4c + - image: dist-android <<: *job-linux-4c From c02c311d84a919f6be3b797866100214c91c5b68 Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Fri, 20 Dec 2024 15:20:15 +0000 Subject: [PATCH 22/28] Remove some dead code around import library generation This was missed when replacing the usage of LLVM for generating import libraries. --- compiler/rustc_codegen_llvm/src/llvm/ffi.rs | 28 ----------- .../rustc_llvm/llvm-wrapper/RustWrapper.cpp | 50 ------------------- 2 files changed, 78 deletions(-) diff --git a/compiler/rustc_codegen_llvm/src/llvm/ffi.rs b/compiler/rustc_codegen_llvm/src/llvm/ffi.rs index d62632d143168..9a2bfd95562f2 100644 --- a/compiler/rustc_codegen_llvm/src/llvm/ffi.rs +++ b/compiler/rustc_codegen_llvm/src/llvm/ffi.rs @@ -56,25 +56,6 @@ pub enum LLVMRustResult { Failure, } -// Rust version of the C struct with the same name in rustc_llvm/llvm-wrapper/RustWrapper.cpp. -#[repr(C)] -pub struct LLVMRustCOFFShortExport { - pub name: *const c_char, - pub ordinal_present: bool, - /// value of `ordinal` only important when `ordinal_present` is true - pub ordinal: u16, -} - -impl LLVMRustCOFFShortExport { - pub fn new(name: *const c_char, ordinal: Option) -> LLVMRustCOFFShortExport { - LLVMRustCOFFShortExport { - name, - ordinal_present: ordinal.is_some(), - ordinal: ordinal.unwrap_or(0), - } - } -} - /// Translation of LLVM's MachineTypes enum, defined in llvm\include\llvm\BinaryFormat\COFF.h. /// /// We include only architectures supported on Windows. @@ -2347,15 +2328,6 @@ unsafe extern "C" { ) -> &'a mut RustArchiveMember<'a>; pub fn LLVMRustArchiveMemberFree<'a>(Member: &'a mut RustArchiveMember<'a>); - pub fn LLVMRustWriteImportLibrary( - ImportName: *const c_char, - Path: *const c_char, - Exports: *const LLVMRustCOFFShortExport, - NumExports: usize, - Machine: u16, - MinGW: bool, - ) -> LLVMRustResult; - pub fn LLVMRustSetDataLayoutFromTargetMachine<'a>(M: &'a Module, TM: &'a TargetMachine); pub fn LLVMRustPositionBuilderAtStart<'a>(B: &Builder<'a>, BB: &'a BasicBlock); diff --git a/compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp b/compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp index b79205ff946d3..36441a95adbf9 100644 --- a/compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp +++ b/compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp @@ -1796,56 +1796,6 @@ extern "C" LLVMValueRef LLVMRustBuildMaxNum(LLVMBuilderRef B, LLVMValueRef LHS, return wrap(unwrap(B)->CreateMaxNum(unwrap(LHS), unwrap(RHS))); } -// This struct contains all necessary info about a symbol exported from a DLL. -struct LLVMRustCOFFShortExport { - const char *name; - bool ordinal_present; - // The value of `ordinal` is only meaningful if `ordinal_present` is true. - uint16_t ordinal; -}; - -// Machine must be a COFF machine type, as defined in PE specs. -extern "C" LLVMRustResult -LLVMRustWriteImportLibrary(const char *ImportName, const char *Path, - const LLVMRustCOFFShortExport *Exports, - size_t NumExports, uint16_t Machine, bool MinGW) { - std::vector ConvertedExports; - ConvertedExports.reserve(NumExports); - - for (size_t i = 0; i < NumExports; ++i) { - bool ordinal_present = Exports[i].ordinal_present; - uint16_t ordinal = ordinal_present ? Exports[i].ordinal : 0; - ConvertedExports.push_back(llvm::object::COFFShortExport{ - Exports[i].name, // Name - std::string{}, // ExtName - std::string{}, // SymbolName - std::string{}, // AliasTarget -#if LLVM_VERSION_GE(19, 0) - std::string{}, // ExportAs -#endif - ordinal, // Ordinal - ordinal_present, // Noname - false, // Data - false, // Private - false // Constant - }); - } - - auto Error = llvm::object::writeImportLibrary( - ImportName, Path, ConvertedExports, - static_cast(Machine), MinGW); - if (Error) { - std::string errorString; - auto stream = llvm::raw_string_ostream(errorString); - stream << Error; - stream.flush(); - LLVMRustSetLastError(errorString.c_str()); - return LLVMRustResult::Failure; - } else { - return LLVMRustResult::Success; - } -} - // Transfers ownership of DiagnosticHandler unique_ptr to the caller. extern "C" DiagnosticHandler * LLVMRustContextGetDiagnosticHandler(LLVMContextRef C) { From 42c00cb6479d6e03742968e06c6d280a272acd53 Mon Sep 17 00:00:00 2001 From: Lukas Markeffsky <@> Date: Fri, 20 Dec 2024 16:57:14 +0100 Subject: [PATCH 23/28] split up `#[rustc_deny_explicit_impl]` attribute This commit splits the `#[rustc_deny_explicit_impl(implement_via_object = ...)]` attribute into two attributes `#[rustc_deny_explicit_impl]` and `#[rustc_do_not_implement_via_object]`. This allows us to have special traits that can have user-defined impls but do not have the automatic trait impl for trait objects (`impl Trait for dyn Trait`). --- compiler/rustc_feature/src/builtin_attrs.rs | 11 +++- .../rustc_hir_analysis/src/coherence/mod.rs | 4 +- compiler/rustc_hir_analysis/src/collect.rs | 45 +---------------- compiler/rustc_middle/src/ty/trait_def.rs | 4 +- compiler/rustc_passes/src/check_attr.rs | 1 + compiler/rustc_span/src/symbol.rs | 1 + library/core/src/future/async_drop.rs | 4 +- library/core/src/marker.rs | 24 ++++++--- library/core/src/mem/transmutability.rs | 4 +- library/core/src/ptr/metadata.rs | 4 +- .../deny-builtin-object-impl.current.stderr | 50 ++++++++++++++----- .../deny-builtin-object-impl.next.stderr | 50 ++++++++++++++----- tests/ui/traits/deny-builtin-object-impl.rs | 41 +++++++++++---- 13 files changed, 152 insertions(+), 91 deletions(-) diff --git a/compiler/rustc_feature/src/builtin_attrs.rs b/compiler/rustc_feature/src/builtin_attrs.rs index 4b9f62fa764c0..63e5ebb8688f6 100644 --- a/compiler/rustc_feature/src/builtin_attrs.rs +++ b/compiler/rustc_feature/src/builtin_attrs.rs @@ -912,11 +912,20 @@ pub const BUILTIN_ATTRIBUTES: &[BuiltinAttribute] = &[ rustc_attr!( rustc_deny_explicit_impl, AttributeType::Normal, - template!(List: "implement_via_object = (true|false)"), + template!(Word), ErrorFollowing, EncodeCrossCrate::No, "#[rustc_deny_explicit_impl] enforces that a trait can have no user-provided impls" ), + rustc_attr!( + rustc_do_not_implement_via_object, + AttributeType::Normal, + template!(Word), + ErrorFollowing, + EncodeCrossCrate::No, + "#[rustc_do_not_implement_via_object] opts out of the automatic trait impl for trait objects \ + (`impl Trait for dyn Trait`)" + ), rustc_attr!( rustc_has_incoherent_inherent_impls, AttributeType::Normal, template!(Word), ErrorFollowing, EncodeCrossCrate::Yes, diff --git a/compiler/rustc_hir_analysis/src/coherence/mod.rs b/compiler/rustc_hir_analysis/src/coherence/mod.rs index 3aad4bafeb5ab..1be4aa2f63ac4 100644 --- a/compiler/rustc_hir_analysis/src/coherence/mod.rs +++ b/compiler/rustc_hir_analysis/src/coherence/mod.rs @@ -206,7 +206,9 @@ fn check_object_overlap<'tcx>( // so this is valid. } else { let mut supertrait_def_ids = tcx.supertrait_def_ids(component_def_id); - if supertrait_def_ids.any(|d| d == trait_def_id) { + if supertrait_def_ids + .any(|d| d == trait_def_id && tcx.trait_def(d).implement_via_object) + { let span = tcx.def_span(impl_def_id); return Err(struct_span_code_err!( tcx.dcx(), diff --git a/compiler/rustc_hir_analysis/src/collect.rs b/compiler/rustc_hir_analysis/src/collect.rs index 5e662bb96bc44..ada70117b626f 100644 --- a/compiler/rustc_hir_analysis/src/collect.rs +++ b/compiler/rustc_hir_analysis/src/collect.rs @@ -1261,49 +1261,8 @@ fn trait_def(tcx: TyCtxt<'_>, def_id: LocalDefId) -> ty::TraitDef { no_dups.then_some(list) }); - let mut deny_explicit_impl = false; - let mut implement_via_object = true; - if let Some(attr) = tcx.get_attr(def_id, sym::rustc_deny_explicit_impl) { - deny_explicit_impl = true; - let mut seen_attr = false; - for meta in attr.meta_item_list().iter().flatten() { - if let Some(meta) = meta.meta_item() - && meta.name_or_empty() == sym::implement_via_object - && let Some(lit) = meta.name_value_literal() - { - if seen_attr { - tcx.dcx().span_err(meta.span, "duplicated `implement_via_object` meta item"); - } - seen_attr = true; - - match lit.symbol { - kw::True => { - implement_via_object = true; - } - kw::False => { - implement_via_object = false; - } - _ => { - tcx.dcx().span_err( - meta.span, - format!( - "unknown literal passed to `implement_via_object` attribute: {}", - lit.symbol - ), - ); - } - } - } else { - tcx.dcx().span_err( - meta.span(), - format!("unknown meta item passed to `rustc_deny_explicit_impl` {meta:?}"), - ); - } - } - if !seen_attr { - tcx.dcx().span_err(attr.span, "missing `implement_via_object` meta item"); - } - } + let deny_explicit_impl = tcx.has_attr(def_id, sym::rustc_deny_explicit_impl); + let implement_via_object = !tcx.has_attr(def_id, sym::rustc_do_not_implement_via_object); ty::TraitDef { def_id: def_id.to_def_id(), diff --git a/compiler/rustc_middle/src/ty/trait_def.rs b/compiler/rustc_middle/src/ty/trait_def.rs index 188107e5d5498..743ea33b20a27 100644 --- a/compiler/rustc_middle/src/ty/trait_def.rs +++ b/compiler/rustc_middle/src/ty/trait_def.rs @@ -70,12 +70,12 @@ pub struct TraitDef { /// Whether to add a builtin `dyn Trait: Trait` implementation. /// This is enabled for all traits except ones marked with - /// `#[rustc_deny_explicit_impl(implement_via_object = false)]`. + /// `#[rustc_do_not_implement_via_object]`. pub implement_via_object: bool, /// Whether a trait is fully built-in, and any implementation is disallowed. /// This only applies to built-in traits, and is marked via - /// `#[rustc_deny_explicit_impl(implement_via_object = ...)]`. + /// `#[rustc_deny_explicit_impl]`. pub deny_explicit_impl: bool, } diff --git a/compiler/rustc_passes/src/check_attr.rs b/compiler/rustc_passes/src/check_attr.rs index 2310dd9dc72db..1f2d5671f7a84 100644 --- a/compiler/rustc_passes/src/check_attr.rs +++ b/compiler/rustc_passes/src/check_attr.rs @@ -186,6 +186,7 @@ impl<'tcx> CheckAttrVisitor<'tcx> { [sym::rustc_coinductive, ..] | [sym::rustc_must_implement_one_of, ..] | [sym::rustc_deny_explicit_impl, ..] + | [sym::rustc_do_not_implement_via_object, ..] | [sym::const_trait, ..] => self.check_must_be_applied_to_trait(attr, span, target), [sym::collapse_debuginfo, ..] => self.check_collapse_debuginfo(attr, span, target), [sym::must_not_suspend, ..] => self.check_must_not_suspend(attr, span, target), diff --git a/compiler/rustc_span/src/symbol.rs b/compiler/rustc_span/src/symbol.rs index a7ff0576f92d6..123e4b1f01f26 100644 --- a/compiler/rustc_span/src/symbol.rs +++ b/compiler/rustc_span/src/symbol.rs @@ -1717,6 +1717,7 @@ symbols! { rustc_diagnostic_macros, rustc_dirty, rustc_do_not_const_check, + rustc_do_not_implement_via_object, rustc_doc_primitive, rustc_driver, rustc_dummy, diff --git a/library/core/src/future/async_drop.rs b/library/core/src/future/async_drop.rs index 7de5fe67cd096..ea6d3b800e64d 100644 --- a/library/core/src/future/async_drop.rs +++ b/library/core/src/future/async_drop.rs @@ -133,7 +133,9 @@ pub trait AsyncDrop { } #[lang = "async_destruct"] -#[rustc_deny_explicit_impl(implement_via_object = false)] +#[cfg_attr(bootstrap, rustc_deny_explicit_impl(implement_via_object = false))] +#[cfg_attr(not(bootstrap), rustc_deny_explicit_impl)] +#[cfg_attr(not(bootstrap), rustc_do_not_implement_via_object)] trait AsyncDestruct { type AsyncDestructor: Future; } diff --git a/library/core/src/marker.rs b/library/core/src/marker.rs index 1620b949590d0..31b27b7ef2641 100644 --- a/library/core/src/marker.rs +++ b/library/core/src/marker.rs @@ -141,7 +141,9 @@ unsafe impl Send for &T {} )] #[fundamental] // for Default, for example, which requires that `[T]: !Default` be evaluatable #[rustc_specialization_trait] -#[rustc_deny_explicit_impl(implement_via_object = false)] +#[cfg_attr(bootstrap, rustc_deny_explicit_impl(implement_via_object = false))] +#[cfg_attr(not(bootstrap), rustc_deny_explicit_impl)] +#[cfg_attr(not(bootstrap), rustc_do_not_implement_via_object)] #[rustc_coinductive] pub trait Sized { // Empty. @@ -181,7 +183,9 @@ pub trait Sized { /// [^1]: Formerly known as *object safe*. #[unstable(feature = "unsize", issue = "18598")] #[lang = "unsize"] -#[rustc_deny_explicit_impl(implement_via_object = false)] +#[cfg_attr(bootstrap, rustc_deny_explicit_impl(implement_via_object = false))] +#[cfg_attr(not(bootstrap), rustc_deny_explicit_impl)] +#[cfg_attr(not(bootstrap), rustc_do_not_implement_via_object)] pub trait Unsize { // Empty. } @@ -815,7 +819,9 @@ impl StructuralPartialEq for PhantomData {} reason = "this trait is unlikely to ever be stabilized, use `mem::discriminant` instead" )] #[lang = "discriminant_kind"] -#[rustc_deny_explicit_impl(implement_via_object = false)] +#[cfg_attr(bootstrap, rustc_deny_explicit_impl(implement_via_object = false))] +#[cfg_attr(not(bootstrap), rustc_deny_explicit_impl)] +#[cfg_attr(not(bootstrap), rustc_do_not_implement_via_object)] pub trait DiscriminantKind { /// The type of the discriminant, which must satisfy the trait /// bounds required by `mem::Discriminant`. @@ -956,7 +962,9 @@ marker_impls! { #[unstable(feature = "const_destruct", issue = "133214")] #[lang = "destruct"] #[rustc_on_unimplemented(message = "can't drop `{Self}`", append_const_msg)] -#[rustc_deny_explicit_impl(implement_via_object = false)] +#[cfg_attr(bootstrap, rustc_deny_explicit_impl(implement_via_object = false))] +#[cfg_attr(not(bootstrap), rustc_deny_explicit_impl)] +#[cfg_attr(not(bootstrap), rustc_do_not_implement_via_object)] #[cfg_attr(not(bootstrap), const_trait)] pub trait Destruct {} @@ -967,7 +975,9 @@ pub trait Destruct {} #[unstable(feature = "tuple_trait", issue = "none")] #[lang = "tuple_trait"] #[diagnostic::on_unimplemented(message = "`{Self}` is not a tuple")] -#[rustc_deny_explicit_impl(implement_via_object = false)] +#[cfg_attr(bootstrap, rustc_deny_explicit_impl(implement_via_object = false))] +#[cfg_attr(not(bootstrap), rustc_deny_explicit_impl)] +#[cfg_attr(not(bootstrap), rustc_do_not_implement_via_object)] pub trait Tuple {} /// A marker for pointer-like types. @@ -1068,7 +1078,9 @@ marker_impls! { reason = "internal trait for implementing various traits for all function pointers" )] #[lang = "fn_ptr_trait"] -#[rustc_deny_explicit_impl(implement_via_object = false)] +#[cfg_attr(bootstrap, rustc_deny_explicit_impl(implement_via_object = false))] +#[cfg_attr(not(bootstrap), rustc_deny_explicit_impl)] +#[cfg_attr(not(bootstrap), rustc_do_not_implement_via_object)] pub trait FnPtr: Copy + Clone { /// Returns the address of the function pointer. #[lang = "fn_ptr_addr"] diff --git a/library/core/src/mem/transmutability.rs b/library/core/src/mem/transmutability.rs index 7fa3c33439170..9cf587650d949 100644 --- a/library/core/src/mem/transmutability.rs +++ b/library/core/src/mem/transmutability.rs @@ -84,7 +84,9 @@ use crate::marker::{ConstParamTy_, UnsizedConstParamTy}; /// `usize` is stable, but not portable. #[unstable(feature = "transmutability", issue = "99571")] #[lang = "transmute_trait"] -#[rustc_deny_explicit_impl(implement_via_object = false)] +#[cfg_attr(bootstrap, rustc_deny_explicit_impl(implement_via_object = false))] +#[cfg_attr(not(bootstrap), rustc_deny_explicit_impl)] +#[cfg_attr(not(bootstrap), rustc_do_not_implement_via_object)] #[rustc_coinductive] pub unsafe trait TransmuteFrom where diff --git a/library/core/src/ptr/metadata.rs b/library/core/src/ptr/metadata.rs index ae9810e558aa1..b1d5e1fa3a059 100644 --- a/library/core/src/ptr/metadata.rs +++ b/library/core/src/ptr/metadata.rs @@ -53,7 +53,9 @@ use crate::ptr::NonNull; /// /// [`to_raw_parts`]: *const::to_raw_parts #[lang = "pointee_trait"] -#[rustc_deny_explicit_impl(implement_via_object = false)] +#[cfg_attr(bootstrap, rustc_deny_explicit_impl(implement_via_object = false))] +#[cfg_attr(not(bootstrap), rustc_deny_explicit_impl)] +#[cfg_attr(not(bootstrap), rustc_do_not_implement_via_object)] pub trait Pointee { /// The type for metadata in pointers and references to `Self`. #[lang = "metadata_type"] diff --git a/tests/ui/traits/deny-builtin-object-impl.current.stderr b/tests/ui/traits/deny-builtin-object-impl.current.stderr index 8423a2519b23b..d6f4762d09966 100644 --- a/tests/ui/traits/deny-builtin-object-impl.current.stderr +++ b/tests/ui/traits/deny-builtin-object-impl.current.stderr @@ -1,20 +1,44 @@ -error[E0277]: the trait bound `dyn NotObject: NotObject` is not satisfied - --> $DIR/deny-builtin-object-impl.rs:19:23 +error[E0322]: explicit impls for the `NotImplYesObject` trait are not permitted + --> $DIR/deny-builtin-object-impl.rs:20:1 | -LL | test_not_object::(); - | ^^^^^^^^^^^^^ the trait `NotObject` is not implemented for `dyn NotObject` +LL | impl NotImplYesObject for () {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ impl of `NotImplYesObject` not allowed + +error[E0277]: the trait bound `dyn NotImplNotObject: NotImplNotObject` is not satisfied + --> $DIR/deny-builtin-object-impl.rs:37:32 + | +LL | test_not_impl_not_object::(); + | ^^^^^^^^^^^^^^^^^^^^ the trait `NotImplNotObject` is not implemented for `dyn NotImplNotObject` + | +help: this trait has no implementations, consider adding one + --> $DIR/deny-builtin-object-impl.rs:12:1 + | +LL | trait NotImplNotObject {} + | ^^^^^^^^^^^^^^^^^^^^^^ +note: required by a bound in `test_not_impl_not_object` + --> $DIR/deny-builtin-object-impl.rs:28:32 + | +LL | fn test_not_impl_not_object() {} + | ^^^^^^^^^^^^^^^^ required by this bound in `test_not_impl_not_object` + +error[E0277]: the trait bound `dyn YesImplNotObject: YesImplNotObject` is not satisfied + --> $DIR/deny-builtin-object-impl.rs:40:32 + | +LL | test_yes_impl_not_object::(); + | ^^^^^^^^^^^^^^^^^^^^ the trait `YesImplNotObject` is not implemented for `dyn YesImplNotObject` | help: this trait has no implementations, consider adding one - --> $DIR/deny-builtin-object-impl.rs:11:1 + --> $DIR/deny-builtin-object-impl.rs:15:1 | -LL | trait NotObject {} - | ^^^^^^^^^^^^^^^ -note: required by a bound in `test_not_object` - --> $DIR/deny-builtin-object-impl.rs:15:23 +LL | trait YesImplNotObject {} + | ^^^^^^^^^^^^^^^^^^^^^^ +note: required by a bound in `test_yes_impl_not_object` + --> $DIR/deny-builtin-object-impl.rs:30:32 | -LL | fn test_not_object() {} - | ^^^^^^^^^ required by this bound in `test_not_object` +LL | fn test_yes_impl_not_object() {} + | ^^^^^^^^^^^^^^^^ required by this bound in `test_yes_impl_not_object` -error: aborting due to 1 previous error +error: aborting due to 3 previous errors -For more information about this error, try `rustc --explain E0277`. +Some errors have detailed explanations: E0277, E0322. +For more information about an error, try `rustc --explain E0277`. diff --git a/tests/ui/traits/deny-builtin-object-impl.next.stderr b/tests/ui/traits/deny-builtin-object-impl.next.stderr index 8423a2519b23b..d6f4762d09966 100644 --- a/tests/ui/traits/deny-builtin-object-impl.next.stderr +++ b/tests/ui/traits/deny-builtin-object-impl.next.stderr @@ -1,20 +1,44 @@ -error[E0277]: the trait bound `dyn NotObject: NotObject` is not satisfied - --> $DIR/deny-builtin-object-impl.rs:19:23 +error[E0322]: explicit impls for the `NotImplYesObject` trait are not permitted + --> $DIR/deny-builtin-object-impl.rs:20:1 | -LL | test_not_object::(); - | ^^^^^^^^^^^^^ the trait `NotObject` is not implemented for `dyn NotObject` +LL | impl NotImplYesObject for () {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ impl of `NotImplYesObject` not allowed + +error[E0277]: the trait bound `dyn NotImplNotObject: NotImplNotObject` is not satisfied + --> $DIR/deny-builtin-object-impl.rs:37:32 + | +LL | test_not_impl_not_object::(); + | ^^^^^^^^^^^^^^^^^^^^ the trait `NotImplNotObject` is not implemented for `dyn NotImplNotObject` + | +help: this trait has no implementations, consider adding one + --> $DIR/deny-builtin-object-impl.rs:12:1 + | +LL | trait NotImplNotObject {} + | ^^^^^^^^^^^^^^^^^^^^^^ +note: required by a bound in `test_not_impl_not_object` + --> $DIR/deny-builtin-object-impl.rs:28:32 + | +LL | fn test_not_impl_not_object() {} + | ^^^^^^^^^^^^^^^^ required by this bound in `test_not_impl_not_object` + +error[E0277]: the trait bound `dyn YesImplNotObject: YesImplNotObject` is not satisfied + --> $DIR/deny-builtin-object-impl.rs:40:32 + | +LL | test_yes_impl_not_object::(); + | ^^^^^^^^^^^^^^^^^^^^ the trait `YesImplNotObject` is not implemented for `dyn YesImplNotObject` | help: this trait has no implementations, consider adding one - --> $DIR/deny-builtin-object-impl.rs:11:1 + --> $DIR/deny-builtin-object-impl.rs:15:1 | -LL | trait NotObject {} - | ^^^^^^^^^^^^^^^ -note: required by a bound in `test_not_object` - --> $DIR/deny-builtin-object-impl.rs:15:23 +LL | trait YesImplNotObject {} + | ^^^^^^^^^^^^^^^^^^^^^^ +note: required by a bound in `test_yes_impl_not_object` + --> $DIR/deny-builtin-object-impl.rs:30:32 | -LL | fn test_not_object() {} - | ^^^^^^^^^ required by this bound in `test_not_object` +LL | fn test_yes_impl_not_object() {} + | ^^^^^^^^^^^^^^^^ required by this bound in `test_yes_impl_not_object` -error: aborting due to 1 previous error +error: aborting due to 3 previous errors -For more information about this error, try `rustc --explain E0277`. +Some errors have detailed explanations: E0277, E0322. +For more information about an error, try `rustc --explain E0277`. diff --git a/tests/ui/traits/deny-builtin-object-impl.rs b/tests/ui/traits/deny-builtin-object-impl.rs index c16dcca8fbc27..9d02ab7bd469e 100644 --- a/tests/ui/traits/deny-builtin-object-impl.rs +++ b/tests/ui/traits/deny-builtin-object-impl.rs @@ -4,18 +4,41 @@ #![feature(rustc_attrs)] -#[rustc_deny_explicit_impl(implement_via_object = true)] -trait YesObject {} +#[rustc_deny_explicit_impl] +trait NotImplYesObject {} -#[rustc_deny_explicit_impl(implement_via_object = false)] -trait NotObject {} +#[rustc_deny_explicit_impl] +#[rustc_do_not_implement_via_object] +trait NotImplNotObject {} -fn test_yes_object() {} +#[rustc_do_not_implement_via_object] +trait YesImplNotObject {} -fn test_not_object() {} +#[rustc_do_not_implement_via_object] +trait YesImplNotObject2 {} + +impl NotImplYesObject for () {} +//~^ ERROR explicit impls for the `NotImplYesObject` trait are not permitted + +// If there is no automatic impl then we can add a manual impl: +impl YesImplNotObject2 for dyn YesImplNotObject2 {} + +fn test_not_impl_yes_object() {} + +fn test_not_impl_not_object() {} + +fn test_yes_impl_not_object() {} + +fn test_yes_impl_not_object2() {} fn main() { - test_yes_object::(); - test_not_object::(); - //~^ ERROR the trait bound `dyn NotObject: NotObject` is not satisfied + test_not_impl_yes_object::(); + + test_not_impl_not_object::(); + //~^ ERROR the trait bound `dyn NotImplNotObject: NotImplNotObject` is not satisfied + + test_yes_impl_not_object::(); + //~^ ERROR the trait bound `dyn YesImplNotObject: YesImplNotObject` is not satisfied + + test_yes_impl_not_object2::(); } From 971a4f2d3b0c44fae115b39e4fa9607a911ed59c Mon Sep 17 00:00:00 2001 From: Lukas Markeffsky <@> Date: Fri, 20 Dec 2024 17:35:29 +0100 Subject: [PATCH 24/28] unimplement `PointerLike` for trait objects --- library/core/src/marker.rs | 1 + tests/ui/dyn-star/dyn-pointer-like.rs | 23 +++++++++++++ tests/ui/dyn-star/dyn-pointer-like.stderr | 39 +++++++++++++++++++++++ 3 files changed, 63 insertions(+) create mode 100644 tests/ui/dyn-star/dyn-pointer-like.rs create mode 100644 tests/ui/dyn-star/dyn-pointer-like.stderr diff --git a/library/core/src/marker.rs b/library/core/src/marker.rs index 31b27b7ef2641..bdfa162919f21 100644 --- a/library/core/src/marker.rs +++ b/library/core/src/marker.rs @@ -990,6 +990,7 @@ pub trait Tuple {} message = "`{Self}` needs to have the same ABI as a pointer", label = "`{Self}` needs to be a pointer-like type" )] +#[cfg_attr(not(bootstrap), rustc_do_not_implement_via_object)] pub trait PointerLike {} #[cfg(not(bootstrap))] diff --git a/tests/ui/dyn-star/dyn-pointer-like.rs b/tests/ui/dyn-star/dyn-pointer-like.rs new file mode 100644 index 0000000000000..f26fa90505cf7 --- /dev/null +++ b/tests/ui/dyn-star/dyn-pointer-like.rs @@ -0,0 +1,23 @@ +// Test that `dyn PointerLike` and `dyn* PointerLike` do not implement `PointerLike`. +// This used to ICE during codegen. + +#![crate_type = "lib"] + +#![feature(pointer_like_trait, dyn_star)] +#![feature(unsized_fn_params)] +#![expect(incomplete_features)] +#![expect(internal_features)] + +use std::marker::PointerLike; + +pub fn lol(x: dyn* PointerLike) { + foo(x); //~ ERROR `dyn* PointerLike` needs to have the same ABI as a pointer +} + +pub fn uwu(x: dyn PointerLike) { + foo(x); //~ ERROR `dyn PointerLike` needs to have the same ABI as a pointer +} + +fn foo(x: T) { + let _: dyn* PointerLike = x; +} diff --git a/tests/ui/dyn-star/dyn-pointer-like.stderr b/tests/ui/dyn-star/dyn-pointer-like.stderr new file mode 100644 index 0000000000000..4c558e92d3f91 --- /dev/null +++ b/tests/ui/dyn-star/dyn-pointer-like.stderr @@ -0,0 +1,39 @@ +error[E0277]: `dyn* PointerLike` needs to have the same ABI as a pointer + --> $DIR/dyn-pointer-like.rs:14:9 + | +LL | foo(x); + | --- ^ the trait `PointerLike` is not implemented for `dyn* PointerLike` + | | + | required by a bound introduced by this call + | + = note: the trait bound `dyn* PointerLike: PointerLike` is not satisfied +note: required by a bound in `foo` + --> $DIR/dyn-pointer-like.rs:21:11 + | +LL | fn foo(x: T) { + | ^^^^^^^^^^^ required by this bound in `foo` +help: consider borrowing here + | +LL | foo(&x); + | + +LL | foo(&mut x); + | ++++ + +error[E0277]: `dyn PointerLike` needs to have the same ABI as a pointer + --> $DIR/dyn-pointer-like.rs:18:9 + | +LL | foo(x); + | --- ^ `dyn PointerLike` needs to be a pointer-like type + | | + | required by a bound introduced by this call + | + = help: the trait `PointerLike` is not implemented for `dyn PointerLike` +note: required by a bound in `foo` + --> $DIR/dyn-pointer-like.rs:21:11 + | +LL | fn foo(x: T) { + | ^^^^^^^^^^^ required by this bound in `foo` + +error: aborting due to 2 previous errors + +For more information about this error, try `rustc --explain E0277`. From 159dba89efa23c36eca92f1b63176314c357d2a4 Mon Sep 17 00:00:00 2001 From: Lukas Markeffsky <@> Date: Fri, 20 Dec 2024 17:37:34 +0100 Subject: [PATCH 25/28] fix `PointerLike` docs --- library/core/src/marker.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/library/core/src/marker.rs b/library/core/src/marker.rs index bdfa162919f21..3d79706f8ecf9 100644 --- a/library/core/src/marker.rs +++ b/library/core/src/marker.rs @@ -982,8 +982,8 @@ pub trait Tuple {} /// A marker for pointer-like types. /// -/// All types that have the same size and alignment as a `usize` or -/// `*const ()` automatically implement this trait. +/// This trait can only be implemented for types that have the same size and alignment +/// as a `usize` or `*const ()`. #[unstable(feature = "pointer_like_trait", issue = "none")] #[lang = "pointer_like"] #[diagnostic::on_unimplemented( From b3cc9b9620fc99399e7991782188dea772e335da Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Fri, 20 Dec 2024 01:07:48 +0000 Subject: [PATCH 26/28] Restrict `#[non_exaustive]` on structs with default field values Do not allow users to apply `#[non_exaustive]` to a struct when they have also used default field values. --- compiler/rustc_passes/messages.ftl | 4 +++ compiler/rustc_passes/src/check_attr.rs | 27 ++++++++++++++++--- compiler/rustc_passes/src/errors.rs | 9 +++++++ .../default-field-values-non_exhaustive.rs | 18 +++++++++++++ ...default-field-values-non_exhaustive.stderr | 23 ++++++++++++++++ 5 files changed, 78 insertions(+), 3 deletions(-) create mode 100644 tests/ui/structs/default-field-values-non_exhaustive.rs create mode 100644 tests/ui/structs/default-field-values-non_exhaustive.stderr diff --git a/compiler/rustc_passes/messages.ftl b/compiler/rustc_passes/messages.ftl index 9cc94b7562468..ba3101e9058aa 100644 --- a/compiler/rustc_passes/messages.ftl +++ b/compiler/rustc_passes/messages.ftl @@ -566,6 +566,10 @@ passes_no_sanitize = `#[no_sanitize({$attr_str})]` should be applied to {$accepted_kind} .label = not {$accepted_kind} +passes_non_exaustive_with_default_field_values = + `#[non_exhaustive]` can't be used to annotate items with default field values + .label = this struct has default field values + passes_non_exported_macro_invalid_attrs = attribute should be applied to function or closure .label = not a function or closure diff --git a/compiler/rustc_passes/src/check_attr.rs b/compiler/rustc_passes/src/check_attr.rs index 2310dd9dc72db..699a5a6cd62d1 100644 --- a/compiler/rustc_passes/src/check_attr.rs +++ b/compiler/rustc_passes/src/check_attr.rs @@ -124,7 +124,7 @@ impl<'tcx> CheckAttrVisitor<'tcx> { [sym::coverage, ..] => self.check_coverage(attr, span, target), [sym::optimize, ..] => self.check_optimize(hir_id, attr, span, target), [sym::no_sanitize, ..] => self.check_no_sanitize(attr, span, target), - [sym::non_exhaustive, ..] => self.check_non_exhaustive(hir_id, attr, span, target), + [sym::non_exhaustive, ..] => self.check_non_exhaustive(hir_id, attr, span, target, item), [sym::marker, ..] => self.check_marker(hir_id, attr, span, target), [sym::target_feature, ..] => { self.check_target_feature(hir_id, attr, span, target, attrs) @@ -684,9 +684,30 @@ impl<'tcx> CheckAttrVisitor<'tcx> { } /// Checks if the `#[non_exhaustive]` attribute on an `item` is valid. - fn check_non_exhaustive(&self, hir_id: HirId, attr: &Attribute, span: Span, target: Target) { + fn check_non_exhaustive( + &self, + hir_id: HirId, + attr: &Attribute, + span: Span, + target: Target, + item: Option>, + ) { match target { - Target::Struct | Target::Enum | Target::Variant => {} + Target::Struct => { + if let Some(ItemLike::Item(hir::Item { + kind: hir::ItemKind::Struct(hir::VariantData::Struct { fields, .. }, _), + .. + })) = item + && !fields.is_empty() + && fields.iter().any(|f| f.default.is_some()) + { + self.dcx().emit_err(errors::NonExhaustiveWithDefaultFieldValues { + attr_span: attr.span, + defn_span: span, + }); + } + } + Target::Enum | Target::Variant => {} // FIXME(#80564): We permit struct fields, match arms and macro defs to have an // `#[non_exhaustive]` attribute with just a lint, because we previously // erroneously allowed it and some crates used it accidentally, to be compatible diff --git a/compiler/rustc_passes/src/errors.rs b/compiler/rustc_passes/src/errors.rs index 5e7bfa5e3bb71..163325f2a3c6f 100644 --- a/compiler/rustc_passes/src/errors.rs +++ b/compiler/rustc_passes/src/errors.rs @@ -119,6 +119,15 @@ pub(crate) struct NonExhaustiveWrongLocation { pub defn_span: Span, } +#[derive(Diagnostic)] +#[diag(passes_non_exaustive_with_default_field_values)] +pub(crate) struct NonExhaustiveWithDefaultFieldValues { + #[primary_span] + pub attr_span: Span, + #[label] + pub defn_span: Span, +} + #[derive(Diagnostic)] #[diag(passes_should_be_applied_to_trait)] pub(crate) struct AttrShouldBeAppliedToTrait { diff --git a/tests/ui/structs/default-field-values-non_exhaustive.rs b/tests/ui/structs/default-field-values-non_exhaustive.rs new file mode 100644 index 0000000000000..0ad2b0766a772 --- /dev/null +++ b/tests/ui/structs/default-field-values-non_exhaustive.rs @@ -0,0 +1,18 @@ +#![feature(default_field_values)] + +#[derive(Default)] +#[non_exhaustive] //~ ERROR `#[non_exhaustive]` can't be used to annotate items with default field values +struct Foo { + x: i32 = 42 + 3, +} + +#[derive(Default)] +enum Bar { + #[non_exhaustive] + #[default] + Baz { //~ ERROR default variant must be exhaustive + x: i32 = 42 + 3, + } +} + +fn main () {} diff --git a/tests/ui/structs/default-field-values-non_exhaustive.stderr b/tests/ui/structs/default-field-values-non_exhaustive.stderr new file mode 100644 index 0000000000000..13013bebe83d9 --- /dev/null +++ b/tests/ui/structs/default-field-values-non_exhaustive.stderr @@ -0,0 +1,23 @@ +error: default variant must be exhaustive + --> $DIR/default-field-values-non_exhaustive.rs:13:5 + | +LL | #[non_exhaustive] + | ----------------- declared `#[non_exhaustive]` here +LL | #[default] +LL | Baz { + | ^^^ + | + = help: consider a manual implementation of `Default` + +error: `#[non_exhaustive]` can't be used to annotate items with default field values + --> $DIR/default-field-values-non_exhaustive.rs:4:1 + | +LL | #[non_exhaustive] + | ^^^^^^^^^^^^^^^^^ +LL | / struct Foo { +LL | | x: i32 = 42 + 3, +LL | | } + | |_- this struct has default field values + +error: aborting due to 2 previous errors + From 496adcf36cbbba53f7f6ca04110924967b33baac Mon Sep 17 00:00:00 2001 From: Marijn Schouten Date: Fri, 20 Dec 2024 18:20:40 +0100 Subject: [PATCH 27/28] remove reference to dangling from slice::Iter This aligns the comment with reality and with IterMut. Also dangling does not seem to take any arguments. --- library/core/src/slice/iter.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/core/src/slice/iter.rs b/library/core/src/slice/iter.rs index c5746157d01b2..a27baf9db224c 100644 --- a/library/core/src/slice/iter.rs +++ b/library/core/src/slice/iter.rs @@ -68,7 +68,7 @@ pub struct Iter<'a, T: 'a> { ptr: NonNull, /// For non-ZSTs, the non-null pointer to the past-the-end element. /// - /// For ZSTs, this is `ptr::dangling(len)`. + /// For ZSTs, this is `ptr::without_provenance_mut(len)`. end_or_len: *const T, _marker: PhantomData<&'a T>, } From 9965ad76201dec64cbc83e925ee8005c275461e8 Mon Sep 17 00:00:00 2001 From: Urgau Date: Thu, 19 Dec 2024 21:59:37 +0100 Subject: [PATCH 28/28] Also lint on option of function pointer comparisons --- compiler/rustc_lint/src/types.rs | 22 ++++++++++++++++++-- tests/ui/lint/fn-ptr-comparisons-some.rs | 17 +++++++++++++++ tests/ui/lint/fn-ptr-comparisons-some.stderr | 13 ++++++++++++ 3 files changed, 50 insertions(+), 2 deletions(-) create mode 100644 tests/ui/lint/fn-ptr-comparisons-some.rs create mode 100644 tests/ui/lint/fn-ptr-comparisons-some.stderr diff --git a/compiler/rustc_lint/src/types.rs b/compiler/rustc_lint/src/types.rs index 33d31d27c738f..b9f0d778e3596 100644 --- a/compiler/rustc_lint/src/types.rs +++ b/compiler/rustc_lint/src/types.rs @@ -4,7 +4,7 @@ use std::ops::ControlFlow; use rustc_abi::{BackendRepr, ExternAbi, TagEncoding, Variants, WrappingRange}; use rustc_data_structures::fx::FxHashSet; use rustc_errors::DiagMessage; -use rustc_hir::{Expr, ExprKind}; +use rustc_hir::{Expr, ExprKind, LangItem}; use rustc_middle::bug; use rustc_middle::ty::layout::{LayoutOf, SizeSkeleton}; use rustc_middle::ty::{ @@ -445,7 +445,25 @@ fn lint_fn_pointer<'tcx>( let (l_ty, l_ty_refs) = peel_refs(l_ty); let (r_ty, r_ty_refs) = peel_refs(r_ty); - if !l_ty.is_fn() || !r_ty.is_fn() { + if l_ty.is_fn() && r_ty.is_fn() { + // both operands are function pointers, fallthrough + } else if let ty::Adt(l_def, l_args) = l_ty.kind() + && let ty::Adt(r_def, r_args) = r_ty.kind() + && cx.tcx.is_lang_item(l_def.did(), LangItem::Option) + && cx.tcx.is_lang_item(r_def.did(), LangItem::Option) + && let Some(l_some_arg) = l_args.get(0) + && let Some(r_some_arg) = r_args.get(0) + && l_some_arg.expect_ty().is_fn() + && r_some_arg.expect_ty().is_fn() + { + // both operands are `Option<{function ptr}>` + return cx.emit_span_lint( + UNPREDICTABLE_FUNCTION_POINTER_COMPARISONS, + e.span, + UnpredictableFunctionPointerComparisons::Warn, + ); + } else { + // types are not function pointers, nothing to do return; } diff --git a/tests/ui/lint/fn-ptr-comparisons-some.rs b/tests/ui/lint/fn-ptr-comparisons-some.rs new file mode 100644 index 0000000000000..152e16b9884ba --- /dev/null +++ b/tests/ui/lint/fn-ptr-comparisons-some.rs @@ -0,0 +1,17 @@ +// This test checks that we lint on Option of fn ptr. +// +// https://github.com/rust-lang/rust/issues/134527. +// +//@ check-pass + +unsafe extern "C" fn func() {} + +type FnPtr = unsafe extern "C" fn(); + +fn main() { + let _ = Some::(func) == Some(func as unsafe extern "C" fn()); + //~^ WARN function pointer comparisons + + // Undecided as of https://github.com/rust-lang/rust/pull/134536 + assert_eq!(Some::(func), Some(func as unsafe extern "C" fn())); +} diff --git a/tests/ui/lint/fn-ptr-comparisons-some.stderr b/tests/ui/lint/fn-ptr-comparisons-some.stderr new file mode 100644 index 0000000000000..eefad05b676de --- /dev/null +++ b/tests/ui/lint/fn-ptr-comparisons-some.stderr @@ -0,0 +1,13 @@ +warning: function pointer comparisons do not produce meaningful results since their addresses are not guaranteed to be unique + --> $DIR/fn-ptr-comparisons-some.rs:12:13 + | +LL | let _ = Some::(func) == Some(func as unsafe extern "C" fn()); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: the address of the same function can vary between different codegen units + = note: furthermore, different functions could have the same address after being merged together + = note: for more information visit + = note: `#[warn(unpredictable_function_pointer_comparisons)]` on by default + +warning: 1 warning emitted +