From 920435f195b49029d0a83679c04d7dda66afb50a Mon Sep 17 00:00:00 2001 From: Chris Denton Date: Fri, 2 Dec 2022 14:07:58 +0000 Subject: [PATCH 01/16] Windows: make Command prefer non-verbatim paths When spawning Commands, the path we use can end up being queried using `env::current_exe` (or the equivalent in other languages). Not all applications handle these paths properly therefore we should have a stronger preference for non-verbatim paths when spawning processes. --- library/std/src/sys/windows/args.rs | 23 ++++++--- library/std/src/sys/windows/path.rs | 65 ++++++++++++++++---------- library/std/src/sys/windows/process.rs | 12 ++--- 3 files changed, 62 insertions(+), 38 deletions(-) diff --git a/library/std/src/sys/windows/args.rs b/library/std/src/sys/windows/args.rs index 6741ae46d32dd..4972416154918 100644 --- a/library/std/src/sys/windows/args.rs +++ b/library/std/src/sys/windows/args.rs @@ -11,10 +11,11 @@ use crate::fmt; use crate::io; use crate::num::NonZeroU16; use crate::os::windows::prelude::*; -use crate::path::PathBuf; -use crate::sys::c; +use crate::path::{Path, PathBuf}; +use crate::sys::path::get_long_path; use crate::sys::process::ensure_no_nuls; use crate::sys::windows::os::current_exe; +use crate::sys::{c, to_u16s}; use crate::sys_common::wstr::WStrUnits; use crate::vec; @@ -302,7 +303,7 @@ pub(crate) fn make_bat_command_line( /// Takes a path and tries to return a non-verbatim path. /// /// This is necessary because cmd.exe does not support verbatim paths. -pub(crate) fn to_user_path(mut path: Vec) -> io::Result> { +pub(crate) fn to_user_path(path: &Path) -> io::Result> { use crate::ptr; use crate::sys::windows::fill_utf16_buf; @@ -315,6 +316,8 @@ pub(crate) fn to_user_path(mut path: Vec) -> io::Result> { const N: u16 = b'N' as _; const C: u16 = b'C' as _; + let mut path = to_u16s(path)?; + // Early return if the path is too long to remove the verbatim prefix. const LEGACY_MAX_PATH: usize = 260; if path.len() > LEGACY_MAX_PATH { @@ -328,7 +331,13 @@ pub(crate) fn to_user_path(mut path: Vec) -> io::Result> { fill_utf16_buf( |buffer, size| c::GetFullPathNameW(lpfilename, size, buffer, ptr::null_mut()), |full_path: &[u16]| { - if full_path == &path[4..path.len() - 1] { full_path.into() } else { path } + if full_path == &path[4..path.len() - 1] { + let mut path: Vec = full_path.into(); + path.push(0); + path + } else { + path + } }, ) }, @@ -341,7 +350,9 @@ pub(crate) fn to_user_path(mut path: Vec) -> io::Result> { |buffer, size| c::GetFullPathNameW(lpfilename, size, buffer, ptr::null_mut()), |full_path: &[u16]| { if full_path == &path[6..path.len() - 1] { - full_path.into() + let mut path: Vec = full_path.into(); + path.push(0); + path } else { // Restore the 'C' in "UNC". path[6] = b'C' as u16; @@ -351,6 +362,6 @@ pub(crate) fn to_user_path(mut path: Vec) -> io::Result> { ) }, // For everything else, leave the path unchanged. - _ => Ok(path), + _ => get_long_path(path, false), } } diff --git a/library/std/src/sys/windows/path.rs b/library/std/src/sys/windows/path.rs index beeca1917a9af..c3573d14c7f92 100644 --- a/library/std/src/sys/windows/path.rs +++ b/library/std/src/sys/windows/path.rs @@ -220,6 +220,19 @@ fn parse_next_component(path: &OsStr, verbatim: bool) -> (&OsStr, &OsStr) { /// /// This path may or may not have a verbatim prefix. pub(crate) fn maybe_verbatim(path: &Path) -> io::Result> { + let path = to_u16s(path)?; + get_long_path(path, true) +} + +/// Get a normalized absolute path that can bypass path length limits. +/// +/// Setting prefer_verbatim to true suggests a stronger preference for verbatim +/// paths even when not strictly necessary. This allows the Windows API to avoid +/// repeating our work. However, if the path may be given back to users or +/// passed to other application then it's preferable to use non-verbatim paths +/// when possible. Non-verbatim paths are better understood by users and handled +/// by more software. +pub(crate) fn get_long_path(mut path: Vec, prefer_verbatim: bool) -> io::Result> { // Normally the MAX_PATH is 260 UTF-16 code units (including the NULL). // However, for APIs such as CreateDirectory[1], the limit is 248. // @@ -243,7 +256,6 @@ pub(crate) fn maybe_verbatim(path: &Path) -> io::Result> { // \\?\UNC\ const UNC_PREFIX: &[u16] = &[SEP, SEP, QUERY, SEP, U, N, C, SEP]; - let mut path = to_u16s(path)?; if path.starts_with(VERBATIM_PREFIX) || path.starts_with(NT_PREFIX) || path == &[0] { // Early return for paths that are already verbatim or empty. return Ok(path); @@ -275,29 +287,34 @@ pub(crate) fn maybe_verbatim(path: &Path) -> io::Result> { |mut absolute| { path.clear(); - // Secondly, add the verbatim prefix. This is easier here because we know the - // path is now absolute and fully normalized (e.g. `/` has been changed to `\`). - let prefix = match absolute { - // C:\ => \\?\C:\ - [_, COLON, SEP, ..] => VERBATIM_PREFIX, - // \\.\ => \\?\ - [SEP, SEP, DOT, SEP, ..] => { - absolute = &absolute[4..]; - VERBATIM_PREFIX - } - // Leave \\?\ and \??\ as-is. - [SEP, SEP, QUERY, SEP, ..] | [SEP, QUERY, QUERY, SEP, ..] => &[], - // \\ => \\?\UNC\ - [SEP, SEP, ..] => { - absolute = &absolute[2..]; - UNC_PREFIX - } - // Anything else we leave alone. - _ => &[], - }; - - path.reserve_exact(prefix.len() + absolute.len() + 1); - path.extend_from_slice(prefix); + // Only prepend the prefix if needed. + if prefer_verbatim || absolute.len() + 1 >= LEGACY_MAX_PATH { + // Secondly, add the verbatim prefix. This is easier here because we know the + // path is now absolute and fully normalized (e.g. `/` has been changed to `\`). + let prefix = match absolute { + // C:\ => \\?\C:\ + [_, COLON, SEP, ..] => VERBATIM_PREFIX, + // \\.\ => \\?\ + [SEP, SEP, DOT, SEP, ..] => { + absolute = &absolute[4..]; + VERBATIM_PREFIX + } + // Leave \\?\ and \??\ as-is. + [SEP, SEP, QUERY, SEP, ..] | [SEP, QUERY, QUERY, SEP, ..] => &[], + // \\ => \\?\UNC\ + [SEP, SEP, ..] => { + absolute = &absolute[2..]; + UNC_PREFIX + } + // Anything else we leave alone. + _ => &[], + }; + + path.reserve_exact(prefix.len() + absolute.len() + 1); + path.extend_from_slice(prefix); + } else { + path.reserve_exact(absolute.len() + 1); + } path.extend_from_slice(absolute); path.push(0); }, diff --git a/library/std/src/sys/windows/process.rs b/library/std/src/sys/windows/process.rs index 9cbb4ef19e9b7..82b3f6acde87b 100644 --- a/library/std/src/sys/windows/process.rs +++ b/library/std/src/sys/windows/process.rs @@ -270,11 +270,7 @@ impl Command { let (program, mut cmd_str) = if is_batch_file { ( command_prompt()?, - args::make_bat_command_line( - &args::to_user_path(program)?, - &self.args, - self.force_quotes_enabled, - )?, + args::make_bat_command_line(&program, &self.args, self.force_quotes_enabled)?, ) } else { let cmd_str = make_command_line(&self.program, &self.args, self.force_quotes_enabled)?; @@ -397,7 +393,7 @@ fn resolve_exe<'a>( if has_exe_suffix { // The application name is a path to a `.exe` file. // Let `CreateProcessW` figure out if it exists or not. - return path::maybe_verbatim(Path::new(exe_path)); + return args::to_user_path(Path::new(exe_path)); } let mut path = PathBuf::from(exe_path); @@ -409,7 +405,7 @@ fn resolve_exe<'a>( // It's ok to use `set_extension` here because the intent is to // remove the extension that was just added. path.set_extension(""); - return path::maybe_verbatim(&path); + return args::to_user_path(&path); } } else { ensure_no_nuls(exe_path)?; @@ -497,7 +493,7 @@ where /// Check if a file exists without following symlinks. fn program_exists(path: &Path) -> Option> { unsafe { - let path = path::maybe_verbatim(path).ok()?; + let path = args::to_user_path(path).ok()?; // Getting attributes using `GetFileAttributesW` does not follow symlinks // and it will almost always be successful if the link exists. // There are some exceptions for special system files (e.g. the pagefile) From 746331edf3a6d055859ed0ed70dde1aa883c517d Mon Sep 17 00:00:00 2001 From: joboet Date: Fri, 17 Feb 2023 15:47:58 +0100 Subject: [PATCH 02/16] std: drop all messages in bounded channel when destroying the last receiver --- library/std/src/sync/mpmc/array.rs | 132 +++++++++++++++++++++++------ library/std/src/sync/mpmc/mod.rs | 4 +- 2 files changed, 109 insertions(+), 27 deletions(-) diff --git a/library/std/src/sync/mpmc/array.rs b/library/std/src/sync/mpmc/array.rs index c6bb09b0417f3..7038176000362 100644 --- a/library/std/src/sync/mpmc/array.rs +++ b/library/std/src/sync/mpmc/array.rs @@ -15,7 +15,7 @@ use super::utils::{Backoff, CachePadded}; use super::waker::SyncWaker; use crate::cell::UnsafeCell; -use crate::mem::MaybeUninit; +use crate::mem::{self, MaybeUninit}; use crate::ptr; use crate::sync::atomic::{self, AtomicUsize, Ordering}; use crate::time::Instant; @@ -25,7 +25,8 @@ struct Slot { /// The current stamp. stamp: AtomicUsize, - /// The message in this slot. + /// The message in this slot. Either read out in `read` or dropped through + /// `discard_all_messages`. msg: UnsafeCell>, } @@ -439,14 +440,13 @@ impl Channel { Some(self.cap) } - /// Disconnects the channel and wakes up all blocked senders and receivers. + /// Disconnects senders and wakes up all blocked receivers. /// /// Returns `true` if this call disconnected the channel. - pub(crate) fn disconnect(&self) -> bool { + pub(crate) fn disconnect_senders(&self) -> bool { let tail = self.tail.fetch_or(self.mark_bit, Ordering::SeqCst); if tail & self.mark_bit == 0 { - self.senders.disconnect(); self.receivers.disconnect(); true } else { @@ -454,6 +454,108 @@ impl Channel { } } + /// Disconnects receivers and wakes up all blocked senders. + /// + /// Returns `true` if this call disconnected the channel. + /// + /// # Safety + /// May only be called once upon dropping the last receiver. The + /// destruction of all other receivers must have been observed with acquire + /// ordering or stronger. + pub(crate) unsafe fn disconnect_receivers(&self) -> bool { + let tail = self.tail.fetch_or(self.mark_bit, Ordering::SeqCst); + self.discard_all_messages(tail); + + if tail & self.mark_bit == 0 { + self.senders.disconnect(); + true + } else { + false + } + } + + /// Discards all messages. + /// + /// `tail` should be the current (and therefore last) value of `tail`. + /// + /// # Safety + /// This method must only be called when dropping the last receiver. The + /// destruction of all other receivers must have been observed with acquire + /// ordering or stronger. + unsafe fn discard_all_messages(&self, tail: usize) { + debug_assert!(self.is_disconnected()); + + /// Use a helper struct with a custom `Drop` to ensure all messages are + /// dropped, even if a destructor panicks. + struct DiscardState<'a, T> { + channel: &'a Channel, + head: usize, + tail: usize, + backoff: Backoff, + } + + impl<'a, T> DiscardState<'a, T> { + fn discard(&mut self) { + loop { + // Deconstruct the head. + let index = self.head & (self.channel.mark_bit - 1); + let lap = self.head & !(self.channel.one_lap - 1); + + // Inspect the corresponding slot. + debug_assert!(index < self.channel.buffer.len()); + let slot = unsafe { self.channel.buffer.get_unchecked(index) }; + let stamp = slot.stamp.load(Ordering::Acquire); + + // If the stamp is ahead of the head by 1, we may drop the message. + if self.head + 1 == stamp { + self.head = if index + 1 < self.channel.cap { + // Same lap, incremented index. + // Set to `{ lap: lap, mark: 0, index: index + 1 }`. + self.head + 1 + } else { + // One lap forward, index wraps around to zero. + // Set to `{ lap: lap.wrapping_add(1), mark: 0, index: 0 }`. + lap.wrapping_add(self.channel.one_lap) + }; + + // We updated the head, so even if this descrutor panics, + // we will not attempt to destroy the slot again. + unsafe { + (*slot.msg.get()).assume_init_drop(); + } + // If the tail equals the head, that means the channel is empty. + } else if self.tail == self.head { + return; + // Otherwise, a sender is about to write into the slot, so we need + // to wait for it to update the stamp. + } else { + self.backoff.spin_heavy(); + } + } + } + } + + impl<'a, T> Drop for DiscardState<'a, T> { + fn drop(&mut self) { + self.discard(); + } + } + + let mut state = DiscardState { + channel: self, + // Only receivers modify `head`, so since we are the last one, + // this value will not change and will not be observed (since + // no new messages can be sent after disconnection). + head: self.head.load(Ordering::Relaxed), + tail: tail & !self.mark_bit, + backoff: Backoff::new(), + }; + state.discard(); + // This point is only reached if no destructor panics, so all messages + // have already been dropped. + mem::forget(state); + } + /// Returns `true` if the channel is disconnected. pub(crate) fn is_disconnected(&self) -> bool { self.tail.load(Ordering::SeqCst) & self.mark_bit != 0 @@ -483,23 +585,3 @@ impl Channel { head.wrapping_add(self.one_lap) == tail & !self.mark_bit } } - -impl Drop for Channel { - fn drop(&mut self) { - // Get the index of the head. - let hix = self.head.load(Ordering::Relaxed) & (self.mark_bit - 1); - - // Loop over all slots that hold a message and drop them. - for i in 0..self.len() { - // Compute the index of the next slot holding a message. - let index = if hix + i < self.cap { hix + i } else { hix + i - self.cap }; - - unsafe { - debug_assert!(index < self.buffer.len()); - let slot = self.buffer.get_unchecked_mut(index); - let msg = &mut *slot.msg.get(); - msg.as_mut_ptr().drop_in_place(); - } - } - } -} diff --git a/library/std/src/sync/mpmc/mod.rs b/library/std/src/sync/mpmc/mod.rs index 7a602cecd3b89..2068dda393a2b 100644 --- a/library/std/src/sync/mpmc/mod.rs +++ b/library/std/src/sync/mpmc/mod.rs @@ -227,7 +227,7 @@ impl Drop for Sender { fn drop(&mut self) { unsafe { match &self.flavor { - SenderFlavor::Array(chan) => chan.release(|c| c.disconnect()), + SenderFlavor::Array(chan) => chan.release(|c| c.disconnect_senders()), SenderFlavor::List(chan) => chan.release(|c| c.disconnect_senders()), SenderFlavor::Zero(chan) => chan.release(|c| c.disconnect()), } @@ -403,7 +403,7 @@ impl Drop for Receiver { fn drop(&mut self) { unsafe { match &self.flavor { - ReceiverFlavor::Array(chan) => chan.release(|c| c.disconnect()), + ReceiverFlavor::Array(chan) => chan.release(|c| c.disconnect_receivers()), ReceiverFlavor::List(chan) => chan.release(|c| c.disconnect_receivers()), ReceiverFlavor::Zero(chan) => chan.release(|c| c.disconnect()), } From 642a3247462a582ee97abea1c06c3fabac3bcb3f Mon Sep 17 00:00:00 2001 From: joboet Date: Fri, 17 Feb 2023 15:58:15 +0100 Subject: [PATCH 03/16] std: add regression test for #107466 Tests that messages are immediately dropped once the last receiver is destroyed. --- library/std/src/sync/mpsc/sync_tests.rs | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/library/std/src/sync/mpsc/sync_tests.rs b/library/std/src/sync/mpsc/sync_tests.rs index 9d2f92ffc9b14..632709fd98d86 100644 --- a/library/std/src/sync/mpsc/sync_tests.rs +++ b/library/std/src/sync/mpsc/sync_tests.rs @@ -1,5 +1,6 @@ use super::*; use crate::env; +use crate::rc::Rc; use crate::sync::mpmc::SendTimeoutError; use crate::thread; use crate::time::Duration; @@ -656,3 +657,15 @@ fn issue_15761() { repro() } } + +#[test] +fn drop_unreceived() { + let (tx, rx) = sync_channel::>(1); + let msg = Rc::new(()); + let weak = Rc::downgrade(&msg); + assert!(tx.send(msg).is_ok()); + drop(rx); + // Messages should be dropped immediately when the last receiver is destroyed. + assert!(weak.upgrade().is_none()); + drop(tx); +} From 4e9e465bd4cbdfe3946ea6f0ff4786f2f495a020 Mon Sep 17 00:00:00 2001 From: joboet Date: Sun, 26 Feb 2023 11:57:27 +0100 Subject: [PATCH 04/16] std: disconnect senders before discarding messages --- library/std/src/sync/mpmc/array.rs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/library/std/src/sync/mpmc/array.rs b/library/std/src/sync/mpmc/array.rs index 7038176000362..fb893695a9a58 100644 --- a/library/std/src/sync/mpmc/array.rs +++ b/library/std/src/sync/mpmc/array.rs @@ -464,14 +464,15 @@ impl Channel { /// ordering or stronger. pub(crate) unsafe fn disconnect_receivers(&self) -> bool { let tail = self.tail.fetch_or(self.mark_bit, Ordering::SeqCst); - self.discard_all_messages(tail); - - if tail & self.mark_bit == 0 { + let disconnected = if tail & self.mark_bit == 0 { self.senders.disconnect(); true } else { false - } + }; + + self.discard_all_messages(tail); + disconnected } /// Discards all messages. From e4ab642e8ce42324798f0ba0e05e139221c2da6d Mon Sep 17 00:00:00 2001 From: Nilstrieb <48135649+Nilstrieb@users.noreply.github.com> Date: Mon, 13 Mar 2023 19:23:11 +0000 Subject: [PATCH 05/16] Use match instead of if in codegen_attrs --- .../rustc_codegen_ssa/src/codegen_attrs.rs | 584 +++++++++--------- 1 file changed, 309 insertions(+), 275 deletions(-) diff --git a/compiler/rustc_codegen_ssa/src/codegen_attrs.rs b/compiler/rustc_codegen_ssa/src/codegen_attrs.rs index f9bb8359208e3..00eab8d7eb690 100644 --- a/compiler/rustc_codegen_ssa/src/codegen_attrs.rs +++ b/compiler/rustc_codegen_ssa/src/codegen_attrs.rs @@ -10,6 +10,7 @@ use rustc_middle::mir::mono::Linkage; use rustc_middle::ty::query::Providers; use rustc_middle::ty::{self as ty, TyCtxt}; use rustc_session::{lint, parse::feature_err}; +use rustc_span::symbol::Ident; use rustc_span::{sym, Span}; use rustc_target::spec::{abi, SanitizerSet}; @@ -84,96 +85,115 @@ fn codegen_fn_attrs(tcx: TyCtxt<'_>, did: DefId) -> CodegenFnAttrs { } }; - if attr.has_name(sym::cold) { - codegen_fn_attrs.flags |= CodegenFnAttrFlags::COLD; - } else if attr.has_name(sym::rustc_allocator) { - codegen_fn_attrs.flags |= CodegenFnAttrFlags::ALLOCATOR; - } else if attr.has_name(sym::ffi_returns_twice) { - codegen_fn_attrs.flags |= CodegenFnAttrFlags::FFI_RETURNS_TWICE; - } else if attr.has_name(sym::ffi_pure) { - codegen_fn_attrs.flags |= CodegenFnAttrFlags::FFI_PURE; - } else if attr.has_name(sym::ffi_const) { - codegen_fn_attrs.flags |= CodegenFnAttrFlags::FFI_CONST; - } else if attr.has_name(sym::rustc_nounwind) { - codegen_fn_attrs.flags |= CodegenFnAttrFlags::NEVER_UNWIND; - } else if attr.has_name(sym::rustc_reallocator) { - codegen_fn_attrs.flags |= CodegenFnAttrFlags::REALLOCATOR; - } else if attr.has_name(sym::rustc_deallocator) { - codegen_fn_attrs.flags |= CodegenFnAttrFlags::DEALLOCATOR; - } else if attr.has_name(sym::rustc_allocator_zeroed) { - codegen_fn_attrs.flags |= CodegenFnAttrFlags::ALLOCATOR_ZEROED; - } else if attr.has_name(sym::naked) { - codegen_fn_attrs.flags |= CodegenFnAttrFlags::NAKED; - } else if attr.has_name(sym::no_mangle) { - codegen_fn_attrs.flags |= CodegenFnAttrFlags::NO_MANGLE; - } else if attr.has_name(sym::no_coverage) { - codegen_fn_attrs.flags |= CodegenFnAttrFlags::NO_COVERAGE; - } else if attr.has_name(sym::rustc_std_internal_symbol) { - codegen_fn_attrs.flags |= CodegenFnAttrFlags::RUSTC_STD_INTERNAL_SYMBOL; - } else if attr.has_name(sym::used) { - let inner = attr.meta_item_list(); - match inner.as_deref() { - Some([item]) if item.has_name(sym::linker) => { - if !tcx.features().used_with_arg { - feature_err( - &tcx.sess.parse_sess, - sym::used_with_arg, - attr.span, - "`#[used(linker)]` is currently unstable", - ) - .emit(); + let Some(Ident { name, .. }) = attr.ident() else { + continue; + }; + + match name { + sym::cold => { + codegen_fn_attrs.flags |= CodegenFnAttrFlags::COLD; + } + sym::rustc_allocator => { + codegen_fn_attrs.flags |= CodegenFnAttrFlags::ALLOCATOR; + } + sym::ffi_returns_twice => { + codegen_fn_attrs.flags |= CodegenFnAttrFlags::FFI_RETURNS_TWICE; + } + sym::ffi_pure => { + codegen_fn_attrs.flags |= CodegenFnAttrFlags::FFI_PURE; + } + sym::ffi_const => { + codegen_fn_attrs.flags |= CodegenFnAttrFlags::FFI_CONST; + } + sym::rustc_nounwind => { + codegen_fn_attrs.flags |= CodegenFnAttrFlags::NEVER_UNWIND; + } + sym::rustc_reallocator => { + codegen_fn_attrs.flags |= CodegenFnAttrFlags::REALLOCATOR; + } + sym::rustc_deallocator => { + codegen_fn_attrs.flags |= CodegenFnAttrFlags::DEALLOCATOR; + } + sym::rustc_allocator_zeroed => { + codegen_fn_attrs.flags |= CodegenFnAttrFlags::ALLOCATOR_ZEROED; + } + sym::naked => { + codegen_fn_attrs.flags |= CodegenFnAttrFlags::NAKED; + } + sym::no_mangle => { + codegen_fn_attrs.flags |= CodegenFnAttrFlags::NO_MANGLE; + } + sym::no_coverage => { + codegen_fn_attrs.flags |= CodegenFnAttrFlags::NO_COVERAGE; + } + sym::rustc_std_internal_symbol => { + codegen_fn_attrs.flags |= CodegenFnAttrFlags::RUSTC_STD_INTERNAL_SYMBOL; + } + sym::used => { + let inner = attr.meta_item_list(); + match inner.as_deref() { + Some([item]) if item.has_name(sym::linker) => { + if !tcx.features().used_with_arg { + feature_err( + &tcx.sess.parse_sess, + sym::used_with_arg, + attr.span, + "`#[used(linker)]` is currently unstable", + ) + .emit(); + } + codegen_fn_attrs.flags |= CodegenFnAttrFlags::USED_LINKER; } - codegen_fn_attrs.flags |= CodegenFnAttrFlags::USED_LINKER; - } - Some([item]) if item.has_name(sym::compiler) => { - if !tcx.features().used_with_arg { - feature_err( - &tcx.sess.parse_sess, - sym::used_with_arg, - attr.span, - "`#[used(compiler)]` is currently unstable", - ) - .emit(); + Some([item]) if item.has_name(sym::compiler) => { + if !tcx.features().used_with_arg { + feature_err( + &tcx.sess.parse_sess, + sym::used_with_arg, + attr.span, + "`#[used(compiler)]` is currently unstable", + ) + .emit(); + } + codegen_fn_attrs.flags |= CodegenFnAttrFlags::USED; + } + Some(_) => { + tcx.sess.emit_err(ExpectedUsedSymbol { span: attr.span }); + } + None => { + // Unfortunately, unconditionally using `llvm.used` causes + // issues in handling `.init_array` with the gold linker, + // but using `llvm.compiler.used` caused a nontrival amount + // of unintentional ecosystem breakage -- particularly on + // Mach-O targets. + // + // As a result, we emit `llvm.compiler.used` only on ELF + // targets. This is somewhat ad-hoc, but actually follows + // our pre-LLVM 13 behavior (prior to the ecosystem + // breakage), and seems to match `clang`'s behavior as well + // (both before and after LLVM 13), possibly because they + // have similar compatibility concerns to us. See + // https://github.com/rust-lang/rust/issues/47384#issuecomment-1019080146 + // and following comments for some discussion of this, as + // well as the comments in `rustc_codegen_llvm` where these + // flags are handled. + // + // Anyway, to be clear: this is still up in the air + // somewhat, and is subject to change in the future (which + // is a good thing, because this would ideally be a bit + // more firmed up). + let is_like_elf = !(tcx.sess.target.is_like_osx + || tcx.sess.target.is_like_windows + || tcx.sess.target.is_like_wasm); + codegen_fn_attrs.flags |= if is_like_elf { + CodegenFnAttrFlags::USED + } else { + CodegenFnAttrFlags::USED_LINKER + }; } - codegen_fn_attrs.flags |= CodegenFnAttrFlags::USED; - } - Some(_) => { - tcx.sess.emit_err(ExpectedUsedSymbol { span: attr.span }); - } - None => { - // Unfortunately, unconditionally using `llvm.used` causes - // issues in handling `.init_array` with the gold linker, - // but using `llvm.compiler.used` caused a nontrival amount - // of unintentional ecosystem breakage -- particularly on - // Mach-O targets. - // - // As a result, we emit `llvm.compiler.used` only on ELF - // targets. This is somewhat ad-hoc, but actually follows - // our pre-LLVM 13 behavior (prior to the ecosystem - // breakage), and seems to match `clang`'s behavior as well - // (both before and after LLVM 13), possibly because they - // have similar compatibility concerns to us. See - // https://github.com/rust-lang/rust/issues/47384#issuecomment-1019080146 - // and following comments for some discussion of this, as - // well as the comments in `rustc_codegen_llvm` where these - // flags are handled. - // - // Anyway, to be clear: this is still up in the air - // somewhat, and is subject to change in the future (which - // is a good thing, because this would ideally be a bit - // more firmed up). - let is_like_elf = !(tcx.sess.target.is_like_osx - || tcx.sess.target.is_like_windows - || tcx.sess.target.is_like_wasm); - codegen_fn_attrs.flags |= if is_like_elf { - CodegenFnAttrFlags::USED - } else { - CodegenFnAttrFlags::USED_LINKER - }; } } - } else if attr.has_name(sym::cmse_nonsecure_entry) { - if let Some(fn_sig) = fn_sig() + sym::cmse_nonsecure_entry => { + if let Some(fn_sig) = fn_sig() && !matches!(fn_sig.skip_binder().abi(), abi::Abi::C { .. }) { struct_span_err!( @@ -184,236 +204,250 @@ fn codegen_fn_attrs(tcx: TyCtxt<'_>, did: DefId) -> CodegenFnAttrs { ) .emit(); } - if !tcx.sess.target.llvm_target.contains("thumbv8m") { - struct_span_err!(tcx.sess, attr.span, E0775, "`#[cmse_nonsecure_entry]` is only valid for targets with the TrustZone-M extension") + if !tcx.sess.target.llvm_target.contains("thumbv8m") { + struct_span_err!(tcx.sess, attr.span, E0775, "`#[cmse_nonsecure_entry]` is only valid for targets with the TrustZone-M extension") .emit(); + } + codegen_fn_attrs.flags |= CodegenFnAttrFlags::CMSE_NONSECURE_ENTRY; } - codegen_fn_attrs.flags |= CodegenFnAttrFlags::CMSE_NONSECURE_ENTRY; - } else if attr.has_name(sym::thread_local) { - codegen_fn_attrs.flags |= CodegenFnAttrFlags::THREAD_LOCAL; - } else if attr.has_name(sym::track_caller) { - if !tcx.is_closure(did.to_def_id()) + sym::thread_local => { + codegen_fn_attrs.flags |= CodegenFnAttrFlags::THREAD_LOCAL; + } + sym::track_caller => { + if !tcx.is_closure(did.to_def_id()) && let Some(fn_sig) = fn_sig() && fn_sig.skip_binder().abi() != abi::Abi::Rust { struct_span_err!(tcx.sess, attr.span, E0737, "`#[track_caller]` requires Rust ABI") .emit(); } - if tcx.is_closure(did.to_def_id()) && !tcx.features().closure_track_caller { - feature_err( - &tcx.sess.parse_sess, - sym::closure_track_caller, - attr.span, - "`#[track_caller]` on closures is currently unstable", - ) - .emit(); - } - codegen_fn_attrs.flags |= CodegenFnAttrFlags::TRACK_CALLER; - } else if attr.has_name(sym::export_name) { - if let Some(s) = attr.value_str() { - if s.as_str().contains('\0') { - // `#[export_name = ...]` will be converted to a null-terminated string, - // so it may not contain any null characters. - struct_span_err!( - tcx.sess, + if tcx.is_closure(did.to_def_id()) && !tcx.features().closure_track_caller { + feature_err( + &tcx.sess.parse_sess, + sym::closure_track_caller, attr.span, - E0648, - "`export_name` may not contain null characters" + "`#[track_caller]` on closures is currently unstable", ) .emit(); } - codegen_fn_attrs.export_name = Some(s); + codegen_fn_attrs.flags |= CodegenFnAttrFlags::TRACK_CALLER; } - } else if attr.has_name(sym::target_feature) { - if !tcx.is_closure(did.to_def_id()) - && let Some(fn_sig) = fn_sig() - && fn_sig.skip_binder().unsafety() == hir::Unsafety::Normal - { - if tcx.sess.target.is_like_wasm || tcx.sess.opts.actually_rustdoc { - // The `#[target_feature]` attribute is allowed on - // WebAssembly targets on all functions, including safe - // ones. Other targets require that `#[target_feature]` is - // only applied to unsafe functions (pending the - // `target_feature_11` feature) because on most targets - // execution of instructions that are not supported is - // considered undefined behavior. For WebAssembly which is a - // 100% safe target at execution time it's not possible to - // execute undefined instructions, and even if a future - // feature was added in some form for this it would be a - // deterministic trap. There is no undefined behavior when - // executing WebAssembly so `#[target_feature]` is allowed - // on safe functions (but again, only for WebAssembly) - // - // Note that this is also allowed if `actually_rustdoc` so - // if a target is documenting some wasm-specific code then - // it's not spuriously denied. - // - // This exception needs to be kept in sync with allowing - // `#[target_feature]` on `main` and `start`. - } else if !tcx.features().target_feature_11 { - let mut err = feature_err( - &tcx.sess.parse_sess, - sym::target_feature_11, - attr.span, - "`#[target_feature(..)]` can only be applied to `unsafe` functions", - ); - err.span_label(tcx.def_span(did), "not an `unsafe` function"); - err.emit(); - } else { - check_target_feature_trait_unsafe(tcx, did, attr.span); + sym::export_name => { + if let Some(s) = attr.value_str() { + if s.as_str().contains('\0') { + // `#[export_name = ...]` will be converted to a null-terminated string, + // so it may not contain any null characters. + struct_span_err!( + tcx.sess, + attr.span, + E0648, + "`export_name` may not contain null characters" + ) + .emit(); + } + codegen_fn_attrs.export_name = Some(s); } } - from_target_feature( - tcx, - attr, - supported_target_features, - &mut codegen_fn_attrs.target_features, - ); - } else if attr.has_name(sym::linkage) { - if let Some(val) = attr.value_str() { - let linkage = Some(linkage_by_name(tcx, did, val.as_str())); - if tcx.is_foreign_item(did) { - codegen_fn_attrs.import_linkage = linkage; - } else { - codegen_fn_attrs.linkage = linkage; + sym::target_feature => { + if !tcx.is_closure(did.to_def_id()) + && let Some(fn_sig) = fn_sig() + && fn_sig.skip_binder().unsafety() == hir::Unsafety::Normal + { + if tcx.sess.target.is_like_wasm || tcx.sess.opts.actually_rustdoc { + // The `#[target_feature]` attribute is allowed on + // WebAssembly targets on all functions, including safe + // ones. Other targets require that `#[target_feature]` is + // only applied to unsafe functions (pending the + // `target_feature_11` feature) because on most targets + // execution of instructions that are not supported is + // considered undefined behavior. For WebAssembly which is a + // 100% safe target at execution time it's not possible to + // execute undefined instructions, and even if a future + // feature was added in some form for this it would be a + // deterministic trap. There is no undefined behavior when + // executing WebAssembly so `#[target_feature]` is allowed + // on safe functions (but again, only for WebAssembly) + // + // Note that this is also allowed if `actually_rustdoc` so + // if a target is documenting some wasm-specific code then + // it's not spuriously denied. + // + // This exception needs to be kept in sync with allowing + // `#[target_feature]` on `main` and `start`. + } else if !tcx.features().target_feature_11 { + let mut err = feature_err( + &tcx.sess.parse_sess, + sym::target_feature_11, + attr.span, + "`#[target_feature(..)]` can only be applied to `unsafe` functions", + ); + err.span_label(tcx.def_span(did), "not an `unsafe` function"); + err.emit(); + } else { + check_target_feature_trait_unsafe(tcx, did, attr.span); + } } + from_target_feature( + tcx, + attr, + supported_target_features, + &mut codegen_fn_attrs.target_features, + ); } - } else if attr.has_name(sym::link_section) { - if let Some(val) = attr.value_str() { - if val.as_str().bytes().any(|b| b == 0) { - let msg = format!( - "illegal null byte in link_section \ - value: `{}`", - &val - ); - tcx.sess.span_err(attr.span, &msg); - } else { - codegen_fn_attrs.link_section = Some(val); + sym::linkage => { + if let Some(val) = attr.value_str() { + let linkage = Some(linkage_by_name(tcx, did, val.as_str())); + if tcx.is_foreign_item(did) { + codegen_fn_attrs.import_linkage = linkage; + } else { + codegen_fn_attrs.linkage = linkage; + } } } - } else if attr.has_name(sym::link_name) { - codegen_fn_attrs.link_name = attr.value_str(); - } else if attr.has_name(sym::link_ordinal) { - link_ordinal_span = Some(attr.span); - if let ordinal @ Some(_) = check_link_ordinal(tcx, attr) { - codegen_fn_attrs.link_ordinal = ordinal; - } - } else if attr.has_name(sym::no_sanitize) { - no_sanitize_span = Some(attr.span); - if let Some(list) = attr.meta_item_list() { - for item in list.iter() { - if item.has_name(sym::address) { - codegen_fn_attrs.no_sanitize |= - SanitizerSet::ADDRESS | SanitizerSet::KERNELADDRESS; - } else if item.has_name(sym::cfi) { - codegen_fn_attrs.no_sanitize |= SanitizerSet::CFI; - } else if item.has_name(sym::kcfi) { - codegen_fn_attrs.no_sanitize |= SanitizerSet::KCFI; - } else if item.has_name(sym::memory) { - codegen_fn_attrs.no_sanitize |= SanitizerSet::MEMORY; - } else if item.has_name(sym::memtag) { - codegen_fn_attrs.no_sanitize |= SanitizerSet::MEMTAG; - } else if item.has_name(sym::shadow_call_stack) { - codegen_fn_attrs.no_sanitize |= SanitizerSet::SHADOWCALLSTACK; - } else if item.has_name(sym::thread) { - codegen_fn_attrs.no_sanitize |= SanitizerSet::THREAD; - } else if item.has_name(sym::hwaddress) { - codegen_fn_attrs.no_sanitize |= SanitizerSet::HWADDRESS; + sym::link_section => { + if let Some(val) = attr.value_str() { + if val.as_str().bytes().any(|b| b == 0) { + let msg = format!( + "illegal null byte in link_section \ + value: `{}`", + &val + ); + tcx.sess.span_err(attr.span, &msg); } else { - tcx.sess - .struct_span_err(item.span(), "invalid argument for `no_sanitize`") - .note("expected one of: `address`, `cfi`, `hwaddress`, `kcfi`, `memory`, `memtag`, `shadow-call-stack`, or `thread`") - .emit(); + codegen_fn_attrs.link_section = Some(val); } } } - } else if attr.has_name(sym::instruction_set) { - codegen_fn_attrs.instruction_set = attr.meta_item_list().and_then(|l| match &l[..] { - [NestedMetaItem::MetaItem(set)] => { - let segments = - set.path.segments.iter().map(|x| x.ident.name).collect::>(); - match segments.as_slice() { - [sym::arm, sym::a32] | [sym::arm, sym::t32] => { - if !tcx.sess.target.has_thumb_interworking { - struct_span_err!( - tcx.sess.diagnostic(), - attr.span, - E0779, - "target does not support `#[instruction_set]`" - ) + sym::link_name => { + codegen_fn_attrs.link_name = attr.value_str(); + } + sym::link_ordinal => { + link_ordinal_span = Some(attr.span); + if let ordinal @ Some(_) = check_link_ordinal(tcx, attr) { + codegen_fn_attrs.link_ordinal = ordinal; + } + } + sym::no_sanitize => { + no_sanitize_span = Some(attr.span); + if let Some(list) = attr.meta_item_list() { + for item in list.iter() { + if item.has_name(sym::address) { + codegen_fn_attrs.no_sanitize |= + SanitizerSet::ADDRESS | SanitizerSet::KERNELADDRESS; + } else if item.has_name(sym::cfi) { + codegen_fn_attrs.no_sanitize |= SanitizerSet::CFI; + } else if item.has_name(sym::kcfi) { + codegen_fn_attrs.no_sanitize |= SanitizerSet::KCFI; + } else if item.has_name(sym::memory) { + codegen_fn_attrs.no_sanitize |= SanitizerSet::MEMORY; + } else if item.has_name(sym::memtag) { + codegen_fn_attrs.no_sanitize |= SanitizerSet::MEMTAG; + } else if item.has_name(sym::shadow_call_stack) { + codegen_fn_attrs.no_sanitize |= SanitizerSet::SHADOWCALLSTACK; + } else if item.has_name(sym::thread) { + codegen_fn_attrs.no_sanitize |= SanitizerSet::THREAD; + } else if item.has_name(sym::hwaddress) { + codegen_fn_attrs.no_sanitize |= SanitizerSet::HWADDRESS; + } else { + tcx.sess + .struct_span_err(item.span(), "invalid argument for `no_sanitize`") + .note("expected one of: `address`, `cfi`, `hwaddress`, `kcfi`, `memory`, `memtag`, `shadow-call-stack`, or `thread`") .emit(); - None - } else if segments[1] == sym::a32 { - Some(InstructionSetAttr::ArmA32) - } else if segments[1] == sym::t32 { - Some(InstructionSetAttr::ArmT32) - } else { - unreachable!() + } + } + } + } + sym::instruction_set => { + codegen_fn_attrs.instruction_set = + attr.meta_item_list().and_then(|l| match &l[..] { + [NestedMetaItem::MetaItem(set)] => { + let segments = + set.path.segments.iter().map(|x| x.ident.name).collect::>(); + match segments.as_slice() { + [sym::arm, sym::a32] | [sym::arm, sym::t32] => { + if !tcx.sess.target.has_thumb_interworking { + struct_span_err!( + tcx.sess.diagnostic(), + attr.span, + E0779, + "target does not support `#[instruction_set]`" + ) + .emit(); + None + } else if segments[1] == sym::a32 { + Some(InstructionSetAttr::ArmA32) + } else if segments[1] == sym::t32 { + Some(InstructionSetAttr::ArmT32) + } else { + unreachable!() + } + } + _ => { + struct_span_err!( + tcx.sess.diagnostic(), + attr.span, + E0779, + "invalid instruction set specified", + ) + .emit(); + None + } } } + [] => { + struct_span_err!( + tcx.sess.diagnostic(), + attr.span, + E0778, + "`#[instruction_set]` requires an argument" + ) + .emit(); + None + } _ => { struct_span_err!( tcx.sess.diagnostic(), attr.span, E0779, - "invalid instruction set specified", + "cannot specify more than one instruction set" ) .emit(); None } - } - } - [] => { - struct_span_err!( - tcx.sess.diagnostic(), - attr.span, - E0778, - "`#[instruction_set]` requires an argument" - ) - .emit(); - None - } - _ => { - struct_span_err!( - tcx.sess.diagnostic(), - attr.span, - E0779, - "cannot specify more than one instruction set" - ) - .emit(); - None - } - }) - } else if attr.has_name(sym::repr) { - codegen_fn_attrs.alignment = match attr.meta_item_list() { - Some(items) => match items.as_slice() { - [item] => match item.name_value_literal() { - Some((sym::align, literal)) => { - let alignment = rustc_attr::parse_alignment(&literal.kind); + }) + } + sym::repr => { + codegen_fn_attrs.alignment = match attr.meta_item_list() { + Some(items) => match items.as_slice() { + [item] => match item.name_value_literal() { + Some((sym::align, literal)) => { + let alignment = rustc_attr::parse_alignment(&literal.kind); - match alignment { - Ok(align) => Some(align), - Err(msg) => { - struct_span_err!( - tcx.sess.diagnostic(), - attr.span, - E0589, - "invalid `repr(align)` attribute: {}", - msg - ) - .emit(); + match alignment { + Ok(align) => Some(align), + Err(msg) => { + struct_span_err!( + tcx.sess.diagnostic(), + attr.span, + E0589, + "invalid `repr(align)` attribute: {}", + msg + ) + .emit(); - None + None + } } } - } + _ => None, + }, + [] => None, _ => None, }, - [] => None, - _ => None, - }, - None => None, - }; + None => None, + }; + } + _ => {} } } From 1d70bd497007e86b0c7713b66402057487ed68f9 Mon Sep 17 00:00:00 2001 From: Nilstrieb <48135649+Nilstrieb@users.noreply.github.com> Date: Mon, 13 Mar 2023 19:35:24 +0000 Subject: [PATCH 06/16] Further codegen_attrs cleanups --- .../rustc_codegen_ssa/src/codegen_attrs.rs | 125 +++++++++--------- 1 file changed, 62 insertions(+), 63 deletions(-) diff --git a/compiler/rustc_codegen_ssa/src/codegen_attrs.rs b/compiler/rustc_codegen_ssa/src/codegen_attrs.rs index 00eab8d7eb690..352128da3ef26 100644 --- a/compiler/rustc_codegen_ssa/src/codegen_attrs.rs +++ b/compiler/rustc_codegen_ssa/src/codegen_attrs.rs @@ -194,16 +194,16 @@ fn codegen_fn_attrs(tcx: TyCtxt<'_>, did: DefId) -> CodegenFnAttrs { } sym::cmse_nonsecure_entry => { if let Some(fn_sig) = fn_sig() - && !matches!(fn_sig.skip_binder().abi(), abi::Abi::C { .. }) - { - struct_span_err!( - tcx.sess, - attr.span, - E0776, - "`#[cmse_nonsecure_entry]` requires C ABI" - ) - .emit(); - } + && !matches!(fn_sig.skip_binder().abi(), abi::Abi::C { .. }) + { + struct_span_err!( + tcx.sess, + attr.span, + E0776, + "`#[cmse_nonsecure_entry]` requires C ABI" + ) + .emit(); + } if !tcx.sess.target.llvm_target.contains("thumbv8m") { struct_span_err!(tcx.sess, attr.span, E0775, "`#[cmse_nonsecure_entry]` is only valid for targets with the TrustZone-M extension") .emit(); @@ -215,12 +215,12 @@ fn codegen_fn_attrs(tcx: TyCtxt<'_>, did: DefId) -> CodegenFnAttrs { } sym::track_caller => { if !tcx.is_closure(did.to_def_id()) - && let Some(fn_sig) = fn_sig() - && fn_sig.skip_binder().abi() != abi::Abi::Rust - { - struct_span_err!(tcx.sess, attr.span, E0737, "`#[track_caller]` requires Rust ABI") - .emit(); - } + && let Some(fn_sig) = fn_sig() + && fn_sig.skip_binder().abi() != abi::Abi::Rust + { + struct_span_err!(tcx.sess, attr.span, E0737, "`#[track_caller]` requires Rust ABI") + .emit(); + } if tcx.is_closure(did.to_def_id()) && !tcx.features().closure_track_caller { feature_err( &tcx.sess.parse_sess, @@ -331,28 +331,38 @@ fn codegen_fn_attrs(tcx: TyCtxt<'_>, did: DefId) -> CodegenFnAttrs { no_sanitize_span = Some(attr.span); if let Some(list) = attr.meta_item_list() { for item in list.iter() { - if item.has_name(sym::address) { - codegen_fn_attrs.no_sanitize |= - SanitizerSet::ADDRESS | SanitizerSet::KERNELADDRESS; - } else if item.has_name(sym::cfi) { - codegen_fn_attrs.no_sanitize |= SanitizerSet::CFI; - } else if item.has_name(sym::kcfi) { - codegen_fn_attrs.no_sanitize |= SanitizerSet::KCFI; - } else if item.has_name(sym::memory) { - codegen_fn_attrs.no_sanitize |= SanitizerSet::MEMORY; - } else if item.has_name(sym::memtag) { - codegen_fn_attrs.no_sanitize |= SanitizerSet::MEMTAG; - } else if item.has_name(sym::shadow_call_stack) { - codegen_fn_attrs.no_sanitize |= SanitizerSet::SHADOWCALLSTACK; - } else if item.has_name(sym::thread) { - codegen_fn_attrs.no_sanitize |= SanitizerSet::THREAD; - } else if item.has_name(sym::hwaddress) { - codegen_fn_attrs.no_sanitize |= SanitizerSet::HWADDRESS; - } else { - tcx.sess + match item.ident().map(|ident| ident.name) { + Some(sym::address) => { + codegen_fn_attrs.no_sanitize |= + SanitizerSet::ADDRESS | SanitizerSet::KERNELADDRESS; + } + Some(sym::cfi) => { + codegen_fn_attrs.no_sanitize |= SanitizerSet::CFI; + } + Some(sym::kcfi) => { + codegen_fn_attrs.no_sanitize |= SanitizerSet::KCFI; + } + Some(sym::memory) => { + codegen_fn_attrs.no_sanitize |= SanitizerSet::MEMORY; + } + Some(sym::memtag) => { + codegen_fn_attrs.no_sanitize |= SanitizerSet::MEMTAG; + } + Some(sym::shadow_call_stack) => { + codegen_fn_attrs.no_sanitize |= SanitizerSet::SHADOWCALLSTACK; + } + Some(sym::thread) => { + codegen_fn_attrs.no_sanitize |= SanitizerSet::THREAD; + } + Some(sym::hwaddress) => { + codegen_fn_attrs.no_sanitize |= SanitizerSet::HWADDRESS; + } + _ => { + tcx.sess .struct_span_err(item.span(), "invalid argument for `no_sanitize`") .note("expected one of: `address`, `cfi`, `hwaddress`, `kcfi`, `memory`, `memtag`, `shadow-call-stack`, or `thread`") .emit(); + } } } } @@ -417,34 +427,23 @@ fn codegen_fn_attrs(tcx: TyCtxt<'_>, did: DefId) -> CodegenFnAttrs { }) } sym::repr => { - codegen_fn_attrs.alignment = match attr.meta_item_list() { - Some(items) => match items.as_slice() { - [item] => match item.name_value_literal() { - Some((sym::align, literal)) => { - let alignment = rustc_attr::parse_alignment(&literal.kind); - - match alignment { - Ok(align) => Some(align), - Err(msg) => { - struct_span_err!( - tcx.sess.diagnostic(), - attr.span, - E0589, - "invalid `repr(align)` attribute: {}", - msg - ) - .emit(); - - None - } - } - } - _ => None, - }, - [] => None, - _ => None, - }, - None => None, + codegen_fn_attrs.alignment = if let Some(items) = attr.meta_item_list() + && let [item] = items.as_slice() + && let Some((sym::align, literal)) = item.name_value_literal() + { + rustc_attr::parse_alignment(&literal.kind).map_err(|msg| { + struct_span_err!( + tcx.sess.diagnostic(), + attr.span, + E0589, + "invalid `repr(align)` attribute: {}", + msg + ) + .emit(); + }) + .ok() + } else { + None }; } _ => {} From 82297a52753d1420dfbf71839c0274a50b025df8 Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Sat, 18 Feb 2023 16:23:57 +0400 Subject: [PATCH 07/16] expand: Pass `ast::Crate` by reference to AST transforming passes Also some more attributes are passed by reference. --- .../rustc_builtin_macros/src/cmdline_attrs.rs | 4 +- .../src/proc_macro_harness.rs | 12 ++-- .../src/standard_library_imports.rs | 10 +-- .../rustc_builtin_macros/src/test_harness.rs | 2 +- compiler/rustc_expand/src/config.rs | 71 +++++++++---------- compiler/rustc_expand/src/expand.rs | 6 +- compiler/rustc_interface/src/passes.rs | 24 +++---- compiler/rustc_interface/src/queries.rs | 6 +- compiler/rustc_plugin_impl/src/load.rs | 6 +- 9 files changed, 62 insertions(+), 79 deletions(-) diff --git a/compiler/rustc_builtin_macros/src/cmdline_attrs.rs b/compiler/rustc_builtin_macros/src/cmdline_attrs.rs index db05c00d2118a..2b6fcc169be06 100644 --- a/compiler/rustc_builtin_macros/src/cmdline_attrs.rs +++ b/compiler/rustc_builtin_macros/src/cmdline_attrs.rs @@ -6,7 +6,7 @@ use rustc_ast::{self as ast, AttrItem, AttrStyle}; use rustc_session::parse::ParseSess; use rustc_span::FileName; -pub fn inject(mut krate: ast::Crate, parse_sess: &ParseSess, attrs: &[String]) -> ast::Crate { +pub fn inject(krate: &mut ast::Crate, parse_sess: &ParseSess, attrs: &[String]) { for raw_attr in attrs { let mut parser = rustc_parse::new_parser_from_source_str( parse_sess, @@ -36,6 +36,4 @@ pub fn inject(mut krate: ast::Crate, parse_sess: &ParseSess, attrs: &[String]) - start_span.to(end_span), )); } - - krate } diff --git a/compiler/rustc_builtin_macros/src/proc_macro_harness.rs b/compiler/rustc_builtin_macros/src/proc_macro_harness.rs index bc513607ddd1d..b2fcd8c5734da 100644 --- a/compiler/rustc_builtin_macros/src/proc_macro_harness.rs +++ b/compiler/rustc_builtin_macros/src/proc_macro_harness.rs @@ -44,14 +44,14 @@ struct CollectProcMacros<'a> { } pub fn inject( + krate: &mut ast::Crate, sess: &Session, resolver: &mut dyn ResolverExpand, - mut krate: ast::Crate, is_proc_macro_crate: bool, has_proc_macro_decls: bool, is_test_crate: bool, handler: &rustc_errors::Handler, -) -> ast::Crate { +) { let ecfg = ExpansionConfig::default("proc_macro".to_string()); let mut cx = ExtCtxt::new(sess, ecfg, resolver, None); @@ -66,22 +66,20 @@ pub fn inject( }; if has_proc_macro_decls || is_proc_macro_crate { - visit::walk_crate(&mut collect, &krate); + visit::walk_crate(&mut collect, krate); } let macros = collect.macros; if !is_proc_macro_crate { - return krate; + return; } if is_test_crate { - return krate; + return; } let decls = mk_decls(&mut cx, ¯os); krate.items.push(decls); - - krate } impl<'a> CollectProcMacros<'a> { diff --git a/compiler/rustc_builtin_macros/src/standard_library_imports.rs b/compiler/rustc_builtin_macros/src/standard_library_imports.rs index e67c0dba68597..d62d5b4cb1487 100644 --- a/compiler/rustc_builtin_macros/src/standard_library_imports.rs +++ b/compiler/rustc_builtin_macros/src/standard_library_imports.rs @@ -8,16 +8,12 @@ use rustc_span::symbol::{kw, sym, Ident, Symbol}; use rustc_span::DUMMY_SP; use thin_vec::thin_vec; -pub fn inject( - mut krate: ast::Crate, - resolver: &mut dyn ResolverExpand, - sess: &Session, -) -> ast::Crate { +pub fn inject(krate: &mut ast::Crate, resolver: &mut dyn ResolverExpand, sess: &Session) { let edition = sess.parse_sess.edition; // the first name in this list is the crate name of the crate with the prelude let names: &[Symbol] = if sess.contains_name(&krate.attrs, sym::no_core) { - return krate; + return; } else if sess.contains_name(&krate.attrs, sym::no_std) { if sess.contains_name(&krate.attrs, sym::compiler_builtins) { &[sym::core] @@ -88,6 +84,4 @@ pub fn inject( ); krate.items.insert(0, use_item); - - krate } diff --git a/compiler/rustc_builtin_macros/src/test_harness.rs b/compiler/rustc_builtin_macros/src/test_harness.rs index d8e3db9e8ee09..82bb86892d7ef 100644 --- a/compiler/rustc_builtin_macros/src/test_harness.rs +++ b/compiler/rustc_builtin_macros/src/test_harness.rs @@ -37,7 +37,7 @@ struct TestCtxt<'a> { /// Traverse the crate, collecting all the test functions, eliding any /// existing main functions, and synthesizing a main test harness -pub fn inject(sess: &Session, resolver: &mut dyn ResolverExpand, krate: &mut ast::Crate) { +pub fn inject(krate: &mut ast::Crate, sess: &Session, resolver: &mut dyn ResolverExpand) { let span_diagnostic = sess.diagnostic(); let panic_strategy = sess.panic_strategy(); let platform_panic_strategy = sess.target.panic_strategy; diff --git a/compiler/rustc_expand/src/config.rs b/compiler/rustc_expand/src/config.rs index d6cb173ba9ba0..9ef185e9497ce 100644 --- a/compiler/rustc_expand/src/config.rs +++ b/compiler/rustc_expand/src/config.rs @@ -192,38 +192,32 @@ fn get_features(sess: &Session, krate_attrs: &[ast::Attribute]) -> Features { } /// `cfg_attr`-process the crate's attributes and compute the crate's features. -pub fn features( - sess: &Session, - mut krate: ast::Crate, - lint_node_id: NodeId, -) -> (ast::Crate, Features) { +pub fn features(sess: &Session, krate: &mut ast::Crate, lint_node_id: NodeId) -> Features { let mut strip_unconfigured = StripUnconfigured { sess, features: None, config_tokens: false, lint_node_id }; - let unconfigured_attrs = krate.attrs.clone(); + let mut unconfigured_attrs = krate.attrs.clone(); let diag = &sess.parse_sess.span_diagnostic; let err_count = diag.err_count(); - let features = match strip_unconfigured.configure_krate_attrs(krate.attrs) { - None => { - // The entire crate is unconfigured. - krate.attrs = ast::AttrVec::new(); - krate.items = ThinVec::new(); - Features::default() - } - Some(attrs) => { - krate.attrs = attrs; - let features = get_features(sess, &krate.attrs); - if err_count == diag.err_count() { - // Avoid reconfiguring malformed `cfg_attr`s. - strip_unconfigured.features = Some(&features); - // Run configuration again, this time with features available - // so that we can perform feature-gating. - strip_unconfigured.configure_krate_attrs(unconfigured_attrs); - } - features + + krate.attrs.flat_map_in_place(|attr| strip_unconfigured.process_cfg_attr(&attr)); + if !strip_unconfigured.in_cfg(&krate.attrs) { + // The entire crate is unconfigured. + krate.attrs = ast::AttrVec::new(); + krate.items = ThinVec::new(); + Features::default() + } else { + let features = get_features(sess, &krate.attrs); + if err_count == diag.err_count() { + // Avoid reconfiguring malformed `cfg_attr`s. + strip_unconfigured.features = Some(&features); + // Run configuration again, this time with features available + // so that we can perform feature-gating. + unconfigured_attrs.flat_map_in_place(|attr| strip_unconfigured.process_cfg_attr(&attr)); + strip_unconfigured.in_cfg(&unconfigured_attrs); } - }; - (krate, features) + features + } } #[macro_export] @@ -254,11 +248,6 @@ impl<'a> StripUnconfigured<'a> { } } - fn configure_krate_attrs(&self, mut attrs: ast::AttrVec) -> Option { - attrs.flat_map_in_place(|attr| self.process_cfg_attr(attr)); - self.in_cfg(&attrs).then_some(attrs) - } - /// Performs cfg-expansion on `stream`, producing a new `AttrTokenStream`. /// This is only used during the invocation of `derive` proc-macros, /// which require that we cfg-expand their entire input. @@ -281,7 +270,7 @@ impl<'a> StripUnconfigured<'a> { .iter() .flat_map(|tree| match tree.clone() { AttrTokenTree::Attributes(mut data) => { - data.attrs.flat_map_in_place(|attr| self.process_cfg_attr(attr)); + data.attrs.flat_map_in_place(|attr| self.process_cfg_attr(&attr)); if self.in_cfg(&data.attrs) { data.tokens = LazyAttrTokenStream::new( @@ -319,12 +308,16 @@ impl<'a> StripUnconfigured<'a> { /// the syntax of any `cfg_attr` is incorrect. fn process_cfg_attrs(&self, node: &mut T) { node.visit_attrs(|attrs| { - attrs.flat_map_in_place(|attr| self.process_cfg_attr(attr)); + attrs.flat_map_in_place(|attr| self.process_cfg_attr(&attr)); }); } - fn process_cfg_attr(&self, attr: Attribute) -> Vec { - if attr.has_name(sym::cfg_attr) { self.expand_cfg_attr(attr, true) } else { vec![attr] } + fn process_cfg_attr(&self, attr: &Attribute) -> Vec { + if attr.has_name(sym::cfg_attr) { + self.expand_cfg_attr(attr, true) + } else { + vec![attr.clone()] + } } /// Parse and expand a single `cfg_attr` attribute into a list of attributes @@ -334,9 +327,9 @@ impl<'a> StripUnconfigured<'a> { /// Gives a compiler warning when the `cfg_attr` contains no attributes and /// is in the original source file. Gives a compiler error if the syntax of /// the attribute is incorrect. - pub(crate) fn expand_cfg_attr(&self, attr: Attribute, recursive: bool) -> Vec { + pub(crate) fn expand_cfg_attr(&self, attr: &Attribute, recursive: bool) -> Vec { let Some((cfg_predicate, expanded_attrs)) = - rustc_parse::parse_cfg_attr(&attr, &self.sess.parse_sess) else { + rustc_parse::parse_cfg_attr(attr, &self.sess.parse_sess) else { return vec![]; }; @@ -365,10 +358,10 @@ impl<'a> StripUnconfigured<'a> { // `#[cfg_attr(false, cfg_attr(true, some_attr))]`. expanded_attrs .into_iter() - .flat_map(|item| self.process_cfg_attr(self.expand_cfg_attr_item(&attr, item))) + .flat_map(|item| self.process_cfg_attr(&self.expand_cfg_attr_item(attr, item))) .collect() } else { - expanded_attrs.into_iter().map(|item| self.expand_cfg_attr_item(&attr, item)).collect() + expanded_attrs.into_iter().map(|item| self.expand_cfg_attr_item(attr, item)).collect() } } diff --git a/compiler/rustc_expand/src/expand.rs b/compiler/rustc_expand/src/expand.rs index 4092a192e0c34..6408ccf7c4324 100644 --- a/compiler/rustc_expand/src/expand.rs +++ b/compiler/rustc_expand/src/expand.rs @@ -1688,7 +1688,7 @@ impl<'a, 'b> InvocationCollector<'a, 'b> { res } - fn expand_cfg_attr(&self, node: &mut impl HasAttrs, attr: ast::Attribute, pos: usize) { + fn expand_cfg_attr(&self, node: &mut impl HasAttrs, attr: &ast::Attribute, pos: usize) { node.visit_attrs(|attrs| { // Repeated `insert` calls is inefficient, but the number of // insertions is almost always 0 or 1 in practice. @@ -1712,7 +1712,7 @@ impl<'a, 'b> InvocationCollector<'a, 'b> { Default::default() } sym::cfg_attr => { - self.expand_cfg_attr(&mut node, attr, pos); + self.expand_cfg_attr(&mut node, &attr, pos); continue; } _ => { @@ -1760,7 +1760,7 @@ impl<'a, 'b> InvocationCollector<'a, 'b> { continue; } sym::cfg_attr => { - self.expand_cfg_attr(node, attr, pos); + self.expand_cfg_attr(node, &attr, pos); continue; } _ => visit_clobber(node, |node| { diff --git a/compiler/rustc_interface/src/passes.rs b/compiler/rustc_interface/src/passes.rs index 71bdd4df95ba3..7c1f6d8c9b872 100644 --- a/compiler/rustc_interface/src/passes.rs +++ b/compiler/rustc_interface/src/passes.rs @@ -76,10 +76,10 @@ pub fn register_plugins<'a>( sess: &'a Session, metadata_loader: &'a dyn MetadataLoader, register_lints: impl Fn(&Session, &mut LintStore), - mut krate: ast::Crate, + krate: &mut ast::Crate, crate_name: Symbol, -) -> Result<(ast::Crate, LintStore)> { - krate = sess.time("attributes_injection", || { +) -> Result { + sess.time("attributes_injection", || { rustc_builtin_macros::cmdline_attrs::inject( krate, &sess.parse_sess, @@ -87,7 +87,7 @@ pub fn register_plugins<'a>( ) }); - let (krate, features) = rustc_expand::config::features(sess, krate, CRATE_NODE_ID); + let features = rustc_expand::config::features(sess, krate, CRATE_NODE_ID); // these need to be set "early" so that expansion sees `quote` if enabled. sess.init_features(features); @@ -117,8 +117,8 @@ pub fn register_plugins<'a>( let mut lint_store = rustc_lint::new_lint_store(sess.enable_internal_lints()); register_lints(sess, &mut lint_store); - let registrars = - sess.time("plugin_loading", || plugin::load::load_plugins(sess, metadata_loader, &krate)); + let registrars = sess + .time("plugin_loading", || plugin::load::load_plugins(sess, metadata_loader, &krate.attrs)); sess.time("plugin_registration", || { let mut registry = plugin::Registry { lint_store: &mut lint_store }; for registrar in registrars { @@ -126,7 +126,7 @@ pub fn register_plugins<'a>( } }); - Ok((krate, lint_store)) + Ok(lint_store) } fn pre_expansion_lint<'a>( @@ -181,8 +181,8 @@ fn configure_and_expand(mut krate: ast::Crate, resolver: &mut Resolver<'_, '_>) pre_expansion_lint(sess, lint_store, tcx.registered_tools(()), &krate, crate_name); rustc_builtin_macros::register_builtin_macros(resolver); - krate = sess.time("crate_injection", || { - rustc_builtin_macros::standard_library_imports::inject(krate, resolver, sess) + sess.time("crate_injection", || { + rustc_builtin_macros::standard_library_imports::inject(&mut krate, resolver, sess) }); util::check_attr_crate_type(sess, &krate.attrs, &mut resolver.lint_buffer()); @@ -263,7 +263,7 @@ fn configure_and_expand(mut krate: ast::Crate, resolver: &mut Resolver<'_, '_>) }); sess.time("maybe_building_test_harness", || { - rustc_builtin_macros::test_harness::inject(sess, resolver, &mut krate) + rustc_builtin_macros::test_harness::inject(&mut krate, sess, resolver) }); let has_proc_macro_decls = sess.time("AST_validation", || { @@ -287,12 +287,12 @@ fn configure_and_expand(mut krate: ast::Crate, resolver: &mut Resolver<'_, '_>) sess.emit_warning(errors::ProcMacroCratePanicAbort); } - krate = sess.time("maybe_create_a_macro_crate", || { + sess.time("maybe_create_a_macro_crate", || { let is_test_crate = sess.opts.test; rustc_builtin_macros::proc_macro_harness::inject( + &mut krate, sess, resolver, - krate, is_proc_macro_crate, has_proc_macro_decls, is_test_crate, diff --git a/compiler/rustc_interface/src/queries.rs b/compiler/rustc_interface/src/queries.rs index 58ad044b399b6..c618297bdc04d 100644 --- a/compiler/rustc_interface/src/queries.rs +++ b/compiler/rustc_interface/src/queries.rs @@ -136,14 +136,14 @@ impl<'tcx> Queries<'tcx> { pub fn register_plugins(&self) -> Result)>> { self.register_plugins.compute(|| { let crate_name = *self.crate_name()?.borrow(); - let krate = self.parse()?.steal(); + let mut krate = self.parse()?.steal(); let empty: &(dyn Fn(&Session, &mut LintStore) + Sync + Send) = &|_, _| {}; - let (krate, lint_store) = passes::register_plugins( + let lint_store = passes::register_plugins( self.session(), &*self.codegen_backend().metadata_loader(), self.compiler.register_lints.as_deref().unwrap_or_else(|| empty), - krate, + &mut krate, crate_name, )?; diff --git a/compiler/rustc_plugin_impl/src/load.rs b/compiler/rustc_plugin_impl/src/load.rs index 8e75e969ae032..27e5cb9f0d014 100644 --- a/compiler/rustc_plugin_impl/src/load.rs +++ b/compiler/rustc_plugin_impl/src/load.rs @@ -3,7 +3,7 @@ use crate::errors::{LoadPluginError, MalformedPluginAttribute}; use crate::Registry; use libloading::Library; -use rustc_ast::Crate; +use rustc_ast::Attribute; use rustc_metadata::locator; use rustc_session::cstore::MetadataLoader; use rustc_session::Session; @@ -20,11 +20,11 @@ type PluginRegistrarFn = fn(&mut Registry<'_>); pub fn load_plugins( sess: &Session, metadata_loader: &dyn MetadataLoader, - krate: &Crate, + attrs: &[Attribute], ) -> Vec { let mut plugins = Vec::new(); - for attr in &krate.attrs { + for attr in attrs { if !attr.has_name(sym::plugin) { continue; } From 0261d25a46a33adbad617365381df1460e06d851 Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Tue, 14 Mar 2023 16:09:39 +0400 Subject: [PATCH 08/16] Add some tests for the current `#![cfg(FALSE)]` crate behavior --- tests/ui/cfg/auxiliary/cfg_false_lib.rs | 6 +++++ tests/ui/cfg/cfg-false-feature.rs | 20 ++++++++++++++ tests/ui/cfg/cfg-false-feature.stderr | 35 +++++++++++++++++++++++++ tests/ui/cfg/cfg_false_no_std.rs | 11 ++++++++ 4 files changed, 72 insertions(+) create mode 100644 tests/ui/cfg/auxiliary/cfg_false_lib.rs create mode 100644 tests/ui/cfg/cfg-false-feature.rs create mode 100644 tests/ui/cfg/cfg-false-feature.stderr create mode 100644 tests/ui/cfg/cfg_false_no_std.rs diff --git a/tests/ui/cfg/auxiliary/cfg_false_lib.rs b/tests/ui/cfg/auxiliary/cfg_false_lib.rs new file mode 100644 index 0000000000000..3c011d72b02c5 --- /dev/null +++ b/tests/ui/cfg/auxiliary/cfg_false_lib.rs @@ -0,0 +1,6 @@ +// It is unclear whether a fully unconfigured crate should link to standard library, +// or what its `no_std`/`no_core`/`compiler_builtins` status, more precisely. +// Currently the usual standard library prelude is added to such crates, +// and therefore they link to libstd. + +#![cfg(FALSE)] diff --git a/tests/ui/cfg/cfg-false-feature.rs b/tests/ui/cfg/cfg-false-feature.rs new file mode 100644 index 0000000000000..21ea3ec79b4d6 --- /dev/null +++ b/tests/ui/cfg/cfg-false-feature.rs @@ -0,0 +1,20 @@ +// It is unclear which features should be in effect in a fully unconfigured crate (issue #104633). +// Currently none on the features are in effect, so we get the feature gates reported. + +// check-pass +// compile-flags: --crate-type lib + +#![feature(decl_macro)] +#![cfg(FALSE)] +#![feature(box_syntax)] + +macro mac() {} //~ WARN `macro` is experimental + //~| WARN unstable syntax can change at any point in the future + +trait A = Clone; //~ WARN trait aliases are experimental + //~| WARN unstable syntax can change at any point in the future + +fn main() { + let box _ = Box::new(0); //~ WARN box pattern syntax is experimental + //~| WARN unstable syntax can change at any point in the future +} diff --git a/tests/ui/cfg/cfg-false-feature.stderr b/tests/ui/cfg/cfg-false-feature.stderr new file mode 100644 index 0000000000000..14673fbdb1444 --- /dev/null +++ b/tests/ui/cfg/cfg-false-feature.stderr @@ -0,0 +1,35 @@ +warning: trait aliases are experimental + --> $DIR/cfg-false-feature.rs:14:1 + | +LL | trait A = Clone; + | ^^^^^^^^^^^^^^^^ + | + = note: see issue #41517 for more information + = help: add `#![feature(trait_alias)]` to the crate attributes to enable + = warning: unstable syntax can change at any point in the future, causing a hard error! + = note: for more information, see issue #65860 + +warning: `macro` is experimental + --> $DIR/cfg-false-feature.rs:11:1 + | +LL | macro mac() {} + | ^^^^^^^^^^^^^^ + | + = note: see issue #39412 for more information + = help: add `#![feature(decl_macro)]` to the crate attributes to enable + = warning: unstable syntax can change at any point in the future, causing a hard error! + = note: for more information, see issue #65860 + +warning: box pattern syntax is experimental + --> $DIR/cfg-false-feature.rs:18:9 + | +LL | let box _ = Box::new(0); + | ^^^^^ + | + = note: see issue #29641 for more information + = help: add `#![feature(box_patterns)]` to the crate attributes to enable + = warning: unstable syntax can change at any point in the future, causing a hard error! + = note: for more information, see issue #65860 + +warning: 3 warnings emitted + diff --git a/tests/ui/cfg/cfg_false_no_std.rs b/tests/ui/cfg/cfg_false_no_std.rs new file mode 100644 index 0000000000000..319ea078187c2 --- /dev/null +++ b/tests/ui/cfg/cfg_false_no_std.rs @@ -0,0 +1,11 @@ +// Currently no error because the panic handler is supplied by libstd linked though the empty +// library, but the desirable behavior is unclear (see comments in cfg_false_lib.rs). + +// check-pass +// aux-build: cfg_false_lib.rs + +#![no_std] + +extern crate cfg_false_lib as _; + +fn main() {} From bf00e7da03723fdce3045da0ba6ba0e0f1b1ca3f Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Tue, 14 Mar 2023 16:53:04 +0400 Subject: [PATCH 09/16] rustc_interface: Add a new query `pre_configure` It partially expands crate attributes before the main expansion pass (without modifying the crate), and the produced preliminary crate attribute list is used for querying a few attributes that are required very early. Crate-level cfg attributes are then expanded normally during the main expansion pass, like attributes on any other nodes. --- .../src/standard_library_imports.rs | 17 +++++-- compiler/rustc_driver_impl/src/lib.rs | 2 +- compiler/rustc_expand/src/base.rs | 2 + compiler/rustc_expand/src/config.rs | 40 +++++----------- compiler/rustc_expand/src/expand.rs | 10 +++- compiler/rustc_interface/src/passes.rs | 46 ++++++++++--------- compiler/rustc_interface/src/queries.rs | 41 +++++++++++++---- compiler/rustc_middle/src/arena.rs | 2 +- compiler/rustc_middle/src/query/mod.rs | 2 +- compiler/rustc_resolve/src/lib.rs | 11 +++-- compiler/rustc_resolve/src/macros.rs | 4 +- tests/ui-fulldeps/lint-tool-test.rs | 1 - tests/ui-fulldeps/lint-tool-test.stderr | 32 ++++++------- 13 files changed, 113 insertions(+), 97 deletions(-) diff --git a/compiler/rustc_builtin_macros/src/standard_library_imports.rs b/compiler/rustc_builtin_macros/src/standard_library_imports.rs index d62d5b4cb1487..e977f43eb6a2b 100644 --- a/compiler/rustc_builtin_macros/src/standard_library_imports.rs +++ b/compiler/rustc_builtin_macros/src/standard_library_imports.rs @@ -8,14 +8,20 @@ use rustc_span::symbol::{kw, sym, Ident, Symbol}; use rustc_span::DUMMY_SP; use thin_vec::thin_vec; -pub fn inject(krate: &mut ast::Crate, resolver: &mut dyn ResolverExpand, sess: &Session) { +pub fn inject( + krate: &mut ast::Crate, + pre_configured_attrs: &[ast::Attribute], + resolver: &mut dyn ResolverExpand, + sess: &Session, +) -> usize { + let orig_num_items = krate.items.len(); let edition = sess.parse_sess.edition; // the first name in this list is the crate name of the crate with the prelude - let names: &[Symbol] = if sess.contains_name(&krate.attrs, sym::no_core) { - return; - } else if sess.contains_name(&krate.attrs, sym::no_std) { - if sess.contains_name(&krate.attrs, sym::compiler_builtins) { + let names: &[Symbol] = if sess.contains_name(pre_configured_attrs, sym::no_core) { + return 0; + } else if sess.contains_name(pre_configured_attrs, sym::no_std) { + if sess.contains_name(pre_configured_attrs, sym::compiler_builtins) { &[sym::core] } else { &[sym::core, sym::compiler_builtins] @@ -84,4 +90,5 @@ pub fn inject(krate: &mut ast::Crate, resolver: &mut dyn ResolverExpand, sess: & ); krate.items.insert(0, use_item); + krate.items.len() - orig_num_items } diff --git a/compiler/rustc_driver_impl/src/lib.rs b/compiler/rustc_driver_impl/src/lib.rs index 555917c8b5e9c..3fd18a98b12cf 100644 --- a/compiler/rustc_driver_impl/src/lib.rs +++ b/compiler/rustc_driver_impl/src/lib.rs @@ -353,7 +353,7 @@ fn run_compiler( { let plugins = queries.register_plugins()?; - let (_, lint_store) = &*plugins.borrow(); + let (.., lint_store) = &*plugins.borrow(); // Lint plugins are registered; now we can process command line flags. if sess.opts.describe_lints { diff --git a/compiler/rustc_expand/src/base.rs b/compiler/rustc_expand/src/base.rs index 713e4fbbdce23..53fa3f096b9dd 100644 --- a/compiler/rustc_expand/src/base.rs +++ b/compiler/rustc_expand/src/base.rs @@ -1004,6 +1004,7 @@ pub struct ExpansionData { pub struct ExtCtxt<'a> { pub sess: &'a Session, pub ecfg: expand::ExpansionConfig<'a>, + pub num_standard_library_imports: usize, pub reduced_recursion_limit: Option, pub root_path: PathBuf, pub resolver: &'a mut dyn ResolverExpand, @@ -1032,6 +1033,7 @@ impl<'a> ExtCtxt<'a> { ExtCtxt { sess, ecfg, + num_standard_library_imports: 0, reduced_recursion_limit: None, resolver, lint_store, diff --git a/compiler/rustc_expand/src/config.rs b/compiler/rustc_expand/src/config.rs index 9ef185e9497ce..a78dc0678d5da 100644 --- a/compiler/rustc_expand/src/config.rs +++ b/compiler/rustc_expand/src/config.rs @@ -24,7 +24,6 @@ use rustc_session::Session; use rustc_span::edition::{Edition, ALL_EDITIONS}; use rustc_span::symbol::{sym, Symbol}; use rustc_span::{Span, DUMMY_SP}; -use thin_vec::ThinVec; /// A folder that strips out items that do not belong in the current configuration. pub struct StripUnconfigured<'a> { @@ -37,7 +36,7 @@ pub struct StripUnconfigured<'a> { pub lint_node_id: NodeId, } -fn get_features(sess: &Session, krate_attrs: &[ast::Attribute]) -> Features { +pub fn features(sess: &Session, krate_attrs: &[Attribute]) -> Features { fn feature_removed(sess: &Session, span: Span, reason: Option<&str>) { sess.emit_err(FeatureRemoved { span, @@ -191,33 +190,16 @@ fn get_features(sess: &Session, krate_attrs: &[ast::Attribute]) -> Features { features } -/// `cfg_attr`-process the crate's attributes and compute the crate's features. -pub fn features(sess: &Session, krate: &mut ast::Crate, lint_node_id: NodeId) -> Features { - let mut strip_unconfigured = - StripUnconfigured { sess, features: None, config_tokens: false, lint_node_id }; - - let mut unconfigured_attrs = krate.attrs.clone(); - let diag = &sess.parse_sess.span_diagnostic; - let err_count = diag.err_count(); - - krate.attrs.flat_map_in_place(|attr| strip_unconfigured.process_cfg_attr(&attr)); - if !strip_unconfigured.in_cfg(&krate.attrs) { - // The entire crate is unconfigured. - krate.attrs = ast::AttrVec::new(); - krate.items = ThinVec::new(); - Features::default() - } else { - let features = get_features(sess, &krate.attrs); - if err_count == diag.err_count() { - // Avoid reconfiguring malformed `cfg_attr`s. - strip_unconfigured.features = Some(&features); - // Run configuration again, this time with features available - // so that we can perform feature-gating. - unconfigured_attrs.flat_map_in_place(|attr| strip_unconfigured.process_cfg_attr(&attr)); - strip_unconfigured.in_cfg(&unconfigured_attrs); - } - features - } +pub fn pre_configure_attrs(sess: &Session, attrs: &[Attribute]) -> ast::AttrVec { + let strip_unconfigured = StripUnconfigured { + sess, + features: None, + config_tokens: false, + lint_node_id: ast::CRATE_NODE_ID, + }; + let attrs: ast::AttrVec = + attrs.iter().flat_map(|attr| strip_unconfigured.process_cfg_attr(attr)).collect(); + if strip_unconfigured.in_cfg(&attrs) { attrs } else { ast::AttrVec::new() } } #[macro_export] diff --git a/compiler/rustc_expand/src/expand.rs b/compiler/rustc_expand/src/expand.rs index 6408ccf7c4324..ec40911545f50 100644 --- a/compiler/rustc_expand/src/expand.rs +++ b/compiler/rustc_expand/src/expand.rs @@ -1038,6 +1038,9 @@ trait InvocationCollectorNode: HasAttrs + HasNodeId + Sized { ) -> Result { Ok(noop_flat_map(node, collector)) } + fn expand_cfg_false(&mut self, collector: &mut InvocationCollector<'_, '_>, span: Span) { + collector.cx.emit_err(RemoveNodeNotSupported { span, descr: Self::descr() }); + } } impl InvocationCollectorNode for P { @@ -1378,6 +1381,11 @@ impl InvocationCollectorNode for ast::Crate { fn noop_visit(&mut self, visitor: &mut V) { noop_visit_crate(self, visitor) } + fn expand_cfg_false(&mut self, collector: &mut InvocationCollector<'_, '_>, _span: Span) { + self.attrs.clear(); + // Standard prelude imports are left in the crate for backward compatibility. + self.items.truncate(collector.cx.num_standard_library_imports); + } } impl InvocationCollectorNode for P { @@ -1756,7 +1764,7 @@ impl<'a, 'b> InvocationCollector<'a, 'b> { continue; } - self.cx.emit_err(RemoveNodeNotSupported { span, descr: Node::descr() }); + node.expand_cfg_false(self, span); continue; } sym::cfg_attr => { diff --git a/compiler/rustc_interface/src/passes.rs b/compiler/rustc_interface/src/passes.rs index 7c1f6d8c9b872..adaf33ef271e0 100644 --- a/compiler/rustc_interface/src/passes.rs +++ b/compiler/rustc_interface/src/passes.rs @@ -3,7 +3,6 @@ use crate::interface::{Compiler, Result}; use crate::proc_macro_decls; use crate::util; -use ast::CRATE_NODE_ID; use rustc_ast::{self as ast, visit}; use rustc_borrowck as mir_borrowck; use rustc_codegen_ssa::traits::CodegenBackend; @@ -76,22 +75,14 @@ pub fn register_plugins<'a>( sess: &'a Session, metadata_loader: &'a dyn MetadataLoader, register_lints: impl Fn(&Session, &mut LintStore), - krate: &mut ast::Crate, + pre_configured_attrs: &[ast::Attribute], crate_name: Symbol, ) -> Result { - sess.time("attributes_injection", || { - rustc_builtin_macros::cmdline_attrs::inject( - krate, - &sess.parse_sess, - &sess.opts.unstable_opts.crate_attr, - ) - }); - - let features = rustc_expand::config::features(sess, krate, CRATE_NODE_ID); // these need to be set "early" so that expansion sees `quote` if enabled. + let features = rustc_expand::config::features(sess, pre_configured_attrs); sess.init_features(features); - let crate_types = util::collect_crate_types(sess, &krate.attrs); + let crate_types = util::collect_crate_types(sess, pre_configured_attrs); sess.init_crate_types(crate_types); let stable_crate_id = StableCrateId::new( @@ -117,8 +108,9 @@ pub fn register_plugins<'a>( let mut lint_store = rustc_lint::new_lint_store(sess.enable_internal_lints()); register_lints(sess, &mut lint_store); - let registrars = sess - .time("plugin_loading", || plugin::load::load_plugins(sess, metadata_loader, &krate.attrs)); + let registrars = sess.time("plugin_loading", || { + plugin::load::load_plugins(sess, metadata_loader, pre_configured_attrs) + }); sess.time("plugin_registration", || { let mut registry = plugin::Registry { lint_store: &mut lint_store }; for registrar in registrars { @@ -173,7 +165,11 @@ impl LintStoreExpand for LintStoreExpandImpl<'_> { /// harness if one is to be provided, injection of a dependency on the /// standard library and prelude, and name resolution. #[instrument(level = "trace", skip(krate, resolver))] -fn configure_and_expand(mut krate: ast::Crate, resolver: &mut Resolver<'_, '_>) -> ast::Crate { +fn configure_and_expand( + mut krate: ast::Crate, + pre_configured_attrs: &[ast::Attribute], + resolver: &mut Resolver<'_, '_>, +) -> ast::Crate { let tcx = resolver.tcx(); let sess = tcx.sess; let lint_store = unerased_lint_store(tcx); @@ -181,11 +177,16 @@ fn configure_and_expand(mut krate: ast::Crate, resolver: &mut Resolver<'_, '_>) pre_expansion_lint(sess, lint_store, tcx.registered_tools(()), &krate, crate_name); rustc_builtin_macros::register_builtin_macros(resolver); - sess.time("crate_injection", || { - rustc_builtin_macros::standard_library_imports::inject(&mut krate, resolver, sess) + let num_standard_library_imports = sess.time("crate_injection", || { + rustc_builtin_macros::standard_library_imports::inject( + &mut krate, + pre_configured_attrs, + resolver, + sess, + ) }); - util::check_attr_crate_type(sess, &krate.attrs, &mut resolver.lint_buffer()); + util::check_attr_crate_type(sess, pre_configured_attrs, &mut resolver.lint_buffer()); // Expand all macros krate = sess.time("macro_expand_crate", || { @@ -222,7 +223,7 @@ fn configure_and_expand(mut krate: ast::Crate, resolver: &mut Resolver<'_, '_>) // Create the config for macro expansion let features = sess.features_untracked(); - let recursion_limit = get_recursion_limit(&krate.attrs, sess); + let recursion_limit = get_recursion_limit(pre_configured_attrs, sess); let cfg = rustc_expand::expand::ExpansionConfig { features: Some(features), recursion_limit, @@ -235,6 +236,7 @@ fn configure_and_expand(mut krate: ast::Crate, resolver: &mut Resolver<'_, '_>) let lint_store = LintStoreExpandImpl(lint_store); let mut ecx = ExtCtxt::new(sess, cfg, resolver, Some(&lint_store)); + ecx.num_standard_library_imports = num_standard_library_imports; // Expand macros now! let krate = sess.time("expand_crate", || ecx.monotonic_expander().expand_crate(krate)); @@ -557,9 +559,9 @@ fn resolver_for_lowering<'tcx>( ) -> &'tcx Steal<(ty::ResolverAstLowering, Lrc)> { let arenas = Resolver::arenas(); let _ = tcx.registered_tools(()); // Uses `crate_for_resolver`. - let krate = tcx.crate_for_resolver(()).steal(); - let mut resolver = Resolver::new(tcx, &krate, &arenas); - let krate = configure_and_expand(krate, &mut resolver); + let (krate, pre_configured_attrs) = tcx.crate_for_resolver(()).steal(); + let mut resolver = Resolver::new(tcx, &pre_configured_attrs, krate.spans.inner_span, &arenas); + let krate = configure_and_expand(krate, &pre_configured_attrs, &mut resolver); // Make sure we don't mutate the cstore from here on. tcx.untracked().cstore.leak(); diff --git a/compiler/rustc_interface/src/queries.rs b/compiler/rustc_interface/src/queries.rs index c618297bdc04d..d2293780836d5 100644 --- a/compiler/rustc_interface/src/queries.rs +++ b/compiler/rustc_interface/src/queries.rs @@ -88,8 +88,9 @@ pub struct Queries<'tcx> { dep_graph_future: Query>, parse: Query, + pre_configure: Query<(ast::Crate, ast::AttrVec)>, crate_name: Query, - register_plugins: Query<(ast::Crate, Lrc)>, + register_plugins: Query<(ast::Crate, ast::AttrVec, Lrc)>, dep_graph: Query, // This just points to what's in `gcx_cell`. gcx: Query<&'tcx GlobalCtxt<'tcx>>, @@ -106,6 +107,7 @@ impl<'tcx> Queries<'tcx> { hir_arena: WorkerLocal::new(|_| rustc_hir::Arena::default()), dep_graph_future: Default::default(), parse: Default::default(), + pre_configure: Default::default(), crate_name: Default::default(), register_plugins: Default::default(), dep_graph: Default::default(), @@ -133,17 +135,36 @@ impl<'tcx> Queries<'tcx> { .compute(|| passes::parse(self.session()).map_err(|mut parse_error| parse_error.emit())) } - pub fn register_plugins(&self) -> Result)>> { + pub fn pre_configure(&self) -> Result> { + self.pre_configure.compute(|| { + let mut krate = self.parse()?.steal(); + + let sess = self.session(); + rustc_builtin_macros::cmdline_attrs::inject( + &mut krate, + &sess.parse_sess, + &sess.opts.unstable_opts.crate_attr, + ); + + let pre_configured_attrs = + rustc_expand::config::pre_configure_attrs(sess, &krate.attrs); + Ok((krate, pre_configured_attrs)) + }) + } + + pub fn register_plugins( + &self, + ) -> Result)>> { self.register_plugins.compute(|| { let crate_name = *self.crate_name()?.borrow(); - let mut krate = self.parse()?.steal(); + let (krate, pre_configured_attrs) = self.pre_configure()?.steal(); let empty: &(dyn Fn(&Session, &mut LintStore) + Sync + Send) = &|_, _| {}; let lint_store = passes::register_plugins( self.session(), &*self.codegen_backend().metadata_loader(), self.compiler.register_lints.as_deref().unwrap_or_else(|| empty), - &mut krate, + &pre_configured_attrs, crate_name, )?; @@ -154,17 +175,17 @@ impl<'tcx> Queries<'tcx> { // called, which happens within passes::register_plugins(). self.dep_graph_future().ok(); - Ok((krate, Lrc::new(lint_store))) + Ok((krate, pre_configured_attrs, Lrc::new(lint_store))) }) } fn crate_name(&self) -> Result> { self.crate_name.compute(|| { Ok({ - let parse_result = self.parse()?; - let krate = parse_result.borrow(); + let pre_configure_result = self.pre_configure()?; + let (_, pre_configured_attrs) = &*pre_configure_result.borrow(); // parse `#[crate_name]` even if `--crate-name` was passed, to make sure it matches. - find_crate_name(self.session(), &krate.attrs) + find_crate_name(self.session(), pre_configured_attrs) }) }) } @@ -188,7 +209,7 @@ impl<'tcx> Queries<'tcx> { pub fn global_ctxt(&'tcx self) -> Result>> { self.gcx.compute(|| { let crate_name = *self.crate_name()?.borrow(); - let (krate, lint_store) = self.register_plugins()?.steal(); + let (krate, pre_configured_attrs, lint_store) = self.register_plugins()?.steal(); let sess = self.session(); @@ -215,7 +236,7 @@ impl<'tcx> Queries<'tcx> { feed.crate_name(crate_name); let feed = tcx.feed_unit_query(); - feed.crate_for_resolver(tcx.arena.alloc(Steal::new(krate))); + feed.crate_for_resolver(tcx.arena.alloc(Steal::new((krate, pre_configured_attrs)))); feed.metadata_loader( tcx.arena.alloc(Steal::new(self.codegen_backend().metadata_loader())), ); diff --git a/compiler/rustc_middle/src/arena.rs b/compiler/rustc_middle/src/arena.rs index 72907fba5e62c..9f16ecbdaa933 100644 --- a/compiler/rustc_middle/src/arena.rs +++ b/compiler/rustc_middle/src/arena.rs @@ -36,7 +36,7 @@ macro_rules! arena_types { )>, [] output_filenames: std::sync::Arc, [] metadata_loader: rustc_data_structures::steal::Steal>, - [] crate_for_resolver: rustc_data_structures::steal::Steal, + [] crate_for_resolver: rustc_data_structures::steal::Steal<(rustc_ast::Crate, rustc_ast::AttrVec)>, [] resolutions: rustc_middle::ty::ResolverGlobalCtxt, [decode] unsafety_check_result: rustc_middle::mir::UnsafetyCheckResult, [decode] code_region: rustc_middle::mir::coverage::CodeRegion, diff --git a/compiler/rustc_middle/src/query/mod.rs b/compiler/rustc_middle/src/query/mod.rs index 75f05c4af23da..919fa3eceaac2 100644 --- a/compiler/rustc_middle/src/query/mod.rs +++ b/compiler/rustc_middle/src/query/mod.rs @@ -2116,7 +2116,7 @@ rustc_queries! { desc { "raw operations for metadata file access" } } - query crate_for_resolver((): ()) -> &'tcx Steal { + query crate_for_resolver((): ()) -> &'tcx Steal<(rustc_ast::Crate, rustc_ast::AttrVec)> { feedable no_hash desc { "the ast before macro expansion and name resolution" } diff --git a/compiler/rustc_resolve/src/lib.rs b/compiler/rustc_resolve/src/lib.rs index cd90fd3ef84d8..aba391bd38975 100644 --- a/compiler/rustc_resolve/src/lib.rs +++ b/compiler/rustc_resolve/src/lib.rs @@ -1180,7 +1180,8 @@ impl<'tcx> Resolver<'_, 'tcx> { impl<'a, 'tcx> Resolver<'a, 'tcx> { pub fn new( tcx: TyCtxt<'tcx>, - krate: &Crate, + attrs: &[ast::Attribute], + crate_span: Span, arenas: &'a ResolverArenas<'a>, ) -> Resolver<'a, 'tcx> { let root_def_id = CRATE_DEF_ID.to_def_id(); @@ -1189,8 +1190,8 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> { None, ModuleKind::Def(DefKind::Mod, root_def_id, kw::Empty), ExpnId::root(), - krate.spans.inner_span, - tcx.sess.contains_name(&krate.attrs, sym::no_implicit_prelude), + crate_span, + tcx.sess.contains_name(attrs, sym::no_implicit_prelude), &mut module_map, ); let empty_module = arenas.new_module( @@ -1222,9 +1223,9 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> { .map(|(name, _)| (Ident::from_str(name), Default::default())) .collect(); - if !tcx.sess.contains_name(&krate.attrs, sym::no_core) { + if !tcx.sess.contains_name(attrs, sym::no_core) { extern_prelude.insert(Ident::with_dummy_span(sym::core), Default::default()); - if !tcx.sess.contains_name(&krate.attrs, sym::no_std) { + if !tcx.sess.contains_name(attrs, sym::no_std) { extern_prelude.insert(Ident::with_dummy_span(sym::std), Default::default()); } } diff --git a/compiler/rustc_resolve/src/macros.rs b/compiler/rustc_resolve/src/macros.rs index 37153854f7e7f..e055ea1bb6ec9 100644 --- a/compiler/rustc_resolve/src/macros.rs +++ b/compiler/rustc_resolve/src/macros.rs @@ -112,8 +112,8 @@ fn fast_print_path(path: &ast::Path) -> Symbol { pub(crate) fn registered_tools(tcx: TyCtxt<'_>, (): ()) -> RegisteredTools { let mut registered_tools = RegisteredTools::default(); - let krate = tcx.crate_for_resolver(()).borrow(); - for attr in tcx.sess.filter_by_name(&krate.attrs, sym::register_tool) { + let (_, pre_configured_attrs) = &*tcx.crate_for_resolver(()).borrow(); + for attr in tcx.sess.filter_by_name(pre_configured_attrs, sym::register_tool) { for nested_meta in attr.meta_item_list().unwrap_or_default() { match nested_meta.ident() { Some(ident) => { diff --git a/tests/ui-fulldeps/lint-tool-test.rs b/tests/ui-fulldeps/lint-tool-test.rs index f92bcd213b844..9a87646e77833 100644 --- a/tests/ui-fulldeps/lint-tool-test.rs +++ b/tests/ui-fulldeps/lint-tool-test.rs @@ -9,7 +9,6 @@ #![cfg_attr(foo, warn(test_lint))] //~^ WARNING lint name `test_lint` is deprecated and may not have an effect in the future //~| WARNING lint name `test_lint` is deprecated and may not have an effect in the future -//~| WARNING lint name `test_lint` is deprecated and may not have an effect in the future #![deny(clippy_group)] //~^ WARNING lint name `clippy_group` is deprecated and may not have an effect in the future //~| WARNING lint name `clippy_group` is deprecated and may not have an effect in the future diff --git a/tests/ui-fulldeps/lint-tool-test.stderr b/tests/ui-fulldeps/lint-tool-test.stderr index 027cf8f80cff2..9dafcbc039db4 100644 --- a/tests/ui-fulldeps/lint-tool-test.stderr +++ b/tests/ui-fulldeps/lint-tool-test.stderr @@ -1,19 +1,13 @@ -warning: lint name `test_lint` is deprecated and may not have an effect in the future. - --> $DIR/lint-tool-test.rs:9:23 - | -LL | #![cfg_attr(foo, warn(test_lint))] - | ^^^^^^^^^ help: change it to: `clippy::test_lint` - | - = note: `#[warn(renamed_and_removed_lints)]` on by default - warning: lint name `clippy_group` is deprecated and may not have an effect in the future. - --> $DIR/lint-tool-test.rs:13:9 + --> $DIR/lint-tool-test.rs:12:9 | LL | #![deny(clippy_group)] | ^^^^^^^^^^^^ help: change it to: `clippy::group` + | + = note: `#[warn(renamed_and_removed_lints)]` on by default warning: lint name `test_group` is deprecated and may not have an effect in the future. - --> $DIR/lint-tool-test.rs:29:9 + --> $DIR/lint-tool-test.rs:28:9 | LL | #[allow(test_group)] | ^^^^^^^^^^ help: change it to: `clippy::test_group` @@ -25,26 +19,26 @@ LL | #![cfg_attr(foo, warn(test_lint))] | ^^^^^^^^^ help: change it to: `clippy::test_lint` warning: lint name `clippy_group` is deprecated and may not have an effect in the future. - --> $DIR/lint-tool-test.rs:13:9 + --> $DIR/lint-tool-test.rs:12:9 | LL | #![deny(clippy_group)] | ^^^^^^^^^^^^ help: change it to: `clippy::group` error: item is named 'lintme' - --> $DIR/lint-tool-test.rs:18:1 + --> $DIR/lint-tool-test.rs:17:1 | LL | fn lintme() { } | ^^^^^^^^^^^^^^^ | note: the lint level is defined here - --> $DIR/lint-tool-test.rs:13:9 + --> $DIR/lint-tool-test.rs:12:9 | LL | #![deny(clippy_group)] | ^^^^^^^^^^^^ = note: `#[deny(clippy::test_lint)]` implied by `#[deny(clippy::group)]` error: item is named 'lintmetoo' - --> $DIR/lint-tool-test.rs:26:5 + --> $DIR/lint-tool-test.rs:25:5 | LL | fn lintmetoo() { } | ^^^^^^^^^^^^^^^^^^ @@ -52,13 +46,13 @@ LL | fn lintmetoo() { } = note: `#[deny(clippy::test_group)]` implied by `#[deny(clippy::group)]` warning: lint name `test_group` is deprecated and may not have an effect in the future. - --> $DIR/lint-tool-test.rs:29:9 + --> $DIR/lint-tool-test.rs:28:9 | LL | #[allow(test_group)] | ^^^^^^^^^^ help: change it to: `clippy::test_group` warning: unknown lint: `this_lint_does_not_exist` - --> $DIR/lint-tool-test.rs:33:8 + --> $DIR/lint-tool-test.rs:32:8 | LL | #[deny(this_lint_does_not_exist)] | ^^^^^^^^^^^^^^^^^^^^^^^^ @@ -80,16 +74,16 @@ LL | #![cfg_attr(foo, warn(test_lint))] | ^^^^^^^^^ help: change it to: `clippy::test_lint` warning: lint name `clippy_group` is deprecated and may not have an effect in the future. - --> $DIR/lint-tool-test.rs:13:9 + --> $DIR/lint-tool-test.rs:12:9 | LL | #![deny(clippy_group)] | ^^^^^^^^^^^^ help: change it to: `clippy::group` warning: lint name `test_group` is deprecated and may not have an effect in the future. - --> $DIR/lint-tool-test.rs:29:9 + --> $DIR/lint-tool-test.rs:28:9 | LL | #[allow(test_group)] | ^^^^^^^^^^ help: change it to: `clippy::test_group` -error: aborting due to 2 previous errors; 11 warnings emitted +error: aborting due to 2 previous errors; 10 warnings emitted From 34aa87292c5cd45c88a72235dad6e973a9f2b62f Mon Sep 17 00:00:00 2001 From: joboet Date: Tue, 14 Mar 2023 16:42:34 +0100 Subject: [PATCH 10/16] std: leak remaining messages in bounded channel if message destructor panics --- library/std/src/sync/mpmc/array.rs | 108 +++++++++++------------------ 1 file changed, 42 insertions(+), 66 deletions(-) diff --git a/library/std/src/sync/mpmc/array.rs b/library/std/src/sync/mpmc/array.rs index fb893695a9a58..492e21d9bdb63 100644 --- a/library/std/src/sync/mpmc/array.rs +++ b/library/std/src/sync/mpmc/array.rs @@ -15,7 +15,7 @@ use super::utils::{Backoff, CachePadded}; use super::waker::SyncWaker; use crate::cell::UnsafeCell; -use crate::mem::{self, MaybeUninit}; +use crate::mem::MaybeUninit; use crate::ptr; use crate::sync::atomic::{self, AtomicUsize, Ordering}; use crate::time::Instant; @@ -479,6 +479,10 @@ impl Channel { /// /// `tail` should be the current (and therefore last) value of `tail`. /// + /// # Panicking + /// If a destructor panics, the remaining messages are leaked, matching the + /// behaviour of the unbounded channel. + /// /// # Safety /// This method must only be called when dropping the last receiver. The /// destruction of all other receivers must have been observed with acquire @@ -486,75 +490,47 @@ impl Channel { unsafe fn discard_all_messages(&self, tail: usize) { debug_assert!(self.is_disconnected()); - /// Use a helper struct with a custom `Drop` to ensure all messages are - /// dropped, even if a destructor panicks. - struct DiscardState<'a, T> { - channel: &'a Channel, - head: usize, - tail: usize, - backoff: Backoff, - } + // Only receivers modify `head`, so since we are the last one, + // this value will not change and will not be observed (since + // no new messages can be sent after disconnection). + let mut head = self.head.load(Ordering::Relaxed); + let tail = tail & !self.mark_bit; - impl<'a, T> DiscardState<'a, T> { - fn discard(&mut self) { - loop { - // Deconstruct the head. - let index = self.head & (self.channel.mark_bit - 1); - let lap = self.head & !(self.channel.one_lap - 1); - - // Inspect the corresponding slot. - debug_assert!(index < self.channel.buffer.len()); - let slot = unsafe { self.channel.buffer.get_unchecked(index) }; - let stamp = slot.stamp.load(Ordering::Acquire); - - // If the stamp is ahead of the head by 1, we may drop the message. - if self.head + 1 == stamp { - self.head = if index + 1 < self.channel.cap { - // Same lap, incremented index. - // Set to `{ lap: lap, mark: 0, index: index + 1 }`. - self.head + 1 - } else { - // One lap forward, index wraps around to zero. - // Set to `{ lap: lap.wrapping_add(1), mark: 0, index: 0 }`. - lap.wrapping_add(self.channel.one_lap) - }; - - // We updated the head, so even if this descrutor panics, - // we will not attempt to destroy the slot again. - unsafe { - (*slot.msg.get()).assume_init_drop(); - } - // If the tail equals the head, that means the channel is empty. - } else if self.tail == self.head { - return; - // Otherwise, a sender is about to write into the slot, so we need - // to wait for it to update the stamp. - } else { - self.backoff.spin_heavy(); - } - } - } - } + let backoff = Backoff::new(); + loop { + // Deconstruct the head. + let index = head & (self.mark_bit - 1); + let lap = head & !(self.one_lap - 1); - impl<'a, T> Drop for DiscardState<'a, T> { - fn drop(&mut self) { - self.discard(); + // Inspect the corresponding slot. + debug_assert!(index < self.buffer.len()); + let slot = unsafe { self.buffer.get_unchecked(index) }; + let stamp = slot.stamp.load(Ordering::Acquire); + + // If the stamp is ahead of the head by 1, we may drop the message. + if head + 1 == stamp { + head = if index + 1 < self.cap { + // Same lap, incremented index. + // Set to `{ lap: lap, mark: 0, index: index + 1 }`. + head + 1 + } else { + // One lap forward, index wraps around to zero. + // Set to `{ lap: lap.wrapping_add(1), mark: 0, index: 0 }`. + lap.wrapping_add(self.one_lap) + }; + + unsafe { + (*slot.msg.get()).assume_init_drop(); + } + // If the tail equals the head, that means the channel is empty. + } else if tail == head { + return; + // Otherwise, a sender is about to write into the slot, so we need + // to wait for it to update the stamp. + } else { + backoff.spin_heavy(); } } - - let mut state = DiscardState { - channel: self, - // Only receivers modify `head`, so since we are the last one, - // this value will not change and will not be observed (since - // no new messages can be sent after disconnection). - head: self.head.load(Ordering::Relaxed), - tail: tail & !self.mark_bit, - backoff: Backoff::new(), - }; - state.discard(); - // This point is only reached if no destructor panics, so all messages - // have already been dropped. - mem::forget(state); } /// Returns `true` if the channel is disconnected. From f28f77f2d9867247156b45d32db0f45448b5901f Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Wed, 22 Feb 2023 01:20:14 +0400 Subject: [PATCH 11/16] resolve: Remove `item_generics_num_lifetimes` --- compiler/rustc_metadata/src/rmeta/decoder.rs | 4 ---- compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs | 4 ---- compiler/rustc_resolve/src/lib.rs | 2 +- 3 files changed, 1 insertion(+), 9 deletions(-) diff --git a/compiler/rustc_metadata/src/rmeta/decoder.rs b/compiler/rustc_metadata/src/rmeta/decoder.rs index 0070e46ffdf02..54982e7fbab19 100644 --- a/compiler/rustc_metadata/src/rmeta/decoder.rs +++ b/compiler/rustc_metadata/src/rmeta/decoder.rs @@ -925,10 +925,6 @@ impl<'a, 'tcx> CrateMetadataRef<'a> { tcx.mk_adt_def(did, adt_kind, variants, repr) } - fn get_generics(self, item_id: DefIndex, sess: &Session) -> ty::Generics { - self.root.tables.generics_of.get(self, item_id).unwrap().decode((self, sess)) - } - fn get_visibility(self, id: DefIndex) -> Visibility { self.root .tables diff --git a/compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs b/compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs index a98433953367b..089bd34ed32fb 100644 --- a/compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs +++ b/compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs @@ -565,10 +565,6 @@ impl CStore { self.get_crate_data(def.krate).def_kind(def.index) } - pub fn item_generics_num_lifetimes(&self, def_id: DefId, sess: &Session) -> usize { - self.get_crate_data(def_id.krate).get_generics(def_id.index, sess).own_counts().lifetimes - } - pub fn module_expansion_untracked(&self, def_id: DefId, sess: &Session) -> ExpnId { self.get_crate_data(def_id.krate).module_expansion(def_id.index, sess) } diff --git a/compiler/rustc_resolve/src/lib.rs b/compiler/rustc_resolve/src/lib.rs index cd90fd3ef84d8..107c9a1991a52 100644 --- a/compiler/rustc_resolve/src/lib.rs +++ b/compiler/rustc_resolve/src/lib.rs @@ -1168,7 +1168,7 @@ impl<'tcx> Resolver<'_, 'tcx> { if let Some(def_id) = def_id.as_local() { self.item_generics_num_lifetimes[&def_id] } else { - self.cstore().item_generics_num_lifetimes(def_id, self.tcx.sess) + self.tcx.generics_of(def_id).own_counts().lifetimes } } From d99e01fa7eb0dd2344040b3402a0f0961126d87e Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Wed, 22 Feb 2023 01:24:55 +0400 Subject: [PATCH 12/16] resolve: Remove `item_attrs_untracked` --- compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs | 8 -------- compiler/rustc_resolve/src/lib.rs | 5 +---- 2 files changed, 1 insertion(+), 12 deletions(-) diff --git a/compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs b/compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs index 089bd34ed32fb..41b50a36718e9 100644 --- a/compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs +++ b/compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs @@ -576,14 +576,6 @@ impl CStore { self.get_crate_data(cnum).num_def_ids() } - pub fn item_attrs_untracked<'a>( - &'a self, - def_id: DefId, - sess: &'a Session, - ) -> impl Iterator + 'a { - self.get_crate_data(def_id.krate).get_item_attrs(def_id.index, sess) - } - pub fn get_proc_macro_quoted_span_untracked( &self, cnum: CrateNum, diff --git a/compiler/rustc_resolve/src/lib.rs b/compiler/rustc_resolve/src/lib.rs index 107c9a1991a52..939f92d39f025 100644 --- a/compiler/rustc_resolve/src/lib.rs +++ b/compiler/rustc_resolve/src/lib.rs @@ -1906,10 +1906,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> { return v.clone(); } - let attr = self - .cstore() - .item_attrs_untracked(def_id, self.tcx.sess) - .find(|a| a.has_name(sym::rustc_legacy_const_generics))?; + let attr = self.tcx.get_attr(def_id, sym::rustc_legacy_const_generics)?; let mut ret = Vec::new(); for meta in attr.meta_item_list()? { match meta.lit()?.kind { From 52c73975b48b49ab6b14896a9d8d6fc665cf85da Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Tue, 14 Mar 2023 18:36:15 +0400 Subject: [PATCH 13/16] resolve: Use `item_name` and `opt_parent` in `Resolver::get_module` This is a cleanup that doesn't introduce new query calls, but this way `def_key` is decoded twice which may matter for performance or may not --- .../rustc_resolve/src/build_reduced_graph.rs | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-) diff --git a/compiler/rustc_resolve/src/build_reduced_graph.rs b/compiler/rustc_resolve/src/build_reduced_graph.rs index 362ef693c48d5..a0613962a00e3 100644 --- a/compiler/rustc_resolve/src/build_reduced_graph.rs +++ b/compiler/rustc_resolve/src/build_reduced_graph.rs @@ -27,7 +27,6 @@ use rustc_hir::def_id::{DefId, LocalDefId, CRATE_DEF_ID}; use rustc_metadata::creader::LoadedMacro; use rustc_middle::metadata::ModChild; use rustc_middle::{bug, ty}; -use rustc_session::cstore::CrateStore; use rustc_span::hygiene::{ExpnId, LocalExpnId, MacroKind}; use rustc_span::symbol::{kw, sym, Ident, Symbol}; use rustc_span::Span; @@ -118,20 +117,14 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> { let def_kind = self.cstore().def_kind(def_id); match def_kind { DefKind::Mod | DefKind::Enum | DefKind::Trait => { - let def_key = self.cstore().def_key(def_id); - let parent = def_key.parent.map(|index| { - self.get_nearest_non_block_module(DefId { index, krate: def_id.krate }) - }); - let name = if let Some(cnum) = def_id.as_crate_root() { - self.cstore().crate_name(cnum) - } else { - def_key.disambiguated_data.data.get_opt_name().expect("module without name") - }; - + let parent = self + .tcx + .opt_parent(def_id) + .map(|parent_id| self.get_nearest_non_block_module(parent_id)); let expn_id = self.cstore().module_expansion_untracked(def_id, &self.tcx.sess); Some(self.new_module( parent, - ModuleKind::Def(def_kind, def_id, name), + ModuleKind::Def(def_kind, def_id, self.tcx.item_name(def_id)), expn_id, self.def_span(def_id), // FIXME: Account for `#[no_implicit_prelude]` attributes. From 18b59f5d6d6fab80b5526bf20859b5a906332e9a Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Tue, 14 Mar 2023 19:01:09 +0400 Subject: [PATCH 14/16] resolve: Minor cleanup to `Resolver::get_module` --- .../rustc_resolve/src/build_reduced_graph.rs | 35 +++++++++---------- 1 file changed, 16 insertions(+), 19 deletions(-) diff --git a/compiler/rustc_resolve/src/build_reduced_graph.rs b/compiler/rustc_resolve/src/build_reduced_graph.rs index a0613962a00e3..fbac219c322fc 100644 --- a/compiler/rustc_resolve/src/build_reduced_graph.rs +++ b/compiler/rustc_resolve/src/build_reduced_graph.rs @@ -115,27 +115,24 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> { if !def_id.is_local() { let def_kind = self.cstore().def_kind(def_id); - match def_kind { - DefKind::Mod | DefKind::Enum | DefKind::Trait => { - let parent = self - .tcx - .opt_parent(def_id) - .map(|parent_id| self.get_nearest_non_block_module(parent_id)); - let expn_id = self.cstore().module_expansion_untracked(def_id, &self.tcx.sess); - Some(self.new_module( - parent, - ModuleKind::Def(def_kind, def_id, self.tcx.item_name(def_id)), - expn_id, - self.def_span(def_id), - // FIXME: Account for `#[no_implicit_prelude]` attributes. - parent.map_or(false, |module| module.no_implicit_prelude), - )) - } - _ => None, + if let DefKind::Mod | DefKind::Enum | DefKind::Trait = def_kind { + let parent = self + .tcx + .opt_parent(def_id) + .map(|parent_id| self.get_nearest_non_block_module(parent_id)); + let expn_id = self.cstore().module_expansion_untracked(def_id, &self.tcx.sess); + return Some(self.new_module( + parent, + ModuleKind::Def(def_kind, def_id, self.tcx.item_name(def_id)), + expn_id, + self.def_span(def_id), + // FIXME: Account for `#[no_implicit_prelude]` attributes. + parent.map_or(false, |module| module.no_implicit_prelude), + )); } - } else { - None } + + None } pub(crate) fn expn_def_scope(&mut self, expn_id: ExpnId) -> Module<'a> { From 177572241076e4885c5c12e407d3ea10d3b2363f Mon Sep 17 00:00:00 2001 From: bohan Date: Sun, 19 Mar 2023 20:12:57 +0800 Subject: [PATCH 15/16] fix: modify the condition that `resolve_imports` stops --- compiler/rustc_resolve/src/imports.rs | 38 +++++++----- tests/ui/macros/nested-use-as.rs | 83 +++++++++++++++++++++++++++ 2 files changed, 106 insertions(+), 15 deletions(-) create mode 100644 tests/ui/macros/nested-use-as.rs diff --git a/compiler/rustc_resolve/src/imports.rs b/compiler/rustc_resolve/src/imports.rs index 6e27bcc5bf3d1..4d4bc1be34973 100644 --- a/compiler/rustc_resolve/src/imports.rs +++ b/compiler/rustc_resolve/src/imports.rs @@ -423,13 +423,17 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> { /// Resolves all imports for the crate. This method performs the fixed- /// point iteration. pub(crate) fn resolve_imports(&mut self) { - let mut prev_num_indeterminates = self.indeterminate_imports.len() + 1; - while self.indeterminate_imports.len() < prev_num_indeterminates { - prev_num_indeterminates = self.indeterminate_imports.len(); + let mut prev_indeterminate_count = usize::MAX; + let mut indeterminate_count = self.indeterminate_imports.len() * 3; + while indeterminate_count < prev_indeterminate_count { + prev_indeterminate_count = indeterminate_count; + indeterminate_count = 0; for import in mem::take(&mut self.indeterminate_imports) { - match self.resolve_import(&import) { - true => self.determined_imports.push(import), - false => self.indeterminate_imports.push(import), + let import_indeterminate_count = self.resolve_import(&import); + indeterminate_count += import_indeterminate_count; + match import_indeterminate_count { + 0 => self.determined_imports.push(import), + _ => self.indeterminate_imports.push(import), } } } @@ -581,9 +585,13 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> { diag.emit(); } - /// Attempts to resolve the given import, returning true if its resolution is determined. - /// If successful, the resolved bindings are written into the module. - fn resolve_import(&mut self, import: &'a Import<'a>) -> bool { + /// Attempts to resolve the given import, returning: + /// - `0` means its resolution is determined. + /// - Other values mean that indeterminate exists under certain namespaces. + /// + /// Meanwhile, if resolve successful, the resolved bindings are written + /// into the module. + fn resolve_import(&mut self, import: &'a Import<'a>) -> usize { debug!( "(resolving import for module) resolving import `{}::...` in `{}`", Segment::names_to_string(&import.module_path), @@ -601,8 +609,8 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> { match path_res { PathResult::Module(module) => module, - PathResult::Indeterminate => return false, - PathResult::NonModule(..) | PathResult::Failed { .. } => return true, + PathResult::Indeterminate => return 3, + PathResult::NonModule(..) | PathResult::Failed { .. } => return 0, } }; @@ -618,12 +626,12 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> { } => (source, target, source_bindings, target_bindings, type_ns_only), ImportKind::Glob { .. } => { self.resolve_glob_import(import); - return true; + return 0; } _ => unreachable!(), }; - let mut indeterminate = false; + let mut indeterminate_count = 0; self.per_ns(|this, ns| { if !type_ns_only || ns == TypeNS { if let Err(Undetermined) = source_bindings[ns].get() { @@ -646,7 +654,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> { let parent = import.parent_scope.module; match source_bindings[ns].get() { - Err(Undetermined) => indeterminate = true, + Err(Undetermined) => indeterminate_count += 1, // Don't update the resolution, because it was never added. Err(Determined) if target.name == kw::Underscore => {} Ok(binding) if binding.is_importable() => { @@ -670,7 +678,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> { } }); - !indeterminate + indeterminate_count } /// Performs final import resolution, consistency checks and error reporting. diff --git a/tests/ui/macros/nested-use-as.rs b/tests/ui/macros/nested-use-as.rs new file mode 100644 index 0000000000000..21aa81e80926e --- /dev/null +++ b/tests/ui/macros/nested-use-as.rs @@ -0,0 +1,83 @@ +// check-pass +// edition:2018 +// issue: https://github.com/rust-lang/rust/issues/97534 + +macro_rules! m { + () => { + macro_rules! foo { + () => {} + } + use foo as bar; + } +} + +m!{} + +use bar as baz; + +baz!{} + +macro_rules! foo2 { + () => {}; +} + +macro_rules! m2 { + () => { + use foo2 as bar2; + }; +} + +m2! {} + +use bar2 as baz2; + +baz2! {} + +macro_rules! n1 { + () => { + macro_rules! n2 { + () => { + macro_rules! n3 { + () => { + macro_rules! n4 { + () => {} + } + use n4 as c4; + } + } + use n3 as c3; + } + } + use n2 as c2; + } +} + +use n1 as c1; +c1!{} +use c2 as a2; +a2!{} +use c3 as a3; +a3!{} +use c4 as a4; +a4!{} + +// https://github.com/rust-lang/rust/pull/108729#issuecomment-1474750675 +// reversed +use d5 as d6; +use d4 as d5; +use d3 as d4; +use d2 as d3; +use d1 as d2; +use foo2 as d1; +d6! {} + +// mess +use f3 as f4; +f5! {} +use f1 as f2; +use f4 as f5; +use f2 as f3; +use foo2 as f1; + +fn main() { +} From 1f67949f0e9cd6c80a3e325164ef17663127f07b 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: Mon, 20 Mar 2023 03:11:28 +0800 Subject: [PATCH 16/16] Lint ambiguous glob re-exports --- compiler/rustc_lint/src/context.rs | 4 ++ compiler/rustc_lint_defs/src/builtin.rs | 40 +++++++++++ compiler/rustc_lint_defs/src/lib.rs | 10 +++ .../src/effective_visibilities.rs | 68 ++++++++++++++----- compiler/rustc_resolve/src/imports.rs | 32 ++++++++- compiler/rustc_resolve/src/lib.rs | 5 +- tests/ui/imports/auxiliary/glob-conflict.rs | 2 + .../local-modularized-tricky-fail-1.rs | 1 + .../local-modularized-tricky-fail-1.stderr | 14 ++-- .../issue-107563-ambiguous-glob-reexports.rs | 33 +++++++++ ...sue-107563-ambiguous-glob-reexports.stderr | 47 +++++++++++++ 11 files changed, 229 insertions(+), 27 deletions(-) create mode 100644 tests/ui/resolve/issue-107563-ambiguous-glob-reexports.rs create mode 100644 tests/ui/resolve/issue-107563-ambiguous-glob-reexports.stderr diff --git a/compiler/rustc_lint/src/context.rs b/compiler/rustc_lint/src/context.rs index f5a711315ea4c..626c09fea07ac 100644 --- a/compiler/rustc_lint/src/context.rs +++ b/compiler/rustc_lint/src/context.rs @@ -910,6 +910,10 @@ pub trait LintContext: Sized { Applicability::MachineApplicable, ); } + BuiltinLintDiagnostics::AmbiguousGlobReexports { name, namespace, first_reexport_span, duplicate_reexport_span } => { + db.span_label(first_reexport_span, format!("the name `{}` in the {} namespace is first re-exported here", name, namespace)); + db.span_label(duplicate_reexport_span, format!("but the name `{}` in the {} namespace is also re-exported here", name, namespace)); + } } // Rewrap `db`, and pass control to the user. decorate(db) diff --git a/compiler/rustc_lint_defs/src/builtin.rs b/compiler/rustc_lint_defs/src/builtin.rs index 91966e75b5fa2..242d147b5ee11 100644 --- a/compiler/rustc_lint_defs/src/builtin.rs +++ b/compiler/rustc_lint_defs/src/builtin.rs @@ -3230,6 +3230,45 @@ declare_lint! { }; } +declare_lint! { + /// The `ambiguous_glob_reexports` lint detects cases where names re-exported via globs + /// collide. Downstream users trying to use the same name re-exported from multiple globs + /// will receive a warning pointing out redefinition of the same name. + /// + /// ### Example + /// + /// ```rust,compile_fail + /// #![deny(ambiguous_glob_reexports)] + /// pub mod foo { + /// pub type X = u8; + /// } + /// + /// pub mod bar { + /// pub type Y = u8; + /// pub type X = u8; + /// } + /// + /// pub use foo::*; + /// pub use bar::*; + /// + /// + /// pub fn main() {} + /// ``` + /// + /// {{produces}} + /// + /// ### Explanation + /// + /// This was previously accepted but it could silently break a crate's downstream users code. + /// For example, if `foo::*` and `bar::*` were re-exported before `bar::X` was added to the + /// re-exports, down stream users could use `this_crate::X` without problems. However, adding + /// `bar::X` would cause compilation errors in downstream crates because `X` is defined + /// multiple times in the same namespace of `this_crate`. + pub AMBIGUOUS_GLOB_REEXPORTS, + Warn, + "ambiguous glob re-exports", +} + declare_lint_pass! { /// Does nothing as a lint pass, but registers some `Lint`s /// that are used by other parts of the compiler. @@ -3337,6 +3376,7 @@ declare_lint_pass! { NAMED_ARGUMENTS_USED_POSITIONALLY, IMPLIED_BOUNDS_ENTAILMENT, BYTE_SLICE_IN_PACKED_STRUCT_WITH_DERIVE, + AMBIGUOUS_GLOB_REEXPORTS, ] } diff --git a/compiler/rustc_lint_defs/src/lib.rs b/compiler/rustc_lint_defs/src/lib.rs index 6f22bdabff450..69a8b691ab216 100644 --- a/compiler/rustc_lint_defs/src/lib.rs +++ b/compiler/rustc_lint_defs/src/lib.rs @@ -529,6 +529,16 @@ pub enum BuiltinLintDiagnostics { vis_span: Span, ident_span: Span, }, + AmbiguousGlobReexports { + /// The name for which collision(s) have occurred. + name: String, + /// The name space for whihc the collision(s) occurred in. + namespace: String, + /// Span where the name is first re-exported. + first_reexport_span: Span, + /// Span where the same name is also re-exported. + duplicate_reexport_span: Span, + }, } /// Lints that are buffered up early on in the `Session` before the diff --git a/compiler/rustc_resolve/src/effective_visibilities.rs b/compiler/rustc_resolve/src/effective_visibilities.rs index 4bb252bfb29dd..a1ae9b8a52181 100644 --- a/compiler/rustc_resolve/src/effective_visibilities.rs +++ b/compiler/rustc_resolve/src/effective_visibilities.rs @@ -4,6 +4,7 @@ use rustc_ast::visit; use rustc_ast::visit::Visitor; use rustc_ast::Crate; use rustc_ast::EnumDef; +use rustc_data_structures::fx::FxHashSet; use rustc_data_structures::intern::Interned; use rustc_hir::def_id::LocalDefId; use rustc_hir::def_id::CRATE_DEF_ID; @@ -70,11 +71,11 @@ impl Resolver<'_, '_> { impl<'r, 'a, 'tcx> EffectiveVisibilitiesVisitor<'r, 'a, 'tcx> { /// Fills the `Resolver::effective_visibilities` table with public & exported items /// For now, this doesn't resolve macros (FIXME) and cannot resolve Impl, as we - /// need access to a TyCtxt for that. + /// need access to a TyCtxt for that. Returns the set of ambiguous re-exports. pub(crate) fn compute_effective_visibilities<'c>( r: &'r mut Resolver<'a, 'tcx>, krate: &'c Crate, - ) { + ) -> FxHashSet>> { let mut visitor = EffectiveVisibilitiesVisitor { r, def_effective_visibilities: Default::default(), @@ -93,18 +94,26 @@ impl<'r, 'a, 'tcx> EffectiveVisibilitiesVisitor<'r, 'a, 'tcx> { } visitor.r.effective_visibilities = visitor.def_effective_visibilities; + let mut exported_ambiguities = FxHashSet::default(); + // Update visibilities for import def ids. These are not used during the // `EffectiveVisibilitiesVisitor` pass, because we have more detailed binding-based // information, but are used by later passes. Effective visibility of an import def id // is the maximum value among visibilities of bindings corresponding to that def id. for (binding, eff_vis) in visitor.import_effective_visibilities.iter() { let NameBindingKind::Import { import, .. } = binding.kind else { unreachable!() }; - if let Some(node_id) = import.id() { - r.effective_visibilities.update_eff_vis(r.local_def_id(node_id), eff_vis, r.tcx) + if !binding.is_ambiguity() { + if let Some(node_id) = import.id() { + r.effective_visibilities.update_eff_vis(r.local_def_id(node_id), eff_vis, r.tcx) + } + } else if binding.ambiguity.is_some() && eff_vis.is_public_at_level(Level::Reexported) { + exported_ambiguities.insert(*binding); } } info!("resolve::effective_visibilities: {:#?}", r.effective_visibilities); + + exported_ambiguities } /// Update effective visibilities of bindings in the given module, @@ -115,21 +124,44 @@ impl<'r, 'a, 'tcx> EffectiveVisibilitiesVisitor<'r, 'a, 'tcx> { let resolutions = self.r.resolutions(module); for (_, name_resolution) in resolutions.borrow().iter() { - if let Some(mut binding) = name_resolution.borrow().binding() && !binding.is_ambiguity() { - // Set the given effective visibility level to `Level::Direct` and - // sets the rest of the `use` chain to `Level::Reexported` until - // we hit the actual exported item. - let mut parent_id = ParentId::Def(module_id); - while let NameBindingKind::Import { binding: nested_binding, .. } = binding.kind { - let binding_id = ImportId::new_unchecked(binding); - self.update_import(binding_id, parent_id); - - parent_id = ParentId::Import(binding_id); - binding = nested_binding; - } + if let Some(mut binding) = name_resolution.borrow().binding() { + if !binding.is_ambiguity() { + // Set the given effective visibility level to `Level::Direct` and + // sets the rest of the `use` chain to `Level::Reexported` until + // we hit the actual exported item. + let mut parent_id = ParentId::Def(module_id); + while let NameBindingKind::Import { binding: nested_binding, .. } = binding.kind + { + let binding_id = ImportId::new_unchecked(binding); + self.update_import(binding_id, parent_id); + + parent_id = ParentId::Import(binding_id); + binding = nested_binding; + } + + if let Some(def_id) = binding.res().opt_def_id().and_then(|id| id.as_local()) { + self.update_def(def_id, binding.vis.expect_local(), parent_id); + } + } else { + // Put the root ambiguity binding and all reexports leading to it into the + // table. They are used by the `ambiguous_glob_reexports` lint. For all + // bindings added to the table here `is_ambiguity` returns true. + let mut parent_id = ParentId::Def(module_id); + while let NameBindingKind::Import { binding: nested_binding, .. } = binding.kind + { + let binding_id = ImportId::new_unchecked(binding); + self.update_import(binding_id, parent_id); - if let Some(def_id) = binding.res().opt_def_id().and_then(|id| id.as_local()) { - self.update_def(def_id, binding.vis.expect_local(), parent_id); + if binding.ambiguity.is_some() { + // Stop at the root ambiguity, further bindings in the chain should not + // be reexported because the root ambiguity blocks any access to them. + // (Those further bindings are most likely not ambiguities themselves.) + break; + } + + parent_id = ParentId::Import(binding_id); + binding = nested_binding; + } } } } diff --git a/compiler/rustc_resolve/src/imports.rs b/compiler/rustc_resolve/src/imports.rs index 6e27bcc5bf3d1..b36fca9bb9623 100644 --- a/compiler/rustc_resolve/src/imports.rs +++ b/compiler/rustc_resolve/src/imports.rs @@ -19,7 +19,9 @@ use rustc_hir::def::{self, DefKind, PartialRes}; use rustc_middle::metadata::ModChild; use rustc_middle::span_bug; use rustc_middle::ty; -use rustc_session::lint::builtin::{PUB_USE_OF_PRIVATE_EXTERN_CRATE, UNUSED_IMPORTS}; +use rustc_session::lint::builtin::{ + AMBIGUOUS_GLOB_REEXPORTS, PUB_USE_OF_PRIVATE_EXTERN_CRATE, UNUSED_IMPORTS, +}; use rustc_session::lint::BuiltinLintDiagnostics; use rustc_span::edit_distance::find_best_match_for_name; use rustc_span::hygiene::LocalExpnId; @@ -506,6 +508,34 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> { } } + pub(crate) fn check_reexport_ambiguities( + &mut self, + exported_ambiguities: FxHashSet>>, + ) { + for module in self.arenas.local_modules().iter() { + module.for_each_child(self, |this, ident, ns, binding| { + if let NameBindingKind::Import { import, .. } = binding.kind + && let Some((amb_binding, _)) = binding.ambiguity + && binding.res() != Res::Err + && exported_ambiguities.contains(&Interned::new_unchecked(binding)) + { + this.lint_buffer.buffer_lint_with_diagnostic( + AMBIGUOUS_GLOB_REEXPORTS, + import.root_id, + import.root_span, + "ambiguous glob re-exports", + BuiltinLintDiagnostics::AmbiguousGlobReexports { + name: ident.to_string(), + namespace: ns.descr().to_string(), + first_reexport_span: import.root_span, + duplicate_reexport_span: amb_binding.span, + }, + ); + } + }); + } + } + fn throw_unresolved_import_error(&self, errors: Vec<(&Import<'_>, UnresolvedImportError)>) { if errors.is_empty() { return; diff --git a/compiler/rustc_resolve/src/lib.rs b/compiler/rustc_resolve/src/lib.rs index cd90fd3ef84d8..de603d87e6d2d 100644 --- a/compiler/rustc_resolve/src/lib.rs +++ b/compiler/rustc_resolve/src/lib.rs @@ -1474,9 +1474,12 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> { pub fn resolve_crate(&mut self, krate: &Crate) { self.tcx.sess.time("resolve_crate", || { self.tcx.sess.time("finalize_imports", || self.finalize_imports()); - self.tcx.sess.time("compute_effective_visibilities", || { + let exported_ambiguities = self.tcx.sess.time("compute_effective_visibilities", || { EffectiveVisibilitiesVisitor::compute_effective_visibilities(self, krate) }); + self.tcx.sess.time("check_reexport_ambiguities", || { + self.check_reexport_ambiguities(exported_ambiguities) + }); self.tcx.sess.time("finalize_macro_resolutions", || self.finalize_macro_resolutions()); self.tcx.sess.time("late_resolve_crate", || self.late_resolve_crate(krate)); self.tcx.sess.time("resolve_main", || self.resolve_main()); diff --git a/tests/ui/imports/auxiliary/glob-conflict.rs b/tests/ui/imports/auxiliary/glob-conflict.rs index c83db64c6437c..8a146378b4395 100644 --- a/tests/ui/imports/auxiliary/glob-conflict.rs +++ b/tests/ui/imports/auxiliary/glob-conflict.rs @@ -1,3 +1,5 @@ +#![allow(ambiguous_glob_reexports)] + mod m1 { pub fn f() {} } diff --git a/tests/ui/imports/local-modularized-tricky-fail-1.rs b/tests/ui/imports/local-modularized-tricky-fail-1.rs index 29e9b8ec841f5..ce700ae0de9b1 100644 --- a/tests/ui/imports/local-modularized-tricky-fail-1.rs +++ b/tests/ui/imports/local-modularized-tricky-fail-1.rs @@ -1,4 +1,5 @@ #![feature(decl_macro)] +#![allow(ambiguous_glob_reexports)] macro_rules! define_exported { () => { #[macro_export] diff --git a/tests/ui/imports/local-modularized-tricky-fail-1.stderr b/tests/ui/imports/local-modularized-tricky-fail-1.stderr index 20eadaaaa56b8..52a01e8bcdfe3 100644 --- a/tests/ui/imports/local-modularized-tricky-fail-1.stderr +++ b/tests/ui/imports/local-modularized-tricky-fail-1.stderr @@ -1,12 +1,12 @@ error[E0659]: `exported` is ambiguous - --> $DIR/local-modularized-tricky-fail-1.rs:28:1 + --> $DIR/local-modularized-tricky-fail-1.rs:29:1 | LL | exported!(); | ^^^^^^^^ ambiguous name | = note: ambiguous because of a conflict between a name from a glob import and a macro-expanded name in the same module during import or macro resolution note: `exported` could refer to the macro defined here - --> $DIR/local-modularized-tricky-fail-1.rs:5:5 + --> $DIR/local-modularized-tricky-fail-1.rs:6:5 | LL | / macro_rules! exported { LL | | () => () @@ -16,7 +16,7 @@ LL | | } LL | define_exported!(); | ------------------ in this macro invocation note: `exported` could also refer to the macro imported here - --> $DIR/local-modularized-tricky-fail-1.rs:22:5 + --> $DIR/local-modularized-tricky-fail-1.rs:23:5 | LL | use inner1::*; | ^^^^^^^^^ @@ -24,7 +24,7 @@ LL | use inner1::*; = note: this error originates in the macro `define_exported` (in Nightly builds, run with -Z macro-backtrace for more info) error[E0659]: `panic` is ambiguous - --> $DIR/local-modularized-tricky-fail-1.rs:35:5 + --> $DIR/local-modularized-tricky-fail-1.rs:36:5 | LL | panic!(); | ^^^^^ ambiguous name @@ -32,7 +32,7 @@ LL | panic!(); = note: ambiguous because of a conflict between a macro-expanded name and a less macro-expanded name from outer scope during import or macro resolution = note: `panic` could refer to a macro from prelude note: `panic` could also refer to the macro defined here - --> $DIR/local-modularized-tricky-fail-1.rs:11:5 + --> $DIR/local-modularized-tricky-fail-1.rs:12:5 | LL | / macro_rules! panic { LL | | () => () @@ -45,7 +45,7 @@ LL | define_panic!(); = note: this error originates in the macro `define_panic` (in Nightly builds, run with -Z macro-backtrace for more info) error[E0659]: `include` is ambiguous - --> $DIR/local-modularized-tricky-fail-1.rs:46:1 + --> $DIR/local-modularized-tricky-fail-1.rs:47:1 | LL | include!(); | ^^^^^^^ ambiguous name @@ -53,7 +53,7 @@ LL | include!(); = note: ambiguous because of a conflict between a macro-expanded name and a less macro-expanded name from outer scope during import or macro resolution = note: `include` could refer to a macro from prelude note: `include` could also refer to the macro defined here - --> $DIR/local-modularized-tricky-fail-1.rs:17:5 + --> $DIR/local-modularized-tricky-fail-1.rs:18:5 | LL | / macro_rules! include { LL | | () => () diff --git a/tests/ui/resolve/issue-107563-ambiguous-glob-reexports.rs b/tests/ui/resolve/issue-107563-ambiguous-glob-reexports.rs new file mode 100644 index 0000000000000..431213e25e462 --- /dev/null +++ b/tests/ui/resolve/issue-107563-ambiguous-glob-reexports.rs @@ -0,0 +1,33 @@ +#![deny(ambiguous_glob_reexports)] + +pub mod foo { + pub type X = u8; +} + +pub mod bar { + pub type X = u8; + pub type Y = u8; +} + +pub use foo::*; +//~^ ERROR ambiguous glob re-exports +pub use bar::*; + +mod ambiguous { + mod m1 { pub type A = u8; } + mod m2 { pub type A = u8; } + pub use self::m1::*; + //~^ ERROR ambiguous glob re-exports + pub use self::m2::*; +} + +pub mod single { + pub use ambiguous::A; + //~^ ERROR `A` is ambiguous +} + +pub mod glob { + pub use ambiguous::*; +} + +pub fn main() {} diff --git a/tests/ui/resolve/issue-107563-ambiguous-glob-reexports.stderr b/tests/ui/resolve/issue-107563-ambiguous-glob-reexports.stderr new file mode 100644 index 0000000000000..07e61dd8643d4 --- /dev/null +++ b/tests/ui/resolve/issue-107563-ambiguous-glob-reexports.stderr @@ -0,0 +1,47 @@ +error[E0659]: `A` is ambiguous + --> $DIR/issue-107563-ambiguous-glob-reexports.rs:25:24 + | +LL | pub use ambiguous::A; + | ^ ambiguous name + | + = note: ambiguous because of multiple glob imports of a name in the same module +note: `A` could refer to the type alias imported here + --> $DIR/issue-107563-ambiguous-glob-reexports.rs:19:13 + | +LL | pub use self::m1::*; + | ^^^^^^^^^^^ + = help: consider adding an explicit import of `A` to disambiguate +note: `A` could also refer to the type alias imported here + --> $DIR/issue-107563-ambiguous-glob-reexports.rs:21:13 + | +LL | pub use self::m2::*; + | ^^^^^^^^^^^ + = help: consider adding an explicit import of `A` to disambiguate + +error: ambiguous glob re-exports + --> $DIR/issue-107563-ambiguous-glob-reexports.rs:12:9 + | +LL | pub use foo::*; + | ^^^^^^ the name `X` in the type namespace is first re-exported here +LL | +LL | pub use bar::*; + | ------ but the name `X` in the type namespace is also re-exported here + | +note: the lint level is defined here + --> $DIR/issue-107563-ambiguous-glob-reexports.rs:1:9 + | +LL | #![deny(ambiguous_glob_reexports)] + | ^^^^^^^^^^^^^^^^^^^^^^^^ + +error: ambiguous glob re-exports + --> $DIR/issue-107563-ambiguous-glob-reexports.rs:19:13 + | +LL | pub use self::m1::*; + | ^^^^^^^^^^^ the name `A` in the type namespace is first re-exported here +LL | +LL | pub use self::m2::*; + | ----------- but the name `A` in the type namespace is also re-exported here + +error: aborting due to 3 previous errors + +For more information about this error, try `rustc --explain E0659`.