From 1e10dfc7c4cf130e2f2f42013a0667eaedc28813 Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Thu, 8 May 2025 14:34:40 +0000 Subject: [PATCH 01/11] Add all rustc_std_internal_symbol to symbols.o rustc_std_internal_symbol is meant to call functions from crates where there is no direct dependency on said crate. As they either have to be added to symbols.o or rustc has to introduce an implicit dependency on them to avoid linker errors. The latter is done for some things like the panic runtime, but adding these symbols to symbols.o allows removing those implicit dependencies. --- compiler/rustc_codegen_ssa/src/back/linker.rs | 1 + .../src/back/symbol_export.rs | 17 ++++++++++++++ compiler/rustc_codegen_ssa/src/base.rs | 22 +------------------ .../src/middle/exported_symbols.rs | 3 +++ src/tools/miri/src/bin/miri.rs | 1 + 5 files changed, 23 insertions(+), 21 deletions(-) diff --git a/compiler/rustc_codegen_ssa/src/back/linker.rs b/compiler/rustc_codegen_ssa/src/back/linker.rs index 8fc83908efbcc..438313b6d1021 100644 --- a/compiler/rustc_codegen_ssa/src/back/linker.rs +++ b/compiler/rustc_codegen_ssa/src/back/linker.rs @@ -1829,6 +1829,7 @@ pub(crate) fn linked_symbols( for_each_exported_symbols_include_dep(tcx, crate_type, |symbol, info, cnum| { if info.level.is_below_threshold(export_threshold) && !tcx.is_compiler_builtins(cnum) || info.used + || info.rustc_std_internal_symbol { symbols.push(( symbol_export::linking_symbol_name_for_instance_in_crate(tcx, symbol, cnum), diff --git a/compiler/rustc_codegen_ssa/src/back/symbol_export.rs b/compiler/rustc_codegen_ssa/src/back/symbol_export.rs index e26f999773dc7..0e1a93b6bd3e7 100644 --- a/compiler/rustc_codegen_ssa/src/back/symbol_export.rs +++ b/compiler/rustc_codegen_ssa/src/back/symbol_export.rs @@ -131,6 +131,9 @@ fn reachable_non_generics_provider(tcx: TyCtxt<'_>, _: LocalCrate) -> DefIdMap, _: LocalCrate) -> DefIdMap( level: info.level, kind: SymbolExportKind::Text, used: info.used, + rustc_std_internal_symbol: info.rustc_std_internal_symbol, }, ) }) @@ -207,6 +212,7 @@ fn exported_symbols_provider_local<'tcx>( level: SymbolExportLevel::C, kind: SymbolExportKind::Text, used: false, + rustc_std_internal_symbol: false, }, )); } @@ -229,6 +235,7 @@ fn exported_symbols_provider_local<'tcx>( level: SymbolExportLevel::Rust, kind: SymbolExportKind::Text, used: false, + rustc_std_internal_symbol: true, }, )); } @@ -243,6 +250,7 @@ fn exported_symbols_provider_local<'tcx>( level: SymbolExportLevel::Rust, kind: SymbolExportKind::Data, used: false, + rustc_std_internal_symbol: true, }, )) } @@ -262,6 +270,7 @@ fn exported_symbols_provider_local<'tcx>( level: SymbolExportLevel::C, kind: SymbolExportKind::Data, used: false, + rustc_std_internal_symbol: false, }, ) })); @@ -287,6 +296,7 @@ fn exported_symbols_provider_local<'tcx>( level: SymbolExportLevel::C, kind: SymbolExportKind::Data, used: false, + rustc_std_internal_symbol: false, }, ) })); @@ -304,6 +314,7 @@ fn exported_symbols_provider_local<'tcx>( level: SymbolExportLevel::C, kind: SymbolExportKind::Data, used: true, + rustc_std_internal_symbol: false, }, )); } @@ -379,6 +390,8 @@ fn exported_symbols_provider_local<'tcx>( } } + // Note: These all set rustc_std_internal_symbol to false as generic functions must not + // be marked with this attribute and we are only handling generic functions here. match *mono_item { MonoItem::Fn(Instance { def: InstanceKind::Item(def), args }) => { let has_generics = args.non_erasable_generics().next().is_some(); @@ -394,6 +407,7 @@ fn exported_symbols_provider_local<'tcx>( level: SymbolExportLevel::Rust, kind: SymbolExportKind::Text, used: false, + rustc_std_internal_symbol: false, }, )); } @@ -416,6 +430,7 @@ fn exported_symbols_provider_local<'tcx>( level: SymbolExportLevel::Rust, kind: SymbolExportKind::Text, used: false, + rustc_std_internal_symbol: false, }, )); } @@ -432,6 +447,7 @@ fn exported_symbols_provider_local<'tcx>( level: SymbolExportLevel::Rust, kind: SymbolExportKind::Text, used: false, + rustc_std_internal_symbol: false, }, )); } @@ -442,6 +458,7 @@ fn exported_symbols_provider_local<'tcx>( level: SymbolExportLevel::Rust, kind: SymbolExportKind::Text, used: false, + rustc_std_internal_symbol: false, }, )); } diff --git a/compiler/rustc_codegen_ssa/src/base.rs b/compiler/rustc_codegen_ssa/src/base.rs index f7863fe4ae264..028f7742b333f 100644 --- a/compiler/rustc_codegen_ssa/src/base.rs +++ b/compiler/rustc_codegen_ssa/src/base.rs @@ -6,7 +6,7 @@ use std::time::{Duration, Instant}; use itertools::Itertools; use rustc_abi::FIRST_VARIANT; use rustc_ast as ast; -use rustc_ast::expand::allocator::{ALLOCATOR_METHODS, AllocatorKind, global_fn_name}; +use rustc_ast::expand::allocator::AllocatorKind; use rustc_attr_data_structures::OptimizeAttr; use rustc_data_structures::fx::{FxHashMap, FxIndexSet}; use rustc_data_structures::profiling::{get_resident_set_size, print_time_passes_entry}; @@ -1075,26 +1075,6 @@ impl CrateInfo { .collect::>(); symbols.sort_unstable_by(|a, b| a.0.cmp(&b.0)); linked_symbols.extend(symbols); - if tcx.allocator_kind(()).is_some() { - // At least one crate needs a global allocator. This crate may be placed - // after the crate that defines it in the linker order, in which case some - // linkers return an error. By adding the global allocator shim methods to - // the linked_symbols list, linking the generated symbols.o will ensure that - // circular dependencies involving the global allocator don't lead to linker - // errors. - linked_symbols.extend(ALLOCATOR_METHODS.iter().map(|method| { - ( - format!( - "{prefix}{}", - mangle_internal_symbol( - tcx, - global_fn_name(method.name).as_str() - ) - ), - SymbolExportKind::Text, - ) - })); - } }); } diff --git a/compiler/rustc_middle/src/middle/exported_symbols.rs b/compiler/rustc_middle/src/middle/exported_symbols.rs index 1d67d0fe3bbf4..4469a5532f95f 100644 --- a/compiler/rustc_middle/src/middle/exported_symbols.rs +++ b/compiler/rustc_middle/src/middle/exported_symbols.rs @@ -35,7 +35,10 @@ pub enum SymbolExportKind { pub struct SymbolExportInfo { pub level: SymbolExportLevel, pub kind: SymbolExportKind, + /// Was the symbol marked as `#[used(compiler)]` or `#[used(linker)]`? pub used: bool, + /// Was the symbol marked as `#[rustc_std_internal_symbol]`? + pub rustc_std_internal_symbol: bool, } #[derive(Eq, PartialEq, Debug, Copy, Clone, TyEncodable, TyDecodable, HashStable)] diff --git a/src/tools/miri/src/bin/miri.rs b/src/tools/miri/src/bin/miri.rs index 0121472d330f5..c6baf8ee1bf9d 100644 --- a/src/tools/miri/src/bin/miri.rs +++ b/src/tools/miri/src/bin/miri.rs @@ -295,6 +295,7 @@ impl rustc_driver::Callbacks for MiriBeRustCompilerCalls { level: SymbolExportLevel::C, kind: SymbolExportKind::Text, used: false, + rustc_std_internal_symbol: false, }, )) } else { From 2d2d70f7325da1588d5c5358556388107ad4e13d Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Thu, 8 May 2025 11:32:22 +0000 Subject: [PATCH 02/11] Remove dependency injection for the panic runtime This used to be necessary for a correct linker order, but ever since the introduction of symbols.o adding the symbols in question to symbols.o would work just as well. We do still add dependencies on the panic runtime to the local crate, but not for #![needs_panic_runtime] crates. This also removes the runtime-depends-on-needs-runtime test. inject_dependency_if used to emit this error, but with symbols.o it is no longer important that there is no dependency and in fact it may be nice to have panic_abort and panic_unwind directly depend on libstd in the future for calling std::process::abort(). --- compiler/rustc_metadata/src/creader.rs | 73 +------------------ compiler/rustc_metadata/src/rmeta/decoder.rs | 4 - tests/ui/panic-runtime/auxiliary/depends.rs | 8 -- .../auxiliary/needs-panic-runtime.rs | 6 -- .../runtime-depend-on-needs-runtime.rs | 9 --- 5 files changed, 3 insertions(+), 97 deletions(-) delete mode 100644 tests/ui/panic-runtime/auxiliary/depends.rs delete mode 100644 tests/ui/panic-runtime/auxiliary/needs-panic-runtime.rs delete mode 100644 tests/ui/panic-runtime/runtime-depend-on-needs-runtime.rs diff --git a/compiler/rustc_metadata/src/creader.rs b/compiler/rustc_metadata/src/creader.rs index de19c10bc57c8..60788a43bd75b 100644 --- a/compiler/rustc_metadata/src/creader.rs +++ b/compiler/rustc_metadata/src/creader.rs @@ -1,7 +1,6 @@ //! Validates all used crates and extern libraries and loads their metadata use std::error::Error; -use std::ops::Fn; use std::path::Path; use std::str::FromStr; use std::time::Duration; @@ -276,12 +275,6 @@ impl CStore { .filter_map(|(cnum, data)| data.as_deref().map(|data| (cnum, data))) } - fn iter_crate_data_mut(&mut self) -> impl Iterator { - self.metas - .iter_enumerated_mut() - .filter_map(|(cnum, data)| data.as_deref_mut().map(|data| (cnum, data))) - } - fn push_dependencies_in_postorder(&self, deps: &mut IndexSet, cnum: CrateNum) { if !deps.contains(&cnum) { let data = self.get_crate_data(cnum); @@ -307,13 +300,6 @@ impl CStore { deps } - fn crate_dependencies_in_reverse_postorder( - &self, - cnum: CrateNum, - ) -> impl Iterator { - self.crate_dependencies_in_postorder(cnum).into_iter().rev() - } - pub(crate) fn injected_panic_runtime(&self) -> Option { self.injected_panic_runtime } @@ -966,27 +952,16 @@ impl<'a, 'tcx> CrateLoader<'a, 'tcx> { // If we need a panic runtime, we try to find an existing one here. At // the same time we perform some general validation of the DAG we've got // going such as ensuring everything has a compatible panic strategy. - // - // The logic for finding the panic runtime here is pretty much the same - // as the allocator case with the only addition that the panic strategy - // compilation mode also comes into play. let desired_strategy = self.sess.panic_strategy(); let mut runtime_found = false; let mut needs_panic_runtime = attr::contains_name(&krate.attrs, sym::needs_panic_runtime); - let mut panic_runtimes = Vec::new(); - for (cnum, data) in self.cstore.iter_crate_data() { + for (_cnum, data) in self.cstore.iter_crate_data() { needs_panic_runtime = needs_panic_runtime || data.needs_panic_runtime(); if data.is_panic_runtime() { - // Inject a dependency from all #![needs_panic_runtime] to this - // #![panic_runtime] crate. - panic_runtimes.push(cnum); runtime_found = runtime_found || data.dep_kind() == CrateDepKind::Explicit; } } - for cnum in panic_runtimes { - self.inject_dependency_if(cnum, "a panic runtime", &|data| data.needs_panic_runtime()); - } // If an explicitly linked and matching panic runtime was found, or if // we just don't need one at all, then we're done here and there's @@ -997,12 +972,10 @@ impl<'a, 'tcx> CrateLoader<'a, 'tcx> { // By this point we know that we (a) need a panic runtime and (b) no // panic runtime was explicitly linked. Here we just load an appropriate - // default runtime for our panic strategy and then inject the - // dependencies. + // default runtime for our panic strategy. // // We may resolve to an already loaded crate (as the crate may not have - // been explicitly linked prior to this) and we may re-inject - // dependencies again, but both of those situations are fine. + // been explicitly linked prior to this), but this is fine. // // Also note that we have yet to perform validation of the crate graph // in terms of everyone has a compatible panic runtime format, that's @@ -1031,7 +1004,6 @@ impl<'a, 'tcx> CrateLoader<'a, 'tcx> { } self.cstore.injected_panic_runtime = Some(cnum); - self.inject_dependency_if(cnum, "a panic runtime", &|data| data.needs_panic_runtime()); } fn inject_profiler_runtime(&mut self) { @@ -1212,45 +1184,6 @@ impl<'a, 'tcx> CrateLoader<'a, 'tcx> { } } - fn inject_dependency_if( - &mut self, - krate: CrateNum, - what: &str, - needs_dep: &dyn Fn(&CrateMetadata) -> bool, - ) { - // Don't perform this validation if the session has errors, as one of - // those errors may indicate a circular dependency which could cause - // this to stack overflow. - if self.dcx().has_errors().is_some() { - return; - } - - // Before we inject any dependencies, make sure we don't inject a - // circular dependency by validating that this crate doesn't - // transitively depend on any crates satisfying `needs_dep`. - for dep in self.cstore.crate_dependencies_in_reverse_postorder(krate) { - let data = self.cstore.get_crate_data(dep); - if needs_dep(&data) { - self.dcx().emit_err(errors::NoTransitiveNeedsDep { - crate_name: self.cstore.get_crate_data(krate).name(), - needs_crate_name: what, - deps_crate_name: data.name(), - }); - } - } - - // All crates satisfying `needs_dep` do not explicitly depend on the - // crate provided for this compile, but in order for this compilation to - // be successfully linked we need to inject a dependency (to order the - // crates on the command line correctly). - for (cnum, data) in self.cstore.iter_crate_data_mut() { - if needs_dep(data) { - info!("injecting a dep from {} to {}", cnum, krate); - data.add_dependency(krate); - } - } - } - fn report_unused_deps(&mut self, krate: &ast::Crate) { // Make a point span rather than covering the whole file let span = krate.spans.inner_span.shrink_to_lo(); diff --git a/compiler/rustc_metadata/src/rmeta/decoder.rs b/compiler/rustc_metadata/src/rmeta/decoder.rs index 1dae858b7ef1a..c35eee8e8162e 100644 --- a/compiler/rustc_metadata/src/rmeta/decoder.rs +++ b/compiler/rustc_metadata/src/rmeta/decoder.rs @@ -1895,10 +1895,6 @@ impl CrateMetadata { self.dependencies.iter().copied() } - pub(crate) fn add_dependency(&mut self, cnum: CrateNum) { - self.dependencies.push(cnum); - } - pub(crate) fn target_modifiers(&self) -> TargetModifiers { self.root.decode_target_modifiers(&self.blob).collect() } diff --git a/tests/ui/panic-runtime/auxiliary/depends.rs b/tests/ui/panic-runtime/auxiliary/depends.rs deleted file mode 100644 index 7a35619b68133..0000000000000 --- a/tests/ui/panic-runtime/auxiliary/depends.rs +++ /dev/null @@ -1,8 +0,0 @@ -//@ no-prefer-dynamic - -#![feature(panic_runtime)] -#![crate_type = "rlib"] -#![panic_runtime] -#![no_std] - -extern crate needs_panic_runtime; diff --git a/tests/ui/panic-runtime/auxiliary/needs-panic-runtime.rs b/tests/ui/panic-runtime/auxiliary/needs-panic-runtime.rs deleted file mode 100644 index fbafee0c24177..0000000000000 --- a/tests/ui/panic-runtime/auxiliary/needs-panic-runtime.rs +++ /dev/null @@ -1,6 +0,0 @@ -//@ no-prefer-dynamic - -#![feature(needs_panic_runtime)] -#![crate_type = "rlib"] -#![needs_panic_runtime] -#![no_std] diff --git a/tests/ui/panic-runtime/runtime-depend-on-needs-runtime.rs b/tests/ui/panic-runtime/runtime-depend-on-needs-runtime.rs deleted file mode 100644 index eb00c071702c3..0000000000000 --- a/tests/ui/panic-runtime/runtime-depend-on-needs-runtime.rs +++ /dev/null @@ -1,9 +0,0 @@ -//@ dont-check-compiler-stderr -//@ aux-build:needs-panic-runtime.rs -//@ aux-build:depends.rs - -extern crate depends; - -fn main() {} - -//~? ERROR the crate `depends` cannot depend on a crate that needs a panic runtime, but it depends on `needs_panic_runtime` From 3a7ae677b86b7a765143da8cfac5e3f712a663df Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Thu, 8 May 2025 14:58:26 +0000 Subject: [PATCH 03/11] Stop handling explicit dependencies on the panic runtime You shouldn't ever need to explicitly depend on it. And we weren't checking that the panic runtime used the correct panic strategy either. --- compiler/rustc_metadata/src/creader.rs | 21 +++++++-------------- 1 file changed, 7 insertions(+), 14 deletions(-) diff --git a/compiler/rustc_metadata/src/creader.rs b/compiler/rustc_metadata/src/creader.rs index 60788a43bd75b..2c882b84c588d 100644 --- a/compiler/rustc_metadata/src/creader.rs +++ b/compiler/rustc_metadata/src/creader.rs @@ -952,27 +952,19 @@ impl<'a, 'tcx> CrateLoader<'a, 'tcx> { // If we need a panic runtime, we try to find an existing one here. At // the same time we perform some general validation of the DAG we've got // going such as ensuring everything has a compatible panic strategy. - let desired_strategy = self.sess.panic_strategy(); - let mut runtime_found = false; let mut needs_panic_runtime = attr::contains_name(&krate.attrs, sym::needs_panic_runtime); - for (_cnum, data) in self.cstore.iter_crate_data() { - needs_panic_runtime = needs_panic_runtime || data.needs_panic_runtime(); - if data.is_panic_runtime() { - runtime_found = runtime_found || data.dep_kind() == CrateDepKind::Explicit; - } + needs_panic_runtime |= data.needs_panic_runtime(); } - // If an explicitly linked and matching panic runtime was found, or if - // we just don't need one at all, then we're done here and there's - // nothing else to do. - if !needs_panic_runtime || runtime_found { + // If we just don't need a panic runtime at all, then we're done here + // and there's nothing else to do. + if !needs_panic_runtime { return; } - // By this point we know that we (a) need a panic runtime and (b) no - // panic runtime was explicitly linked. Here we just load an appropriate - // default runtime for our panic strategy. + // By this point we know that we need a panic runtime. Here we just load + // an appropriate default runtime for our panic strategy. // // We may resolve to an already loaded crate (as the crate may not have // been explicitly linked prior to this), but this is fine. @@ -980,6 +972,7 @@ impl<'a, 'tcx> CrateLoader<'a, 'tcx> { // Also note that we have yet to perform validation of the crate graph // in terms of everyone has a compatible panic runtime format, that's // performed later as part of the `dependency_format` module. + let desired_strategy = self.sess.panic_strategy(); let name = match desired_strategy { PanicStrategy::Unwind => sym::panic_unwind, PanicStrategy::Abort => sym::panic_abort, From f8d1bfa0a680ffc848e512ef799f687aae723cbe Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Thu, 8 May 2025 15:03:04 +0000 Subject: [PATCH 04/11] Avoid exporting panic_unwind as stdlib cargo feature There is already panic-unwind to enable it. --- library/std/Cargo.toml | 2 +- library/sysroot/Cargo.toml | 2 +- src/tools/miri/cargo-miri/src/setup.rs | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/library/std/Cargo.toml b/library/std/Cargo.toml index 31371f06b3865..844e2af209118 100644 --- a/library/std/Cargo.toml +++ b/library/std/Cargo.toml @@ -93,7 +93,7 @@ backtrace = [ 'miniz_oxide/rustc-dep-of-std', ] -panic-unwind = ["panic_unwind"] +panic-unwind = ["dep:panic_unwind"] compiler-builtins-c = ["alloc/compiler-builtins-c"] compiler-builtins-mem = ["alloc/compiler-builtins-mem"] compiler-builtins-no-asm = ["alloc/compiler-builtins-no-asm"] diff --git a/library/sysroot/Cargo.toml b/library/sysroot/Cargo.toml index c149d513c32b4..3adc022497167 100644 --- a/library/sysroot/Cargo.toml +++ b/library/sysroot/Cargo.toml @@ -26,7 +26,7 @@ debug_typeid = ["std/debug_typeid"] llvm-libunwind = ["std/llvm-libunwind"] system-llvm-libunwind = ["std/system-llvm-libunwind"] optimize_for_size = ["std/optimize_for_size"] -panic-unwind = ["std/panic_unwind"] +panic-unwind = ["std/panic-unwind"] panic_immediate_abort = ["std/panic_immediate_abort"] profiler = ["dep:profiler_builtins"] std_detect_file_io = ["std/std_detect_file_io"] diff --git a/src/tools/miri/cargo-miri/src/setup.rs b/src/tools/miri/cargo-miri/src/setup.rs index b9b58c04f9e4a..e399f66fbc9cd 100644 --- a/src/tools/miri/cargo-miri/src/setup.rs +++ b/src/tools/miri/cargo-miri/src/setup.rs @@ -83,7 +83,7 @@ pub fn setup( SysrootConfig::NoStd } else { SysrootConfig::WithStd { - std_features: ["panic_unwind", "backtrace"].into_iter().map(Into::into).collect(), + std_features: ["panic-unwind", "backtrace"].into_iter().map(Into::into).collect(), } }; let cargo_cmd = { From 6df545614789f2b2e8db6bb2b34e97105c8619be Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Fri, 9 May 2025 09:56:27 +0000 Subject: [PATCH 05/11] Make comment on activate_injected_dep a doc comment --- .../rustc_metadata/src/dependency_format.rs | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/compiler/rustc_metadata/src/dependency_format.rs b/compiler/rustc_metadata/src/dependency_format.rs index fcae33c73c9c0..f2f5afd84a09a 100644 --- a/compiler/rustc_metadata/src/dependency_format.rs +++ b/compiler/rustc_metadata/src/dependency_format.rs @@ -370,15 +370,15 @@ fn attempt_static(tcx: TyCtxt<'_>, unavailable: &mut Vec) -> Option, list: &mut DependencyList, From 269ee72c330c21a9e0526e69fc0f85d762043d2d Mon Sep 17 00:00:00 2001 From: Jeremy Smart Date: Wed, 11 Jun 2025 23:01:57 -0400 Subject: [PATCH 06/11] add ChildExt(::send_signal) --- library/std/src/os/unix/mod.rs | 3 ++ library/std/src/os/unix/process.rs | 35 +++++++++++++++++++++ library/std/src/sys/pal/unix/linux/pidfd.rs | 6 +++- library/std/src/sys/process/unix/unix.rs | 10 ++++-- 4 files changed, 50 insertions(+), 4 deletions(-) diff --git a/library/std/src/os/unix/mod.rs b/library/std/src/os/unix/mod.rs index 5802b6539651c..78c957270c451 100644 --- a/library/std/src/os/unix/mod.rs +++ b/library/std/src/os/unix/mod.rs @@ -116,6 +116,9 @@ pub mod prelude { #[stable(feature = "rust1", since = "1.0.0")] pub use super::io::{AsFd, AsRawFd, BorrowedFd, FromRawFd, IntoRawFd, OwnedFd, RawFd}; #[doc(no_inline)] + #[unstable(feature = "unix_send_signal", issue = "141975")] + pub use super::process::ChildExt; + #[doc(no_inline)] #[stable(feature = "rust1", since = "1.0.0")] pub use super::process::{CommandExt, ExitStatusExt}; #[doc(no_inline)] diff --git a/library/std/src/os/unix/process.rs b/library/std/src/os/unix/process.rs index 57ce3c5a4bf4a..3f4fe2e56ec31 100644 --- a/library/std/src/os/unix/process.rs +++ b/library/std/src/os/unix/process.rs @@ -378,6 +378,41 @@ impl ExitStatusExt for process::ExitStatusError { } } +#[unstable(feature = "unix_send_signal", issue = "141975")] +pub trait ChildExt: Sealed { + /// Sends a signal to a child process. + /// + /// # Errors + /// + /// This function will return an error if the signal is invalid. The integer values associated + /// with signals are implemenation-specific, so it's encouraged to use a crate that provides + /// posix bindings. + /// + /// # Examples + /// + /// ```rust + /// #![feature(unix_send_signal)] + /// + /// use std::{io, os::unix::process::ChildExt, process::{Command, Stdio}}; + /// + /// use libc::SIGTERM; + /// + /// fn main() -> io::Result<()> { + /// let child = Command::new("cat").stdin(Stdio::piped()).spawn()?; + /// child.send_signal(SIGTERM)?; + /// Ok(()) + /// } + /// ``` + fn send_signal(&self, signal: i32) -> io::Result<()>; +} + +#[unstable(feature = "unix_send_signal", issue = "141975")] +impl ChildExt for process::Child { + fn send_signal(&self, signal: i32) -> io::Result<()> { + self.handle.send_signal(signal) + } +} + #[stable(feature = "process_extensions", since = "1.2.0")] impl FromRawFd for process::Stdio { #[inline] diff --git a/library/std/src/sys/pal/unix/linux/pidfd.rs b/library/std/src/sys/pal/unix/linux/pidfd.rs index 2d949ec9e91f7..47e9a616bfdaf 100644 --- a/library/std/src/sys/pal/unix/linux/pidfd.rs +++ b/library/std/src/sys/pal/unix/linux/pidfd.rs @@ -13,11 +13,15 @@ pub(crate) struct PidFd(FileDesc); impl PidFd { pub fn kill(&self) -> io::Result<()> { + self.send_signal(libc::SIGKILL) + } + + pub(crate) fn send_signal(&self, signal: i32) -> io::Result<()> { cvt(unsafe { libc::syscall( libc::SYS_pidfd_send_signal, self.0.as_raw_fd(), - libc::SIGKILL, + signal, crate::ptr::null::<()>(), 0, ) diff --git a/library/std/src/sys/process/unix/unix.rs b/library/std/src/sys/process/unix/unix.rs index 4f595ac9a1c5f..59014a828621e 100644 --- a/library/std/src/sys/process/unix/unix.rs +++ b/library/std/src/sys/process/unix/unix.rs @@ -964,8 +964,12 @@ impl Process { } pub fn kill(&mut self) -> io::Result<()> { + self.send_signal(libc::SIGKILL) + } + + pub(crate) fn send_signal(&self, signal: i32) -> io::Result<()> { // If we've already waited on this process then the pid can be recycled - // and used for another process, and we probably shouldn't be killing + // and used for another process, and we probably shouldn't be signaling // random processes, so return Ok because the process has exited already. if self.status.is_some() { return Ok(()); @@ -973,9 +977,9 @@ impl Process { #[cfg(target_os = "linux")] if let Some(pid_fd) = self.pidfd.as_ref() { // pidfd_send_signal predates pidfd_open. so if we were able to get an fd then sending signals will work too - return pid_fd.kill(); + return pid_fd.send_signal(signal); } - cvt(unsafe { libc::kill(self.pid, libc::SIGKILL) }).map(drop) + cvt(unsafe { libc::kill(self.pid, signal) }).map(drop) } pub fn wait(&mut self) -> io::Result { From 96fd9fc2dd8aa45ab19c2e07ed172b50566215ac Mon Sep 17 00:00:00 2001 From: Deadbeef Date: Sun, 15 Jun 2025 14:59:05 +0800 Subject: [PATCH 07/11] use `if let` guards where possible --- compiler/rustc_builtin_macros/src/test.rs | 5 +--- .../rustc_parse/src/parser/diagnostics.rs | 27 ++++++++----------- 2 files changed, 12 insertions(+), 20 deletions(-) diff --git a/compiler/rustc_builtin_macros/src/test.rs b/compiler/rustc_builtin_macros/src/test.rs index b439fa34f5b12..b067578794b30 100644 --- a/compiler/rustc_builtin_macros/src/test.rs +++ b/compiler/rustc_builtin_macros/src/test.rs @@ -118,10 +118,7 @@ pub(crate) fn expand_test_or_bench( let (item, is_stmt) = match item { Annotatable::Item(i) => (i, false), - Annotatable::Stmt(stmt) if matches!(stmt.kind, ast::StmtKind::Item(_)) => { - // FIXME: Use an 'if let' guard once they are implemented - if let ast::StmtKind::Item(i) = stmt.kind { (i, true) } else { unreachable!() } - } + Annotatable::Stmt(box ast::Stmt { kind: ast::StmtKind::Item(i), .. }) => (i, true), other => { not_testable_error(cx, attr_sp, None); return vec![other]; diff --git a/compiler/rustc_parse/src/parser/diagnostics.rs b/compiler/rustc_parse/src/parser/diagnostics.rs index b49a13ce584fb..215202c4bd50b 100644 --- a/compiler/rustc_parse/src/parser/diagnostics.rs +++ b/compiler/rustc_parse/src/parser/diagnostics.rs @@ -2273,23 +2273,18 @@ impl<'a> Parser<'a> { ), // Also catches `fn foo(&a)`. PatKind::Ref(ref inner_pat, mutab) - if matches!(inner_pat.clone().kind, PatKind::Ident(..)) => + if let PatKind::Ident(_, ident, _) = inner_pat.clone().kind => { - match inner_pat.clone().kind { - PatKind::Ident(_, ident, _) => { - let mutab = mutab.prefix_str(); - ( - ident, - "self: ", - format!("{ident}: &{mutab}TypeName"), - "_: ", - pat.span.shrink_to_lo(), - pat.span, - pat.span.shrink_to_lo(), - ) - } - _ => unreachable!(), - } + let mutab = mutab.prefix_str(); + ( + ident, + "self: ", + format!("{ident}: &{mutab}TypeName"), + "_: ", + pat.span.shrink_to_lo(), + pat.span, + pat.span.shrink_to_lo(), + ) } _ => { // Otherwise, try to get a type and emit a suggestion. From 1ac89f190fa8a77d4503138f5bc7667c253457f6 Mon Sep 17 00:00:00 2001 From: xizheyin Date: Thu, 5 Jun 2025 21:57:35 +0800 Subject: [PATCH 08/11] Refactor `rustc_attr_data_structures` documentation Signed-off-by: xizheyin --- .../src/attributes.rs | 58 ++++++++++++++----- .../rustc_attr_data_structures/src/lib.rs | 4 ++ 2 files changed, 48 insertions(+), 14 deletions(-) diff --git a/compiler/rustc_attr_data_structures/src/attributes.rs b/compiler/rustc_attr_data_structures/src/attributes.rs index 845e4d5e5d0fa..a0a50fd4b7c2c 100644 --- a/compiler/rustc_attr_data_structures/src/attributes.rs +++ b/compiler/rustc_attr_data_structures/src/attributes.rs @@ -130,24 +130,54 @@ impl Deprecation { } } -/// Represent parsed, *built in*, inert attributes. +/// Represents parsed *built-in* inert attributes. /// -/// That means attributes that are not actually ever expanded. -/// For more information on this, see the module docs on the [`rustc_attr_parsing`] crate. -/// They're instead used as markers, to guide the compilation process in various way in most every stage of the compiler. -/// These are kept around after the AST, into the HIR and further on. +/// ## Overview +/// These attributes are markers that guide the compilation process and are never expanded into other code. +/// They persist throughout the compilation phases, from AST to HIR and beyond. /// -/// The word "parsed" could be a little misleading here, because the parser already parses -/// attributes early on. However, the result, an [`ast::Attribute`] -/// is only parsed at a high level, still containing a token stream in many cases. That is -/// because the structure of the contents varies from attribute to attribute. -/// With a parsed attribute I mean that each attribute is processed individually into a -/// final structure, which on-site (the place where the attribute is useful for, think the -/// the place where `must_use` is checked) little to no extra parsing or validating needs to -/// happen. +/// ## Attribute Processing +/// While attributes are initially parsed by [`rustc_parse`] into [`ast::Attribute`], they still contain raw token streams +/// because different attributes have different internal structures. This enum represents the final, +/// fully parsed form of these attributes, where each variant contains contains all the information and +/// structure relevant for the specific attribute. /// -/// For more docs, look in [`rustc_attr_parsing`]. +/// Some attributes can be applied multiple times to the same item, and they are "collapsed" into a single +/// semantic attribute. For example: +/// ```rust +/// #[repr(C)] +/// #[repr(packed)] +/// struct S { } +/// ``` +/// This is equivalent to `#[repr(C, packed)]` and results in a single [`AttributeKind::Repr`] containing +/// both `C` and `packed` annotations. This collapsing happens during parsing and is reflected in the +/// data structures defined in this enum. /// +/// ## Usage +/// These parsed attributes are used throughout the compiler to: +/// - Control code generation (e.g., `#[repr]`) +/// - Mark API stability (`#[stable]`, `#[unstable]`) +/// - Provide documentation (`#[doc]`) +/// - Guide compiler behavior (e.g., `#[allow_internal_unstable]`) +/// +/// ## Note on Attribute Organization +/// Some attributes like `InlineAttr`, `OptimizeAttr`, and `InstructionSetAttr` are defined separately +/// from this enum because they are used in specific compiler phases (like code generation) and don't +/// need to persist throughout the entire compilation process. They are typically processed and +/// converted into their final form earlier in the compilation pipeline. +/// +/// For example: +/// - `InlineAttr` is used during code generation to control function inlining +/// - `OptimizeAttr` is used to control optimization levels +/// - `InstructionSetAttr` is used for target-specific code generation +/// +/// These attributes are handled by their respective compiler passes in the [`rustc_codegen_ssa`] crate +/// and don't need to be preserved in the same way as the attributes in this enum. +/// +/// For more details on attribute parsing, see the [`rustc_attr_parsing`] crate. +/// +/// [`rustc_parse`]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_parse/index.html +/// [`rustc_codegen_ssa`]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_codegen_ssa/index.html /// [`rustc_attr_parsing`]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_attr_parsing/index.html #[derive(Clone, Debug, HashStable_Generic, Encodable, Decodable, PrintAttribute)] pub enum AttributeKind { diff --git a/compiler/rustc_attr_data_structures/src/lib.rs b/compiler/rustc_attr_data_structures/src/lib.rs index b0fc19d1cd7ba..f8355be09adfb 100644 --- a/compiler/rustc_attr_data_structures/src/lib.rs +++ b/compiler/rustc_attr_data_structures/src/lib.rs @@ -1,3 +1,7 @@ +//! Data structures for representing parsed attributes in the Rust compiler. +//! For detailed documentation about attribute processing, +//! see [rustc_attr_parsing](https://doc.rust-lang.org/nightly/nightly-rustc/rustc_attr_parsing/index.html). + // tidy-alphabetical-start #![allow(internal_features)] #![doc(rust_logo)] From a0db28f37c85a8a42e2eaf9144cebc3dc9c13875 Mon Sep 17 00:00:00 2001 From: Deadbeef Date: Sun, 15 Jun 2025 14:59:05 +0800 Subject: [PATCH 09/11] clarify `rustc_do_not_const_check` comment --- compiler/rustc_feature/src/builtin_attrs.rs | 2 +- .../rust-analyzer/crates/hir-expand/src/inert_attr_macro.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/compiler/rustc_feature/src/builtin_attrs.rs b/compiler/rustc_feature/src/builtin_attrs.rs index 73a21789c5d25..7d6e471e7e951 100644 --- a/compiler/rustc_feature/src/builtin_attrs.rs +++ b/compiler/rustc_feature/src/builtin_attrs.rs @@ -887,7 +887,7 @@ pub static BUILTIN_ATTRIBUTES: &[BuiltinAttribute] = &[ rustc_legacy_const_generics, Normal, template!(List: "N"), ErrorFollowing, EncodeCrossCrate::Yes, ), - // Do not const-check this function's body. It will always get replaced during CTFE. + // Do not const-check this function's body. It will always get replaced during CTFE via `hook_special_const_fn`. rustc_attr!( rustc_do_not_const_check, Normal, template!(Word), WarnFollowing, EncodeCrossCrate::Yes, "`#[rustc_do_not_const_check]` skips const-check for this function's body", diff --git a/src/tools/rust-analyzer/crates/hir-expand/src/inert_attr_macro.rs b/src/tools/rust-analyzer/crates/hir-expand/src/inert_attr_macro.rs index 543ac0619dd3e..385c98ef87784 100644 --- a/src/tools/rust-analyzer/crates/hir-expand/src/inert_attr_macro.rs +++ b/src/tools/rust-analyzer/crates/hir-expand/src/inert_attr_macro.rs @@ -486,7 +486,7 @@ pub const INERT_ATTRIBUTES: &[BuiltinAttribute] = &[ rustc_legacy_const_generics, Normal, template!(List: "N"), ErrorFollowing, INTERNAL_UNSTABLE ), - // Do not const-check this function's body. It will always get replaced during CTFE. + // Do not const-check this function's body. It will always get replaced during CTFE via `hook_special_const_fn`. rustc_attr!( rustc_do_not_const_check, Normal, template!(Word), WarnFollowing, INTERNAL_UNSTABLE ), From 38712030ca9d7844ca0ba606be493b4fda08dcaa Mon Sep 17 00:00:00 2001 From: Christopher Berner Date: Wed, 28 May 2025 21:01:04 -0700 Subject: [PATCH 10/11] Stabilize "file_lock" feature --- library/std/src/fs.rs | 25 ++++++++++--------------- src/tools/miri/src/lib.rs | 2 +- src/tools/miri/tests/pass/shims/fs.rs | 1 - 3 files changed, 11 insertions(+), 17 deletions(-) diff --git a/library/std/src/fs.rs b/library/std/src/fs.rs index 0cd794fd3efbb..865ea620a283f 100644 --- a/library/std/src/fs.rs +++ b/library/std/src/fs.rs @@ -121,7 +121,7 @@ pub struct File { /// /// [`try_lock`]: File::try_lock /// [`try_lock_shared`]: File::try_lock_shared -#[unstable(feature = "file_lock", issue = "130994")] +#[stable(feature = "file_lock", since = "CURRENT_RUSTC_VERSION")] pub enum TryLockError { /// The lock could not be acquired due to an I/O error on the file. The standard library will /// not return an [`ErrorKind::WouldBlock`] error inside [`TryLockError::Error`] @@ -366,10 +366,10 @@ pub fn write, C: AsRef<[u8]>>(path: P, contents: C) -> io::Result inner(path.as_ref(), contents.as_ref()) } -#[unstable(feature = "file_lock", issue = "130994")] +#[stable(feature = "file_lock", since = "CURRENT_RUSTC_VERSION")] impl error::Error for TryLockError {} -#[unstable(feature = "file_lock", issue = "130994")] +#[stable(feature = "file_lock", since = "CURRENT_RUSTC_VERSION")] impl fmt::Debug for TryLockError { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match self { @@ -379,7 +379,7 @@ impl fmt::Debug for TryLockError { } } -#[unstable(feature = "file_lock", issue = "130994")] +#[stable(feature = "file_lock", since = "CURRENT_RUSTC_VERSION")] impl fmt::Display for TryLockError { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match self { @@ -390,7 +390,7 @@ impl fmt::Display for TryLockError { } } -#[unstable(feature = "file_lock", issue = "130994")] +#[stable(feature = "file_lock", since = "CURRENT_RUSTC_VERSION")] impl From for io::Error { fn from(err: TryLockError) -> io::Error { match err { @@ -713,7 +713,6 @@ impl File { /// # Examples /// /// ```no_run - /// #![feature(file_lock)] /// use std::fs::File; /// /// fn main() -> std::io::Result<()> { @@ -722,7 +721,7 @@ impl File { /// Ok(()) /// } /// ``` - #[unstable(feature = "file_lock", issue = "130994")] + #[stable(feature = "file_lock", since = "CURRENT_RUSTC_VERSION")] pub fn lock(&self) -> io::Result<()> { self.inner.lock() } @@ -766,7 +765,6 @@ impl File { /// # Examples /// /// ```no_run - /// #![feature(file_lock)] /// use std::fs::File; /// /// fn main() -> std::io::Result<()> { @@ -775,7 +773,7 @@ impl File { /// Ok(()) /// } /// ``` - #[unstable(feature = "file_lock", issue = "130994")] + #[stable(feature = "file_lock", since = "CURRENT_RUSTC_VERSION")] pub fn lock_shared(&self) -> io::Result<()> { self.inner.lock_shared() } @@ -824,7 +822,6 @@ impl File { /// # Examples /// /// ```no_run - /// #![feature(file_lock)] /// use std::fs::{File, TryLockError}; /// /// fn main() -> std::io::Result<()> { @@ -840,7 +837,7 @@ impl File { /// Ok(()) /// } /// ``` - #[unstable(feature = "file_lock", issue = "130994")] + #[stable(feature = "file_lock", since = "CURRENT_RUSTC_VERSION")] pub fn try_lock(&self) -> Result<(), TryLockError> { self.inner.try_lock() } @@ -888,7 +885,6 @@ impl File { /// # Examples /// /// ```no_run - /// #![feature(file_lock)] /// use std::fs::{File, TryLockError}; /// /// fn main() -> std::io::Result<()> { @@ -905,7 +901,7 @@ impl File { /// Ok(()) /// } /// ``` - #[unstable(feature = "file_lock", issue = "130994")] + #[stable(feature = "file_lock", since = "CURRENT_RUSTC_VERSION")] pub fn try_lock_shared(&self) -> Result<(), TryLockError> { self.inner.try_lock_shared() } @@ -933,7 +929,6 @@ impl File { /// # Examples /// /// ```no_run - /// #![feature(file_lock)] /// use std::fs::File; /// /// fn main() -> std::io::Result<()> { @@ -943,7 +938,7 @@ impl File { /// Ok(()) /// } /// ``` - #[unstable(feature = "file_lock", issue = "130994")] + #[stable(feature = "file_lock", since = "CURRENT_RUSTC_VERSION")] pub fn unlock(&self) -> io::Result<()> { self.inner.unlock() } diff --git a/src/tools/miri/src/lib.rs b/src/tools/miri/src/lib.rs index 344e12e9fa349..048ed81a816f8 100644 --- a/src/tools/miri/src/lib.rs +++ b/src/tools/miri/src/lib.rs @@ -16,7 +16,7 @@ #![feature(unqualified_local_imports)] #![feature(derive_coerce_pointee)] #![feature(arbitrary_self_types)] -#![feature(file_lock)] +#![cfg_attr(bootstrap, feature(file_lock))] // Configure clippy and other lints #![allow( clippy::collapsible_else_if, diff --git a/src/tools/miri/tests/pass/shims/fs.rs b/src/tools/miri/tests/pass/shims/fs.rs index 2f30827c9336b..9d5725773e647 100644 --- a/src/tools/miri/tests/pass/shims/fs.rs +++ b/src/tools/miri/tests/pass/shims/fs.rs @@ -2,7 +2,6 @@ #![feature(io_error_more)] #![feature(io_error_uncategorized)] -#![feature(file_lock)] use std::collections::BTreeMap; use std::ffi::OsString; From ce457e1c2628e41a02bc2dcdc44346e7c7d0a9e2 Mon Sep 17 00:00:00 2001 From: Markus Reiter Date: Sat, 8 Mar 2025 20:29:17 +0100 Subject: [PATCH 11/11] Get rid of `EscapeDebugInner`. --- library/core/src/ascii.rs | 16 +-- library/core/src/char/mod.rs | 62 +++------ library/core/src/escape.rs | 222 ++++++++++++++++++++++++++------ library/core/src/slice/ascii.rs | 2 +- 4 files changed, 210 insertions(+), 92 deletions(-) diff --git a/library/core/src/ascii.rs b/library/core/src/ascii.rs index 5b3711b4071ab..d3c6c046e717f 100644 --- a/library/core/src/ascii.rs +++ b/library/core/src/ascii.rs @@ -9,9 +9,10 @@ #![stable(feature = "core_ascii", since = "1.26.0")] +use crate::escape::{AlwaysEscaped, EscapeIterInner}; +use crate::fmt; use crate::iter::FusedIterator; use crate::num::NonZero; -use crate::{escape, fmt}; mod ascii_char; #[unstable(feature = "ascii_char", issue = "110998")] @@ -24,7 +25,7 @@ pub use ascii_char::AsciiChar as Char; #[must_use = "iterators are lazy and do nothing unless consumed"] #[stable(feature = "rust1", since = "1.0.0")] #[derive(Clone)] -pub struct EscapeDefault(escape::EscapeIterInner<4>); +pub struct EscapeDefault(EscapeIterInner<4, AlwaysEscaped>); /// Returns an iterator that produces an escaped version of a `u8`. /// @@ -96,17 +97,12 @@ pub fn escape_default(c: u8) -> EscapeDefault { impl EscapeDefault { #[inline] pub(crate) const fn new(c: u8) -> Self { - Self(escape::EscapeIterInner::ascii(c)) + Self(EscapeIterInner::ascii(c)) } #[inline] pub(crate) fn empty() -> Self { - Self(escape::EscapeIterInner::empty()) - } - - #[inline] - pub(crate) fn as_str(&self) -> &str { - self.0.as_str() + Self(EscapeIterInner::empty()) } } @@ -168,7 +164,7 @@ impl FusedIterator for EscapeDefault {} #[stable(feature = "ascii_escape_display", since = "1.39.0")] impl fmt::Display for EscapeDefault { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - f.write_str(self.0.as_str()) + fmt::Display::fmt(&self.0, f) } } diff --git a/library/core/src/char/mod.rs b/library/core/src/char/mod.rs index 5b9f0e2143f5d..82a3f6f916be3 100644 --- a/library/core/src/char/mod.rs +++ b/library/core/src/char/mod.rs @@ -44,7 +44,7 @@ pub use self::methods::{encode_utf8_raw, encode_utf8_raw_unchecked}; // perma-un use crate::ascii; pub(crate) use self::methods::EscapeDebugExtArgs; use crate::error::Error; -use crate::escape; +use crate::escape::{AlwaysEscaped, EscapeIterInner, MaybeEscaped}; use crate::fmt::{self, Write}; use crate::iter::{FusedIterator, TrustedLen, TrustedRandomAccess, TrustedRandomAccessNoCoerce}; use crate::num::NonZero; @@ -161,12 +161,12 @@ pub const fn from_digit(num: u32, radix: u32) -> Option { /// [`escape_unicode`]: char::escape_unicode #[derive(Clone, Debug)] #[stable(feature = "rust1", since = "1.0.0")] -pub struct EscapeUnicode(escape::EscapeIterInner<10>); +pub struct EscapeUnicode(EscapeIterInner<10, AlwaysEscaped>); impl EscapeUnicode { #[inline] const fn new(c: char) -> Self { - Self(escape::EscapeIterInner::unicode(c)) + Self(EscapeIterInner::unicode(c)) } } @@ -215,7 +215,7 @@ impl FusedIterator for EscapeUnicode {} #[stable(feature = "char_struct_display", since = "1.16.0")] impl fmt::Display for EscapeUnicode { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - f.write_str(self.0.as_str()) + fmt::Display::fmt(&self.0, f) } } @@ -227,22 +227,22 @@ impl fmt::Display for EscapeUnicode { /// [`escape_default`]: char::escape_default #[derive(Clone, Debug)] #[stable(feature = "rust1", since = "1.0.0")] -pub struct EscapeDefault(escape::EscapeIterInner<10>); +pub struct EscapeDefault(EscapeIterInner<10, AlwaysEscaped>); impl EscapeDefault { #[inline] const fn printable(c: ascii::Char) -> Self { - Self(escape::EscapeIterInner::ascii(c.to_u8())) + Self(EscapeIterInner::ascii(c.to_u8())) } #[inline] const fn backslash(c: ascii::Char) -> Self { - Self(escape::EscapeIterInner::backslash(c)) + Self(EscapeIterInner::backslash(c)) } #[inline] const fn unicode(c: char) -> Self { - Self(escape::EscapeIterInner::unicode(c)) + Self(EscapeIterInner::unicode(c)) } } @@ -290,8 +290,9 @@ impl FusedIterator for EscapeDefault {} #[stable(feature = "char_struct_display", since = "1.16.0")] impl fmt::Display for EscapeDefault { + #[inline] fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - f.write_str(self.0.as_str()) + fmt::Display::fmt(&self.0, f) } } @@ -303,37 +304,22 @@ impl fmt::Display for EscapeDefault { /// [`escape_debug`]: char::escape_debug #[stable(feature = "char_escape_debug", since = "1.20.0")] #[derive(Clone, Debug)] -pub struct EscapeDebug(EscapeDebugInner); - -#[derive(Clone, Debug)] -// Note: It’s possible to manually encode the EscapeDebugInner inside of -// EscapeIterInner (e.g. with alive=254..255 indicating that data[0..4] holds -// a char) which would likely result in a more optimised code. For now we use -// the option easier to implement. -enum EscapeDebugInner { - Bytes(escape::EscapeIterInner<10>), - Char(char), -} +pub struct EscapeDebug(EscapeIterInner<10, MaybeEscaped>); impl EscapeDebug { #[inline] const fn printable(chr: char) -> Self { - Self(EscapeDebugInner::Char(chr)) + Self(EscapeIterInner::printable(chr)) } #[inline] const fn backslash(c: ascii::Char) -> Self { - Self(EscapeDebugInner::Bytes(escape::EscapeIterInner::backslash(c))) + Self(EscapeIterInner::backslash(c)) } #[inline] const fn unicode(c: char) -> Self { - Self(EscapeDebugInner::Bytes(escape::EscapeIterInner::unicode(c))) - } - - #[inline] - fn clear(&mut self) { - self.0 = EscapeDebugInner::Bytes(escape::EscapeIterInner::empty()); + Self(EscapeIterInner::unicode(c)) } } @@ -343,13 +329,7 @@ impl Iterator for EscapeDebug { #[inline] fn next(&mut self) -> Option { - match self.0 { - EscapeDebugInner::Bytes(ref mut bytes) => bytes.next().map(char::from), - EscapeDebugInner::Char(chr) => { - self.clear(); - Some(chr) - } - } + self.0.next() } #[inline] @@ -367,10 +347,7 @@ impl Iterator for EscapeDebug { #[stable(feature = "char_escape_debug", since = "1.20.0")] impl ExactSizeIterator for EscapeDebug { fn len(&self) -> usize { - match &self.0 { - EscapeDebugInner::Bytes(bytes) => bytes.len(), - EscapeDebugInner::Char(_) => 1, - } + self.0.len() } } @@ -379,11 +356,9 @@ impl FusedIterator for EscapeDebug {} #[stable(feature = "char_escape_debug", since = "1.20.0")] impl fmt::Display for EscapeDebug { + #[inline] fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - match &self.0 { - EscapeDebugInner::Bytes(bytes) => f.write_str(bytes.as_str()), - EscapeDebugInner::Char(chr) => f.write_char(*chr), - } + fmt::Display::fmt(&self.0, f) } } @@ -480,6 +455,7 @@ macro_rules! casemappingiter_impls { #[stable(feature = "char_struct_display", since = "1.16.0")] impl fmt::Display for $ITER_NAME { + #[inline] fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { fmt::Display::fmt(&self.0, f) } diff --git a/library/core/src/escape.rs b/library/core/src/escape.rs index 0c3329f676eeb..f459c58270818 100644 --- a/library/core/src/escape.rs +++ b/library/core/src/escape.rs @@ -1,11 +1,16 @@ //! Helper code for character escaping. use crate::ascii; +use crate::fmt::{self, Write}; +use crate::marker::PhantomData; use crate::num::NonZero; use crate::ops::Range; const HEX_DIGITS: [ascii::Char; 16] = *b"0123456789abcdef".as_ascii().unwrap(); +/// Escapes a character with `\x` representation. +/// +/// Returns a buffer with the escaped representation and its corresponding range. #[inline] const fn backslash(a: ascii::Char) -> ([ascii::Char; N], Range) { const { assert!(N >= 2) }; @@ -18,6 +23,9 @@ const fn backslash(a: ascii::Char) -> ([ascii::Char; N], Range(byte: u8) -> ([ascii::Char; N], Range) { const { assert!(N >= 4) }; @@ -35,6 +43,7 @@ const fn hex_escape(byte: u8) -> ([ascii::Char; N], Range) { (output, 0..4) } +/// Returns a buffer with the verbatim character and its corresponding range. #[inline] const fn verbatim(a: ascii::Char) -> ([ascii::Char; N], Range) { const { assert!(N >= 1) }; @@ -48,7 +57,7 @@ const fn verbatim(a: ascii::Char) -> ([ascii::Char; N], Range(byte: u8) -> ([ascii::Char; N], Range) { const { assert!(N >= 4) }; @@ -122,9 +131,9 @@ const fn escape_ascii(byte: u8) -> ([ascii::Char; N], Range) } } -/// Escapes a character `\u{NNNN}` representation. +/// Escapes a character with `\u{NNNN}` representation. /// -/// Returns a buffer and the length of the escaped representation. +/// Returns a buffer with the escaped representation and its corresponding range. const fn escape_unicode(c: char) -> ([ascii::Char; N], Range) { const { assert!(N >= 10 && N < u8::MAX as usize) }; @@ -149,77 +158,214 @@ const fn escape_unicode(c: char) -> ([ascii::Char; N], Range (output, (start as u8)..(N as u8)) } -/// An iterator over an fixed-size array. -/// -/// This is essentially equivalent to array’s IntoIter except that indexes are -/// limited to u8 to reduce size of the structure. -#[derive(Clone, Debug)] -pub(crate) struct EscapeIterInner { - // The element type ensures this is always ASCII, and thus also valid UTF-8. - data: [ascii::Char; N], - - // Invariant: `alive.start <= alive.end <= N` +#[derive(Clone, Copy)] +union MaybeEscapedCharacter { + pub escape_seq: [ascii::Char; N], + pub literal: char, +} + +/// Marker type to indicate that the character is always escaped, +/// used to optimize the iterator implementation. +#[derive(Clone, Copy)] +#[non_exhaustive] +pub(crate) struct AlwaysEscaped; + +/// Marker type to indicate that the character may be escaped, +/// used to optimize the iterator implementation. +#[derive(Clone, Copy)] +#[non_exhaustive] +pub(crate) struct MaybeEscaped; + +/// An iterator over a possibly escaped character. +#[derive(Clone)] +pub(crate) struct EscapeIterInner { + // Invariant: + // + // If `alive.end <= Self::LITERAL_ESCAPE_START`, `data` must contain + // printable ASCII characters in the `alive` range of its `escape_seq` variant. + // + // If `alive.end > Self::LITERAL_ESCAPE_START`, `data` must contain a + // `char` in its `literal` variant, and the `alive` range must have a + // length of at most `1`. + data: MaybeEscapedCharacter, alive: Range, + escaping: PhantomData, } -impl EscapeIterInner { +impl EscapeIterInner { + const LITERAL_ESCAPE_START: u8 = 128; + + /// # Safety + /// + /// `data.escape_seq` must contain an escape sequence in the range given by `alive`. + #[inline] + const unsafe fn new(data: MaybeEscapedCharacter, alive: Range) -> Self { + // Longer escape sequences are not useful given `alive.end` is at most + // `Self::LITERAL_ESCAPE_START`. + const { assert!(N < Self::LITERAL_ESCAPE_START as usize) }; + + // Check bounds, which implicitly also checks the invariant + // `alive.end <= Self::LITERAL_ESCAPE_START`. + debug_assert!(alive.end <= (N + 1) as u8); + + Self { data, alive, escaping: PhantomData } + } + pub(crate) const fn backslash(c: ascii::Char) -> Self { - let (data, range) = backslash(c); - Self { data, alive: range } + let (escape_seq, alive) = backslash(c); + // SAFETY: `escape_seq` contains an escape sequence in the range given by `alive`. + unsafe { Self::new(MaybeEscapedCharacter { escape_seq }, alive) } } pub(crate) const fn ascii(c: u8) -> Self { - let (data, range) = escape_ascii(c); - Self { data, alive: range } + let (escape_seq, alive) = escape_ascii(c); + // SAFETY: `escape_seq` contains an escape sequence in the range given by `alive`. + unsafe { Self::new(MaybeEscapedCharacter { escape_seq }, alive) } } pub(crate) const fn unicode(c: char) -> Self { - let (data, range) = escape_unicode(c); - Self { data, alive: range } + let (escape_seq, alive) = escape_unicode(c); + // SAFETY: `escape_seq` contains an escape sequence in the range given by `alive`. + unsafe { Self::new(MaybeEscapedCharacter { escape_seq }, alive) } } #[inline] pub(crate) const fn empty() -> Self { - Self { data: [ascii::Char::Null; N], alive: 0..0 } + // SAFETY: `0..0` ensures an empty escape sequence. + unsafe { Self::new(MaybeEscapedCharacter { escape_seq: [ascii::Char::Null; N] }, 0..0) } } #[inline] - pub(crate) fn as_ascii(&self) -> &[ascii::Char] { - // SAFETY: `self.alive` is guaranteed to be a valid range for indexing `self.data`. - unsafe { - self.data.get_unchecked(usize::from(self.alive.start)..usize::from(self.alive.end)) - } + pub(crate) fn len(&self) -> usize { + usize::from(self.alive.end - self.alive.start) } #[inline] - pub(crate) fn as_str(&self) -> &str { - self.as_ascii().as_str() + pub(crate) fn advance_by(&mut self, n: usize) -> Result<(), NonZero> { + self.alive.advance_by(n) } #[inline] - pub(crate) fn len(&self) -> usize { - usize::from(self.alive.end - self.alive.start) + pub(crate) fn advance_back_by(&mut self, n: usize) -> Result<(), NonZero> { + self.alive.advance_back_by(n) + } + + /// Returns a `char` if `self.data` contains one in its `literal` variant. + #[inline] + const fn to_char(&self) -> Option { + if self.alive.end > Self::LITERAL_ESCAPE_START { + // SAFETY: We just checked that `self.data` contains a `char` in + // its `literal` variant. + return Some(unsafe { self.data.literal }); + } + + None } + /// Returns the printable ASCII characters in the `escape_seq` variant of `self.data` + /// as a string. + /// + /// # Safety + /// + /// - `self.data` must contain printable ASCII characters in its `escape_seq` variant. + /// - `self.alive` must be a valid range for `self.data.escape_seq`. + #[inline] + unsafe fn to_str_unchecked(&self) -> &str { + debug_assert!(self.alive.end <= Self::LITERAL_ESCAPE_START); + + // SAFETY: The caller guarantees `self.data` contains printable ASCII + // characters in its `escape_seq` variant, and `self.alive` is + // a valid range for `self.data.escape_seq`. + unsafe { + self.data + .escape_seq + .get_unchecked(usize::from(self.alive.start)..usize::from(self.alive.end)) + .as_str() + } + } +} + +impl EscapeIterInner { pub(crate) fn next(&mut self) -> Option { let i = self.alive.next()?; - // SAFETY: `i` is guaranteed to be a valid index for `self.data`. - unsafe { Some(self.data.get_unchecked(usize::from(i)).to_u8()) } + // SAFETY: The `AlwaysEscaped` marker guarantees that `self.data` + // contains printable ASCII characters in its `escape_seq` + // variant, and `i` is guaranteed to be a valid index for + // `self.data.escape_seq`. + unsafe { Some(self.data.escape_seq.get_unchecked(usize::from(i)).to_u8()) } } pub(crate) fn next_back(&mut self) -> Option { let i = self.alive.next_back()?; - // SAFETY: `i` is guaranteed to be a valid index for `self.data`. - unsafe { Some(self.data.get_unchecked(usize::from(i)).to_u8()) } + // SAFETY: The `AlwaysEscaped` marker guarantees that `self.data` + // contains printable ASCII characters in its `escape_seq` + // variant, and `i` is guaranteed to be a valid index for + // `self.data.escape_seq`. + unsafe { Some(self.data.escape_seq.get_unchecked(usize::from(i)).to_u8()) } } +} - pub(crate) fn advance_by(&mut self, n: usize) -> Result<(), NonZero> { - self.alive.advance_by(n) +impl EscapeIterInner { + // This is the only way to create any `EscapeIterInner` containing a `char` in + // the `literal` variant of its `self.data`, meaning the `AlwaysEscaped` marker + // guarantees that `self.data` contains printable ASCII characters in its + // `escape_seq` variant. + pub(crate) const fn printable(c: char) -> Self { + Self { + data: MaybeEscapedCharacter { literal: c }, + // Uphold the invariant `alive.end > Self::LITERAL_ESCAPE_START`, and ensure + // `len` behaves correctly for iterating through one character literal. + alive: Self::LITERAL_ESCAPE_START..(Self::LITERAL_ESCAPE_START + 1), + escaping: PhantomData, + } } - pub(crate) fn advance_back_by(&mut self, n: usize) -> Result<(), NonZero> { - self.alive.advance_back_by(n) + pub(crate) fn next(&mut self) -> Option { + let i = self.alive.next()?; + + if let Some(c) = self.to_char() { + return Some(c); + } + + // SAFETY: At this point, `self.data` must contain printable ASCII + // characters in its `escape_seq` variant, and `i` is + // guaranteed to be a valid index for `self.data.escape_seq`. + Some(char::from(unsafe { self.data.escape_seq.get_unchecked(usize::from(i)).to_u8() })) + } +} + +impl fmt::Display for EscapeIterInner { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + // SAFETY: The `AlwaysEscaped` marker guarantees that `self.data` + // contains printable ASCII chars, and `self.alive` is + // guaranteed to be a valid range for `self.data`. + f.write_str(unsafe { self.to_str_unchecked() }) + } +} + +impl fmt::Display for EscapeIterInner { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + if let Some(c) = self.to_char() { + return f.write_char(c); + } + + // SAFETY: At this point, `self.data` must contain printable ASCII + // characters in its `escape_seq` variant, and `self.alive` + // is guaranteed to be a valid range for `self.data`. + f.write_str(unsafe { self.to_str_unchecked() }) + } +} + +impl fmt::Debug for EscapeIterInner { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.debug_tuple("EscapeIterInner").field(&format_args!("'{}'", self)).finish() + } +} + +impl fmt::Debug for EscapeIterInner { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.debug_tuple("EscapeIterInner").field(&format_args!("'{}'", self)).finish() } } diff --git a/library/core/src/slice/ascii.rs b/library/core/src/slice/ascii.rs index b4d9a1b1ca4fd..181ae82959c66 100644 --- a/library/core/src/slice/ascii.rs +++ b/library/core/src/slice/ascii.rs @@ -308,7 +308,7 @@ impl<'a> fmt::Display for EscapeAscii<'a> { if let Some(&b) = bytes.first() { // guaranteed to be non-empty, better to write it as a str - f.write_str(ascii::escape_default(b).as_str())?; + fmt::Display::fmt(&ascii::escape_default(b), f)?; bytes = &bytes[1..]; } }