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/18] 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/18] 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/18] 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/18] 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/18] 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/18] 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 cb88030b286dab999d835aeebd21a2cc56c96b8c Mon Sep 17 00:00:00 2001 From: Adrian Taylor Date: Thu, 19 Dec 2024 11:12:35 +0000 Subject: [PATCH 07/18] 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 fae72074c69c2feea0ca7a1522d201a438f2f96b Mon Sep 17 00:00:00 2001 From: Adrian Taylor Date: Thu, 19 Dec 2024 17:00:58 +0000 Subject: [PATCH 08/18] 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 05731afff2e7fb21eab3bc3c0c62c3d61a6d1d0a Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Wed, 7 Aug 2024 14:44:21 +0200 Subject: [PATCH 09/18] Add `--doctest-compilation-args` option to allow passing arguments to doctest compilation --- src/librustdoc/config.rs | 5 +++++ src/librustdoc/doctest.rs | 11 +++++++++++ src/librustdoc/lib.rs | 10 +++++++++- 3 files changed, 25 insertions(+), 1 deletion(-) diff --git a/src/librustdoc/config.rs b/src/librustdoc/config.rs index 34c91e33db700..af3c7cc7be345 100644 --- a/src/librustdoc/config.rs +++ b/src/librustdoc/config.rs @@ -172,6 +172,9 @@ pub(crate) struct Options { /// This is mainly useful for other tools that reads that debuginfo to figure out /// how to call the compiler with the same arguments. pub(crate) expanded_args: Vec, + + /// Arguments to be used when compiling doctests. + pub(crate) doctest_compilation_args: Vec, } impl fmt::Debug for Options { @@ -774,6 +777,7 @@ impl Options { let scrape_examples_options = ScrapeExamplesOptions::new(matches, dcx); let with_examples = matches.opt_strs("with-examples"); let call_locations = crate::scrape_examples::load_call_locations(with_examples, dcx); + let doctest_compilation_args = matches.opt_strs("doctest-compilation-args"); let unstable_features = rustc_feature::UnstableFeatures::from_environment(crate_name.as_deref()); @@ -819,6 +823,7 @@ impl Options { scrape_examples_options, unstable_features, expanded_args: args, + doctest_compilation_args, }; let render_options = RenderOptions { output, diff --git a/src/librustdoc/doctest.rs b/src/librustdoc/doctest.rs index ce44cb1c319aa..a53316dc6c15f 100644 --- a/src/librustdoc/doctest.rs +++ b/src/librustdoc/doctest.rs @@ -78,6 +78,17 @@ pub(crate) fn generate_args_file(file_path: &Path, options: &RustdocOptions) -> content.push(format!("-Z{unstable_option_str}")); } + for compilation_args in &options.doctest_compilation_args { + for flag in compilation_args + .split_whitespace() + .map(|flag| flag.trim()) + .filter(|flag| !flag.is_empty()) + { + // Very simple parsing implementation. Might be a good idea to correctly handle strings. + content.push(flag.to_string()); + } + } + let content = content.join("\n"); file.write_all(content.as_bytes()) diff --git a/src/librustdoc/lib.rs b/src/librustdoc/lib.rs index 7655c2e0e15e1..ff1c9c61720fa 100644 --- a/src/librustdoc/lib.rs +++ b/src/librustdoc/lib.rs @@ -642,6 +642,15 @@ fn opts() -> Vec { "Includes trait implementations and other crate info from provided path. Only use with --merge=finalize", "path/to/doc.parts/", ), + opt(Unstable, Flag, "", "html-no-source", "Disable HTML source code pages generation", ""), + opt( + Unstable, + Multi, + "", + "doctest-compilation-args", + "", + "add arguments to be used when compiling doctests", + ), // deprecated / removed options opt(Unstable, FlagMulti, "", "disable-minification", "removed", ""), opt( @@ -684,7 +693,6 @@ fn opts() -> Vec { "removed, see issue #44136 for more information", "[rust]", ), - opt(Unstable, Flag, "", "html-no-source", "Disable HTML source code pages generation", ""), ] } From 2bd869082bdf4fb950febbf5a6f7b64a81dfe2d1 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Wed, 7 Aug 2024 14:50:50 +0200 Subject: [PATCH 10/18] Add regression test for `--doctest-compilation-args` --- tests/rustdoc-ui/doctest/rustflags.rs | 12 ++++++++++++ tests/rustdoc-ui/doctest/rustflags.stdout | 6 ++++++ 2 files changed, 18 insertions(+) create mode 100644 tests/rustdoc-ui/doctest/rustflags.rs create mode 100644 tests/rustdoc-ui/doctest/rustflags.stdout diff --git a/tests/rustdoc-ui/doctest/rustflags.rs b/tests/rustdoc-ui/doctest/rustflags.rs new file mode 100644 index 0000000000000..fa460e355472a --- /dev/null +++ b/tests/rustdoc-ui/doctest/rustflags.rs @@ -0,0 +1,12 @@ +//@ check-pass +//@ compile-flags: --test -Zunstable-options --doctest-compilation-args=--cfg=testcase_must_be_present +//@ normalize-stdout-test: "tests/rustdoc-ui/doctest" -> "$$DIR" +//@ normalize-stdout-test: "finished in \d+\.\d+s" -> "finished in $$TIME" + +/// ``` +/// #[cfg(testcase_must_be_present)] +/// fn must_be_present() {} +/// +/// fn main() { must_be_present() } +/// ``` +pub struct Bar; diff --git a/tests/rustdoc-ui/doctest/rustflags.stdout b/tests/rustdoc-ui/doctest/rustflags.stdout new file mode 100644 index 0000000000000..b9da6637745bc --- /dev/null +++ b/tests/rustdoc-ui/doctest/rustflags.stdout @@ -0,0 +1,6 @@ + +running 1 test +test $DIR/rustflags.rs - Bar (line 6) ... ok + +test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in $TIME + From d3c970a1986d960ee92d3a12ded89b3a8293d82f Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Wed, 7 Aug 2024 14:55:47 +0200 Subject: [PATCH 11/18] Add explanations about `--doctest-compilation-args` in rustdoc book --- src/doc/rustdoc/src/unstable-features.md | 94 +++++++++++++++++++----- 1 file changed, 76 insertions(+), 18 deletions(-) diff --git a/src/doc/rustdoc/src/unstable-features.md b/src/doc/rustdoc/src/unstable-features.md index f19c3a51f619b..f1aa8a53422dc 100644 --- a/src/doc/rustdoc/src/unstable-features.md +++ b/src/doc/rustdoc/src/unstable-features.md @@ -315,7 +315,7 @@ Markdown file, the URL given to `--markdown-playground-url` will take precedence `--playground-url` and `#![doc(html_playground_url = "url")]` are present when rendering crate docs, the attribute will take precedence. -### `--sort-modules-by-appearance`: control how items on module pages are sorted +## `--sort-modules-by-appearance`: control how items on module pages are sorted Using this flag looks like this: @@ -328,7 +328,7 @@ some consideration for their stability, and names that end in a number). Giving `rustdoc` will disable this sorting and instead make it print the items in the order they appear in the source. -### `--show-type-layout`: add a section to each type's docs describing its memory layout +## `--show-type-layout`: add a section to each type's docs describing its memory layout * Tracking issue: [#113248](https://github.com/rust-lang/rust/issues/113248) @@ -346,7 +346,7 @@ of that type will take in memory. Note that most layout information is **completely unstable** and may even differ between compilations. -### `--resource-suffix`: modifying the name of CSS/JavaScript in crate docs +## `--resource-suffix`: modifying the name of CSS/JavaScript in crate docs * Tracking issue: [#54765](https://github.com/rust-lang/rust/issues/54765) @@ -361,7 +361,7 @@ all these files are linked from every page, changing where they are can be cumbe specially cache them. This flag will rename all these files in the output to include the suffix in the filename. For example, `light.css` would become `light-suf.css` with the above command. -### `--extern-html-root-url`: control how rustdoc links to non-local crates +## `--extern-html-root-url`: control how rustdoc links to non-local crates Using this flag looks like this: @@ -376,7 +376,7 @@ flags to control that behavior. When the `--extern-html-root-url` flag is given one of your dependencies, rustdoc use that URL for those docs. Keep in mind that if those docs exist in the output directory, those local docs will still override this flag. -### `-Z force-unstable-if-unmarked` +## `-Z force-unstable-if-unmarked` Using this flag looks like this: @@ -389,7 +389,7 @@ This is an internal flag intended for the standard library and compiler that app allows `rustdoc` to be able to generate documentation for the compiler crates and the standard library, as an equivalent command-line argument is provided to `rustc` when building those crates. -### `--index-page`: provide a top-level landing page for docs +## `--index-page`: provide a top-level landing page for docs This feature allows you to generate an index-page with a given markdown file. A good example of it is the [rust documentation index](https://doc.rust-lang.org/nightly/index.html). @@ -398,18 +398,18 @@ With this, you'll have a page which you can customize as much as you want at the Using `index-page` option enables `enable-index-page` option as well. -### `--enable-index-page`: generate a default index page for docs +## `--enable-index-page`: generate a default index page for docs This feature allows the generation of a default index-page which lists the generated crates. -### `--nocapture`: disable output capture for test +## `--nocapture`: disable output capture for test When this flag is used with `--test`, the output (stdout and stderr) of your tests won't be captured by rustdoc. Instead, the output will be directed to your terminal, as if you had run the test executable manually. This is especially useful for debugging your tests! -### `--check`: only checks the documentation +## `--check`: only checks the documentation When this flag is supplied, rustdoc will type check and lint your code, but will not generate any documentation or run your doctests. @@ -420,7 +420,7 @@ Using this flag looks like: rustdoc -Z unstable-options --check src/lib.rs ``` -### `--static-root-path`: control how static files are loaded in HTML output +## `--static-root-path`: control how static files are loaded in HTML output Using this flag looks like this: @@ -435,7 +435,7 @@ JavaScript, and font files in a single location, rather than duplicating it once files like the search index will still load from the documentation root, but anything that gets renamed with `--resource-suffix` will load from the given path. -### `--persist-doctests`: persist doctest executables after running +## `--persist-doctests`: persist doctest executables after running * Tracking issue: [#56925](https://github.com/rust-lang/rust/issues/56925) @@ -449,7 +449,7 @@ This flag allows you to keep doctest executables around after they're compiled o Usually, rustdoc will immediately discard a compiled doctest after it's been tested, but with this option, you can keep those binaries around for farther testing. -### `--show-coverage`: calculate the percentage of items with documentation +## `--show-coverage`: calculate the percentage of items with documentation * Tracking issue: [#58154](https://github.com/rust-lang/rust/issues/58154) @@ -500,7 +500,7 @@ Calculating code examples follows these rules: * typedef 2. If one of the previously listed items has a code example, then it'll be counted. -#### JSON output +### JSON output When using `--output-format json` with this option, it will display the coverage information in JSON format. For example, here is the JSON for a file with one documented item and one @@ -522,7 +522,7 @@ Note that the third item is the crate root, which in this case is undocumented. If you want the JSON output to be displayed on `stdout` instead of having a file generated, you can use `-o -`. -### `-w`/`--output-format`: output format +## `-w`/`--output-format`: output format `--output-format json` emits documentation in the experimental [JSON format](https://doc.rust-lang.org/nightly/nightly-rustc/rustdoc_json_types/). `--output-format html` has no effect, @@ -542,7 +542,7 @@ It can also be used with `--show-coverage`. Take a look at its [documentation](#--show-coverage-calculate-the-percentage-of-items-with-documentation) for more information. -### `--enable-per-target-ignores`: allow `ignore-foo` style filters for doctests +## `--enable-per-target-ignores`: allow `ignore-foo` style filters for doctests * Tracking issue: [#64245](https://github.com/rust-lang/rust/issues/64245) @@ -577,7 +577,7 @@ struct Foo; In older versions, this will be ignored on all targets, but on newer versions `ignore-gnu` will override `ignore`. -### `--runtool`, `--runtool-arg`: program to run tests with; args to pass to it +## `--runtool`, `--runtool-arg`: program to run tests with; args to pass to it * Tracking issue: [#64245](https://github.com/rust-lang/rust/issues/64245) @@ -596,7 +596,7 @@ $ rustdoc src/lib.rs -Z unstable-options --runtool valgrind Another use case would be to run a test inside an emulator, or through a Virtual Machine. -### `--with-examples`: include examples of uses of items as documentation +## `--with-examples`: include examples of uses of items as documentation * Tracking issue: [#88791](https://github.com/rust-lang/rust/issues/88791) @@ -625,7 +625,7 @@ crate being documented (`foobar`) and a path to output the calls To scrape examples from test code, e.g. functions marked `#[test]`, then add the `--scrape-tests` flag. -### `--generate-link-to-definition`: Generate links on types in source code +## `--generate-link-to-definition`: Generate links on types in source code * Tracking issue: [#89095](https://github.com/rust-lang/rust/issues/89095) @@ -664,3 +664,61 @@ Similar to cargo `build.rustc-wrapper` option, this flag takes a `rustc` wrapper The first argument to the program will be the test builder program. This flag can be passed multiple times to nest wrappers. + +## Passing arguments to rustc when compiling doctests + +You can use the `--doctest-compilation-args` option if you want to add options when compiling the +doctest. For example if you have: + +```rust,no_run +/// ``` +/// #![deny(warnings)] +/// #![feature(async_await)] +/// +/// let x = 12; +/// ``` +pub struct Bar; +``` + +And you run `rustdoc --test` on it, you will get: + +```console +running 1 test +test foo.rs - Bar (line 1) ... FAILED + +failures: + +---- foo.rs - Bar (line 1) stdout ---- +error: the feature `async_await` has been stable since 1.39.0 and no longer requires an attribute to enable + --> foo.rs:2:12 + | +3 | #![feature(async_await)] + | ^^^^^^^^^^^ + | +note: the lint level is defined here + --> foo.rs:1:9 + | +2 | #![deny(warnings)] + | ^^^^^^^^ + = note: `#[deny(stable_features)]` implied by `#[deny(warnings)]` + +error: aborting due to 1 previous error + +Couldn't compile the test. + +failures: + foo.rs - Bar (line 1) + +test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.03s +``` + +But if you can limit the lint level to warning by using `--doctest_compilation_args=--cap-lints=warn`: + +```console +$ rustdoc --test --doctest_compilation_args=--cap-lints=warn file.rs + +running 1 test +test tests/rustdoc-ui/doctest/rustflags.rs - Bar (line 5) ... ok + +test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.06s +``` 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 12/18] 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 24fafe7d14303643de24cc31bb6d1acc0568bcb5 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Mon, 19 Aug 2024 17:52:21 +0200 Subject: [PATCH 13/18] Update `run-make/rustdoc-default-output` test --- tests/run-make/rustdoc-default-output/output-default.stdout | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/run-make/rustdoc-default-output/output-default.stdout b/tests/run-make/rustdoc-default-output/output-default.stdout index 125443ce61964..c2d9309ba2e6d 100644 --- a/tests/run-make/rustdoc-default-output/output-default.stdout +++ b/tests/run-make/rustdoc-default-output/output-default.stdout @@ -189,6 +189,10 @@ Options: --include-parts-dir path/to/doc.parts/ Includes trait implementations and other crate info from provided path. Only use with --merge=finalize + --html-no-source + Disable HTML source code pages generation + --doctest-compilation-args add arguments to be used when compiling doctests + --disable-minification removed --plugin-path DIR @@ -209,8 +213,6 @@ Options: removed, see issue #44136 for more information - --html-no-source - Disable HTML source code pages generation @path Read newline separated options from `path` From cbb3df41fb48d0a5ebcdd0f4b44cc3ad6a5a837a Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Wed, 28 Aug 2024 16:19:52 +0200 Subject: [PATCH 14/18] Split arguments from `--doctest-compilation-args` like a shell would --- src/librustdoc/doctest.rs | 72 ++++++++++++++++++++++++++++++++++----- 1 file changed, 64 insertions(+), 8 deletions(-) diff --git a/src/librustdoc/doctest.rs b/src/librustdoc/doctest.rs index a53316dc6c15f..a93e2f927182e 100644 --- a/src/librustdoc/doctest.rs +++ b/src/librustdoc/doctest.rs @@ -50,6 +50,46 @@ pub(crate) struct GlobalTestOptions { pub(crate) args_file: PathBuf, } +/// Function used to split command line arguments just like a shell would. +fn split_args(args: &str) -> Vec { + let mut out = Vec::new(); + let mut iter = args.chars(); + let mut current = String::new(); + + while let Some(c) = iter.next() { + if c == '\\' { + if let Some(c) = iter.next() { + // If it's escaped, even a quote or a whitespace will be ignored. + current.push(c); + } + } else if c == '"' || c == '\'' { + while let Some(new_c) = iter.next() { + if new_c == c { + break; + } else if new_c == '\\' { + if let Some(c) = iter.next() { + // If it's escaped, even a quote will be ignored. + current.push(c); + } + } else { + current.push(new_c); + } + } + } else if " \n\t\r".contains(c) { + if !current.is_empty() { + out.push(current.clone()); + current.clear(); + } + } else { + current.push(c); + } + } + if !current.is_empty() { + out.push(current); + } + out +} + pub(crate) fn generate_args_file(file_path: &Path, options: &RustdocOptions) -> Result<(), String> { let mut file = File::create(file_path) .map_err(|error| format!("failed to create args file: {error:?}"))?; @@ -79,14 +119,7 @@ pub(crate) fn generate_args_file(file_path: &Path, options: &RustdocOptions) -> } for compilation_args in &options.doctest_compilation_args { - for flag in compilation_args - .split_whitespace() - .map(|flag| flag.trim()) - .filter(|flag| !flag.is_empty()) - { - // Very simple parsing implementation. Might be a good idea to correctly handle strings. - content.push(flag.to_string()); - } + content.extend(split_args(compilation_args)); } let content = content.join("\n"); @@ -1003,6 +1036,29 @@ fn doctest_run_fn( Ok(()) } +#[cfg(test)] +#[test] +fn check_split_args() { + fn compare(input: &str, expected: &[&str]) { + let output = split_args(input); + let expected = expected.iter().map(|s| s.to_string()).collect::>(); + assert_eq!(expected, output, "test failed for {input:?}"); + } + + compare("'a' \"b\"c", &["a", "bc"]); + compare("'a' \"b \"c d", &["a", "b c", "d"]); + compare("'a' \"b\\\"c\"", &["a", "b\"c"]); + compare("'a\"'", &["a\""]); + compare("\"a'\"", &["a'"]); + compare("\\ a", &[" a"]); + compare("\\\\", &["\\"]); + compare("a'", &["a"]); + compare("a ", &["a"]); + compare("a b", &["a", "b"]); + compare("a\n\t \rb", &["a", "b"]); + compare("a\n\t1 \rb", &["a", "1", "b"]); +} + #[cfg(test)] // used in tests impl DocTestVisitor for Vec { fn visit_test(&mut self, _test: String, _config: LangString, rel_line: MdRelLine) { From 55653a5178c07a0a854066e79062de4c585561a5 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Fri, 20 Dec 2024 12:35:57 +0100 Subject: [PATCH 15/18] Add explanations on how arguments are split --- src/doc/rustdoc/src/unstable-features.md | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/src/doc/rustdoc/src/unstable-features.md b/src/doc/rustdoc/src/unstable-features.md index f1aa8a53422dc..667f07cf95630 100644 --- a/src/doc/rustdoc/src/unstable-features.md +++ b/src/doc/rustdoc/src/unstable-features.md @@ -722,3 +722,22 @@ test tests/rustdoc-ui/doctest/rustflags.rs - Bar (line 5) ... ok test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.06s ``` + +The parsing of arguments works as follows: if it encounters a `"` or a `'`, it will continue +until it finds the character unescaped (without a prepending `\`). If not inside a string, a +whitespace character will also split arguments. Example: + +```text +"hello 'a'\" ok" how are 'you today?' +``` + +will be split as follows: + +```text +[ + "hello 'a'\" ok", + "how", + "are", + "you today?", +] +``` From bc03e40a29574c18ebfc04bd179065815ebe83f8 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Fri, 20 Dec 2024 15:14:08 +0100 Subject: [PATCH 16/18] Move test into the `tests.rs` file --- src/librustdoc/doctest.rs | 23 ----------------------- src/librustdoc/doctest/tests.rs | 22 ++++++++++++++++++++++ 2 files changed, 22 insertions(+), 23 deletions(-) diff --git a/src/librustdoc/doctest.rs b/src/librustdoc/doctest.rs index a93e2f927182e..e6e5123d0bba0 100644 --- a/src/librustdoc/doctest.rs +++ b/src/librustdoc/doctest.rs @@ -1036,29 +1036,6 @@ fn doctest_run_fn( Ok(()) } -#[cfg(test)] -#[test] -fn check_split_args() { - fn compare(input: &str, expected: &[&str]) { - let output = split_args(input); - let expected = expected.iter().map(|s| s.to_string()).collect::>(); - assert_eq!(expected, output, "test failed for {input:?}"); - } - - compare("'a' \"b\"c", &["a", "bc"]); - compare("'a' \"b \"c d", &["a", "b c", "d"]); - compare("'a' \"b\\\"c\"", &["a", "b\"c"]); - compare("'a\"'", &["a\""]); - compare("\"a'\"", &["a'"]); - compare("\\ a", &[" a"]); - compare("\\\\", &["\\"]); - compare("a'", &["a"]); - compare("a ", &["a"]); - compare("a b", &["a", "b"]); - compare("a\n\t \rb", &["a", "b"]); - compare("a\n\t1 \rb", &["a", "1", "b"]); -} - #[cfg(test)] // used in tests impl DocTestVisitor for Vec { fn visit_test(&mut self, _test: String, _config: LangString, rel_line: MdRelLine) { diff --git a/src/librustdoc/doctest/tests.rs b/src/librustdoc/doctest/tests.rs index 160d0f222b4e0..fa6cca3681be5 100644 --- a/src/librustdoc/doctest/tests.rs +++ b/src/librustdoc/doctest/tests.rs @@ -379,3 +379,25 @@ fn main() { let (output, len) = make_test(input, None, false, &opts, None); assert_eq!((output, len), (expected, 1)); } + +#[test] +fn check_split_args() { + fn compare(input: &str, expected: &[&str]) { + let output = super::split_args(input); + let expected = expected.iter().map(|s| s.to_string()).collect::>(); + assert_eq!(expected, output, "test failed for {input:?}"); + } + + compare("'a' \"b\"c", &["a", "bc"]); + compare("'a' \"b \"c d", &["a", "b c", "d"]); + compare("'a' \"b\\\"c\"", &["a", "b\"c"]); + compare("'a\"'", &["a\""]); + compare("\"a'\"", &["a'"]); + compare("\\ a", &[" a"]); + compare("\\\\", &["\\"]); + compare("a'", &["a"]); + compare("a ", &["a"]); + compare("a b", &["a", "b"]); + compare("a\n\t \rb", &["a", "b"]); + compare("a\n\t1 \rb", &["a", "1", "b"]); +} From 2d914bed2dea1316fdbb55010f3bb2c5412942cb Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Fri, 20 Dec 2024 15:39:26 +0100 Subject: [PATCH 17/18] Add test to ensure passing `--doctest_compilation_args` multiple times work --- src/doc/rustdoc/src/unstable-features.md | 2 +- .../doctest/rustflags-multiple-args.rs | 17 +++++++++++++++++ .../doctest/rustflags-multiple-args.stdout | 6 ++++++ 3 files changed, 24 insertions(+), 1 deletion(-) create mode 100644 tests/rustdoc-ui/doctest/rustflags-multiple-args.rs create mode 100644 tests/rustdoc-ui/doctest/rustflags-multiple-args.stdout diff --git a/src/doc/rustdoc/src/unstable-features.md b/src/doc/rustdoc/src/unstable-features.md index 667f07cf95630..d1d42e4732258 100644 --- a/src/doc/rustdoc/src/unstable-features.md +++ b/src/doc/rustdoc/src/unstable-features.md @@ -667,7 +667,7 @@ This flag can be passed multiple times to nest wrappers. ## Passing arguments to rustc when compiling doctests -You can use the `--doctest-compilation-args` option if you want to add options when compiling the +You can use the `--doctest-compilation-args` flag if you want to add options when compiling the doctest. For example if you have: ```rust,no_run diff --git a/tests/rustdoc-ui/doctest/rustflags-multiple-args.rs b/tests/rustdoc-ui/doctest/rustflags-multiple-args.rs new file mode 100644 index 0000000000000..8519920e53bef --- /dev/null +++ b/tests/rustdoc-ui/doctest/rustflags-multiple-args.rs @@ -0,0 +1,17 @@ +// This test checks that the test behave when `--doctest-compilation-args` is passed +// multiple times. + +//@ check-pass +//@ compile-flags: --test -Zunstable-options --doctest-compilation-args=--cfg=testcase_must_be_present +//@ compile-flags: --doctest-compilation-args=--cfg=another +//@ normalize-stdout-test: "tests/rustdoc-ui/doctest" -> "$$DIR" +//@ normalize-stdout-test: "finished in \d+\.\d+s" -> "finished in $$TIME" + +/// ``` +/// #[cfg(testcase_must_be_present)] +/// #[cfg(another)] +/// fn must_be_present() {} +/// +/// fn main() { must_be_present() } +/// ``` +pub struct Bar; diff --git a/tests/rustdoc-ui/doctest/rustflags-multiple-args.stdout b/tests/rustdoc-ui/doctest/rustflags-multiple-args.stdout new file mode 100644 index 0000000000000..0e8a9e1efcf6f --- /dev/null +++ b/tests/rustdoc-ui/doctest/rustflags-multiple-args.stdout @@ -0,0 +1,6 @@ + +running 1 test +test $DIR/rustflags-multiple-args.rs - Bar (line 10) ... ok + +test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in $TIME + From 9965ad76201dec64cbc83e925ee8005c275461e8 Mon Sep 17 00:00:00 2001 From: Urgau Date: Thu, 19 Dec 2024 21:59:37 +0100 Subject: [PATCH 18/18] 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 +