From e960d0e751f0348726505a3d000023315259488e Mon Sep 17 00:00:00 2001 From: Camille GILLOT Date: Sun, 8 Oct 2023 12:21:58 +0000 Subject: [PATCH 01/18] Remove -Zdep-tasks. --- compiler/rustc_interface/src/tests.rs | 1 - .../rustc_query_system/src/dep_graph/graph.rs | 26 +++---------------- compiler/rustc_session/src/options.rs | 3 --- 3 files changed, 4 insertions(+), 26 deletions(-) diff --git a/compiler/rustc_interface/src/tests.rs b/compiler/rustc_interface/src/tests.rs index 2510ce7146022..60b9946f8266e 100644 --- a/compiler/rustc_interface/src/tests.rs +++ b/compiler/rustc_interface/src/tests.rs @@ -681,7 +681,6 @@ fn test_unstable_options_tracking_hash() { // tidy-alphabetical-start untracked!(assert_incr_state, Some(String::from("loaded"))); untracked!(deduplicate_diagnostics, false); - untracked!(dep_tasks, true); untracked!(dont_buffer_diagnostics, true); untracked!(dump_dep_graph, true); untracked!(dump_mir, Some(String::from("abc"))); diff --git a/compiler/rustc_query_system/src/dep_graph/graph.rs b/compiler/rustc_query_system/src/dep_graph/graph.rs index c7e92d7b2b227..a44000700cbfe 100644 --- a/compiler/rustc_query_system/src/dep_graph/graph.rs +++ b/compiler/rustc_query_system/src/dep_graph/graph.rs @@ -149,7 +149,6 @@ impl DepGraph { DepNode { kind: D::DEP_KIND_RED, hash: Fingerprint::ZERO.into() }, EdgesVec::new(), None, - false, ); assert_eq!(red_node_index, DepNodeIndex::FOREVER_RED_NODE); match red_node_prev_index_and_color { @@ -373,8 +372,6 @@ impl DepGraphData { let current_fingerprint = hash_result.map(|f| dcx.with_stable_hashing_context(|mut hcx| f(&mut hcx, &result))); - let print_status = cfg!(debug_assertions) && dcx.sess().opts.unstable_opts.dep_tasks; - // Intern the new `DepNode`. let (dep_node_index, prev_and_color) = self.current.intern_node( dcx.profiler(), @@ -382,7 +379,6 @@ impl DepGraphData { key, edges, current_fingerprint, - print_status, ); hashing_timer.finish_with_query_invocation_id(dep_node_index.into()); @@ -589,8 +585,6 @@ impl DepGraph { cx.with_stable_hashing_context(|mut hcx| hash_result(&mut hcx, result)) }); - let print_status = cfg!(debug_assertions) && cx.sess().opts.unstable_opts.dep_tasks; - // Intern the new `DepNode` with the dependencies up-to-now. let (dep_node_index, prev_and_color) = data.current.intern_node( cx.profiler(), @@ -598,7 +592,6 @@ impl DepGraph { node, edges, current_fingerprint, - print_status, ); hashing_timer.finish_with_query_invocation_id(dep_node_index.into()); @@ -1219,20 +1212,13 @@ impl CurrentDepGraph { key: DepNode, edges: EdgesVec, fingerprint: Option, - print_status: bool, ) -> (DepNodeIndex, Option<(SerializedDepNodeIndex, DepNodeColor)>) { - let print_status = cfg!(debug_assertions) && print_status; - // Get timer for profiling `DepNode` interning let _node_intern_timer = self.node_intern_event_id.map(|eid| profiler.generic_activity_with_event_id(eid)); if let Some(prev_index) = prev_graph.node_to_index_opt(&key) { - let get_dep_node_index = |color, fingerprint| { - if print_status { - eprintln!("[task::{color:}] {key:?}"); - } - + let get_dep_node_index = |fingerprint| { let mut prev_index_to_index = self.prev_index_to_index.lock(); let dep_node_index = match prev_index_to_index[prev_index] { @@ -1256,12 +1242,12 @@ impl CurrentDepGraph { if fingerprint == prev_graph.fingerprint_by_index(prev_index) { // This is a green node: it existed in the previous compilation, // its query was re-executed, and it has the same result as before. - let dep_node_index = get_dep_node_index("green", fingerprint); + let dep_node_index = get_dep_node_index(fingerprint); (dep_node_index, Some((prev_index, DepNodeColor::Green(dep_node_index)))) } else { // This is a red node: it existed in the previous compilation, its query // was re-executed, but it has a different result from before. - let dep_node_index = get_dep_node_index("red", fingerprint); + let dep_node_index = get_dep_node_index(fingerprint); (dep_node_index, Some((prev_index, DepNodeColor::Red))) } } else { @@ -1269,14 +1255,10 @@ impl CurrentDepGraph { // session, its query was re-executed, but it doesn't compute a result hash // (i.e. it represents a `no_hash` query), so we have no way of determining // whether or not the result was the same as before. - let dep_node_index = get_dep_node_index("unknown", Fingerprint::ZERO); + let dep_node_index = get_dep_node_index(Fingerprint::ZERO); (dep_node_index, Some((prev_index, DepNodeColor::Red))) } } else { - if print_status { - eprintln!("[task::new] {key:?}"); - } - let fingerprint = fingerprint.unwrap_or(Fingerprint::ZERO); // This is a new node: it didn't exist in the previous compilation session. diff --git a/compiler/rustc_session/src/options.rs b/compiler/rustc_session/src/options.rs index c1424db600eff..5c9438ab9bd9b 100644 --- a/compiler/rustc_session/src/options.rs +++ b/compiler/rustc_session/src/options.rs @@ -1452,9 +1452,6 @@ options! { dep_info_omit_d_target: bool = (false, parse_bool, [TRACKED], "in dep-info output, omit targets for tracking dependencies of the dep-info files \ themselves (default: no)"), - dep_tasks: bool = (false, parse_bool, [UNTRACKED], - "print tasks that execute and the color their dep node gets (requires debug build) \ - (default: no)"), dont_buffer_diagnostics: bool = (false, parse_bool, [UNTRACKED], "emit diagnostics rather than buffering (breaks NLL error downgrading, sorting) \ (default: no)"), From 8531ebfc951a2002207bc5e604bc7584c1c1a560 Mon Sep 17 00:00:00 2001 From: Camille GILLOT Date: Sun, 8 Oct 2023 12:24:20 +0000 Subject: [PATCH 02/18] Remove useless debugging. --- compiler/rustc_query_impl/src/plumbing.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/compiler/rustc_query_impl/src/plumbing.rs b/compiler/rustc_query_impl/src/plumbing.rs index 4516708ce17c9..5bdf88aea1c53 100644 --- a/compiler/rustc_query_impl/src/plumbing.rs +++ b/compiler/rustc_query_impl/src/plumbing.rs @@ -437,8 +437,6 @@ where ); if let Some(key) = Q::Key::recover(tcx, &dep_node) { - #[cfg(debug_assertions)] - let _guard = tracing::span!(tracing::Level::TRACE, stringify!($name), ?key).entered(); force_query(query, QueryCtxt::new(tcx), key, dep_node); true } else { From 3733316c6b5bc2d66c2b5ce4a426038315956dff Mon Sep 17 00:00:00 2001 From: Chris Denton Date: Mon, 16 Oct 2023 18:27:12 +0100 Subject: [PATCH 03/18] Create `windows/api.rs` for safer FFI --- library/std/src/sys/windows/api.rs | 157 ++++++++++++++++++ library/std/src/sys/windows/c/windows_sys.lst | 2 + library/std/src/sys/windows/c/windows_sys.rs | 21 +++ library/std/src/sys/windows/fs.rs | 56 ++----- library/std/src/sys/windows/mod.rs | 16 +- library/std/src/sys/windows/os.rs | 6 +- library/std/src/sys/windows/stack_overflow.rs | 4 +- library/std/src/sys/windows/stdio.rs | 3 +- 8 files changed, 212 insertions(+), 53 deletions(-) create mode 100644 library/std/src/sys/windows/api.rs diff --git a/library/std/src/sys/windows/api.rs b/library/std/src/sys/windows/api.rs new file mode 100644 index 0000000000000..e9f0bbfbe2e7d --- /dev/null +++ b/library/std/src/sys/windows/api.rs @@ -0,0 +1,157 @@ +//! # Safe(r) wrappers around Windows API functions. +//! +//! This module contains fairly thin wrappers around Windows API functions, +//! aimed at centralising safety instead of having unsafe blocks spread +//! throughout higher level code. This makes it much easier to audit FFI safety. +//! +//! Not all functions can be made completely safe without more context but in +//! such cases we should still endeavour to reduce the caller's burden of safety +//! as much as possible. +//! +//! ## Guidelines for wrappers +//! +//! Items here should be named similarly to their raw Windows API name, except +//! that they follow Rust's case conventions. E.g. function names are +//! lower_snake_case. The idea here is that it should be easy for a Windows +//! C/C++ programmer to identify the underlying function that's being wrapped +//! while not looking too out of place in Rust code. +//! +//! Every use of an `unsafe` block must have a related SAFETY comment, even if +//! it's trivially safe (for example, see `get_last_error`). Public unsafe +//! functions must document what the caller has to do to call them safely. +//! +//! Avoid unchecked `as` casts. For integers, either assert that the integer +//! is in range or use `try_into` instead. For pointers, prefer to use +//! `ptr.cast::()` when possible. +//! +//! This module must only depend on core and not on std types as the eventual +//! hope is to have std depend on sys and not the other way around. +//! However, some amount of glue code may currently be necessary so such code +//! should go in sys/windows/mod.rs rather than here. See `IoResult` as an example. + +use core::ffi::c_void; +use core::ptr::addr_of; + +use super::c; + +/// Helper method for getting the size of `T` as a u32. +/// Errors at compile time if the size would overflow. +/// +/// While a type larger than u32::MAX is unlikely, it is possible if only because of a bug. +/// However, one key motivation for this function is to avoid the temptation to +/// use frequent `as` casts. This is risky because they are too powerful. +/// For example, the following will compile today: +/// +/// `std::mem::size_of:: as u32` +/// +/// Note that `size_of` is never actually called, instead a function pointer is +/// converted to a `u32`. Clippy would warn about this but, alas, it's not run +/// on the standard library. +const fn win32_size_of() -> u32 { + // Const assert that the size is less than u32::MAX. + // Uses a trait to workaround restriction on using generic types in inner items. + trait Win32SizeOf: Sized { + const WIN32_SIZE_OF: u32 = { + let size = core::mem::size_of::(); + assert!(size <= u32::MAX as usize); + size as u32 + }; + } + impl Win32SizeOf for T {} + + T::WIN32_SIZE_OF +} + +/// The `SetFileInformationByHandle` function takes a generic parameter by +/// making the user specify the type (class), a pointer to the data and its +/// size. This trait allows attaching that information to a Rust type so that +/// [`set_file_information_by_handle`] can be called safely. +/// +/// This trait is designed so that it can support variable sized types. +/// However, currently Rust's std only uses fixed sized structures. +/// +/// # Safety +/// +/// * `as_ptr` must return a pointer to memory that is readable up to `size` bytes. +/// * `CLASS` must accurately reflect the type pointed to by `as_ptr`. E.g. +/// the `FILE_BASIC_INFO` structure has the class `FileBasicInfo`. +pub unsafe trait SetFileInformation { + /// The type of information to set. + const CLASS: i32; + /// A pointer to the file information to set. + fn as_ptr(&self) -> *const c_void; + /// The size of the type pointed to by `as_ptr`. + fn size(&self) -> u32; +} +/// Helper trait for implementing `SetFileInformation` for statically sized types. +unsafe trait SizedSetFileInformation: Sized { + const CLASS: i32; +} +unsafe impl SetFileInformation for T { + const CLASS: i32 = T::CLASS; + fn as_ptr(&self) -> *const c_void { + addr_of!(*self).cast::() + } + fn size(&self) -> u32 { + win32_size_of::() + } +} + +// SAFETY: FILE_BASIC_INFO, FILE_END_OF_FILE_INFO, FILE_ALLOCATION_INFO, +// FILE_DISPOSITION_INFO, FILE_DISPOSITION_INFO_EX and FILE_IO_PRIORITY_HINT_INFO +// are all plain `repr(C)` structs that only contain primitive types. +// The given information classes correctly match with the struct. +unsafe impl SizedSetFileInformation for c::FILE_BASIC_INFO { + const CLASS: i32 = c::FileBasicInfo; +} +unsafe impl SizedSetFileInformation for c::FILE_END_OF_FILE_INFO { + const CLASS: i32 = c::FileEndOfFileInfo; +} +unsafe impl SizedSetFileInformation for c::FILE_ALLOCATION_INFO { + const CLASS: i32 = c::FileAllocationInfo; +} +unsafe impl SizedSetFileInformation for c::FILE_DISPOSITION_INFO { + const CLASS: i32 = c::FileDispositionInfo; +} +unsafe impl SizedSetFileInformation for c::FILE_DISPOSITION_INFO_EX { + const CLASS: i32 = c::FileDispositionInfoEx; +} +unsafe impl SizedSetFileInformation for c::FILE_IO_PRIORITY_HINT_INFO { + const CLASS: i32 = c::FileIoPriorityHintInfo; +} + +#[inline] +pub fn set_file_information_by_handle( + handle: c::HANDLE, + info: &T, +) -> Result<(), WinError> { + unsafe fn set_info( + handle: c::HANDLE, + class: i32, + info: *const c_void, + size: u32, + ) -> Result<(), WinError> { + let result = c::SetFileInformationByHandle(handle, class, info, size); + (result != 0).then_some(()).ok_or_else(|| get_last_error()) + } + // SAFETY: The `SetFileInformation` trait ensures that this is safe. + unsafe { set_info(handle, T::CLASS, info.as_ptr(), info.size()) } +} + +/// Gets the error from the last function. +/// This must be called immediately after the function that sets the error to +/// avoid the risk of another function overwriting it. +pub fn get_last_error() -> WinError { + // SAFETY: This just returns a thread-local u32 and has no other effects. + unsafe { WinError { code: c::GetLastError() } } +} + +/// An error code as returned by [`get_last_error`]. +/// +/// This is usually a 16-bit Win32 error code but may be a 32-bit HRESULT or NTSTATUS. +/// Check the documentation of the Windows API function being called for expected errors. +#[derive(Clone, Copy, PartialEq, Eq)] +#[repr(transparent)] +pub struct WinError { + pub code: u32, +} diff --git a/library/std/src/sys/windows/c/windows_sys.lst b/library/std/src/sys/windows/c/windows_sys.lst index dad4a4223b2a3..bc5bfe80fb109 100644 --- a/library/std/src/sys/windows/c/windows_sys.lst +++ b/library/std/src/sys/windows/c/windows_sys.lst @@ -2224,6 +2224,7 @@ Windows.Win32.Storage.FileSystem.FILE_ACCESS_RIGHTS Windows.Win32.Storage.FileSystem.FILE_ADD_FILE Windows.Win32.Storage.FileSystem.FILE_ADD_SUBDIRECTORY Windows.Win32.Storage.FileSystem.FILE_ALL_ACCESS +Windows.Win32.Storage.FileSystem.FILE_ALLOCATION_INFO Windows.Win32.Storage.FileSystem.FILE_APPEND_DATA Windows.Win32.Storage.FileSystem.FILE_ATTRIBUTE_ARCHIVE Windows.Win32.Storage.FileSystem.FILE_ATTRIBUTE_COMPRESSED @@ -2284,6 +2285,7 @@ Windows.Win32.Storage.FileSystem.FILE_GENERIC_READ Windows.Win32.Storage.FileSystem.FILE_GENERIC_WRITE Windows.Win32.Storage.FileSystem.FILE_ID_BOTH_DIR_INFO Windows.Win32.Storage.FileSystem.FILE_INFO_BY_HANDLE_CLASS +Windows.Win32.Storage.FileSystem.FILE_IO_PRIORITY_HINT_INFO Windows.Win32.Storage.FileSystem.FILE_LIST_DIRECTORY Windows.Win32.Storage.FileSystem.FILE_NAME_NORMALIZED Windows.Win32.Storage.FileSystem.FILE_NAME_OPENED diff --git a/library/std/src/sys/windows/c/windows_sys.rs b/library/std/src/sys/windows/c/windows_sys.rs index 20b44966a39f5..d66dfda6b8e04 100644 --- a/library/std/src/sys/windows/c/windows_sys.rs +++ b/library/std/src/sys/windows/c/windows_sys.rs @@ -3107,6 +3107,16 @@ impl ::core::clone::Clone for FILETIME { pub type FILE_ACCESS_RIGHTS = u32; pub const FILE_ADD_FILE: FILE_ACCESS_RIGHTS = 2u32; pub const FILE_ADD_SUBDIRECTORY: FILE_ACCESS_RIGHTS = 4u32; +#[repr(C)] +pub struct FILE_ALLOCATION_INFO { + pub AllocationSize: i64, +} +impl ::core::marker::Copy for FILE_ALLOCATION_INFO {} +impl ::core::clone::Clone for FILE_ALLOCATION_INFO { + fn clone(&self) -> Self { + *self + } +} pub const FILE_ALL_ACCESS: FILE_ACCESS_RIGHTS = 2032127u32; pub const FILE_APPEND_DATA: FILE_ACCESS_RIGHTS = 4u32; pub const FILE_ATTRIBUTE_ARCHIVE: FILE_FLAGS_AND_ATTRIBUTES = 32u32; @@ -3248,6 +3258,16 @@ impl ::core::clone::Clone for FILE_ID_BOTH_DIR_INFO { } } pub type FILE_INFO_BY_HANDLE_CLASS = i32; +#[repr(C)] +pub struct FILE_IO_PRIORITY_HINT_INFO { + pub PriorityHint: PRIORITY_HINT, +} +impl ::core::marker::Copy for FILE_IO_PRIORITY_HINT_INFO {} +impl ::core::clone::Clone for FILE_IO_PRIORITY_HINT_INFO { + fn clone(&self) -> Self { + *self + } +} pub const FILE_LIST_DIRECTORY: FILE_ACCESS_RIGHTS = 1u32; pub const FILE_NAME_NORMALIZED: GETFINALPATHNAMEBYHANDLE_FLAGS = 0u32; pub const FILE_NAME_OPENED: GETFINALPATHNAMEBYHANDLE_FLAGS = 8u32; @@ -3753,6 +3773,7 @@ pub const PIPE_SERVER_END: NAMED_PIPE_MODE = 1u32; pub const PIPE_TYPE_BYTE: NAMED_PIPE_MODE = 0u32; pub const PIPE_TYPE_MESSAGE: NAMED_PIPE_MODE = 4u32; pub const PIPE_WAIT: NAMED_PIPE_MODE = 0u32; +pub type PRIORITY_HINT = i32; pub type PROCESSOR_ARCHITECTURE = u16; pub type PROCESS_CREATION_FLAGS = u32; #[repr(C)] diff --git a/library/std/src/sys/windows/fs.rs b/library/std/src/sys/windows/fs.rs index 6ded683aade1f..3bd8a30969ceb 100644 --- a/library/std/src/sys/windows/fs.rs +++ b/library/std/src/sys/windows/fs.rs @@ -19,7 +19,7 @@ use crate::thread; use core::ffi::c_void; use super::path::maybe_verbatim; -use super::to_u16s; +use super::{api, to_u16s, IoResult}; pub struct File { handle: Handle, @@ -123,7 +123,7 @@ impl Iterator for ReadDir { let mut wfd = mem::zeroed(); loop { if c::FindNextFileW(self.handle.0, &mut wfd) == 0 { - if c::GetLastError() == c::ERROR_NO_MORE_FILES { + if api::get_last_error().code == c::ERROR_NO_MORE_FILES { return None; } else { return Some(Err(Error::last_os_error())); @@ -318,17 +318,8 @@ impl File { } pub fn truncate(&self, size: u64) -> io::Result<()> { - let mut info = c::FILE_END_OF_FILE_INFO { EndOfFile: size as c::LARGE_INTEGER }; - let size = mem::size_of_val(&info); - cvt(unsafe { - c::SetFileInformationByHandle( - self.handle.as_raw_handle(), - c::FileEndOfFileInfo, - &mut info as *mut _ as *mut _, - size as c::DWORD, - ) - })?; - Ok(()) + let info = c::FILE_END_OF_FILE_INFO { EndOfFile: size as i64 }; + api::set_file_information_by_handle(self.handle.as_raw_handle(), &info).io_result() } #[cfg(not(target_vendor = "uwp"))] @@ -565,23 +556,14 @@ impl File { } pub fn set_permissions(&self, perm: FilePermissions) -> io::Result<()> { - let mut info = c::FILE_BASIC_INFO { + let info = c::FILE_BASIC_INFO { CreationTime: 0, LastAccessTime: 0, LastWriteTime: 0, ChangeTime: 0, FileAttributes: perm.attrs, }; - let size = mem::size_of_val(&info); - cvt(unsafe { - c::SetFileInformationByHandle( - self.handle.as_raw_handle(), - c::FileBasicInfo, - &mut info as *mut _ as *mut _, - size as c::DWORD, - ) - })?; - Ok(()) + api::set_file_information_by_handle(self.handle.as_raw_handle(), &info).io_result() } pub fn set_times(&self, times: FileTimes) -> io::Result<()> { @@ -641,38 +623,20 @@ impl File { /// If the operation is not supported for this filesystem or OS version /// then errors will be `ERROR_NOT_SUPPORTED` or `ERROR_INVALID_PARAMETER`. fn posix_delete(&self) -> io::Result<()> { - let mut info = c::FILE_DISPOSITION_INFO_EX { + let info = c::FILE_DISPOSITION_INFO_EX { Flags: c::FILE_DISPOSITION_FLAG_DELETE | c::FILE_DISPOSITION_FLAG_POSIX_SEMANTICS | c::FILE_DISPOSITION_FLAG_IGNORE_READONLY_ATTRIBUTE, }; - let size = mem::size_of_val(&info); - cvt(unsafe { - c::SetFileInformationByHandle( - self.handle.as_raw_handle(), - c::FileDispositionInfoEx, - &mut info as *mut _ as *mut _, - size as c::DWORD, - ) - })?; - Ok(()) + api::set_file_information_by_handle(self.handle.as_raw_handle(), &info).io_result() } /// Delete a file using win32 semantics. The file won't actually be deleted /// until all file handles are closed. However, marking a file for deletion /// will prevent anyone from opening a new handle to the file. fn win32_delete(&self) -> io::Result<()> { - let mut info = c::FILE_DISPOSITION_INFO { DeleteFile: c::TRUE as _ }; - let size = mem::size_of_val(&info); - cvt(unsafe { - c::SetFileInformationByHandle( - self.handle.as_raw_handle(), - c::FileDispositionInfo, - &mut info as *mut _ as *mut _, - size as c::DWORD, - ) - })?; - Ok(()) + let info = c::FILE_DISPOSITION_INFO { DeleteFile: c::TRUE as _ }; + api::set_file_information_by_handle(self.handle.as_raw_handle(), &info).io_result() } /// Fill the given buffer with as many directory entries as will fit. diff --git a/library/std/src/sys/windows/mod.rs b/library/std/src/sys/windows/mod.rs index b609ad2472c62..c4e56e13be325 100644 --- a/library/std/src/sys/windows/mod.rs +++ b/library/std/src/sys/windows/mod.rs @@ -44,6 +44,18 @@ cfg_if::cfg_if! { } } +mod api; + +/// Map a Result to io::Result. +trait IoResult { + fn io_result(self) -> crate::io::Result; +} +impl IoResult for Result { + fn io_result(self) -> crate::io::Result { + self.map_err(|e| crate::io::Error::from_raw_os_error(e.code as i32)) + } +} + // SAFETY: must be called only once during runtime initialization. // NOTE: this is not guaranteed to run, for example when Rust code is called externally. pub unsafe fn init(_argc: isize, _argv: *const *const u8, _sigpipe: u8) { @@ -241,11 +253,11 @@ where // not an actual error. c::SetLastError(0); let k = match f1(buf.as_mut_ptr().cast::(), n as c::DWORD) { - 0 if c::GetLastError() == 0 => 0, + 0 if api::get_last_error().code == 0 => 0, 0 => return Err(crate::io::Error::last_os_error()), n => n, } as usize; - if k == n && c::GetLastError() == c::ERROR_INSUFFICIENT_BUFFER { + if k == n && api::get_last_error().code == c::ERROR_INSUFFICIENT_BUFFER { n = n.saturating_mul(2).min(c::DWORD::MAX as usize); } else if k > n { n = k; diff --git a/library/std/src/sys/windows/os.rs b/library/std/src/sys/windows/os.rs index 58afca088ef9a..8cc905101de7a 100644 --- a/library/std/src/sys/windows/os.rs +++ b/library/std/src/sys/windows/os.rs @@ -17,10 +17,10 @@ use crate::ptr; use crate::slice; use crate::sys::{c, cvt}; -use super::to_u16s; +use super::{api, to_u16s}; pub fn errno() -> i32 { - unsafe { c::GetLastError() as i32 } + api::get_last_error().code as i32 } /// Gets a detailed string description for the given error number. @@ -336,7 +336,7 @@ fn home_dir_crt() -> Option { super::fill_utf16_buf( |buf, mut sz| { match c::GetUserProfileDirectoryW(token, buf, &mut sz) { - 0 if c::GetLastError() != c::ERROR_INSUFFICIENT_BUFFER => 0, + 0 if api::get_last_error().code != c::ERROR_INSUFFICIENT_BUFFER => 0, 0 => sz, _ => sz - 1, // sz includes the null terminator } diff --git a/library/std/src/sys/windows/stack_overflow.rs b/library/std/src/sys/windows/stack_overflow.rs index 0caf0a317a4aa..627763da8561b 100644 --- a/library/std/src/sys/windows/stack_overflow.rs +++ b/library/std/src/sys/windows/stack_overflow.rs @@ -3,6 +3,8 @@ use crate::sys::c; use crate::thread; +use super::api; + pub struct Handler; impl Handler { @@ -10,7 +12,7 @@ impl Handler { // This API isn't available on XP, so don't panic in that case and just // pray it works out ok. if c::SetThreadStackGuarantee(&mut 0x5000) == 0 - && c::GetLastError() as u32 != c::ERROR_CALL_NOT_IMPLEMENTED as u32 + && api::get_last_error().code != c::ERROR_CALL_NOT_IMPLEMENTED { panic!("failed to reserve stack space for exception handling"); } diff --git a/library/std/src/sys/windows/stdio.rs b/library/std/src/sys/windows/stdio.rs index 3fcaaa508e3c8..a9ff909aa6794 100644 --- a/library/std/src/sys/windows/stdio.rs +++ b/library/std/src/sys/windows/stdio.rs @@ -9,6 +9,7 @@ use crate::str; use crate::sys::c; use crate::sys::cvt; use crate::sys::handle::Handle; +use crate::sys::windows::api; use core::str::utf8_char_width; #[cfg(test)] @@ -369,7 +370,7 @@ fn read_u16s(handle: c::HANDLE, buf: &mut [MaybeUninit]) -> io::Result Date: Sat, 14 Oct 2023 18:49:51 +0000 Subject: [PATCH 04/18] make E0277 use short paths add note change wording short_ty_string on t --- .../traits/error_reporting/type_err_ctxt_ext.rs | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/compiler/rustc_trait_selection/src/traits/error_reporting/type_err_ctxt_ext.rs b/compiler/rustc_trait_selection/src/traits/error_reporting/type_err_ctxt_ext.rs index 8adfb27a3f448..f83ee576c2941 100644 --- a/compiler/rustc_trait_selection/src/traits/error_reporting/type_err_ctxt_ext.rs +++ b/compiler/rustc_trait_selection/src/traits/error_reporting/type_err_ctxt_ext.rs @@ -429,14 +429,18 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> { return; } let trait_ref = trait_predicate.to_poly_trait_ref(); - - let (post_message, pre_message, type_def) = self + let (post_message, pre_message, type_def, file_note) = self .get_parent_trait_ref(obligation.cause.code()) .map(|(t, s)| { + let (t, file) = self.tcx.short_ty_string(t); ( format!(" in `{t}`"), format!("within `{t}`, "), s.map(|s| (format!("within this `{t}`"), s)), + file.and_then(|file| Some(format!( + "the full trait has been written to '{}'", + file.display(), + ))) ) }) .unwrap_or_default(); @@ -544,6 +548,8 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> { err.emit(); return; } + + file_note.map(|note| err.note(note)); if let Some(s) = label { // If it has a custom `#[rustc_on_unimplemented]` // error message, let's display it as the label! @@ -1077,7 +1083,7 @@ pub(super) trait InferCtxtPrivExt<'tcx> { fn get_parent_trait_ref( &self, code: &ObligationCauseCode<'tcx>, - ) -> Option<(String, Option)>; + ) -> Option<(Ty<'tcx>, Option)>; /// If the `Self` type of the unsatisfied trait `trait_ref` implements a trait /// with the same path as `trait_ref`, a help message about @@ -1927,7 +1933,7 @@ impl<'tcx> InferCtxtPrivExt<'tcx> for TypeErrCtxt<'_, 'tcx> { fn get_parent_trait_ref( &self, code: &ObligationCauseCode<'tcx>, - ) -> Option<(String, Option)> { + ) -> Option<(Ty<'tcx>, Option)> { match code { ObligationCauseCode::BuiltinDerivedObligation(data) => { let parent_trait_ref = self.resolve_vars_if_possible(data.parent_trait_pred); @@ -1937,7 +1943,7 @@ impl<'tcx> InferCtxtPrivExt<'tcx> for TypeErrCtxt<'_, 'tcx> { let ty = parent_trait_ref.skip_binder().self_ty(); let span = TyCategory::from_ty(self.tcx, ty) .map(|(_, def_id)| self.tcx.def_span(def_id)); - Some((ty.to_string(), span)) + Some((ty, span)) } } } From 828f069c12b356631ef2f7b707d51708861a788d Mon Sep 17 00:00:00 2001 From: Urgau Date: Fri, 20 Oct 2023 19:33:22 +0200 Subject: [PATCH 05/18] Remove most indentation in check-cfg impl --- compiler/rustc_interface/src/interface.rs | 381 ++++++++++------------ 1 file changed, 181 insertions(+), 200 deletions(-) diff --git a/compiler/rustc_interface/src/interface.rs b/compiler/rustc_interface/src/interface.rs index d2ce77ad53511..2218892538212 100644 --- a/compiler/rustc_interface/src/interface.rs +++ b/compiler/rustc_interface/src/interface.rs @@ -139,7 +139,7 @@ pub fn parse_check_cfg(handler: &EarlyErrorHandler, specs: Vec) -> Check let filename = FileName::cfg_spec_source_code(&s); macro_rules! error { - ($reason: expr) => { + ($reason:expr) => { handler.early_error(format!( concat!("invalid `--check-cfg` argument: `{}` (", $reason, ")"), s @@ -147,217 +147,198 @@ pub fn parse_check_cfg(handler: &EarlyErrorHandler, specs: Vec) -> Check }; } - let expected_error = - || error!("expected `cfg(name, values(\"value1\", \"value2\", ... \"valueN\"))`"); - - match maybe_new_parser_from_source_str(&sess, filename, s.to_string()) { - Ok(mut parser) => match parser.parse_meta_item() { - Ok(meta_item) if parser.token == token::Eof => { - if let Some(args) = meta_item.meta_item_list() { - if meta_item.has_name(sym::names) { - // defaults are flipped for the old syntax - if old_syntax == None { - check_cfg.exhaustive_names = false; - check_cfg.exhaustive_values = false; - } - old_syntax = Some(true); - - check_cfg.exhaustive_names = true; - for arg in args { - if arg.is_word() && arg.ident().is_some() { - let ident = arg.ident().expect("multi-segment cfg key"); - check_cfg - .expecteds - .entry(ident.name.to_string()) - .or_insert(ExpectedValues::Any); - } else { - error!("`names()` arguments must be simple identifiers"); - } - } - } else if meta_item.has_name(sym::values) { - // defaults are flipped for the old syntax - if old_syntax == None { - check_cfg.exhaustive_names = false; - check_cfg.exhaustive_values = false; - } - old_syntax = Some(true); - - if let Some((name, values)) = args.split_first() { - if name.is_word() && name.ident().is_some() { - let ident = name.ident().expect("multi-segment cfg key"); - let expected_values = check_cfg - .expecteds - .entry(ident.name.to_string()) - .and_modify(|expected_values| match expected_values { - ExpectedValues::Some(_) => {} - ExpectedValues::Any => { - // handle the case where names(...) was done - // before values by changing to a list - *expected_values = - ExpectedValues::Some(FxHashSet::default()); - } - }) - .or_insert_with(|| { - ExpectedValues::Some(FxHashSet::default()) - }); - - let ExpectedValues::Some(expected_values) = expected_values - else { - bug!("`expected_values` should be a list a values") - }; - - for val in values { - if let Some(LitKind::Str(s, _)) = - val.lit().map(|lit| &lit.kind) - { - expected_values.insert(Some(s.to_string())); - } else { - error!( - "`values()` arguments must be string literals" - ); - } - } - - if values.is_empty() { - expected_values.insert(None); - } - } else { - error!( - "`values()` first argument must be a simple identifier" - ); - } - } else if args.is_empty() { - check_cfg.exhaustive_values = true; - } else { - expected_error(); - } - } else if meta_item.has_name(sym::cfg) { - old_syntax = Some(false); - - let mut names = Vec::new(); - let mut values: FxHashSet<_> = Default::default(); - - let mut any_specified = false; - let mut values_specified = false; - let mut values_any_specified = false; - - for arg in args { - if arg.is_word() && let Some(ident) = arg.ident() { - if values_specified { - error!("`cfg()` names cannot be after values"); - } - names.push(ident); - } else if arg.has_name(sym::any) - && let Some(args) = arg.meta_item_list() - { - if any_specified { - error!("`any()` cannot be specified multiple times"); - } - any_specified = true; - if !args.is_empty() { - error!("`any()` must be empty"); - } - } else if arg.has_name(sym::values) - && let Some(args) = arg.meta_item_list() - { - if names.is_empty() { - error!( - "`values()` cannot be specified before the names" - ); - } else if values_specified { - error!( - "`values()` cannot be specified multiple times" - ); - } - values_specified = true; - - for arg in args { - if let Some(LitKind::Str(s, _)) = - arg.lit().map(|lit| &lit.kind) - { - values.insert(Some(s.to_string())); - } else if arg.has_name(sym::any) - && let Some(args) = arg.meta_item_list() - { - if values_any_specified { - error!( - "`any()` in `values()` cannot be specified multiple times" - ); - } - values_any_specified = true; - if !args.is_empty() { - error!("`any()` must be empty"); - } - } else { - error!( - "`values()` arguments must be string literals or `any()`" - ); - } - } - } else { - error!( - "`cfg()` arguments must be simple identifiers, `any()` or `values(...)`" - ); - } + let expected_error = || -> ! { + error!("expected `cfg(name, values(\"value1\", \"value2\", ... \"valueN\"))`") + }; + + let Ok(mut parser) = maybe_new_parser_from_source_str(&sess, filename, s.to_string()) + else { + expected_error(); + }; + + let meta_item = match parser.parse_meta_item() { + Ok(meta_item) if parser.token == token::Eof => meta_item, + Ok(..) => expected_error(), + Err(err) => { + err.cancel(); + expected_error(); + } + }; + + let Some(args) = meta_item.meta_item_list() else { + expected_error(); + }; + + if meta_item.has_name(sym::names) { + // defaults are flipped for the old syntax + if old_syntax == None { + check_cfg.exhaustive_names = false; + check_cfg.exhaustive_values = false; + } + old_syntax = Some(true); + + check_cfg.exhaustive_names = true; + for arg in args { + if arg.is_word() && arg.ident().is_some() { + let ident = arg.ident().expect("multi-segment cfg key"); + check_cfg + .expecteds + .entry(ident.name.to_string()) + .or_insert(ExpectedValues::Any); + } else { + error!("`names()` arguments must be simple identifiers"); + } + } + } else if meta_item.has_name(sym::values) { + // defaults are flipped for the old syntax + if old_syntax == None { + check_cfg.exhaustive_names = false; + check_cfg.exhaustive_values = false; + } + old_syntax = Some(true); + + if let Some((name, values)) = args.split_first() { + if name.is_word() && name.ident().is_some() { + let ident = name.ident().expect("multi-segment cfg key"); + let expected_values = check_cfg + .expecteds + .entry(ident.name.to_string()) + .and_modify(|expected_values| match expected_values { + ExpectedValues::Some(_) => {} + ExpectedValues::Any => { + // handle the case where names(...) was done + // before values by changing to a list + *expected_values = ExpectedValues::Some(FxHashSet::default()); } + }) + .or_insert_with(|| ExpectedValues::Some(FxHashSet::default())); + + let ExpectedValues::Some(expected_values) = expected_values else { + bug!("`expected_values` should be a list a values") + }; - if values.is_empty() && !values_any_specified && !any_specified { - values.insert(None); - } else if !values.is_empty() && values_any_specified { + for val in values { + if let Some(LitKind::Str(s, _)) = val.lit().map(|lit| &lit.kind) { + expected_values.insert(Some(s.to_string())); + } else { + error!("`values()` arguments must be string literals"); + } + } + + if values.is_empty() { + expected_values.insert(None); + } + } else { + error!("`values()` first argument must be a simple identifier"); + } + } else if args.is_empty() { + check_cfg.exhaustive_values = true; + } else { + expected_error(); + } + } else if meta_item.has_name(sym::cfg) { + old_syntax = Some(false); + + let mut names = Vec::new(); + let mut values: FxHashSet<_> = Default::default(); + + let mut any_specified = false; + let mut values_specified = false; + let mut values_any_specified = false; + + for arg in args { + if arg.is_word() && let Some(ident) = arg.ident() { + if values_specified { + error!("`cfg()` names cannot be after values"); + } + names.push(ident); + } else if arg.has_name(sym::any) + && let Some(args) = arg.meta_item_list() + { + if any_specified { + error!("`any()` cannot be specified multiple times"); + } + any_specified = true; + if !args.is_empty() { + error!("`any()` must be empty"); + } + } else if arg.has_name(sym::values) + && let Some(args) = arg.meta_item_list() + { + if names.is_empty() { + error!("`values()` cannot be specified before the names"); + } else if values_specified { + error!("`values()` cannot be specified multiple times"); + } + values_specified = true; + + for arg in args { + if let Some(LitKind::Str(s, _)) = + arg.lit().map(|lit| &lit.kind) + { + values.insert(Some(s.to_string())); + } else if arg.has_name(sym::any) + && let Some(args) = arg.meta_item_list() + { + if values_any_specified { error!( - "`values()` arguments cannot specify string literals and `any()` at the same time" + "`any()` in `values()` cannot be specified multiple times" ); } - - if any_specified { - if !names.is_empty() - || !values.is_empty() - || values_any_specified - { - error!("`cfg(any())` can only be provided in isolation"); - } - - check_cfg.exhaustive_names = false; - } else { - for name in names { - check_cfg - .expecteds - .entry(name.to_string()) - .and_modify(|v| match v { - ExpectedValues::Some(v) - if !values_any_specified => - { - v.extend(values.clone()) - } - ExpectedValues::Some(_) => *v = ExpectedValues::Any, - ExpectedValues::Any => {} - }) - .or_insert_with(|| { - if values_any_specified { - ExpectedValues::Any - } else { - ExpectedValues::Some(values.clone()) - } - }); - } + values_any_specified = true; + if !args.is_empty() { + error!("`any()` must be empty"); } } else { - expected_error(); + error!( + "`values()` arguments must be string literals or `any()`" + ); } - } else { - expected_error(); } + } else { + error!( + "`cfg()` arguments must be simple identifiers, `any()` or `values(...)`" + ); } - Ok(..) => expected_error(), - Err(err) => { - err.cancel(); - expected_error(); + } + + if values.is_empty() && !values_any_specified && !any_specified { + values.insert(None); + } else if !values.is_empty() && values_any_specified { + error!( + "`values()` arguments cannot specify string literals and `any()` at the same time" + ); + } + + if any_specified { + if !names.is_empty() || !values.is_empty() || values_any_specified { + error!("`cfg(any())` can only be provided in isolation"); + } + + check_cfg.exhaustive_names = false; + } else { + for name in names { + check_cfg + .expecteds + .entry(name.to_string()) + .and_modify(|v| match v { + ExpectedValues::Some(v) if !values_any_specified => { + v.extend(values.clone()) + } + ExpectedValues::Some(_) => *v = ExpectedValues::Any, + ExpectedValues::Any => {} + }) + .or_insert_with(|| { + if values_any_specified { + ExpectedValues::Any + } else { + ExpectedValues::Some(values.clone()) + } + }); } - }, - Err(errs) => { - drop(errs); - expected_error(); } + } else { + expected_error(); } } From 1ef96a9e06124ca3ce95f9ceb3be6f24d0e7154a Mon Sep 17 00:00:00 2001 From: Urgau Date: Fri, 20 Oct 2023 19:34:45 +0200 Subject: [PATCH 06/18] Fix residual (never merged) check-cfg syntax in doc --- src/doc/unstable-book/src/compiler-flags/check-cfg.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/doc/unstable-book/src/compiler-flags/check-cfg.md b/src/doc/unstable-book/src/compiler-flags/check-cfg.md index 7a3ef5e9e2be6..0e15c79076fa4 100644 --- a/src/doc/unstable-book/src/compiler-flags/check-cfg.md +++ b/src/doc/unstable-book/src/compiler-flags/check-cfg.md @@ -139,7 +139,7 @@ fn do_mumble_frotz() {} ```bash # This turns on checking for feature values, but not for condition names. -rustc --check-cfg 'configure(feature, values("zapping", "lasers"))' \ +rustc --check-cfg 'cfg(feature, values("zapping", "lasers"))' \ --check-cfg 'cfg(any())' \ --cfg 'feature="zapping"' -Z unstable-options ``` From b7debe34e6fb2d329e21739c97ce3d7161303af5 Mon Sep 17 00:00:00 2001 From: David Tolnay Date: Thu, 26 Oct 2023 17:18:21 -0700 Subject: [PATCH 07/18] Parse rustc version at compile time --- compiler/rustc_attr/src/builtin.rs | 41 ++++--------- compiler/rustc_attr/src/lib.rs | 2 +- compiler/rustc_macros/src/current_version.rs | 59 +++++++++++++++++++ compiler/rustc_macros/src/lib.rs | 6 ++ compiler/rustc_middle/src/middle/stability.rs | 27 ++++----- compiler/rustc_session/src/lib.rs | 3 + compiler/rustc_session/src/version.rs | 19 ++++++ src/librustdoc/html/render/mod.rs | 5 +- .../clippy_utils/src/qualify_min_const_fn.rs | 23 +++----- 9 files changed, 124 insertions(+), 61 deletions(-) create mode 100644 compiler/rustc_macros/src/current_version.rs create mode 100644 compiler/rustc_session/src/version.rs diff --git a/compiler/rustc_attr/src/builtin.rs b/compiler/rustc_attr/src/builtin.rs index 363ba0d4a815b..ff23e3da43800 100644 --- a/compiler/rustc_attr/src/builtin.rs +++ b/compiler/rustc_attr/src/builtin.rs @@ -10,10 +10,9 @@ use rustc_session::config::ExpectedValues; use rustc_session::lint::builtin::UNEXPECTED_CFGS; use rustc_session::lint::BuiltinLintDiagnostics; use rustc_session::parse::{feature_err, ParseSess}; -use rustc_session::Session; +use rustc_session::{RustcVersion, Session}; use rustc_span::hygiene::Transparency; use rustc_span::{symbol::sym, symbol::Symbol, Span}; -use std::fmt::{self, Display}; use std::num::NonZeroU32; use crate::session_diagnostics::{self, IncorrectReprFormatGenericCause}; @@ -24,8 +23,6 @@ use crate::session_diagnostics::{self, IncorrectReprFormatGenericCause}; /// For more, see [this pull request](https://github.com/rust-lang/rust/pull/100591). pub const VERSION_PLACEHOLDER: &str = "CURRENT_RUSTC_VERSION"; -pub const CURRENT_RUSTC_VERSION: &str = env!("CFG_RELEASE"); - pub fn is_builtin_attr(attr: &Attribute) -> bool { attr.is_doc_comment() || attr.ident().is_some_and(|ident| is_builtin_attr_name(ident.name)) } @@ -153,7 +150,7 @@ pub enum StabilityLevel { #[derive(Encodable, Decodable, PartialEq, Copy, Clone, Debug, Eq, Hash)] #[derive(HashStable_Generic)] pub enum Since { - Version(Version), + Version(RustcVersion), /// Stabilized in the upcoming version, whatever number that is. Current, /// Failed to parse a stabilization version. @@ -382,7 +379,7 @@ fn parse_stability(sess: &Session, attr: &Attribute) -> Option<(Symbol, Stabilit let since = if let Some(since) = since { if since.as_str() == VERSION_PLACEHOLDER { Since::Current - } else if let Some(version) = parse_version(since.as_str(), false) { + } else if let Some(version) = parse_version(since) { Since::Version(version) } else { sess.emit_err(session_diagnostics::InvalidSince { span: attr.span }); @@ -567,31 +564,20 @@ fn gate_cfg(gated_cfg: &GatedCfg, cfg_span: Span, sess: &ParseSess, features: &F } } -#[derive(Encodable, Decodable, Copy, Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)] -#[derive(HashStable_Generic)] -pub struct Version { - pub major: u16, - pub minor: u16, - pub patch: u16, -} - -fn parse_version(s: &str, allow_appendix: bool) -> Option { - let mut components = s.split('-'); +/// Parse a rustc version number written inside string literal in an attribute, +/// like appears in `since = "1.0.0"`. Suffixes like "-dev" and "-nightly" are +/// not accepted in this position, unlike when parsing CFG_RELEASE. +fn parse_version(s: Symbol) -> Option { + let mut components = s.as_str().split('-'); let d = components.next()?; - if !allow_appendix && components.next().is_some() { + if components.next().is_some() { return None; } let mut digits = d.splitn(3, '.'); let major = digits.next()?.parse().ok()?; let minor = digits.next()?.parse().ok()?; let patch = digits.next().unwrap_or("0").parse().ok()?; - Some(Version { major, minor, patch }) -} - -impl Display for Version { - fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { - write!(formatter, "{}.{}.{}", self.major, self.minor, self.patch) - } + Some(RustcVersion { major, minor, patch }) } /// Evaluate a cfg-like condition (with `any` and `all`), using `eval` to @@ -623,17 +609,16 @@ pub fn eval_condition( return false; } }; - let Some(min_version) = parse_version(min_version.as_str(), false) else { + let Some(min_version) = parse_version(*min_version) else { sess.emit_warning(session_diagnostics::UnknownVersionLiteral { span: *span }); return false; }; - let rustc_version = parse_version(CURRENT_RUSTC_VERSION, true).unwrap(); // See https://github.com/rust-lang/rust/issues/64796#issuecomment-640851454 for details if sess.assume_incomplete_release { - rustc_version > min_version + RustcVersion::CURRENT > min_version } else { - rustc_version >= min_version + RustcVersion::CURRENT >= min_version } } ast::MetaItemKind::List(mis) => { diff --git a/compiler/rustc_attr/src/lib.rs b/compiler/rustc_attr/src/lib.rs index 53e3eaaab3768..868c041225581 100644 --- a/compiler/rustc_attr/src/lib.rs +++ b/compiler/rustc_attr/src/lib.rs @@ -27,6 +27,6 @@ pub use StabilityLevel::*; pub use rustc_ast::attr::*; -pub(crate) use rustc_ast::HashStableContext; +pub(crate) use rustc_session::HashStableContext; fluent_messages! { "../messages.ftl" } diff --git a/compiler/rustc_macros/src/current_version.rs b/compiler/rustc_macros/src/current_version.rs new file mode 100644 index 0000000000000..5e3b91c17bf6c --- /dev/null +++ b/compiler/rustc_macros/src/current_version.rs @@ -0,0 +1,59 @@ +use proc_macro::TokenStream; +use proc_macro2::Span; +use quote::quote; +use syn::parse::{Parse, ParseStream}; +use syn::{parenthesized, parse_macro_input, LitStr, Token}; + +pub struct Input { + variable: LitStr, +} + +mod kw { + syn::custom_keyword!(env); +} + +impl Parse for Input { + // Input syntax is `env!("CFG_RELEASE")` to facilitate grepping. + fn parse(input: ParseStream<'_>) -> syn::Result { + let paren; + input.parse::()?; + input.parse::()?; + parenthesized!(paren in input); + let variable: LitStr = paren.parse()?; + Ok(Input { variable }) + } +} + +pub(crate) fn current_version(input: TokenStream) -> TokenStream { + let input = parse_macro_input!(input as Input); + + TokenStream::from(match RustcVersion::parse_env_var(&input.variable) { + Ok(RustcVersion { major, minor, patch }) => quote!( + Self { major: #major, minor: #minor, patch: #patch } + ), + Err(err) => syn::Error::new(Span::call_site(), err).into_compile_error(), + }) +} + +struct RustcVersion { + major: u16, + minor: u16, + patch: u16, +} + +impl RustcVersion { + fn parse_env_var(env_var: &LitStr) -> Result> { + let value = proc_macro::tracked_env::var(env_var.value())?; + Self::parse_str(&value) + .ok_or_else(|| format!("failed to parse rustc version: {:?}", value).into()) + } + + fn parse_str(value: &str) -> Option { + // Ignore any suffixes such as "-dev" or "-nightly". + let mut components = value.split('-').next().unwrap().splitn(3, '.'); + let major = components.next()?.parse().ok()?; + let minor = components.next()?.parse().ok()?; + let patch = components.next().unwrap_or("0").parse().ok()?; + Some(RustcVersion { major, minor, patch }) + } +} diff --git a/compiler/rustc_macros/src/lib.rs b/compiler/rustc_macros/src/lib.rs index 776ba8f9ca11a..193dbd75fbd57 100644 --- a/compiler/rustc_macros/src/lib.rs +++ b/compiler/rustc_macros/src/lib.rs @@ -15,6 +15,7 @@ use synstructure::decl_derive; use proc_macro::TokenStream; +mod current_version; mod diagnostics; mod hash_stable; mod lift; @@ -25,6 +26,11 @@ mod symbols; mod type_foldable; mod type_visitable; +#[proc_macro] +pub fn current_rustc_version(input: TokenStream) -> TokenStream { + current_version::current_version(input) +} + #[proc_macro] pub fn rustc_queries(input: TokenStream) -> TokenStream { query::rustc_queries(input) diff --git a/compiler/rustc_middle/src/middle/stability.rs b/compiler/rustc_middle/src/middle/stability.rs index 84893b8e627a9..20547696d5a73 100644 --- a/compiler/rustc_middle/src/middle/stability.rs +++ b/compiler/rustc_middle/src/middle/stability.rs @@ -16,7 +16,7 @@ use rustc_middle::ty::print::with_no_trimmed_paths; use rustc_session::lint::builtin::{DEPRECATED, DEPRECATED_IN_FUTURE, SOFT_UNSTABLE}; use rustc_session::lint::{BuiltinLintDiagnostics, Level, Lint, LintBuffer}; use rustc_session::parse::feature_err_issue; -use rustc_session::Session; +use rustc_session::{RustcVersion, Session}; use rustc_span::symbol::{sym, Symbol}; use rustc_span::Span; use std::num::NonZeroU32; @@ -129,11 +129,6 @@ pub fn deprecation_in_effect(depr: &Deprecation) -> bool { let is_since_rustc_version = depr.is_since_rustc_version; let since = depr.since.as_ref().map(Symbol::as_str); - fn parse_version(ver: &str) -> Vec { - // We ignore non-integer components of the version (e.g., "nightly"). - ver.split(|c| c == '.' || c == '-').flat_map(|s| s.parse()).collect() - } - if !is_since_rustc_version { // The `since` field doesn't have semantic purpose without `#![staged_api]`. return true; @@ -144,16 +139,18 @@ pub fn deprecation_in_effect(depr: &Deprecation) -> bool { return false; } - if let Some(rustc) = option_env!("CFG_RELEASE") { - let since: Vec = parse_version(&since); - let rustc: Vec = parse_version(rustc); - // We simply treat invalid `since` attributes as relating to a previous - // Rust version, thus always displaying the warning. - if since.len() != 3 { - return true; - } - return since <= rustc; + // We ignore non-integer components of the version (e.g., "nightly"). + let since: Vec = + since.split(|c| c == '.' || c == '-').flat_map(|s| s.parse()).collect(); + + // We simply treat invalid `since` attributes as relating to a previous + // Rust version, thus always displaying the warning. + if since.len() != 3 { + return true; } + + let rustc = RustcVersion::CURRENT; + return since.as_slice() <= &[rustc.major, rustc.minor, rustc.patch]; }; // Assume deprecation is in effect if "since" field is missing diff --git a/compiler/rustc_session/src/lib.rs b/compiler/rustc_session/src/lib.rs index 7da0bcf01bfbf..17ac3e991c58c 100644 --- a/compiler/rustc_session/src/lib.rs +++ b/compiler/rustc_session/src/lib.rs @@ -43,6 +43,9 @@ pub mod output; pub use getopts; +mod version; +pub use version::RustcVersion; + fluent_messages! { "../messages.ftl" } /// Requirements for a `StableHashingContext` to be used in this crate. diff --git a/compiler/rustc_session/src/version.rs b/compiler/rustc_session/src/version.rs new file mode 100644 index 0000000000000..1ad8620bfba55 --- /dev/null +++ b/compiler/rustc_session/src/version.rs @@ -0,0 +1,19 @@ +use std::fmt::{self, Display}; + +#[derive(Encodable, Decodable, Copy, Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)] +#[derive(HashStable_Generic)] +pub struct RustcVersion { + pub major: u16, + pub minor: u16, + pub patch: u16, +} + +impl RustcVersion { + pub const CURRENT: Self = current_rustc_version!(env!("CFG_RELEASE")); +} + +impl Display for RustcVersion { + fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(formatter, "{}.{}.{}", self.major, self.minor, self.patch) + } +} diff --git a/src/librustdoc/html/render/mod.rs b/src/librustdoc/html/render/mod.rs index e01341acf438e..88d01b7a5e57d 100644 --- a/src/librustdoc/html/render/mod.rs +++ b/src/librustdoc/html/render/mod.rs @@ -48,13 +48,14 @@ use std::str; use std::string::ToString; use askama::Template; -use rustc_attr::{ConstStability, Deprecation, Since, StabilityLevel, CURRENT_RUSTC_VERSION}; +use rustc_attr::{ConstStability, Deprecation, Since, StabilityLevel}; use rustc_data_structures::captures::Captures; use rustc_data_structures::fx::{FxHashMap, FxHashSet}; use rustc_hir::def_id::{DefId, DefIdSet}; use rustc_hir::Mutability; use rustc_middle::middle::stability; use rustc_middle::ty::{self, TyCtxt}; +use rustc_session::RustcVersion; use rustc_span::{ symbol::{sym, Symbol}, BytePos, FileName, RealFileName, @@ -979,7 +980,7 @@ fn render_stability_since_raw_with_extra( fn since_to_string(since: &Since) -> Option { match since { Since::Version(since) => Some(since.to_string()), - Since::Current => Some(CURRENT_RUSTC_VERSION.to_owned()), + Since::Current => Some(RustcVersion::CURRENT.to_string()), Since::Err => None, } } diff --git a/src/tools/clippy/clippy_utils/src/qualify_min_const_fn.rs b/src/tools/clippy/clippy_utils/src/qualify_min_const_fn.rs index 1e465ac91b76e..31f7b87de635e 100644 --- a/src/tools/clippy/clippy_utils/src/qualify_min_const_fn.rs +++ b/src/tools/clippy/clippy_utils/src/qualify_min_const_fn.rs @@ -5,7 +5,7 @@ use crate::msrvs::Msrv; use hir::LangItem; -use rustc_attr::{Since, CURRENT_RUSTC_VERSION}; +use rustc_attr::Since; use rustc_const_eval::transform::check_consts::ConstCx; use rustc_hir as hir; use rustc_hir::def_id::DefId; @@ -372,23 +372,16 @@ fn is_const_fn(tcx: TyCtxt<'_>, def_id: DefId, msrv: &Msrv) -> bool { // as a part of an unimplemented MSRV check https://github.com/rust-lang/rust/issues/65262. let const_stab_rust_version = match since { - Since::Version(version) => RustcVersion::new( - u32::from(version.major), - u32::from(version.minor), - u32::from(version.patch), - ), - Since::Current => { - // HACK(nilstrieb): CURRENT_RUSTC_VERSION can return versions like 1.66.0-dev. - // `rustc-semver` doesn't accept the `-dev` version number so we have to strip it off. - let short_version = CURRENT_RUSTC_VERSION.split('-').next().unwrap(); - RustcVersion::parse(short_version).unwrap_or_else(|err| { - panic!("`rustc_attr::StabilityLevel::Stable::since` is ill-formatted: `{CURRENT_RUSTC_VERSION}`, {err:?}") - }) - }, + Since::Version(version) => version, + Since::Current => rustc_session::RustcVersion::CURRENT, Since::Err => return false, }; - msrv.meets(const_stab_rust_version) + msrv.meets(RustcVersion::new( + u32::from(const_stab_rust_version.major), + u32::from(const_stab_rust_version.minor), + u32::from(const_stab_rust_version.patch), + )) } else { // Unstable const fn with the feature enabled. msrv.current().is_none() From 84a1a689ccc8eb6b8be48f3385bc9b0aeb5c71a6 Mon Sep 17 00:00:00 2001 From: Urgau Date: Sat, 21 Oct 2023 19:11:24 +0200 Subject: [PATCH 08/18] Better guard against wrong input with check-cfg any() --- compiler/rustc_interface/src/interface.rs | 10 +++++++--- tests/ui/check-cfg/invalid-arguments.any_values.stderr | 2 ++ tests/ui/check-cfg/invalid-arguments.rs | 4 +++- .../ui/check-cfg/invalid-arguments.unterminated.stderr | 2 ++ 4 files changed, 14 insertions(+), 4 deletions(-) create mode 100644 tests/ui/check-cfg/invalid-arguments.any_values.stderr create mode 100644 tests/ui/check-cfg/invalid-arguments.unterminated.stderr diff --git a/compiler/rustc_interface/src/interface.rs b/compiler/rustc_interface/src/interface.rs index 2218892538212..422c2fc65ad6a 100644 --- a/compiler/rustc_interface/src/interface.rs +++ b/compiler/rustc_interface/src/interface.rs @@ -311,11 +311,15 @@ pub fn parse_check_cfg(handler: &EarlyErrorHandler, specs: Vec) -> Check } if any_specified { - if !names.is_empty() || !values.is_empty() || values_any_specified { + if names.is_empty() + && values.is_empty() + && !values_specified + && !values_any_specified + { + check_cfg.exhaustive_names = false; + } else { error!("`cfg(any())` can only be provided in isolation"); } - - check_cfg.exhaustive_names = false; } else { for name in names { check_cfg diff --git a/tests/ui/check-cfg/invalid-arguments.any_values.stderr b/tests/ui/check-cfg/invalid-arguments.any_values.stderr new file mode 100644 index 0000000000000..f9a9c4a6e1328 --- /dev/null +++ b/tests/ui/check-cfg/invalid-arguments.any_values.stderr @@ -0,0 +1,2 @@ +error: invalid `--check-cfg` argument: `cfg(any(),values())` (`values()` cannot be specified before the names) + diff --git a/tests/ui/check-cfg/invalid-arguments.rs b/tests/ui/check-cfg/invalid-arguments.rs index 79bef89c95740..a56f48e0af93d 100644 --- a/tests/ui/check-cfg/invalid-arguments.rs +++ b/tests/ui/check-cfg/invalid-arguments.rs @@ -6,7 +6,7 @@ // revisions: multiple_values_any not_empty_any not_empty_values_any // revisions: values_any_missing_values values_any_before_ident ident_in_values_1 // revisions: ident_in_values_2 unknown_meta_item_1 unknown_meta_item_2 unknown_meta_item_3 -// revisions: mixed_values_any mixed_any giberich +// revisions: mixed_values_any mixed_any any_values giberich unterminated // // compile-flags: -Z unstable-options // [anything_else]compile-flags: --check-cfg=anything_else(...) @@ -29,6 +29,8 @@ // [unknown_meta_item_3]compile-flags: --check-cfg=cfg(foo,values(test())) // [mixed_values_any]compile-flags: --check-cfg=cfg(foo,values("bar",any())) // [mixed_any]compile-flags: --check-cfg=cfg(any(),values(any())) +// [any_values]compile-flags: --check-cfg=cfg(any(),values()) // [giberich]compile-flags: --check-cfg=cfg(...) +// [unterminated]compile-flags: --check-cfg=cfg( fn main() {} diff --git a/tests/ui/check-cfg/invalid-arguments.unterminated.stderr b/tests/ui/check-cfg/invalid-arguments.unterminated.stderr new file mode 100644 index 0000000000000..80161a6aa0fc8 --- /dev/null +++ b/tests/ui/check-cfg/invalid-arguments.unterminated.stderr @@ -0,0 +1,2 @@ +error: invalid `--check-cfg` argument: `cfg(` (expected `cfg(name, values("value1", "value2", ... "valueN"))`) + From 2ef5897a89f9bbd9d38b9eb1222b5bedef98814b Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Fri, 27 Oct 2023 13:19:24 +0200 Subject: [PATCH 09/18] fix failure to detect a too-big-type after adding padding --- compiler/rustc_abi/src/layout.rs | 5 +++++ .../rustc_ty_utils/src/layout_sanity_check.rs | 3 +++ tests/ui/layout/too-big-with-padding.rs | 18 ++++++++++++++++++ tests/ui/layout/too-big-with-padding.stderr | 8 ++++++++ 4 files changed, 34 insertions(+) create mode 100644 tests/ui/layout/too-big-with-padding.rs create mode 100644 tests/ui/layout/too-big-with-padding.stderr diff --git a/compiler/rustc_abi/src/layout.rs b/compiler/rustc_abi/src/layout.rs index 00d862ca27b7b..9127e1d06e88f 100644 --- a/compiler/rustc_abi/src/layout.rs +++ b/compiler/rustc_abi/src/layout.rs @@ -539,6 +539,7 @@ pub trait LayoutCalculator { // Align the maximum variant size to the largest alignment. size = size.align_to(align.abi); + // FIXME(oli-obk): deduplicate and harden these checks if size.bytes() >= dl.obj_size_bound() { return None; } @@ -1103,6 +1104,10 @@ fn univariant< inverse_memory_index.into_iter().map(|it| it.index() as u32).collect() }; let size = min_size.align_to(align.abi); + // FIXME(oli-obk): deduplicate and harden these checks + if size.bytes() >= dl.obj_size_bound() { + return None; + } let mut layout_of_single_non_zst_field = None; let mut abi = Abi::Aggregate { sized }; // Try to make this a Scalar/ScalarPair. diff --git a/compiler/rustc_ty_utils/src/layout_sanity_check.rs b/compiler/rustc_ty_utils/src/layout_sanity_check.rs index 8633334381ada..6332c614a90bb 100644 --- a/compiler/rustc_ty_utils/src/layout_sanity_check.rs +++ b/compiler/rustc_ty_utils/src/layout_sanity_check.rs @@ -19,6 +19,9 @@ pub(super) fn sanity_check_layout<'tcx>( if layout.size.bytes() % layout.align.abi.bytes() != 0 { bug!("size is not a multiple of align, in the following layout:\n{layout:#?}"); } + if layout.size.bytes() >= cx.tcx.data_layout.obj_size_bound() { + bug!("size is too large, in the following layout:\n{layout:#?}"); + } if !cfg!(debug_assertions) { // Stop here, the rest is kind of expensive. diff --git a/tests/ui/layout/too-big-with-padding.rs b/tests/ui/layout/too-big-with-padding.rs new file mode 100644 index 0000000000000..cf41ac872c21b --- /dev/null +++ b/tests/ui/layout/too-big-with-padding.rs @@ -0,0 +1,18 @@ +// build-fail +// compile-flags: --target i686-unknown-linux-gnu --crate-type lib +// needs-llvm-components: x86 +#![feature(no_core, lang_items)] +#![allow(internal_features)] +#![no_std] +#![no_core] + +// 0x7fffffff is fine, but after rounding up it becomes too big +#[repr(C, align(2))] +pub struct Example([u8; 0x7fffffff]); + +pub fn lib(_x: Example) {} //~ERROR: too big for the current architecture + +#[lang = "sized"] +pub trait Sized {} +#[lang = "copy"] +pub trait Copy: Sized {} diff --git a/tests/ui/layout/too-big-with-padding.stderr b/tests/ui/layout/too-big-with-padding.stderr new file mode 100644 index 0000000000000..5cc854adce0d3 --- /dev/null +++ b/tests/ui/layout/too-big-with-padding.stderr @@ -0,0 +1,8 @@ +error: values of the type `Example` are too big for the current architecture + --> $DIR/too-big-with-padding.rs:13:1 + | +LL | pub fn lib(_x: Example) {} + | ^^^^^^^^^^^^^^^^^^^^^^^ + +error: aborting due to previous error + From 6dbad236419075603a616e7b19f585a1a308cf56 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Thu, 19 Oct 2023 16:34:41 +0000 Subject: [PATCH 10/18] When encountering sealed traits, point types that implement it ``` error[E0277]: the trait bound `S: d::Hidden` is not satisfied --> $DIR/sealed-trait-local.rs:53:20 | LL | impl c::Sealed for S {} | ^ the trait `d::Hidden` is not implemented for `S` | note: required by a bound in `c::Sealed` --> $DIR/sealed-trait-local.rs:17:23 | LL | pub trait Sealed: self::d::Hidden { | ^^^^^^^^^^^^^^^ required by this bound in `Sealed` = note: `Sealed` is a "sealed trait", because to implement it you also need to implement `c::d::Hidden`, which is not accessible; this is usually done to force you to use one of the provided types that already implement it = help: the following types implement the trait: - c::X - c::Y ``` The last `help` is new. --- .../src/traits/error_reporting/suggestions.rs | 30 ++++++++++++- .../sealed-traits/sealed-trait-local.rs | 40 ++++++++++++++++- .../sealed-traits/sealed-trait-local.stderr | 44 ++++++++++++++++--- 3 files changed, 105 insertions(+), 9 deletions(-) diff --git a/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs b/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs index 947ea3eece3f0..aad07d7683a75 100644 --- a/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs +++ b/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs @@ -2691,8 +2691,6 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> { if let DefKind::Trait = tcx.def_kind(item_def_id) && !visible_item { - // FIXME(estebank): extend this to search for all the types that do - // implement this trait and list them. err.note(format!( "`{short_item_name}` is a \"sealed trait\", because to implement \ it you also need to implement `{}`, which is not accessible; \ @@ -2700,6 +2698,34 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> { types that already implement it", with_no_trimmed_paths!(tcx.def_path_str(def_id)), )); + let impls_of = tcx.trait_impls_of(def_id); + let impls = impls_of + .non_blanket_impls() + .values() + .flatten() + .chain(impls_of.blanket_impls().iter()) + .collect::>(); + if !impls.is_empty() { + let len = impls.len(); + let mut types = impls.iter() + .map(|t| with_no_trimmed_paths!(format!( + " {}", + tcx.type_of(*t).instantiate_identity(), + ))) + .collect::>(); + let post = if types.len() > 9 { + types.truncate(8); + format!("\nand {} others", len - 8) + } else { + String::new() + }; + err.help(format!( + "the following type{} implement{} the trait:\n{}{post}", + pluralize!(len), + if len == 1 { "s" } else { "" }, + types.join("\n"), + )); + } } } } else { diff --git a/tests/ui/privacy/sealed-traits/sealed-trait-local.rs b/tests/ui/privacy/sealed-traits/sealed-trait-local.rs index 778ddf0f81763..9ae01259a7810 100644 --- a/tests/ui/privacy/sealed-traits/sealed-trait-local.rs +++ b/tests/ui/privacy/sealed-traits/sealed-trait-local.rs @@ -13,7 +13,43 @@ pub mod a { } } -struct S; -impl a::Sealed for S {} //~ ERROR the trait bound `S: Hidden` is not satisfied +pub mod c { + pub trait Sealed: self::d::Hidden { + fn foo() {} + } + + struct X; + impl Sealed for X {} + impl self::d::Hidden for X {} + + struct Y; + impl Sealed for Y {} + impl self::d::Hidden for Y {} + + mod d { + pub trait Hidden {} + } +} +pub mod e { + pub trait Sealed: self::f::Hidden { + fn foo() {} + } + + struct X; + impl self::f::Hidden for X {} + + struct Y; + impl self::f::Hidden for Y {} + impl Sealed for T {} + + mod f { + pub trait Hidden {} + } +} + +struct S; +impl a::Sealed for S {} //~ ERROR the trait bound +impl c::Sealed for S {} //~ ERROR the trait bound +impl e::Sealed for S {} //~ ERROR the trait bound fn main() {} diff --git a/tests/ui/privacy/sealed-traits/sealed-trait-local.stderr b/tests/ui/privacy/sealed-traits/sealed-trait-local.stderr index 5f8076fc84d0e..a7f77a1c0c020 100644 --- a/tests/ui/privacy/sealed-traits/sealed-trait-local.stderr +++ b/tests/ui/privacy/sealed-traits/sealed-trait-local.stderr @@ -1,16 +1,50 @@ -error[E0277]: the trait bound `S: Hidden` is not satisfied - --> $DIR/sealed-trait-local.rs:17:20 +error[E0277]: the trait bound `S: b::Hidden` is not satisfied + --> $DIR/sealed-trait-local.rs:52:20 | LL | impl a::Sealed for S {} - | ^ the trait `Hidden` is not implemented for `S` + | ^ the trait `b::Hidden` is not implemented for `S` | -note: required by a bound in `Sealed` +note: required by a bound in `a::Sealed` --> $DIR/sealed-trait-local.rs:3:23 | LL | pub trait Sealed: self::b::Hidden { | ^^^^^^^^^^^^^^^ required by this bound in `Sealed` = note: `Sealed` is a "sealed trait", because to implement it you also need to implement `a::b::Hidden`, which is not accessible; this is usually done to force you to use one of the provided types that already implement it + = help: the following type implements the trait: + a::X -error: aborting due to previous error +error[E0277]: the trait bound `S: d::Hidden` is not satisfied + --> $DIR/sealed-trait-local.rs:53:20 + | +LL | impl c::Sealed for S {} + | ^ the trait `d::Hidden` is not implemented for `S` + | +note: required by a bound in `c::Sealed` + --> $DIR/sealed-trait-local.rs:17:23 + | +LL | pub trait Sealed: self::d::Hidden { + | ^^^^^^^^^^^^^^^ required by this bound in `Sealed` + = note: `Sealed` is a "sealed trait", because to implement it you also need to implement `c::d::Hidden`, which is not accessible; this is usually done to force you to use one of the provided types that already implement it + = help: the following types implement the trait: + c::X + c::Y + +error[E0277]: the trait bound `S: f::Hidden` is not satisfied + --> $DIR/sealed-trait-local.rs:54:20 + | +LL | impl e::Sealed for S {} + | ^ the trait `f::Hidden` is not implemented for `S` + | +note: required by a bound in `e::Sealed` + --> $DIR/sealed-trait-local.rs:35:23 + | +LL | pub trait Sealed: self::f::Hidden { + | ^^^^^^^^^^^^^^^ required by this bound in `Sealed` + = note: `Sealed` is a "sealed trait", because to implement it you also need to implement `e::f::Hidden`, which is not accessible; this is usually done to force you to use one of the provided types that already implement it + = help: the following types implement the trait: + e::X + e::Y + +error: aborting due to 3 previous errors For more information about this error, try `rustc --explain E0277`. From a65d99d08702b9ceea0d153e432f5bd5f58a783a Mon Sep 17 00:00:00 2001 From: Milo <50248166+Milo123459@users.noreply.github.com> Date: Fri, 27 Oct 2023 20:14:49 +0100 Subject: [PATCH 11/18] Update type_err_ctxt_ext.rs Co-authored-by: Esteban Kuber --- .../src/traits/error_reporting/type_err_ctxt_ext.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/compiler/rustc_trait_selection/src/traits/error_reporting/type_err_ctxt_ext.rs b/compiler/rustc_trait_selection/src/traits/error_reporting/type_err_ctxt_ext.rs index f83ee576c2941..3e7b713208214 100644 --- a/compiler/rustc_trait_selection/src/traits/error_reporting/type_err_ctxt_ext.rs +++ b/compiler/rustc_trait_selection/src/traits/error_reporting/type_err_ctxt_ext.rs @@ -437,10 +437,10 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> { format!(" in `{t}`"), format!("within `{t}`, "), s.map(|s| (format!("within this `{t}`"), s)), - file.and_then(|file| Some(format!( + file.map(|file| format!( "the full trait has been written to '{}'", file.display(), - ))) + )) ) }) .unwrap_or_default(); From 98c469ce933612e9688f6b5a2ed9f83c1bb028ac Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Thu, 26 Oct 2023 17:12:57 +1100 Subject: [PATCH 12/18] Remove an unneeded dependency. --- Cargo.lock | 1 - compiler/rustc_interface/Cargo.toml | 1 - 2 files changed, 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index ff21d5f8c0861..bd121921b0b96 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4054,7 +4054,6 @@ dependencies = [ "rustc_hir_analysis", "rustc_hir_typeck", "rustc_incremental", - "rustc_index", "rustc_lint", "rustc_macros", "rustc_metadata", diff --git a/compiler/rustc_interface/Cargo.toml b/compiler/rustc_interface/Cargo.toml index ae008674d0127..b280665057cc3 100644 --- a/compiler/rustc_interface/Cargo.toml +++ b/compiler/rustc_interface/Cargo.toml @@ -26,7 +26,6 @@ rustc_middle = { path = "../rustc_middle" } rustc_ast_lowering = { path = "../rustc_ast_lowering" } rustc_ast_passes = { path = "../rustc_ast_passes" } rustc_incremental = { path = "../rustc_incremental" } -rustc_index = { path = "../rustc_index" } rustc_traits = { path = "../rustc_traits" } rustc_data_structures = { path = "../rustc_data_structures" } rustc_codegen_ssa = { path = "../rustc_codegen_ssa" } From 3feec48d70efc479ca0c14d65d89f001330b4fc1 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Fri, 27 Oct 2023 08:24:49 +1100 Subject: [PATCH 13/18] Fix a comment. --- compiler/rustc_interface/src/callbacks.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/rustc_interface/src/callbacks.rs b/compiler/rustc_interface/src/callbacks.rs index 45b1aeb4a5c36..6fa989bb96ce0 100644 --- a/compiler/rustc_interface/src/callbacks.rs +++ b/compiler/rustc_interface/src/callbacks.rs @@ -26,7 +26,7 @@ fn track_span_parent(def_id: rustc_span::def_id::LocalDefId) { }) } -/// This is a callback from `rustc_ast` as it cannot access the implicit state +/// This is a callback from `rustc_errors` as it cannot access the implicit state /// in `rustc_middle` otherwise. It is used when diagnostic messages are /// emitted and stores them in the current query, if there is one. fn track_diagnostic(diagnostic: &mut Diagnostic, f: &mut dyn FnMut(&mut Diagnostic)) { From 2142d014ab9010ed87b3d5c865190bd1210c9073 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Fri, 27 Oct 2023 15:40:21 +1100 Subject: [PATCH 14/18] Streamline `rustc_interface` tests. In `test_edition_parsing`, change the `build_session_options_and_crate_config` call to `build_session_options`, because the config isn't used. That leaves a single call site for `build_session_options_and_crate_config`, so just inline and remove it. --- compiler/rustc_interface/src/tests.rs | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-) diff --git a/compiler/rustc_interface/src/tests.rs b/compiler/rustc_interface/src/tests.rs index 3d191669b1adb..444c184dc2dd3 100644 --- a/compiler/rustc_interface/src/tests.rs +++ b/compiler/rustc_interface/src/tests.rs @@ -39,18 +39,10 @@ use std::sync::Arc; type CfgSpecs = FxHashSet<(String, Option)>; -fn build_session_options_and_crate_config( - handler: &mut EarlyErrorHandler, - matches: getopts::Matches, -) -> (Options, CfgSpecs) { - let sessopts = build_session_options(handler, &matches); - let cfg = parse_cfgspecs(handler, matches.opt_strs("cfg")); - (sessopts, cfg) -} - fn mk_session(handler: &mut EarlyErrorHandler, matches: getopts::Matches) -> (Session, CfgSpecs) { let registry = registry::Registry::new(&[]); - let (sessopts, cfg) = build_session_options_and_crate_config(handler, matches); + let sessopts = build_session_options(handler, &matches); + let cfg = parse_cfgspecs(handler, matches.opt_strs("cfg")); let temps_dir = sessopts.unstable_opts.temps_dir.as_deref().map(PathBuf::from); let io = CompilerIO { input: Input::Str { name: FileName::Custom(String::new()), input: String::new() }, @@ -880,6 +872,6 @@ fn test_edition_parsing() { let mut handler = EarlyErrorHandler::new(ErrorOutputType::default()); let matches = optgroups().parse(&["--edition=2018".to_string()]).unwrap(); - let (sessopts, _) = build_session_options_and_crate_config(&mut handler, matches); + let sessopts = build_session_options(&mut handler, &matches); assert!(sessopts.edition == Edition::Edition2018) } From 32986d895f9105de00a87d7f3d8897477ab64803 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Fri, 27 Oct 2023 15:49:09 +1100 Subject: [PATCH 15/18] Change `CrateConfig` from `FxIndexSet` to `FxHashSet`. Because its order doesn't matter. This is well demonstrated by `to_crate_config`, which creates a `CrateConfig` from an `FxHashSet`. --- compiler/rustc_session/src/parse.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/compiler/rustc_session/src/parse.rs b/compiler/rustc_session/src/parse.rs index abb0ab5630c13..316b9cc7acb18 100644 --- a/compiler/rustc_session/src/parse.rs +++ b/compiler/rustc_session/src/parse.rs @@ -9,7 +9,7 @@ use crate::lint::{ builtin::UNSTABLE_SYNTAX_PRE_EXPANSION, BufferedEarlyLint, BuiltinLintDiagnostics, Lint, LintId, }; use rustc_ast::node_id::NodeId; -use rustc_data_structures::fx::{FxHashMap, FxHashSet, FxIndexSet}; +use rustc_data_structures::fx::{FxHashMap, FxHashSet}; use rustc_data_structures::sync::{AppendOnlyVec, Lock, Lrc}; use rustc_errors::{emitter::SilentEmitter, Handler}; use rustc_errors::{ @@ -27,7 +27,7 @@ use std::str; /// The set of keys (and, optionally, values) that define the compilation /// environment of the crate, used to drive conditional compilation. -pub type CrateConfig = FxIndexSet<(Symbol, Option)>; +pub type CrateConfig = FxHashSet<(Symbol, Option)>; pub type CrateCheckConfig = CheckCfg; /// Collected spans during parsing for places where a certain feature was @@ -237,7 +237,7 @@ impl ParseSess { Self { span_diagnostic: handler, unstable_features: UnstableFeatures::from_environment(None), - config: FxIndexSet::default(), + config: FxHashSet::default(), check_config: CrateCheckConfig::default(), edition: ExpnId::root().expn_data().edition, raw_identifier_spans: Default::default(), From 75e415ba86cc16ee5c0fd3c9007c94ea98995ab3 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Fri, 27 Oct 2023 15:54:14 +1100 Subject: [PATCH 16/18] Optimize `parse_cfgspecs`. In `parse_cfg`, we now construct a `FxHashSet` directly instead of constructing a `FxHashSet` and then immediately converting it to a `FxHashSet`(!) (The type names made this behaviour non-obvious. The next commit will make the type names clearer.) --- compiler/rustc_interface/src/interface.rs | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/compiler/rustc_interface/src/interface.rs b/compiler/rustc_interface/src/interface.rs index 87badcbdf15ba..26d7d26c31cb5 100644 --- a/compiler/rustc_interface/src/interface.rs +++ b/compiler/rustc_interface/src/interface.rs @@ -16,7 +16,7 @@ use rustc_parse::maybe_new_parser_from_source_str; use rustc_query_impl::QueryCtxt; use rustc_query_system::query::print_query_stack; use rustc_session::config::{self, CheckCfg, ExpectedValues, Input, OutFileName, OutputFilenames}; -use rustc_session::parse::{CrateConfig, ParseSess}; +use rustc_session::parse::ParseSess; use rustc_session::CompilerIO; use rustc_session::Session; use rustc_session::{lint, EarlyErrorHandler}; @@ -67,7 +67,7 @@ pub fn parse_cfgspecs( cfgspecs: Vec, ) -> FxHashSet<(String, Option)> { rustc_span::create_default_session_if_not_set_then(move |_| { - let cfg = cfgspecs + cfgspecs .into_iter() .map(|s| { let sess = ParseSess::with_silent_emitter(Some(format!( @@ -97,7 +97,10 @@ pub fn parse_cfgspecs( } MetaItemKind::NameValue(..) | MetaItemKind::Word => { let ident = meta_item.ident().expect("multi-segment cfg key"); - return (ident.name, meta_item.value_str()); + return ( + ident.name.to_string(), + meta_item.value_str().map(|sym| sym.to_string()), + ); } } } @@ -118,8 +121,7 @@ pub fn parse_cfgspecs( error!(r#"expected `key` or `key="value"`"#); } }) - .collect::(); - cfg.into_iter().map(|(a, b)| (a.to_string(), b.map(|b| b.to_string()))).collect() + .collect::>() }) } From 5e5499715714a383d76a3e69807e22093b616c66 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Fri, 27 Oct 2023 15:58:02 +1100 Subject: [PATCH 17/18] Clean up config mess. `parse_cfgspecs` and `parse_check_cfg` run very early, before the main interner is running. They each use a short-lived interner and convert all interned symbols to strings in their output data structures. Once the main interner starts up, these data structures get converted into new data structures that are identical except with the strings converted to symbols. All is not obvious from the current code, which is a mess, particularly with inconsistent naming that obscures the parallel string/symbol data structures. This commit clean things up a lot. - The existing `CheckCfg` type is generic, allowing both `CheckCfg` and `CheckCfg` forms. This is really useful, but it defaults to `String`. The commit removes the default so we have to use `CheckCfg` and `CheckCfg` explicitly, which makes things clearer. - Introduces `Cfg`, which is generic over `String` and `Symbol`, similar to `CheckCfg`. - Renames some things. - `parse_cfgspecs` -> `parse_cfg` - `CfgSpecs` -> `Cfg`, plus it's used in more places, rather than the underlying `FxHashSet` type. - `CrateConfig` -> `Cfg`. - `CrateCheckConfig` -> `CheckCfg` - Adds some comments explaining the string-to-symbol conversions. - `to_crate_check_config`, which converts `CheckCfg` to `CheckCfg`, is inlined and removed and combined with the overly-general `CheckCfg::map_data` to produce `CheckCfg::::intern`. - `build_configuration` now does the `Cfg`-to-`Cfg` conversion, so callers don't need to, which removes the need for `to_crate_config`. The diff for two of the fields in `Config` is a good example of the improved clarity: ``` - pub crate_cfg: FxHashSet<(String, Option)>, - pub crate_check_cfg: CheckCfg, + pub crate_cfg: Cfg, + pub crate_check_cfg: CheckCfg, ``` Compare that with the diff for the corresponding fields in `ParseSess`, and the relationship to `Config` is much clearer than before: ``` - pub config: CrateConfig, - pub check_config: CrateCheckConfig, + pub config: Cfg, + pub check_config: CheckCfg, ``` --- compiler/rustc_driver_impl/src/lib.rs | 2 +- compiler/rustc_interface/src/interface.rs | 27 +++++++------ compiler/rustc_interface/src/tests.rs | 21 +++++----- compiler/rustc_interface/src/util.rs | 20 +++++----- compiler/rustc_session/src/config.rs | 47 +++++++++++------------ compiler/rustc_session/src/parse.rs | 15 +++----- src/librustdoc/core.rs | 2 +- src/librustdoc/doctest.rs | 2 +- 8 files changed, 65 insertions(+), 71 deletions(-) diff --git a/compiler/rustc_driver_impl/src/lib.rs b/compiler/rustc_driver_impl/src/lib.rs index 331843ab05151..635930742fc8b 100644 --- a/compiler/rustc_driver_impl/src/lib.rs +++ b/compiler/rustc_driver_impl/src/lib.rs @@ -318,7 +318,7 @@ fn run_compiler( return Ok(()); } - let cfg = interface::parse_cfgspecs(&early_error_handler, matches.opt_strs("cfg")); + let cfg = interface::parse_cfg(&early_error_handler, matches.opt_strs("cfg")); let check_cfg = interface::parse_check_cfg(&early_error_handler, matches.opt_strs("check-cfg")); let (odir, ofile) = make_output(&matches); let mut config = interface::Config { diff --git a/compiler/rustc_interface/src/interface.rs b/compiler/rustc_interface/src/interface.rs index 26d7d26c31cb5..7672523292b4f 100644 --- a/compiler/rustc_interface/src/interface.rs +++ b/compiler/rustc_interface/src/interface.rs @@ -15,7 +15,9 @@ use rustc_middle::{bug, ty}; use rustc_parse::maybe_new_parser_from_source_str; use rustc_query_impl::QueryCtxt; use rustc_query_system::query::print_query_stack; -use rustc_session::config::{self, CheckCfg, ExpectedValues, Input, OutFileName, OutputFilenames}; +use rustc_session::config::{ + self, Cfg, CheckCfg, ExpectedValues, Input, OutFileName, OutputFilenames, +}; use rustc_session::parse::ParseSess; use rustc_session::CompilerIO; use rustc_session::Session; @@ -61,14 +63,13 @@ impl Compiler { } } -/// Converts strings provided as `--cfg [cfgspec]` into a `crate_cfg`. -pub fn parse_cfgspecs( - handler: &EarlyErrorHandler, - cfgspecs: Vec, -) -> FxHashSet<(String, Option)> { +/// Converts strings provided as `--cfg [cfgspec]` into a `Cfg`. +pub fn parse_cfg(handler: &EarlyErrorHandler, cfgs: Vec) -> Cfg { + // This creates a short-lived `SessionGlobals`, containing an interner. The + // parsed values are converted from symbols to strings before exiting + // because the symbols are meaningless once the interner is gone. rustc_span::create_default_session_if_not_set_then(move |_| { - cfgspecs - .into_iter() + cfgs.into_iter() .map(|s| { let sess = ParseSess::with_silent_emitter(Some(format!( "this error occurred on the command line: `--cfg={s}`" @@ -121,12 +122,14 @@ pub fn parse_cfgspecs( error!(r#"expected `key` or `key="value"`"#); } }) - .collect::>() + .collect::>() }) } /// Converts strings provided as `--check-cfg [specs]` into a `CheckCfg`. -pub fn parse_check_cfg(handler: &EarlyErrorHandler, specs: Vec) -> CheckCfg { +pub fn parse_check_cfg(handler: &EarlyErrorHandler, specs: Vec) -> CheckCfg { + // The comment about `SessionGlobals` and symbols in `parse_cfg` above + // applies here too. rustc_span::create_default_session_if_not_set_then(move |_| { // If any --check-cfg is passed then exhaustive_values and exhaustive_names // are enabled by default. @@ -374,8 +377,8 @@ pub struct Config { pub opts: config::Options, /// cfg! configuration in addition to the default ones - pub crate_cfg: FxHashSet<(String, Option)>, - pub crate_check_cfg: CheckCfg, + pub crate_cfg: Cfg, + pub crate_check_cfg: CheckCfg, pub input: Input, pub output_dir: Option, diff --git a/compiler/rustc_interface/src/tests.rs b/compiler/rustc_interface/src/tests.rs index 444c184dc2dd3..26cd83ac9687b 100644 --- a/compiler/rustc_interface/src/tests.rs +++ b/compiler/rustc_interface/src/tests.rs @@ -1,17 +1,16 @@ #![allow(rustc::bad_opt_access)] -use crate::interface::parse_cfgspecs; - -use rustc_data_structures::fx::FxHashSet; +use crate::interface::parse_cfg; use rustc_data_structures::profiling::TimePassesFormat; use rustc_errors::{emitter::HumanReadableErrorType, registry, ColorConfig}; use rustc_session::config::rustc_optgroups; +use rustc_session::config::Cfg; use rustc_session::config::DebugInfo; use rustc_session::config::Input; use rustc_session::config::InstrumentXRay; use rustc_session::config::LinkSelfContained; use rustc_session::config::Polonius; use rustc_session::config::TraitSolver; -use rustc_session::config::{build_configuration, build_session_options, to_crate_config}; +use rustc_session::config::{build_configuration, build_session_options}; use rustc_session::config::{ BranchProtection, Externs, OomStrategy, OutFileName, OutputType, OutputTypes, PAuthKey, PacRet, ProcMacroExecutionStrategy, SymbolManglingVersion, WasiExecModel, @@ -31,18 +30,18 @@ use rustc_span::FileName; use rustc_span::SourceFileHashAlgorithm; use rustc_target::spec::{CodeModel, LinkerFlavorCli, MergeFunctions, PanicStrategy, RelocModel}; use rustc_target::spec::{RelroLevel, SanitizerSet, SplitDebuginfo, StackProtector, TlsModel}; - use std::collections::{BTreeMap, BTreeSet}; use std::num::NonZeroUsize; use std::path::{Path, PathBuf}; use std::sync::Arc; -type CfgSpecs = FxHashSet<(String, Option)>; - -fn mk_session(handler: &mut EarlyErrorHandler, matches: getopts::Matches) -> (Session, CfgSpecs) { +fn mk_session( + handler: &mut EarlyErrorHandler, + matches: getopts::Matches, +) -> (Session, Cfg) { let registry = registry::Registry::new(&[]); let sessopts = build_session_options(handler, &matches); - let cfg = parse_cfgspecs(handler, matches.opt_strs("cfg")); + let cfg = parse_cfg(handler, matches.opt_strs("cfg")); let temps_dir = sessopts.unstable_opts.temps_dir.as_deref().map(PathBuf::from); let io = CompilerIO { input: Input::Str { name: FileName::Custom(String::new()), input: String::new() }, @@ -133,7 +132,7 @@ fn test_switch_implies_cfg_test() { let matches = optgroups().parse(&["--test".to_string()]).unwrap(); let mut handler = EarlyErrorHandler::new(ErrorOutputType::default()); let (sess, cfg) = mk_session(&mut handler, matches); - let cfg = build_configuration(&sess, to_crate_config(cfg)); + let cfg = build_configuration(&sess, cfg); assert!(cfg.contains(&(sym::test, None))); }); } @@ -145,7 +144,7 @@ fn test_switch_implies_cfg_test_unless_cfg_test() { let matches = optgroups().parse(&["--test".to_string(), "--cfg=test".to_string()]).unwrap(); let mut handler = EarlyErrorHandler::new(ErrorOutputType::default()); let (sess, cfg) = mk_session(&mut handler, matches); - let cfg = build_configuration(&sess, to_crate_config(cfg)); + let cfg = build_configuration(&sess, cfg); let mut test_items = cfg.iter().filter(|&&(name, _)| name == sym::test); assert!(test_items.next().is_some()); assert!(test_items.next().is_none()); diff --git a/compiler/rustc_interface/src/util.rs b/compiler/rustc_interface/src/util.rs index d2455a036ccd5..4d0be65697a42 100644 --- a/compiler/rustc_interface/src/util.rs +++ b/compiler/rustc_interface/src/util.rs @@ -3,18 +3,17 @@ use info; use libloading::Library; use rustc_ast as ast; use rustc_codegen_ssa::traits::CodegenBackend; -use rustc_data_structures::fx::{FxHashMap, FxHashSet}; +use rustc_data_structures::fx::FxHashMap; #[cfg(parallel_compiler)] use rustc_data_structures::sync; use rustc_errors::registry::Registry; use rustc_parse::validate_attr; use rustc_session as session; -use rustc_session::config::CheckCfg; -use rustc_session::config::{self, CrateType}; -use rustc_session::config::{OutFileName, OutputFilenames, OutputTypes}; +use rustc_session::config::{ + self, Cfg, CheckCfg, CrateType, OutFileName, OutputFilenames, OutputTypes, +}; use rustc_session::filesearch::sysroot_candidates; use rustc_session::lint::{self, BuiltinLintDiagnostics, LintBuffer}; -use rustc_session::parse::CrateConfig; use rustc_session::{filesearch, output, Session}; use rustc_span::edit_distance::find_best_match_for_name; use rustc_span::edition::Edition; @@ -38,7 +37,7 @@ pub type MakeBackendFn = fn() -> Box; /// This is performed by checking whether a set of permitted features /// is available on the target machine, by querying the codegen backend. pub fn add_configuration( - cfg: &mut CrateConfig, + cfg: &mut Cfg, sess: &mut Session, codegen_backend: &dyn CodegenBackend, ) { @@ -60,8 +59,8 @@ pub fn add_configuration( pub fn create_session( handler: &EarlyErrorHandler, sopts: config::Options, - cfg: FxHashSet<(String, Option)>, - check_cfg: CheckCfg, + cfg: Cfg, + check_cfg: CheckCfg, locale_resources: &'static [&'static str], file_loader: Option>, io: CompilerIO, @@ -121,12 +120,13 @@ pub fn create_session( codegen_backend.init(&sess); - let mut cfg = config::build_configuration(&sess, config::to_crate_config(cfg)); + let mut cfg = config::build_configuration(&sess, cfg); add_configuration(&mut cfg, &mut sess, &*codegen_backend); - let mut check_cfg = config::to_crate_check_config(check_cfg); + let mut check_cfg = check_cfg.intern(); check_cfg.fill_well_known(&sess.target); + // These configs use symbols, rather than strings. sess.parse_sess.config = cfg; sess.parse_sess.check_config = check_cfg; diff --git a/compiler/rustc_session/src/config.rs b/compiler/rustc_session/src/config.rs index 7aced414ed67a..fef4dfba6d22c 100644 --- a/compiler/rustc_session/src/config.rs +++ b/compiler/rustc_session/src/config.rs @@ -16,7 +16,6 @@ use rustc_target::spec::LinkSelfContainedComponents; use rustc_target::spec::{PanicStrategy, RelocModel, SanitizerSet, SplitDebuginfo}; use rustc_target::spec::{Target, TargetTriple, TargetWarnings, TARGETS}; -use crate::parse::{CrateCheckConfig, CrateConfig}; use rustc_feature::UnstableFeatures; use rustc_span::edition::{Edition, DEFAULT_EDITION, EDITION_NAME_LIST, LATEST_STABLE_EDITION}; use rustc_span::source_map::{FileName, FilePathMapping}; @@ -1248,8 +1247,8 @@ pub const fn default_lib_output() -> CrateType { CrateType::Rlib } -fn default_configuration(sess: &Session) -> CrateConfig { - // NOTE: This should be kept in sync with `CrateCheckConfig::fill_well_known` below. +fn default_configuration(sess: &Session) -> Cfg { + // NOTE: This should be kept in sync with `CheckCfg::::fill_well_known` below. let end = &sess.target.endian; let arch = &sess.target.arch; let wordsz = sess.target.pointer_width.to_string(); @@ -1265,7 +1264,7 @@ fn default_configuration(sess: &Session) -> CrateConfig { sess.emit_fatal(err); }); - let mut ret = CrateConfig::default(); + let mut ret = Cfg::default(); ret.reserve(7); // the minimum number of insertions // Target bindings. ret.insert((sym::target_os, Some(Symbol::intern(os)))); @@ -1358,15 +1357,14 @@ fn default_configuration(sess: &Session) -> CrateConfig { ret } -/// Converts the crate `cfg!` configuration from `String` to `Symbol`. -/// `rustc_interface::interface::Config` accepts this in the compiler configuration, -/// but the symbol interner is not yet set up then, so we must convert it later. -pub fn to_crate_config(cfg: FxHashSet<(String, Option)>) -> CrateConfig { - cfg.into_iter().map(|(a, b)| (Symbol::intern(&a), b.map(|b| Symbol::intern(&b)))).collect() -} +/// The parsed `--cfg` options that define the compilation environment of the +/// crate, used to drive conditional compilation. `T` is always `String` or +/// `Symbol`. Strings are used temporarily very early on. Once the the main +/// symbol interner is running, they are converted to symbols. +pub type Cfg = FxHashSet<(T, Option)>; -/// The parsed `--check-cfg` options -pub struct CheckCfg { +/// The parsed `--check-cfg` options. The `` structure is similar to `Cfg`. +pub struct CheckCfg { /// Is well known names activated pub exhaustive_names: bool, /// Is well known values activated @@ -1385,8 +1383,8 @@ impl Default for CheckCfg { } } -impl CheckCfg { - fn map_data(self, f: impl Fn(T) -> O) -> CheckCfg { +impl CheckCfg { + pub fn intern(self) -> CheckCfg { CheckCfg { exhaustive_names: self.exhaustive_names, exhaustive_values: self.exhaustive_values, @@ -1395,10 +1393,10 @@ impl CheckCfg { .into_iter() .map(|(name, values)| { ( - f(name), + Symbol::intern(&name), match values { ExpectedValues::Some(values) => ExpectedValues::Some( - values.into_iter().map(|b| b.map(|b| f(b))).collect(), + values.into_iter().map(|b| b.map(|b| Symbol::intern(&b))).collect(), ), ExpectedValues::Any => ExpectedValues::Any, }, @@ -1441,14 +1439,7 @@ impl<'a, T: Eq + Hash + Copy + 'a> Extend<&'a T> for ExpectedValues { } } -/// Converts the crate `--check-cfg` options from `String` to `Symbol`. -/// `rustc_interface::interface::Config` accepts this in the compiler configuration, -/// but the symbol interner is not yet set up then, so we must convert it later. -pub fn to_crate_check_config(cfg: CheckCfg) -> CrateCheckConfig { - cfg.map_data(|s| Symbol::intern(&s)) -} - -impl CrateCheckConfig { +impl CheckCfg { pub fn fill_well_known(&mut self, current_target: &Target) { if !self.exhaustive_values && !self.exhaustive_names { return; @@ -1588,7 +1579,13 @@ impl CrateCheckConfig { } } -pub fn build_configuration(sess: &Session, mut user_cfg: CrateConfig) -> CrateConfig { +pub fn build_configuration(sess: &Session, user_cfg: Cfg) -> Cfg { + // We can now intern these strings. + let mut user_cfg: Cfg = user_cfg + .into_iter() + .map(|(a, b)| (Symbol::intern(&a), b.map(|b| Symbol::intern(&b)))) + .collect(); + // Combine the configuration requested by the session (command line) with // some default and generated configuration items. let default_cfg = default_configuration(sess); diff --git a/compiler/rustc_session/src/parse.rs b/compiler/rustc_session/src/parse.rs index 316b9cc7acb18..5b98ee5d992df 100644 --- a/compiler/rustc_session/src/parse.rs +++ b/compiler/rustc_session/src/parse.rs @@ -1,7 +1,7 @@ //! Contains `ParseSess` which holds state living beyond what one `Parser` might. //! It also serves as an input to the parser itself. -use crate::config::CheckCfg; +use crate::config::{Cfg, CheckCfg}; use crate::errors::{ CliFeatureDiagnosticHelp, FeatureDiagnosticForIssue, FeatureDiagnosticHelp, FeatureGateError, }; @@ -25,11 +25,6 @@ use rustc_span::{Span, Symbol}; use rustc_ast::attr::AttrIdGenerator; use std::str; -/// The set of keys (and, optionally, values) that define the compilation -/// environment of the crate, used to drive conditional compilation. -pub type CrateConfig = FxHashSet<(Symbol, Option)>; -pub type CrateCheckConfig = CheckCfg; - /// Collected spans during parsing for places where a certain feature was /// used and should be feature gated accordingly in `check_crate`. #[derive(Default)] @@ -193,8 +188,8 @@ pub fn add_feature_diagnostics_for_issue( pub struct ParseSess { pub span_diagnostic: Handler, pub unstable_features: UnstableFeatures, - pub config: CrateConfig, - pub check_config: CrateCheckConfig, + pub config: Cfg, + pub check_config: CheckCfg, pub edition: Edition, /// Places where raw identifiers were used. This is used to avoid complaining about idents /// clashing with keywords in new editions. @@ -237,8 +232,8 @@ impl ParseSess { Self { span_diagnostic: handler, unstable_features: UnstableFeatures::from_environment(None), - config: FxHashSet::default(), - check_config: CrateCheckConfig::default(), + config: Cfg::default(), + check_config: CheckCfg::default(), edition: ExpnId::root().expn_data().edition, raw_identifier_spans: Default::default(), bad_unicode_identifiers: Lock::new(Default::default()), diff --git a/src/librustdoc/core.rs b/src/librustdoc/core.rs index 94e557dcfdb54..bcc61df7c4d84 100644 --- a/src/librustdoc/core.rs +++ b/src/librustdoc/core.rs @@ -255,7 +255,7 @@ pub(crate) fn create_config( interface::Config { opts: sessopts, - crate_cfg: interface::parse_cfgspecs(handler, cfgs), + crate_cfg: interface::parse_cfg(handler, cfgs), crate_check_cfg: interface::parse_check_cfg(handler, check_cfgs), input, output_file: None, diff --git a/src/librustdoc/doctest.rs b/src/librustdoc/doctest.rs index 97913345e8fb2..c61c98e4de9c6 100644 --- a/src/librustdoc/doctest.rs +++ b/src/librustdoc/doctest.rs @@ -92,7 +92,7 @@ pub(crate) fn run(options: RustdocOptions) -> Result<(), ErrorGuaranteed> { cfgs.push("doctest".to_owned()); let config = interface::Config { opts: sessopts, - crate_cfg: interface::parse_cfgspecs(&early_error_handler, cfgs), + crate_cfg: interface::parse_cfg(&early_error_handler, cfgs), crate_check_cfg: interface::parse_check_cfg( &early_error_handler, options.check_cfgs.clone(), From 5438004766fc0f9b123608425b8ec7111f536640 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Sat, 28 Oct 2023 09:22:30 +1100 Subject: [PATCH 18/18] Change `Cfg` to an `FxIndexSet`. Despite what I claimed in an earlier commit, the ordering does matter to some degree. Using `FxIndexSet` prevents changes to the error message order in `tests/ui/check-cfg/mix.rs`. --- compiler/rustc_session/src/config.rs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/compiler/rustc_session/src/config.rs b/compiler/rustc_session/src/config.rs index fef4dfba6d22c..854e2b779935c 100644 --- a/compiler/rustc_session/src/config.rs +++ b/compiler/rustc_session/src/config.rs @@ -9,7 +9,7 @@ use crate::utils::{CanonicalizedPath, NativeLib, NativeLibKind}; use crate::{lint, HashStableContext}; use crate::{EarlyErrorHandler, Session}; -use rustc_data_structures::fx::{FxHashMap, FxHashSet}; +use rustc_data_structures::fx::{FxHashMap, FxHashSet, FxIndexSet}; use rustc_data_structures::stable_hasher::{StableOrd, ToStableHashKey}; use rustc_target::abi::Align; use rustc_target::spec::LinkSelfContainedComponents; @@ -1361,7 +1361,10 @@ fn default_configuration(sess: &Session) -> Cfg { /// crate, used to drive conditional compilation. `T` is always `String` or /// `Symbol`. Strings are used temporarily very early on. Once the the main /// symbol interner is running, they are converted to symbols. -pub type Cfg = FxHashSet<(T, Option)>; +/// +/// An `FxIndexSet` is used to ensure deterministic ordering of error messages +/// relating to `--cfg`. +pub type Cfg = FxIndexSet<(T, Option)>; /// The parsed `--check-cfg` options. The `` structure is similar to `Cfg`. pub struct CheckCfg {