From 53469cfc3f0cc25530c1b5407f5dad97f103f8c6 Mon Sep 17 00:00:00 2001 From: "Marc R. Schoolderman" <4155359+squell@users.noreply.github.com> Date: Fri, 17 Mar 2023 14:33:50 +0100 Subject: [PATCH 01/15] fix typo in documentation for std::fs::Permissions --- library/std/src/fs.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/std/src/fs.rs b/library/std/src/fs.rs index c550378e7d6b7..342ea15623184 100644 --- a/library/std/src/fs.rs +++ b/library/std/src/fs.rs @@ -1406,7 +1406,7 @@ impl Permissions { /// On Unix-based platforms this checks if *any* of the owner, group or others /// write permission bits are set. It does not check if the current /// user is in the file's assigned group. It also does not check ACLs. - /// Therefore even if this returns true you may not be able to write to the + /// Therefore even if this returns false you may not be able to write to the /// file, and vice versa. The [`PermissionsExt`] trait gives direct access /// to the permission bits but also does not read ACLs. If you need to /// accurately know whether or not a file is writable use the `access()` From 0df002a2e2001bf87f56d27ebf300daa4b574b6b Mon Sep 17 00:00:00 2001 From: Marc Schoolderman Date: Tue, 29 Aug 2023 11:20:56 +0200 Subject: [PATCH 02/15] rewording after comments by @thomcc --- library/std/src/fs.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/library/std/src/fs.rs b/library/std/src/fs.rs index 342ea15623184..e506b321c2518 100644 --- a/library/std/src/fs.rs +++ b/library/std/src/fs.rs @@ -1406,11 +1406,11 @@ impl Permissions { /// On Unix-based platforms this checks if *any* of the owner, group or others /// write permission bits are set. It does not check if the current /// user is in the file's assigned group. It also does not check ACLs. - /// Therefore even if this returns false you may not be able to write to the - /// file, and vice versa. The [`PermissionsExt`] trait gives direct access - /// to the permission bits but also does not read ACLs. If you need to - /// accurately know whether or not a file is writable use the `access()` - /// function from libc. + /// Therefore the return value of this function cannot be relied upon + /// to predict whether attempts to read or write the file will actually succeed. + /// The [`PermissionsExt`] trait gives direct access to the permission bits but + /// also does not read ACLs. If you need to accurately know whether or not a file + /// is writable use the `access()` function from libc. /// /// [`PermissionsExt`]: crate::os::unix::fs::PermissionsExt /// From 2ed49fc8c75fdf3b47a05840218b2c8489fea794 Mon Sep 17 00:00:00 2001 From: John Millikin Date: Wed, 25 Oct 2023 17:01:21 +0900 Subject: [PATCH 03/15] Convert `Unix{Datagram,Stream}::{set_}passcred()` to per-OS traits These methods are the pre-stabilized API for obtaining peer credentials from an `AF_UNIX` socket, part of the `unix_socket_ancillary_data` feature. Their current behavior is to get/set one of the `SO_PASSCRED` (Linux), `LOCAL_CREDS_PERSISTENT` (FreeBSD), or `LOCAL_CREDS` (NetBSD) socket options. On other targets the `{set_}passcred()` methods do not exist. There are two problems with this approach: 1. Having public methods only exist for certain targets isn't permitted in a stable `std` API. 2. These options have generally similar purposes, but they are non-POSIX and their details can differ in subtle and surprising ways (such as whether they continue to be set after the next call to `recvmsg()`). Splitting into OS-specific extension traits is the preferred solution to both problems. --- library/std/src/os/android/net.rs | 3 + library/std/src/os/freebsd/mod.rs | 1 + library/std/src/os/freebsd/net.rs | 65 +++++++++++++++++++ library/std/src/os/linux/net.rs | 3 + library/std/src/os/net/linux_ext/mod.rs | 3 + library/std/src/os/net/linux_ext/socket.rs | 63 ++++++++++++++++++ library/std/src/os/netbsd/mod.rs | 1 + library/std/src/os/netbsd/net.rs | 65 +++++++++++++++++++ library/std/src/os/unix/net/datagram.rs | 75 ++++------------------ library/std/src/os/unix/net/stream.rs | 75 ++++------------------ library/std/src/os/unix/net/tests.rs | 3 + library/std/src/sys/pal/unix/net.rs | 26 +++++--- 12 files changed, 247 insertions(+), 136 deletions(-) create mode 100644 library/std/src/os/freebsd/net.rs create mode 100644 library/std/src/os/net/linux_ext/socket.rs create mode 100644 library/std/src/os/netbsd/net.rs diff --git a/library/std/src/os/android/net.rs b/library/std/src/os/android/net.rs index fe40d6319c2ff..349e73eaabdaf 100644 --- a/library/std/src/os/android/net.rs +++ b/library/std/src/os/android/net.rs @@ -5,5 +5,8 @@ #[stable(feature = "unix_socket_abstract", since = "1.70.0")] pub use crate::os::net::linux_ext::addr::SocketAddrExt; +#[unstable(feature = "unix_socket_ancillary_data", issue = "76915")] +pub use crate::os::net::linux_ext::socket::UnixSocketExt; + #[unstable(feature = "tcp_quickack", issue = "96256")] pub use crate::os::net::linux_ext::tcp::TcpStreamExt; diff --git a/library/std/src/os/freebsd/mod.rs b/library/std/src/os/freebsd/mod.rs index c072fae557fe0..39912e6970d45 100644 --- a/library/std/src/os/freebsd/mod.rs +++ b/library/std/src/os/freebsd/mod.rs @@ -3,4 +3,5 @@ #![stable(feature = "raw_ext", since = "1.1.0")] pub mod fs; +pub mod net; pub mod raw; diff --git a/library/std/src/os/freebsd/net.rs b/library/std/src/os/freebsd/net.rs new file mode 100644 index 0000000000000..33990d54caae0 --- /dev/null +++ b/library/std/src/os/freebsd/net.rs @@ -0,0 +1,65 @@ +//! FreeBSD-specific networking functionality. + +#![unstable(feature = "unix_socket_ancillary_data", issue = "76915")] + +use crate::io; +use crate::os::unix::net; +use crate::sealed::Sealed; +use crate::sys_common::AsInner; + +/// FreeBSD-specific functionality for `AF_UNIX` sockets [`UnixDatagram`] +/// and [`UnixStream`]. +/// +/// [`UnixDatagram`]: net::UnixDatagram +/// [`UnixStream`]: net::UnixStream +#[unstable(feature = "unix_socket_ancillary_data", issue = "76915")] +pub trait UnixSocketExt: Sealed { + /// Query the current setting of socket option `LOCAL_CREDS_PERSISTENT`. + #[unstable(feature = "unix_socket_ancillary_data", issue = "76915")] + fn local_creds_persistent(&self) -> io::Result; + + /// Enable or disable socket option `LOCAL_CREDS_PERSISTENT`. + /// + /// This option enables the credentials of the sending process to be + /// received as a control message in [`AncillaryData`]. + /// + /// [`AncillaryData`]: net::AncillaryData + /// + /// # Examples + /// + /// ```no_run + /// #![feature(unix_socket_ancillary_data)] + /// use std::os::freebsd::net::UnixSocketExt; + /// use std::os::unix::net::UnixDatagram; + /// + /// fn main() -> std::io::Result<()> { + /// let sock = UnixDatagram::unbound()?; + /// sock.set_local_creds_persistent(true).expect("set_local_creds_persistent failed"); + /// Ok(()) + /// } + /// ``` + #[unstable(feature = "unix_socket_ancillary_data", issue = "76915")] + fn set_local_creds_persistent(&self, local_creds_persistent: bool) -> io::Result<()>; +} + +#[unstable(feature = "unix_socket_ancillary_data", issue = "76915")] +impl UnixSocketExt for net::UnixDatagram { + fn local_creds_persistent(&self) -> io::Result { + self.as_inner().local_creds_persistent() + } + + fn set_local_creds_persistent(&self, local_creds_persistent: bool) -> io::Result<()> { + self.as_inner().set_local_creds_persistent(local_creds_persistent) + } +} + +#[unstable(feature = "unix_socket_ancillary_data", issue = "76915")] +impl UnixSocketExt for net::UnixStream { + fn local_creds_persistent(&self) -> io::Result { + self.as_inner().local_creds_persistent() + } + + fn set_local_creds_persistent(&self, local_creds_persistent: bool) -> io::Result<()> { + self.as_inner().set_local_creds_persistent(local_creds_persistent) + } +} diff --git a/library/std/src/os/linux/net.rs b/library/std/src/os/linux/net.rs index c8e734d740b68..f898e70548706 100644 --- a/library/std/src/os/linux/net.rs +++ b/library/std/src/os/linux/net.rs @@ -5,5 +5,8 @@ #[stable(feature = "unix_socket_abstract", since = "1.70.0")] pub use crate::os::net::linux_ext::addr::SocketAddrExt; +#[unstable(feature = "unix_socket_ancillary_data", issue = "76915")] +pub use crate::os::net::linux_ext::socket::UnixSocketExt; + #[unstable(feature = "tcp_quickack", issue = "96256")] pub use crate::os::net::linux_ext::tcp::TcpStreamExt; diff --git a/library/std/src/os/net/linux_ext/mod.rs b/library/std/src/os/net/linux_ext/mod.rs index 62e78cc50d4ab..d0979640c32ea 100644 --- a/library/std/src/os/net/linux_ext/mod.rs +++ b/library/std/src/os/net/linux_ext/mod.rs @@ -5,6 +5,9 @@ #[stable(feature = "unix_socket_abstract", since = "1.70.0")] pub(crate) mod addr; +#[unstable(feature = "unix_socket_ancillary_data", issue = "76915")] +pub(crate) mod socket; + #[unstable(feature = "tcp_quickack", issue = "96256")] pub(crate) mod tcp; diff --git a/library/std/src/os/net/linux_ext/socket.rs b/library/std/src/os/net/linux_ext/socket.rs new file mode 100644 index 0000000000000..4e4168f693c34 --- /dev/null +++ b/library/std/src/os/net/linux_ext/socket.rs @@ -0,0 +1,63 @@ +//! Linux and Android-specific socket functionality. + +use crate::io; +use crate::os::unix::net; +use crate::sealed::Sealed; +use crate::sys_common::AsInner; + +/// Linux-specific functionality for `AF_UNIX` sockets [`UnixDatagram`] +/// and [`UnixStream`]. +/// +/// [`UnixDatagram`]: net::UnixDatagram +/// [`UnixStream`]: net::UnixStream +#[unstable(feature = "unix_socket_ancillary_data", issue = "76915")] +pub trait UnixSocketExt: Sealed { + /// Query the current setting of socket option `SO_PASSCRED`. + #[unstable(feature = "unix_socket_ancillary_data", issue = "76915")] + fn passcred(&self) -> io::Result; + + /// Enable or disable socket option `SO_PASSCRED`. + /// + /// This option enables the credentials of the sending process to be + /// received as a control message in [`AncillaryData`]. + /// + /// [`AncillaryData`]: net::AncillaryData + /// + /// # Examples + /// + /// ```no_run + /// #![feature(unix_socket_ancillary_data)] + /// use std::os::linux::net::UnixSocketExt; + /// use std::os::unix::net::UnixDatagram; + /// + /// fn main() -> std::io::Result<()> { + /// let sock = UnixDatagram::unbound()?; + /// sock.set_passcred(true).expect("set_passcred failed"); + /// Ok(()) + /// } + /// ``` + #[unstable(feature = "unix_socket_ancillary_data", issue = "76915")] + fn set_passcred(&self, passcred: bool) -> io::Result<()>; +} + +#[unstable(feature = "unix_socket_ancillary_data", issue = "76915")] +impl UnixSocketExt for net::UnixDatagram { + fn passcred(&self) -> io::Result { + self.as_inner().passcred() + } + + fn set_passcred(&self, passcred: bool) -> io::Result<()> { + self.as_inner().set_passcred(passcred) + } +} + +#[unstable(feature = "unix_socket_ancillary_data", issue = "76915")] +impl UnixSocketExt for net::UnixStream { + fn passcred(&self) -> io::Result { + self.as_inner().passcred() + } + + fn set_passcred(&self, passcred: bool) -> io::Result<()> { + self.as_inner().set_passcred(passcred) + } +} diff --git a/library/std/src/os/netbsd/mod.rs b/library/std/src/os/netbsd/mod.rs index 497a51a1df6fd..2f21e98a6f4cb 100644 --- a/library/std/src/os/netbsd/mod.rs +++ b/library/std/src/os/netbsd/mod.rs @@ -3,4 +3,5 @@ #![stable(feature = "raw_ext", since = "1.1.0")] pub mod fs; +pub mod net; pub mod raw; diff --git a/library/std/src/os/netbsd/net.rs b/library/std/src/os/netbsd/net.rs new file mode 100644 index 0000000000000..5c82f43077d4c --- /dev/null +++ b/library/std/src/os/netbsd/net.rs @@ -0,0 +1,65 @@ +//! NetBSD-specific networking functionality. + +#![unstable(feature = "unix_socket_ancillary_data", issue = "76915")] + +use crate::io; +use crate::os::unix::net; +use crate::sealed::Sealed; +use crate::sys_common::AsInner; + +/// NetBSD-specific functionality for `AF_UNIX` sockets [`UnixDatagram`] +/// and [`UnixStream`]. +/// +/// [`UnixDatagram`]: net::UnixDatagram +/// [`UnixStream`]: net::UnixStream +#[unstable(feature = "unix_socket_ancillary_data", issue = "76915")] +pub trait UnixSocketExt: Sealed { + /// Query the current setting of socket option `LOCAL_CREDS`. + #[unstable(feature = "unix_socket_ancillary_data", issue = "76915")] + fn local_creds(&self) -> io::Result; + + /// Enable or disable socket option `LOCAL_CREDS`. + /// + /// This option enables the credentials of the sending process to be + /// received as a control message in [`AncillaryData`]. + /// + /// [`AncillaryData`]: net::AncillaryData + /// + /// # Examples + /// + /// ```no_run + /// #![feature(unix_socket_ancillary_data)] + /// use std::os::netbsd::net::UnixSocketExt; + /// use std::os::unix::net::UnixDatagram; + /// + /// fn main() -> std::io::Result<()> { + /// let sock = UnixDatagram::unbound()?; + /// sock.set_local_creds(true).expect("set_local_creds failed"); + /// Ok(()) + /// } + /// ``` + #[unstable(feature = "unix_socket_ancillary_data", issue = "76915")] + fn set_local_creds(&self, local_creds: bool) -> io::Result<()>; +} + +#[unstable(feature = "unix_socket_ancillary_data", issue = "76915")] +impl UnixSocketExt for net::UnixDatagram { + fn local_creds(&self) -> io::Result { + self.as_inner().local_creds() + } + + fn set_local_creds(&self, local_creds: bool) -> io::Result<()> { + self.as_inner().set_local_creds(local_creds) + } +} + +#[unstable(feature = "unix_socket_ancillary_data", issue = "76915")] +impl UnixSocketExt for net::UnixStream { + fn local_creds(&self) -> io::Result { + self.as_inner().local_creds() + } + + fn set_local_creds(&self, local_creds: bool) -> io::Result<()> { + self.as_inner().set_local_creds(local_creds) + } +} diff --git a/library/std/src/os/unix/net/datagram.rs b/library/std/src/os/unix/net/datagram.rs index 0b4d955294ca5..df698c17f6cc2 100644 --- a/library/std/src/os/unix/net/datagram.rs +++ b/library/std/src/os/unix/net/datagram.rs @@ -6,6 +6,7 @@ use crate::io::{IoSlice, IoSliceMut}; use crate::net::Shutdown; use crate::os::unix::io::{AsFd, AsRawFd, BorrowedFd, FromRawFd, IntoRawFd, OwnedFd, RawFd}; use crate::path::Path; +use crate::sealed::Sealed; use crate::sys::cvt; use crate::sys::net::Socket; use crate::sys_common::{AsInner, FromInner, IntoInner}; @@ -54,6 +55,10 @@ const MSG_NOSIGNAL: libc::c_int = 0x0; #[stable(feature = "unix_socket", since = "1.10.0")] pub struct UnixDatagram(Socket); +/// Allows extension traits within `std`. +#[unstable(feature = "sealed", issue = "none")] +impl Sealed for UnixDatagram {} + #[stable(feature = "unix_socket", since = "1.10.0")] impl fmt::Debug for UnixDatagram { fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result { @@ -802,69 +807,6 @@ impl UnixDatagram { self.0.set_nonblocking(nonblocking) } - /// Moves the socket to pass unix credentials as control message in [`SocketAncillary`]. - /// - /// Set the socket option `SO_PASSCRED`. - /// - /// # Examples - /// - #[cfg_attr( - any( - target_os = "android", - target_os = "linux", - target_os = "netbsd", - target_os = "freebsd", - ), - doc = "```no_run" - )] - #[cfg_attr( - not(any( - target_os = "android", - target_os = "linux", - target_os = "netbsd", - target_os = "freebsd" - )), - doc = "```ignore" - )] - /// #![feature(unix_socket_ancillary_data)] - /// use std::os::unix::net::UnixDatagram; - /// - /// fn main() -> std::io::Result<()> { - /// let sock = UnixDatagram::unbound()?; - /// sock.set_passcred(true).expect("set_passcred function failed"); - /// Ok(()) - /// } - /// ``` - #[cfg(any( - doc, - target_os = "android", - target_os = "linux", - target_os = "netbsd", - target_os = "freebsd" - ))] - #[unstable(feature = "unix_socket_ancillary_data", issue = "76915")] - pub fn set_passcred(&self, passcred: bool) -> io::Result<()> { - self.0.set_passcred(passcred) - } - - /// Get the current value of the socket for passing unix credentials in [`SocketAncillary`]. - /// This value can be change by [`set_passcred`]. - /// - /// Get the socket option `SO_PASSCRED`. - /// - /// [`set_passcred`]: UnixDatagram::set_passcred - #[cfg(any( - doc, - target_os = "android", - target_os = "linux", - target_os = "netbsd", - target_os = "freebsd" - ))] - #[unstable(feature = "unix_socket_ancillary_data", issue = "76915")] - pub fn passcred(&self) -> io::Result { - self.0.passcred() - } - /// Set the id of the socket for network filtering purpose /// #[cfg_attr( @@ -1038,3 +980,10 @@ impl From for UnixDatagram { unsafe { Self::from_raw_fd(owned.into_raw_fd()) } } } + +impl AsInner for UnixDatagram { + #[inline] + fn as_inner(&self) -> &Socket { + &self.0 + } +} diff --git a/library/std/src/os/unix/net/stream.rs b/library/std/src/os/unix/net/stream.rs index b1cd504e21939..9205160455dde 100644 --- a/library/std/src/os/unix/net/stream.rs +++ b/library/std/src/os/unix/net/stream.rs @@ -19,6 +19,7 @@ use crate::os::unix::io::{AsFd, AsRawFd, BorrowedFd, FromRawFd, IntoRawFd, Owned ))] use crate::os::unix::ucred; use crate::path::Path; +use crate::sealed::Sealed; use crate::sys::cvt; use crate::sys::net::Socket; use crate::sys_common::{AsInner, FromInner}; @@ -59,6 +60,10 @@ pub use ucred::UCred; #[stable(feature = "unix_socket", since = "1.10.0")] pub struct UnixStream(pub(super) Socket); +/// Allows extension traits within `std`. +#[unstable(feature = "sealed", issue = "none")] +impl Sealed for UnixStream {} + #[stable(feature = "unix_socket", since = "1.10.0")] impl fmt::Debug for UnixStream { fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result { @@ -394,69 +399,6 @@ impl UnixStream { self.0.set_nonblocking(nonblocking) } - /// Moves the socket to pass unix credentials as control message in [`SocketAncillary`]. - /// - /// Set the socket option `SO_PASSCRED`. - /// - /// # Examples - /// - #[cfg_attr( - any( - target_os = "android", - target_os = "linux", - target_os = "netbsd", - target_os = "freebsd" - ), - doc = "```no_run" - )] - #[cfg_attr( - not(any( - target_os = "android", - target_os = "linux", - target_os = "netbsd", - target_os = "freebsd" - )), - doc = "```ignore" - )] - /// #![feature(unix_socket_ancillary_data)] - /// use std::os::unix::net::UnixStream; - /// - /// fn main() -> std::io::Result<()> { - /// let socket = UnixStream::connect("/tmp/sock")?; - /// socket.set_passcred(true).expect("Couldn't set passcred"); - /// Ok(()) - /// } - /// ``` - #[cfg(any( - doc, - target_os = "android", - target_os = "linux", - target_os = "netbsd", - target_os = "freebsd" - ))] - #[unstable(feature = "unix_socket_ancillary_data", issue = "76915")] - pub fn set_passcred(&self, passcred: bool) -> io::Result<()> { - self.0.set_passcred(passcred) - } - - /// Get the current value of the socket for passing unix credentials in [`SocketAncillary`]. - /// This value can be change by [`set_passcred`]. - /// - /// Get the socket option `SO_PASSCRED`. - /// - /// [`set_passcred`]: UnixStream::set_passcred - #[cfg(any( - doc, - target_os = "android", - target_os = "linux", - target_os = "netbsd", - target_os = "freebsd" - ))] - #[unstable(feature = "unix_socket_ancillary_data", issue = "76915")] - pub fn passcred(&self) -> io::Result { - self.0.passcred() - } - /// Set the id of the socket for network filtering purpose /// #[cfg_attr( @@ -766,3 +708,10 @@ impl From for UnixStream { unsafe { Self::from_raw_fd(owned.into_raw_fd()) } } } + +impl AsInner for UnixStream { + #[inline] + fn as_inner(&self) -> &Socket { + &self.0 + } +} diff --git a/library/std/src/os/unix/net/tests.rs b/library/std/src/os/unix/net/tests.rs index 6a6af9efd78ed..b413f3caa39bc 100644 --- a/library/std/src/os/unix/net/tests.rs +++ b/library/std/src/os/unix/net/tests.rs @@ -13,6 +13,9 @@ use crate::os::android::net::SocketAddrExt; #[cfg(target_os = "linux")] use crate::os::linux::net::SocketAddrExt; +#[cfg(any(target_os = "android", target_os = "linux"))] +use crate::os::linux::net::UnixSocketExt; + macro_rules! or_panic { ($e:expr) => { match $e { diff --git a/library/std/src/sys/pal/unix/net.rs b/library/std/src/sys/pal/unix/net.rs index 60546a3de4509..09750b6ffc897 100644 --- a/library/std/src/sys/pal/unix/net.rs +++ b/library/std/src/sys/pal/unix/net.rs @@ -465,25 +465,31 @@ impl Socket { } #[cfg(target_os = "netbsd")] - pub fn set_passcred(&self, passcred: bool) -> io::Result<()> { - setsockopt(self, 0 as libc::c_int, libc::LOCAL_CREDS, passcred as libc::c_int) + pub fn set_local_creds(&self, local_creds: bool) -> io::Result<()> { + setsockopt(self, 0 as libc::c_int, libc::LOCAL_CREDS, local_creds as libc::c_int) } #[cfg(target_os = "netbsd")] - pub fn passcred(&self) -> io::Result { - let passcred: libc::c_int = getsockopt(self, 0 as libc::c_int, libc::LOCAL_CREDS)?; - Ok(passcred != 0) + pub fn local_creds(&self) -> io::Result { + let local_creds: libc::c_int = getsockopt(self, 0 as libc::c_int, libc::LOCAL_CREDS)?; + Ok(local_creds != 0) } #[cfg(target_os = "freebsd")] - pub fn set_passcred(&self, passcred: bool) -> io::Result<()> { - setsockopt(self, libc::AF_LOCAL, libc::LOCAL_CREDS_PERSISTENT, passcred as libc::c_int) + pub fn set_local_creds_persistent(&self, local_creds_persistent: bool) -> io::Result<()> { + setsockopt( + self, + libc::AF_LOCAL, + libc::LOCAL_CREDS_PERSISTENT, + local_creds_persistent as libc::c_int, + ) } #[cfg(target_os = "freebsd")] - pub fn passcred(&self) -> io::Result { - let passcred: libc::c_int = getsockopt(self, libc::AF_LOCAL, libc::LOCAL_CREDS_PERSISTENT)?; - Ok(passcred != 0) + pub fn local_creds_persistent(&self) -> io::Result { + let local_creds_persistent: libc::c_int = + getsockopt(self, libc::AF_LOCAL, libc::LOCAL_CREDS_PERSISTENT)?; + Ok(local_creds_persistent != 0) } #[cfg(not(any(target_os = "solaris", target_os = "illumos", target_os = "vita")))] From 71a697327bd64bdebcdf81b10c3dd03dfe33cb68 Mon Sep 17 00:00:00 2001 From: carschandler <92899389+carschandler@users.noreply.github.com> Date: Mon, 5 Feb 2024 15:23:05 -0600 Subject: [PATCH 04/15] Update E0716.md for clarity When reading through this, I got slightly hung up thinking the `let` it was referring to was the `let tmp` on line 25, which was confusing considering the comment states that the temporary is freed at the end of the block. I think adding this clarification could potentially help some beginners like myself without being overly verbose. --- compiler/rustc_error_codes/src/error_codes/E0716.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/compiler/rustc_error_codes/src/error_codes/E0716.md b/compiler/rustc_error_codes/src/error_codes/E0716.md index c3546cd744f7b..b50c8b8e7ca23 100644 --- a/compiler/rustc_error_codes/src/error_codes/E0716.md +++ b/compiler/rustc_error_codes/src/error_codes/E0716.md @@ -30,8 +30,9 @@ let q = p; Whenever a temporary is created, it is automatically dropped (freed) according to fixed rules. Ordinarily, the temporary is dropped at the end of the enclosing -statement -- in this case, after the `let`. This is illustrated in the example -above by showing that `tmp` would be freed as we exit the block. +statement -- in this case, after the outer `let` that assigns to `p`. This is +illustrated in the example above by showing that `tmp` would be freed as we exit +the block. To fix this problem, you need to create a local variable to store the value in rather than relying on a temporary. For example, you might change the original From b35376512031a24e236f66cbca8712ecea5e721c Mon Sep 17 00:00:00 2001 From: Marc Schoolderman Date: Thu, 22 Feb 2024 11:42:39 +0100 Subject: [PATCH 05/15] remove potentially misleading sentence about libc::access --- library/std/src/fs.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/library/std/src/fs.rs b/library/std/src/fs.rs index e506b321c2518..d5387e2e2b448 100644 --- a/library/std/src/fs.rs +++ b/library/std/src/fs.rs @@ -1409,8 +1409,7 @@ impl Permissions { /// Therefore the return value of this function cannot be relied upon /// to predict whether attempts to read or write the file will actually succeed. /// The [`PermissionsExt`] trait gives direct access to the permission bits but - /// also does not read ACLs. If you need to accurately know whether or not a file - /// is writable use the `access()` function from libc. + /// also does not read ACLs. /// /// [`PermissionsExt`]: crate::os::unix::fs::PermissionsExt /// From c0ce0f3c3feede6f6f98ecb4d0c42668dd4f5afe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E8=AE=B8=E6=9D=B0=E5=8F=8B=20Jieyou=20Xu=20=28Joe=29?= Date: Wed, 28 Feb 2024 14:13:42 +0000 Subject: [PATCH 06/15] Display short types for unimplemented trait --- .../error_reporting/on_unimplemented.rs | 7 ++++++- .../src/traits/error_reporting/suggestions.rs | 3 ++- .../error_reporting/type_err_ctxt_ext.rs | 1 + .../ui/traits/on_unimplemented_long_types.rs | 17 +++++++++++++++ .../traits/on_unimplemented_long_types.stderr | 21 +++++++++++++++++++ 5 files changed, 47 insertions(+), 2 deletions(-) create mode 100644 tests/ui/traits/on_unimplemented_long_types.rs create mode 100644 tests/ui/traits/on_unimplemented_long_types.stderr diff --git a/compiler/rustc_trait_selection/src/traits/error_reporting/on_unimplemented.rs b/compiler/rustc_trait_selection/src/traits/error_reporting/on_unimplemented.rs index 4ba2da95fb324..5c9b1fd93dfe5 100644 --- a/compiler/rustc_trait_selection/src/traits/error_reporting/on_unimplemented.rs +++ b/compiler/rustc_trait_selection/src/traits/error_reporting/on_unimplemented.rs @@ -669,6 +669,7 @@ impl<'tcx> OnUnimplementedDirective { options.iter().filter_map(|(k, v)| v.clone().map(|v| (*k, v))).collect(); for command in self.subcommands.iter().chain(Some(self)).rev() { + debug!(?command); if let Some(ref condition) = command.condition && !attr::eval_condition(condition, &tcx.sess, Some(tcx.features()), &mut |cfg| { let value = cfg.value.map(|v| { @@ -824,7 +825,11 @@ impl<'tcx> OnUnimplementedFormatString { .filter_map(|param| { let value = match param.kind { GenericParamDefKind::Type { .. } | GenericParamDefKind::Const { .. } => { - trait_ref.args[param.index as usize].to_string() + if let Some(ty) = trait_ref.args[param.index as usize].as_type() { + tcx.short_ty_string(ty, &mut None) + } else { + trait_ref.args[param.index as usize].to_string() + } } GenericParamDefKind::Lifetime => return None, }; diff --git a/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs b/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs index 85f6da0d6cc82..8248963c9cc3c 100644 --- a/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs +++ b/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs @@ -3507,7 +3507,8 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> { } ObligationCauseCode::OpaqueReturnType(expr_info) => { if let Some((expr_ty, expr_span)) = expr_info { - let expr_ty = with_forced_trimmed_paths!(self.ty_to_string(expr_ty)); + let expr_ty = + with_forced_trimmed_paths!(self.tcx.short_ty_string(expr_ty, &mut None)); err.span_label( expr_span, with_forced_trimmed_paths!(format!( diff --git a/compiler/rustc_trait_selection/src/traits/error_reporting/type_err_ctxt_ext.rs b/compiler/rustc_trait_selection/src/traits/error_reporting/type_err_ctxt_ext.rs index 2b74b15ec9faa..0223856f24c12 100644 --- a/compiler/rustc_trait_selection/src/traits/error_reporting/type_err_ctxt_ext.rs +++ b/compiler/rustc_trait_selection/src/traits/error_reporting/type_err_ctxt_ext.rs @@ -389,6 +389,7 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> { kind: _, } = *obligation.cause.code() { + debug!("ObligationCauseCode::CompareImplItemObligation"); return self.report_extra_impl_obligation( span, impl_item_def_id, diff --git a/tests/ui/traits/on_unimplemented_long_types.rs b/tests/ui/traits/on_unimplemented_long_types.rs new file mode 100644 index 0000000000000..60c3327902e11 --- /dev/null +++ b/tests/ui/traits/on_unimplemented_long_types.rs @@ -0,0 +1,17 @@ +//@ compile-flags: --diagnostic-width=60 -Z write-long-types-to-disk=yes +//@ normalize-stderr-test: "long-type-\d+" -> "long-type-hash" + +pub fn foo() -> impl std::fmt::Display { + //~^ ERROR doesn't implement `std::fmt::Display` + Some(Some(Some(Some(Some(Some(Some(Some(Some(Some(Some( + Some(Some(Some(Some(Some(Some(Some(Some(Some(Some(Some( + Some(Some(Some(Some(Some(Some(Some(Some(Some(Some(Some( + Some(Some(Some(Some(Some(Some(Some(Some(Some(Some(Some( + Some(Some(Some(Some(Some(Some(Some(Some(())))))))), + ))))))))))), + ))))))))))), + ))))))))))), + ))))))))))) +} + +fn main() {} diff --git a/tests/ui/traits/on_unimplemented_long_types.stderr b/tests/ui/traits/on_unimplemented_long_types.stderr new file mode 100644 index 0000000000000..93fd19ea6fc2d --- /dev/null +++ b/tests/ui/traits/on_unimplemented_long_types.stderr @@ -0,0 +1,21 @@ +error[E0277]: `Option>>` doesn't implement `std::fmt::Display` + --> $DIR/on_unimplemented_long_types.rs:4:17 + | +LL | pub fn foo() -> impl std::fmt::Display { + | ^^^^^^^^^^^^^^^^^^^^^^ `Option>>` cannot be formatted with the default formatter +LL | +LL | / Some(Some(Some(Some(Some(Some(Some(Some(Some(S... +LL | | Some(Some(Some(Some(Some(Some(Some(Some(So... +LL | | Some(Some(Some(Some(Some(Some(Some(Som... +LL | | Some(Some(Some(Some(Some(Some(Some... +... | +LL | | ))))))))))), +LL | | ))))))))))) + | |_______________- return type was inferred to be `Option>>` here + | + = help: the trait `std::fmt::Display` is not implemented for `Option>>` + = note: in format strings you may be able to use `{:?}` (or {:#?} for pretty-print) instead + +error: aborting due to 1 previous error + +For more information about this error, try `rustc --explain E0277`. From a1dbb61c0902a6856f052161d7fc7ee7850a951b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E8=AE=B8=E6=9D=B0=E5=8F=8B=20Jieyou=20Xu=20=28Joe=29?= Date: Wed, 28 Feb 2024 19:54:05 +0000 Subject: [PATCH 07/15] Unify long type name file and note in note_obligation_cause_code --- .../error_reporting/on_unimplemented.rs | 28 ++++++--- .../src/traits/error_reporting/suggestions.rs | 62 +++++++------------ .../error_reporting/type_err_ctxt_ext.rs | 2 +- .../traits/on_unimplemented_long_types.stderr | 2 + 4 files changed, 46 insertions(+), 48 deletions(-) diff --git a/compiler/rustc_trait_selection/src/traits/error_reporting/on_unimplemented.rs b/compiler/rustc_trait_selection/src/traits/error_reporting/on_unimplemented.rs index 5c9b1fd93dfe5..6773372397206 100644 --- a/compiler/rustc_trait_selection/src/traits/error_reporting/on_unimplemented.rs +++ b/compiler/rustc_trait_selection/src/traits/error_reporting/on_unimplemented.rs @@ -16,6 +16,7 @@ use rustc_session::lint::builtin::UNKNOWN_OR_MALFORMED_DIAGNOSTIC_ATTRIBUTES; use rustc_span::symbol::{kw, sym, Symbol}; use rustc_span::Span; use std::iter; +use std::path::PathBuf; use crate::errors::{ EmptyOnClauseInOnUnimplemented, InvalidOnClauseInOnUnimplemented, NoValueInOnUnimplemented, @@ -111,6 +112,8 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> { trait_ref: ty::PolyTraitRef<'tcx>, obligation: &PredicateObligation<'tcx>, ) -> OnUnimplementedNote { + let mut long_ty_file = None; + let (def_id, args) = self .impl_similar_to(trait_ref, obligation) .unwrap_or_else(|| (trait_ref.def_id(), trait_ref.skip_binder().args)); @@ -265,7 +268,7 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> { })); if let Ok(Some(command)) = OnUnimplementedDirective::of_item(self.tcx, def_id) { - command.evaluate(self.tcx, trait_ref, &flags) + command.evaluate(self.tcx, trait_ref, &flags, &mut long_ty_file) } else { OnUnimplementedNote::default() } @@ -657,6 +660,7 @@ impl<'tcx> OnUnimplementedDirective { tcx: TyCtxt<'tcx>, trait_ref: ty::TraitRef<'tcx>, options: &[(Symbol, Option)], + long_ty_file: &mut Option, ) -> OnUnimplementedNote { let mut message = None; let mut label = None; @@ -681,7 +685,12 @@ impl<'tcx> OnUnimplementedDirective { span: cfg.span, is_diagnostic_namespace_variant: false } - .format(tcx, trait_ref, &options_map) + .format( + tcx, + trait_ref, + &options_map, + long_ty_file + ) ) }); @@ -710,10 +719,14 @@ impl<'tcx> OnUnimplementedDirective { } OnUnimplementedNote { - label: label.map(|l| l.format(tcx, trait_ref, &options_map)), - message: message.map(|m| m.format(tcx, trait_ref, &options_map)), - notes: notes.into_iter().map(|n| n.format(tcx, trait_ref, &options_map)).collect(), - parent_label: parent_label.map(|e_s| e_s.format(tcx, trait_ref, &options_map)), + label: label.map(|l| l.format(tcx, trait_ref, &options_map, long_ty_file)), + message: message.map(|m| m.format(tcx, trait_ref, &options_map, long_ty_file)), + notes: notes + .into_iter() + .map(|n| n.format(tcx, trait_ref, &options_map, long_ty_file)) + .collect(), + parent_label: parent_label + .map(|e_s| e_s.format(tcx, trait_ref, &options_map, long_ty_file)), append_const_msg, } } @@ -815,6 +828,7 @@ impl<'tcx> OnUnimplementedFormatString { tcx: TyCtxt<'tcx>, trait_ref: ty::TraitRef<'tcx>, options: &FxHashMap, + long_ty_file: &mut Option, ) -> String { let name = tcx.item_name(trait_ref.def_id); let trait_str = tcx.def_path_str(trait_ref.def_id); @@ -826,7 +840,7 @@ impl<'tcx> OnUnimplementedFormatString { let value = match param.kind { GenericParamDefKind::Type { .. } | GenericParamDefKind::Const { .. } => { if let Some(ty) = trait_ref.args[param.index as usize].as_type() { - tcx.short_ty_string(ty, &mut None) + tcx.short_ty_string(ty, long_ty_file) } else { trait_ref.args[param.index as usize].to_string() } diff --git a/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs b/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs index 8248963c9cc3c..bba9a9d01f623 100644 --- a/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs +++ b/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs @@ -2676,6 +2676,8 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> { ) where T: ToPredicate<'tcx>, { + let mut long_ty_file = None; + let tcx = self.tcx; let predicate = predicate.to_predicate(tcx); match *cause_code { @@ -2858,21 +2860,13 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> { } } ObligationCauseCode::Coercion { source, target } => { - let mut file = None; - let source = tcx.short_ty_string(self.resolve_vars_if_possible(source), &mut file); - let target = tcx.short_ty_string(self.resolve_vars_if_possible(target), &mut file); + let source = + tcx.short_ty_string(self.resolve_vars_if_possible(source), &mut long_ty_file); + let target = + tcx.short_ty_string(self.resolve_vars_if_possible(target), &mut long_ty_file); err.note(with_forced_trimmed_paths!(format!( "required for the cast from `{source}` to `{target}`", ))); - if let Some(file) = file { - err.note(format!( - "the full name for the type has been written to '{}'", - file.display(), - )); - err.note( - "consider using `--verbose` to print the full type name to the console", - ); - } } ObligationCauseCode::RepeatElementCopy { is_constable, @@ -3175,8 +3169,7 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> { // Don't print the tuple of capture types 'print: { if !is_upvar_tys_infer_tuple { - let mut file = None; - let ty_str = tcx.short_ty_string(ty, &mut file); + let ty_str = tcx.short_ty_string(ty, &mut long_ty_file); let msg = format!("required because it appears within the type `{ty_str}`"); match ty.kind() { ty::Adt(def, _) => match tcx.opt_item_ident(def.did()) { @@ -3274,9 +3267,8 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> { let mut parent_trait_pred = self.resolve_vars_if_possible(data.derived.parent_trait_pred); let parent_def_id = parent_trait_pred.def_id(); - let mut file = None; - let self_ty_str = - tcx.short_ty_string(parent_trait_pred.skip_binder().self_ty(), &mut file); + let self_ty_str = tcx + .short_ty_string(parent_trait_pred.skip_binder().self_ty(), &mut long_ty_file); let trait_name = parent_trait_pred.print_modifiers_and_trait_path().to_string(); let msg = format!("required for `{self_ty_str}` to implement `{trait_name}`"); let mut is_auto_trait = false; @@ -3334,15 +3326,6 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> { } }; - if let Some(file) = file { - err.note(format!( - "the full type name has been written to '{}'", - file.display(), - )); - err.note( - "consider using `--verbose` to print the full type name to the console", - ); - } let mut parent_predicate = parent_trait_pred; let mut data = &data.derived; let mut count = 0; @@ -3383,22 +3366,14 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> { count, pluralize!(count) )); - let mut file = None; - let self_ty = - tcx.short_ty_string(parent_trait_pred.skip_binder().self_ty(), &mut file); + let self_ty = tcx.short_ty_string( + parent_trait_pred.skip_binder().self_ty(), + &mut long_ty_file, + ); err.note(format!( "required for `{self_ty}` to implement `{}`", parent_trait_pred.print_modifiers_and_trait_path() )); - if let Some(file) = file { - err.note(format!( - "the full type name has been written to '{}'", - file.display(), - )); - err.note( - "consider using `--verbose` to print the full type name to the console", - ); - } } // #74711: avoid a stack overflow ensure_sufficient_stack(|| { @@ -3507,8 +3482,7 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> { } ObligationCauseCode::OpaqueReturnType(expr_info) => { if let Some((expr_ty, expr_span)) = expr_info { - let expr_ty = - with_forced_trimmed_paths!(self.tcx.short_ty_string(expr_ty, &mut None)); + let expr_ty = self.tcx.short_ty_string(expr_ty, &mut long_ty_file); err.span_label( expr_span, with_forced_trimmed_paths!(format!( @@ -3518,6 +3492,14 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> { } } } + + if let Some(file) = long_ty_file { + err.note(format!( + "the full name for the type has been written to '{}'", + file.display(), + )); + err.note("consider using `--verbose` to print the full type name to the console"); + } } #[instrument( diff --git a/compiler/rustc_trait_selection/src/traits/error_reporting/type_err_ctxt_ext.rs b/compiler/rustc_trait_selection/src/traits/error_reporting/type_err_ctxt_ext.rs index 0223856f24c12..0c66ce5b46cb7 100644 --- a/compiler/rustc_trait_selection/src/traits/error_reporting/type_err_ctxt_ext.rs +++ b/compiler/rustc_trait_selection/src/traits/error_reporting/type_err_ctxt_ext.rs @@ -440,7 +440,7 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> { ) }) .unwrap_or_default(); - let file_note = file.map(|file| format!( + let file_note = file.as_ref().map(|file| format!( "the full trait has been written to '{}'", file.display(), )); diff --git a/tests/ui/traits/on_unimplemented_long_types.stderr b/tests/ui/traits/on_unimplemented_long_types.stderr index 93fd19ea6fc2d..4c8210f073bd6 100644 --- a/tests/ui/traits/on_unimplemented_long_types.stderr +++ b/tests/ui/traits/on_unimplemented_long_types.stderr @@ -15,6 +15,8 @@ LL | | ))))))))))) | = help: the trait `std::fmt::Display` is not implemented for `Option>>` = note: in format strings you may be able to use `{:?}` (or {:#?} for pretty-print) instead + = note: the full name for the type has been written to '$TEST_BUILD_DIR/traits/on_unimplemented_long_types/on_unimplemented_long_types.long-type-hash.txt' + = note: consider using `--verbose` to print the full type name to the console error: aborting due to 1 previous error From d89c2c569aaf509c97469cf7433d26f7f87b40bf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Thu, 29 Feb 2024 16:23:40 +0000 Subject: [PATCH 08/15] Small clean up of E0277 message logic --- .../src/traits/error_reporting/suggestions.rs | 24 +++++++------------ 1 file changed, 9 insertions(+), 15 deletions(-) diff --git a/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs b/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs index bca7b2327495c..e82e94993d249 100644 --- a/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs +++ b/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs @@ -4769,21 +4769,15 @@ pub(super) fn get_explanation_based_on_obligation<'tcx>( } else { String::new() }; - match ty_desc { - Some(desc) => format!( - "{}the trait `{}` is not implemented for {} `{}`{post}", - pre_message, - trait_predicate.print_modifiers_and_trait_path(), - desc, - tcx.short_ty_string(trait_ref.skip_binder().self_ty(), &mut None), - ), - None => format!( - "{}the trait `{}` is not implemented for `{}`{post}", - pre_message, - trait_predicate.print_modifiers_and_trait_path(), - tcx.short_ty_string(trait_ref.skip_binder().self_ty(), &mut None), - ), - } + let desc = match ty_desc { + Some(desc) => format!(" {desc}"), + None => String::new(), + }; + format!( + "{pre_message}the trait `{}` is not implemented for{desc} `{}`{post}", + trait_predicate.print_modifiers_and_trait_path(), + tcx.short_ty_string(trait_ref.skip_binder().self_ty(), &mut None), + ) } } From 69f2c9c101259307c9d278744d32310d7a12dbf8 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Mon, 27 Nov 2023 19:23:20 +1100 Subject: [PATCH 09/15] Move `gather_comments`. To the module where it is used, so it doesn't have to be `pub`. --- Cargo.lock | 1 + compiler/rustc_ast/src/util/comments.rs | 126 +----------------- compiler/rustc_ast_pretty/Cargo.toml | 1 + compiler/rustc_ast_pretty/src/pprust/state.rs | 125 ++++++++++++++++- 4 files changed, 126 insertions(+), 127 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 0b74d71fd8929..7be392a8435dc 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3487,6 +3487,7 @@ version = "0.0.0" dependencies = [ "itertools 0.11.0", "rustc_ast", + "rustc_lexer", "rustc_span", "thin-vec", ] diff --git a/compiler/rustc_ast/src/util/comments.rs b/compiler/rustc_ast/src/util/comments.rs index bdf5143b0f706..cbc1afc6bf1e8 100644 --- a/compiler/rustc_ast/src/util/comments.rs +++ b/compiler/rustc_ast/src/util/comments.rs @@ -1,6 +1,5 @@ use crate::token::CommentKind; -use rustc_span::source_map::SourceMap; -use rustc_span::{BytePos, CharPos, FileName, Pos, Symbol}; +use rustc_span::{BytePos, Symbol}; #[cfg(test)] mod tests; @@ -131,126 +130,3 @@ pub fn beautify_doc_string(data: Symbol, kind: CommentKind) -> Symbol { } data } - -/// Returns `None` if the first `col` chars of `s` contain a non-whitespace char. -/// Otherwise returns `Some(k)` where `k` is first char offset after that leading -/// whitespace. Note that `k` may be outside bounds of `s`. -fn all_whitespace(s: &str, col: CharPos) -> Option { - let mut idx = 0; - for (i, ch) in s.char_indices().take(col.to_usize()) { - if !ch.is_whitespace() { - return None; - } - idx = i + ch.len_utf8(); - } - Some(idx) -} - -fn trim_whitespace_prefix(s: &str, col: CharPos) -> &str { - let len = s.len(); - match all_whitespace(s, col) { - Some(col) => { - if col < len { - &s[col..] - } else { - "" - } - } - None => s, - } -} - -fn split_block_comment_into_lines(text: &str, col: CharPos) -> Vec { - let mut res: Vec = vec![]; - let mut lines = text.lines(); - // just push the first line - res.extend(lines.next().map(|it| it.to_string())); - // for other lines, strip common whitespace prefix - for line in lines { - res.push(trim_whitespace_prefix(line, col).to_string()) - } - res -} - -// it appears this function is called only from pprust... that's -// probably not a good thing. -pub fn gather_comments(sm: &SourceMap, path: FileName, src: String) -> Vec { - let sm = SourceMap::new(sm.path_mapping().clone()); - let source_file = sm.new_source_file(path, src); - let text = (*source_file.src.as_ref().unwrap()).clone(); - - let text: &str = text.as_str(); - let start_bpos = source_file.start_pos; - let mut pos = 0; - let mut comments: Vec = Vec::new(); - let mut code_to_the_left = false; - - if let Some(shebang_len) = rustc_lexer::strip_shebang(text) { - comments.push(Comment { - style: CommentStyle::Isolated, - lines: vec![text[..shebang_len].to_string()], - pos: start_bpos, - }); - pos += shebang_len; - } - - for token in rustc_lexer::tokenize(&text[pos..]) { - let token_text = &text[pos..pos + token.len as usize]; - match token.kind { - rustc_lexer::TokenKind::Whitespace => { - if let Some(mut idx) = token_text.find('\n') { - code_to_the_left = false; - while let Some(next_newline) = &token_text[idx + 1..].find('\n') { - idx += 1 + next_newline; - comments.push(Comment { - style: CommentStyle::BlankLine, - lines: vec![], - pos: start_bpos + BytePos((pos + idx) as u32), - }); - } - } - } - rustc_lexer::TokenKind::BlockComment { doc_style, .. } => { - if doc_style.is_none() { - let code_to_the_right = !matches!( - text[pos + token.len as usize..].chars().next(), - Some('\r' | '\n') - ); - let style = match (code_to_the_left, code_to_the_right) { - (_, true) => CommentStyle::Mixed, - (false, false) => CommentStyle::Isolated, - (true, false) => CommentStyle::Trailing, - }; - - // Count the number of chars since the start of the line by rescanning. - let pos_in_file = start_bpos + BytePos(pos as u32); - let line_begin_in_file = source_file.line_begin_pos(pos_in_file); - let line_begin_pos = (line_begin_in_file - start_bpos).to_usize(); - let col = CharPos(text[line_begin_pos..pos].chars().count()); - - let lines = split_block_comment_into_lines(token_text, col); - comments.push(Comment { style, lines, pos: pos_in_file }) - } - } - rustc_lexer::TokenKind::LineComment { doc_style } => { - if doc_style.is_none() { - comments.push(Comment { - style: if code_to_the_left { - CommentStyle::Trailing - } else { - CommentStyle::Isolated - }, - lines: vec![token_text.to_string()], - pos: start_bpos + BytePos(pos as u32), - }) - } - } - _ => { - code_to_the_left = true; - } - } - pos += token.len as usize; - } - - comments -} diff --git a/compiler/rustc_ast_pretty/Cargo.toml b/compiler/rustc_ast_pretty/Cargo.toml index 12a08f06558ba..b38a2915a43df 100644 --- a/compiler/rustc_ast_pretty/Cargo.toml +++ b/compiler/rustc_ast_pretty/Cargo.toml @@ -7,6 +7,7 @@ edition = "2021" # tidy-alphabetical-start itertools = "0.11" rustc_ast = { path = "../rustc_ast" } +rustc_lexer = { path = "../rustc_lexer" } rustc_span = { path = "../rustc_span" } thin-vec = "0.2.12" # tidy-alphabetical-end diff --git a/compiler/rustc_ast_pretty/src/pprust/state.rs b/compiler/rustc_ast_pretty/src/pprust/state.rs index 6e1974f48b26b..6119c6c84f8b7 100644 --- a/compiler/rustc_ast_pretty/src/pprust/state.rs +++ b/compiler/rustc_ast_pretty/src/pprust/state.rs @@ -14,7 +14,7 @@ use rustc_ast::ptr::P; use rustc_ast::token::{self, BinOpToken, CommentKind, Delimiter, Nonterminal, Token, TokenKind}; use rustc_ast::tokenstream::{Spacing, TokenStream, TokenTree}; use rustc_ast::util::classify; -use rustc_ast::util::comments::{gather_comments, Comment, CommentStyle}; +use rustc_ast::util::comments::{Comment, CommentStyle}; use rustc_ast::util::parser; use rustc_ast::{self as ast, AttrArgs, AttrArgsEq, BlockCheckMode, PatKind}; use rustc_ast::{attr, BindingAnnotation, ByRef, DelimArgs, RangeEnd, RangeSyntax, Term}; @@ -24,7 +24,7 @@ use rustc_ast::{InlineAsmOptions, InlineAsmTemplatePiece}; use rustc_span::edition::Edition; use rustc_span::source_map::{SourceMap, Spanned}; use rustc_span::symbol::{kw, sym, Ident, IdentPrinter, Symbol}; -use rustc_span::{BytePos, FileName, Span, DUMMY_SP}; +use rustc_span::{BytePos, CharPos, FileName, Pos, Span, DUMMY_SP}; use std::borrow::Cow; use thin_vec::ThinVec; @@ -59,6 +59,127 @@ pub struct Comments<'a> { current: usize, } +/// Returns `None` if the first `col` chars of `s` contain a non-whitespace char. +/// Otherwise returns `Some(k)` where `k` is first char offset after that leading +/// whitespace. Note that `k` may be outside bounds of `s`. +fn all_whitespace(s: &str, col: CharPos) -> Option { + let mut idx = 0; + for (i, ch) in s.char_indices().take(col.to_usize()) { + if !ch.is_whitespace() { + return None; + } + idx = i + ch.len_utf8(); + } + Some(idx) +} + +fn trim_whitespace_prefix(s: &str, col: CharPos) -> &str { + let len = s.len(); + match all_whitespace(s, col) { + Some(col) => { + if col < len { + &s[col..] + } else { + "" + } + } + None => s, + } +} + +fn split_block_comment_into_lines(text: &str, col: CharPos) -> Vec { + let mut res: Vec = vec![]; + let mut lines = text.lines(); + // just push the first line + res.extend(lines.next().map(|it| it.to_string())); + // for other lines, strip common whitespace prefix + for line in lines { + res.push(trim_whitespace_prefix(line, col).to_string()) + } + res +} + +fn gather_comments(sm: &SourceMap, path: FileName, src: String) -> Vec { + let sm = SourceMap::new(sm.path_mapping().clone()); + let source_file = sm.new_source_file(path, src); + let text = (*source_file.src.as_ref().unwrap()).clone(); + + let text: &str = text.as_str(); + let start_bpos = source_file.start_pos; + let mut pos = 0; + let mut comments: Vec = Vec::new(); + let mut code_to_the_left = false; + + if let Some(shebang_len) = rustc_lexer::strip_shebang(text) { + comments.push(Comment { + style: CommentStyle::Isolated, + lines: vec![text[..shebang_len].to_string()], + pos: start_bpos, + }); + pos += shebang_len; + } + + for token in rustc_lexer::tokenize(&text[pos..]) { + let token_text = &text[pos..pos + token.len as usize]; + match token.kind { + rustc_lexer::TokenKind::Whitespace => { + if let Some(mut idx) = token_text.find('\n') { + code_to_the_left = false; + while let Some(next_newline) = &token_text[idx + 1..].find('\n') { + idx += 1 + next_newline; + comments.push(Comment { + style: CommentStyle::BlankLine, + lines: vec![], + pos: start_bpos + BytePos((pos + idx) as u32), + }); + } + } + } + rustc_lexer::TokenKind::BlockComment { doc_style, .. } => { + if doc_style.is_none() { + let code_to_the_right = !matches!( + text[pos + token.len as usize..].chars().next(), + Some('\r' | '\n') + ); + let style = match (code_to_the_left, code_to_the_right) { + (_, true) => CommentStyle::Mixed, + (false, false) => CommentStyle::Isolated, + (true, false) => CommentStyle::Trailing, + }; + + // Count the number of chars since the start of the line by rescanning. + let pos_in_file = start_bpos + BytePos(pos as u32); + let line_begin_in_file = source_file.line_begin_pos(pos_in_file); + let line_begin_pos = (line_begin_in_file - start_bpos).to_usize(); + let col = CharPos(text[line_begin_pos..pos].chars().count()); + + let lines = split_block_comment_into_lines(token_text, col); + comments.push(Comment { style, lines, pos: pos_in_file }) + } + } + rustc_lexer::TokenKind::LineComment { doc_style } => { + if doc_style.is_none() { + comments.push(Comment { + style: if code_to_the_left { + CommentStyle::Trailing + } else { + CommentStyle::Isolated + }, + lines: vec![token_text.to_string()], + pos: start_bpos + BytePos(pos as u32), + }) + } + } + _ => { + code_to_the_left = true; + } + } + pos += token.len as usize; + } + + comments +} + impl<'a> Comments<'a> { pub fn new(sm: &'a SourceMap, filename: FileName, input: String) -> Comments<'a> { let comments = gather_comments(sm, filename, input); From 392159b5618192af5761788d2c7e8256b331ce95 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Fri, 1 Mar 2024 16:06:29 +1100 Subject: [PATCH 10/15] Move `HandleStore` into `server.rs`. This just moves the server-relevant parts of handles into `server.rs`. It introduces a new higher-order macro `with_api_handle_types` to avoid some duplication. This fixes two `FIXME` comments, and makes things clearer, by not having server code in `client.rs`. --- library/proc_macro/src/bridge/client.rs | 85 ++----------------------- library/proc_macro/src/bridge/mod.rs | 17 +++++ library/proc_macro/src/bridge/server.rs | 75 +++++++++++++++++++++- library/proc_macro/src/bridge/symbol.rs | 8 +-- 4 files changed, 99 insertions(+), 86 deletions(-) diff --git a/library/proc_macro/src/bridge/client.rs b/library/proc_macro/src/bridge/client.rs index 52b9ef470dccb..b54f2db88ffc9 100644 --- a/library/proc_macro/src/bridge/client.rs +++ b/library/proc_macro/src/bridge/client.rs @@ -5,16 +5,16 @@ use super::*; use std::marker::PhantomData; use std::sync::atomic::AtomicU32; -macro_rules! define_handles { +macro_rules! define_client_handles { ( 'owned: $($oty:ident,)* 'interned: $($ity:ident,)* ) => { #[repr(C)] #[allow(non_snake_case)] - pub struct HandleCounters { - $($oty: AtomicU32,)* - $($ity: AtomicU32,)* + pub(super) struct HandleCounters { + $(pub(super) $oty: AtomicU32,)* + $(pub(super) $ity: AtomicU32,)* } impl HandleCounters { @@ -29,22 +29,6 @@ macro_rules! define_handles { } } - // FIXME(eddyb) generate the definition of `HandleStore` in `server.rs`. - #[allow(non_snake_case)] - pub(super) struct HandleStore { - $($oty: handle::OwnedStore,)* - $($ity: handle::InternedStore,)* - } - - impl HandleStore { - pub(super) fn new(handle_counters: &'static HandleCounters) -> Self { - HandleStore { - $($oty: handle::OwnedStore::new(&handle_counters.$oty),)* - $($ity: handle::InternedStore::new(&handle_counters.$ity),)* - } - } - } - $( pub(crate) struct $oty { handle: handle::Handle, @@ -72,53 +56,18 @@ macro_rules! define_handles { } } - impl DecodeMut<'_, '_, HandleStore>> - for Marked - { - fn decode(r: &mut Reader<'_>, s: &mut HandleStore>) -> Self { - s.$oty.take(handle::Handle::decode(r, &mut ())) - } - } - impl Encode for &$oty { fn encode(self, w: &mut Writer, s: &mut S) { self.handle.encode(w, s); } } - impl<'s, S: server::Types> Decode<'_, 's, HandleStore>> - for &'s Marked - { - fn decode(r: &mut Reader<'_>, s: &'s HandleStore>) -> Self { - &s.$oty[handle::Handle::decode(r, &mut ())] - } - } - impl Encode for &mut $oty { fn encode(self, w: &mut Writer, s: &mut S) { self.handle.encode(w, s); } } - impl<'s, S: server::Types> DecodeMut<'_, 's, HandleStore>> - for &'s mut Marked - { - fn decode( - r: &mut Reader<'_>, - s: &'s mut HandleStore> - ) -> Self { - &mut s.$oty[handle::Handle::decode(r, &mut ())] - } - } - - impl Encode>> - for Marked - { - fn encode(self, w: &mut Writer, s: &mut HandleStore>) { - s.$oty.alloc(self).encode(w, s); - } - } - impl DecodeMut<'_, '_, S> for $oty { fn decode(r: &mut Reader<'_>, s: &mut S) -> Self { $oty { @@ -145,22 +94,6 @@ macro_rules! define_handles { } } - impl DecodeMut<'_, '_, HandleStore>> - for Marked - { - fn decode(r: &mut Reader<'_>, s: &mut HandleStore>) -> Self { - s.$ity.copy(handle::Handle::decode(r, &mut ())) - } - } - - impl Encode>> - for Marked - { - fn encode(self, w: &mut Writer, s: &mut HandleStore>) { - s.$ity.alloc(self).encode(w, s); - } - } - impl DecodeMut<'_, '_, S> for $ity { fn decode(r: &mut Reader<'_>, s: &mut S) -> Self { $ity { @@ -172,15 +105,7 @@ macro_rules! define_handles { )* } } -define_handles! { - 'owned: - FreeFunctions, - TokenStream, - SourceFile, - - 'interned: - Span, -} +with_api_handle_types!(define_client_handles); // FIXME(eddyb) generate these impls by pattern-matching on the // names of methods - also could use the presence of `fn drop` diff --git a/library/proc_macro/src/bridge/mod.rs b/library/proc_macro/src/bridge/mod.rs index 9b337f23867f4..67c72cf98d405 100644 --- a/library/proc_macro/src/bridge/mod.rs +++ b/library/proc_macro/src/bridge/mod.rs @@ -113,6 +113,23 @@ macro_rules! with_api { }; } +// Similar to `with_api`, but only lists the types requiring handles, and they +// are divided into the two storage categories. +macro_rules! with_api_handle_types { + ($m:ident) => { + $m! { + 'owned: + FreeFunctions, + TokenStream, + SourceFile, + + 'interned: + Span, + // Symbol is handled manually + } + }; +} + // FIXME(eddyb) this calls `encode` for each argument, but in reverse, // to match the ordering in `reverse_decode`. macro_rules! reverse_encode { diff --git a/library/proc_macro/src/bridge/server.rs b/library/proc_macro/src/bridge/server.rs index 2ea87d866ff3e..8736d1806fbe9 100644 --- a/library/proc_macro/src/bridge/server.rs +++ b/library/proc_macro/src/bridge/server.rs @@ -5,8 +5,79 @@ use super::*; use std::cell::Cell; use std::marker::PhantomData; -// FIXME(eddyb) generate the definition of `HandleStore` in `server.rs`. -use super::client::HandleStore; +macro_rules! define_server_handles { + ( + 'owned: $($oty:ident,)* + 'interned: $($ity:ident,)* + ) => { + #[allow(non_snake_case)] + pub(super) struct HandleStore { + $($oty: handle::OwnedStore,)* + $($ity: handle::InternedStore,)* + } + + impl HandleStore { + fn new(handle_counters: &'static client::HandleCounters) -> Self { + HandleStore { + $($oty: handle::OwnedStore::new(&handle_counters.$oty),)* + $($ity: handle::InternedStore::new(&handle_counters.$ity),)* + } + } + } + + $( + impl Encode>> for Marked { + fn encode(self, w: &mut Writer, s: &mut HandleStore>) { + s.$oty.alloc(self).encode(w, s); + } + } + + impl DecodeMut<'_, '_, HandleStore>> + for Marked + { + fn decode(r: &mut Reader<'_>, s: &mut HandleStore>) -> Self { + s.$oty.take(handle::Handle::decode(r, &mut ())) + } + } + + impl<'s, S: Types> Decode<'_, 's, HandleStore>> + for &'s Marked + { + fn decode(r: &mut Reader<'_>, s: &'s HandleStore>) -> Self { + &s.$oty[handle::Handle::decode(r, &mut ())] + } + } + + impl<'s, S: Types> DecodeMut<'_, 's, HandleStore>> + for &'s mut Marked + { + fn decode( + r: &mut Reader<'_>, + s: &'s mut HandleStore> + ) -> Self { + &mut s.$oty[handle::Handle::decode(r, &mut ())] + } + } + )* + + $( + impl Encode>> for Marked { + fn encode(self, w: &mut Writer, s: &mut HandleStore>) { + s.$ity.alloc(self).encode(w, s); + } + } + + impl DecodeMut<'_, '_, HandleStore>> + for Marked + { + fn decode(r: &mut Reader<'_>, s: &mut HandleStore>) -> Self { + s.$ity.copy(handle::Handle::decode(r, &mut ())) + } + } + )* + } +} +with_api_handle_types!(define_server_handles); pub trait Types { type FreeFunctions: 'static; diff --git a/library/proc_macro/src/bridge/symbol.rs b/library/proc_macro/src/bridge/symbol.rs index 02225d20b26e1..86ce2cc189588 100644 --- a/library/proc_macro/src/bridge/symbol.rs +++ b/library/proc_macro/src/bridge/symbol.rs @@ -109,18 +109,18 @@ impl Encode for Symbol { } } -impl DecodeMut<'_, '_, client::HandleStore>> +impl DecodeMut<'_, '_, server::HandleStore>> for Marked { - fn decode(r: &mut Reader<'_>, s: &mut client::HandleStore>) -> Self { + fn decode(r: &mut Reader<'_>, s: &mut server::HandleStore>) -> Self { Mark::mark(S::intern_symbol(<&str>::decode(r, s))) } } -impl Encode>> +impl Encode>> for Marked { - fn encode(self, w: &mut Writer, s: &mut client::HandleStore>) { + fn encode(self, w: &mut Writer, s: &mut server::HandleStore>) { S::with_symbol_string(&self.unmark(), |sym| sym.encode(w, s)) } } From fb8ac0647755ba40dd08e9a8d0546f6619602502 Mon Sep 17 00:00:00 2001 From: Alexander Date: Fri, 1 Mar 2024 11:51:28 +0000 Subject: [PATCH 11/15] remove hidden use of Global --- library/alloc/src/collections/btree/map.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/alloc/src/collections/btree/map.rs b/library/alloc/src/collections/btree/map.rs index 271ea88fff652..58ffa9710d3e6 100644 --- a/library/alloc/src/collections/btree/map.rs +++ b/library/alloc/src/collections/btree/map.rs @@ -180,7 +180,7 @@ pub struct BTreeMap< /// `ManuallyDrop` to control drop order (needs to be dropped after all the nodes). pub(super) alloc: ManuallyDrop, // For dropck; the `Box` avoids making the `Unpin` impl more strict than before - _marker: PhantomData>, + _marker: PhantomData>, } #[stable(feature = "btree_drop", since = "1.7.0")] From bcccab88ca68292436ce470d85dd4df4315adf5f Mon Sep 17 00:00:00 2001 From: Tobias Bucher Date: Fri, 1 Mar 2024 18:57:42 +0100 Subject: [PATCH 12/15] Use the guaranteed precision of a couple of float functions in docs --- library/std/src/f32.rs | 24 +++++++++++++----------- library/std/src/f64.rs | 24 +++++++++++++----------- 2 files changed, 26 insertions(+), 22 deletions(-) diff --git a/library/std/src/f32.rs b/library/std/src/f32.rs index 6ec389400ae81..de9bde51f2a30 100644 --- a/library/std/src/f32.rs +++ b/library/std/src/f32.rs @@ -186,11 +186,8 @@ impl f32 { /// let x = 3.5_f32; /// let y = -3.5_f32; /// - /// let abs_difference_x = (x.abs() - x).abs(); - /// let abs_difference_y = (y.abs() - (-y)).abs(); - /// - /// assert!(abs_difference_x <= f32::EPSILON); - /// assert!(abs_difference_y <= f32::EPSILON); + /// assert_eq!(x.abs(), x); + /// assert_eq!(y.abs(), -y); /// /// assert!(f32::NAN.abs().is_nan()); /// ``` @@ -276,10 +273,17 @@ impl f32 { /// let x = 4.0_f32; /// let b = 60.0_f32; /// - /// // 100.0 - /// let abs_difference = (m.mul_add(x, b) - ((m * x) + b)).abs(); + /// assert_eq!(m.mul_add(x, b), 100.0); + /// assert_eq!(m * x + b, 100.0); /// - /// assert!(abs_difference <= f32::EPSILON); + /// let one_plus_eps = 1.0_f32 + f32::EPSILON; + /// let one_minus_eps = 1.0_f32 - f32::EPSILON; + /// let minus_one = -1.0_f32; + /// + /// // The exact result (1 + eps) * (1 - eps) = 1 - eps * eps. + /// assert_eq!(one_plus_eps.mul_add(one_minus_eps, minus_one), -f32::EPSILON * f32::EPSILON); + /// // Different rounding with the non-fused multiply and add. + /// assert_eq!(one_plus_eps * one_minus_eps + minus_one, 0.0); /// ``` #[rustc_allow_incoherent_impl] #[must_use = "method returns a new number and does not mutate the original value"] @@ -426,9 +430,7 @@ impl f32 { /// let negative = -4.0_f32; /// let negative_zero = -0.0_f32; /// - /// let abs_difference = (positive.sqrt() - 2.0).abs(); - /// - /// assert!(abs_difference <= f32::EPSILON); + /// assert_eq!(positive.sqrt(), 2.0); /// assert!(negative.sqrt().is_nan()); /// assert!(negative_zero.sqrt() == negative_zero); /// ``` diff --git a/library/std/src/f64.rs b/library/std/src/f64.rs index 7385576c33717..944186d602c8f 100644 --- a/library/std/src/f64.rs +++ b/library/std/src/f64.rs @@ -186,11 +186,8 @@ impl f64 { /// let x = 3.5_f64; /// let y = -3.5_f64; /// - /// let abs_difference_x = (x.abs() - x).abs(); - /// let abs_difference_y = (y.abs() - (-y)).abs(); - /// - /// assert!(abs_difference_x < 1e-10); - /// assert!(abs_difference_y < 1e-10); + /// assert_eq!(x.abs(), x); + /// assert_eq!(y.abs(), -y); /// /// assert!(f64::NAN.abs().is_nan()); /// ``` @@ -276,10 +273,17 @@ impl f64 { /// let x = 4.0_f64; /// let b = 60.0_f64; /// - /// // 100.0 - /// let abs_difference = (m.mul_add(x, b) - ((m * x) + b)).abs(); + /// assert_eq!(m.mul_add(x, b), 100.0); + /// assert_eq!(m * x + b, 100.0); /// - /// assert!(abs_difference < 1e-10); + /// let one_plus_eps = 1.0_f64 + f64::EPSILON; + /// let one_minus_eps = 1.0_f64 - f64::EPSILON; + /// let minus_one = -1.0_f64; + /// + /// // The exact result (1 + eps) * (1 - eps) = 1 - eps * eps. + /// assert_eq!(one_plus_eps.mul_add(one_minus_eps, minus_one), -f64::EPSILON * f64::EPSILON); + /// // Different rounding with the non-fused multiply and add. + /// assert_eq!(one_plus_eps * one_minus_eps + minus_one, 0.0); /// ``` #[rustc_allow_incoherent_impl] #[must_use = "method returns a new number and does not mutate the original value"] @@ -426,9 +430,7 @@ impl f64 { /// let negative = -4.0_f64; /// let negative_zero = -0.0_f64; /// - /// let abs_difference = (positive.sqrt() - 2.0).abs(); - /// - /// assert!(abs_difference < 1e-10); + /// assert_eq!(positive.sqrt(), 2.0); /// assert!(negative.sqrt().is_nan()); /// assert!(negative_zero.sqrt() == negative_zero); /// ``` From 62baa670e325f36e5a8950d25a0bd335e53e95c3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E8=AE=B8=E6=9D=B0=E5=8F=8B=20Jieyou=20Xu=20=28Joe=29?= Date: Fri, 1 Mar 2024 19:02:34 +0000 Subject: [PATCH 13/15] Avoid silently writing to a file when the involved ty is long --- compiler/rustc_hir_typeck/src/method/suggest.rs | 15 +++++++++++++-- .../traits/error_reporting/on_unimplemented.rs | 5 ++--- .../traits/error_reporting/type_err_ctxt_ext.rs | 14 +++++++++++++- .../ui/traits/on_unimplemented_long_types.stderr | 2 ++ 4 files changed, 30 insertions(+), 6 deletions(-) diff --git a/compiler/rustc_hir_typeck/src/method/suggest.rs b/compiler/rustc_hir_typeck/src/method/suggest.rs index f0586328835ce..b67588f8b8c8f 100644 --- a/compiler/rustc_hir_typeck/src/method/suggest.rs +++ b/compiler/rustc_hir_typeck/src/method/suggest.rs @@ -1054,6 +1054,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { bound_list.into_iter().map(|(_, path)| path).collect::>().join("\n"); let actual_prefix = rcvr_ty.prefix_string(self.tcx); info!("unimplemented_traits.len() == {}", unimplemented_traits.len()); + let mut long_ty_file = None; let (primary_message, label) = if unimplemented_traits.len() == 1 && unimplemented_traits_only { @@ -1066,8 +1067,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { // Avoid crashing. return (None, None); } - let OnUnimplementedNote { message, label, .. } = - self.err_ctxt().on_unimplemented_note(trait_ref, &obligation); + let OnUnimplementedNote { message, label, .. } = self + .err_ctxt() + .on_unimplemented_note(trait_ref, &obligation, &mut long_ty_file); (message, label) }) .unwrap() @@ -1081,6 +1083,15 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { ) }); err.primary_message(primary_message); + if let Some(file) = long_ty_file { + err.note(format!( + "the full name for the type has been written to '{}'", + file.display(), + )); + err.note( + "consider using `--verbose` to print the full type name to the console", + ); + } if let Some(label) = label { custom_span_label = true; err.span_label(span, label); diff --git a/compiler/rustc_trait_selection/src/traits/error_reporting/on_unimplemented.rs b/compiler/rustc_trait_selection/src/traits/error_reporting/on_unimplemented.rs index 6773372397206..126bc0c9ec0fc 100644 --- a/compiler/rustc_trait_selection/src/traits/error_reporting/on_unimplemented.rs +++ b/compiler/rustc_trait_selection/src/traits/error_reporting/on_unimplemented.rs @@ -111,9 +111,8 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> { &self, trait_ref: ty::PolyTraitRef<'tcx>, obligation: &PredicateObligation<'tcx>, + long_ty_file: &mut Option, ) -> OnUnimplementedNote { - let mut long_ty_file = None; - let (def_id, args) = self .impl_similar_to(trait_ref, obligation) .unwrap_or_else(|| (trait_ref.def_id(), trait_ref.skip_binder().args)); @@ -268,7 +267,7 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> { })); if let Ok(Some(command)) = OnUnimplementedDirective::of_item(self.tcx, def_id) { - command.evaluate(self.tcx, trait_ref, &flags, &mut long_ty_file) + command.evaluate(self.tcx, trait_ref, &flags, long_ty_file) } else { OnUnimplementedNote::default() } diff --git a/compiler/rustc_trait_selection/src/traits/error_reporting/type_err_ctxt_ext.rs b/compiler/rustc_trait_selection/src/traits/error_reporting/type_err_ctxt_ext.rs index 0c66ce5b46cb7..0bb8dbfe8c78d 100644 --- a/compiler/rustc_trait_selection/src/traits/error_reporting/type_err_ctxt_ext.rs +++ b/compiler/rustc_trait_selection/src/traits/error_reporting/type_err_ctxt_ext.rs @@ -445,13 +445,16 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> { file.display(), )); + let mut long_ty_file = None; + let OnUnimplementedNote { message, label, notes, parent_label, append_const_msg, - } = self.on_unimplemented_note(trait_ref, &obligation); + } = self.on_unimplemented_note(trait_ref, &obligation, &mut long_ty_file); + let have_alt_message = message.is_some() || label.is_some(); let is_try_conversion = self.is_try_conversion(span, trait_ref.def_id()); let is_unsize = @@ -506,6 +509,13 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> { let mut err = struct_span_code_err!(self.dcx(), span, E0277, "{}", err_msg); + if let Some(long_ty_file) = long_ty_file { + err.note(format!( + "the full name for the type has been written to '{}'", + long_ty_file.display(), + )); + err.note("consider using `--verbose` to print the full type name to the console"); + } let mut suggested = false; if is_try_conversion { suggested = self.try_conversion_context(&obligation, trait_ref.skip_binder(), &mut err); @@ -753,6 +763,8 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> { return err.emit(); } + + err } diff --git a/tests/ui/traits/on_unimplemented_long_types.stderr b/tests/ui/traits/on_unimplemented_long_types.stderr index 4c8210f073bd6..bddc5695696b5 100644 --- a/tests/ui/traits/on_unimplemented_long_types.stderr +++ b/tests/ui/traits/on_unimplemented_long_types.stderr @@ -13,6 +13,8 @@ LL | | ))))))))))), LL | | ))))))))))) | |_______________- return type was inferred to be `Option>>` here | + = note: the full name for the type has been written to '$TEST_BUILD_DIR/traits/on_unimplemented_long_types/on_unimplemented_long_types.long-type-hash.txt' + = note: consider using `--verbose` to print the full type name to the console = help: the trait `std::fmt::Display` is not implemented for `Option>>` = note: in format strings you may be able to use `{:?}` (or {:#?} for pretty-print) instead = note: the full name for the type has been written to '$TEST_BUILD_DIR/traits/on_unimplemented_long_types/on_unimplemented_long_types.long-type-hash.txt' From 50ff36239a424c855a5faeac45ac269e1c3f54d8 Mon Sep 17 00:00:00 2001 From: carschandler <92899389+carschandler@users.noreply.github.com> Date: Fri, 1 Mar 2024 17:31:02 -0600 Subject: [PATCH 14/15] Update E0716.md Clearer wording --- compiler/rustc_error_codes/src/error_codes/E0716.md | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/compiler/rustc_error_codes/src/error_codes/E0716.md b/compiler/rustc_error_codes/src/error_codes/E0716.md index b50c8b8e7ca23..be60716a26426 100644 --- a/compiler/rustc_error_codes/src/error_codes/E0716.md +++ b/compiler/rustc_error_codes/src/error_codes/E0716.md @@ -30,9 +30,8 @@ let q = p; Whenever a temporary is created, it is automatically dropped (freed) according to fixed rules. Ordinarily, the temporary is dropped at the end of the enclosing -statement -- in this case, after the outer `let` that assigns to `p`. This is -illustrated in the example above by showing that `tmp` would be freed as we exit -the block. +statement -- in this case, after the `let p`. This is illustrated in the example +above by showing that `tmp` would be freed as we exit the block. To fix this problem, you need to create a local variable to store the value in rather than relying on a temporary. For example, you might change the original From 7f97dfe700be69d359eaebf97275285f8a85faf4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Thu, 29 Feb 2024 16:30:01 +0000 Subject: [PATCH 15/15] Account for unmet `T: !Copy` in E0277 message --- .../src/traits/error_reporting/suggestions.rs | 17 ++++++++++++----- tests/ui/traits/negative-bounds/simple.stderr | 8 ++++---- 2 files changed, 16 insertions(+), 9 deletions(-) diff --git a/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs b/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs index e82e94993d249..06f908f01a97e 100644 --- a/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs +++ b/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs @@ -4773,11 +4773,18 @@ pub(super) fn get_explanation_based_on_obligation<'tcx>( Some(desc) => format!(" {desc}"), None => String::new(), }; - format!( - "{pre_message}the trait `{}` is not implemented for{desc} `{}`{post}", - trait_predicate.print_modifiers_and_trait_path(), - tcx.short_ty_string(trait_ref.skip_binder().self_ty(), &mut None), - ) + if let ty::ImplPolarity::Positive = trait_predicate.polarity() { + format!( + "{pre_message}the trait `{}` is not implemented for{desc} `{}`{post}", + trait_predicate.print_modifiers_and_trait_path(), + tcx.short_ty_string(trait_ref.skip_binder().self_ty(), &mut None), + ) + } else { + // "the trait bound `T: !Send` is not satisfied" reads better than "`!Send` is + // not implemented for `T`". + // FIXME: add note explaining explicit negative trait bounds. + format!("{pre_message}the trait bound `{trait_predicate}` is not satisfied{post}") + } } } diff --git a/tests/ui/traits/negative-bounds/simple.stderr b/tests/ui/traits/negative-bounds/simple.stderr index 6d750739e197c..b8d12138794bf 100644 --- a/tests/ui/traits/negative-bounds/simple.stderr +++ b/tests/ui/traits/negative-bounds/simple.stderr @@ -2,7 +2,7 @@ error[E0277]: the trait bound `T: !Copy` is not satisfied --> $DIR/simple.rs:10:16 | LL | not_copy::(); - | ^ the trait `!Copy` is not implemented for `T` + | ^ the trait bound `T: !Copy` is not satisfied | note: required by a bound in `not_copy` --> $DIR/simple.rs:3:16 @@ -14,7 +14,7 @@ error[E0277]: the trait bound `T: !Copy` is not satisfied --> $DIR/simple.rs:15:16 | LL | not_copy::(); - | ^ the trait `!Copy` is not implemented for `T` + | ^ the trait bound `T: !Copy` is not satisfied | note: required by a bound in `not_copy` --> $DIR/simple.rs:3:16 @@ -26,7 +26,7 @@ error[E0277]: the trait bound `Copyable: !Copy` is not satisfied --> $DIR/simple.rs:30:16 | LL | not_copy::(); - | ^^^^^^^^ the trait `!Copy` is not implemented for `Copyable` + | ^^^^^^^^ the trait bound `Copyable: !Copy` is not satisfied | = help: the trait `Copy` is implemented for `Copyable` note: required by a bound in `not_copy` @@ -44,7 +44,7 @@ error[E0277]: the trait bound `NotNecessarilyCopyable: !Copy` is not satisfied --> $DIR/simple.rs:37:16 | LL | not_copy::(); - | ^^^^^^^^^^^^^^^^^^^^^^ the trait `!Copy` is not implemented for `NotNecessarilyCopyable` + | ^^^^^^^^^^^^^^^^^^^^^^ the trait bound `NotNecessarilyCopyable: !Copy` is not satisfied | note: required by a bound in `not_copy` --> $DIR/simple.rs:3:16