From b03dba6f80b00457fb415668503518d6de7380e4 Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Tue, 15 Sep 2020 23:35:08 -0400 Subject: [PATCH 01/13] Add Linux-specific pidfd process extensions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Background: Over the last year, pidfd support was added to the Linux kernel. This allows interacting with other processes. In particular, this allows waiting on a child process with a timeout in a race-free way, bypassing all of the awful signal-handler tricks that are usually required. Pidfds can be obtained for a child process (as well as any other process) via the `pidfd_open` syscall. Unfortunately, this requires several conditions to hold in order to be race-free (i.e. the pid is not reused). Per `man pidfd_open`: ``` · the disposition of SIGCHLD has not been explicitly set to SIG_IGN (see sigaction(2)); · the SA_NOCLDWAIT flag was not specified while establishing a han‐ dler for SIGCHLD or while setting the disposition of that signal to SIG_DFL (see sigaction(2)); and · the zombie process was not reaped elsewhere in the program (e.g., either by an asynchronously executed signal handler or by wait(2) or similar in another thread). If any of these conditions does not hold, then the child process (along with a PID file descriptor that refers to it) should instead be created using clone(2) with the CLONE_PIDFD flag. ``` Sadly, these conditions are impossible to guarantee once any libraries are used. For example, C code runnng in a different thread could call `wait()`, which is impossible to detect from Rust code trying to open a pidfd. While pid reuse issues should (hopefully) be rare in practice, we can do better. By passing the `CLONE_PIDFD` flag to `clone()` or `clone3()`, we can obtain a pidfd for the child process in a guaranteed race-free manner. This PR: This PR adds Linux-specific process extension methods to allow obtaining pidfds for processes spawned via the standard `Command` API. Other than being made available to user code, the standard library does not make use of these pidfds in any way. In particular, the implementation of `Child::wait` is completely unchanged. Two Linux-specific helper methods are added: `CommandExt::create_pidfd` and `ChildExt::pidfd`. These methods are intended to serve as a building block for libraries to build higher-level abstractions - in particular, waiting on a process with a timeout. I've included a basic test, which verifies that pidfds are created iff the `create_pidfd` method is used. This test is somewhat special - it should always succeed on systems with the `clone3` system call available, and always fail on systems without `clone3` available. I'm not sure how to best ensure this programatically. This PR relies on the newer `clone3` system call to pass the `CLONE_FD`, rather than the older `clone` system call. `clone3` was added to Linux in the same release as pidfds, so this shouldn't unnecessarily limit the kernel versions that this code supports. Unresolved questions: * What should the name of the feature gate be for these newly added methods? * Should the `pidfd` method distinguish between an error occurring and `create_pidfd` not being called? --- library/std/src/os/linux/mod.rs | 1 + library/std/src/os/linux/process.rs | 47 ++++++++ library/std/src/process.rs | 2 +- .../src/sys/unix/process/process_common.rs | 6 + .../std/src/sys/unix/process/process_unix.rs | 109 ++++++++++++++++-- src/test/ui/command/command-create-pidfd.rs | 27 +++++ 6 files changed, 184 insertions(+), 8 deletions(-) create mode 100644 library/std/src/os/linux/process.rs create mode 100644 src/test/ui/command/command-create-pidfd.rs diff --git a/library/std/src/os/linux/mod.rs b/library/std/src/os/linux/mod.rs index f179a524336fc..f7bb63df0c1ff 100644 --- a/library/std/src/os/linux/mod.rs +++ b/library/std/src/os/linux/mod.rs @@ -3,4 +3,5 @@ #![stable(feature = "raw_ext", since = "1.1.0")] pub mod fs; +pub mod process; pub mod raw; diff --git a/library/std/src/os/linux/process.rs b/library/std/src/os/linux/process.rs new file mode 100644 index 0000000000000..661d3cef7a03a --- /dev/null +++ b/library/std/src/os/linux/process.rs @@ -0,0 +1,47 @@ +//! Linux-specific extensions to primitives in the `std::process` module. + +#![unstable(feature = "linux_pidfd", issue = "none")] + +use crate::process; +use crate::sys_common::AsInnerMut; +use crate::io::Result; + +/// Os-specific extensions to [`process::Child`] +/// +/// [`process::Child`]: crate::process::Child +pub trait ChildExt { + /// Obtains the pidfd created for this child process, if available. + /// + /// A pidfd will only ever be available if `create_pidfd(true)` was called + /// when the corresponding `Command` was created. + /// + /// Even if `create_pidfd(true)` is called, a pidfd may not be available + /// due to an older version of Linux being in use, or if + /// some other error occured. + /// + /// See `man pidfd_open` for more details about pidfds. + fn pidfd(&self) -> Result; +} + +/// Os-specific extensions to [`process::Command`] +/// +/// [`process::Command`]: crate::process::Command +pub trait CommandExt { + /// Sets whether or this `Command` will attempt to create a pidfd + /// for the child. If this method is never called, a pidfd will + /// not be crated. + /// + /// The pidfd can be retrieved from the child via [`ChildExt::pidfd`] + /// + /// A pidfd will only be created if it is possible to do so + /// in a guaranteed race-free manner (e.g. if the `clone3` system call is + /// supported). Otherwise, [`ChildExit::pidfd`] will return an error. + fn create_pidfd(&mut self, val: bool) -> &mut process::Command; +} + +impl CommandExt for process::Command { + fn create_pidfd(&mut self, val: bool) -> &mut process::Command { + self.as_inner_mut().create_pidfd(val); + self + } +} diff --git a/library/std/src/process.rs b/library/std/src/process.rs index 15ac9e402c589..3c960c927de45 100644 --- a/library/std/src/process.rs +++ b/library/std/src/process.rs @@ -161,7 +161,7 @@ use crate::sys_common::{AsInner, AsInnerMut, FromInner, IntoInner}; /// [`wait`]: Child::wait #[stable(feature = "process", since = "1.0.0")] pub struct Child { - handle: imp::Process, + pub(crate) handle: imp::Process, /// The handle for writing to the child's standard input (stdin), if it has /// been captured. To avoid partially moving diff --git a/library/std/src/sys/unix/process/process_common.rs b/library/std/src/sys/unix/process/process_common.rs index b9dcc4e4b9e38..38a5915e62b97 100644 --- a/library/std/src/sys/unix/process/process_common.rs +++ b/library/std/src/sys/unix/process/process_common.rs @@ -79,6 +79,7 @@ pub struct Command { stdin: Option, stdout: Option, stderr: Option, + pub(crate) make_pidfd: bool, } // Create a new type for argv, so that we can make it `Send` and `Sync` @@ -141,6 +142,7 @@ impl Command { stdin: None, stdout: None, stderr: None, + make_pidfd: false, } } @@ -176,6 +178,10 @@ impl Command { pub fn groups(&mut self, groups: &[gid_t]) { self.groups = Some(Box::from(groups)); } + + pub fn create_pidfd(&mut self, val: bool) { + self.make_pidfd = val; + } pub fn saw_nul(&self) -> bool { self.saw_nul diff --git a/library/std/src/sys/unix/process/process_unix.rs b/library/std/src/sys/unix/process/process_unix.rs index 2eb64a99e599a..008d95c7aaa89 100644 --- a/library/std/src/sys/unix/process/process_unix.rs +++ b/library/std/src/sys/unix/process/process_unix.rs @@ -6,6 +6,7 @@ use crate::ptr; use crate::sys; use crate::sys::cvt; use crate::sys::process::process_common::*; +use crate::sync::atomic::{AtomicBool, Ordering}; #[cfg(target_os = "vxworks")] use libc::RTP_ID as pid_t; @@ -48,7 +49,8 @@ impl Command { // a lock any more because the parent won't do anything and the child is // in its own process. Thus the parent drops the lock guard while the child // forgets it to avoid unlocking it on a new thread, which would be invalid. - let (env_lock, result) = unsafe { (sys::os::env_lock(), cvt(libc::fork())?) }; + let env_lock = unsafe { sys::os::env_lock() }; + let (result, pidfd) = self.do_fork()?; let pid = unsafe { match result { @@ -76,12 +78,12 @@ impl Command { } n => { drop(env_lock); - n + n as pid_t } } }; - let mut p = Process { pid, status: None }; + let mut p = Process { pid, status: None, pidfd }; drop(output); let mut bytes = [0; 8]; @@ -114,6 +116,85 @@ impl Command { } } + // Attempts to fork the process. If successful, returns + // Ok((0, -1)) in the child, and Ok((child_pid, child_pidfd)) in the parent. + fn do_fork(&mut self) -> Result<(libc::c_long, libc::pid_t), io::Error> { + // If we fail to create a pidfd for any reason, this will + // stay as -1, which indicates an error + let mut pidfd: libc::pid_t = -1; + + // On Linux, attempt to use the `clone3` syscall, which + // supports more argument (in prarticular, the ability to create a pidfd). + // If this fails, we will fall through this block to a call to `fork()` + cfg_if::cfg_if! { + if #[cfg(target_os = "linux")] { + static HAS_CLONE3: AtomicBool = AtomicBool::new(true); + + const CLONE_PIDFD: u64 = 0x00001000; + + #[repr(C)] + struct clone_args { + flags: u64, + pidfd: u64, + child_tid: u64, + parent_tid: u64, + exit_signal: u64, + stack: u64, + stack_size: u64, + tls: u64, + set_tid: u64, + set_tid_size: u64, + cgroup: u64, + } + + syscall! { + fn clone3(cl_args: *mut clone_args, len: libc::size_t) -> libc::c_long + } + + if HAS_CLONE3.load(Ordering::Relaxed) { + let mut flags = 0; + if self.make_pidfd { + flags |= CLONE_PIDFD; + } + + let mut args = clone_args { + flags, + pidfd: &mut pidfd as *mut libc::pid_t as u64, + child_tid: 0, + parent_tid: 0, + exit_signal: libc::SIGCHLD as u64, + stack: 0, + stack_size: 0, + tls: 0, + set_tid: 0, + set_tid_size: 0, + cgroup: 0 + }; + + let args_ptr = &mut args as *mut clone_args; + let args_size = crate::mem::size_of::(); + + let res = cvt(unsafe { clone3(args_ptr, args_size) }); + match res { + Ok(n) => return Ok((n, pidfd)), + Err(e) => match e.raw_os_error() { + // Multiple threads can race to execute this store, + // but that's fine - that just means that multiple threads + // will have tried and failed to execute the same syscall, + // with no other side effects. + Some(libc::ENOSYS) => HAS_CLONE3.store(false, Ordering::Relaxed), + _ => return Err(e) + } + } + } + } + } + // If we get here, we are either not on Linux, + // or we are on Linux and the 'clone3' syscall does not exist + cvt(unsafe { libc::fork() }.into()).map(|res| (res, pidfd)) + } + + pub fn exec(&mut self, default: Stdio) -> io::Error { let envp = self.capture_env(); @@ -265,8 +346,6 @@ impl Command { #[cfg(not(any( target_os = "macos", target_os = "freebsd", - all(target_os = "linux", target_env = "gnu"), - all(target_os = "linux", target_env = "musl"), )))] fn posix_spawn( &mut self, @@ -281,8 +360,6 @@ impl Command { #[cfg(any( target_os = "macos", target_os = "freebsd", - all(target_os = "linux", target_env = "gnu"), - all(target_os = "linux", target_env = "musl"), ))] fn posix_spawn( &mut self, @@ -430,6 +507,12 @@ impl Command { pub struct Process { pid: pid_t, status: Option, + // On Linux, stores the pidfd created for this child. + // This is -1 if the user did not request pidfd creation, + // or if the pidfd could not be created for some reason + // (e.g. the `clone3` syscall was not available). + #[cfg(target_os = "linux")] + pidfd: libc::c_int, } impl Process { @@ -546,6 +629,18 @@ impl fmt::Display for ExitStatus { } } +#[cfg(target_os = "linux")] +#[unstable(feature = "linux_pidfd", issue = "none")] +impl crate::os::linux::process::ChildExt for crate::process::Child { + fn pidfd(&self) -> crate::io::Result { + if self.handle.pidfd > 0 { + Ok(self.handle.pidfd) + } else { + Err(crate::io::Error::from(crate::io::ErrorKind::Other)) + } + } +} + #[cfg(test)] #[path = "process_unix/tests.rs"] mod tests; diff --git a/src/test/ui/command/command-create-pidfd.rs b/src/test/ui/command/command-create-pidfd.rs new file mode 100644 index 0000000000000..248ae3457d715 --- /dev/null +++ b/src/test/ui/command/command-create-pidfd.rs @@ -0,0 +1,27 @@ +// run-pass +// linux-only - pidfds are a linux-specific concept + +#![feature(linux_pidfd)] +use std::os::linux::process::{CommandExt, ChildExt}; +use std::process::Command; + +fn main() { + // We don't assert the precise value, since the standard libarary + // may be opened other file descriptors before our code ran. + let _ = Command::new("echo") + .create_pidfd(true) + .spawn() + .unwrap() + .pidfd().expect("failed to obtain pidfd"); + + let _ = Command::new("echo") + .create_pidfd(false) + .spawn() + .unwrap() + .pidfd().expect_err("pidfd should not have been created when create_pid(false) is set"); + + let _ = Command::new("echo") + .spawn() + .unwrap() + .pidfd().expect_err("pidfd should not have been created"); +} From 2e20fe3a8e5569aada0176943509c677fdd12474 Mon Sep 17 00:00:00 2001 From: Josh Triplett Date: Sat, 17 Oct 2020 20:10:58 -0700 Subject: [PATCH 02/13] Typo fix Co-authored-by: bjorn3 --- library/std/src/sys/unix/process/process_unix.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/std/src/sys/unix/process/process_unix.rs b/library/std/src/sys/unix/process/process_unix.rs index 008d95c7aaa89..4b1b49dac364d 100644 --- a/library/std/src/sys/unix/process/process_unix.rs +++ b/library/std/src/sys/unix/process/process_unix.rs @@ -124,7 +124,7 @@ impl Command { let mut pidfd: libc::pid_t = -1; // On Linux, attempt to use the `clone3` syscall, which - // supports more argument (in prarticular, the ability to create a pidfd). + // supports more argument (in particular, the ability to create a pidfd). // If this fails, we will fall through this block to a call to `fork()` cfg_if::cfg_if! { if #[cfg(target_os = "linux")] { From ed86f6e5cfbb2a3302f029fa78875625e5541fc9 Mon Sep 17 00:00:00 2001 From: Dominik Stolz Date: Sat, 6 Feb 2021 14:15:49 +0100 Subject: [PATCH 03/13] Add PidFd type and seal traits Improve docs --- library/std/src/os/linux/process.rs | 146 +++++++++++--- .../src/sys/unix/process/process_common.rs | 8 +- .../std/src/sys/unix/process/process_unix.rs | 178 ++++++++++-------- src/test/ui/command/command-create-pidfd.rs | 4 +- 4 files changed, 233 insertions(+), 103 deletions(-) diff --git a/library/std/src/os/linux/process.rs b/library/std/src/os/linux/process.rs index 661d3cef7a03a..435c0227c7eca 100644 --- a/library/std/src/os/linux/process.rs +++ b/library/std/src/os/linux/process.rs @@ -2,40 +2,142 @@ #![unstable(feature = "linux_pidfd", issue = "none")] -use crate::process; -use crate::sys_common::AsInnerMut; use crate::io::Result; +use crate::os::unix::io::{AsRawFd, FromRawFd, IntoRawFd, RawFd}; +use crate::process; +use crate::sys::fd::FileDesc; +use crate::sys_common::{AsInner, AsInnerMut, FromInner, IntoInner}; -/// Os-specific extensions to [`process::Child`] +/// This type represents a file descriptor that refers to a process. +/// +/// A `PidFd` can be obtained by setting the corresponding option on [`Command`] +/// with [`create_pidfd`]. Subsequently, the created pidfd can be retrieved +/// from the [`Child`] by calling [`pidfd`] or [`take_pidfd`]. +/// +/// Example: +/// ```no_run +/// #![feature(linux_pidfd)] +/// use std::os::linux::process::{CommandExt, ChildExt}; +/// use std::process::Command; +/// +/// let mut child = Command::new("echo") +/// .create_pidfd(true) +/// .spawn() +/// .expect("Failed to spawn child"); /// -/// [`process::Child`]: crate::process::Child -pub trait ChildExt { - /// Obtains the pidfd created for this child process, if available. +/// let pidfd = child +/// .take_pidfd() +/// .expect("Failed to retrieve pidfd"); +/// +/// // The file descriptor will be closed when `pidfd` is dropped. +/// ``` +/// Refer to the man page of `pidfd_open(2)` for further details. +/// +/// [`Command`]: process::Command +/// [`create_pidfd`]: CommandExt::create_pidfd +/// [`Child`]: process::Child +/// [`pidfd`]: fn@ChildExt::pidfd +/// [`take_pidfd`]: ChildExt::take_pidfd +#[derive(Debug)] +pub struct PidFd { + inner: FileDesc, +} + +impl AsInner for PidFd { + fn as_inner(&self) -> &FileDesc { + &self.inner + } +} + +impl FromInner for PidFd { + fn from_inner(inner: FileDesc) -> PidFd { + PidFd { inner } + } +} + +impl IntoInner for PidFd { + fn into_inner(self) -> FileDesc { + self.inner + } +} + +impl AsRawFd for PidFd { + fn as_raw_fd(&self) -> RawFd { + self.as_inner().raw() + } +} + +impl FromRawFd for PidFd { + unsafe fn from_raw_fd(fd: RawFd) -> Self { + Self::from_inner(FileDesc::new(fd)) + } +} + +impl IntoRawFd for PidFd { + fn into_raw_fd(self) -> RawFd { + self.into_inner().into_raw() + } +} + +mod private_child_ext { + pub trait Sealed {} + impl Sealed for crate::process::Child {} +} + +/// Os-specific extensions for [`Child`] +/// +/// [`Child`]: process::Child +pub trait ChildExt: private_child_ext::Sealed { + /// Obtains a reference to the [`PidFd`] created for this [`Child`], if available. + /// + /// A pidfd will only be available if its creation was requested with + /// [`create_pidfd`] when the corresponding [`Command`] was created. /// - /// A pidfd will only ever be available if `create_pidfd(true)` was called - /// when the corresponding `Command` was created. + /// Even if requested, a pidfd may not be available due to an older + /// version of Linux being in use, or if some other error occurred. + /// + /// [`Command`]: process::Command + /// [`create_pidfd`]: CommandExt::create_pidfd + /// [`Child`]: process::Child + fn pidfd(&self) -> Result<&PidFd>; + + /// Takes ownership of the [`PidFd`] created for this [`Child`], if available. /// - /// Even if `create_pidfd(true)` is called, a pidfd may not be available - /// due to an older version of Linux being in use, or if - /// some other error occured. + /// A pidfd will only be available if its creation was requested with + /// [`create_pidfd`] when the corresponding [`Command`] was created. /// - /// See `man pidfd_open` for more details about pidfds. - fn pidfd(&self) -> Result; + /// Even if requested, a pidfd may not be available due to an older + /// version of Linux being in use, or if some other error occurred. + /// + /// [`Command`]: process::Command + /// [`create_pidfd`]: CommandExt::create_pidfd + /// [`Child`]: process::Child + fn take_pidfd(&mut self) -> Result; +} + +mod private_command_ext { + pub trait Sealed {} + impl Sealed for crate::process::Command {} } -/// Os-specific extensions to [`process::Command`] +/// Os-specific extensions for [`Command`] /// -/// [`process::Command`]: crate::process::Command -pub trait CommandExt { - /// Sets whether or this `Command` will attempt to create a pidfd - /// for the child. If this method is never called, a pidfd will - /// not be crated. +/// [`Command`]: process::Command +pub trait CommandExt: private_command_ext::Sealed { + /// Sets whether a [`PidFd`](struct@PidFd) should be created for the [`Child`] + /// spawned by this [`Command`]. + /// By default, no pidfd will be created. /// - /// The pidfd can be retrieved from the child via [`ChildExt::pidfd`] + /// The pidfd can be retrieved from the child with [`pidfd`] or [`take_pidfd`]. /// /// A pidfd will only be created if it is possible to do so - /// in a guaranteed race-free manner (e.g. if the `clone3` system call is - /// supported). Otherwise, [`ChildExit::pidfd`] will return an error. + /// in a guaranteed race-free manner (e.g. if the `clone3` system call + /// is supported). Otherwise, [`pidfd`] will return an error. + /// + /// [`Command`]: process::Command + /// [`Child`]: process::Child + /// [`pidfd`]: fn@ChildExt::pidfd + /// [`take_pidfd`]: ChildExt::take_pidfd fn create_pidfd(&mut self, val: bool) -> &mut process::Command; } diff --git a/library/std/src/sys/unix/process/process_common.rs b/library/std/src/sys/unix/process/process_common.rs index 38a5915e62b97..9f6e120134a95 100644 --- a/library/std/src/sys/unix/process/process_common.rs +++ b/library/std/src/sys/unix/process/process_common.rs @@ -79,7 +79,7 @@ pub struct Command { stdin: Option, stdout: Option, stderr: Option, - pub(crate) make_pidfd: bool, + pub(crate) create_pidfd: bool, } // Create a new type for argv, so that we can make it `Send` and `Sync` @@ -142,7 +142,7 @@ impl Command { stdin: None, stdout: None, stderr: None, - make_pidfd: false, + create_pidfd: false, } } @@ -178,9 +178,9 @@ impl Command { pub fn groups(&mut self, groups: &[gid_t]) { self.groups = Some(Box::from(groups)); } - + pub fn create_pidfd(&mut self, val: bool) { - self.make_pidfd = val; + self.create_pidfd = val; } pub fn saw_nul(&self) -> bool { diff --git a/library/std/src/sys/unix/process/process_unix.rs b/library/std/src/sys/unix/process/process_unix.rs index 4b1b49dac364d..dc5005c768662 100644 --- a/library/std/src/sys/unix/process/process_unix.rs +++ b/library/std/src/sys/unix/process/process_unix.rs @@ -3,16 +3,20 @@ use crate::fmt; use crate::io::{self, Error, ErrorKind}; use crate::mem; use crate::ptr; +use crate::sync::atomic::{AtomicBool, Ordering}; use crate::sys; use crate::sys::cvt; use crate::sys::process::process_common::*; -use crate::sync::atomic::{AtomicBool, Ordering}; +use crate::sys_common::FromInner; + +#[cfg(target_os = "linux")] +use crate::os::linux::process::PidFd; #[cfg(target_os = "vxworks")] use libc::RTP_ID as pid_t; #[cfg(not(target_os = "vxworks"))] -use libc::{c_int, gid_t, pid_t, uid_t}; +use libc::{c_int, c_long, gid_t, pid_t, uid_t}; //////////////////////////////////////////////////////////////////////////////// // Command @@ -78,12 +82,12 @@ impl Command { } n => { drop(env_lock); - n as pid_t + n } } }; - let mut p = Process { pid, status: None, pidfd }; + let mut p = Process::new(pid, pidfd); drop(output); let mut bytes = [0; 8]; @@ -118,83 +122,85 @@ impl Command { // Attempts to fork the process. If successful, returns // Ok((0, -1)) in the child, and Ok((child_pid, child_pidfd)) in the parent. - fn do_fork(&mut self) -> Result<(libc::c_long, libc::pid_t), io::Error> { + fn do_fork(&mut self) -> Result<(pid_t, pid_t), io::Error> { // If we fail to create a pidfd for any reason, this will // stay as -1, which indicates an error - let mut pidfd: libc::pid_t = -1; + let mut pidfd: pid_t = -1; // On Linux, attempt to use the `clone3` syscall, which - // supports more argument (in particular, the ability to create a pidfd). + // supports more arguments (in particular, the ability to create a pidfd). // If this fails, we will fall through this block to a call to `fork()` - cfg_if::cfg_if! { - if #[cfg(target_os = "linux")] { - static HAS_CLONE3: AtomicBool = AtomicBool::new(true); - - const CLONE_PIDFD: u64 = 0x00001000; - - #[repr(C)] - struct clone_args { - flags: u64, - pidfd: u64, - child_tid: u64, - parent_tid: u64, - exit_signal: u64, - stack: u64, - stack_size: u64, - tls: u64, - set_tid: u64, - set_tid_size: u64, - cgroup: u64, - } + #[cfg(target_os = "linux")] + { + static HAS_CLONE3: AtomicBool = AtomicBool::new(true); + + const CLONE_PIDFD: u64 = 0x00001000; + + #[repr(C)] + struct clone_args { + flags: u64, + pidfd: u64, + child_tid: u64, + parent_tid: u64, + exit_signal: u64, + stack: u64, + stack_size: u64, + tls: u64, + set_tid: u64, + set_tid_size: u64, + cgroup: u64, + } - syscall! { - fn clone3(cl_args: *mut clone_args, len: libc::size_t) -> libc::c_long - } + syscall! { + fn clone3(cl_args: *mut clone_args, len: libc::size_t) -> c_long + } - if HAS_CLONE3.load(Ordering::Relaxed) { - let mut flags = 0; - if self.make_pidfd { - flags |= CLONE_PIDFD; - } + if HAS_CLONE3.load(Ordering::Relaxed) { + let mut flags = 0; + if self.create_pidfd { + flags |= CLONE_PIDFD; + } - let mut args = clone_args { - flags, - pidfd: &mut pidfd as *mut libc::pid_t as u64, - child_tid: 0, - parent_tid: 0, - exit_signal: libc::SIGCHLD as u64, - stack: 0, - stack_size: 0, - tls: 0, - set_tid: 0, - set_tid_size: 0, - cgroup: 0 - }; - - let args_ptr = &mut args as *mut clone_args; - let args_size = crate::mem::size_of::(); - - let res = cvt(unsafe { clone3(args_ptr, args_size) }); - match res { - Ok(n) => return Ok((n, pidfd)), - Err(e) => match e.raw_os_error() { - // Multiple threads can race to execute this store, - // but that's fine - that just means that multiple threads - // will have tried and failed to execute the same syscall, - // with no other side effects. - Some(libc::ENOSYS) => HAS_CLONE3.store(false, Ordering::Relaxed), - _ => return Err(e) - } - } + let mut args = clone_args { + flags, + pidfd: &mut pidfd as *mut pid_t as u64, + child_tid: 0, + parent_tid: 0, + exit_signal: libc::SIGCHLD as u64, + stack: 0, + stack_size: 0, + tls: 0, + set_tid: 0, + set_tid_size: 0, + cgroup: 0, + }; + + let args_ptr = &mut args as *mut clone_args; + let args_size = crate::mem::size_of::(); + + let res = cvt(unsafe { clone3(args_ptr, args_size) }); + match res { + Ok(n) => return Ok((n as pid_t, pidfd)), + Err(e) => match e.raw_os_error() { + // Multiple threads can race to execute this store, + // but that's fine - that just means that multiple threads + // will have tried and failed to execute the same syscall, + // with no other side effects. + Some(libc::ENOSYS) => HAS_CLONE3.store(false, Ordering::Relaxed), + // Fallback to fork if `EPERM` is returned. (e.g. blocked by seccomp) + Some(libc::EPERM) => {} + _ => return Err(e), + }, } } } + // If we get here, we are either not on Linux, // or we are on Linux and the 'clone3' syscall does not exist - cvt(unsafe { libc::fork() }.into()).map(|res| (res, pidfd)) + // or we do not have permission to call it + cvt(unsafe { libc::fork() }).map(|res| (res, pidfd)) } - pub fn exec(&mut self, default: Stdio) -> io::Error { let envp = self.capture_env(); @@ -346,6 +352,8 @@ impl Command { #[cfg(not(any( target_os = "macos", target_os = "freebsd", + all(target_os = "linux", target_env = "gnu"), + all(target_os = "linux", target_env = "musl"), )))] fn posix_spawn( &mut self, @@ -360,6 +368,8 @@ impl Command { #[cfg(any( target_os = "macos", target_os = "freebsd", + all(target_os = "linux", target_env = "gnu"), + all(target_os = "linux", target_env = "musl"), ))] fn posix_spawn( &mut self, @@ -374,6 +384,7 @@ impl Command { || (self.env_saw_path() && !self.program_is_path()) || !self.get_closures().is_empty() || self.get_groups().is_some() + || self.create_pidfd { return Ok(None); } @@ -418,7 +429,7 @@ impl Command { None => None, }; - let mut p = Process { pid: 0, status: None }; + let mut p = Process::new(0, -1); struct PosixSpawnFileActions<'a>(&'a mut MaybeUninit); @@ -508,14 +519,25 @@ pub struct Process { pid: pid_t, status: Option, // On Linux, stores the pidfd created for this child. - // This is -1 if the user did not request pidfd creation, + // This is None if the user did not request pidfd creation, // or if the pidfd could not be created for some reason // (e.g. the `clone3` syscall was not available). #[cfg(target_os = "linux")] - pidfd: libc::c_int, + pidfd: Option, } impl Process { + #[cfg(target_os = "linux")] + fn new(pid: pid_t, pidfd: pid_t) -> Self { + let pidfd = (pidfd >= 0).then(|| PidFd::from_inner(sys::fd::FileDesc::new(pidfd))); + Process { pid, status: None, pidfd } + } + + #[cfg(not(target_os = "linux"))] + fn new(pid: pid_t, _pidfd: pid_t) -> Self { + Process { pid, status: None } + } + pub fn id(&self) -> u32 { self.pid as u32 } @@ -632,12 +654,18 @@ impl fmt::Display for ExitStatus { #[cfg(target_os = "linux")] #[unstable(feature = "linux_pidfd", issue = "none")] impl crate::os::linux::process::ChildExt for crate::process::Child { - fn pidfd(&self) -> crate::io::Result { - if self.handle.pidfd > 0 { - Ok(self.handle.pidfd) - } else { - Err(crate::io::Error::from(crate::io::ErrorKind::Other)) - } + fn pidfd(&self) -> io::Result<&PidFd> { + self.handle + .pidfd + .as_ref() + .ok_or_else(|| Error::new(ErrorKind::Other, "No pidfd was created.")) + } + + fn take_pidfd(&mut self) -> io::Result { + self.handle + .pidfd + .take() + .ok_or_else(|| Error::new(ErrorKind::Other, "No pidfd was created.")) } } diff --git a/src/test/ui/command/command-create-pidfd.rs b/src/test/ui/command/command-create-pidfd.rs index 248ae3457d715..2e08eb1be540a 100644 --- a/src/test/ui/command/command-create-pidfd.rs +++ b/src/test/ui/command/command-create-pidfd.rs @@ -6,8 +6,8 @@ use std::os::linux::process::{CommandExt, ChildExt}; use std::process::Command; fn main() { - // We don't assert the precise value, since the standard libarary - // may be opened other file descriptors before our code ran. + // We don't assert the precise value, since the standard library + // might have opened other file descriptors before our code runs. let _ = Command::new("echo") .create_pidfd(true) .spawn() From f23b30c66c01865aff90f1bb7ec4deb5cb77f818 Mon Sep 17 00:00:00 2001 From: Dominik Stolz Date: Wed, 10 Mar 2021 11:21:01 +0100 Subject: [PATCH 04/13] Add tracking issue and link to man-page --- library/std/src/os/linux/process.rs | 5 +++-- library/std/src/sys/unix/process/process_unix.rs | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/library/std/src/os/linux/process.rs b/library/std/src/os/linux/process.rs index 435c0227c7eca..17b9ae82bce4d 100644 --- a/library/std/src/os/linux/process.rs +++ b/library/std/src/os/linux/process.rs @@ -1,6 +1,6 @@ //! Linux-specific extensions to primitives in the `std::process` module. -#![unstable(feature = "linux_pidfd", issue = "none")] +#![unstable(feature = "linux_pidfd", issue = "82971")] use crate::io::Result; use crate::os::unix::io::{AsRawFd, FromRawFd, IntoRawFd, RawFd}; @@ -31,13 +31,14 @@ use crate::sys_common::{AsInner, AsInnerMut, FromInner, IntoInner}; /// /// // The file descriptor will be closed when `pidfd` is dropped. /// ``` -/// Refer to the man page of `pidfd_open(2)` for further details. +/// Refer to the man page of [`pidfd_open(2)`] for further details. /// /// [`Command`]: process::Command /// [`create_pidfd`]: CommandExt::create_pidfd /// [`Child`]: process::Child /// [`pidfd`]: fn@ChildExt::pidfd /// [`take_pidfd`]: ChildExt::take_pidfd +/// [`pidfd_open(2)`]: https://man7.org/linux/man-pages/man2/pidfd_open.2.html #[derive(Debug)] pub struct PidFd { inner: FileDesc, diff --git a/library/std/src/sys/unix/process/process_unix.rs b/library/std/src/sys/unix/process/process_unix.rs index dc5005c768662..5241b2f909f5d 100644 --- a/library/std/src/sys/unix/process/process_unix.rs +++ b/library/std/src/sys/unix/process/process_unix.rs @@ -652,7 +652,7 @@ impl fmt::Display for ExitStatus { } #[cfg(target_os = "linux")] -#[unstable(feature = "linux_pidfd", issue = "none")] +#[unstable(feature = "linux_pidfd", issue = "82971")] impl crate::os::linux::process::ChildExt for crate::process::Child { fn pidfd(&self) -> io::Result<&PidFd> { self.handle From 1ba71abddd744ff8bfbbb100c64ec8cbc52df62e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Mi=C4=85sko?= Date: Thu, 11 Mar 2021 00:00:00 +0000 Subject: [PATCH 05/13] Inline Attribute::has_name --- compiler/rustc_ast/src/attr/mod.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/compiler/rustc_ast/src/attr/mod.rs b/compiler/rustc_ast/src/attr/mod.rs index 1e224dbf83390..40b0cefd83aa6 100644 --- a/compiler/rustc_ast/src/attr/mod.rs +++ b/compiler/rustc_ast/src/attr/mod.rs @@ -120,6 +120,7 @@ impl NestedMetaItem { } impl Attribute { + #[inline] pub fn has_name(&self, name: Symbol) -> bool { match self.kind { AttrKind::Normal(ref item, _) => item.path == name, From 49431909a6a8ccb915302d57c869e86d2a576af6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Mi=C4=85sko?= Date: Thu, 11 Mar 2021 00:00:00 +0000 Subject: [PATCH 06/13] Validate rustc_layout_scalar_valid_range_{start,end} attributes --- compiler/rustc_passes/src/check_attr.rs | 37 ++++++++++++++++++- ...invalid_rustc_layout_scalar_valid_range.rs | 23 ++++++++++++ ...lid_rustc_layout_scalar_valid_range.stderr | 31 ++++++++++++++++ 3 files changed, 90 insertions(+), 1 deletion(-) create mode 100644 src/test/ui/invalid/invalid_rustc_layout_scalar_valid_range.rs create mode 100644 src/test/ui/invalid/invalid_rustc_layout_scalar_valid_range.stderr diff --git a/compiler/rustc_passes/src/check_attr.rs b/compiler/rustc_passes/src/check_attr.rs index c7b266f18bf8d..ae8883754d6f2 100644 --- a/compiler/rustc_passes/src/check_attr.rs +++ b/compiler/rustc_passes/src/check_attr.rs @@ -8,7 +8,7 @@ use rustc_middle::hir::map::Map; use rustc_middle::ty::query::Providers; use rustc_middle::ty::TyCtxt; -use rustc_ast::{Attribute, LitKind, NestedMetaItem}; +use rustc_ast::{Attribute, Lit, LitKind, NestedMetaItem}; use rustc_errors::{pluralize, struct_span_err}; use rustc_hir as hir; use rustc_hir::def_id::LocalDefId; @@ -87,6 +87,10 @@ impl CheckAttrVisitor<'tcx> { self.check_export_name(hir_id, &attr, span, target) } else if self.tcx.sess.check_name(attr, sym::rustc_args_required_const) { self.check_rustc_args_required_const(&attr, span, target, item) + } else if self.tcx.sess.check_name(attr, sym::rustc_layout_scalar_valid_range_start) { + self.check_rustc_layout_scalar_valid_range(&attr, span, target) + } else if self.tcx.sess.check_name(attr, sym::rustc_layout_scalar_valid_range_end) { + self.check_rustc_layout_scalar_valid_range(&attr, span, target) } else if self.tcx.sess.check_name(attr, sym::allow_internal_unstable) { self.check_allow_internal_unstable(hir_id, &attr, span, target, &attrs) } else if self.tcx.sess.check_name(attr, sym::rustc_allow_const_fn_unstable) { @@ -807,6 +811,37 @@ impl CheckAttrVisitor<'tcx> { } } + fn check_rustc_layout_scalar_valid_range( + &self, + attr: &Attribute, + span: &Span, + target: Target, + ) -> bool { + if target != Target::Struct { + self.tcx + .sess + .struct_span_err(attr.span, "attribute should be applied to a struct") + .span_label(*span, "not a struct") + .emit(); + return false; + } + + let list = match attr.meta_item_list() { + None => return false, + Some(it) => it, + }; + + if matches!(&list[..], &[NestedMetaItem::Literal(Lit { kind: LitKind::Int(..), .. })]) { + true + } else { + self.tcx + .sess + .struct_span_err(attr.span, "expected exactly one integer literal argument") + .emit(); + false + } + } + /// Checks if `#[rustc_legacy_const_generics]` is applied to a function and has a valid argument. fn check_rustc_legacy_const_generics( &self, diff --git a/src/test/ui/invalid/invalid_rustc_layout_scalar_valid_range.rs b/src/test/ui/invalid/invalid_rustc_layout_scalar_valid_range.rs new file mode 100644 index 0000000000000..25fe4be660b24 --- /dev/null +++ b/src/test/ui/invalid/invalid_rustc_layout_scalar_valid_range.rs @@ -0,0 +1,23 @@ +#![feature(rustc_attrs)] + +#[rustc_layout_scalar_valid_range_start(u32::MAX)] //~ ERROR +pub struct A(u32); + +#[rustc_layout_scalar_valid_range_end(1, 2)] //~ ERROR +pub struct B(u8); + +#[rustc_layout_scalar_valid_range_end(a = "a")] //~ ERROR +pub struct C(i32); + +#[rustc_layout_scalar_valid_range_end(1)] //~ ERROR +enum E { + X = 1, + Y = 14, +} + +fn main() { + let _ = A(0); + let _ = B(0); + let _ = C(0); + let _ = E::X; +} diff --git a/src/test/ui/invalid/invalid_rustc_layout_scalar_valid_range.stderr b/src/test/ui/invalid/invalid_rustc_layout_scalar_valid_range.stderr new file mode 100644 index 0000000000000..7e95fedebdfc6 --- /dev/null +++ b/src/test/ui/invalid/invalid_rustc_layout_scalar_valid_range.stderr @@ -0,0 +1,31 @@ +error: expected exactly one integer literal argument + --> $DIR/invalid_rustc_layout_scalar_valid_range.rs:3:1 + | +LL | #[rustc_layout_scalar_valid_range_start(u32::MAX)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: expected exactly one integer literal argument + --> $DIR/invalid_rustc_layout_scalar_valid_range.rs:6:1 + | +LL | #[rustc_layout_scalar_valid_range_end(1, 2)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: expected exactly one integer literal argument + --> $DIR/invalid_rustc_layout_scalar_valid_range.rs:9:1 + | +LL | #[rustc_layout_scalar_valid_range_end(a = "a")] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: attribute should be applied to a struct + --> $DIR/invalid_rustc_layout_scalar_valid_range.rs:12:1 + | +LL | #[rustc_layout_scalar_valid_range_end(1)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +LL | / enum E { +LL | | X = 1, +LL | | Y = 14, +LL | | } + | |_- not a struct + +error: aborting due to 4 previous errors + From 0466b6ac6d2af6e9aa6cdcdde18acbf2a5056944 Mon Sep 17 00:00:00 2001 From: Yuki Okushi Date: Sat, 13 Mar 2021 05:56:30 +0900 Subject: [PATCH 07/13] Improve the wording for the `can't reassign` error --- compiler/rustc_parse/src/parser/stmt.rs | 3 ++- src/test/ui/parser/let-binop.stderr | 6 ++++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/compiler/rustc_parse/src/parser/stmt.rs b/compiler/rustc_parse/src/parser/stmt.rs index f6599927c6e6d..92e67e7929180 100644 --- a/compiler/rustc_parse/src/parser/stmt.rs +++ b/compiler/rustc_parse/src/parser/stmt.rs @@ -291,7 +291,7 @@ impl<'a> Parser<'a> { Ok(P(ast::Local { ty, pat, init, id: DUMMY_NODE_ID, span: lo.to(hi), attrs, tokens: None })) } - /// Parses the RHS of a local variable declaration (e.g., '= 14;'). + /// Parses the RHS of a local variable declaration (e.g., `= 14;`). fn parse_initializer(&mut self, eq_optional: bool) -> PResult<'a, Option>> { let eq_consumed = match self.token.kind { token::BinOpEq(..) => { @@ -306,6 +306,7 @@ impl<'a> Parser<'a> { "=".to_string(), Applicability::MaybeIncorrect, ) + .help("if you meant to overwrite, remove the `let` binding") .emit(); self.bump(); true diff --git a/src/test/ui/parser/let-binop.stderr b/src/test/ui/parser/let-binop.stderr index 90295854a2d2d..dd33e9157cfd5 100644 --- a/src/test/ui/parser/let-binop.stderr +++ b/src/test/ui/parser/let-binop.stderr @@ -3,18 +3,24 @@ error: can't reassign to an uninitialized variable | LL | let a: i8 *= 1; | ^^ help: initialize the variable + | + = help: if you meant to overwrite, remove the `let` binding error: can't reassign to an uninitialized variable --> $DIR/let-binop.rs:6:11 | LL | let b += 1; | ^^ help: initialize the variable + | + = help: if you meant to overwrite, remove the `let` binding error: can't reassign to an uninitialized variable --> $DIR/let-binop.rs:8:11 | LL | let c *= 1; | ^^ help: initialize the variable + | + = help: if you meant to overwrite, remove the `let` binding error: aborting due to 3 previous errors From c74fdf0328e3666443b749c7b02cfe82e826eeaa Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Sat, 13 Mar 2021 19:14:18 +0300 Subject: [PATCH 08/13] More precise spans for HIR paths --- compiler/rustc_ast/src/ast.rs | 8 ++++++++ compiler/rustc_ast_lowering/src/path.rs | 13 ++++++++----- compiler/rustc_hir/src/hir.rs | 2 +- .../rustc_infer/src/traits/error_reporting/mod.rs | 2 +- compiler/rustc_typeck/src/astconv/mod.rs | 9 +++++++-- compiler/rustc_typeck/src/check/fn_ctxt/checks.rs | 2 +- src/test/ui/bad/bad-sized.rs | 1 + src/test/ui/bad/bad-sized.stderr | 15 ++++++++++++++- .../cfg-attr-multi-true.stderr | 2 +- src/test/ui/issues/issue-78622.stderr | 2 +- src/test/ui/mir/issue-80742.stderr | 2 +- .../issue-75361-mismatched-impl.stderr | 2 +- .../associated-item-privacy-inherent.stderr | 2 +- src/test/ui/privacy/private-inferred-type.stderr | 2 +- src/test/ui/regions/issue-28848.stderr | 2 +- .../generics-default-stability.stderr | 8 ++++---- .../ui/structs/struct-path-associated-type.stderr | 4 ++-- .../suggestions/mut-borrow-needed-by-trait.stderr | 2 +- .../suggest-std-when-using-type.stderr | 4 ++-- src/test/ui/traits/item-privacy.stderr | 2 +- src/test/ui/unspecified-self-in-trait-ref.stderr | 2 +- src/test/ui/wf/wf-static-method.stderr | 2 +- 22 files changed, 60 insertions(+), 30 deletions(-) diff --git a/compiler/rustc_ast/src/ast.rs b/compiler/rustc_ast/src/ast.rs index d6297addc0cef..61bc44f36909b 100644 --- a/compiler/rustc_ast/src/ast.rs +++ b/compiler/rustc_ast/src/ast.rs @@ -149,9 +149,17 @@ impl PathSegment { pub fn from_ident(ident: Ident) -> Self { PathSegment { ident, id: DUMMY_NODE_ID, args: None } } + pub fn path_root(span: Span) -> Self { PathSegment::from_ident(Ident::new(kw::PathRoot, span)) } + + pub fn span(&self) -> Span { + match &self.args { + Some(args) => self.ident.span.to(args.span()), + None => self.ident.span, + } + } } /// The arguments of a path segment. diff --git a/compiler/rustc_ast_lowering/src/path.rs b/compiler/rustc_ast_lowering/src/path.rs index cb4d5ea6ee650..46dac2f1af4f4 100644 --- a/compiler/rustc_ast_lowering/src/path.rs +++ b/compiler/rustc_ast_lowering/src/path.rs @@ -30,6 +30,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { let partial_res = self.resolver.get_partial_res(id).unwrap_or_else(|| PartialRes::new(Res::Err)); + let path_span_lo = p.span.shrink_to_lo(); let proj_start = p.segments.len() - partial_res.unresolved_segments(); let path = self.arena.alloc(hir::Path { res: self.lower_res(partial_res.base_res()), @@ -108,7 +109,9 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { ) }, )), - span: p.span, + span: p.segments[..proj_start] + .last() + .map_or(path_span_lo, |segment| path_span_lo.to(segment.span())), }); // Simple case, either no projections, or only fully-qualified. @@ -127,7 +130,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { // e.g., `Vec` in `Vec::new` or `::Item` in // `::Item::default`. let new_id = self.next_id(); - self.arena.alloc(self.ty_path(new_id, p.span, hir::QPath::Resolved(qself, path))) + self.arena.alloc(self.ty_path(new_id, path.span, hir::QPath::Resolved(qself, path))) }; // Anything after the base path are associated "extensions", @@ -141,7 +144,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { // 3. `<>::IntoIter>::Item` // * final path is `<<>::IntoIter>::Item>::clone` for (i, segment) in p.segments.iter().enumerate().skip(proj_start) { - let segment = self.arena.alloc(self.lower_path_segment( + let hir_segment = self.arena.alloc(self.lower_path_segment( p.span, segment, param_mode, @@ -150,7 +153,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { itctx.reborrow(), None, )); - let qpath = hir::QPath::TypeRelative(ty, segment); + let qpath = hir::QPath::TypeRelative(ty, hir_segment); // It's finished, return the extension of the right node type. if i == p.segments.len() - 1 { @@ -159,7 +162,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { // Wrap the associated extension in another type node. let new_id = self.next_id(); - ty = self.arena.alloc(self.ty_path(new_id, p.span, qpath)); + ty = self.arena.alloc(self.ty_path(new_id, path_span_lo.to(segment.span()), qpath)); } // We should've returned in the for loop above. diff --git a/compiler/rustc_hir/src/hir.rs b/compiler/rustc_hir/src/hir.rs index 8f61adcd8e288..20935231274f7 100644 --- a/compiler/rustc_hir/src/hir.rs +++ b/compiler/rustc_hir/src/hir.rs @@ -1809,7 +1809,7 @@ impl<'hir> QPath<'hir> { pub fn span(&self) -> Span { match *self { QPath::Resolved(_, path) => path.span, - QPath::TypeRelative(_, ps) => ps.ident.span, + QPath::TypeRelative(qself, ps) => qself.span.to(ps.ident.span), QPath::LangItem(_, span) => span, } } diff --git a/compiler/rustc_infer/src/traits/error_reporting/mod.rs b/compiler/rustc_infer/src/traits/error_reporting/mod.rs index 835f75ec8ef06..ad15af9ab3f2d 100644 --- a/compiler/rustc_infer/src/traits/error_reporting/mod.rs +++ b/compiler/rustc_infer/src/traits/error_reporting/mod.rs @@ -104,7 +104,7 @@ pub fn report_object_safety_error( ", ); - if tcx.sess.trait_methods_not_found.borrow().contains(&span) { + if tcx.sess.trait_methods_not_found.borrow().iter().any(|full_span| full_span.contains(span)) { // Avoid emitting error caused by non-existing method (#58734) err.cancel(); } diff --git a/compiler/rustc_typeck/src/astconv/mod.rs b/compiler/rustc_typeck/src/astconv/mod.rs index 947363fc3ed08..5a837cb0c30a5 100644 --- a/compiler/rustc_typeck/src/astconv/mod.rs +++ b/compiler/rustc_typeck/src/astconv/mod.rs @@ -1413,8 +1413,13 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o { name: Symbol, ) { let mut err = struct_span_err!(self.tcx().sess, span, E0223, "ambiguous associated type"); - if let (Some(_), Ok(snippet)) = ( - self.tcx().sess.confused_type_with_std_module.borrow().get(&span), + if let (true, Ok(snippet)) = ( + self.tcx() + .sess + .confused_type_with_std_module + .borrow() + .keys() + .any(|full_span| full_span.contains(span)), self.tcx().sess.source_map().span_to_snippet(span), ) { err.span_suggestion( diff --git a/compiler/rustc_typeck/src/check/fn_ctxt/checks.rs b/compiler/rustc_typeck/src/check/fn_ctxt/checks.rs index 5b8b25c210018..528a6d1bd52e2 100644 --- a/compiler/rustc_typeck/src/check/fn_ctxt/checks.rs +++ b/compiler/rustc_typeck/src/check/fn_ctxt/checks.rs @@ -439,7 +439,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { qpath: &QPath<'_>, hir_id: hir::HirId, ) -> Option<(&'tcx ty::VariantDef, Ty<'tcx>)> { - let path_span = qpath.qself_span(); + let path_span = qpath.span(); let (def, ty) = self.finish_resolving_struct_path(qpath, path_span, hir_id); let variant = match def { Res::Err => { diff --git a/src/test/ui/bad/bad-sized.rs b/src/test/ui/bad/bad-sized.rs index b899c59ff2ea7..a15219679788d 100644 --- a/src/test/ui/bad/bad-sized.rs +++ b/src/test/ui/bad/bad-sized.rs @@ -5,4 +5,5 @@ pub fn main() { //~^ ERROR only auto traits can be used as additional traits in a trait object //~| ERROR the size for values of type //~| ERROR the size for values of type + //~| ERROR the size for values of type } diff --git a/src/test/ui/bad/bad-sized.stderr b/src/test/ui/bad/bad-sized.stderr index 260d78b543a4c..768893d6e25d4 100644 --- a/src/test/ui/bad/bad-sized.stderr +++ b/src/test/ui/bad/bad-sized.stderr @@ -31,7 +31,20 @@ LL | let x: Vec = Vec::new(); = help: the trait `Sized` is not implemented for `dyn Trait` = note: required by `Vec::::new` -error: aborting due to 3 previous errors +error[E0277]: the size for values of type `dyn Trait` cannot be known at compilation time + --> $DIR/bad-sized.rs:4:37 + | +LL | let x: Vec = Vec::new(); + | ^^^ doesn't have a size known at compile-time + | + ::: $SRC_DIR/alloc/src/vec/mod.rs:LL:COL + | +LL | pub struct Vec { + | - required by this bound in `Vec` + | + = help: the trait `Sized` is not implemented for `dyn Trait` + +error: aborting due to 4 previous errors Some errors have detailed explanations: E0225, E0277. For more information about an error, try `rustc --explain E0225`. diff --git a/src/test/ui/conditional-compilation/cfg-attr-multi-true.stderr b/src/test/ui/conditional-compilation/cfg-attr-multi-true.stderr index 21b3a6f1f33b6..5f278f94b93bd 100644 --- a/src/test/ui/conditional-compilation/cfg-attr-multi-true.stderr +++ b/src/test/ui/conditional-compilation/cfg-attr-multi-true.stderr @@ -10,7 +10,7 @@ warning: use of deprecated struct `MustUseDeprecated` --> $DIR/cfg-attr-multi-true.rs:19:5 | LL | MustUseDeprecated::new(); - | ^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^^^^ warning: use of deprecated struct `MustUseDeprecated` --> $DIR/cfg-attr-multi-true.rs:13:17 diff --git a/src/test/ui/issues/issue-78622.stderr b/src/test/ui/issues/issue-78622.stderr index f13073da0a36e..f7d44f21d3bec 100644 --- a/src/test/ui/issues/issue-78622.stderr +++ b/src/test/ui/issues/issue-78622.stderr @@ -2,7 +2,7 @@ error[E0223]: ambiguous associated type --> $DIR/issue-78622.rs:5:5 | LL | S::A:: {} - | ^^^^^^^^^ help: use fully-qualified syntax: `::A` + | ^^^^ help: use fully-qualified syntax: `::A` error: aborting due to previous error diff --git a/src/test/ui/mir/issue-80742.stderr b/src/test/ui/mir/issue-80742.stderr index 8cbd0220e6768..8400aab308e06 100644 --- a/src/test/ui/mir/issue-80742.stderr +++ b/src/test/ui/mir/issue-80742.stderr @@ -56,7 +56,7 @@ LL | struct Inline | - required by this bound in `Inline` ... LL | let dst = Inline::::new(0); - | ^^^^^^^^^^^^^^^^^^^^^^^^ doesn't have a size known at compile-time + | ^^^^^^^^^^^^^^^^^^^ doesn't have a size known at compile-time | = help: the trait `Sized` is not implemented for `dyn Debug` help: consider relaxing the implicit `Sized` restriction diff --git a/src/test/ui/mismatched_types/issue-75361-mismatched-impl.stderr b/src/test/ui/mismatched_types/issue-75361-mismatched-impl.stderr index 4b86a1fede163..a64cb82305a48 100644 --- a/src/test/ui/mismatched_types/issue-75361-mismatched-impl.stderr +++ b/src/test/ui/mismatched_types/issue-75361-mismatched-impl.stderr @@ -13,7 +13,7 @@ help: the lifetime requirements from the `impl` do not correspond to the require --> $DIR/issue-75361-mismatched-impl.rs:12:55 | LL | fn adjacent_edges(&self) -> Box>; - | ^^^^^^^^^^^^^^ consider borrowing this type parameter in the trait + | ^^^^ consider borrowing this type parameter in the trait error: aborting due to previous error diff --git a/src/test/ui/privacy/associated-item-privacy-inherent.stderr b/src/test/ui/privacy/associated-item-privacy-inherent.stderr index 1e94e7c620d03..f8585014fd6d8 100644 --- a/src/test/ui/privacy/associated-item-privacy-inherent.stderr +++ b/src/test/ui/privacy/associated-item-privacy-inherent.stderr @@ -222,7 +222,7 @@ error: type `priv_parent_substs::Priv` is private --> $DIR/associated-item-privacy-inherent.rs:101:9 | LL | Pub::CONST; - | ^^^^^^^^^^ private type + | ^^^ private type ... LL | priv_parent_substs::mac!(); | --------------------------- in this macro invocation diff --git a/src/test/ui/privacy/private-inferred-type.stderr b/src/test/ui/privacy/private-inferred-type.stderr index 8c8163d3906b3..11bcb9074d097 100644 --- a/src/test/ui/privacy/private-inferred-type.stderr +++ b/src/test/ui/privacy/private-inferred-type.stderr @@ -56,7 +56,7 @@ error: type `Priv` is private --> $DIR/private-inferred-type.rs:104:5 | LL | m::Pub::INHERENT_ASSOC_CONST; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ private type + | ^^^^^^ private type error: type `Priv` is private --> $DIR/private-inferred-type.rs:105:5 diff --git a/src/test/ui/regions/issue-28848.stderr b/src/test/ui/regions/issue-28848.stderr index 726844a31841f..83313b34316b4 100644 --- a/src/test/ui/regions/issue-28848.stderr +++ b/src/test/ui/regions/issue-28848.stderr @@ -2,7 +2,7 @@ error[E0478]: lifetime bound not satisfied --> $DIR/issue-28848.rs:10:5 | LL | Foo::<'a, 'b>::xmute(u) - | ^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^ | note: lifetime parameter instantiated with the lifetime `'b` as defined on the function body at 9:16 --> $DIR/issue-28848.rs:9:16 diff --git a/src/test/ui/stability-attribute/generics-default-stability.stderr b/src/test/ui/stability-attribute/generics-default-stability.stderr index a5df70bb8b3dd..45194413cceec 100644 --- a/src/test/ui/stability-attribute/generics-default-stability.stderr +++ b/src/test/ui/stability-attribute/generics-default-stability.stderr @@ -100,7 +100,7 @@ warning: use of deprecated type alias `unstable_generic_param::Alias4`: test --> $DIR/generics-default-stability.rs:160:28 | LL | let _: Alias4 = Alias4::Some(1); - | ^^^^^^^^^^^^ + | ^^^^^^ warning: use of deprecated type alias `unstable_generic_param::Alias4`: test --> $DIR/generics-default-stability.rs:160:12 @@ -124,7 +124,7 @@ warning: use of deprecated type alias `unstable_generic_param::Alias4`: test --> $DIR/generics-default-stability.rs:166:28 | LL | let _: Alias4 = Alias4::Some(0); - | ^^^^^^^^^^^^ + | ^^^^^^ warning: use of deprecated type alias `unstable_generic_param::Alias4`: test --> $DIR/generics-default-stability.rs:166:12 @@ -136,7 +136,7 @@ warning: use of deprecated type alias `unstable_generic_param::Alias5`: test --> $DIR/generics-default-stability.rs:171:28 | LL | let _: Alias5 = Alias5::Some(1); - | ^^^^^^^^^^^^ + | ^^^^^^ warning: use of deprecated type alias `unstable_generic_param::Alias5`: test --> $DIR/generics-default-stability.rs:171:12 @@ -160,7 +160,7 @@ warning: use of deprecated type alias `unstable_generic_param::Alias5`: test --> $DIR/generics-default-stability.rs:178:28 | LL | let _: Alias5 = Alias5::Some(0); - | ^^^^^^^^^^^^ + | ^^^^^^ warning: use of deprecated type alias `unstable_generic_param::Alias5`: test --> $DIR/generics-default-stability.rs:178:12 diff --git a/src/test/ui/structs/struct-path-associated-type.stderr b/src/test/ui/structs/struct-path-associated-type.stderr index f8a2c7c6b6c20..0b1b6a5e3af28 100644 --- a/src/test/ui/structs/struct-path-associated-type.stderr +++ b/src/test/ui/structs/struct-path-associated-type.stderr @@ -14,7 +14,7 @@ error[E0071]: expected struct, variant or union type, found associated type --> $DIR/struct-path-associated-type.rs:14:13 | LL | let z = T::A:: {}; - | ^^^^^^^^^^ not a struct + | ^^^^ not a struct error[E0071]: expected struct, variant or union type, found associated type --> $DIR/struct-path-associated-type.rs:18:9 @@ -38,7 +38,7 @@ error[E0223]: ambiguous associated type --> $DIR/struct-path-associated-type.rs:33:13 | LL | let z = S::A:: {}; - | ^^^^^^^^^^ help: use fully-qualified syntax: `::A` + | ^^^^ help: use fully-qualified syntax: `::A` error[E0223]: ambiguous associated type --> $DIR/struct-path-associated-type.rs:35:9 diff --git a/src/test/ui/suggestions/mut-borrow-needed-by-trait.stderr b/src/test/ui/suggestions/mut-borrow-needed-by-trait.stderr index 3120b739c0295..b8ef230b44bb7 100644 --- a/src/test/ui/suggestions/mut-borrow-needed-by-trait.stderr +++ b/src/test/ui/suggestions/mut-borrow-needed-by-trait.stderr @@ -11,7 +11,7 @@ error[E0277]: the trait bound `&dyn std::io::Write: std::io::Write` is not satis --> $DIR/mut-borrow-needed-by-trait.rs:17:14 | LL | let fp = BufWriter::new(fp); - | ^^^^^^^^^^^^^^ the trait `std::io::Write` is not implemented for `&dyn std::io::Write` + | ^^^^^^^^^ the trait `std::io::Write` is not implemented for `&dyn std::io::Write` | ::: $SRC_DIR/std/src/io/buffered/bufwriter.rs:LL:COL | diff --git a/src/test/ui/suggestions/suggest-std-when-using-type.stderr b/src/test/ui/suggestions/suggest-std-when-using-type.stderr index 5199faa5c8ec6..7f4c80f50e267 100644 --- a/src/test/ui/suggestions/suggest-std-when-using-type.stderr +++ b/src/test/ui/suggestions/suggest-std-when-using-type.stderr @@ -2,12 +2,12 @@ error[E0223]: ambiguous associated type --> $DIR/suggest-std-when-using-type.rs:2:14 | LL | let pi = f32::consts::PI; - | ^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^ | help: you are looking for the module in `std`, not the primitive type | LL | let pi = std::f32::consts::PI; - | ^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^^^ error[E0599]: no function or associated item named `from_utf8` found for type `str` in the current scope --> $DIR/suggest-std-when-using-type.rs:5:14 diff --git a/src/test/ui/traits/item-privacy.stderr b/src/test/ui/traits/item-privacy.stderr index 6fd82142d61f7..b7dad54a6d3a9 100644 --- a/src/test/ui/traits/item-privacy.stderr +++ b/src/test/ui/traits/item-privacy.stderr @@ -113,7 +113,7 @@ error[E0038]: the trait `assoc_const::C` cannot be made into an object --> $DIR/item-privacy.rs:101:5 | LL | C::A; - | ^^^^ `assoc_const::C` cannot be made into an object + | ^ `assoc_const::C` cannot be made into an object | = help: consider moving `C` to another trait = help: consider moving `B` to another trait diff --git a/src/test/ui/unspecified-self-in-trait-ref.stderr b/src/test/ui/unspecified-self-in-trait-ref.stderr index 9310b3d7ede00..c9518170222c0 100644 --- a/src/test/ui/unspecified-self-in-trait-ref.stderr +++ b/src/test/ui/unspecified-self-in-trait-ref.stderr @@ -31,7 +31,7 @@ LL | | } | |_- type parameter `A` must be specified for this ... LL | let e = Bar::::lol(); - | ^^^^^^^^^^^^^^^^^ missing reference to `A` + | ^^^^^^^^^^^^ missing reference to `A` | = note: because of the default `Self` reference, type parameters must be specified on object types diff --git a/src/test/ui/wf/wf-static-method.stderr b/src/test/ui/wf/wf-static-method.stderr index 93d16514a5078..0c98a809025ac 100644 --- a/src/test/ui/wf/wf-static-method.stderr +++ b/src/test/ui/wf/wf-static-method.stderr @@ -19,7 +19,7 @@ error[E0478]: lifetime bound not satisfied --> $DIR/wf-static-method.rs:26:18 | LL | let me = Self::make_me(); - | ^^^^^^^^^^^^^ + | ^^^^ | note: lifetime parameter instantiated with the lifetime `'b` as defined on the impl at 23:10 --> $DIR/wf-static-method.rs:23:10 From 71a784d763f78d91ac88e34a72405dfbba17f336 Mon Sep 17 00:00:00 2001 From: Motoki Ikeda Date: Sat, 13 Mar 2021 00:51:26 +0900 Subject: [PATCH 09/13] Fix a typo in `swap_nonoverlapping_bytes` --- library/core/src/ptr/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/core/src/ptr/mod.rs b/library/core/src/ptr/mod.rs index 5026c48bdf4b4..5ac260fc883c2 100644 --- a/library/core/src/ptr/mod.rs +++ b/library/core/src/ptr/mod.rs @@ -512,7 +512,7 @@ unsafe fn swap_nonoverlapping_bytes(x: *mut u8, y: *mut u8, len: usize) { let t = t.as_mut_ptr() as *mut u8; // SAFETY: As `i < len`, and as the caller must guarantee that `x` and `y` are valid - // for `len` bytes, `x + i` and `y + i` must be valid adresses, which fulfills the + // for `len` bytes, `x + i` and `y + i` must be valid addresses, which fulfills the // safety contract for `add`. // // Also, the caller must guarantee that `x` and `y` are valid for writes, properly aligned, From 5ec0540da506fa9d3c0ca2aa3cca65055752e500 Mon Sep 17 00:00:00 2001 From: Motoki Ikeda Date: Sun, 14 Mar 2021 16:39:29 +0900 Subject: [PATCH 10/13] Fix a typo in thread_local_dtor.rs --- library/std/src/sys_common/thread_local_dtor.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/std/src/sys_common/thread_local_dtor.rs b/library/std/src/sys_common/thread_local_dtor.rs index 6f5ebf4a27158..f9971fb6f21ef 100644 --- a/library/std/src/sys_common/thread_local_dtor.rs +++ b/library/std/src/sys_common/thread_local_dtor.rs @@ -1,6 +1,6 @@ //! Thread-local destructor //! -//! Besides thread-local "keys" (pointer-sized non-adressable thread-local store +//! Besides thread-local "keys" (pointer-sized non-addressable thread-local store //! with an associated destructor), many platforms also provide thread-local //! destructors that are not associated with any particular data. These are //! often more efficient. From 6ddd840f36a3951fb1ef9c7649fab161500c8268 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=96mer=20Sinan=20A=C4=9Facan?= Date: Sun, 14 Mar 2021 16:00:02 +0300 Subject: [PATCH 11/13] Minor refactoring in try_index_step Merges `if-let` and `if x.is_some() { ... }` blocks --- compiler/rustc_typeck/src/check/place_op.rs | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/compiler/rustc_typeck/src/check/place_op.rs b/compiler/rustc_typeck/src/check/place_op.rs index 254e41706f90b..5bd385107ca39 100644 --- a/compiler/rustc_typeck/src/check/place_op.rs +++ b/compiler/rustc_typeck/src/check/place_op.rs @@ -103,9 +103,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { let method = self.try_overloaded_place_op(expr.span, self_ty, &[input_ty], PlaceOp::Index); - let result = method.map(|ok| { + if let Some(result) = method { debug!("try_index_step: success, using overloaded indexing"); - let method = self.register_infer_ok_obligations(ok); + let method = self.register_infer_ok_obligations(result); let mut adjustments = self.adjust_steps(autoderef); if let ty::Ref(region, _, hir::Mutability::Not) = method.sig.inputs()[0].kind() { @@ -128,10 +128,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { self.apply_adjustments(base_expr, adjustments); self.write_method_call(expr.hir_id, method); - (input_ty, self.make_overloaded_place_return_type(method).ty) - }); - if result.is_some() { - return result; + + return Some((input_ty, self.make_overloaded_place_return_type(method).ty)); } } From 14038c7df25d38b2f5b6790604374e1a33a01fe5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=96mer=20Sinan=20A=C4=9Facan?= Date: Sun, 14 Mar 2021 17:06:18 +0300 Subject: [PATCH 12/13] Remove duplicate asserts, replace eq assert with assert_eq --- compiler/rustc_mir_build/src/build/expr/into.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/compiler/rustc_mir_build/src/build/expr/into.rs b/compiler/rustc_mir_build/src/build/expr/into.rs index 47f75825fb6af..a2adbdddc40fe 100644 --- a/compiler/rustc_mir_build/src/build/expr/into.rs +++ b/compiler/rustc_mir_build/src/build/expr/into.rs @@ -427,7 +427,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { block.unit() } ExprKind::Index { .. } | ExprKind::Deref { .. } | ExprKind::Field { .. } => { - debug_assert!(Category::of(&expr.kind) == Some(Category::Place)); + debug_assert_eq!(Category::of(&expr.kind), Some(Category::Place)); // Create a "fake" temporary variable so that we check that the // value is Sized. Usually, this is caught in type checking, but @@ -436,8 +436,6 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { this.local_decls.push(LocalDecl::new(expr.ty, expr.span)); } - debug_assert!(Category::of(&expr.kind) == Some(Category::Place)); - let place = unpack!(block = this.as_place(block, expr)); let rvalue = Rvalue::Use(this.consume_by_copy_or_move(place)); this.cfg.push_assign(block, source_info, destination, rvalue); From a4cc3cae04525c7fd6edc8a4301a4034c82fdfad Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Sun, 21 Feb 2021 16:32:38 +0300 Subject: [PATCH 13/13] expand: Resolve and expand inner attributes on out-of-line modules --- Cargo.lock | 6 +- compiler/rustc_ast/src/ast.rs | 2 +- compiler/rustc_expand/src/expand.rs | 20 +++-- compiler/rustc_expand/src/module.rs | 58 +++++++++----- compiler/rustc_resolve/src/macros.rs | 27 ++++++- compiler/rustc_span/src/symbol.rs | 1 + .../inner-cfg-non-inline-mod.rs | 7 ++ .../module_with_cfg.rs | 3 + .../proc-macro/inner-attr-non-inline-mod.rs | 18 +++++ .../inner-attr-non-inline-mod.stderr | 40 ++++++++++ .../inner-attr-non-inline-mod.stdout | 76 +++++++++++++++++++ src/test/ui/proc-macro/module_with_attrs.rs | 4 + 12 files changed, 230 insertions(+), 32 deletions(-) create mode 100644 src/test/ui/conditional-compilation/inner-cfg-non-inline-mod.rs create mode 100644 src/test/ui/conditional-compilation/module_with_cfg.rs create mode 100644 src/test/ui/proc-macro/inner-attr-non-inline-mod.rs create mode 100644 src/test/ui/proc-macro/inner-attr-non-inline-mod.stderr create mode 100644 src/test/ui/proc-macro/inner-attr-non-inline-mod.stdout create mode 100644 src/test/ui/proc-macro/module_with_attrs.rs diff --git a/Cargo.lock b/Cargo.lock index 5ca91f42bdc6a..3c278a6a491a9 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4774,7 +4774,7 @@ version = "0.11.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "f301af10236f6df4160f7c3f04eec6dbc70ace82d23326abad5edee88801c6b6" dependencies = [ - "semver-parser 0.10.1", + "semver-parser 0.10.2", "serde", ] @@ -4786,9 +4786,9 @@ checksum = "388a1df253eca08550bef6c72392cfe7c30914bf41df5269b68cbd6ff8f570a3" [[package]] name = "semver-parser" -version = "0.10.1" +version = "0.10.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "42ef146c2ad5e5f4b037cd6ce2ebb775401729b19a82040c1beac9d36c7d1428" +checksum = "00b0bef5b7f9e0df16536d3961cfb6e84331c065b4066afb39768d0e319411f7" dependencies = [ "pest", ] diff --git a/compiler/rustc_ast/src/ast.rs b/compiler/rustc_ast/src/ast.rs index d6297addc0cef..a934bdd79801b 100644 --- a/compiler/rustc_ast/src/ast.rs +++ b/compiler/rustc_ast/src/ast.rs @@ -2297,7 +2297,7 @@ impl FnRetTy { } } -#[derive(Clone, PartialEq, Encodable, Decodable, Debug)] +#[derive(Clone, Copy, PartialEq, Encodable, Decodable, Debug)] pub enum Inline { Yes, No, diff --git a/compiler/rustc_expand/src/expand.rs b/compiler/rustc_expand/src/expand.rs index dd2eaa0f3d53c..a1e5979f62dcd 100644 --- a/compiler/rustc_expand/src/expand.rs +++ b/compiler/rustc_expand/src/expand.rs @@ -1282,16 +1282,13 @@ impl<'a, 'b> MutVisitor for InvocationCollector<'a, 'b> { let (file_path, dir_path, dir_ownership) = match mod_kind { ModKind::Loaded(_, inline, _) => { // Inline `mod foo { ... }`, but we still need to push directories. - assert!( - *inline == Inline::Yes, - "`mod` item is loaded from a file for the second time" - ); let (dir_path, dir_ownership) = mod_dir_path( &self.cx.sess, ident, &attrs, &self.cx.current_expansion.module, self.cx.current_expansion.dir_ownership, + *inline, ); item.attrs = attrs; (None, dir_path, dir_ownership) @@ -1322,10 +1319,19 @@ impl<'a, 'b> MutVisitor for InvocationCollector<'a, 'b> { item.attrs = attrs; if item.attrs.len() > old_attrs_len { // If we loaded an out-of-line module and added some inner attributes, - // then we need to re-configure it. - // FIXME: Attributes also need to be recollected - // for resolution and expansion. + // then we need to re-configure it and re-collect attributes for + // resolution and expansion. item = configure!(self, item); + + if let Some(attr) = self.take_first_attr(&mut item) { + return self + .collect_attr( + attr, + Annotatable::Item(item), + AstFragmentKind::Items, + ) + .make_items(); + } } (Some(file_path), dir_path, dir_ownership) } diff --git a/compiler/rustc_expand/src/module.rs b/compiler/rustc_expand/src/module.rs index 2ec656d4895e7..c5ce0baaa8f6a 100644 --- a/compiler/rustc_expand/src/module.rs +++ b/compiler/rustc_expand/src/module.rs @@ -1,6 +1,6 @@ use crate::base::ModuleData; use rustc_ast::ptr::P; -use rustc_ast::{token, Attribute, Item}; +use rustc_ast::{token, Attribute, Inline, Item}; use rustc_errors::{struct_span_err, DiagnosticBuilder}; use rustc_parse::new_parser_from_file; use rustc_session::parse::ParseSess; @@ -83,29 +83,49 @@ crate fn mod_dir_path( attrs: &[Attribute], module: &ModuleData, mut dir_ownership: DirOwnership, + inline: Inline, ) -> (PathBuf, DirOwnership) { - if let Some(file_path) = mod_file_path_from_attr(sess, attrs, &module.dir_path) { - // For inline modules file path from `#[path]` is actually the directory path - // for historical reasons, so we don't pop the last segment here. - return (file_path, DirOwnership::Owned { relative: None }); - } + match inline { + Inline::Yes => { + if let Some(file_path) = mod_file_path_from_attr(sess, attrs, &module.dir_path) { + // For inline modules file path from `#[path]` is actually the directory path + // for historical reasons, so we don't pop the last segment here. + return (file_path, DirOwnership::Owned { relative: None }); + } - // We have to push on the current module name in the case of relative - // paths in order to ensure that any additional module paths from inline - // `mod x { ... }` come after the relative extension. - // - // For example, a `mod z { ... }` inside `x/y.rs` should set the current - // directory path to `/x/y/z`, not `/x/z` with a relative offset of `y`. - let mut dir_path = module.dir_path.clone(); - if let DirOwnership::Owned { relative } = &mut dir_ownership { - if let Some(ident) = relative.take() { - // Remove the relative offset. + // We have to push on the current module name in the case of relative + // paths in order to ensure that any additional module paths from inline + // `mod x { ... }` come after the relative extension. + // + // For example, a `mod z { ... }` inside `x/y.rs` should set the current + // directory path to `/x/y/z`, not `/x/z` with a relative offset of `y`. + let mut dir_path = module.dir_path.clone(); + if let DirOwnership::Owned { relative } = &mut dir_ownership { + if let Some(ident) = relative.take() { + // Remove the relative offset. + dir_path.push(&*ident.as_str()); + } + } dir_path.push(&*ident.as_str()); + + (dir_path, dir_ownership) } - } - dir_path.push(&*ident.as_str()); + Inline::No => { + // FIXME: This is a subset of `parse_external_mod` without actual parsing, + // check whether the logic for unloaded, loaded and inline modules can be unified. + let file_path = mod_file_path(sess, ident, &attrs, &module.dir_path, dir_ownership) + .map(|mp| { + dir_ownership = mp.dir_ownership; + mp.file_path + }) + .unwrap_or_default(); + + // Extract the directory path for submodules of the module. + let dir_path = file_path.parent().unwrap_or(&file_path).to_owned(); - (dir_path, dir_ownership) + (dir_path, dir_ownership) + } + } } fn mod_file_path<'a>( diff --git a/compiler/rustc_resolve/src/macros.rs b/compiler/rustc_resolve/src/macros.rs index 2d149c476a60f..2e47d4cecee4c 100644 --- a/compiler/rustc_resolve/src/macros.rs +++ b/compiler/rustc_resolve/src/macros.rs @@ -6,7 +6,7 @@ use crate::Namespace::*; use crate::{AmbiguityError, AmbiguityErrorMisc, AmbiguityKind, BuiltinMacroState, Determinacy}; use crate::{CrateLint, ParentScope, ResolutionError, Resolver, Scope, ScopeSet, Weak}; use crate::{ModuleKind, ModuleOrUniformRoot, NameBinding, PathResult, Segment, ToNameBinding}; -use rustc_ast::{self as ast, NodeId}; +use rustc_ast::{self as ast, Inline, ItemKind, ModKind, NodeId}; use rustc_ast_lowering::ResolverAstLowering; use rustc_ast_pretty::pprust; use rustc_attr::StabilityLevel; @@ -14,6 +14,7 @@ use rustc_data_structures::fx::FxHashSet; use rustc_data_structures::ptr_key::PtrKey; use rustc_data_structures::sync::Lrc; use rustc_errors::struct_span_err; +use rustc_expand::base::Annotatable; use rustc_expand::base::{Indeterminate, ResolverExpand, SyntaxExtension, SyntaxExtensionKind}; use rustc_expand::compile_declarative_macro; use rustc_expand::expand::{AstFragment, Invocation, InvocationKind}; @@ -153,6 +154,26 @@ crate fn registered_attrs_and_tools( (registered_attrs, registered_tools) } +// Some feature gates for inner attributes are reported as lints for backward compatibility. +fn soft_custom_inner_attributes_gate(path: &ast::Path, invoc: &Invocation) -> bool { + match &path.segments[..] { + // `#![test]` + [seg] if seg.ident.name == sym::test => return true, + // `#![rustfmt::skip]` on out-of-line modules + [seg1, seg2] if seg1.ident.name == sym::rustfmt && seg2.ident.name == sym::skip => { + if let InvocationKind::Attr { item, .. } = &invoc.kind { + if let Annotatable::Item(item) = item { + if let ItemKind::Mod(_, ModKind::Loaded(_, Inline::No, _)) = item.kind { + return true; + } + } + } + } + _ => {} + } + false +} + impl<'a> ResolverExpand for Resolver<'a> { fn next_node_id(&mut self) -> NodeId { self.next_node_id() @@ -267,6 +288,7 @@ impl<'a> ResolverExpand for Resolver<'a> { parent_scope, node_id, force, + soft_custom_inner_attributes_gate(path, invoc), )?; let span = invoc.span(); @@ -440,6 +462,7 @@ impl<'a> Resolver<'a> { parent_scope: &ParentScope<'a>, node_id: NodeId, force: bool, + soft_custom_inner_attributes_gate: bool, ) -> Result<(Lrc, Res), Indeterminate> { let (ext, res) = match self.resolve_macro_path(path, Some(kind), parent_scope, true, force) { @@ -507,7 +530,7 @@ impl<'a> Resolver<'a> { Res::NonMacroAttr(..) => "custom inner attributes are unstable", _ => unreachable!(), }; - if path == &sym::test { + if soft_custom_inner_attributes_gate { self.session.parse_sess.buffer_lint(SOFT_UNSTABLE, path.span, node_id, msg); } else { feature_err(&self.session.parse_sess, sym::custom_inner_attributes, path.span, msg) diff --git a/compiler/rustc_span/src/symbol.rs b/compiler/rustc_span/src/symbol.rs index 79ca3c194cc82..4fcb8b6c1b7ab 100644 --- a/compiler/rustc_span/src/symbol.rs +++ b/compiler/rustc_span/src/symbol.rs @@ -1111,6 +1111,7 @@ symbols! { size_of, size_of_val, sized, + skip, slice, slice_alloc, slice_patterns, diff --git a/src/test/ui/conditional-compilation/inner-cfg-non-inline-mod.rs b/src/test/ui/conditional-compilation/inner-cfg-non-inline-mod.rs new file mode 100644 index 0000000000000..af5a6462e8a75 --- /dev/null +++ b/src/test/ui/conditional-compilation/inner-cfg-non-inline-mod.rs @@ -0,0 +1,7 @@ +// check-pass + +mod module_with_cfg; + +mod module_with_cfg {} // Ok, the module above is configured away by an inner attribute. + +fn main() {} diff --git a/src/test/ui/conditional-compilation/module_with_cfg.rs b/src/test/ui/conditional-compilation/module_with_cfg.rs new file mode 100644 index 0000000000000..56c4baadf22b4 --- /dev/null +++ b/src/test/ui/conditional-compilation/module_with_cfg.rs @@ -0,0 +1,3 @@ +// ignore-test + +#![cfg_attr(all(), cfg(FALSE))] diff --git a/src/test/ui/proc-macro/inner-attr-non-inline-mod.rs b/src/test/ui/proc-macro/inner-attr-non-inline-mod.rs new file mode 100644 index 0000000000000..30c2666df470c --- /dev/null +++ b/src/test/ui/proc-macro/inner-attr-non-inline-mod.rs @@ -0,0 +1,18 @@ +// compile-flags: -Z span-debug +// error-pattern:custom inner attributes are unstable +// error-pattern:inner macro attributes are unstable +// error-pattern:this was previously accepted +// aux-build:test-macros.rs + +#![no_std] // Don't load unnecessary hygiene information from std +extern crate std; + +#[macro_use] +extern crate test_macros; + +#[deny(unused_attributes)] +mod module_with_attrs; +//~^ ERROR non-inline modules in proc macro input are unstable +//~| ERROR custom inner attributes are unstable + +fn main() {} diff --git a/src/test/ui/proc-macro/inner-attr-non-inline-mod.stderr b/src/test/ui/proc-macro/inner-attr-non-inline-mod.stderr new file mode 100644 index 0000000000000..4286896dfc392 --- /dev/null +++ b/src/test/ui/proc-macro/inner-attr-non-inline-mod.stderr @@ -0,0 +1,40 @@ +error[E0658]: inner macro attributes are unstable + --> $DIR/module_with_attrs.rs:4:4 + | +LL | #![print_attr] + | ^^^^^^^^^^ + | + = note: see issue #54726 for more information + = help: add `#![feature(custom_inner_attributes)]` to the crate attributes to enable + +error[E0658]: non-inline modules in proc macro input are unstable + --> $DIR/inner-attr-non-inline-mod.rs:14:1 + | +LL | mod module_with_attrs; + | ^^^^^^^^^^^^^^^^^^^^^^ + | + = note: see issue #54727 for more information + = help: add `#![feature(proc_macro_hygiene)]` to the crate attributes to enable + +error[E0658]: custom inner attributes are unstable + --> $DIR/inner-attr-non-inline-mod.rs:14:1 + | +LL | mod module_with_attrs; + | ^^^^^^^^^^^^^^^^^^^^^^ + | + = note: see issue #54726 for more information + = help: add `#![feature(custom_inner_attributes)]` to the crate attributes to enable + +error: custom inner attributes are unstable + --> $DIR/module_with_attrs.rs:3:4 + | +LL | #![rustfmt::skip] + | ^^^^^^^^^^^^^ + | + = note: `#[deny(soft_unstable)]` on by default + = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! + = note: for more information, see issue #64266 + +error: aborting due to 4 previous errors + +For more information about this error, try `rustc --explain E0658`. diff --git a/src/test/ui/proc-macro/inner-attr-non-inline-mod.stdout b/src/test/ui/proc-macro/inner-attr-non-inline-mod.stdout new file mode 100644 index 0000000000000..dbef342ef241d --- /dev/null +++ b/src/test/ui/proc-macro/inner-attr-non-inline-mod.stdout @@ -0,0 +1,76 @@ +PRINT-ATTR INPUT (DISPLAY): #[deny(unused_attributes)] mod module_with_attrs { # ! [rustfmt :: skip] } +PRINT-ATTR INPUT (DEBUG): TokenStream [ + Punct { + ch: '#', + spacing: Alone, + span: $DIR/inner-attr-non-inline-mod.rs:14:1: 14:23 (#0), + }, + Group { + delimiter: Bracket, + stream: TokenStream [ + Ident { + ident: "deny", + span: $DIR/inner-attr-non-inline-mod.rs:14:1: 14:23 (#0), + }, + Group { + delimiter: Parenthesis, + stream: TokenStream [ + Ident { + ident: "unused_attributes", + span: $DIR/inner-attr-non-inline-mod.rs:14:1: 14:23 (#0), + }, + ], + span: $DIR/inner-attr-non-inline-mod.rs:14:1: 14:23 (#0), + }, + ], + span: $DIR/inner-attr-non-inline-mod.rs:14:1: 14:23 (#0), + }, + Ident { + ident: "mod", + span: $DIR/inner-attr-non-inline-mod.rs:14:1: 14:23 (#0), + }, + Ident { + ident: "module_with_attrs", + span: $DIR/inner-attr-non-inline-mod.rs:14:1: 14:23 (#0), + }, + Group { + delimiter: Brace, + stream: TokenStream [ + Punct { + ch: '#', + spacing: Joint, + span: $DIR/inner-attr-non-inline-mod.rs:14:1: 14:23 (#0), + }, + Punct { + ch: '!', + spacing: Alone, + span: $DIR/inner-attr-non-inline-mod.rs:14:1: 14:23 (#0), + }, + Group { + delimiter: Bracket, + stream: TokenStream [ + Ident { + ident: "rustfmt", + span: $DIR/inner-attr-non-inline-mod.rs:14:1: 14:23 (#0), + }, + Punct { + ch: ':', + spacing: Joint, + span: $DIR/inner-attr-non-inline-mod.rs:14:1: 14:23 (#0), + }, + Punct { + ch: ':', + spacing: Alone, + span: $DIR/inner-attr-non-inline-mod.rs:14:1: 14:23 (#0), + }, + Ident { + ident: "skip", + span: $DIR/inner-attr-non-inline-mod.rs:14:1: 14:23 (#0), + }, + ], + span: $DIR/inner-attr-non-inline-mod.rs:14:1: 14:23 (#0), + }, + ], + span: $DIR/inner-attr-non-inline-mod.rs:14:1: 14:23 (#0), + }, +] diff --git a/src/test/ui/proc-macro/module_with_attrs.rs b/src/test/ui/proc-macro/module_with_attrs.rs new file mode 100644 index 0000000000000..63e66a62ac380 --- /dev/null +++ b/src/test/ui/proc-macro/module_with_attrs.rs @@ -0,0 +1,4 @@ +// ignore-test + +#![rustfmt::skip] +#![print_attr]