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/18] 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/18] 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/18] 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/18] 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/18] 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 0e1db54b7eba50b79ff318eb31bf44e0c7b6a4c2 Mon Sep 17 00:00:00 2001 From: Chris Denton Date: Sat, 14 Jun 2025 17:38:10 +0000 Subject: [PATCH 06/18] Windows: Use anonymous pipes in Command --- library/std/src/sys/pal/windows/c.rs | 17 ++ .../std/src/sys/pal/windows/c/bindings.txt | 17 ++ .../std/src/sys/pal/windows/c/windows_sys.rs | 19 +- library/std/src/sys/pal/windows/pipe.rs | 175 +++++++++--------- 4 files changed, 142 insertions(+), 86 deletions(-) diff --git a/library/std/src/sys/pal/windows/c.rs b/library/std/src/sys/pal/windows/c.rs index ac1c5e9932e1c..53dec105d0c85 100644 --- a/library/std/src/sys/pal/windows/c.rs +++ b/library/std/src/sys/pal/windows/c.rs @@ -119,6 +119,23 @@ unsafe extern "system" { pub fn ProcessPrng(pbdata: *mut u8, cbdata: usize) -> BOOL; } +windows_targets::link!("ntdll.dll" "system" fn NtCreateNamedPipeFile( + filehandle: *mut HANDLE, + desiredaccess: FILE_ACCESS_RIGHTS, + objectattributes: *const OBJECT_ATTRIBUTES, + iostatusblock: *mut IO_STATUS_BLOCK, + shareaccess: FILE_SHARE_MODE, + createdisposition: NTCREATEFILE_CREATE_DISPOSITION, + createoptions: NTCREATEFILE_CREATE_OPTIONS, + namedpipetype: u32, + readmode: u32, + completionmode: u32, + maximuminstances: u32, + inboundquota: u32, + outboundquota: u32, + defaulttimeout: *const u64, +) -> NTSTATUS); + // Functions that aren't available on every version of Windows that we support, // but we still use them and just provide some form of a fallback implementation. compat_fn_with_fallback! { diff --git a/library/std/src/sys/pal/windows/c/bindings.txt b/library/std/src/sys/pal/windows/c/bindings.txt index a99c474c763c5..827d96e73db41 100644 --- a/library/std/src/sys/pal/windows/c/bindings.txt +++ b/library/std/src/sys/pal/windows/c/bindings.txt @@ -2060,6 +2060,14 @@ FILE_OPEN_REPARSE_POINT FILE_OPEN_REQUIRING_OPLOCK FILE_OVERWRITE FILE_OVERWRITE_IF +FILE_PIPE_ACCEPT_REMOTE_CLIENTS +FILE_PIPE_BYTE_STREAM_MODE +FILE_PIPE_BYTE_STREAM_TYPE +FILE_PIPE_COMPLETE_OPERATION +FILE_PIPE_MESSAGE_MODE +FILE_PIPE_MESSAGE_TYPE +FILE_PIPE_QUEUE_OPERATION +FILE_PIPE_REJECT_REMOTE_CLIENTS FILE_RANDOM_ACCESS FILE_READ_ATTRIBUTES FILE_READ_DATA @@ -2294,7 +2302,16 @@ NtOpenFile NtReadFile NTSTATUS NtWriteFile +OBJ_CASE_INSENSITIVE OBJ_DONT_REPARSE +OBJ_EXCLUSIVE +OBJ_FORCE_ACCESS_CHECK +OBJ_IGNORE_IMPERSONATED_DEVICEMAP +OBJ_INHERIT +OBJ_KERNEL_HANDLE +OBJ_OPENIF +OBJ_OPENLINK +OBJ_PERMANENT OPEN_ALWAYS OPEN_EXISTING OpenProcessToken diff --git a/library/std/src/sys/pal/windows/c/windows_sys.rs b/library/std/src/sys/pal/windows/c/windows_sys.rs index 95bf8040229d0..b2e3aabc63353 100644 --- a/library/std/src/sys/pal/windows/c/windows_sys.rs +++ b/library/std/src/sys/pal/windows/c/windows_sys.rs @@ -1,4 +1,4 @@ -// Bindings generated by `windows-bindgen` 0.61.0 +// Bindings generated by `windows-bindgen` 0.61.1 #![allow(non_snake_case, non_upper_case_globals, non_camel_case_types, dead_code, clippy::all)] @@ -2552,6 +2552,14 @@ pub const FILE_OPEN_REPARSE_POINT: NTCREATEFILE_CREATE_OPTIONS = 2097152u32; pub const FILE_OPEN_REQUIRING_OPLOCK: NTCREATEFILE_CREATE_OPTIONS = 65536u32; pub const FILE_OVERWRITE: NTCREATEFILE_CREATE_DISPOSITION = 4u32; pub const FILE_OVERWRITE_IF: NTCREATEFILE_CREATE_DISPOSITION = 5u32; +pub const FILE_PIPE_ACCEPT_REMOTE_CLIENTS: u32 = 0u32; +pub const FILE_PIPE_BYTE_STREAM_MODE: u32 = 0u32; +pub const FILE_PIPE_BYTE_STREAM_TYPE: u32 = 0u32; +pub const FILE_PIPE_COMPLETE_OPERATION: u32 = 1u32; +pub const FILE_PIPE_MESSAGE_MODE: u32 = 1u32; +pub const FILE_PIPE_MESSAGE_TYPE: u32 = 1u32; +pub const FILE_PIPE_QUEUE_OPERATION: u32 = 0u32; +pub const FILE_PIPE_REJECT_REMOTE_CLIENTS: u32 = 2u32; pub const FILE_RANDOM_ACCESS: NTCREATEFILE_CREATE_OPTIONS = 2048u32; pub const FILE_READ_ATTRIBUTES: FILE_ACCESS_RIGHTS = 128u32; pub const FILE_READ_DATA: FILE_ACCESS_RIGHTS = 1u32; @@ -2983,7 +2991,16 @@ impl Default for OBJECT_ATTRIBUTES { } } pub type OBJECT_ATTRIBUTE_FLAGS = u32; +pub const OBJ_CASE_INSENSITIVE: OBJECT_ATTRIBUTE_FLAGS = 64u32; pub const OBJ_DONT_REPARSE: OBJECT_ATTRIBUTE_FLAGS = 4096u32; +pub const OBJ_EXCLUSIVE: OBJECT_ATTRIBUTE_FLAGS = 32u32; +pub const OBJ_FORCE_ACCESS_CHECK: OBJECT_ATTRIBUTE_FLAGS = 1024u32; +pub const OBJ_IGNORE_IMPERSONATED_DEVICEMAP: OBJECT_ATTRIBUTE_FLAGS = 2048u32; +pub const OBJ_INHERIT: OBJECT_ATTRIBUTE_FLAGS = 2u32; +pub const OBJ_KERNEL_HANDLE: OBJECT_ATTRIBUTE_FLAGS = 512u32; +pub const OBJ_OPENIF: OBJECT_ATTRIBUTE_FLAGS = 128u32; +pub const OBJ_OPENLINK: OBJECT_ATTRIBUTE_FLAGS = 256u32; +pub const OBJ_PERMANENT: OBJECT_ATTRIBUTE_FLAGS = 16u32; pub const OPEN_ALWAYS: FILE_CREATION_DISPOSITION = 4u32; pub const OPEN_EXISTING: FILE_CREATION_DISPOSITION = 3u32; #[repr(C)] diff --git a/library/std/src/sys/pal/windows/pipe.rs b/library/std/src/sys/pal/windows/pipe.rs index 00d469fbaf8c7..bc5d05c45052a 100644 --- a/library/std/src/sys/pal/windows/pipe.rs +++ b/library/std/src/sys/pal/windows/pipe.rs @@ -1,14 +1,9 @@ -use crate::ffi::OsStr; use crate::io::{self, BorrowedCursor, IoSlice, IoSliceMut}; +use crate::ops::Neg; use crate::os::windows::prelude::*; -use crate::path::Path; -use crate::random::{DefaultRandomSource, Random}; -use crate::sync::atomic::Ordering::Relaxed; -use crate::sync::atomic::{Atomic, AtomicUsize}; +use crate::sys::api::utf16; use crate::sys::c; -use crate::sys::fs::{File, OpenOptions}; use crate::sys::handle::Handle; -use crate::sys::pal::windows::api::{self, WinError}; use crate::sys_common::{FromInner, IntoInner}; use crate::{mem, ptr}; @@ -62,92 +57,113 @@ pub fn anon_pipe(ours_readable: bool, their_handle_inheritable: bool) -> io::Res // Note that we specifically do *not* use `CreatePipe` here because // unfortunately the anonymous pipes returned do not support overlapped - // operations. Instead, we create a "hopefully unique" name and create a - // named pipe which has overlapped operations enabled. + // operations. Instead, we use `NtCreateNamedPipeFile` to create the + // anonymous pipe with overlapped support. // - // Once we do this, we connect do it as usual via `CreateFileW`, and then + // Once we do this, we connect to it via `NtOpenFile`, and then // we return those reader/writer halves. Note that the `ours` pipe return // value is always the named pipe, whereas `theirs` is just the normal file. // This should hopefully shield us from child processes which assume their // stdout is a named pipe, which would indeed be odd! unsafe { - let ours; - let mut name; - let mut tries = 0; - loop { - tries += 1; - name = format!( - r"\\.\pipe\__rust_anonymous_pipe1__.{}.{}", - c::GetCurrentProcessId(), - random_number(), + let mut io_status = c::IO_STATUS_BLOCK::default(); + let mut object_attributes = c::OBJECT_ATTRIBUTES::default(); + object_attributes.Length = size_of::() as u32; + + // Open a handle to the pipe filesystem (`\??\PIPE\`). + // This will be used when creating a new annon pipe. + let pipe_fs = { + let path = c::UNICODE_STRING::from_ref(utf16!(r"\??\PIPE\")); + object_attributes.ObjectName = &path; + let mut pipe_fs = ptr::null_mut(); + let status = c::NtOpenFile( + &mut pipe_fs, + c::SYNCHRONIZE | c::GENERIC_READ, + &object_attributes, + &mut io_status, + c::FILE_SHARE_READ | c::FILE_SHARE_WRITE, + c::FILE_SYNCHRONOUS_IO_NONALERT, // synchronous access ); - let wide_name = OsStr::new(&name).encode_wide().chain(Some(0)).collect::>(); - let mut flags = c::FILE_FLAG_FIRST_PIPE_INSTANCE | c::FILE_FLAG_OVERLAPPED; - if ours_readable { - flags |= c::PIPE_ACCESS_INBOUND; + if c::nt_success(status) { + Handle::from_raw_handle(pipe_fs) } else { - flags |= c::PIPE_ACCESS_OUTBOUND; + return Err(io::Error::from_raw_os_error(c::RtlNtStatusToDosError(status) as i32)); } + }; - let handle = c::CreateNamedPipeW( - wide_name.as_ptr(), - flags, - c::PIPE_TYPE_BYTE - | c::PIPE_READMODE_BYTE - | c::PIPE_WAIT - | c::PIPE_REJECT_REMOTE_CLIENTS, + // From now on we're using handles instead of paths to create and open pipes. + // So set the `ObjectName` to a zero length string. + let empty = c::UNICODE_STRING::default(); + object_attributes.ObjectName = ∅ + + // Create our side of the pipe for async access. + let ours = { + // Use the pipe filesystem as the root directory. + // With no name provided, an anonymous pipe will be created. + object_attributes.RootDirectory = pipe_fs.as_raw_handle(); + + // A negative timeout value is a relative time (rather than an absolute time). + // The time is given in 100's of nanoseconds so this is 50 milliseconds. + // This value was chosen to be consistent with the default timeout set by `CreateNamedPipeW` + // See: https://learn.microsoft.com/en-us/windows/win32/api/namedpipeapi/nf-namedpipeapi-createnamedpipew + let timeout = (50_i64 * 10000).neg() as u64; + + let mut ours = ptr::null_mut(); + let status = c::NtCreateNamedPipeFile( + &mut ours, + c::SYNCHRONIZE | if ours_readable { c::GENERIC_READ } else { c::GENERIC_WRITE }, + &object_attributes, + &mut io_status, + if ours_readable { c::FILE_SHARE_WRITE } else { c::FILE_SHARE_READ }, + c::FILE_CREATE, + 0, + c::FILE_PIPE_BYTE_STREAM_TYPE, + c::FILE_PIPE_BYTE_STREAM_MODE, + c::FILE_PIPE_QUEUE_OPERATION, + // only allow one client pipe 1, PIPE_BUFFER_CAPACITY, PIPE_BUFFER_CAPACITY, - 0, - ptr::null_mut(), + &timeout, ); - - // We pass the `FILE_FLAG_FIRST_PIPE_INSTANCE` flag above, and we're - // also just doing a best effort at selecting a unique name. If - // `ERROR_ACCESS_DENIED` is returned then it could mean that we - // accidentally conflicted with an already existing pipe, so we try - // again. - // - // Don't try again too much though as this could also perhaps be a - // legit error. - if handle == c::INVALID_HANDLE_VALUE { - let error = api::get_last_error(); - if tries < 10 && error == WinError::ACCESS_DENIED { - continue; - } else { - return Err(io::Error::from_raw_os_error(error.code as i32)); - } + if c::nt_success(status) { + Handle::from_raw_handle(ours) + } else { + return Err(io::Error::from_raw_os_error(c::RtlNtStatusToDosError(status) as i32)); } + }; - ours = Handle::from_raw_handle(handle); - break; - } + // Open their side of the pipe for synchronous access. + let theirs = { + // We can reopen the anonymous pipe without a name by setting + // RootDirectory to the pipe handle and not setting a path name, + object_attributes.RootDirectory = ours.as_raw_handle(); - // Connect to the named pipe we just created. This handle is going to be - // returned in `theirs`, so if `ours` is readable we want this to be - // writable, otherwise if `ours` is writable we want this to be - // readable. - // - // Additionally we don't enable overlapped mode on this because most - // client processes aren't enabled to work with that. - let mut opts = OpenOptions::new(); - opts.write(ours_readable); - opts.read(!ours_readable); - opts.share_mode(0); - let size = size_of::(); - let mut sa = c::SECURITY_ATTRIBUTES { - nLength: size as u32, - lpSecurityDescriptor: ptr::null_mut(), - bInheritHandle: their_handle_inheritable as i32, + if their_handle_inheritable { + object_attributes.Attributes |= c::OBJ_INHERIT; + } + let mut theirs = ptr::null_mut(); + let status = c::NtOpenFile( + &mut theirs, + c::SYNCHRONIZE + | if ours_readable { + c::GENERIC_WRITE | c::FILE_READ_ATTRIBUTES + } else { + c::GENERIC_READ + }, + &object_attributes, + &mut io_status, + 0, + c::FILE_NON_DIRECTORY_FILE | c::FILE_SYNCHRONOUS_IO_NONALERT, + ); + if c::nt_success(status) { + Handle::from_raw_handle(theirs) + } else { + return Err(io::Error::from_raw_os_error(c::RtlNtStatusToDosError(status) as i32)); + } }; - opts.security_attributes(&mut sa); - let theirs = File::open(Path::new(&name), &opts)?; - Ok(Pipes { - ours: AnonPipe { inner: ours }, - theirs: AnonPipe { inner: theirs.into_inner() }, - }) + Ok(Pipes { ours: AnonPipe { inner: ours }, theirs: AnonPipe { inner: theirs } }) } } @@ -191,17 +207,6 @@ pub fn spawn_pipe_relay( Ok(theirs) } -fn random_number() -> usize { - static N: Atomic = AtomicUsize::new(0); - loop { - if N.load(Relaxed) != 0 { - return N.fetch_add(1, Relaxed); - } - - N.store(usize::random(&mut DefaultRandomSource), Relaxed); - } -} - impl AnonPipe { pub fn handle(&self) -> &Handle { &self.inner From 3cb0cba054d9d1871f3a10345d5c30cfc7ac214c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E7=8E=8B=E5=AE=87=E9=80=B8?= Date: Sun, 1 Jun 2025 23:12:35 +0800 Subject: [PATCH 07/18] Handle win32 separator & prefixes for cygwin paths --- library/std/src/path.rs | 17 +- library/std/src/sys/path/cygwin.rs | 92 ++++ library/std/src/sys/path/mod.rs | 5 + library/std/src/sys/path/windows.rs | 175 +------- library/std/src/sys/path/windows/tests.rs | 2 + library/std/src/sys/path/windows_prefix.rs | 182 ++++++++ library/std/tests/path.rs | 472 +++++++++++++++++++++ 7 files changed, 772 insertions(+), 173 deletions(-) create mode 100644 library/std/src/sys/path/cygwin.rs create mode 100644 library/std/src/sys/path/windows_prefix.rs diff --git a/library/std/src/path.rs b/library/std/src/path.rs index 826d9f0f39dc6..be8d9f1b05416 100644 --- a/library/std/src/path.rs +++ b/library/std/src/path.rs @@ -1316,8 +1316,17 @@ impl PathBuf { need_sep = false } + let need_clear = if cfg!(target_os = "cygwin") { + // If path is absolute and its prefix is none, it is like `/foo`, + // and will be handled below. + path.prefix().is_some() + } else { + // On Unix: prefix is always None. + path.is_absolute() || path.prefix().is_some() + }; + // absolute `path` replaces `self` - if path.is_absolute() || path.prefix().is_some() { + if need_clear { self.inner.truncate(0); // verbatim paths need . and .. removed @@ -3616,6 +3625,11 @@ impl Error for NormalizeError {} /// paths, this is currently equivalent to calling /// [`GetFullPathNameW`][windows-path]. /// +/// On Cygwin, this is currently equivalent to calling [`cygwin_conv_path`][cygwin-path] +/// with mode `CCP_WIN_A_TO_POSIX`, and then being processed like other POSIX platforms. +/// If a Windows path is given, it will be converted to an absolute POSIX path without +/// keeping `..`. +/// /// Note that these [may change in the future][changes]. /// /// # Errors @@ -3673,6 +3687,7 @@ impl Error for NormalizeError {} /// [changes]: io#platform-specific-behavior /// [posix-semantics]: https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap04.html#tag_04_13 /// [windows-path]: https://docs.microsoft.com/en-us/windows/win32/api/fileapi/nf-fileapi-getfullpathnamew +/// [cygwin-path]: https://cygwin.com/cygwin-api/func-cygwin-conv-path.html #[stable(feature = "absolute_path", since = "1.79.0")] pub fn absolute>(path: P) -> io::Result { let path = path.as_ref(); diff --git a/library/std/src/sys/path/cygwin.rs b/library/std/src/sys/path/cygwin.rs new file mode 100644 index 0000000000000..e90372805bbf9 --- /dev/null +++ b/library/std/src/sys/path/cygwin.rs @@ -0,0 +1,92 @@ +use crate::ffi::OsString; +use crate::os::unix::ffi::OsStringExt; +use crate::path::{Path, PathBuf}; +use crate::sys::common::small_c_string::run_path_with_cstr; +use crate::sys::cvt; +use crate::{io, ptr}; + +#[inline] +pub fn is_sep_byte(b: u8) -> bool { + b == b'/' || b == b'\\' +} + +/// Cygwin allways prefers `/` over `\`, and it always converts all `/` to `\` +/// internally when calling Win32 APIs. Therefore, the server component of path +/// `\\?\UNC\localhost/share` is `localhost/share` on Win32, but `localhost` +/// on Cygwin. +#[inline] +pub fn is_verbatim_sep(b: u8) -> bool { + b == b'/' || b == b'\\' +} + +pub use super::windows_prefix::parse_prefix; + +pub const MAIN_SEP_STR: &str = "/"; +pub const MAIN_SEP: char = '/'; + +unsafe extern "C" { + // Doc: https://cygwin.com/cygwin-api/func-cygwin-conv-path.html + // Src: https://github.com/cygwin/cygwin/blob/718a15ba50e0d01c79800bd658c2477f9a603540/winsup/cygwin/path.cc#L3902 + // Safety: + // * `what` should be `CCP_WIN_A_TO_POSIX` here + // * `from` is null-terminated UTF-8 path + // * `to` is buffer, the buffer size is `size`. + // + // Converts a path to an absolute POSIX path, no matter the input is Win32 path or POSIX path. + fn cygwin_conv_path( + what: libc::c_uint, + from: *const libc::c_char, + to: *mut u8, + size: libc::size_t, + ) -> libc::ssize_t; +} + +const CCP_WIN_A_TO_POSIX: libc::c_uint = 2; + +/// Make a POSIX path absolute. +pub(crate) fn absolute(path: &Path) -> io::Result { + run_path_with_cstr(path, &|path| { + let conv = CCP_WIN_A_TO_POSIX; + let size = cvt(unsafe { cygwin_conv_path(conv, path.as_ptr(), ptr::null_mut(), 0) })?; + // If success, size should not be 0. + debug_assert!(size >= 1); + let size = size as usize; + let mut buffer = Vec::with_capacity(size); + cvt(unsafe { cygwin_conv_path(conv, path.as_ptr(), buffer.as_mut_ptr(), size) })?; + unsafe { + buffer.set_len(size - 1); + } + Ok(PathBuf::from(OsString::from_vec(buffer))) + }) + .map(|path| { + if path.prefix().is_some() { + return path; + } + + // From unix.rs + let mut components = path.components(); + let path_os = path.as_os_str().as_encoded_bytes(); + + let mut normalized = if path_os.starts_with(b"//") && !path_os.starts_with(b"///") { + components.next(); + PathBuf::from("//") + } else { + PathBuf::new() + }; + normalized.extend(components); + + if path_os.ends_with(b"/") { + normalized.push(""); + } + + normalized + }) +} + +pub(crate) fn is_absolute(path: &Path) -> bool { + if path.as_os_str().as_encoded_bytes().starts_with(b"\\") { + path.has_root() && path.prefix().is_some() + } else { + path.has_root() + } +} diff --git a/library/std/src/sys/path/mod.rs b/library/std/src/sys/path/mod.rs index 1fa4e80d6780c..a4ff4338cf5f4 100644 --- a/library/std/src/sys/path/mod.rs +++ b/library/std/src/sys/path/mod.rs @@ -1,6 +1,7 @@ cfg_if::cfg_if! { if #[cfg(target_os = "windows")] { mod windows; + mod windows_prefix; pub use windows::*; } else if #[cfg(all(target_vendor = "fortanix", target_env = "sgx"))] { mod sgx; @@ -11,6 +12,10 @@ cfg_if::cfg_if! { } else if #[cfg(target_os = "uefi")] { mod uefi; pub use uefi::*; + } else if #[cfg(target_os = "cygwin")] { + mod cygwin; + mod windows_prefix; + pub use cygwin::*; } else { mod unix; pub use unix::*; diff --git a/library/std/src/sys/path/windows.rs b/library/std/src/sys/path/windows.rs index e0e003f6a8192..f124e1e5a71c7 100644 --- a/library/std/src/sys/path/windows.rs +++ b/library/std/src/sys/path/windows.rs @@ -1,5 +1,5 @@ use crate::ffi::{OsStr, OsString}; -use crate::path::{Path, PathBuf, Prefix}; +use crate::path::{Path, PathBuf}; use crate::sys::api::utf16; use crate::sys::pal::{c, fill_utf16_buf, os2path, to_u16s}; use crate::{io, ptr}; @@ -7,6 +7,8 @@ use crate::{io, ptr}; #[cfg(test)] mod tests; +pub use super::windows_prefix::parse_prefix; + pub const MAIN_SEP_STR: &str = "\\"; pub const MAIN_SEP: char = '\\'; @@ -77,177 +79,6 @@ pub(crate) fn append_suffix(path: PathBuf, suffix: &OsStr) -> PathBuf { path.into() } -struct PrefixParser<'a, const LEN: usize> { - path: &'a OsStr, - prefix: [u8; LEN], -} - -impl<'a, const LEN: usize> PrefixParser<'a, LEN> { - #[inline] - fn get_prefix(path: &OsStr) -> [u8; LEN] { - let mut prefix = [0; LEN]; - // SAFETY: Only ASCII characters are modified. - for (i, &ch) in path.as_encoded_bytes().iter().take(LEN).enumerate() { - prefix[i] = if ch == b'/' { b'\\' } else { ch }; - } - prefix - } - - fn new(path: &'a OsStr) -> Self { - Self { path, prefix: Self::get_prefix(path) } - } - - fn as_slice(&self) -> PrefixParserSlice<'a, '_> { - PrefixParserSlice { - path: self.path, - prefix: &self.prefix[..LEN.min(self.path.len())], - index: 0, - } - } -} - -struct PrefixParserSlice<'a, 'b> { - path: &'a OsStr, - prefix: &'b [u8], - index: usize, -} - -impl<'a> PrefixParserSlice<'a, '_> { - fn strip_prefix(&self, prefix: &str) -> Option { - self.prefix[self.index..] - .starts_with(prefix.as_bytes()) - .then_some(Self { index: self.index + prefix.len(), ..*self }) - } - - fn prefix_bytes(&self) -> &'a [u8] { - &self.path.as_encoded_bytes()[..self.index] - } - - fn finish(self) -> &'a OsStr { - // SAFETY: The unsafety here stems from converting between &OsStr and - // &[u8] and back. This is safe to do because (1) we only look at ASCII - // contents of the encoding and (2) new &OsStr values are produced only - // from ASCII-bounded slices of existing &OsStr values. - unsafe { OsStr::from_encoded_bytes_unchecked(&self.path.as_encoded_bytes()[self.index..]) } - } -} - -pub fn parse_prefix(path: &OsStr) -> Option> { - use Prefix::{DeviceNS, Disk, UNC, Verbatim, VerbatimDisk, VerbatimUNC}; - - let parser = PrefixParser::<8>::new(path); - let parser = parser.as_slice(); - if let Some(parser) = parser.strip_prefix(r"\\") { - // \\ - - // The meaning of verbatim paths can change when they use a different - // separator. - if let Some(parser) = parser.strip_prefix(r"?\") - && !parser.prefix_bytes().iter().any(|&x| x == b'/') - { - // \\?\ - if let Some(parser) = parser.strip_prefix(r"UNC\") { - // \\?\UNC\server\share - - let path = parser.finish(); - let (server, path) = parse_next_component(path, true); - let (share, _) = parse_next_component(path, true); - - Some(VerbatimUNC(server, share)) - } else { - let path = parser.finish(); - - // in verbatim paths only recognize an exact drive prefix - if let Some(drive) = parse_drive_exact(path) { - // \\?\C: - Some(VerbatimDisk(drive)) - } else { - // \\?\prefix - let (prefix, _) = parse_next_component(path, true); - Some(Verbatim(prefix)) - } - } - } else if let Some(parser) = parser.strip_prefix(r".\") { - // \\.\COM42 - let path = parser.finish(); - let (prefix, _) = parse_next_component(path, false); - Some(DeviceNS(prefix)) - } else { - let path = parser.finish(); - let (server, path) = parse_next_component(path, false); - let (share, _) = parse_next_component(path, false); - - if !server.is_empty() && !share.is_empty() { - // \\server\share - Some(UNC(server, share)) - } else { - // no valid prefix beginning with "\\" recognized - None - } - } - } else { - // If it has a drive like `C:` then it's a disk. - // Otherwise there is no prefix. - parse_drive(path).map(Disk) - } -} - -// Parses a drive prefix, e.g. "C:" and "C:\whatever" -fn parse_drive(path: &OsStr) -> Option { - // In most DOS systems, it is not possible to have more than 26 drive letters. - // See . - fn is_valid_drive_letter(drive: &u8) -> bool { - drive.is_ascii_alphabetic() - } - - match path.as_encoded_bytes() { - [drive, b':', ..] if is_valid_drive_letter(drive) => Some(drive.to_ascii_uppercase()), - _ => None, - } -} - -// Parses a drive prefix exactly, e.g. "C:" -fn parse_drive_exact(path: &OsStr) -> Option { - // only parse two bytes: the drive letter and the drive separator - if path.as_encoded_bytes().get(2).map(|&x| is_sep_byte(x)).unwrap_or(true) { - parse_drive(path) - } else { - None - } -} - -// Parse the next path component. -// -// Returns the next component and the rest of the path excluding the component and separator. -// Does not recognize `/` as a separator character if `verbatim` is true. -fn parse_next_component(path: &OsStr, verbatim: bool) -> (&OsStr, &OsStr) { - let separator = if verbatim { is_verbatim_sep } else { is_sep_byte }; - - match path.as_encoded_bytes().iter().position(|&x| separator(x)) { - Some(separator_start) => { - let separator_end = separator_start + 1; - - let component = &path.as_encoded_bytes()[..separator_start]; - - // Panic safe - // The max `separator_end` is `bytes.len()` and `bytes[bytes.len()..]` is a valid index. - let path = &path.as_encoded_bytes()[separator_end..]; - - // SAFETY: `path` is a valid wtf8 encoded slice and each of the separators ('/', '\') - // is encoded in a single byte, therefore `bytes[separator_start]` and - // `bytes[separator_end]` must be code point boundaries and thus - // `bytes[..separator_start]` and `bytes[separator_end..]` are valid wtf8 slices. - unsafe { - ( - OsStr::from_encoded_bytes_unchecked(component), - OsStr::from_encoded_bytes_unchecked(path), - ) - } - } - None => (path, OsStr::new("")), - } -} - /// Returns a UTF-16 encoded path capable of bypassing the legacy `MAX_PATH` limits. /// /// This path may or may not have a verbatim prefix. diff --git a/library/std/src/sys/path/windows/tests.rs b/library/std/src/sys/path/windows/tests.rs index 9eb79203dcac7..830f48d7bfc94 100644 --- a/library/std/src/sys/path/windows/tests.rs +++ b/library/std/src/sys/path/windows/tests.rs @@ -1,4 +1,6 @@ +use super::super::windows_prefix::*; use super::*; +use crate::path::Prefix; #[test] fn test_parse_next_component() { diff --git a/library/std/src/sys/path/windows_prefix.rs b/library/std/src/sys/path/windows_prefix.rs new file mode 100644 index 0000000000000..b9dfe754485ab --- /dev/null +++ b/library/std/src/sys/path/windows_prefix.rs @@ -0,0 +1,182 @@ +//! Parse Windows prefixes, for both Windows and Cygwin. + +use super::{is_sep_byte, is_verbatim_sep}; +use crate::ffi::OsStr; +use crate::path::Prefix; + +struct PrefixParser<'a, const LEN: usize> { + path: &'a OsStr, + prefix: [u8; LEN], +} + +impl<'a, const LEN: usize> PrefixParser<'a, LEN> { + #[inline] + fn get_prefix(path: &OsStr) -> [u8; LEN] { + let mut prefix = [0; LEN]; + // SAFETY: Only ASCII characters are modified. + for (i, &ch) in path.as_encoded_bytes().iter().take(LEN).enumerate() { + prefix[i] = if ch == b'/' { b'\\' } else { ch }; + } + prefix + } + + fn new(path: &'a OsStr) -> Self { + Self { path, prefix: Self::get_prefix(path) } + } + + fn as_slice(&self) -> PrefixParserSlice<'a, '_> { + PrefixParserSlice { + path: self.path, + prefix: &self.prefix[..LEN.min(self.path.len())], + index: 0, + } + } +} + +struct PrefixParserSlice<'a, 'b> { + path: &'a OsStr, + prefix: &'b [u8], + index: usize, +} + +impl<'a> PrefixParserSlice<'a, '_> { + fn strip_prefix(&self, prefix: &str) -> Option { + self.prefix[self.index..] + .starts_with(prefix.as_bytes()) + .then_some(Self { index: self.index + prefix.len(), ..*self }) + } + + fn prefix_bytes(&self) -> &'a [u8] { + &self.path.as_encoded_bytes()[..self.index] + } + + fn finish(self) -> &'a OsStr { + // SAFETY: The unsafety here stems from converting between &OsStr and + // &[u8] and back. This is safe to do because (1) we only look at ASCII + // contents of the encoding and (2) new &OsStr values are produced only + // from ASCII-bounded slices of existing &OsStr values. + unsafe { OsStr::from_encoded_bytes_unchecked(&self.path.as_encoded_bytes()[self.index..]) } + } +} + +pub fn parse_prefix(path: &OsStr) -> Option> { + use Prefix::{DeviceNS, Disk, UNC, Verbatim, VerbatimDisk, VerbatimUNC}; + + let parser = PrefixParser::<8>::new(path); + let parser = parser.as_slice(); + if let Some(parser) = parser.strip_prefix(r"\\") { + // \\ + + // It's a POSIX path. + if cfg!(target_os = "cygwin") && !path.as_encoded_bytes().iter().any(|&x| x == b'\\') { + return None; + } + + // The meaning of verbatim paths can change when they use a different + // separator. + if let Some(parser) = parser.strip_prefix(r"?\") + // Cygwin allows `/` in verbatim paths. + && (cfg!(target_os = "cygwin") || !parser.prefix_bytes().iter().any(|&x| x == b'/')) + { + // \\?\ + if let Some(parser) = parser.strip_prefix(r"UNC\") { + // \\?\UNC\server\share + + let path = parser.finish(); + let (server, path) = parse_next_component(path, true); + let (share, _) = parse_next_component(path, true); + + Some(VerbatimUNC(server, share)) + } else { + let path = parser.finish(); + + // in verbatim paths only recognize an exact drive prefix + if let Some(drive) = parse_drive_exact(path) { + // \\?\C: + Some(VerbatimDisk(drive)) + } else { + // \\?\prefix + let (prefix, _) = parse_next_component(path, true); + Some(Verbatim(prefix)) + } + } + } else if let Some(parser) = parser.strip_prefix(r".\") { + // \\.\COM42 + let path = parser.finish(); + let (prefix, _) = parse_next_component(path, false); + Some(DeviceNS(prefix)) + } else { + let path = parser.finish(); + let (server, path) = parse_next_component(path, false); + let (share, _) = parse_next_component(path, false); + + if !server.is_empty() && !share.is_empty() { + // \\server\share + Some(UNC(server, share)) + } else { + // no valid prefix beginning with "\\" recognized + None + } + } + } else { + // If it has a drive like `C:` then it's a disk. + // Otherwise there is no prefix. + Some(Disk(parse_drive(path)?)) + } +} + +// Parses a drive prefix, e.g. "C:" and "C:\whatever" +fn parse_drive(path: &OsStr) -> Option { + // In most DOS systems, it is not possible to have more than 26 drive letters. + // See . + fn is_valid_drive_letter(drive: &u8) -> bool { + drive.is_ascii_alphabetic() + } + + match path.as_encoded_bytes() { + [drive, b':', ..] if is_valid_drive_letter(drive) => Some(drive.to_ascii_uppercase()), + _ => None, + } +} + +// Parses a drive prefix exactly, e.g. "C:" +fn parse_drive_exact(path: &OsStr) -> Option { + // only parse two bytes: the drive letter and the drive separator + if path.as_encoded_bytes().get(2).map(|&x| is_sep_byte(x)).unwrap_or(true) { + parse_drive(path) + } else { + None + } +} + +// Parse the next path component. +// +// Returns the next component and the rest of the path excluding the component and separator. +// Does not recognize `/` as a separator character on Windows if `verbatim` is true. +pub(crate) fn parse_next_component(path: &OsStr, verbatim: bool) -> (&OsStr, &OsStr) { + let separator = if verbatim { is_verbatim_sep } else { is_sep_byte }; + + match path.as_encoded_bytes().iter().position(|&x| separator(x)) { + Some(separator_start) => { + let separator_end = separator_start + 1; + + let component = &path.as_encoded_bytes()[..separator_start]; + + // Panic safe + // The max `separator_end` is `bytes.len()` and `bytes[bytes.len()..]` is a valid index. + let path = &path.as_encoded_bytes()[separator_end..]; + + // SAFETY: `path` is a valid wtf8 encoded slice and each of the separators ('/', '\') + // is encoded in a single byte, therefore `bytes[separator_start]` and + // `bytes[separator_end]` must be code point boundaries and thus + // `bytes[..separator_start]` and `bytes[separator_end..]` are valid wtf8 slices. + unsafe { + ( + OsStr::from_encoded_bytes_unchecked(component), + OsStr::from_encoded_bytes_unchecked(path), + ) + } + } + None => (path, OsStr::new("")), + } +} diff --git a/library/std/tests/path.rs b/library/std/tests/path.rs index be0dda1d426f3..901d2770f203e 100644 --- a/library/std/tests/path.rs +++ b/library/std/tests/path.rs @@ -1112,6 +1112,473 @@ pub fn test_decompositions_windows() { ); } +// Unix paths are tested in `test_decompositions_unix` above. +#[test] +#[cfg(target_os = "cygwin")] +pub fn test_decompositions_cygwin() { + t!("\\", + iter: ["/"], + has_root: true, + is_absolute: false, + parent: None, + file_name: None, + file_stem: None, + extension: None, + file_prefix: None + ); + + t!("c:", + iter: ["c:"], + has_root: false, + is_absolute: false, + parent: None, + file_name: None, + file_stem: None, + extension: None, + file_prefix: None + ); + + t!("c:\\", + iter: ["c:", "/"], + has_root: true, + is_absolute: true, + parent: None, + file_name: None, + file_stem: None, + extension: None, + file_prefix: None + ); + + t!("c:/", + iter: ["c:", "/"], + has_root: true, + is_absolute: true, + parent: None, + file_name: None, + file_stem: None, + extension: None, + file_prefix: None + ); + + t!("a\\b\\c", + iter: ["a", "b", "c"], + has_root: false, + is_absolute: false, + parent: Some("a\\b"), + file_name: Some("c"), + file_stem: Some("c"), + extension: None, + file_prefix: Some("c") + ); + + t!("\\a", + iter: ["/", "a"], + has_root: true, + is_absolute: false, + parent: Some("\\"), + file_name: Some("a"), + file_stem: Some("a"), + extension: None, + file_prefix: Some("a") + ); + + t!("c:\\foo.txt", + iter: ["c:", "/", "foo.txt"], + has_root: true, + is_absolute: true, + parent: Some("c:\\"), + file_name: Some("foo.txt"), + file_stem: Some("foo"), + extension: Some("txt"), + file_prefix: Some("foo") + ); + + t!("\\\\server\\share\\foo.txt", + iter: ["\\\\server\\share", "/", "foo.txt"], + has_root: true, + is_absolute: true, + parent: Some("\\\\server\\share\\"), + file_name: Some("foo.txt"), + file_stem: Some("foo"), + extension: Some("txt"), + file_prefix: Some("foo") + ); + + t!("//server/share\\foo.txt", + iter: ["//server/share", "/", "foo.txt"], + has_root: true, + is_absolute: true, + parent: Some("//server/share\\"), + file_name: Some("foo.txt"), + file_stem: Some("foo"), + extension: Some("txt"), + file_prefix: Some("foo") + ); + + t!("//server/share/foo.txt", + iter: ["/", "server", "share", "foo.txt"], + has_root: true, + is_absolute: true, + parent: Some("//server/share"), + file_name: Some("foo.txt"), + file_stem: Some("foo"), + extension: Some("txt"), + file_prefix: Some("foo") + ); + + t!("\\\\server\\share", + iter: ["\\\\server\\share", "/"], + has_root: true, + is_absolute: true, + parent: None, + file_name: None, + file_stem: None, + extension: None, + file_prefix: None + ); + + t!("\\\\server", + iter: ["/", "server"], + has_root: true, + is_absolute: false, + parent: Some("\\"), + file_name: Some("server"), + file_stem: Some("server"), + extension: None, + file_prefix: Some("server") + ); + + t!("\\\\?\\bar\\foo.txt", + iter: ["\\\\?\\bar", "/", "foo.txt"], + has_root: true, + is_absolute: true, + parent: Some("\\\\?\\bar\\"), + file_name: Some("foo.txt"), + file_stem: Some("foo"), + extension: Some("txt"), + file_prefix: Some("foo") + ); + + t!("\\\\?\\bar", + iter: ["\\\\?\\bar"], + has_root: true, + is_absolute: true, + parent: None, + file_name: None, + file_stem: None, + extension: None, + file_prefix: None + ); + + t!("\\\\?\\", + iter: ["\\\\?\\"], + has_root: true, + is_absolute: true, + parent: None, + file_name: None, + file_stem: None, + extension: None, + file_prefix: None + ); + + t!("\\\\?\\UNC\\server\\share\\foo.txt", + iter: ["\\\\?\\UNC\\server\\share", "/", "foo.txt"], + has_root: true, + is_absolute: true, + parent: Some("\\\\?\\UNC\\server\\share\\"), + file_name: Some("foo.txt"), + file_stem: Some("foo"), + extension: Some("txt"), + file_prefix: Some("foo") + ); + + t!("\\\\?\\UNC\\server/share\\foo.txt", + iter: ["\\\\?\\UNC\\server/share", "/", "foo.txt"], + has_root: true, + is_absolute: true, + parent: Some("\\\\?\\UNC\\server/share\\"), + file_name: Some("foo.txt"), + file_stem: Some("foo"), + extension: Some("txt"), + file_prefix: Some("foo") + ); + + t!("//?/UNC/server\\share/foo.txt", + iter: ["//?/UNC/server\\share", "/", "foo.txt"], + has_root: true, + is_absolute: true, + parent: Some("//?/UNC/server\\share/"), + file_name: Some("foo.txt"), + file_stem: Some("foo"), + extension: Some("txt"), + file_prefix: Some("foo") + ); + + t!("//?/UNC/server/share/foo.txt", + iter: ["/", "?", "UNC", "server", "share", "foo.txt"], + has_root: true, + is_absolute: true, + parent: Some("//?/UNC/server/share"), + file_name: Some("foo.txt"), + file_stem: Some("foo"), + extension: Some("txt"), + file_prefix: Some("foo") + ); + + t!("\\\\?\\UNC\\server", + iter: ["\\\\?\\UNC\\server"], + has_root: true, + is_absolute: true, + parent: None, + file_name: None, + file_stem: None, + extension: None, + file_prefix: None + ); + + t!("\\\\?\\UNC\\", + iter: ["\\\\?\\UNC\\"], + has_root: true, + is_absolute: true, + parent: None, + file_name: None, + file_stem: None, + extension: None, + file_prefix: None + ); + + t!("\\\\?\\C:\\foo.txt", + iter: ["\\\\?\\C:", "/", "foo.txt"], + has_root: true, + is_absolute: true, + parent: Some("\\\\?\\C:\\"), + file_name: Some("foo.txt"), + file_stem: Some("foo"), + extension: Some("txt"), + file_prefix: Some("foo") + ); + + t!("//?/C:\\foo.txt", + iter: ["//?/C:", "/", "foo.txt"], + has_root: true, + is_absolute: true, + parent: Some("//?/C:\\"), + file_name: Some("foo.txt"), + file_stem: Some("foo"), + extension: Some("txt"), + file_prefix: Some("foo") + ); + + t!("//?/C:/foo.txt", + iter: ["/", "?", "C:", "foo.txt"], + has_root: true, + is_absolute: true, + parent: Some("//?/C:"), + file_name: Some("foo.txt"), + file_stem: Some("foo"), + extension: Some("txt"), + file_prefix: Some("foo") + ); + + t!("\\\\?\\C:\\", + iter: ["\\\\?\\C:", "/"], + has_root: true, + is_absolute: true, + parent: None, + file_name: None, + file_stem: None, + extension: None, + file_prefix: None + ); + + t!("\\\\?\\C:", + iter: ["\\\\?\\C:"], + has_root: true, + is_absolute: true, + parent: None, + file_name: None, + file_stem: None, + extension: None, + file_prefix: None + ); + + t!("\\\\?\\foo/bar", + iter: ["\\\\?\\foo", "/", "bar"], + has_root: true, + is_absolute: true, + parent: Some("\\\\?\\foo/"), + file_name: Some("bar"), + file_stem: Some("bar"), + extension: None, + file_prefix: Some("bar") + ); + + t!("\\\\?\\C:/foo/bar", + iter: ["\\\\?\\C:", "/", "foo", "bar"], + has_root: true, + is_absolute: true, + parent: Some("\\\\?\\C:/foo"), + file_name: Some("bar"), + file_stem: Some("bar"), + extension: None, + file_prefix: Some("bar") + ); + + t!("\\\\.\\foo\\bar", + iter: ["\\\\.\\foo", "/", "bar"], + has_root: true, + is_absolute: true, + parent: Some("\\\\.\\foo\\"), + file_name: Some("bar"), + file_stem: Some("bar"), + extension: None, + file_prefix: Some("bar") + ); + + t!("\\\\.\\foo", + iter: ["\\\\.\\foo", "/"], + has_root: true, + is_absolute: true, + parent: None, + file_name: None, + file_stem: None, + extension: None, + file_prefix: None + ); + + t!("\\\\.\\foo/bar", + iter: ["\\\\.\\foo", "/", "bar"], + has_root: true, + is_absolute: true, + parent: Some("\\\\.\\foo/"), + file_name: Some("bar"), + file_stem: Some("bar"), + extension: None, + file_prefix: Some("bar") + ); + + t!("\\\\.\\foo\\bar/baz", + iter: ["\\\\.\\foo", "/", "bar", "baz"], + has_root: true, + is_absolute: true, + parent: Some("\\\\.\\foo\\bar"), + file_name: Some("baz"), + file_stem: Some("baz"), + extension: None, + file_prefix: Some("baz") + ); + + t!("\\\\.\\", + iter: ["\\\\.\\", "/"], + has_root: true, + is_absolute: true, + parent: None, + file_name: None, + file_stem: None, + extension: None, + file_prefix: None + ); + + t!("//.\\foo/bar", + iter: ["//.\\foo", "/", "bar"], + has_root: true, + is_absolute: true, + parent: Some("//.\\foo/"), + file_name: Some("bar"), + file_stem: Some("bar"), + extension: None, + file_prefix: Some("bar") + ); + + t!("\\\\./foo/bar", + iter: ["\\\\./foo", "/", "bar"], + has_root: true, + is_absolute: true, + parent: Some("\\\\./foo/"), + file_name: Some("bar"), + file_stem: Some("bar"), + extension: None, + file_prefix: Some("bar") + ); + + t!("//./foo\\bar", + iter: ["//./foo", "/", "bar"], + has_root: true, + is_absolute: true, + parent: Some("//./foo\\"), + file_name: Some("bar"), + file_stem: Some("bar"), + extension: None, + file_prefix: Some("bar") + ); + + t!("//./?/C:/foo/bar", + iter: ["/", "?", "C:", "foo", "bar"], + has_root: true, + is_absolute: true, + parent: Some("//./?/C:/foo"), + file_name: Some("bar"), + file_stem: Some("bar"), + extension: None, + file_prefix: Some("bar") + ); + + t!("//././../././../?/C:/foo/bar", + iter: ["/", "..", "..", "?", "C:", "foo", "bar"], + has_root: true, + is_absolute: true, + parent: Some("//././../././../?/C:/foo"), + file_name: Some("bar"), + file_stem: Some("bar"), + extension: None, + file_prefix: Some("bar") + ); + + t!("\\\\?\\a\\b\\", + iter: ["\\\\?\\a", "/", "b"], + has_root: true, + is_absolute: true, + parent: Some("\\\\?\\a\\"), + file_name: Some("b"), + file_stem: Some("b"), + extension: None, + file_prefix: Some("b") + ); + + t!("\\\\?\\C:\\foo.txt.zip", + iter: ["\\\\?\\C:", "/", "foo.txt.zip"], + has_root: true, + is_absolute: true, + parent: Some("\\\\?\\C:\\"), + file_name: Some("foo.txt.zip"), + file_stem: Some("foo.txt"), + extension: Some("zip"), + file_prefix: Some("foo") + ); + + t!("\\\\?\\C:\\.foo.txt.zip", + iter: ["\\\\?\\C:", "/", ".foo.txt.zip"], + has_root: true, + is_absolute: true, + parent: Some("\\\\?\\C:\\"), + file_name: Some(".foo.txt.zip"), + file_stem: Some(".foo.txt"), + extension: Some("zip"), + file_prefix: Some(".foo") + ); + + t!("\\\\?\\C:\\.foo", + iter: ["\\\\?\\C:", "/", ".foo"], + has_root: true, + is_absolute: true, + parent: Some("\\\\?\\C:\\"), + file_name: Some(".foo"), + file_stem: Some(".foo"), + extension: None, + file_prefix: Some(".foo") + ); +} + #[test] pub fn test_stem_ext() { t!("foo", @@ -1227,6 +1694,11 @@ pub fn test_push() { tp!("/foo/bar", "/", "/"); tp!("/foo/bar", "/baz", "/baz"); tp!("/foo/bar", "./baz", "/foo/bar/./baz"); + + if cfg!(target_os = "cygwin") { + tp!("c:\\", "windows", "c:\\windows"); + tp!("c:", "windows", "c:windows"); + } } else { tp!("", "foo", "foo"); tp!("foo", "bar", r"foo\bar"); From 32c0cb0f5a18ae1cb1966944d287e15ff9225b7d Mon Sep 17 00:00:00 2001 From: Jieyou Xu Date: Mon, 16 Jun 2025 16:17:32 +0800 Subject: [PATCH 08/18] Add union with default field values case test In particular, there should be no additional errors (default field values for `union` fields are currently erroneously accepted). --- .../ui/structs/default-field-values/failures.rs | 6 ++++++ .../structs/default-field-values/failures.stderr | 16 ++++++++-------- 2 files changed, 14 insertions(+), 8 deletions(-) diff --git a/tests/ui/structs/default-field-values/failures.rs b/tests/ui/structs/default-field-values/failures.rs index dee6566bd0ea1..77640d4f461b7 100644 --- a/tests/ui/structs/default-field-values/failures.rs +++ b/tests/ui/structs/default-field-values/failures.rs @@ -49,6 +49,12 @@ enum E { Variant {} //~ ERROR the `#[default]` attribute may only be used on unit enum variants } +union U +{ + x: i32 = 1, + y: f32 = 2., +} + fn main () { let _ = Foo { .. }; // ok let _ = Foo::default(); // ok diff --git a/tests/ui/structs/default-field-values/failures.stderr b/tests/ui/structs/default-field-values/failures.stderr index aaa75fd3180c5..4f9aedff93e41 100644 --- a/tests/ui/structs/default-field-values/failures.stderr +++ b/tests/ui/structs/default-field-values/failures.stderr @@ -28,19 +28,19 @@ LL | pub struct S; | error: missing field `bar` in initializer - --> $DIR/failures.rs:55:19 + --> $DIR/failures.rs:61:19 | LL | let _ = Bar { .. }; | ^ fields that do not have a defaulted value must be provided explicitly error: missing field `bar` in initializer - --> $DIR/failures.rs:56:27 + --> $DIR/failures.rs:62:27 | LL | let _ = Bar { baz: 0, .. }; | ^ fields that do not have a defaulted value must be provided explicitly error[E0308]: mismatched types - --> $DIR/failures.rs:60:17 + --> $DIR/failures.rs:66:17 | LL | let _ = Rak(..); | --- ^^ expected `i32`, found `RangeFull` @@ -53,19 +53,19 @@ note: tuple struct defined here LL | pub struct Rak(i32 = 42); | ^^^ help: you might have meant to use `..` to skip providing a value for expected fields, but this is only supported on non-tuple struct literals; it is instead interpreted as a `std::ops::RangeFull` literal - --> $DIR/failures.rs:60:17 + --> $DIR/failures.rs:66:17 | LL | let _ = Rak(..); | ^^ error[E0061]: this struct takes 1 argument but 2 arguments were supplied - --> $DIR/failures.rs:62:13 + --> $DIR/failures.rs:68:13 | LL | let _ = Rak(0, ..); | ^^^ -- unexpected argument #2 of type `RangeFull` | help: you might have meant to use `..` to skip providing a value for expected fields, but this is only supported on non-tuple struct literals; it is instead interpreted as a `std::ops::RangeFull` literal - --> $DIR/failures.rs:62:20 + --> $DIR/failures.rs:68:20 | LL | let _ = Rak(0, ..); | ^^ @@ -81,13 +81,13 @@ LL + let _ = Rak(0); | error[E0061]: this struct takes 1 argument but 2 arguments were supplied - --> $DIR/failures.rs:64:13 + --> $DIR/failures.rs:70:13 | LL | let _ = Rak(.., 0); | ^^^ -- unexpected argument #1 of type `RangeFull` | help: you might have meant to use `..` to skip providing a value for expected fields, but this is only supported on non-tuple struct literals; it is instead interpreted as a `std::ops::RangeFull` literal - --> $DIR/failures.rs:64:17 + --> $DIR/failures.rs:70:17 | LL | let _ = Rak(.., 0); | ^^ From 718df66f4f2224efd25947ca32947998436dea88 Mon Sep 17 00:00:00 2001 From: Neal Date: Fri, 13 Jun 2025 21:07:58 -0400 Subject: [PATCH 09/18] Two changes: Have BorrowError & BorrowMutError derive Debug and add more information to Display implementation for BorrowError/BorrowMutError - The BorrowError/BorrowMutError Debug implementations do not print anything differently from what the derived implementation does, so we don't need it. - This change also adds the location field of BorrowError/BorrowMutError to the the Display output when it is present, rewords the error message, and uses the Display trait for outputting the error message instead of Debug. --- library/core/src/cell.rs | 42 +++++++++++++++++----------------------- 1 file changed, 18 insertions(+), 24 deletions(-) diff --git a/library/core/src/cell.rs b/library/core/src/cell.rs index a4b6efe35fc14..1db0f7362df41 100644 --- a/library/core/src/cell.rs +++ b/library/core/src/cell.rs @@ -732,54 +732,48 @@ pub struct RefCell { /// An error returned by [`RefCell::try_borrow`]. #[stable(feature = "try_borrow", since = "1.13.0")] #[non_exhaustive] +#[derive(Debug)] pub struct BorrowError { #[cfg(feature = "debug_refcell")] location: &'static crate::panic::Location<'static>, } #[stable(feature = "try_borrow", since = "1.13.0")] -impl Debug for BorrowError { +impl Display for BorrowError { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - let mut builder = f.debug_struct("BorrowError"); - #[cfg(feature = "debug_refcell")] - builder.field("location", self.location); + let res = write!( + f, + "RefCell already mutably borrowed; a previous borrow was at {}", + self.location + ); - builder.finish() - } -} + #[cfg(not(feature = "debug_refcell"))] + let res = Display::fmt("RefCell already mutably borrowed", f); -#[stable(feature = "try_borrow", since = "1.13.0")] -impl Display for BorrowError { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - Display::fmt("already mutably borrowed", f) + res } } /// An error returned by [`RefCell::try_borrow_mut`]. #[stable(feature = "try_borrow", since = "1.13.0")] #[non_exhaustive] +#[derive(Debug)] pub struct BorrowMutError { #[cfg(feature = "debug_refcell")] location: &'static crate::panic::Location<'static>, } #[stable(feature = "try_borrow", since = "1.13.0")] -impl Debug for BorrowMutError { +impl Display for BorrowMutError { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - let mut builder = f.debug_struct("BorrowMutError"); - #[cfg(feature = "debug_refcell")] - builder.field("location", self.location); + let res = write!(f, "RefCell already borrowed; a previous borrow was at {}", self.location); - builder.finish() - } -} + #[cfg(not(feature = "debug_refcell"))] + let res = Display::fmt("RefCell already borrowed", f); -#[stable(feature = "try_borrow", since = "1.13.0")] -impl Display for BorrowMutError { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - Display::fmt("already borrowed", f) + res } } @@ -788,7 +782,7 @@ impl Display for BorrowMutError { #[track_caller] #[cold] fn panic_already_borrowed(err: BorrowMutError) -> ! { - panic!("already borrowed: {:?}", err) + panic!("{err}") } // This ensures the panicking code is outlined from `borrow` for `RefCell`. @@ -796,7 +790,7 @@ fn panic_already_borrowed(err: BorrowMutError) -> ! { #[track_caller] #[cold] fn panic_already_mutably_borrowed(err: BorrowError) -> ! { - panic!("already mutably borrowed: {:?}", err) + panic!("{err}") } // Positive values represent the number of `Ref` active. Negative values From 2fca05a378d91df3943545e8161e0e2e93943125 Mon Sep 17 00:00:00 2001 From: Neal Date: Fri, 13 Jun 2025 21:16:39 -0400 Subject: [PATCH 10/18] Rename BorrowFlag type to BorrowCounter It's actually used as a counter so update the name to reflect that. --- library/core/src/cell.rs | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/library/core/src/cell.rs b/library/core/src/cell.rs index 1db0f7362df41..a2c1ba835f366 100644 --- a/library/core/src/cell.rs +++ b/library/core/src/cell.rs @@ -719,7 +719,7 @@ impl Cell<[T; N]> { #[rustc_diagnostic_item = "RefCell"] #[stable(feature = "rust1", since = "1.0.0")] pub struct RefCell { - borrow: Cell, + borrow: Cell, // Stores the location of the earliest currently active borrow. // This gets updated whenever we go from having zero borrows // to having a single borrow. When a borrow occurs, this gets included @@ -800,22 +800,22 @@ fn panic_already_mutably_borrowed(err: BorrowError) -> ! { // // `Ref` and `RefMut` are both two words in size, and so there will likely never // be enough `Ref`s or `RefMut`s in existence to overflow half of the `usize` -// range. Thus, a `BorrowFlag` will probably never overflow or underflow. +// range. Thus, a `BorrowCounter` will probably never overflow or underflow. // However, this is not a guarantee, as a pathological program could repeatedly // create and then mem::forget `Ref`s or `RefMut`s. Thus, all code must // explicitly check for overflow and underflow in order to avoid unsafety, or at // least behave correctly in the event that overflow or underflow happens (e.g., // see BorrowRef::new). -type BorrowFlag = isize; -const UNUSED: BorrowFlag = 0; +type BorrowCounter = isize; +const UNUSED: BorrowCounter = 0; #[inline(always)] -fn is_writing(x: BorrowFlag) -> bool { +fn is_writing(x: BorrowCounter) -> bool { x < UNUSED } #[inline(always)] -fn is_reading(x: BorrowFlag) -> bool { +fn is_reading(x: BorrowCounter) -> bool { x > UNUSED } @@ -1395,12 +1395,12 @@ impl From for RefCell { impl, U> CoerceUnsized> for RefCell {} struct BorrowRef<'b> { - borrow: &'b Cell, + borrow: &'b Cell, } impl<'b> BorrowRef<'b> { #[inline] - fn new(borrow: &'b Cell) -> Option> { + fn new(borrow: &'b Cell) -> Option> { let b = borrow.get().wrapping_add(1); if !is_reading(b) { // Incrementing borrow can result in a non-reading value (<= 0) in these cases: @@ -1441,7 +1441,7 @@ impl Clone for BorrowRef<'_> { debug_assert!(is_reading(borrow)); // Prevent the borrow counter from overflowing into // a writing borrow. - assert!(borrow != BorrowFlag::MAX); + assert!(borrow != BorrowCounter::MAX); self.borrow.set(borrow + 1); BorrowRef { borrow: self.borrow } } @@ -1789,7 +1789,7 @@ impl<'b, T: ?Sized> RefMut<'b, T> { } struct BorrowRefMut<'b> { - borrow: &'b Cell, + borrow: &'b Cell, } impl Drop for BorrowRefMut<'_> { @@ -1803,7 +1803,7 @@ impl Drop for BorrowRefMut<'_> { impl<'b> BorrowRefMut<'b> { #[inline] - fn new(borrow: &'b Cell) -> Option> { + fn new(borrow: &'b Cell) -> Option> { // NOTE: Unlike BorrowRefMut::clone, new is called to create the initial // mutable reference, and so there must currently be no existing // references. Thus, while clone increments the mutable refcount, here @@ -1827,7 +1827,7 @@ impl<'b> BorrowRefMut<'b> { let borrow = self.borrow.get(); debug_assert!(is_writing(borrow)); // Prevent the borrow counter from underflowing. - assert!(borrow != BorrowFlag::MIN); + assert!(borrow != BorrowCounter::MIN); self.borrow.set(borrow - 1); BorrowRefMut { borrow: self.borrow } } From 23e35c6bd371485eff7c5f58cb59ee28da59ba62 Mon Sep 17 00:00:00 2001 From: Tomoaki Kobayashi Date: Tue, 27 May 2025 02:30:53 +0900 Subject: [PATCH 11/18] Add support for repetition to `proc_macro::quote` --- library/proc_macro/src/lib.rs | 3 +- library/proc_macro/src/quote.rs | 328 +++++++++++++++++- tests/ui/proc-macro/quote/auxiliary/basic.rs | 152 +++++++- .../does-not-have-iter-interpolated-dup.rs | 6 +- ...does-not-have-iter-interpolated-dup.stderr | 11 +- .../quote/does-not-have-iter-interpolated.rs | 6 +- .../does-not-have-iter-interpolated.stderr | 11 +- .../quote/does-not-have-iter-separated.rs | 6 +- .../quote/does-not-have-iter-separated.stderr | 10 +- .../ui/proc-macro/quote/does-not-have-iter.rs | 6 +- .../quote/does-not-have-iter.stderr | 10 +- tests/ui/proc-macro/quote/not-quotable.stderr | 4 +- tests/ui/proc-macro/quote/not-repeatable.rs | 6 +- .../ui/proc-macro/quote/not-repeatable.stderr | 23 +- 14 files changed, 519 insertions(+), 63 deletions(-) diff --git a/library/proc_macro/src/lib.rs b/library/proc_macro/src/lib.rs index 32c306be94ecd..b0084edf2f85c 100644 --- a/library/proc_macro/src/lib.rs +++ b/library/proc_macro/src/lib.rs @@ -45,6 +45,7 @@ mod diagnostic; mod escape; mod to_tokens; +use core::ops::BitOr; use std::ffi::CStr; use std::ops::{Range, RangeBounds}; use std::path::PathBuf; @@ -237,7 +238,7 @@ impl Default for TokenStream { } #[unstable(feature = "proc_macro_quote", issue = "54722")] -pub use quote::{quote, quote_span}; +pub use quote::{HasIterator, RepInterp, ThereIsNoIteratorInRepetition, ext, quote, quote_span}; fn tree_to_bridge_tree( tree: TokenTree, diff --git a/library/proc_macro/src/quote.rs b/library/proc_macro/src/quote.rs index bcb15912bb65e..dbb55cd9fb300 100644 --- a/library/proc_macro/src/quote.rs +++ b/library/proc_macro/src/quote.rs @@ -5,9 +5,183 @@ //! items from `proc_macro`, to build a `proc_macro::TokenStream`. use crate::{ - Delimiter, Group, Ident, Literal, Punct, Spacing, Span, ToTokens, TokenStream, TokenTree, + BitOr, Delimiter, Group, Ident, Literal, Punct, Spacing, Span, ToTokens, TokenStream, TokenTree, }; +#[doc(hidden)] +pub struct HasIterator; // True +#[doc(hidden)] +pub struct ThereIsNoIteratorInRepetition; // False + +impl BitOr for ThereIsNoIteratorInRepetition { + type Output = ThereIsNoIteratorInRepetition; + fn bitor(self, _rhs: ThereIsNoIteratorInRepetition) -> ThereIsNoIteratorInRepetition { + ThereIsNoIteratorInRepetition + } +} + +impl BitOr for HasIterator { + type Output = HasIterator; + fn bitor(self, _rhs: ThereIsNoIteratorInRepetition) -> HasIterator { + HasIterator + } +} + +impl BitOr for ThereIsNoIteratorInRepetition { + type Output = HasIterator; + fn bitor(self, _rhs: HasIterator) -> HasIterator { + HasIterator + } +} + +impl BitOr for HasIterator { + type Output = HasIterator; + fn bitor(self, _rhs: HasIterator) -> HasIterator { + HasIterator + } +} + +/// Extension traits used by the implementation of `quote!`. These are defined +/// in separate traits, rather than as a single trait due to ambiguity issues. +/// +/// These traits expose a `quote_into_iter` method which should allow calling +/// whichever impl happens to be applicable. Calling that method repeatedly on +/// the returned value should be idempotent. +#[doc(hidden)] +pub mod ext { + use core::slice; + use std::collections::btree_set::{self, BTreeSet}; + + use super::{ + HasIterator as HasIter, RepInterp, ThereIsNoIteratorInRepetition as DoesNotHaveIter, + }; + use crate::ToTokens; + + /// Extension trait providing the `quote_into_iter` method on iterators. + #[doc(hidden)] + pub trait RepIteratorExt: Iterator + Sized { + fn quote_into_iter(self) -> (Self, HasIter) { + (self, HasIter) + } + } + + impl RepIteratorExt for T {} + + /// Extension trait providing the `quote_into_iter` method for + /// non-iterable types. These types interpolate the same value in each + /// iteration of the repetition. + #[doc(hidden)] + pub trait RepToTokensExt { + /// Pretend to be an iterator for the purposes of `quote_into_iter`. + /// This allows repeated calls to `quote_into_iter` to continue + /// correctly returning DoesNotHaveIter. + fn next(&self) -> Option<&Self> { + Some(self) + } + + fn quote_into_iter(&self) -> (&Self, DoesNotHaveIter) { + (self, DoesNotHaveIter) + } + } + + impl RepToTokensExt for T {} + + /// Extension trait providing the `quote_into_iter` method for types that + /// can be referenced as an iterator. + #[doc(hidden)] + pub trait RepAsIteratorExt<'q> { + type Iter: Iterator; + + fn quote_into_iter(&'q self) -> (Self::Iter, HasIter); + } + + impl<'q, T: RepAsIteratorExt<'q> + ?Sized> RepAsIteratorExt<'q> for &T { + type Iter = T::Iter; + + fn quote_into_iter(&'q self) -> (Self::Iter, HasIter) { + ::quote_into_iter(*self) + } + } + + impl<'q, T: RepAsIteratorExt<'q> + ?Sized> RepAsIteratorExt<'q> for &mut T { + type Iter = T::Iter; + + fn quote_into_iter(&'q self) -> (Self::Iter, HasIter) { + ::quote_into_iter(*self) + } + } + + impl<'q, T: 'q> RepAsIteratorExt<'q> for [T] { + type Iter = slice::Iter<'q, T>; + + fn quote_into_iter(&'q self) -> (Self::Iter, HasIter) { + (self.iter(), HasIter) + } + } + + impl<'q, T: 'q, const N: usize> RepAsIteratorExt<'q> for [T; N] { + type Iter = slice::Iter<'q, T>; + + fn quote_into_iter(&'q self) -> (Self::Iter, HasIter) { + (self.iter(), HasIter) + } + } + + impl<'q, T: 'q> RepAsIteratorExt<'q> for Vec { + type Iter = slice::Iter<'q, T>; + + fn quote_into_iter(&'q self) -> (Self::Iter, HasIter) { + (self.iter(), HasIter) + } + } + + impl<'q, T: 'q> RepAsIteratorExt<'q> for BTreeSet { + type Iter = btree_set::Iter<'q, T>; + + fn quote_into_iter(&'q self) -> (Self::Iter, HasIter) { + (self.iter(), HasIter) + } + } + + impl<'q, T: RepAsIteratorExt<'q>> RepAsIteratorExt<'q> for RepInterp { + type Iter = T::Iter; + + fn quote_into_iter(&'q self) -> (Self::Iter, HasIter) { + self.0.quote_into_iter() + } + } +} + +// Helper type used within interpolations to allow for repeated binding names. +// Implements the relevant traits, and exports a dummy `next()` method. +#[derive(Copy, Clone)] +#[doc(hidden)] +pub struct RepInterp(pub T); + +impl RepInterp { + // This method is intended to look like `Iterator::next`, and is called when + // a name is bound multiple times, as the previous binding will shadow the + // original `Iterator` object. This allows us to avoid advancing the + // iterator multiple times per iteration. + pub fn next(self) -> Option { + Some(self.0) + } +} + +impl Iterator for RepInterp { + type Item = T::Item; + + fn next(&mut self) -> Option { + self.0.next() + } +} + +impl ToTokens for RepInterp { + fn to_tokens(&self, tokens: &mut TokenStream) { + self.0.to_tokens(tokens); + } +} + macro_rules! minimal_quote_tt { (($($t:tt)*)) => { Group::new(Delimiter::Parenthesis, minimal_quote!($($t)*)) }; ([$($t:tt)*]) => { Group::new(Delimiter::Bracket, minimal_quote!($($t)*)) }; @@ -20,7 +194,13 @@ macro_rules! minimal_quote_tt { (>) => { Punct::new('>', Spacing::Alone) }; (&) => { Punct::new('&', Spacing::Alone) }; (=) => { Punct::new('=', Spacing::Alone) }; + (#) => { Punct::new('#', Spacing::Alone) }; + (|) => { Punct::new('|', Spacing::Alone) }; + (:) => { Punct::new(':', Spacing::Alone) }; + (*) => { Punct::new('*', Spacing::Alone) }; + (_) => { Ident::new("_", Span::def_site()) }; ($i:ident) => { Ident::new(stringify!($i), Span::def_site()) }; + ($lit:literal) => { stringify!($lit).parse::().unwrap() }; } macro_rules! minimal_quote_ts { @@ -36,6 +216,39 @@ macro_rules! minimal_quote_ts { [c.0, c.1].into_iter().collect::() } }; + (=>) => { + { + let mut c = ( + TokenTree::from(Punct::new('=', Spacing::Joint)), + TokenTree::from(Punct::new('>', Spacing::Alone)) + ); + c.0.set_span(Span::def_site()); + c.1.set_span(Span::def_site()); + [c.0, c.1].into_iter().collect::() + } + }; + (+=) => { + { + let mut c = ( + TokenTree::from(Punct::new('+', Spacing::Joint)), + TokenTree::from(Punct::new('=', Spacing::Alone)) + ); + c.0.set_span(Span::def_site()); + c.1.set_span(Span::def_site()); + [c.0, c.1].into_iter().collect::() + } + }; + (!=) => { + { + let mut c = ( + TokenTree::from(Punct::new('!', Spacing::Joint)), + TokenTree::from(Punct::new('=', Spacing::Alone)) + ); + c.0.set_span(Span::def_site()); + c.1.set_span(Span::def_site()); + [c.0, c.1].into_iter().collect::() + } + }; ($t:tt) => { TokenTree::from(minimal_quote_tt!($t)) }; } @@ -71,17 +284,99 @@ pub fn quote(stream: TokenStream) -> TokenStream { let mut after_dollar = false; let mut tokens = crate::TokenStream::new(); - for tree in stream { + let mut iter = stream.into_iter().peekable(); + while let Some(tree) = iter.next() { if after_dollar { after_dollar = false; match tree { + TokenTree::Group(tt) => { + // Handles repetition by expanding `$( CONTENTS ) SEP_OPT *` to `{ REP_EXPANDED }`. + let contents = tt.stream(); + + // The `*` token is also consumed here. + let sep_opt: Option = match (iter.next(), iter.peek()) { + (Some(TokenTree::Punct(sep)), Some(TokenTree::Punct(star))) + if sep.spacing() == Spacing::Joint && star.as_char() == '*' => + { + iter.next(); + Some(sep) + } + (Some(TokenTree::Punct(star)), _) if star.as_char() == '*' => None, + _ => panic!("`$(...)` must be followed by `*` in `quote!`"), + }; + + let mut rep_expanded = TokenStream::new(); + + // Append setup code for a `while`, where recursively quoted `CONTENTS` + // and `SEP_OPT` are repeatedly processed, to `REP_EXPANDED`. + let meta_vars = collect_meta_vars(contents.clone()); + minimal_quote!( + use crate::ext::*; + (@ if sep_opt.is_some() { + minimal_quote!(let mut _i = 0usize;) + } else { + minimal_quote!(();) + }) + let has_iter = crate::ThereIsNoIteratorInRepetition; + ) + .to_tokens(&mut rep_expanded); + for meta_var in &meta_vars { + minimal_quote!( + #[allow(unused_mut)] + let (mut (@ meta_var), i) = (@ meta_var).quote_into_iter(); + let has_iter = has_iter | i; + ) + .to_tokens(&mut rep_expanded); + } + minimal_quote!(let _: crate::HasIterator = has_iter;) + .to_tokens(&mut rep_expanded); + + // Append the `while` to `REP_EXPANDED`. + let mut while_body = TokenStream::new(); + for meta_var in &meta_vars { + minimal_quote!( + let (@ meta_var) = match (@ meta_var).next() { + Some(_x) => crate::RepInterp(_x), + None => break, + }; + ) + .to_tokens(&mut while_body); + } + minimal_quote!( + (@ if let Some(sep) = sep_opt { + minimal_quote!( + if _i > 0 { + (@ minimal_quote!(crate::ToTokens::to_tokens(&crate::TokenTree::Punct(crate::Punct::new( + (@ TokenTree::from(Literal::character(sep.as_char()))), + (@ minimal_quote!(crate::Spacing::Alone)), + )), &mut ts);)) + } + _i += 1; + ) + } else { + minimal_quote!(();) + }) + (@ quote(contents.clone())).to_tokens(&mut ts); + ) + .to_tokens(&mut while_body); + rep_expanded.extend(vec![ + TokenTree::Ident(Ident::new("while", Span::call_site())), + TokenTree::Ident(Ident::new("true", Span::call_site())), + TokenTree::Group(Group::new(Delimiter::Brace, while_body)), + ]); + + minimal_quote!((@ TokenTree::Group(Group::new(Delimiter::Brace, rep_expanded)))).to_tokens(&mut tokens); + continue; + } TokenTree::Ident(_) => { minimal_quote!(crate::ToTokens::to_tokens(&(@ tree), &mut ts);) .to_tokens(&mut tokens); continue; } TokenTree::Punct(ref tt) if tt.as_char() == '$' => {} - _ => panic!("`$` must be followed by an ident or `$` in `quote!`"), + _ => panic!( + "`$` must be followed by an ident or `$` or a repetition group in `quote!`" + ), } } else if let TokenTree::Punct(ref tt) = tree { if tt.as_char() == '$' { @@ -155,6 +450,33 @@ pub fn quote(stream: TokenStream) -> TokenStream { } } +/// Helper function to support macro repetitions like `$( CONTENTS ) SEP_OPT *` in `quote!`. +/// Recursively collects all `Ident`s (meta-variables) that follow a `$` +/// from the given `CONTENTS` stream, preserving their order of appearance. +fn collect_meta_vars(content_stream: TokenStream) -> Vec { + fn helper(stream: TokenStream, out: &mut Vec) { + let mut iter = stream.into_iter().peekable(); + while let Some(tree) = iter.next() { + match &tree { + TokenTree::Punct(tt) if tt.as_char() == '$' => { + if let Some(TokenTree::Ident(id)) = iter.peek() { + out.push(id.clone()); + iter.next(); + } + } + TokenTree::Group(tt) => { + helper(tt.stream(), out); + } + _ => {} + } + } + } + + let mut vars = Vec::new(); + helper(content_stream, &mut vars); + vars +} + /// Quote a `Span` into a `TokenStream`. /// This is needed to implement a custom quoter. #[unstable(feature = "proc_macro_quote", issue = "54722")] diff --git a/tests/ui/proc-macro/quote/auxiliary/basic.rs b/tests/ui/proc-macro/quote/auxiliary/basic.rs index ef726bbfbe3a0..c50bb964eab06 100644 --- a/tests/ui/proc-macro/quote/auxiliary/basic.rs +++ b/tests/ui/proc-macro/quote/auxiliary/basic.rs @@ -4,6 +4,7 @@ extern crate proc_macro; use std::borrow::Cow; +use std::collections::BTreeSet; use std::ffi::{CStr, CString}; use proc_macro::*; @@ -12,6 +13,8 @@ use proc_macro::*; pub fn run_tests(_: TokenStream) -> TokenStream { test_quote_impl(); test_substitution(); + test_iter(); + test_array(); test_advanced(); test_integer(); test_floating(); @@ -24,6 +27,13 @@ pub fn run_tests(_: TokenStream) -> TokenStream { test_ident(); test_underscore(); test_duplicate(); + test_fancy_repetition(); + test_nested_fancy_repetition(); + test_duplicate_name_repetition(); + test_duplicate_name_repetition_no_copy(); + test_btreeset_repetition(); + test_variable_name_conflict(); + test_nonrep_in_repetition(); test_empty_quote(); test_box_str(); test_cow(); @@ -34,6 +44,7 @@ pub fn run_tests(_: TokenStream) -> TokenStream { test_inner_block_comment(); test_outer_attr(); test_inner_attr(); + test_star_after_repetition(); test_quote_raw_id(); TokenStream::new() @@ -49,20 +60,9 @@ pub fn run_tests(_: TokenStream) -> TokenStream { // - fn test_type_inference_for_span // - wrong-type-span.rs // - format_ident: +// - fn test_closure // - fn test_format_ident // - fn test_format_ident_strip_raw -// - repetition: -// - fn test_iter -// - fn test_array -// - fn test_fancy_repetition -// - fn test_nested_fancy_repetition -// - fn test_duplicate_name_repetition -// - fn test_duplicate_name_repetition_no_copy -// - fn test_btreeset_repetition -// - fn test_variable_name_conflict -// - fn test_nonrep_in_repetition -// - fn test_closure -// - fn test_star_after_repetition struct X; @@ -99,6 +99,39 @@ fn test_substitution() { assert_eq!(expected, tokens.to_string()); } +fn test_iter() { + let primes = &[X, X, X, X]; + + assert_eq!("X X X X", quote!($($primes)*).to_string()); + + assert_eq!("X, X, X, X,", quote!($($primes,)*).to_string()); + + assert_eq!("X, X, X, X", quote!($($primes),*).to_string()); +} + +fn test_array() { + let array: [u8; 40] = [0; 40]; + let _ = quote!($($array $array)*); + + let ref_array: &[u8; 40] = &[0; 40]; + let _ = quote!($($ref_array $ref_array)*); + + let ref_slice: &[u8] = &[0; 40]; + let _ = quote!($($ref_slice $ref_slice)*); + + let array: [X; 2] = [X, X]; // !Copy + let _ = quote!($($array $array)*); + + let ref_array: &[X; 2] = &[X, X]; + let _ = quote!($($ref_array $ref_array)*); + + let ref_slice: &[X] = &[X, X]; + let _ = quote!($($ref_slice $ref_slice)*); + + let array_of_array: [[u8; 2]; 2] = [[0; 2]; 2]; + let _ = quote!($($($array_of_array)*)*); +} + fn test_advanced() { let generics = quote!( <'a, T> ); @@ -279,6 +312,88 @@ fn test_duplicate() { assert_eq!(expected, tokens.to_string()); } +fn test_fancy_repetition() { + let foo = vec!["a", "b"]; + let bar = vec![true, false]; + + let tokens = quote! { + $($foo: $bar),* + }; + + let expected = r#""a" : true, "b" : false"#; + assert_eq!(expected, tokens.to_string()); +} + +fn test_nested_fancy_repetition() { + let nested = vec![vec!['a', 'b', 'c'], vec!['x', 'y', 'z']]; + + let tokens = quote! { + $( + $($nested)* + ),* + }; + + let expected = "'a' 'b' 'c', 'x' 'y' 'z'"; + assert_eq!(expected, tokens.to_string()); +} + +fn test_duplicate_name_repetition() { + let foo = &["a", "b"]; + + let tokens = quote! { + $($foo: $foo),* + $($foo: $foo),* + }; + + let expected = r#""a" : "a", "b" : "b" "a" : "a", "b" : "b""#; + assert_eq!(expected, tokens.to_string()); +} + +fn test_duplicate_name_repetition_no_copy() { + let foo = vec!["a".to_owned(), "b".to_owned()]; + + let tokens = quote! { + $($foo: $foo),* + }; + + let expected = r#""a" : "a", "b" : "b""#; + assert_eq!(expected, tokens.to_string()); +} + +fn test_btreeset_repetition() { + let mut set = BTreeSet::new(); + set.insert("a".to_owned()); + set.insert("b".to_owned()); + + let tokens = quote! { + $($set: $set),* + }; + + let expected = r#""a" : "a", "b" : "b""#; + assert_eq!(expected, tokens.to_string()); +} + +fn test_variable_name_conflict() { + // The implementation of `#(...),*` uses the variable `_i` but it should be + // fine, if a little confusing when debugging. + let _i = vec!['a', 'b']; + let tokens = quote! { $($_i),* }; + let expected = "'a', 'b'"; + assert_eq!(expected, tokens.to_string()); +} + +fn test_nonrep_in_repetition() { + let rep = vec!["a", "b"]; + let nonrep = "c"; + + let tokens = quote! { + $($rep $rep : $nonrep $nonrep),* + }; + + let expected = r#""a" "a" : "c" "c", "b" "b" : "c" "c""#; + assert_eq!(expected, tokens.to_string()); +} + fn test_empty_quote() { let tokens = quote!(); assert_eq!("", tokens.to_string()); @@ -355,6 +470,19 @@ fn test_inner_attr() { assert_eq!(expected, tokens.to_string()); } +// https://github.com/dtolnay/quote/issues/130 +fn test_star_after_repetition() { + let c = vec!['0', '1']; + let tokens = quote! { + $( + f($c); + )* + *out = None; + }; + let expected = "f('0'); f('1'); * out = None;"; + assert_eq!(expected, tokens.to_string()); +} + fn test_quote_raw_id() { let id = quote!(r#raw_id); assert_eq!(id.to_string(), "r#raw_id"); diff --git a/tests/ui/proc-macro/quote/does-not-have-iter-interpolated-dup.rs b/tests/ui/proc-macro/quote/does-not-have-iter-interpolated-dup.rs index 2f67ae1bc6edc..418e3dd444dda 100644 --- a/tests/ui/proc-macro/quote/does-not-have-iter-interpolated-dup.rs +++ b/tests/ui/proc-macro/quote/does-not-have-iter-interpolated-dup.rs @@ -1,7 +1,3 @@ -// FIXME(quote): `proc_macro::quote!` doesn't support repetition at the moment, so the stderr is -// expected to be incorrect. -//@ known-bug: #54722 - #![feature(proc_macro_quote)] extern crate proc_macro; @@ -13,5 +9,5 @@ fn main() { // Without some protection against repetitions with no iterator somewhere // inside, this would loop infinitely. - quote!($($nonrep $nonrep)*); + quote!($($nonrep $nonrep)*); //~ ERROR mismatched types } diff --git a/tests/ui/proc-macro/quote/does-not-have-iter-interpolated-dup.stderr b/tests/ui/proc-macro/quote/does-not-have-iter-interpolated-dup.stderr index 5f28a46f3181d..ecb12c1df3b6a 100644 --- a/tests/ui/proc-macro/quote/does-not-have-iter-interpolated-dup.stderr +++ b/tests/ui/proc-macro/quote/does-not-have-iter-interpolated-dup.stderr @@ -1,10 +1,13 @@ -error: proc macro panicked - --> $DIR/does-not-have-iter-interpolated-dup.rs:16:5 +error[E0308]: mismatched types + --> $DIR/does-not-have-iter-interpolated-dup.rs:12:5 | LL | quote!($($nonrep $nonrep)*); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ - | - = help: message: `$` must be followed by an ident or `$` in `quote!` + | | + | expected `HasIterator`, found `ThereIsNoIteratorInRepetition` + | expected due to this + | here the type of `has_iter` is inferred to be `ThereIsNoIteratorInRepetition` error: aborting due to 1 previous error +For more information about this error, try `rustc --explain E0308`. diff --git a/tests/ui/proc-macro/quote/does-not-have-iter-interpolated.rs b/tests/ui/proc-macro/quote/does-not-have-iter-interpolated.rs index 1efb3eac64247..507936770aa4f 100644 --- a/tests/ui/proc-macro/quote/does-not-have-iter-interpolated.rs +++ b/tests/ui/proc-macro/quote/does-not-have-iter-interpolated.rs @@ -1,7 +1,3 @@ -// FIXME(quote): `proc_macro::quote!` doesn't support repetition at the moment, so the stderr is -// expected to be incorrect. -//@ known-bug: #54722 - #![feature(proc_macro_quote)] extern crate proc_macro; @@ -13,5 +9,5 @@ fn main() { // Without some protection against repetitions with no iterator somewhere // inside, this would loop infinitely. - quote!($($nonrep)*); + quote!($($nonrep)*); //~ ERROR mismatched types } diff --git a/tests/ui/proc-macro/quote/does-not-have-iter-interpolated.stderr b/tests/ui/proc-macro/quote/does-not-have-iter-interpolated.stderr index 595aa8587634a..093e2ebc09858 100644 --- a/tests/ui/proc-macro/quote/does-not-have-iter-interpolated.stderr +++ b/tests/ui/proc-macro/quote/does-not-have-iter-interpolated.stderr @@ -1,10 +1,13 @@ -error: proc macro panicked - --> $DIR/does-not-have-iter-interpolated.rs:16:5 +error[E0308]: mismatched types + --> $DIR/does-not-have-iter-interpolated.rs:12:5 | LL | quote!($($nonrep)*); | ^^^^^^^^^^^^^^^^^^^ - | - = help: message: `$` must be followed by an ident or `$` in `quote!` + | | + | expected `HasIterator`, found `ThereIsNoIteratorInRepetition` + | expected due to this + | here the type of `has_iter` is inferred to be `ThereIsNoIteratorInRepetition` error: aborting due to 1 previous error +For more information about this error, try `rustc --explain E0308`. diff --git a/tests/ui/proc-macro/quote/does-not-have-iter-separated.rs b/tests/ui/proc-macro/quote/does-not-have-iter-separated.rs index 5f2ddabc390da..7e41b08f2636d 100644 --- a/tests/ui/proc-macro/quote/does-not-have-iter-separated.rs +++ b/tests/ui/proc-macro/quote/does-not-have-iter-separated.rs @@ -1,7 +1,3 @@ -// FIXME(quote): `proc_macro::quote!` doesn't support repetition at the moment, so the stderr is -// expected to be incorrect. -//@ known-bug: #54722 - #![feature(proc_macro_quote)] extern crate proc_macro; @@ -9,5 +5,5 @@ extern crate proc_macro; use proc_macro::quote; fn main() { - quote!($(a b),*); + quote!($(a b),*); //~ ERROR mismatched types } diff --git a/tests/ui/proc-macro/quote/does-not-have-iter-separated.stderr b/tests/ui/proc-macro/quote/does-not-have-iter-separated.stderr index f6f5d7e007d0f..937209e675ec8 100644 --- a/tests/ui/proc-macro/quote/does-not-have-iter-separated.stderr +++ b/tests/ui/proc-macro/quote/does-not-have-iter-separated.stderr @@ -1,10 +1,12 @@ -error: proc macro panicked - --> $DIR/does-not-have-iter-separated.rs:12:5 +error[E0308]: mismatched types + --> $DIR/does-not-have-iter-separated.rs:8:5 | LL | quote!($(a b),*); | ^^^^^^^^^^^^^^^^ - | - = help: message: `$` must be followed by an ident or `$` in `quote!` + | | + | expected `HasIterator`, found `ThereIsNoIteratorInRepetition` + | expected due to this error: aborting due to 1 previous error +For more information about this error, try `rustc --explain E0308`. diff --git a/tests/ui/proc-macro/quote/does-not-have-iter.rs b/tests/ui/proc-macro/quote/does-not-have-iter.rs index 25ffd786cc614..038851ff76ed6 100644 --- a/tests/ui/proc-macro/quote/does-not-have-iter.rs +++ b/tests/ui/proc-macro/quote/does-not-have-iter.rs @@ -1,7 +1,3 @@ -// FIXME(quote): `proc_macro::quote!` doesn't support repetition at the moment, so the stderr is -// expected to be incorrect. -//@ known-bug: #54722 - #![feature(proc_macro_quote)] extern crate proc_macro; @@ -9,5 +5,5 @@ extern crate proc_macro; use proc_macro::quote; fn main() { - quote!($(a b)*); + quote!($(a b)*); //~ ERROR mismatched types } diff --git a/tests/ui/proc-macro/quote/does-not-have-iter.stderr b/tests/ui/proc-macro/quote/does-not-have-iter.stderr index 0ed1daffc8cdf..e74ea3348992f 100644 --- a/tests/ui/proc-macro/quote/does-not-have-iter.stderr +++ b/tests/ui/proc-macro/quote/does-not-have-iter.stderr @@ -1,10 +1,12 @@ -error: proc macro panicked - --> $DIR/does-not-have-iter.rs:12:5 +error[E0308]: mismatched types + --> $DIR/does-not-have-iter.rs:8:5 | LL | quote!($(a b)*); | ^^^^^^^^^^^^^^^ - | - = help: message: `$` must be followed by an ident or `$` in `quote!` + | | + | expected `HasIterator`, found `ThereIsNoIteratorInRepetition` + | expected due to this error: aborting due to 1 previous error +For more information about this error, try `rustc --explain E0308`. diff --git a/tests/ui/proc-macro/quote/not-quotable.stderr b/tests/ui/proc-macro/quote/not-quotable.stderr index 62a02638e548b..d1c3d06f2b661 100644 --- a/tests/ui/proc-macro/quote/not-quotable.stderr +++ b/tests/ui/proc-macro/quote/not-quotable.stderr @@ -15,8 +15,8 @@ LL | let _ = quote! { $ip }; Cow<'_, T> Option Rc - bool - and 24 others + RepInterp + and 25 others error: aborting due to 1 previous error diff --git a/tests/ui/proc-macro/quote/not-repeatable.rs b/tests/ui/proc-macro/quote/not-repeatable.rs index d115da7318156..0291e4ddf88d6 100644 --- a/tests/ui/proc-macro/quote/not-repeatable.rs +++ b/tests/ui/proc-macro/quote/not-repeatable.rs @@ -1,7 +1,3 @@ -// FIXME(quote): `proc_macro::quote!` doesn't support repetition at the moment, so the stderr is -// expected to be incorrect. -//@ known-bug: #54722 - #![feature(proc_macro_quote)] extern crate proc_macro; @@ -12,5 +8,5 @@ struct Ipv4Addr; fn main() { let ip = Ipv4Addr; - let _ = quote! { $($ip)* }; + let _ = quote! { $($ip)* }; //~ ERROR the method `quote_into_iter` exists for struct `Ipv4Addr`, but its trait bounds were not satisfied } diff --git a/tests/ui/proc-macro/quote/not-repeatable.stderr b/tests/ui/proc-macro/quote/not-repeatable.stderr index 18fbcd7379858..aeda08d7de68b 100644 --- a/tests/ui/proc-macro/quote/not-repeatable.stderr +++ b/tests/ui/proc-macro/quote/not-repeatable.stderr @@ -1,10 +1,25 @@ -error: proc macro panicked - --> $DIR/not-repeatable.rs:15:13 +error[E0599]: the method `quote_into_iter` exists for struct `Ipv4Addr`, but its trait bounds were not satisfied + --> $DIR/not-repeatable.rs:11:13 | +LL | struct Ipv4Addr; + | --------------- method `quote_into_iter` not found for this struct because it doesn't satisfy `Ipv4Addr: Iterator`, `Ipv4Addr: ToTokens`, `Ipv4Addr: proc_macro::ext::RepIteratorExt` or `Ipv4Addr: proc_macro::ext::RepToTokensExt` +... LL | let _ = quote! { $($ip)* }; - | ^^^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^^^^^ method cannot be called on `Ipv4Addr` due to unsatisfied trait bounds | - = help: message: `$` must be followed by an ident or `$` in `quote!` + = note: the following trait bounds were not satisfied: + `Ipv4Addr: Iterator` + which is required by `Ipv4Addr: proc_macro::ext::RepIteratorExt` + `&Ipv4Addr: Iterator` + which is required by `&Ipv4Addr: proc_macro::ext::RepIteratorExt` + `Ipv4Addr: ToTokens` + which is required by `Ipv4Addr: proc_macro::ext::RepToTokensExt` + `&mut Ipv4Addr: Iterator` + which is required by `&mut Ipv4Addr: proc_macro::ext::RepIteratorExt` +note: the traits `Iterator` and `ToTokens` must be implemented + --> $SRC_DIR/proc_macro/src/to_tokens.rs:LL:COL + --> $SRC_DIR/core/src/iter/traits/iterator.rs:LL:COL error: aborting due to 1 previous error +For more information about this error, try `rustc --explain E0599`. From 7cfc51bd509f91ca7418f5dc01780003f48fa2f1 Mon Sep 17 00:00:00 2001 From: rustbot <47979223+rustbot@users.noreply.github.com> Date: Mon, 16 Jun 2025 19:00:49 +0200 Subject: [PATCH 12/18] Update books --- src/doc/book | 2 +- src/doc/reference | 2 +- src/doc/rust-by-example | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/doc/book b/src/doc/book index 634724ea85ebb..4433c9f0cad84 160000 --- a/src/doc/book +++ b/src/doc/book @@ -1 +1 @@ -Subproject commit 634724ea85ebb08a542970bf8871ac8b0f77fd15 +Subproject commit 4433c9f0cad8460bee05ede040587f8a1fa3f1de diff --git a/src/doc/reference b/src/doc/reference index 8e0f593a30f3b..d4c66b346f4b7 160000 --- a/src/doc/reference +++ b/src/doc/reference @@ -1 +1 @@ -Subproject commit 8e0f593a30f3b56ddb0908fb7ab9249974e08738 +Subproject commit d4c66b346f4b72d29e70390a3fa3ea7d4e064db1 diff --git a/src/doc/rust-by-example b/src/doc/rust-by-example index 21f4e32b8b40d..9baa9e863116c 160000 --- a/src/doc/rust-by-example +++ b/src/doc/rust-by-example @@ -1 +1 @@ -Subproject commit 21f4e32b8b40d36453fae16ec07ad4b857c445b6 +Subproject commit 9baa9e863116cb9524a177d5a5c475baac18928a From 8a7b50a5da5315031883bee788b0da92690a4063 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Mon, 16 Jun 2025 17:32:09 +0000 Subject: [PATCH 13/18] Fold unnecessary visit_struct_field_def in AstValidator --- .../rustc_ast_passes/src/ast_validation.rs | 20 ++----------------- 1 file changed, 2 insertions(+), 18 deletions(-) diff --git a/compiler/rustc_ast_passes/src/ast_validation.rs b/compiler/rustc_ast_passes/src/ast_validation.rs index 018887d0e8eaa..b69a91e2f5f86 100644 --- a/compiler/rustc_ast_passes/src/ast_validation.rs +++ b/compiler/rustc_ast_passes/src/ast_validation.rs @@ -224,20 +224,6 @@ impl<'a> AstValidator<'a> { } } - fn visit_struct_field_def(&mut self, field: &'a FieldDef) { - if let Some(ref ident) = field.ident - && ident.name == kw::Underscore - { - self.visit_vis(&field.vis); - self.visit_ident(ident); - self.visit_ty_common(&field.ty); - self.walk_ty(&field.ty); - walk_list!(self, visit_attribute, &field.attrs); - } else { - self.visit_field_def(field); - } - } - fn dcx(&self) -> DiagCtxtHandle<'a> { self.sess.dcx() } @@ -1135,8 +1121,7 @@ impl<'a> Visitor<'a> for AstValidator<'a> { VariantData::Struct { fields, .. } => { self.visit_attrs_vis_ident(&item.attrs, &item.vis, ident); self.visit_generics(generics); - // Permit `Anon{Struct,Union}` as field type. - walk_list!(self, visit_struct_field_def, fields); + walk_list!(self, visit_field_def, fields); } _ => visit::walk_item(self, item), }, @@ -1148,8 +1133,7 @@ impl<'a> Visitor<'a> for AstValidator<'a> { VariantData::Struct { fields, .. } => { self.visit_attrs_vis_ident(&item.attrs, &item.vis, ident); self.visit_generics(generics); - // Permit `Anon{Struct,Union}` as field type. - walk_list!(self, visit_struct_field_def, fields); + walk_list!(self, visit_field_def, fields); } _ => visit::walk_item(self, item), } From 994794a50bd4adfd7f0351872206e170c1bc8d58 Mon Sep 17 00:00:00 2001 From: Urgau Date: Mon, 16 Jun 2025 18:29:37 +0200 Subject: [PATCH 14/18] Handle same-crate macro for borrowck semicolon suggestion --- .../src/diagnostics/explain_borrow.rs | 5 +- .../span-semicolon-issue-139049.fixed | 59 +++++------------- .../borrowck/span-semicolon-issue-139049.rs | 59 +++++------------- .../span-semicolon-issue-139049.stderr | 62 ++++++++++--------- 4 files changed, 65 insertions(+), 120 deletions(-) diff --git a/compiler/rustc_borrowck/src/diagnostics/explain_borrow.rs b/compiler/rustc_borrowck/src/diagnostics/explain_borrow.rs index c4b0f503664ee..095c0df98acc1 100644 --- a/compiler/rustc_borrowck/src/diagnostics/explain_borrow.rs +++ b/compiler/rustc_borrowck/src/diagnostics/explain_borrow.rs @@ -342,10 +342,7 @@ impl<'tcx> BorrowExplanation<'tcx> { } } } else if let LocalInfo::BlockTailTemp(info) = local_decl.local_info() { - let sp = info - .span - .find_ancestor_in_same_ctxt(local_decl.source_info.span) - .unwrap_or(info.span); + let sp = info.span.find_oldest_ancestor_in_same_ctxt(); if info.tail_result_is_ignored { // #85581: If the first mutable borrow's scope contains // the second borrow, this suggestion isn't helpful. diff --git a/tests/ui/borrowck/span-semicolon-issue-139049.fixed b/tests/ui/borrowck/span-semicolon-issue-139049.fixed index 0b263b222963f..c01d1242dd73f 100644 --- a/tests/ui/borrowck/span-semicolon-issue-139049.fixed +++ b/tests/ui/borrowck/span-semicolon-issue-139049.fixed @@ -1,52 +1,25 @@ -// Make sure the generated suggestion suggest editing the user -// code instead of the std macro implementation +// Make sure the generated suggestion suggest editing the user code instead of +// the macro implementation (which might come from an external crate). +// issue: //@ run-rustfix #![allow(dead_code)] -use std::fmt::{self, Display}; - -struct Mutex; - -impl Mutex { - fn lock(&self) -> MutexGuard<'_> { - MutexGuard(self) - } -} - -struct MutexGuard<'a>(&'a Mutex); - -impl<'a> Drop for MutexGuard<'a> { - fn drop(&mut self) {} -} - -struct Out; - -impl Out { - fn write_fmt(&self, _args: fmt::Arguments) {} -} - -impl<'a> Display for MutexGuard<'a> { - fn fmt(&self, _formatter: &mut fmt::Formatter) -> fmt::Result { - Ok(()) - } -} +// You could assume that this comes from an extern crate (it doesn't +// because an aux crate would be overkill for this test). +macro_rules! perform { ($e:expr) => { D(&$e).end() } } +//~^ ERROR does not live long enough +//~| ERROR does not live long enough fn main() { - let _write = { - let mutex = Mutex; - write!(Out, "{}", mutex.lock()); - //~^ ERROR `mutex` does not live long enough - //~| SUGGESTION ; - }; - - let _write = { - use std::io::Write as _; + { let l = (); perform!(l); }; + //~^ SUGGESTION ; - let mutex = Mutex; - let x = write!(std::io::stdout(), "{}", mutex.lock()); x - //~^ ERROR `mutex` does not live long enough - //~| SUGGESTION let x - }; + let _x = { let l = (); let x = perform!(l); x }; + //~^ SUGGESTION let x } + +struct D(T); +impl Drop for D { fn drop(&mut self) {} } +impl D { fn end(&self) -> String { String::new() } } diff --git a/tests/ui/borrowck/span-semicolon-issue-139049.rs b/tests/ui/borrowck/span-semicolon-issue-139049.rs index a92742ac94b2c..43558756c718d 100644 --- a/tests/ui/borrowck/span-semicolon-issue-139049.rs +++ b/tests/ui/borrowck/span-semicolon-issue-139049.rs @@ -1,52 +1,25 @@ -// Make sure the generated suggestion suggest editing the user -// code instead of the std macro implementation +// Make sure the generated suggestion suggest editing the user code instead of +// the macro implementation (which might come from an external crate). +// issue: //@ run-rustfix #![allow(dead_code)] -use std::fmt::{self, Display}; - -struct Mutex; - -impl Mutex { - fn lock(&self) -> MutexGuard<'_> { - MutexGuard(self) - } -} - -struct MutexGuard<'a>(&'a Mutex); - -impl<'a> Drop for MutexGuard<'a> { - fn drop(&mut self) {} -} - -struct Out; - -impl Out { - fn write_fmt(&self, _args: fmt::Arguments) {} -} - -impl<'a> Display for MutexGuard<'a> { - fn fmt(&self, _formatter: &mut fmt::Formatter) -> fmt::Result { - Ok(()) - } -} +// You could assume that this comes from an extern crate (it doesn't +// because an aux crate would be overkill for this test). +macro_rules! perform { ($e:expr) => { D(&$e).end() } } +//~^ ERROR does not live long enough +//~| ERROR does not live long enough fn main() { - let _write = { - let mutex = Mutex; - write!(Out, "{}", mutex.lock()) - //~^ ERROR `mutex` does not live long enough - //~| SUGGESTION ; - }; - - let _write = { - use std::io::Write as _; + { let l = (); perform!(l) }; + //~^ SUGGESTION ; - let mutex = Mutex; - write!(std::io::stdout(), "{}", mutex.lock()) - //~^ ERROR `mutex` does not live long enough - //~| SUGGESTION let x - }; + let _x = { let l = (); perform!(l) }; + //~^ SUGGESTION let x } + +struct D(T); +impl Drop for D { fn drop(&mut self) {} } +impl D { fn end(&self) -> String { String::new() } } diff --git a/tests/ui/borrowck/span-semicolon-issue-139049.stderr b/tests/ui/borrowck/span-semicolon-issue-139049.stderr index 123bdf4bc67e4..8d2de67382bd8 100644 --- a/tests/ui/borrowck/span-semicolon-issue-139049.stderr +++ b/tests/ui/borrowck/span-semicolon-issue-139049.stderr @@ -1,46 +1,48 @@ -error[E0597]: `mutex` does not live long enough - --> $DIR/span-semicolon-issue-139049.rs:39:27 +error[E0597]: `l` does not live long enough + --> $DIR/span-semicolon-issue-139049.rs:11:41 | -LL | let mutex = Mutex; - | ----- binding `mutex` declared here -LL | write!(Out, "{}", mutex.lock()) - | ^^^^^------- - | | - | borrowed value does not live long enough - | a temporary with access to the borrow is created here ... +LL | macro_rules! perform { ($e:expr) => { D(&$e).end() } } + | --^^^- + | | | + | | borrowed value does not live long enough + | a temporary with access to the borrow is created here ... ... -LL | }; - | -- ... and the borrow might be used here, when that temporary is dropped and runs the `Drop` code for type `MutexGuard` - | | - | `mutex` dropped here while still borrowed +LL | { let l = (); perform!(l) }; + | - ----------- -- ... and the borrow might be used here, when that temporary is dropped and runs the `Drop` code for type `D` + | | | | + | | | `l` dropped here while still borrowed + | | in this macro invocation + | binding `l` declared here | + = note: this error originates in the macro `perform` (in Nightly builds, run with -Z macro-backtrace for more info) help: consider adding semicolon after the expression so its temporaries are dropped sooner, before the local variables declared by the block are dropped | -LL | write!(Out, "{}", mutex.lock()); - | + +LL | { let l = (); perform!(l); }; + | + -error[E0597]: `mutex` does not live long enough - --> $DIR/span-semicolon-issue-139049.rs:48:41 +error[E0597]: `l` does not live long enough + --> $DIR/span-semicolon-issue-139049.rs:11:41 | -LL | let mutex = Mutex; - | ----- binding `mutex` declared here -LL | write!(std::io::stdout(), "{}", mutex.lock()) - | ^^^^^------- - | | - | borrowed value does not live long enough - | a temporary with access to the borrow is created here ... +LL | macro_rules! perform { ($e:expr) => { D(&$e).end() } } + | --^^^- + | | | + | | borrowed value does not live long enough + | a temporary with access to the borrow is created here ... ... -LL | }; - | -- ... and the borrow might be used here, when that temporary is dropped and runs the `Drop` code for type `MutexGuard` - | | - | `mutex` dropped here while still borrowed +LL | let _x = { let l = (); perform!(l) }; + | - ----------- -- ... and the borrow might be used here, when that temporary is dropped and runs the `Drop` code for type `D` + | | | | + | | | `l` dropped here while still borrowed + | | in this macro invocation + | binding `l` declared here | = note: the temporary is part of an expression at the end of a block; consider forcing this temporary to be dropped sooner, before the block's local variables are dropped + = note: this error originates in the macro `perform` (in Nightly builds, run with -Z macro-backtrace for more info) help: for example, you could save the expression's value in a new local variable `x` and then make `x` be the expression at the end of the block | -LL | let x = write!(std::io::stdout(), "{}", mutex.lock()); x - | +++++++ +++ +LL | let _x = { let l = (); let x = perform!(l); x }; + | +++++++ +++ error: aborting due to 2 previous errors From 7b29a5d2827f4336ee2c3ca163378c9dc5a82238 Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Mon, 16 Jun 2025 12:07:24 -0700 Subject: [PATCH 15/18] Revert overeager warning for misuse of `--print native-static-libs` In a PR to emit warnings on misuse of `--print native-static-libs`, we did not consider the matter of composing parts of build systems. If you are not directly invoking rustc, it can be difficult to know when you will in fact compile a staticlib, so making sure everyone uses `--print native-static-lib` correctly can be just a nuisance. This reverts the following commits: - f66787a08d57dc1296619b314d2be596085bfeef - 72a9219e82c157041bfc8dfd378c9cb2b09c0650 - 98bb597c05c32365abbd6898f278b097352774ed - c59b70841c36277464b51161e3fcf12dfcb667e0 Next cycle we can reland a slightly more narrowly focused variant or one that focuses on `--emit` instead of `--print native-static-libs`. But in its current state, I am not sure the warning is very useful. --- compiler/rustc_codegen_ssa/src/back/link.rs | 19 ------------------- ...rning-print-link-info-without-staticlib.rs | 5 ----- ...g-print-link-info-without-staticlib.stderr | 6 ------ ...t-warning-while-exe-and-print-link-info.rs | 3 --- ...rning-while-exe-and-print-link-info.stderr | 4 ---- tests/ui/print-request/stability.rs | 1 - 6 files changed, 38 deletions(-) delete mode 100644 tests/ui/print-request/emit-warning-print-link-info-without-staticlib.rs delete mode 100644 tests/ui/print-request/emit-warning-print-link-info-without-staticlib.stderr delete mode 100644 tests/ui/print-request/emit-warning-while-exe-and-print-link-info.rs delete mode 100644 tests/ui/print-request/emit-warning-while-exe-and-print-link-info.stderr diff --git a/compiler/rustc_codegen_ssa/src/back/link.rs b/compiler/rustc_codegen_ssa/src/back/link.rs index 8c52ed6ed1234..8882ba359b774 100644 --- a/compiler/rustc_codegen_ssa/src/back/link.rs +++ b/compiler/rustc_codegen_ssa/src/back/link.rs @@ -69,23 +69,6 @@ pub fn ensure_removed(dcx: DiagCtxtHandle<'_>, path: &Path) { } } -fn check_link_info_print_request(sess: &Session, crate_types: &[CrateType]) { - let print_native_static_libs = - sess.opts.prints.iter().any(|p| p.kind == PrintKind::NativeStaticLibs); - let has_staticlib = crate_types.iter().any(|ct| *ct == CrateType::Staticlib); - if print_native_static_libs { - if !has_staticlib { - sess.dcx() - .warn(format!("cannot output linkage information without staticlib crate-type")); - sess.dcx() - .note(format!("consider `--crate-type staticlib` to print linkage information")); - } else if !sess.opts.output_types.should_link() { - sess.dcx() - .warn(format!("cannot output linkage information when --emit link is not passed")); - } - } -} - /// Performs the linkage portion of the compilation phase. This will generate all /// of the requested outputs for this compilation session. pub fn link_binary( @@ -208,8 +191,6 @@ pub fn link_binary( } } - check_link_info_print_request(sess, &codegen_results.crate_info.crate_types); - // Remove the temporary object file and metadata if we aren't saving temps. sess.time("link_binary_remove_temps", || { // If the user requests that temporaries are saved, don't delete any. diff --git a/tests/ui/print-request/emit-warning-print-link-info-without-staticlib.rs b/tests/ui/print-request/emit-warning-print-link-info-without-staticlib.rs deleted file mode 100644 index b100c062bba4c..0000000000000 --- a/tests/ui/print-request/emit-warning-print-link-info-without-staticlib.rs +++ /dev/null @@ -1,5 +0,0 @@ -//@ compile-flags: --print native-static-libs -//@ check-pass -//~? WARN cannot output linkage information without staticlib crate-type - -fn main() {} diff --git a/tests/ui/print-request/emit-warning-print-link-info-without-staticlib.stderr b/tests/ui/print-request/emit-warning-print-link-info-without-staticlib.stderr deleted file mode 100644 index ceff08baa13a4..0000000000000 --- a/tests/ui/print-request/emit-warning-print-link-info-without-staticlib.stderr +++ /dev/null @@ -1,6 +0,0 @@ -warning: cannot output linkage information without staticlib crate-type - -note: consider `--crate-type staticlib` to print linkage information - -warning: 1 warning emitted - diff --git a/tests/ui/print-request/emit-warning-while-exe-and-print-link-info.rs b/tests/ui/print-request/emit-warning-while-exe-and-print-link-info.rs deleted file mode 100644 index 3e9ca457a9c9b..0000000000000 --- a/tests/ui/print-request/emit-warning-while-exe-and-print-link-info.rs +++ /dev/null @@ -1,3 +0,0 @@ -//@ compile-flags: --print native-static-libs --crate-type staticlib --emit metadata -//@ check-pass -//~? WARN cannot output linkage information when --emit link is not passed diff --git a/tests/ui/print-request/emit-warning-while-exe-and-print-link-info.stderr b/tests/ui/print-request/emit-warning-while-exe-and-print-link-info.stderr deleted file mode 100644 index b32e1437d6b50..0000000000000 --- a/tests/ui/print-request/emit-warning-while-exe-and-print-link-info.stderr +++ /dev/null @@ -1,4 +0,0 @@ -warning: cannot output linkage information when --emit link is not passed - -warning: 1 warning emitted - diff --git a/tests/ui/print-request/stability.rs b/tests/ui/print-request/stability.rs index fbcdf916cc7cd..54142ce78cefb 100644 --- a/tests/ui/print-request/stability.rs +++ b/tests/ui/print-request/stability.rs @@ -110,4 +110,3 @@ fn main() {} //[check_cfg]~? ERROR the `-Z unstable-options` flag must also be passed to enable the `check-cfg` print option //[supported_crate_types]~? ERROR the `-Z unstable-options` flag must also be passed to enable the `supported-crate-types` print option //[target_spec_json]~? ERROR the `-Z unstable-options` flag must also be passed to enable the `target-spec-json` print option -//[native_static_libs]~? WARNING cannot output linkage information without staticlib crate-type From 7859a37ffe0484389d17645f04b79326379525f0 Mon Sep 17 00:00:00 2001 From: ostylk Date: Wed, 11 Jun 2025 18:58:01 +0200 Subject: [PATCH 16/18] explicitly set llvm_abiname option on existing ppc64 targets --- .../rustc_target/src/spec/targets/powerpc64_unknown_freebsd.rs | 1 + .../src/spec/targets/powerpc64_unknown_linux_gnu.rs | 1 + .../src/spec/targets/powerpc64_unknown_linux_musl.rs | 1 + .../rustc_target/src/spec/targets/powerpc64_unknown_openbsd.rs | 1 + compiler/rustc_target/src/spec/targets/powerpc64_wrs_vxworks.rs | 1 + .../src/spec/targets/powerpc64le_unknown_freebsd.rs | 1 + .../src/spec/targets/powerpc64le_unknown_linux_gnu.rs | 1 + .../src/spec/targets/powerpc64le_unknown_linux_musl.rs | 1 + tests/ui/check-cfg/well-known-values.stderr | 2 +- 9 files changed, 9 insertions(+), 1 deletion(-) diff --git a/compiler/rustc_target/src/spec/targets/powerpc64_unknown_freebsd.rs b/compiler/rustc_target/src/spec/targets/powerpc64_unknown_freebsd.rs index dba45776c949a..66733d5d4b89c 100644 --- a/compiler/rustc_target/src/spec/targets/powerpc64_unknown_freebsd.rs +++ b/compiler/rustc_target/src/spec/targets/powerpc64_unknown_freebsd.rs @@ -10,6 +10,7 @@ pub(crate) fn target() -> Target { base.add_pre_link_args(LinkerFlavor::Gnu(Cc::Yes, Lld::No), &["-m64"]); base.max_atomic_width = Some(64); base.stack_probes = StackProbeType::Inline; + base.llvm_abiname = "elfv2".into(); Target { llvm_target: "powerpc64-unknown-freebsd".into(), diff --git a/compiler/rustc_target/src/spec/targets/powerpc64_unknown_linux_gnu.rs b/compiler/rustc_target/src/spec/targets/powerpc64_unknown_linux_gnu.rs index 1f67bc7f3c28a..ecf68ddff8c2f 100644 --- a/compiler/rustc_target/src/spec/targets/powerpc64_unknown_linux_gnu.rs +++ b/compiler/rustc_target/src/spec/targets/powerpc64_unknown_linux_gnu.rs @@ -10,6 +10,7 @@ pub(crate) fn target() -> Target { base.add_pre_link_args(LinkerFlavor::Gnu(Cc::Yes, Lld::No), &["-m64"]); base.max_atomic_width = Some(64); base.stack_probes = StackProbeType::Inline; + base.llvm_abiname = "elfv1".into(); Target { llvm_target: "powerpc64-unknown-linux-gnu".into(), diff --git a/compiler/rustc_target/src/spec/targets/powerpc64_unknown_linux_musl.rs b/compiler/rustc_target/src/spec/targets/powerpc64_unknown_linux_musl.rs index 49413d27a4555..e205aef82856e 100644 --- a/compiler/rustc_target/src/spec/targets/powerpc64_unknown_linux_musl.rs +++ b/compiler/rustc_target/src/spec/targets/powerpc64_unknown_linux_musl.rs @@ -12,6 +12,7 @@ pub(crate) fn target() -> Target { base.stack_probes = StackProbeType::Inline; // FIXME(compiler-team#422): musl targets should be dynamically linked by default. base.crt_static_default = true; + base.llvm_abiname = "elfv2".into(); Target { llvm_target: "powerpc64-unknown-linux-musl".into(), diff --git a/compiler/rustc_target/src/spec/targets/powerpc64_unknown_openbsd.rs b/compiler/rustc_target/src/spec/targets/powerpc64_unknown_openbsd.rs index f5ca54291c697..bcb328020eea8 100644 --- a/compiler/rustc_target/src/spec/targets/powerpc64_unknown_openbsd.rs +++ b/compiler/rustc_target/src/spec/targets/powerpc64_unknown_openbsd.rs @@ -10,6 +10,7 @@ pub(crate) fn target() -> Target { base.add_pre_link_args(LinkerFlavor::Gnu(Cc::Yes, Lld::No), &["-m64"]); base.max_atomic_width = Some(64); base.stack_probes = StackProbeType::Inline; + base.llvm_abiname = "elfv2".into(); Target { llvm_target: "powerpc64-unknown-openbsd".into(), diff --git a/compiler/rustc_target/src/spec/targets/powerpc64_wrs_vxworks.rs b/compiler/rustc_target/src/spec/targets/powerpc64_wrs_vxworks.rs index 3e4a58f568af9..37c888ba51443 100644 --- a/compiler/rustc_target/src/spec/targets/powerpc64_wrs_vxworks.rs +++ b/compiler/rustc_target/src/spec/targets/powerpc64_wrs_vxworks.rs @@ -10,6 +10,7 @@ pub(crate) fn target() -> Target { base.add_pre_link_args(LinkerFlavor::Gnu(Cc::Yes, Lld::No), &["-m64"]); base.max_atomic_width = Some(64); base.stack_probes = StackProbeType::Inline; + base.llvm_abiname = "elfv1".into(); Target { llvm_target: "powerpc64-unknown-linux-gnu".into(), diff --git a/compiler/rustc_target/src/spec/targets/powerpc64le_unknown_freebsd.rs b/compiler/rustc_target/src/spec/targets/powerpc64le_unknown_freebsd.rs index 4640d537e8ea8..3096c4d14ad87 100644 --- a/compiler/rustc_target/src/spec/targets/powerpc64le_unknown_freebsd.rs +++ b/compiler/rustc_target/src/spec/targets/powerpc64le_unknown_freebsd.rs @@ -8,6 +8,7 @@ pub(crate) fn target() -> Target { base.add_pre_link_args(LinkerFlavor::Gnu(Cc::Yes, Lld::No), &["-m64"]); base.max_atomic_width = Some(64); base.stack_probes = StackProbeType::Inline; + base.llvm_abiname = "elfv2".into(); Target { llvm_target: "powerpc64le-unknown-freebsd".into(), diff --git a/compiler/rustc_target/src/spec/targets/powerpc64le_unknown_linux_gnu.rs b/compiler/rustc_target/src/spec/targets/powerpc64le_unknown_linux_gnu.rs index dd3f660d81e4e..9e406af53b5b5 100644 --- a/compiler/rustc_target/src/spec/targets/powerpc64le_unknown_linux_gnu.rs +++ b/compiler/rustc_target/src/spec/targets/powerpc64le_unknown_linux_gnu.rs @@ -8,6 +8,7 @@ pub(crate) fn target() -> Target { base.add_pre_link_args(LinkerFlavor::Gnu(Cc::Yes, Lld::No), &["-m64"]); base.max_atomic_width = Some(64); base.stack_probes = StackProbeType::Inline; + base.llvm_abiname = "elfv2".into(); Target { llvm_target: "powerpc64le-unknown-linux-gnu".into(), diff --git a/compiler/rustc_target/src/spec/targets/powerpc64le_unknown_linux_musl.rs b/compiler/rustc_target/src/spec/targets/powerpc64le_unknown_linux_musl.rs index 9e2bfe2c56fe4..f145c5b8c1451 100644 --- a/compiler/rustc_target/src/spec/targets/powerpc64le_unknown_linux_musl.rs +++ b/compiler/rustc_target/src/spec/targets/powerpc64le_unknown_linux_musl.rs @@ -10,6 +10,7 @@ pub(crate) fn target() -> Target { base.stack_probes = StackProbeType::Inline; // FIXME(compiler-team#422): musl targets should be dynamically linked by default. base.crt_static_default = true; + base.llvm_abiname = "elfv2".into(); Target { llvm_target: "powerpc64le-unknown-linux-musl".into(), diff --git a/tests/ui/check-cfg/well-known-values.stderr b/tests/ui/check-cfg/well-known-values.stderr index 532c1ab13d118..2484974cdc27c 100644 --- a/tests/ui/check-cfg/well-known-values.stderr +++ b/tests/ui/check-cfg/well-known-values.stderr @@ -129,7 +129,7 @@ warning: unexpected `cfg` condition value: `_UNEXPECTED_VALUE` LL | target_abi = "_UNEXPECTED_VALUE", | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | - = note: expected values for `target_abi` are: ``, `abi64`, `abiv2`, `abiv2hf`, `eabi`, `eabihf`, `fortanix`, `ilp32`, `ilp32e`, `llvm`, `macabi`, `sim`, `softfloat`, `spe`, `uwp`, `vec-extabi`, and `x32` + = note: expected values for `target_abi` are: ``, `abi64`, `abiv2`, `abiv2hf`, `eabi`, `eabihf`, `elfv1`, `elfv2`, `fortanix`, `ilp32`, `ilp32e`, `llvm`, `macabi`, `sim`, `softfloat`, `spe`, `uwp`, `vec-extabi`, and `x32` = note: see for more information about checking conditional configuration warning: unexpected `cfg` condition value: `_UNEXPECTED_VALUE` From 9c1180b6238d163fc384d60d85647385d9210343 Mon Sep 17 00:00:00 2001 From: ostylk Date: Wed, 11 Jun 2025 16:28:23 +0200 Subject: [PATCH 17/18] indicate ppc64 elf abi in e_flags --- .../rustc_codegen_ssa/src/back/metadata.rs | 18 ++++++++++++++++++ tests/ui/check-cfg/well-known-values.stderr | 2 +- 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/compiler/rustc_codegen_ssa/src/back/metadata.rs b/compiler/rustc_codegen_ssa/src/back/metadata.rs index a16862c41ee53..d091c46d9c181 100644 --- a/compiler/rustc_codegen_ssa/src/back/metadata.rs +++ b/compiler/rustc_codegen_ssa/src/back/metadata.rs @@ -379,6 +379,24 @@ pub(super) fn elf_e_flags(architecture: Architecture, sess: &Session) -> u32 { }; e_flags } + Architecture::PowerPc64 => { + const EF_PPC64_ABI_UNKNOWN: u32 = 0; + const EF_PPC64_ABI_ELF_V1: u32 = 1; + const EF_PPC64_ABI_ELF_V2: u32 = 2; + + match sess.target.options.llvm_abiname.as_ref() { + // If the flags do not correctly indicate the ABI, + // linkers such as ld.lld assume that the ppc64 object files are always ELFv2 + // which leads to broken binaries if ELFv1 is used for the object files. + "elfv1" => EF_PPC64_ABI_ELF_V1, + "elfv2" => EF_PPC64_ABI_ELF_V2, + "" if sess.target.options.binary_format.to_object() == BinaryFormat::Elf => { + bug!("No ABI specified for this PPC64 ELF target"); + } + // Fall back + _ => EF_PPC64_ABI_UNKNOWN, + } + } _ => 0, } } diff --git a/tests/ui/check-cfg/well-known-values.stderr b/tests/ui/check-cfg/well-known-values.stderr index 2484974cdc27c..532c1ab13d118 100644 --- a/tests/ui/check-cfg/well-known-values.stderr +++ b/tests/ui/check-cfg/well-known-values.stderr @@ -129,7 +129,7 @@ warning: unexpected `cfg` condition value: `_UNEXPECTED_VALUE` LL | target_abi = "_UNEXPECTED_VALUE", | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | - = note: expected values for `target_abi` are: ``, `abi64`, `abiv2`, `abiv2hf`, `eabi`, `eabihf`, `elfv1`, `elfv2`, `fortanix`, `ilp32`, `ilp32e`, `llvm`, `macabi`, `sim`, `softfloat`, `spe`, `uwp`, `vec-extabi`, and `x32` + = note: expected values for `target_abi` are: ``, `abi64`, `abiv2`, `abiv2hf`, `eabi`, `eabihf`, `fortanix`, `ilp32`, `ilp32e`, `llvm`, `macabi`, `sim`, `softfloat`, `spe`, `uwp`, `vec-extabi`, and `x32` = note: see for more information about checking conditional configuration warning: unexpected `cfg` condition value: `_UNEXPECTED_VALUE` From 2dd9cc113067113453efcff722e3a5c0893a6244 Mon Sep 17 00:00:00 2001 From: Jieyou Xu Date: Mon, 16 Jun 2025 17:53:44 +0800 Subject: [PATCH 18/18] Reject union default field values --- compiler/rustc_ast_lowering/messages.ftl | 2 + compiler/rustc_ast_lowering/src/errors.rs | 7 +++ compiler/rustc_ast_lowering/src/item.rs | 42 +++++++++++---- .../feature-gate-default-field-values.rs | 10 ++++ .../feature-gate-default-field-values.stderr | 54 +++++++++++++------ .../structs/default-field-values/failures.rs | 4 +- .../default-field-values/failures.stderr | 14 ++++- 7 files changed, 103 insertions(+), 30 deletions(-) diff --git a/compiler/rustc_ast_lowering/messages.ftl b/compiler/rustc_ast_lowering/messages.ftl index 5ef76fb64aaf2..50eb7c7ae99bd 100644 --- a/compiler/rustc_ast_lowering/messages.ftl +++ b/compiler/rustc_ast_lowering/messages.ftl @@ -179,6 +179,8 @@ ast_lowering_underscore_expr_lhs_assign = in expressions, `_` can only be used on the left-hand side of an assignment .label = `_` not allowed here +ast_lowering_union_default_field_values = unions cannot have default field values + ast_lowering_unstable_inline_assembly = inline assembly is not stable yet on this architecture ast_lowering_unstable_inline_assembly_label_operand_with_outputs = using both label and output operands for inline assembly is unstable diff --git a/compiler/rustc_ast_lowering/src/errors.rs b/compiler/rustc_ast_lowering/src/errors.rs index 576fa9731e906..b444324ef9143 100644 --- a/compiler/rustc_ast_lowering/src/errors.rs +++ b/compiler/rustc_ast_lowering/src/errors.rs @@ -475,3 +475,10 @@ pub(crate) struct UseConstGenericArg { #[suggestion_part(code = "{other_args}")] pub call_args: Span, } + +#[derive(Diagnostic)] +#[diag(ast_lowering_union_default_field_values)] +pub(crate) struct UnionWithDefault { + #[primary_span] + pub span: Span, +} diff --git a/compiler/rustc_ast_lowering/src/item.rs b/compiler/rustc_ast_lowering/src/item.rs index 49110c93954ad..ef27d0ef69b14 100644 --- a/compiler/rustc_ast_lowering/src/item.rs +++ b/compiler/rustc_ast_lowering/src/item.rs @@ -17,6 +17,7 @@ use tracing::instrument; use super::errors::{ InvalidAbi, InvalidAbiSuggestion, MisplacedRelaxTraitBound, TupleStructWithDefault, + UnionWithDefault, }; use super::stability::{enabled_names, gate_unstable_abi}; use super::{ @@ -316,7 +317,7 @@ impl<'hir> LoweringContext<'_, 'hir> { ImplTraitContext::Disallowed(ImplTraitPosition::Generic), |this| { this.arena.alloc_from_iter( - enum_definition.variants.iter().map(|x| this.lower_variant(x)), + enum_definition.variants.iter().map(|x| this.lower_variant(i, x)), ) }, ); @@ -328,7 +329,7 @@ impl<'hir> LoweringContext<'_, 'hir> { generics, id, ImplTraitContext::Disallowed(ImplTraitPosition::Generic), - |this| this.lower_variant_data(hir_id, struct_def), + |this| this.lower_variant_data(hir_id, i, struct_def), ); hir::ItemKind::Struct(ident, generics, struct_def) } @@ -338,7 +339,7 @@ impl<'hir> LoweringContext<'_, 'hir> { generics, id, ImplTraitContext::Disallowed(ImplTraitPosition::Generic), - |this| this.lower_variant_data(hir_id, vdata), + |this| this.lower_variant_data(hir_id, i, vdata), ); hir::ItemKind::Union(ident, generics, vdata) } @@ -714,13 +715,13 @@ impl<'hir> LoweringContext<'_, 'hir> { } } - fn lower_variant(&mut self, v: &Variant) -> hir::Variant<'hir> { + fn lower_variant(&mut self, item_kind: &ItemKind, v: &Variant) -> hir::Variant<'hir> { let hir_id = self.lower_node_id(v.id); self.lower_attrs(hir_id, &v.attrs, v.span); hir::Variant { hir_id, def_id: self.local_def_id(v.id), - data: self.lower_variant_data(hir_id, &v.data), + data: self.lower_variant_data(hir_id, item_kind, &v.data), disr_expr: v.disr_expr.as_ref().map(|e| self.lower_anon_const_to_anon_const(e)), ident: self.lower_ident(v.ident), span: self.lower_span(v.span), @@ -730,15 +731,36 @@ impl<'hir> LoweringContext<'_, 'hir> { fn lower_variant_data( &mut self, parent_id: hir::HirId, + item_kind: &ItemKind, vdata: &VariantData, ) -> hir::VariantData<'hir> { match vdata { - VariantData::Struct { fields, recovered } => hir::VariantData::Struct { - fields: self + VariantData::Struct { fields, recovered } => { + let fields = self .arena - .alloc_from_iter(fields.iter().enumerate().map(|f| self.lower_field_def(f))), - recovered: *recovered, - }, + .alloc_from_iter(fields.iter().enumerate().map(|f| self.lower_field_def(f))); + + if let ItemKind::Union(..) = item_kind { + for field in &fields[..] { + if let Some(default) = field.default { + // Unions cannot derive `Default`, and it's not clear how to use default + // field values of unions if that was supported. Therefore, blanket reject + // trying to use field values with unions. + if self.tcx.features().default_field_values() { + self.dcx().emit_err(UnionWithDefault { span: default.span }); + } else { + let _ = self.dcx().span_delayed_bug( + default.span, + "expected union default field values feature gate error but none \ + was produced", + ); + } + } + } + } + + hir::VariantData::Struct { fields, recovered: *recovered } + } VariantData::Tuple(fields, id) => { let ctor_id = self.lower_node_id(*id); self.alias_attrs(ctor_id, parent_id); diff --git a/tests/ui/feature-gates/feature-gate-default-field-values.rs b/tests/ui/feature-gates/feature-gate-default-field-values.rs index d2e41a7160259..4631f51b9d840 100644 --- a/tests/ui/feature-gates/feature-gate-default-field-values.rs +++ b/tests/ui/feature-gates/feature-gate-default-field-values.rs @@ -58,6 +58,16 @@ pub enum OptEnum { } } +// Default field values may not be used on `union`s (at least, this is not described in the accepted +// RFC, and it's not currently clear how to extend the design to do so). We emit a feature gate +// error when the feature is not enabled, but syntactically reject default field values when used +// with unions when the feature is enabled. This can be adjusted if there's an acceptable design +// extension, or just unconditionally reject always. +union U { + x: i32 = 0, //~ ERROR default values on fields are experimental + y: f32 = 0.0, //~ ERROR default values on fields are experimental +} + fn main () { let x = Foo { .. }; //~ ERROR base expression required after `..` let y = Foo::default(); diff --git a/tests/ui/feature-gates/feature-gate-default-field-values.stderr b/tests/ui/feature-gates/feature-gate-default-field-values.stderr index 104d72a39861d..292c38990726e 100644 --- a/tests/ui/feature-gates/feature-gate-default-field-values.stderr +++ b/tests/ui/feature-gates/feature-gate-default-field-values.stderr @@ -124,8 +124,28 @@ LL | optional: () = (), = help: add `#![feature(default_field_values)]` to the crate attributes to enable = note: this compiler was built on YYYY-MM-DD; consider upgrading it if it is out of date +error[E0658]: default values on fields are experimental + --> $DIR/feature-gate-default-field-values.rs:67:11 + | +LL | x: i32 = 0, + | ^^^^ + | + = note: see issue #132162 for more information + = help: add `#![feature(default_field_values)]` to the crate attributes to enable + = note: this compiler was built on YYYY-MM-DD; consider upgrading it if it is out of date + +error[E0658]: default values on fields are experimental + --> $DIR/feature-gate-default-field-values.rs:68:11 + | +LL | y: f32 = 0.0, + | ^^^^^^ + | + = note: see issue #132162 for more information + = help: add `#![feature(default_field_values)]` to the crate attributes to enable + = note: this compiler was built on YYYY-MM-DD; consider upgrading it if it is out of date + error[E0797]: base expression required after `..` - --> $DIR/feature-gate-default-field-values.rs:62:21 + --> $DIR/feature-gate-default-field-values.rs:72:21 | LL | let x = Foo { .. }; | ^ @@ -140,7 +160,7 @@ LL | let x = Foo { ../* expr */ }; | ++++++++++ error[E0797]: base expression required after `..` - --> $DIR/feature-gate-default-field-values.rs:64:29 + --> $DIR/feature-gate-default-field-values.rs:74:29 | LL | let z = Foo { baz: 1, .. }; | ^ @@ -155,7 +175,7 @@ LL | let z = Foo { baz: 1, ../* expr */ }; | ++++++++++ error[E0797]: base expression required after `..` - --> $DIR/feature-gate-default-field-values.rs:70:26 + --> $DIR/feature-gate-default-field-values.rs:80:26 | LL | let x = Bar::Foo { .. }; | ^ @@ -170,7 +190,7 @@ LL | let x = Bar::Foo { ../* expr */ }; | ++++++++++ error[E0797]: base expression required after `..` - --> $DIR/feature-gate-default-field-values.rs:72:34 + --> $DIR/feature-gate-default-field-values.rs:82:34 | LL | let z = Bar::Foo { baz: 1, .. }; | ^ @@ -185,7 +205,7 @@ LL | let z = Bar::Foo { baz: 1, ../* expr */ }; | ++++++++++ error[E0797]: base expression required after `..` - --> $DIR/feature-gate-default-field-values.rs:78:31 + --> $DIR/feature-gate-default-field-values.rs:88:31 | LL | let x = Qux:: { .. }; | ^ @@ -200,7 +220,7 @@ LL | let x = Qux:: { ../* expr */ }; | ++++++++++ error[E0797]: base expression required after `..` - --> $DIR/feature-gate-default-field-values.rs:79:73 + --> $DIR/feature-gate-default-field-values.rs:89:73 | LL | assert!(matches!(Qux:: { bar: S, baz: 42, bat: 2, bay: 4, .. }, x)); | ^ @@ -215,7 +235,7 @@ LL | assert!(matches!(Qux:: { bar: S, baz: 42, bat: 2, bay: 4, ../* | ++++++++++ error[E0797]: base expression required after `..` - --> $DIR/feature-gate-default-field-values.rs:82:38 + --> $DIR/feature-gate-default-field-values.rs:92:38 | LL | let y = Opt { mandatory: None, .. }; | ^ @@ -230,7 +250,7 @@ LL | let y = Opt { mandatory: None, ../* expr */ }; | ++++++++++ error[E0797]: base expression required after `..` - --> $DIR/feature-gate-default-field-values.rs:86:47 + --> $DIR/feature-gate-default-field-values.rs:96:47 | LL | assert!(matches!(Opt { mandatory: None, .. }, z)); | ^ @@ -245,7 +265,7 @@ LL | assert!(matches!(Opt { mandatory: None, ../* expr */ }, z)); | ++++++++++ error[E0797]: base expression required after `..` - --> $DIR/feature-gate-default-field-values.rs:88:30 + --> $DIR/feature-gate-default-field-values.rs:98:30 | LL | assert!(matches!(Opt { .. }, z)); | ^ @@ -256,7 +276,7 @@ LL | assert!(matches!(Opt { ../* expr */ }, z)); | ++++++++++ error[E0797]: base expression required after `..` - --> $DIR/feature-gate-default-field-values.rs:90:44 + --> $DIR/feature-gate-default-field-values.rs:100:44 | LL | assert!(matches!(Opt { optional: (), .. }, z)); | ^ @@ -267,7 +287,7 @@ LL | assert!(matches!(Opt { optional: (), ../* expr */ }, z)); | ++++++++++ error[E0797]: base expression required after `..` - --> $DIR/feature-gate-default-field-values.rs:92:61 + --> $DIR/feature-gate-default-field-values.rs:102:61 | LL | assert!(matches!(Opt { optional: (), mandatory: None, .. }, z)); | ^ @@ -279,7 +299,7 @@ LL + assert!(matches!(Opt { optional: (), mandatory: None, }, z)); | error[E0797]: base expression required after `..` - --> $DIR/feature-gate-default-field-values.rs:94:51 + --> $DIR/feature-gate-default-field-values.rs:104:51 | LL | let y = OptEnum::Variant { mandatory: None, .. }; | ^ @@ -294,7 +314,7 @@ LL | let y = OptEnum::Variant { mandatory: None, ../* expr */ }; | ++++++++++ error[E0797]: base expression required after `..` - --> $DIR/feature-gate-default-field-values.rs:98:60 + --> $DIR/feature-gate-default-field-values.rs:108:60 | LL | assert!(matches!(OptEnum::Variant { mandatory: None, .. }, z)); | ^ @@ -309,7 +329,7 @@ LL | assert!(matches!(OptEnum::Variant { mandatory: None, ../* expr */ }, z) | ++++++++++ error[E0797]: base expression required after `..` - --> $DIR/feature-gate-default-field-values.rs:100:43 + --> $DIR/feature-gate-default-field-values.rs:110:43 | LL | assert!(matches!(OptEnum::Variant { .. }, z)); | ^ @@ -320,7 +340,7 @@ LL | assert!(matches!(OptEnum::Variant { ../* expr */ }, z)); | ++++++++++ error[E0797]: base expression required after `..` - --> $DIR/feature-gate-default-field-values.rs:102:57 + --> $DIR/feature-gate-default-field-values.rs:112:57 | LL | assert!(matches!(OptEnum::Variant { optional: (), .. }, z)); | ^ @@ -331,7 +351,7 @@ LL | assert!(matches!(OptEnum::Variant { optional: (), ../* expr */ }, z)); | ++++++++++ error[E0797]: base expression required after `..` - --> $DIR/feature-gate-default-field-values.rs:104:74 + --> $DIR/feature-gate-default-field-values.rs:114:74 | LL | assert!(matches!(OptEnum::Variant { optional: (), mandatory: None, .. }, z)); | ^ @@ -342,7 +362,7 @@ LL - assert!(matches!(OptEnum::Variant { optional: (), mandatory: None, .. } LL + assert!(matches!(OptEnum::Variant { optional: (), mandatory: None, }, z)); | -error: aborting due to 29 previous errors +error: aborting due to 31 previous errors Some errors have detailed explanations: E0658, E0797. For more information about an error, try `rustc --explain E0658`. diff --git a/tests/ui/structs/default-field-values/failures.rs b/tests/ui/structs/default-field-values/failures.rs index 77640d4f461b7..9c5b7172929c6 100644 --- a/tests/ui/structs/default-field-values/failures.rs +++ b/tests/ui/structs/default-field-values/failures.rs @@ -51,8 +51,8 @@ enum E { union U { - x: i32 = 1, - y: f32 = 2., + x: i32 = 1, //~ ERROR unions cannot have default field values + y: f32 = 2., //~ ERROR unions cannot have default field values } fn main () { diff --git a/tests/ui/structs/default-field-values/failures.stderr b/tests/ui/structs/default-field-values/failures.stderr index 4f9aedff93e41..5e3d4c89c2a3c 100644 --- a/tests/ui/structs/default-field-values/failures.stderr +++ b/tests/ui/structs/default-field-values/failures.stderr @@ -12,6 +12,18 @@ error: default fields are not supported in tuple structs LL | pub struct Rak(i32 = 42); | ^^ default fields are only supported on structs +error: unions cannot have default field values + --> $DIR/failures.rs:54:14 + | +LL | x: i32 = 1, + | ^ + +error: unions cannot have default field values + --> $DIR/failures.rs:55:14 + | +LL | y: f32 = 2., + | ^^ + error[E0277]: the trait bound `S: Default` is not satisfied --> $DIR/failures.rs:16:5 | @@ -102,7 +114,7 @@ LL - let _ = Rak(.., 0); LL + let _ = Rak(0); | -error: aborting due to 8 previous errors +error: aborting due to 10 previous errors Some errors have detailed explanations: E0061, E0277, E0308. For more information about an error, try `rustc --explain E0061`.