From 99182dd8058f0a1153b8b7fcf873028caa6fbfa7 Mon Sep 17 00:00:00 2001 From: joboet Date: Thu, 6 Oct 2022 22:46:15 +0200 Subject: [PATCH 01/27] std: use semaphore for thread parking on Apple platforms --- .../std/src/sys/unix/thread_parker/darwin.rs | 131 ++++++++++++++++++ library/std/src/sys/unix/thread_parker/mod.rs | 10 +- 2 files changed, 140 insertions(+), 1 deletion(-) create mode 100644 library/std/src/sys/unix/thread_parker/darwin.rs diff --git a/library/std/src/sys/unix/thread_parker/darwin.rs b/library/std/src/sys/unix/thread_parker/darwin.rs new file mode 100644 index 0000000000000..510839d5dafbf --- /dev/null +++ b/library/std/src/sys/unix/thread_parker/darwin.rs @@ -0,0 +1,131 @@ +//! Thread parking for Darwin-based systems. +//! +//! Darwin actually has futex syscalls (`__ulock_wait`/`__ulock_wake`), but they +//! cannot be used in `std` because they are non-public (their use will lead to +//! rejection from the App Store) and because they are only available starting +//! with macOS version 10.12, even though the minimum target version is 10.7. +//! +//! Therefore, we need to look for other synchronization primitives. Luckily, Darwin +//! supports semaphores, which allow us to implement the behaviour we need with +//! only one primitive (as opposed to a mutex-condvar pair). We use the semaphore +//! provided by libdispatch, as the underlying Mach semaphore is only dubiously +//! public. + +use crate::pin::Pin; +use crate::sync::atomic::{ + AtomicI8, + Ordering::{Acquire, Release}, +}; +use crate::time::Duration; + +type dispatch_semaphore_t = *mut crate::ffi::c_void; +type dispatch_time_t = u64; + +const DISPATCH_TIME_NOW: dispatch_time_t = 0; +const DISPATCH_TIME_FOREVER: dispatch_time_t = !0; + +#[link(name = "System", kind = "dylib")] +extern "C" { + fn dispatch_time(when: dispatch_time_t, delta: i64) -> dispatch_time_t; + fn dispatch_semaphore_create(val: isize) -> dispatch_semaphore_t; + fn dispatch_semaphore_wait(dsema: dispatch_semaphore_t, timeout: dispatch_time_t) -> isize; + fn dispatch_semaphore_signal(dsema: dispatch_semaphore_t) -> isize; + fn dispatch_release(object: *mut crate::ffi::c_void); +} + +const EMPTY: i8 = 0; +const NOTIFIED: i8 = 1; +const PARKED: i8 = -1; + +pub struct Parker { + semaphore: dispatch_semaphore_t, + state: AtomicI8, +} + +unsafe impl Sync for Parker {} +unsafe impl Send for Parker {} + +impl Parker { + pub unsafe fn new(parker: *mut Parker) { + let semaphore = dispatch_semaphore_create(0); + assert!( + !semaphore.is_null(), + "failed to create dispatch semaphore for thread synchronization" + ); + parker.write(Parker { semaphore, state: AtomicI8::new(EMPTY) }) + } + + // Does not need `Pin`, but other implementation do. + pub unsafe fn park(self: Pin<&Self>) { + // The semaphore counter must be zero at this point, because unparking + // threads will not actually increase it until we signalled that we + // are waiting. + + // Change NOTIFIED to EMPTY and EMPTY to PARKED. + if self.state.fetch_sub(1, Acquire) == NOTIFIED { + return; + } + + // Another thread may increase the semaphore counter from this point on. + // If it is faster than us, we will decrement it again immediately below. + // If we are faster, we wait. + + // Ensure that the semaphore counter has actually been decremented, even + // if the call timed out for some reason. + while dispatch_semaphore_wait(self.semaphore, DISPATCH_TIME_FOREVER) != 0 {} + + // At this point, the semaphore counter is zero again. + + // We were definitely woken up, so we don't need to check the state. + // Still, we need to reset the state using a swap to observe the state + // change with acquire ordering. + self.state.swap(EMPTY, Acquire); + } + + // Does not need `Pin`, but other implementation do. + pub unsafe fn park_timeout(self: Pin<&Self>, dur: Duration) { + if self.state.fetch_sub(1, Acquire) == NOTIFIED { + return; + } + + let nanos = dur.as_nanos().try_into().unwrap_or(i64::MAX); + let timeout = dispatch_time(DISPATCH_TIME_NOW, nanos); + + let timeout = dispatch_semaphore_wait(self.semaphore, timeout) != 0; + + let state = self.state.swap(EMPTY, Acquire); + if state == NOTIFIED && timeout { + // If the state was NOTIFIED but semaphore_wait returned without + // decrementing the count because of a timeout, it means another + // thread is about to call semaphore_signal. We must wait for that + // to happen to ensure the semaphore count is reset. + while dispatch_semaphore_wait(self.semaphore, DISPATCH_TIME_FOREVER) != 0 {} + } else { + // Either a timeout occurred and we reset the state before any thread + // tried to wake us up, or we were woken up and reset the state, + // making sure to observe the state change with acquire ordering. + // Either way, the semaphore counter is now zero again. + } + } + + // Does not need `Pin`, but other implementation do. + pub fn unpark(self: Pin<&Self>) { + let state = self.state.swap(NOTIFIED, Release); + if state == PARKED { + unsafe { + dispatch_semaphore_signal(self.semaphore); + } + } + } +} + +impl Drop for Parker { + fn drop(&mut self) { + // SAFETY: + // We always ensure that the semaphore count is reset, so this will + // never cause an exception. + unsafe { + dispatch_release(self.semaphore); + } + } +} diff --git a/library/std/src/sys/unix/thread_parker/mod.rs b/library/std/src/sys/unix/thread_parker/mod.rs index e2453580dc72a..724ec2d482edb 100644 --- a/library/std/src/sys/unix/thread_parker/mod.rs +++ b/library/std/src/sys/unix/thread_parker/mod.rs @@ -11,7 +11,15 @@ )))] cfg_if::cfg_if! { - if #[cfg(target_os = "netbsd")] { + if #[cfg(any( + target_os = "macos", + target_os = "ios", + target_os = "watchos", + target_os = "tvos", + ))] { + mod darwin; + pub use darwin::Parker; + } else if #[cfg(target_os = "netbsd")] { mod netbsd; pub use netbsd::Parker; } else { From 0ad4dd494a67a0fffc5a3e4df08f0c26cf074c59 Mon Sep 17 00:00:00 2001 From: joboet Date: Thu, 6 Oct 2022 22:46:47 +0200 Subject: [PATCH 02/27] std: add thread parking tests --- library/std/src/thread/tests.rs | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/library/std/src/thread/tests.rs b/library/std/src/thread/tests.rs index 130e47c8d44f0..dfb8765ab4eed 100644 --- a/library/std/src/thread/tests.rs +++ b/library/std/src/thread/tests.rs @@ -244,6 +244,28 @@ fn test_try_panic_any_message_unit_struct() { } } +#[test] +fn test_park_unpark_before() { + for _ in 0..10 { + thread::current().unpark(); + thread::park(); + } +} + +#[test] +fn test_park_unpark_called_other_thread() { + for _ in 0..10 { + let th = thread::current(); + + let _guard = thread::spawn(move || { + super::sleep(Duration::from_millis(50)); + th.unpark(); + }); + + thread::park(); + } +} + #[test] fn test_park_timeout_unpark_before() { for _ in 0..10 { From b0b072d7477a2d10ac7bee77b84ec33151f8eb65 Mon Sep 17 00:00:00 2001 From: Jhonny Bill Mena Date: Tue, 4 Oct 2022 13:23:53 -0400 Subject: [PATCH 03/27] ADD - codegen_ssa initial diags translations machinery ADD - migrate MissingNativeStaticLibrary fatal error --- compiler/rustc_codegen_ssa/src/errors.rs | 9 +++++++++ compiler/rustc_codegen_ssa/src/lib.rs | 1 + .../rustc_error_messages/locales/en-US/codegen_ssa.ftl | 1 + compiler/rustc_error_messages/src/lib.rs | 1 + 4 files changed, 12 insertions(+) create mode 100644 compiler/rustc_codegen_ssa/src/errors.rs create mode 100644 compiler/rustc_error_messages/locales/en-US/codegen_ssa.ftl diff --git a/compiler/rustc_codegen_ssa/src/errors.rs b/compiler/rustc_codegen_ssa/src/errors.rs new file mode 100644 index 0000000000000..170b7983dbe5f --- /dev/null +++ b/compiler/rustc_codegen_ssa/src/errors.rs @@ -0,0 +1,9 @@ +//! Errors emitted by codegen_ssa + +use rustc_macros::SessionDiagnostic; + +#[derive(SessionDiagnostic)] +#[diag(codegen_ssa::missing_native_static_library)] +pub struct MissingNativeStaticLibrary<'a> { + pub library_name: &'a str, +} diff --git a/compiler/rustc_codegen_ssa/src/lib.rs b/compiler/rustc_codegen_ssa/src/lib.rs index 3ef9a634e1857..e62b6f342b2e9 100644 --- a/compiler/rustc_codegen_ssa/src/lib.rs +++ b/compiler/rustc_codegen_ssa/src/lib.rs @@ -43,6 +43,7 @@ pub mod base; pub mod common; pub mod coverageinfo; pub mod debuginfo; +pub mod errors; pub mod glue; pub mod meth; pub mod mir; diff --git a/compiler/rustc_error_messages/locales/en-US/codegen_ssa.ftl b/compiler/rustc_error_messages/locales/en-US/codegen_ssa.ftl new file mode 100644 index 0000000000000..deba0dc6f9954 --- /dev/null +++ b/compiler/rustc_error_messages/locales/en-US/codegen_ssa.ftl @@ -0,0 +1 @@ +codegen_ssa_missing_native_static_library = could not find native static library `{$library_name}`, perhaps an -L flag is missing? diff --git a/compiler/rustc_error_messages/src/lib.rs b/compiler/rustc_error_messages/src/lib.rs index 18be60975e4eb..10f8efdffa8b6 100644 --- a/compiler/rustc_error_messages/src/lib.rs +++ b/compiler/rustc_error_messages/src/lib.rs @@ -63,6 +63,7 @@ fluent_messages! { symbol_mangling => "../locales/en-US/symbol_mangling.ftl", trait_selection => "../locales/en-US/trait_selection.ftl", ty_utils => "../locales/en-US/ty_utils.ftl", + codegen_ssa => "../locales/en-US/codegen_ssa.ftl", } pub use fluent_generated::{self as fluent, DEFAULT_LOCALE_RESOURCES}; From 4e0de5319c3543b7960394f612a0527923522ae1 Mon Sep 17 00:00:00 2001 From: Jhonny Bill Mena Date: Wed, 24 Aug 2022 23:40:07 -0400 Subject: [PATCH 04/27] ADD - migrate lib.def write fatal error MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This diagnostic has no UI test 🤔 Should we add some? If so, how? --- compiler/rustc_codegen_ssa/src/back/linker.rs | 7 ++++--- compiler/rustc_codegen_ssa/src/errors.rs | 6 ++++++ .../rustc_error_messages/locales/en-US/codegen_ssa.ftl | 2 ++ 3 files changed, 12 insertions(+), 3 deletions(-) diff --git a/compiler/rustc_codegen_ssa/src/back/linker.rs b/compiler/rustc_codegen_ssa/src/back/linker.rs index c71d332475a5f..60547acc95661 100644 --- a/compiler/rustc_codegen_ssa/src/back/linker.rs +++ b/compiler/rustc_codegen_ssa/src/back/linker.rs @@ -1,5 +1,6 @@ use super::command::Command; use super::symbol_export; +use crate::errors::LibDefWriteFailure; use rustc_span::symbol::sym; use std::ffi::{OsStr, OsString}; @@ -666,7 +667,7 @@ impl<'a> Linker for GccLinker<'a> { } }; if let Err(e) = res { - self.sess.fatal(&format!("failed to write lib.def file: {}", e)); + self.sess.emit_fatal(LibDefWriteFailure { error_description: format!("{e}") }); } } else if is_windows { let res: io::Result<()> = try { @@ -681,7 +682,7 @@ impl<'a> Linker for GccLinker<'a> { } }; if let Err(e) = res { - self.sess.fatal(&format!("failed to write list.def file: {}", e)); + self.sess.emit_fatal(LibDefWriteFailure { error_description: format!("{e}") }); } } else { // Write an LD version script @@ -972,7 +973,7 @@ impl<'a> Linker for MsvcLinker<'a> { } }; if let Err(e) = res { - self.sess.fatal(&format!("failed to write lib.def file: {}", e)); + self.sess.emit_fatal(LibDefWriteFailure { error_description: format!("{e}") }); } let mut arg = OsString::from("/DEF:"); arg.push(path); diff --git a/compiler/rustc_codegen_ssa/src/errors.rs b/compiler/rustc_codegen_ssa/src/errors.rs index 170b7983dbe5f..718ad5c7bf566 100644 --- a/compiler/rustc_codegen_ssa/src/errors.rs +++ b/compiler/rustc_codegen_ssa/src/errors.rs @@ -7,3 +7,9 @@ use rustc_macros::SessionDiagnostic; pub struct MissingNativeStaticLibrary<'a> { pub library_name: &'a str, } + +#[derive(SessionDiagnostic)] +#[diag(codegen_ssa::lib_def_write_failure)] +pub struct LibDefWriteFailure { + pub error_description: String, +} diff --git a/compiler/rustc_error_messages/locales/en-US/codegen_ssa.ftl b/compiler/rustc_error_messages/locales/en-US/codegen_ssa.ftl index deba0dc6f9954..597f3488fc1be 100644 --- a/compiler/rustc_error_messages/locales/en-US/codegen_ssa.ftl +++ b/compiler/rustc_error_messages/locales/en-US/codegen_ssa.ftl @@ -1 +1,3 @@ codegen_ssa_missing_native_static_library = could not find native static library `{$library_name}`, perhaps an -L flag is missing? + +codegen_ssa_lib_def_write_failure = failed to write lib.def file: {$error_description} From 0a2d7f83cb3aea0556ef7cfd7662a028ac974d87 Mon Sep 17 00:00:00 2001 From: Jhonny Bill Mena Date: Fri, 26 Aug 2022 20:21:55 -0400 Subject: [PATCH 05/27] UPDATE - LibDefWriteFailure to accept type instead of formatted string MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This follows team’s suggestions in this thread https://rust-lang.zulipchat.com/#narrow/stream/336883-i18n/topic/.23100717.20diag.20translation/near/295305249 --- compiler/rustc_codegen_ssa/src/back/linker.rs | 12 ++++++------ compiler/rustc_codegen_ssa/src/errors.rs | 3 ++- .../locales/en-US/codegen_ssa.ftl | 2 +- 3 files changed, 9 insertions(+), 8 deletions(-) diff --git a/compiler/rustc_codegen_ssa/src/back/linker.rs b/compiler/rustc_codegen_ssa/src/back/linker.rs index 60547acc95661..e9335e53125f3 100644 --- a/compiler/rustc_codegen_ssa/src/back/linker.rs +++ b/compiler/rustc_codegen_ssa/src/back/linker.rs @@ -666,8 +666,8 @@ impl<'a> Linker for GccLinker<'a> { writeln!(f, "_{}", sym)?; } }; - if let Err(e) = res { - self.sess.emit_fatal(LibDefWriteFailure { error_description: format!("{e}") }); + if let Err(error) = res { + self.sess.emit_fatal(LibDefWriteFailure { error }); } } else if is_windows { let res: io::Result<()> = try { @@ -681,8 +681,8 @@ impl<'a> Linker for GccLinker<'a> { writeln!(f, " {}", symbol)?; } }; - if let Err(e) = res { - self.sess.emit_fatal(LibDefWriteFailure { error_description: format!("{e}") }); + if let Err(error) = res { + self.sess.emit_fatal(LibDefWriteFailure { error }); } } else { // Write an LD version script @@ -972,8 +972,8 @@ impl<'a> Linker for MsvcLinker<'a> { writeln!(f, " {}", symbol)?; } }; - if let Err(e) = res { - self.sess.emit_fatal(LibDefWriteFailure { error_description: format!("{e}") }); + if let Err(error) = res { + self.sess.emit_fatal(LibDefWriteFailure { error }); } let mut arg = OsString::from("/DEF:"); arg.push(path); diff --git a/compiler/rustc_codegen_ssa/src/errors.rs b/compiler/rustc_codegen_ssa/src/errors.rs index 718ad5c7bf566..13d9c1a7b6b8d 100644 --- a/compiler/rustc_codegen_ssa/src/errors.rs +++ b/compiler/rustc_codegen_ssa/src/errors.rs @@ -1,6 +1,7 @@ //! Errors emitted by codegen_ssa use rustc_macros::SessionDiagnostic; +use std::io::Error; #[derive(SessionDiagnostic)] #[diag(codegen_ssa::missing_native_static_library)] @@ -11,5 +12,5 @@ pub struct MissingNativeStaticLibrary<'a> { #[derive(SessionDiagnostic)] #[diag(codegen_ssa::lib_def_write_failure)] pub struct LibDefWriteFailure { - pub error_description: String, + pub error: Error, } diff --git a/compiler/rustc_error_messages/locales/en-US/codegen_ssa.ftl b/compiler/rustc_error_messages/locales/en-US/codegen_ssa.ftl index 597f3488fc1be..d84a774710ac1 100644 --- a/compiler/rustc_error_messages/locales/en-US/codegen_ssa.ftl +++ b/compiler/rustc_error_messages/locales/en-US/codegen_ssa.ftl @@ -1,3 +1,3 @@ codegen_ssa_missing_native_static_library = could not find native static library `{$library_name}`, perhaps an -L flag is missing? -codegen_ssa_lib_def_write_failure = failed to write lib.def file: {$error_description} +codegen_ssa_lib_def_write_failure = failed to write lib.def file: {$error} From 086e70f13e9259d7949fbfeec6fa824c6327f42d Mon Sep 17 00:00:00 2001 From: Jhonny Bill Mena Date: Fri, 7 Oct 2022 10:03:45 -0400 Subject: [PATCH 06/27] UPDATE - migrate linker.rs to new diagnostics infra --- compiler/rustc_codegen_ssa/src/back/linker.rs | 35 ++++++++-------- compiler/rustc_codegen_ssa/src/errors.rs | 42 +++++++++++++++++++ .../locales/en-US/codegen_ssa.ftl | 18 ++++++++ 3 files changed, 77 insertions(+), 18 deletions(-) diff --git a/compiler/rustc_codegen_ssa/src/back/linker.rs b/compiler/rustc_codegen_ssa/src/back/linker.rs index e9335e53125f3..debcffcd326f6 100644 --- a/compiler/rustc_codegen_ssa/src/back/linker.rs +++ b/compiler/rustc_codegen_ssa/src/back/linker.rs @@ -1,6 +1,6 @@ use super::command::Command; use super::symbol_export; -use crate::errors::LibDefWriteFailure; +use crate::errors; use rustc_span::symbol::sym; use std::ffi::{OsStr, OsString}; @@ -91,13 +91,13 @@ pub fn get_linker<'a>( arg.push(format!("{}\\lib\\{}\\store", root_lib_path.display(), a)); cmd.arg(&arg); } else { - warn!("arch is not supported"); + sess.emit_warning(errors::UnsupportedArch); } } else { - warn!("MSVC root path lib location not found"); + sess.emit_warning(errors::MsvcPathNotFound); } } else { - warn!("link.exe not found"); + sess.emit_warning(errors::LinkExeNotFound); } } @@ -435,11 +435,11 @@ impl<'a> Linker for GccLinker<'a> { // FIXME(81490): ld64 doesn't support these flags but macOS 11 // has -needed-l{} / -needed_library {} // but we have no way to detect that here. - self.sess.warn("`as-needed` modifier not implemented yet for ld64"); + self.sess.emit_warning(errors::Ld64UnimplementedModifier); } else if self.is_gnu && !self.sess.target.is_like_windows { self.linker_arg("--no-as-needed"); } else { - self.sess.warn("`as-needed` modifier not supported for current linker"); + self.sess.emit_warning(errors::LinkerUnsupportedModifier); } } self.hint_dynamic(); @@ -493,7 +493,7 @@ impl<'a> Linker for GccLinker<'a> { // FIXME(81490): ld64 as of macOS 11 supports the -needed_framework // flag but we have no way to detect that here. // self.cmd.arg("-needed_framework").arg(framework); - self.sess.warn("`as-needed` modifier not implemented yet for ld64"); + self.sess.emit_warning(errors::Ld64UnimplementedModifier); } self.cmd.arg("-framework").arg(framework); } @@ -667,7 +667,7 @@ impl<'a> Linker for GccLinker<'a> { } }; if let Err(error) = res { - self.sess.emit_fatal(LibDefWriteFailure { error }); + self.sess.emit_fatal(errors::LibDefWriteFailure { error }); } } else if is_windows { let res: io::Result<()> = try { @@ -682,7 +682,7 @@ impl<'a> Linker for GccLinker<'a> { } }; if let Err(error) = res { - self.sess.emit_fatal(LibDefWriteFailure { error }); + self.sess.emit_fatal(errors::LibDefWriteFailure { error }); } } else { // Write an LD version script @@ -698,8 +698,8 @@ impl<'a> Linker for GccLinker<'a> { } writeln!(f, "\n local:\n *;\n}};")?; }; - if let Err(e) = res { - self.sess.fatal(&format!("failed to write version script: {}", e)); + if let Err(error) = res { + self.sess.emit_fatal(errors::VersionScriptWriteFailure { error }); } } @@ -916,9 +916,8 @@ impl<'a> Linker for MsvcLinker<'a> { self.cmd.arg(arg); } } - Err(err) => { - self.sess - .warn(&format!("error enumerating natvis directory: {}", err)); + Err(error) => { + self.sess.emit_warning(errors::NoNatvisDirectory { error }); } } } @@ -973,7 +972,7 @@ impl<'a> Linker for MsvcLinker<'a> { } }; if let Err(error) = res { - self.sess.emit_fatal(LibDefWriteFailure { error }); + self.sess.emit_fatal(errors::LibDefWriteFailure { error }); } let mut arg = OsString::from("/DEF:"); arg.push(path); @@ -1436,7 +1435,7 @@ impl<'a> Linker for L4Bender<'a> { fn export_symbols(&mut self, _: &Path, _: CrateType, _: &[String]) { // ToDo, not implemented, copy from GCC - self.sess.warn("exporting symbols not implemented yet for L4Bender"); + self.sess.emit_warning(errors::L4BenderExportingSymbolsUnimplemented); return; } @@ -1728,8 +1727,8 @@ impl<'a> Linker for BpfLinker<'a> { writeln!(f, "{}", sym)?; } }; - if let Err(e) = res { - self.sess.fatal(&format!("failed to write symbols file: {}", e)); + if let Err(error) = res { + self.sess.emit_fatal(errors::SymbolFileWriteFailure { error }); } else { self.cmd.arg("--export-symbols").arg(&path); } diff --git a/compiler/rustc_codegen_ssa/src/errors.rs b/compiler/rustc_codegen_ssa/src/errors.rs index 13d9c1a7b6b8d..05d89b32618e7 100644 --- a/compiler/rustc_codegen_ssa/src/errors.rs +++ b/compiler/rustc_codegen_ssa/src/errors.rs @@ -14,3 +14,45 @@ pub struct MissingNativeStaticLibrary<'a> { pub struct LibDefWriteFailure { pub error: Error, } + +#[derive(SessionDiagnostic)] +#[diag(codegen_ssa::version_script_write_failure)] +pub struct VersionScriptWriteFailure { + pub error: Error, +} + +#[derive(SessionDiagnostic)] +#[diag(codegen_ssa::symbol_file_write_failure)] +pub struct SymbolFileWriteFailure { + pub error: Error, +} + +#[derive(SessionDiagnostic)] +#[diag(codegen_ssa::unsupported_arch)] +pub struct UnsupportedArch; + +#[derive(SessionDiagnostic)] +#[diag(codegen_ssa::msvc_path_not_found)] +pub struct MsvcPathNotFound; + +#[derive(SessionDiagnostic)] +#[diag(codegen_ssa::link_exe_not_found)] +pub struct LinkExeNotFound; + +#[derive(SessionDiagnostic)] +#[diag(codegen_ssa::ld64_unimplemented_modifier)] +pub struct Ld64UnimplementedModifier; + +#[derive(SessionDiagnostic)] +#[diag(codegen_ssa::linker_unsupported_modifier)] +pub struct LinkerUnsupportedModifier; + +#[derive(SessionDiagnostic)] +#[diag(codegen_ssa::L4Bender_exporting_symbols_unimplemented)] +pub struct L4BenderExportingSymbolsUnimplemented; + +#[derive(SessionDiagnostic)] +#[diag(codegen_ssa::no_natvis_directory)] +pub struct NoNatvisDirectory { + pub error: Error, +} diff --git a/compiler/rustc_error_messages/locales/en-US/codegen_ssa.ftl b/compiler/rustc_error_messages/locales/en-US/codegen_ssa.ftl index d84a774710ac1..090a4dc9510b6 100644 --- a/compiler/rustc_error_messages/locales/en-US/codegen_ssa.ftl +++ b/compiler/rustc_error_messages/locales/en-US/codegen_ssa.ftl @@ -1,3 +1,21 @@ codegen_ssa_missing_native_static_library = could not find native static library `{$library_name}`, perhaps an -L flag is missing? codegen_ssa_lib_def_write_failure = failed to write lib.def file: {$error} + +codegen_ssa_version_script_write_failure = failed to write version script: {$error} + +codegen_ssa_symbol_file_write_failure = failed to write symbols file: {$error} + +codegen_ssa_unsupported_arch = arch is not supported + +codegen_ssa_msvc_path_not_found = MSVC root path lib location not found + +codegen_ssa_link_exe_not_found = link.exe not found + +codegen_ssa_ld64_unimplemented_modifier = `as-needed` modifier not implemented yet for ld64 + +codegen_ssa_linker_unsupported_modifier = `as-needed` modifier not supported for current linker + +codegen_ssa_L4Bender_exporting_symbols_unimplemented = exporting symbols not implemented yet for L4Bender + +codegen_ssa_no_natvis_directory = error enumerating natvis directory: {$error} From d9197dbbcd35c69a210806cea18d2ce0f3691839 Mon Sep 17 00:00:00 2001 From: Jhonny Bill Mena Date: Sun, 28 Aug 2022 19:58:12 -0400 Subject: [PATCH 07/27] UPDATE - migrate write.rs to new diagnostics infra --- compiler/rustc_codegen_ssa/src/back/write.rs | 41 ++++++++-------- compiler/rustc_codegen_ssa/src/errors.rs | 48 +++++++++++++++++++ .../locales/en-US/codegen_ssa.ftl | 8 ++++ 3 files changed, 75 insertions(+), 22 deletions(-) diff --git a/compiler/rustc_codegen_ssa/src/back/write.rs b/compiler/rustc_codegen_ssa/src/back/write.rs index 6188094bbbdd4..125b231b27486 100644 --- a/compiler/rustc_codegen_ssa/src/back/write.rs +++ b/compiler/rustc_codegen_ssa/src/back/write.rs @@ -2,11 +2,11 @@ use super::link::{self, ensure_removed}; use super::lto::{self, SerializedModule}; use super::symbol_export::symbol_name_for_instance_in_crate; +use crate::errors; +use crate::traits::*; use crate::{ CachedModuleCodegen, CodegenResults, CompiledModule, CrateInfo, ModuleCodegen, ModuleKind, }; - -use crate::traits::*; use jobserver::{Acquired, Client}; use rustc_data_structures::fx::FxHashMap; use rustc_data_structures::memmap::Mmap; @@ -530,7 +530,7 @@ fn produce_final_output_artifacts( // Produce final compile outputs. let copy_gracefully = |from: &Path, to: &Path| { if let Err(e) = fs::copy(from, to) { - sess.err(&format!("could not copy {:?} to {:?}: {}", from, to, e)); + sess.emit_err(errors::CopyPath::new(from, to, e)); } }; @@ -546,7 +546,7 @@ fn produce_final_output_artifacts( ensure_removed(sess.diagnostic(), &path); } } else { - let ext = crate_output + let extension = crate_output .temp_path(output_type, None) .extension() .unwrap() @@ -557,19 +557,11 @@ fn produce_final_output_artifacts( if crate_output.outputs.contains_key(&output_type) { // 2) Multiple codegen units, with `--emit foo=some_name`. We have // no good solution for this case, so warn the user. - sess.warn(&format!( - "ignoring emit path because multiple .{} files \ - were produced", - ext - )); + sess.emit_warning(errors::IgnoringEmitPath { extension }); } else if crate_output.single_output_file.is_some() { // 3) Multiple codegen units, with `-o some_name`. We have // no good solution for this case, so warn the user. - sess.warn(&format!( - "ignoring -o because multiple .{} files \ - were produced", - ext - )); + sess.emit_warning(errors::IgnoringOutput { extension }); } else { // 4) Multiple codegen units, but no explicit name. We // just leave the `foo.0.x` files in place. @@ -880,14 +872,19 @@ fn execute_copy_from_cache_work_item( ); match link_or_copy(&source_file, &output_path) { Ok(_) => Some(output_path), - Err(err) => { - let diag_handler = cgcx.create_diag_handler(); - diag_handler.err(&format!( - "unable to copy {} to {}: {}", - source_file.display(), - output_path.display(), - err - )); + Err(_) => { + // FIXME: + // Should we add Translations support in Handler, or should we pass a session here ? + // + // As Luis Cardoso mentioned here https://github.com/rust-lang/rust/pull/100753#discussion_r952975345, + // Translations support in Handler is tricky because SessionDiagnostic is not a trait, + // and we can't implement it in Handler because rustc_errors cannot depend on rustc_session. + // + // As for passing a session here, my understanding is that all these errors should be reported via + // the Shared Handler, which leads us to probably having to support Translations in another way. + + // let diag_handler = cgcx.create_diag_handler(); + // diag_handler.emit_err(errors::CopyPathBuf { source_file, output_path, error }); None } } diff --git a/compiler/rustc_codegen_ssa/src/errors.rs b/compiler/rustc_codegen_ssa/src/errors.rs index 05d89b32618e7..2ae5e659d2dac 100644 --- a/compiler/rustc_codegen_ssa/src/errors.rs +++ b/compiler/rustc_codegen_ssa/src/errors.rs @@ -1,7 +1,10 @@ //! Errors emitted by codegen_ssa +use rustc_errors::{DiagnosticArgValue, IntoDiagnosticArg}; use rustc_macros::SessionDiagnostic; +use std::borrow::Cow; use std::io::Error; +use std::path::{Path, PathBuf}; #[derive(SessionDiagnostic)] #[diag(codegen_ssa::missing_native_static_library)] @@ -56,3 +59,48 @@ pub struct L4BenderExportingSymbolsUnimplemented; pub struct NoNatvisDirectory { pub error: Error, } + +#[derive(SessionDiagnostic)] +#[diag(codegen_ssa::copy_path_buf)] +pub struct CopyPathBuf { + pub source_file: PathBuf, + pub output_path: PathBuf, + pub error: Error, +} + +// Reports Paths using `Debug` implementation rather than Path's `Display` implementation. +#[derive(SessionDiagnostic)] +#[diag(codegen_ssa::copy_path)] +pub struct CopyPath<'a> { + from: DebugArgPath<'a>, + to: DebugArgPath<'a>, + error: Error, +} + +impl<'a> CopyPath<'a> { + pub fn new(from: &'a Path, to: &'a Path, error: Error) -> CopyPath<'a> { + CopyPath { from: DebugArgPath { path: from }, to: DebugArgPath { path: to }, error } + } +} + +struct DebugArgPath<'a> { + pub path: &'a Path, +} + +impl IntoDiagnosticArg for DebugArgPath<'_> { + fn into_diagnostic_arg(self) -> rustc_errors::DiagnosticArgValue<'static> { + DiagnosticArgValue::Str(Cow::Owned(format!("{:?}", self.path))) + } +} + +#[derive(SessionDiagnostic)] +#[diag(codegen_ssa::ignoring_emit_path)] +pub struct IgnoringEmitPath { + pub extension: String, +} + +#[derive(SessionDiagnostic)] +#[diag(codegen_ssa::ignoring_output)] +pub struct IgnoringOutput { + pub extension: String, +} diff --git a/compiler/rustc_error_messages/locales/en-US/codegen_ssa.ftl b/compiler/rustc_error_messages/locales/en-US/codegen_ssa.ftl index 090a4dc9510b6..0d021edc4f76c 100644 --- a/compiler/rustc_error_messages/locales/en-US/codegen_ssa.ftl +++ b/compiler/rustc_error_messages/locales/en-US/codegen_ssa.ftl @@ -19,3 +19,11 @@ codegen_ssa_linker_unsupported_modifier = `as-needed` modifier not supported for codegen_ssa_L4Bender_exporting_symbols_unimplemented = exporting symbols not implemented yet for L4Bender codegen_ssa_no_natvis_directory = error enumerating natvis directory: {$error} + +codegen_ssa_copy_path = could not copy {$from} to {$to}: {$error} + +codegen_ssa_copy_path_buf = unable to copy {$source_file} to {$output_path}: {$error} + +codegen_ssa_ignoring_emit_path = ignoring emit path because multiple .{$extension} files were produced + +codegen_ssa_ignoring_output = ignoring -o because multiple .{$extension} files were produced From 67eb01c3f3c9ce8237bb09ecb4a77b3030af025b Mon Sep 17 00:00:00 2001 From: Jhonny Bill Mena Date: Sat, 10 Sep 2022 13:16:37 -0400 Subject: [PATCH 08/27] UPDATE - codege-ssa errors to new Diagnostic macro name --- compiler/rustc_codegen_ssa/src/errors.rs | 32 ++++++++++++------------ 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/compiler/rustc_codegen_ssa/src/errors.rs b/compiler/rustc_codegen_ssa/src/errors.rs index 2ae5e659d2dac..8c09aa96c8ef2 100644 --- a/compiler/rustc_codegen_ssa/src/errors.rs +++ b/compiler/rustc_codegen_ssa/src/errors.rs @@ -1,66 +1,66 @@ //! Errors emitted by codegen_ssa use rustc_errors::{DiagnosticArgValue, IntoDiagnosticArg}; -use rustc_macros::SessionDiagnostic; +use rustc_macros::Diagnostic; use std::borrow::Cow; use std::io::Error; use std::path::{Path, PathBuf}; -#[derive(SessionDiagnostic)] +#[derive(Diagnostic)] #[diag(codegen_ssa::missing_native_static_library)] pub struct MissingNativeStaticLibrary<'a> { pub library_name: &'a str, } -#[derive(SessionDiagnostic)] +#[derive(Diagnostic)] #[diag(codegen_ssa::lib_def_write_failure)] pub struct LibDefWriteFailure { pub error: Error, } -#[derive(SessionDiagnostic)] +#[derive(Diagnostic)] #[diag(codegen_ssa::version_script_write_failure)] pub struct VersionScriptWriteFailure { pub error: Error, } -#[derive(SessionDiagnostic)] +#[derive(Diagnostic)] #[diag(codegen_ssa::symbol_file_write_failure)] pub struct SymbolFileWriteFailure { pub error: Error, } -#[derive(SessionDiagnostic)] +#[derive(Diagnostic)] #[diag(codegen_ssa::unsupported_arch)] pub struct UnsupportedArch; -#[derive(SessionDiagnostic)] +#[derive(Diagnostic)] #[diag(codegen_ssa::msvc_path_not_found)] pub struct MsvcPathNotFound; -#[derive(SessionDiagnostic)] +#[derive(Diagnostic)] #[diag(codegen_ssa::link_exe_not_found)] pub struct LinkExeNotFound; -#[derive(SessionDiagnostic)] +#[derive(Diagnostic)] #[diag(codegen_ssa::ld64_unimplemented_modifier)] pub struct Ld64UnimplementedModifier; -#[derive(SessionDiagnostic)] +#[derive(Diagnostic)] #[diag(codegen_ssa::linker_unsupported_modifier)] pub struct LinkerUnsupportedModifier; -#[derive(SessionDiagnostic)] +#[derive(Diagnostic)] #[diag(codegen_ssa::L4Bender_exporting_symbols_unimplemented)] pub struct L4BenderExportingSymbolsUnimplemented; -#[derive(SessionDiagnostic)] +#[derive(Diagnostic)] #[diag(codegen_ssa::no_natvis_directory)] pub struct NoNatvisDirectory { pub error: Error, } -#[derive(SessionDiagnostic)] +#[derive(Diagnostic)] #[diag(codegen_ssa::copy_path_buf)] pub struct CopyPathBuf { pub source_file: PathBuf, @@ -69,7 +69,7 @@ pub struct CopyPathBuf { } // Reports Paths using `Debug` implementation rather than Path's `Display` implementation. -#[derive(SessionDiagnostic)] +#[derive(Diagnostic)] #[diag(codegen_ssa::copy_path)] pub struct CopyPath<'a> { from: DebugArgPath<'a>, @@ -93,13 +93,13 @@ impl IntoDiagnosticArg for DebugArgPath<'_> { } } -#[derive(SessionDiagnostic)] +#[derive(Diagnostic)] #[diag(codegen_ssa::ignoring_emit_path)] pub struct IgnoringEmitPath { pub extension: String, } -#[derive(SessionDiagnostic)] +#[derive(Diagnostic)] #[diag(codegen_ssa::ignoring_output)] pub struct IgnoringOutput { pub extension: String, From 7548d952af07e7df18bb894b3f470d321f4d6c04 Mon Sep 17 00:00:00 2001 From: Jhonny Bill Mena Date: Sat, 10 Sep 2022 13:24:46 -0400 Subject: [PATCH 09/27] UPDATE - resolve fixme and emit errors via Handler --- compiler/rustc_codegen_ssa/src/back/write.rs | 19 ++++++------------- 1 file changed, 6 insertions(+), 13 deletions(-) diff --git a/compiler/rustc_codegen_ssa/src/back/write.rs b/compiler/rustc_codegen_ssa/src/back/write.rs index 125b231b27486..1f577e9f3524f 100644 --- a/compiler/rustc_codegen_ssa/src/back/write.rs +++ b/compiler/rustc_codegen_ssa/src/back/write.rs @@ -872,19 +872,12 @@ fn execute_copy_from_cache_work_item( ); match link_or_copy(&source_file, &output_path) { Ok(_) => Some(output_path), - Err(_) => { - // FIXME: - // Should we add Translations support in Handler, or should we pass a session here ? - // - // As Luis Cardoso mentioned here https://github.com/rust-lang/rust/pull/100753#discussion_r952975345, - // Translations support in Handler is tricky because SessionDiagnostic is not a trait, - // and we can't implement it in Handler because rustc_errors cannot depend on rustc_session. - // - // As for passing a session here, my understanding is that all these errors should be reported via - // the Shared Handler, which leads us to probably having to support Translations in another way. - - // let diag_handler = cgcx.create_diag_handler(); - // diag_handler.emit_err(errors::CopyPathBuf { source_file, output_path, error }); + Err(error) => { + cgcx.create_diag_handler().emit_err(errors::CopyPathBuf { + source_file, + output_path, + error, + }); None } } From 0f97d4a1417cdf58b7a39e351dcdf40cf58e48b9 Mon Sep 17 00:00:00 2001 From: Jhonny Bill Mena Date: Tue, 13 Sep 2022 08:46:47 -0400 Subject: [PATCH 10/27] DELETE - unused error after PR# 100101 was merged --- compiler/rustc_codegen_ssa/src/errors.rs | 6 ------ compiler/rustc_error_messages/locales/en-US/codegen_ssa.ftl | 2 -- 2 files changed, 8 deletions(-) diff --git a/compiler/rustc_codegen_ssa/src/errors.rs b/compiler/rustc_codegen_ssa/src/errors.rs index 8c09aa96c8ef2..cf98cb2468ab7 100644 --- a/compiler/rustc_codegen_ssa/src/errors.rs +++ b/compiler/rustc_codegen_ssa/src/errors.rs @@ -6,12 +6,6 @@ use std::borrow::Cow; use std::io::Error; use std::path::{Path, PathBuf}; -#[derive(Diagnostic)] -#[diag(codegen_ssa::missing_native_static_library)] -pub struct MissingNativeStaticLibrary<'a> { - pub library_name: &'a str, -} - #[derive(Diagnostic)] #[diag(codegen_ssa::lib_def_write_failure)] pub struct LibDefWriteFailure { diff --git a/compiler/rustc_error_messages/locales/en-US/codegen_ssa.ftl b/compiler/rustc_error_messages/locales/en-US/codegen_ssa.ftl index 0d021edc4f76c..4d43b2eb0b696 100644 --- a/compiler/rustc_error_messages/locales/en-US/codegen_ssa.ftl +++ b/compiler/rustc_error_messages/locales/en-US/codegen_ssa.ftl @@ -1,5 +1,3 @@ -codegen_ssa_missing_native_static_library = could not find native static library `{$library_name}`, perhaps an -L flag is missing? - codegen_ssa_lib_def_write_failure = failed to write lib.def file: {$error} codegen_ssa_version_script_write_failure = failed to write version script: {$error} From 12aa84bdf3231f563b7f86186dbece2023d1235a Mon Sep 17 00:00:00 2001 From: Jhonny Bill Mena Date: Sun, 2 Oct 2022 23:21:15 -0400 Subject: [PATCH 11/27] ADD - initial port of link.rs --- compiler/rustc_codegen_ssa/src/back/link.rs | 89 ++++++-------- compiler/rustc_codegen_ssa/src/errors.rs | 109 +++++++++++++++++- compiler/rustc_codegen_ssa/src/lib.rs | 1 + .../locales/en-US/codegen_ssa.ftl | 25 ++++ 4 files changed, 166 insertions(+), 58 deletions(-) diff --git a/compiler/rustc_codegen_ssa/src/back/link.rs b/compiler/rustc_codegen_ssa/src/back/link.rs index 9a0c379b4e44d..ac2a8f969df00 100644 --- a/compiler/rustc_codegen_ssa/src/back/link.rs +++ b/compiler/rustc_codegen_ssa/src/back/link.rs @@ -31,7 +31,9 @@ use super::command::Command; use super::linker::{self, Linker}; use super::metadata::{create_rmeta_file, MetadataPosition}; use super::rpath::{self, RPathConfig}; -use crate::{looks_like_rust_object_file, CodegenResults, CompiledModule, CrateInfo, NativeLib}; +use crate::{ + errors, looks_like_rust_object_file, CodegenResults, CompiledModule, CrateInfo, NativeLib, +}; use cc::windows_registry; use regex::Regex; @@ -93,7 +95,7 @@ pub fn link_binary<'a>( let tmpdir = TempFileBuilder::new() .prefix("rustc") .tempdir() - .unwrap_or_else(|err| sess.fatal(&format!("couldn't create a temp dir: {}", err))); + .unwrap_or_else(|error| sess.emit_fatal(errors::CreateTempDir { error })); let path = MaybeTempDir::new(tmpdir, sess.opts.cg.save_temps); let out_filename = out_filename( sess, @@ -208,7 +210,7 @@ pub fn link_binary<'a>( pub fn each_linked_rlib( info: &CrateInfo, f: &mut dyn FnMut(CrateNum, &Path), -) -> Result<(), String> { +) -> Result<(), errors::LinkRlibError> { let crates = info.used_crates.iter(); let mut fmts = None; for (ty, list) in info.dependency_formats.iter() { @@ -224,26 +226,23 @@ pub fn each_linked_rlib( } } let Some(fmts) = fmts else { - return Err("could not find formats for rlibs".to_string()); + return Err(errors::LinkRlibError::MissingFormat); }; for &cnum in crates { match fmts.get(cnum.as_usize() - 1) { Some(&Linkage::NotLinked | &Linkage::IncludedFromDylib) => continue, Some(_) => {} - None => return Err("could not find formats for rlibs".to_string()), + None => return Err(errors::LinkRlibError::MissingFormat), } - let name = info.crate_name[&cnum]; + let crate_name = info.crate_name[&cnum]; let used_crate_source = &info.used_crate_source[&cnum]; if let Some((path, _)) = &used_crate_source.rlib { f(cnum, &path); } else { if used_crate_source.rmeta.is_some() { - return Err(format!( - "could not find rlib for: `{}`, found rmeta (metadata) file", - name - )); + return Err(errors::LinkRlibError::OnlyRmetaFound { crate_name }); } else { - return Err(format!("could not find rlib for: `{}`", name)); + return Err(errors::LinkRlibError::NotFound { crate_name }); } } } @@ -340,10 +339,7 @@ fn link_rlib<'a>( // -whole-archive and it isn't clear how we can currently handle such a // situation correctly. // See https://github.com/rust-lang/rust/issues/88085#issuecomment-901050897 - sess.err( - "the linking modifiers `+bundle` and `+whole-archive` are not compatible \ - with each other when generating rlibs", - ); + sess.emit_err(errors::IncompatibleLinkingModifiers); } NativeLibKind::Static { bundle: None | Some(true), .. } => {} NativeLibKind::Static { bundle: Some(false), .. } @@ -365,12 +361,11 @@ fn link_rlib<'a>( )); continue; } - ab.add_archive(&location, Box::new(|_| false)).unwrap_or_else(|e| { - sess.fatal(&format!( - "failed to add native library {}: {}", - location.to_string_lossy(), - e - )); + ab.add_archive(&location, Box::new(|_| false)).unwrap_or_else(|error| { + sess.emit_fatal(errors::AddNativeLibrary { + library_path: &location.to_string_lossy(), + error, + }); }); } } @@ -385,8 +380,11 @@ fn link_rlib<'a>( tmpdir.as_ref(), ); - ab.add_archive(&output_path, Box::new(|_| false)).unwrap_or_else(|e| { - sess.fatal(&format!("failed to add native library {}: {}", output_path.display(), e)); + ab.add_archive(&output_path, Box::new(|_| false)).unwrap_or_else(|error| { + sess.emit_fatal(errors::AddNativeLibrary { + library_path: &output_path.display().to_string(), + error, + }); }); } @@ -451,14 +449,11 @@ fn collate_raw_dylibs( // FIXME: when we add support for ordinals, figure out if we need to do anything // if we have two DllImport values with the same name but different ordinals. if import.calling_convention != old_import.calling_convention { - sess.span_err( - import.span, - &format!( - "multiple declarations of external function `{}` from \ - library `{}` have different calling conventions", - import.name, name, - ), - ); + sess.emit_err(errors::MultipleExternalFuncDecl { + span: import.span, + function: import.name, + library_name: &name, + }); } } } @@ -560,7 +555,7 @@ fn link_staticlib<'a>( all_native_libs.extend(codegen_results.crate_info.native_libraries[&cnum].iter().cloned()); }); if let Err(e) = res { - sess.fatal(&e); + sess.emit_fatal(e); } ab.build(out_filename); @@ -673,9 +668,8 @@ fn link_dwarf_object<'a>( }) { Ok(()) => {} Err(e) => { - sess.struct_err("linking dwarf objects with thorin failed") - .note(&format!("{:?}", e)) - .emit(); + let thorin_error = errors::ThorinErrorWrapper(e); + sess.emit_err(errors::ThorinDwarfLinking { thorin_error }); sess.abort_if_errors(); } } @@ -879,23 +873,14 @@ fn link_natively<'a>( let mut output = prog.stderr.clone(); output.extend_from_slice(&prog.stdout); let escaped_output = escape_string(&output); - let mut err = sess.struct_err(&format!( - "linking with `{}` failed: {}", - linker_path.display(), - prog.status - )); - err.note(&format!("{:?}", &cmd)).note(&escaped_output); - if escaped_output.contains("undefined reference to") { - err.help( - "some `extern` functions couldn't be found; some native libraries may \ - need to be installed or have their path specified", - ); - err.note("use the `-l` flag to specify native libraries to link"); - err.note("use the `cargo:rustc-link-lib` directive to specify the native \ - libraries to link with Cargo (see https://doc.rust-lang.org/cargo/reference/build-scripts.html#cargorustc-link-libkindname)"); - } - err.emit(); - + // FIXME: Add UI tests for this error. + let err = errors::LinkingFailed { + linker_path: &linker_path, + exit_status: prog.status, + command: &cmd, + escaped_output: &escaped_output, + }; + sess.diagnostic().emit_err(err); // If MSVC's `link.exe` was expected but the return code // is not a Microsoft LNK error then suggest a way to fix or // install the Visual Studio build tools. diff --git a/compiler/rustc_codegen_ssa/src/errors.rs b/compiler/rustc_codegen_ssa/src/errors.rs index cf98cb2468ab7..6bd1dc2e03f66 100644 --- a/compiler/rustc_codegen_ssa/src/errors.rs +++ b/compiler/rustc_codegen_ssa/src/errors.rs @@ -1,10 +1,16 @@ //! Errors emitted by codegen_ssa -use rustc_errors::{DiagnosticArgValue, IntoDiagnosticArg}; +use crate::back::command::Command; +use rustc_errors::{ + fluent, DiagnosticArgValue, DiagnosticBuilder, ErrorGuaranteed, Handler, IntoDiagnostic, + IntoDiagnosticArg, +}; use rustc_macros::Diagnostic; +use rustc_span::{Span, Symbol}; use std::borrow::Cow; use std::io::Error; use std::path::{Path, PathBuf}; +use std::process::ExitStatus; #[derive(Diagnostic)] #[diag(codegen_ssa::lib_def_write_failure)] @@ -73,17 +79,15 @@ pub struct CopyPath<'a> { impl<'a> CopyPath<'a> { pub fn new(from: &'a Path, to: &'a Path, error: Error) -> CopyPath<'a> { - CopyPath { from: DebugArgPath { path: from }, to: DebugArgPath { path: to }, error } + CopyPath { from: DebugArgPath(from), to: DebugArgPath(to), error } } } -struct DebugArgPath<'a> { - pub path: &'a Path, -} +struct DebugArgPath<'a>(pub &'a Path); impl IntoDiagnosticArg for DebugArgPath<'_> { fn into_diagnostic_arg(self) -> rustc_errors::DiagnosticArgValue<'static> { - DiagnosticArgValue::Str(Cow::Owned(format!("{:?}", self.path))) + DiagnosticArgValue::Str(Cow::Owned(format!("{:?}", self.0))) } } @@ -98,3 +102,96 @@ pub struct IgnoringEmitPath { pub struct IgnoringOutput { pub extension: String, } + +#[derive(Diagnostic)] +#[diag(codegen_ssa::create_temp_dir)] +pub struct CreateTempDir { + pub error: Error, +} + +#[derive(Diagnostic)] +#[diag(codegen_ssa::incompatible_linking_modifiers)] +pub struct IncompatibleLinkingModifiers; + +#[derive(Diagnostic)] +#[diag(codegen_ssa::add_native_library)] +pub struct AddNativeLibrary<'a> { + pub library_path: &'a str, + pub error: Error, +} + +#[derive(Diagnostic)] +#[diag(codegen_ssa::multiple_external_func_decl)] +pub struct MultipleExternalFuncDecl<'a> { + #[primary_span] + pub span: Span, + pub function: Symbol, + pub library_name: &'a str, +} + +pub enum LinkRlibError { + MissingFormat, + OnlyRmetaFound { crate_name: Symbol }, + NotFound { crate_name: Symbol }, +} + +impl IntoDiagnostic<'_, !> for LinkRlibError { + fn into_diagnostic(self, handler: &Handler) -> DiagnosticBuilder<'_, !> { + match self { + LinkRlibError::MissingFormat => { + handler.struct_fatal(fluent::codegen_ssa::rlib_missing_format) + } + LinkRlibError::OnlyRmetaFound { crate_name } => { + let mut diag = handler.struct_fatal(fluent::codegen_ssa::rlib_only_rmeta_found); + diag.set_arg("crate_name", crate_name); + diag + } + LinkRlibError::NotFound { crate_name } => { + let mut diag = handler.struct_fatal(fluent::codegen_ssa::rlib_not_found); + diag.set_arg("crate_name", crate_name); + diag + } + } + } +} + +#[derive(Diagnostic)] +#[diag(codegen_ssa::thorin_dwarf_linking)] +#[note] +pub struct ThorinDwarfLinking { + pub thorin_error: ThorinErrorWrapper, +} +pub struct ThorinErrorWrapper(pub thorin::Error); + +// FIXME: How should we support translations for external crate errors? +impl IntoDiagnosticArg for ThorinErrorWrapper { + fn into_diagnostic_arg(self) -> DiagnosticArgValue<'static> { + DiagnosticArgValue::Str(Cow::Owned(format!("{:?}", self.0))) + } +} + +pub struct LinkingFailed<'a> { + pub linker_path: &'a PathBuf, + pub exit_status: ExitStatus, + pub command: &'a Command, + pub escaped_output: &'a str, +} + +impl IntoDiagnostic<'_> for LinkingFailed<'_> { + fn into_diagnostic(self, handler: &Handler) -> DiagnosticBuilder<'_, ErrorGuaranteed> { + let mut diag = handler.struct_err(fluent::codegen_ssa::linking_failed); + diag.set_arg("linker_path", format!("{}", self.linker_path.display())); + diag.set_arg("exit_status", format!("{}", self.exit_status)); + + diag.note(format!("{:?}", self.command)).note(self.escaped_output); + + // Trying to match an error from OS linkers + // which by now we have no way to translate. + if self.escaped_output.contains("undefined reference to") { + diag.note(fluent::codegen_ssa::extern_funcs_not_found) + .note(fluent::codegen_ssa::specify_libraries_to_link) + .note(fluent::codegen_ssa::use_cargo_directive); + } + diag + } +} diff --git a/compiler/rustc_codegen_ssa/src/lib.rs b/compiler/rustc_codegen_ssa/src/lib.rs index e62b6f342b2e9..ceebe4d417f7d 100644 --- a/compiler/rustc_codegen_ssa/src/lib.rs +++ b/compiler/rustc_codegen_ssa/src/lib.rs @@ -6,6 +6,7 @@ #![feature(strict_provenance)] #![feature(int_roundings)] #![feature(if_let_guard)] +#![feature(never_type)] #![recursion_limit = "256"] #![allow(rustc::potential_query_instability)] diff --git a/compiler/rustc_error_messages/locales/en-US/codegen_ssa.ftl b/compiler/rustc_error_messages/locales/en-US/codegen_ssa.ftl index 4d43b2eb0b696..d996a096a892e 100644 --- a/compiler/rustc_error_messages/locales/en-US/codegen_ssa.ftl +++ b/compiler/rustc_error_messages/locales/en-US/codegen_ssa.ftl @@ -25,3 +25,28 @@ codegen_ssa_copy_path_buf = unable to copy {$source_file} to {$output_path}: {$e codegen_ssa_ignoring_emit_path = ignoring emit path because multiple .{$extension} files were produced codegen_ssa_ignoring_output = ignoring -o because multiple .{$extension} files were produced + +codegen_ssa_create_temp_dir = couldn't create a temp dir: {$error} + +codegen_ssa_incompatible_linking_modifiers = the linking modifiers `+bundle` and `+whole-archive` are not compatible with each other when generating rlibs + +codegen_ssa_add_native_library = failed to add native library {$library_path}: {$error} + +codegen_ssa_multiple_external_func_decl = multiple declarations of external function `{$function}` from library `{$library_name}` have different calling conventions + +codegen_ssa_rlib_missing_format = could not find formats for rlibs + +codegen_ssa_rlib_only_rmeta_found = could not find rlib for: `{$crate_name}`, found rmeta (metadata) file + +codegen_ssa_rlib_not_found = could not find rlib for: `{$crate_name}` + +codegen_ssa_thorin_dwarf_linking = linking dwarf objects with thorin failed + .note = {$thorin_error} + +codegen_ssa_linking_failed = linking with `{$linker_path}` failed: {$exit_status} + +codegen_ssa_extern_funcs_not_found = some `extern` functions couldn't be found; some native libraries may need to be installed or have their path specified + +codegen_ssa_specify_libraries_to_link = use the `-l` flag to specify native libraries to link + +codegen_ssa_use_cargo_directive = use the `cargo:rustc-link-lib` directive to specify the native libraries to link with Cargo (see https://doc.rust-lang.org/cargo/reference/build-scripts.html#cargorustc-link-libkindname) From a25f939170bb73d813c5eb4cbed631253ba0cc15 Mon Sep 17 00:00:00 2001 From: Jhonny Bill Mena Date: Tue, 4 Oct 2022 13:25:13 -0400 Subject: [PATCH 12/27] Address PR comments - UPDATE - revert migration of logs - UPDATE - use derive on LinkRlibError enum - [Gardening] UPDATE - alphabetically sort fluent_messages - UPDATE - use PathBuf and unify both AddNativeLibrary to use Display (which is what PathBuf uses when conforming to IntoDiagnosticArg) - UPDATE - fluent messages sort after rebase --- compiler/rustc_codegen_ssa/src/back/link.rs | 10 +---- compiler/rustc_codegen_ssa/src/back/linker.rs | 6 +-- compiler/rustc_codegen_ssa/src/errors.rs | 42 ++++--------------- .../locales/en-US/codegen_ssa.ftl | 6 --- compiler/rustc_error_messages/src/lib.rs | 4 +- 5 files changed, 15 insertions(+), 53 deletions(-) diff --git a/compiler/rustc_codegen_ssa/src/back/link.rs b/compiler/rustc_codegen_ssa/src/back/link.rs index ac2a8f969df00..e798fb421df31 100644 --- a/compiler/rustc_codegen_ssa/src/back/link.rs +++ b/compiler/rustc_codegen_ssa/src/back/link.rs @@ -362,10 +362,7 @@ fn link_rlib<'a>( continue; } ab.add_archive(&location, Box::new(|_| false)).unwrap_or_else(|error| { - sess.emit_fatal(errors::AddNativeLibrary { - library_path: &location.to_string_lossy(), - error, - }); + sess.emit_fatal(errors::AddNativeLibrary { library_path: location, error }); }); } } @@ -381,10 +378,7 @@ fn link_rlib<'a>( ); ab.add_archive(&output_path, Box::new(|_| false)).unwrap_or_else(|error| { - sess.emit_fatal(errors::AddNativeLibrary { - library_path: &output_path.display().to_string(), - error, - }); + sess.emit_fatal(errors::AddNativeLibrary { library_path: output_path, error }); }); } diff --git a/compiler/rustc_codegen_ssa/src/back/linker.rs b/compiler/rustc_codegen_ssa/src/back/linker.rs index debcffcd326f6..bad22ccb1fedd 100644 --- a/compiler/rustc_codegen_ssa/src/back/linker.rs +++ b/compiler/rustc_codegen_ssa/src/back/linker.rs @@ -91,13 +91,13 @@ pub fn get_linker<'a>( arg.push(format!("{}\\lib\\{}\\store", root_lib_path.display(), a)); cmd.arg(&arg); } else { - sess.emit_warning(errors::UnsupportedArch); + warn!("arch is not supported"); } } else { - sess.emit_warning(errors::MsvcPathNotFound); + warn!("MSVC root path lib location not found"); } } else { - sess.emit_warning(errors::LinkExeNotFound); + warn!("link.exe not found"); } } diff --git a/compiler/rustc_codegen_ssa/src/errors.rs b/compiler/rustc_codegen_ssa/src/errors.rs index 6bd1dc2e03f66..bd399ffb5a6f9 100644 --- a/compiler/rustc_codegen_ssa/src/errors.rs +++ b/compiler/rustc_codegen_ssa/src/errors.rs @@ -30,18 +30,6 @@ pub struct SymbolFileWriteFailure { pub error: Error, } -#[derive(Diagnostic)] -#[diag(codegen_ssa::unsupported_arch)] -pub struct UnsupportedArch; - -#[derive(Diagnostic)] -#[diag(codegen_ssa::msvc_path_not_found)] -pub struct MsvcPathNotFound; - -#[derive(Diagnostic)] -#[diag(codegen_ssa::link_exe_not_found)] -pub struct LinkExeNotFound; - #[derive(Diagnostic)] #[diag(codegen_ssa::ld64_unimplemented_modifier)] pub struct Ld64UnimplementedModifier; @@ -115,8 +103,8 @@ pub struct IncompatibleLinkingModifiers; #[derive(Diagnostic)] #[diag(codegen_ssa::add_native_library)] -pub struct AddNativeLibrary<'a> { - pub library_path: &'a str, +pub struct AddNativeLibrary { + pub library_path: PathBuf, pub error: Error, } @@ -129,30 +117,16 @@ pub struct MultipleExternalFuncDecl<'a> { pub library_name: &'a str, } +#[derive(Diagnostic)] pub enum LinkRlibError { + #[diag(codegen_ssa::rlib_missing_format)] MissingFormat, + + #[diag(codegen_ssa::rlib_only_rmeta_found)] OnlyRmetaFound { crate_name: Symbol }, - NotFound { crate_name: Symbol }, -} -impl IntoDiagnostic<'_, !> for LinkRlibError { - fn into_diagnostic(self, handler: &Handler) -> DiagnosticBuilder<'_, !> { - match self { - LinkRlibError::MissingFormat => { - handler.struct_fatal(fluent::codegen_ssa::rlib_missing_format) - } - LinkRlibError::OnlyRmetaFound { crate_name } => { - let mut diag = handler.struct_fatal(fluent::codegen_ssa::rlib_only_rmeta_found); - diag.set_arg("crate_name", crate_name); - diag - } - LinkRlibError::NotFound { crate_name } => { - let mut diag = handler.struct_fatal(fluent::codegen_ssa::rlib_not_found); - diag.set_arg("crate_name", crate_name); - diag - } - } - } + #[diag(codegen_ssa::rlib_not_found)] + NotFound { crate_name: Symbol }, } #[derive(Diagnostic)] diff --git a/compiler/rustc_error_messages/locales/en-US/codegen_ssa.ftl b/compiler/rustc_error_messages/locales/en-US/codegen_ssa.ftl index d996a096a892e..99ddf6842dab8 100644 --- a/compiler/rustc_error_messages/locales/en-US/codegen_ssa.ftl +++ b/compiler/rustc_error_messages/locales/en-US/codegen_ssa.ftl @@ -4,12 +4,6 @@ codegen_ssa_version_script_write_failure = failed to write version script: {$err codegen_ssa_symbol_file_write_failure = failed to write symbols file: {$error} -codegen_ssa_unsupported_arch = arch is not supported - -codegen_ssa_msvc_path_not_found = MSVC root path lib location not found - -codegen_ssa_link_exe_not_found = link.exe not found - codegen_ssa_ld64_unimplemented_modifier = `as-needed` modifier not implemented yet for ld64 codegen_ssa_linker_unsupported_modifier = `as-needed` modifier not supported for current linker diff --git a/compiler/rustc_error_messages/src/lib.rs b/compiler/rustc_error_messages/src/lib.rs index 10f8efdffa8b6..77f87d5b007e3 100644 --- a/compiler/rustc_error_messages/src/lib.rs +++ b/compiler/rustc_error_messages/src/lib.rs @@ -40,9 +40,10 @@ fluent_messages! { attr => "../locales/en-US/attr.ftl", borrowck => "../locales/en-US/borrowck.ftl", builtin_macros => "../locales/en-US/builtin_macros.ftl", + codegen_gcc => "../locales/en-US/codegen_gcc.ftl", + codegen_ssa => "../locales/en-US/codegen_ssa.ftl", compiletest => "../locales/en-US/compiletest.ftl", const_eval => "../locales/en-US/const_eval.ftl", - codegen_gcc => "../locales/en-US/codegen_gcc.ftl", driver => "../locales/en-US/driver.ftl", expand => "../locales/en-US/expand.ftl", hir_analysis => "../locales/en-US/hir_analysis.ftl", @@ -63,7 +64,6 @@ fluent_messages! { symbol_mangling => "../locales/en-US/symbol_mangling.ftl", trait_selection => "../locales/en-US/trait_selection.ftl", ty_utils => "../locales/en-US/ty_utils.ftl", - codegen_ssa => "../locales/en-US/codegen_ssa.ftl", } pub use fluent_generated::{self as fluent, DEFAULT_LOCALE_RESOURCES}; From 13d4f27c829a332f50a2109647f3b16bce119b8d Mon Sep 17 00:00:00 2001 From: Jhonny Bill Mena Date: Tue, 4 Oct 2022 13:21:22 -0400 Subject: [PATCH 13/27] ADD - implement IntoDiagnostic for thorin::Error wrapper --- compiler/rustc_codegen_ssa/src/back/link.rs | 3 +- compiler/rustc_codegen_ssa/src/errors.rs | 202 +++++++++++++++++- .../locales/en-US/codegen_ssa.ftl | 79 ++++++- 3 files changed, 269 insertions(+), 15 deletions(-) diff --git a/compiler/rustc_codegen_ssa/src/back/link.rs b/compiler/rustc_codegen_ssa/src/back/link.rs index e798fb421df31..95e72184ff037 100644 --- a/compiler/rustc_codegen_ssa/src/back/link.rs +++ b/compiler/rustc_codegen_ssa/src/back/link.rs @@ -662,8 +662,7 @@ fn link_dwarf_object<'a>( }) { Ok(()) => {} Err(e) => { - let thorin_error = errors::ThorinErrorWrapper(e); - sess.emit_err(errors::ThorinDwarfLinking { thorin_error }); + sess.emit_err(errors::ThorinErrorWrapper(e)); sess.abort_if_errors(); } } diff --git a/compiler/rustc_codegen_ssa/src/errors.rs b/compiler/rustc_codegen_ssa/src/errors.rs index bd399ffb5a6f9..0ffe887202261 100644 --- a/compiler/rustc_codegen_ssa/src/errors.rs +++ b/compiler/rustc_codegen_ssa/src/errors.rs @@ -129,18 +129,200 @@ pub enum LinkRlibError { NotFound { crate_name: Symbol }, } -#[derive(Diagnostic)] -#[diag(codegen_ssa::thorin_dwarf_linking)] -#[note] -pub struct ThorinDwarfLinking { - pub thorin_error: ThorinErrorWrapper, -} pub struct ThorinErrorWrapper(pub thorin::Error); -// FIXME: How should we support translations for external crate errors? -impl IntoDiagnosticArg for ThorinErrorWrapper { - fn into_diagnostic_arg(self) -> DiagnosticArgValue<'static> { - DiagnosticArgValue::Str(Cow::Owned(format!("{:?}", self.0))) +impl IntoDiagnostic<'_> for ThorinErrorWrapper { + fn into_diagnostic(self, handler: &Handler) -> DiagnosticBuilder<'_, ErrorGuaranteed> { + let mut diag; + match self.0 { + thorin::Error::ReadInput(_) => { + diag = handler.struct_err(fluent::codegen_ssa::thorin_read_input_failure); + diag + } + thorin::Error::ParseFileKind(_) => { + diag = handler.struct_err(fluent::codegen_ssa::thorin_parse_input_file_kind); + diag + } + thorin::Error::ParseObjectFile(_) => { + diag = handler.struct_err(fluent::codegen_ssa::thorin_parse_input_object_file); + diag + } + thorin::Error::ParseArchiveFile(_) => { + diag = handler.struct_err(fluent::codegen_ssa::thorin_parse_input_archive_file); + diag + } + thorin::Error::ParseArchiveMember(_) => { + diag = handler.struct_err(fluent::codegen_ssa::thorin_parse_archive_member); + diag + } + thorin::Error::InvalidInputKind => { + diag = handler.struct_err(fluent::codegen_ssa::thorin_invalid_input_kind); + diag + } + thorin::Error::DecompressData(_) => { + diag = handler.struct_err(fluent::codegen_ssa::thorin_decompress_data); + diag + } + thorin::Error::NamelessSection(_, offset) => { + diag = handler.struct_err(fluent::codegen_ssa::thorin_section_without_name); + diag.set_arg("offset", format!("0x{:08x}", offset)); + diag + } + thorin::Error::RelocationWithInvalidSymbol(section, offset) => { + diag = + handler.struct_err(fluent::codegen_ssa::thorin_relocation_with_invalid_symbol); + diag.set_arg("section", section); + diag.set_arg("offset", format!("0x{:08x}", offset)); + diag + } + thorin::Error::MultipleRelocations(section, offset) => { + diag = handler.struct_err(fluent::codegen_ssa::thorin_multiple_relocations); + diag.set_arg("section", section); + diag.set_arg("offset", format!("0x{:08x}", offset)); + diag + } + thorin::Error::UnsupportedRelocation(section, offset) => { + diag = handler.struct_err(fluent::codegen_ssa::thorin_unsupported_relocation); + diag.set_arg("section", section); + diag.set_arg("offset", format!("0x{:08x}", offset)); + diag + } + thorin::Error::MissingDwoName(id) => { + diag = handler.struct_err(fluent::codegen_ssa::thorin_missing_dwo_name); + diag.set_arg("id", format!("0x{:08x}", id)); + diag + } + thorin::Error::NoCompilationUnits => { + diag = handler.struct_err(fluent::codegen_ssa::thorin_no_compilation_units); + diag + } + thorin::Error::NoDie => { + diag = handler.struct_err(fluent::codegen_ssa::thorin_no_die); + diag + } + thorin::Error::TopLevelDieNotUnit => { + diag = handler.struct_err(fluent::codegen_ssa::thorin_top_level_die_not_unit); + diag + } + thorin::Error::MissingRequiredSection(section) => { + diag = handler.struct_err(fluent::codegen_ssa::thorin_missing_required_section); + diag.set_arg("section", section); + diag + } + thorin::Error::ParseUnitAbbreviations(_) => { + diag = handler.struct_err(fluent::codegen_ssa::thorin_parse_unit_abbreviations); + diag + } + thorin::Error::ParseUnitAttribute(_) => { + diag = handler.struct_err(fluent::codegen_ssa::thorin_parse_unit_attribute); + diag + } + thorin::Error::ParseUnitHeader(_) => { + diag = handler.struct_err(fluent::codegen_ssa::thorin_parse_unit_header); + diag + } + thorin::Error::ParseUnit(_) => { + diag = handler.struct_err(fluent::codegen_ssa::thorin_parse_unit); + diag + } + thorin::Error::IncompatibleIndexVersion(section, format, actual) => { + diag = handler.struct_err(fluent::codegen_ssa::thorin_incompatible_index_version); + diag.set_arg("section", section); + diag.set_arg("actual", actual); + diag.set_arg("format", format); + diag + } + thorin::Error::OffsetAtIndex(_, index) => { + diag = handler.struct_err(fluent::codegen_ssa::thorin_offset_at_index); + diag.set_arg("index", index); + diag + } + thorin::Error::StrAtOffset(_, offset) => { + diag = handler.struct_err(fluent::codegen_ssa::thorin_str_at_offset); + diag.set_arg("offset", format!("0x{:08x}", offset)); + diag + } + thorin::Error::ParseIndex(_, section) => { + diag = handler.struct_err(fluent::codegen_ssa::thorin_parse_index); + diag.set_arg("section", section); + diag + } + thorin::Error::UnitNotInIndex(unit) => { + diag = handler.struct_err(fluent::codegen_ssa::thorin_unit_not_in_index); + diag.set_arg("unit", format!("0x{:08x}", unit)); + diag + } + thorin::Error::RowNotInIndex(_, row) => { + diag = handler.struct_err(fluent::codegen_ssa::thorin_row_not_in_index); + diag.set_arg("row", row); + diag + } + thorin::Error::SectionNotInRow => { + diag = handler.struct_err(fluent::codegen_ssa::thorin_section_not_in_row); + diag + } + thorin::Error::EmptyUnit(unit) => { + diag = handler.struct_err(fluent::codegen_ssa::thorin_empty_unit); + diag.set_arg("unit", format!("0x{:08x}", unit)); + diag + } + thorin::Error::MultipleDebugInfoSection => { + diag = handler.struct_err(fluent::codegen_ssa::thorin_multiple_debug_info_section); + diag + } + thorin::Error::MultipleDebugTypesSection => { + diag = handler.struct_err(fluent::codegen_ssa::thorin_multiple_debug_types_section); + diag + } + thorin::Error::NotSplitUnit => { + diag = handler.struct_err(fluent::codegen_ssa::thorin_not_split_unit); + diag + } + thorin::Error::DuplicateUnit(unit) => { + diag = handler.struct_err(fluent::codegen_ssa::thorin_duplicate_unit); + diag.set_arg("unit", format!("0x{:08x}", unit)); + diag + } + thorin::Error::MissingReferencedUnit(unit) => { + diag = handler.struct_err(fluent::codegen_ssa::thorin_missing_referenced_unit); + diag.set_arg("unit", format!("0x{:08x}", unit)); + diag + } + thorin::Error::NoOutputObjectCreated => { + diag = handler.struct_err(fluent::codegen_ssa::thorin_not_output_object_created); + diag + } + thorin::Error::MixedInputEncodings => { + diag = handler.struct_err(fluent::codegen_ssa::thorin_mixed_input_encodings); + diag + } + thorin::Error::Io(e) => { + diag = handler.struct_err(fluent::codegen_ssa::thorin_io); + diag.set_arg("error", format!("{e}")); + diag + } + thorin::Error::ObjectRead(e) => { + diag = handler.struct_err(fluent::codegen_ssa::thorin_object_read); + diag.set_arg("error", format!("{e}")); + diag + } + thorin::Error::ObjectWrite(e) => { + diag = handler.struct_err(fluent::codegen_ssa::thorin_object_write); + diag.set_arg("error", format!("{e}")); + diag + } + thorin::Error::GimliRead(e) => { + diag = handler.struct_err(fluent::codegen_ssa::thorin_gimli_read); + diag.set_arg("error", format!("{e}")); + diag + } + thorin::Error::GimliWrite(e) => { + diag = handler.struct_err(fluent::codegen_ssa::thorin_gimli_write); + diag.set_arg("error", format!("{e}")); + diag + } + _ => unimplemented!("Untranslated thorin error"), + } } } diff --git a/compiler/rustc_error_messages/locales/en-US/codegen_ssa.ftl b/compiler/rustc_error_messages/locales/en-US/codegen_ssa.ftl index 99ddf6842dab8..0d0388a039e2d 100644 --- a/compiler/rustc_error_messages/locales/en-US/codegen_ssa.ftl +++ b/compiler/rustc_error_messages/locales/en-US/codegen_ssa.ftl @@ -34,9 +34,6 @@ codegen_ssa_rlib_only_rmeta_found = could not find rlib for: `{$crate_name}`, fo codegen_ssa_rlib_not_found = could not find rlib for: `{$crate_name}` -codegen_ssa_thorin_dwarf_linking = linking dwarf objects with thorin failed - .note = {$thorin_error} - codegen_ssa_linking_failed = linking with `{$linker_path}` failed: {$exit_status} codegen_ssa_extern_funcs_not_found = some `extern` functions couldn't be found; some native libraries may need to be installed or have their path specified @@ -44,3 +41,79 @@ codegen_ssa_extern_funcs_not_found = some `extern` functions couldn't be found; codegen_ssa_specify_libraries_to_link = use the `-l` flag to specify native libraries to link codegen_ssa_use_cargo_directive = use the `cargo:rustc-link-lib` directive to specify the native libraries to link with Cargo (see https://doc.rust-lang.org/cargo/reference/build-scripts.html#cargorustc-link-libkindname) + +codegen_ssa_thorin_read_input_failure = failed to read input file + +codegen_ssa_thorin_parse_input_file_kind = failed to parse input file kind + +codegen_ssa_thorin_parse_input_object_file = failed to parse input object file + +codegen_ssa_thorin_parse_input_archive_file = failed to parse input archive file + +codegen_ssa_thorin_parse_archive_member = failed to parse archive member + +codegen_ssa_thorin_invalid_input_kind = input is not an archive or elf object + +codegen_ssa_thorin_decompress_data = failed to decompress compressed section + +codegen_ssa_thorin_section_without_name = section without name at offset {$offset} + +codegen_ssa_thorin_relocation_with_invalid_symbol = relocation with invalid symbol for section `{$section}` at offset {$offset} + +codegen_ssa_thorin_multiple_relocations = multiple relocations for section `{$section}` at offset {$offset} + +codegen_ssa_thorin_unsupported_relocation = unsupported relocation for section {$section} at offset {$offset} + +codegen_ssa_thorin_missing_dwo_name = missing path attribute to DWARF object ({$id}) + +codegen_ssa_thorin_no_compilation_units = input object has no compilation units + +codegen_ssa_thorin_no_die = no top-level debugging information entry in compilation/type unit + +codegen_ssa_thorin_top_level_die_not_unit = top-level debugging information entry is not a compilation/type unit + +codegen_ssa_thorin_missing_required_section = input object missing required section `{$section}` + +codegen_ssa_thorin_parse_unit_abbreviations = failed to parse unit abbreviations + +codegen_ssa_thorin_parse_unit_attribute = failed to parse unit attribute + +codegen_ssa_thorin_parse_unit_header = failed to parse unit header + +codegen_ssa_thorin_parse_unit = failed to parse unit + +codegen_ssa_thorin_incompatible_index_version = incompatible `{$section}` index version: found version {$actual}, expected version {$format} + +codegen_ssa_thorin_offset_at_index = read offset at index {$index} of `.debug_str_offsets.dwo` section + +codegen_ssa_thorin_str_at_offset = read string at offset {$offset} of `.debug_str.dwo` section + +codegen_ssa_thorin_parse_index = failed to parse `{$section}` index section + +codegen_ssa_thorin_unit_not_in_index = unit {$unit} from input package is not in its index + +codegen_ssa_thorin_row_not_in_index = row {$row} found in index's hash table not present in index + +codegen_ssa_thorin_section_not_in_row = section not found in unit's row in index + +codegen_ssa_thorin_empty_unit = unit {$unit} in input DWARF object with no data + +codegen_ssa_thorin_multiple_debug_info_section = multiple `.debug_info.dwo` sections + +codegen_ssa_thorin_multiple_debug_types_section = multiple `.debug_types.dwo` sections in a package + +codegen_ssa_thorin_not_split_unit = regular compilation unit in object (missing dwo identifier) + +codegen_ssa_thorin_duplicate_unit = duplicate split compilation unit ({$unit}) + +codegen_ssa_thorin_missing_referenced_unit = unit {$unit} referenced by executable was not found + +codegen_ssa_thorin_not_output_object_created = no output object was created from inputs + +codegen_ssa_thorin_mixed_input_encodings = input objects haved mixed encodings + +codegen_ssa_thorin_io = {$error} +codegen_ssa_thorin_object_read = {$error} +codegen_ssa_thorin_object_write = {$error} +codegen_ssa_thorin_gimli_read = {$error} +codegen_ssa_thorin_gimli_write = {$error} From b4c8a7b952de72bc70e798408efbd4124fa15c59 Mon Sep 17 00:00:00 2001 From: joboet Date: Sat, 8 Oct 2022 09:07:28 +0200 Subject: [PATCH 14/27] std: remove unused linker attribute --- library/std/src/sys/unix/thread_parker/darwin.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/std/src/sys/unix/thread_parker/darwin.rs b/library/std/src/sys/unix/thread_parker/darwin.rs index 510839d5dafbf..2f5356fe2276b 100644 --- a/library/std/src/sys/unix/thread_parker/darwin.rs +++ b/library/std/src/sys/unix/thread_parker/darwin.rs @@ -24,7 +24,7 @@ type dispatch_time_t = u64; const DISPATCH_TIME_NOW: dispatch_time_t = 0; const DISPATCH_TIME_FOREVER: dispatch_time_t = !0; -#[link(name = "System", kind = "dylib")] +// Contained in libSystem.dylib, which is linked by default. extern "C" { fn dispatch_time(when: dispatch_time_t, delta: i64) -> dispatch_time_t; fn dispatch_semaphore_create(val: isize) -> dispatch_semaphore_t; From c320ab98ff1d4adb32cece206aa895e4effae175 Mon Sep 17 00:00:00 2001 From: joboet Date: Sat, 8 Oct 2022 09:12:06 +0200 Subject: [PATCH 15/27] std: do not use dispatch semaphore under miri (yet) --- library/std/src/sys/unix/thread_parker/mod.rs | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/library/std/src/sys/unix/thread_parker/mod.rs b/library/std/src/sys/unix/thread_parker/mod.rs index 724ec2d482edb..35f1e68a87e5b 100644 --- a/library/std/src/sys/unix/thread_parker/mod.rs +++ b/library/std/src/sys/unix/thread_parker/mod.rs @@ -11,11 +11,14 @@ )))] cfg_if::cfg_if! { - if #[cfg(any( - target_os = "macos", - target_os = "ios", - target_os = "watchos", - target_os = "tvos", + if #[cfg(all( + any( + target_os = "macos", + target_os = "ios", + target_os = "watchos", + target_os = "tvos", + ), + not(miri), ))] { mod darwin; pub use darwin::Parker; From febbf71219001cd7948c3d1e5ffa68706896d955 Mon Sep 17 00:00:00 2001 From: David Wood Date: Mon, 3 Oct 2022 15:34:00 +0100 Subject: [PATCH 16/27] macros: tidy up lint changes Small tweaks to changes made in a previous PR, unrelated to eager translation. Signed-off-by: David Wood --- compiler/rustc_macros/src/diagnostics/diagnostic.rs | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/compiler/rustc_macros/src/diagnostics/diagnostic.rs b/compiler/rustc_macros/src/diagnostics/diagnostic.rs index 83040a652b18a..406a56cd4ae3e 100644 --- a/compiler/rustc_macros/src/diagnostics/diagnostic.rs +++ b/compiler/rustc_macros/src/diagnostics/diagnostic.rs @@ -105,8 +105,8 @@ impl<'a> LintDiagnosticDerive<'a> { }); let msg = builder.each_variant(&mut structure, |mut builder, variant| { - // HACK(wafflelapkin): initialize slug (???) - let _preamble = builder.preamble(&variant); + // Collect the slug by generating the preamble. + let _ = builder.preamble(&variant); match builder.slug.value_ref() { None => { @@ -125,7 +125,10 @@ impl<'a> LintDiagnosticDerive<'a> { let diag = &builder.diag; structure.gen_impl(quote! { gen impl<'__a> rustc_errors::DecorateLint<'__a, ()> for @Self { - fn decorate_lint<'__b>(self, #diag: &'__b mut rustc_errors::DiagnosticBuilder<'__a, ()>) -> &'__b mut rustc_errors::DiagnosticBuilder<'__a, ()> { + fn decorate_lint<'__b>( + self, + #diag: &'__b mut rustc_errors::DiagnosticBuilder<'__a, ()> + ) -> &'__b mut rustc_errors::DiagnosticBuilder<'__a, ()> { use rustc_errors::IntoDiagnosticArg; #implementation } From 508d7e6d26bd636081c96ee6ed61f833917343ea Mon Sep 17 00:00:00 2001 From: David Wood Date: Mon, 3 Oct 2022 14:02:49 +0100 Subject: [PATCH 17/27] errors: use `HashMap` to store diagnostic args Eager translation will enable subdiagnostics to be translated multiple times with different arguments - this requires the ability to replace the value of one argument with a new value, which is better suited to a `HashMap` than the previous storage, a `Vec`. Signed-off-by: David Wood --- compiler/rustc_codegen_ssa/src/back/write.rs | 7 +++-- .../src/annotate_snippet_emitter_writer.rs | 4 +-- compiler/rustc_errors/src/diagnostic.rs | 19 ++++++++---- compiler/rustc_errors/src/emitter.rs | 4 +-- compiler/rustc_errors/src/json.rs | 4 +-- compiler/rustc_errors/src/translation.rs | 30 +++++++++++++------ .../passes/check_code_block_syntax.rs | 7 +++-- 7 files changed, 49 insertions(+), 26 deletions(-) diff --git a/compiler/rustc_codegen_ssa/src/back/write.rs b/compiler/rustc_codegen_ssa/src/back/write.rs index 6188094bbbdd4..e3534544b6810 100644 --- a/compiler/rustc_codegen_ssa/src/back/write.rs +++ b/compiler/rustc_codegen_ssa/src/back/write.rs @@ -15,7 +15,10 @@ use rustc_data_structures::profiling::TimingGuard; use rustc_data_structures::profiling::VerboseTimingGuard; use rustc_data_structures::sync::Lrc; use rustc_errors::emitter::Emitter; -use rustc_errors::{translation::Translate, DiagnosticId, FatalError, Handler, Level}; +use rustc_errors::{ + translation::{to_fluent_args, Translate}, + DiagnosticId, FatalError, Handler, Level, +}; use rustc_fs_util::link_or_copy; use rustc_hir::def_id::{CrateNum, LOCAL_CRATE}; use rustc_incremental::{ @@ -1750,7 +1753,7 @@ impl Translate for SharedEmitter { impl Emitter for SharedEmitter { fn emit_diagnostic(&mut self, diag: &rustc_errors::Diagnostic) { - let fluent_args = self.to_fluent_args(diag.args()); + let fluent_args = to_fluent_args(diag.args()); drop(self.sender.send(SharedEmitterMessage::Diagnostic(Diagnostic { msg: self.translate_messages(&diag.message, &fluent_args).to_string(), code: diag.code.clone(), diff --git a/compiler/rustc_errors/src/annotate_snippet_emitter_writer.rs b/compiler/rustc_errors/src/annotate_snippet_emitter_writer.rs index b32fc3c719bbd..f14b8ee3254f3 100644 --- a/compiler/rustc_errors/src/annotate_snippet_emitter_writer.rs +++ b/compiler/rustc_errors/src/annotate_snippet_emitter_writer.rs @@ -7,7 +7,7 @@ use crate::emitter::FileWithAnnotatedLines; use crate::snippet::Line; -use crate::translation::Translate; +use crate::translation::{to_fluent_args, Translate}; use crate::{ CodeSuggestion, Diagnostic, DiagnosticId, DiagnosticMessage, Emitter, FluentBundle, LazyFallbackBundle, Level, MultiSpan, Style, SubDiagnostic, @@ -46,7 +46,7 @@ impl Translate for AnnotateSnippetEmitterWriter { impl Emitter for AnnotateSnippetEmitterWriter { /// The entry point for the diagnostics generation fn emit_diagnostic(&mut self, diag: &Diagnostic) { - let fluent_args = self.to_fluent_args(diag.args()); + let fluent_args = to_fluent_args(diag.args()); let mut children = diag.children.clone(); let (mut primary_span, suggestions) = self.primary_span_formatted(&diag, &fluent_args); diff --git a/compiler/rustc_errors/src/diagnostic.rs b/compiler/rustc_errors/src/diagnostic.rs index 31e410aaaf082..a267bf6c0b4d1 100644 --- a/compiler/rustc_errors/src/diagnostic.rs +++ b/compiler/rustc_errors/src/diagnostic.rs @@ -27,7 +27,11 @@ pub struct SuggestionsDisabled; /// Simplified version of `FluentArg` that can implement `Encodable` and `Decodable`. Collection of /// `DiagnosticArg` are converted to `FluentArgs` (consuming the collection) at the start of /// diagnostic emission. -pub type DiagnosticArg<'source> = (Cow<'source, str>, DiagnosticArgValue<'source>); +pub type DiagnosticArg<'iter, 'source> = + (&'iter DiagnosticArgName<'source>, &'iter DiagnosticArgValue<'source>); + +/// Name of a diagnostic argument. +pub type DiagnosticArgName<'source> = Cow<'source, str>; /// Simplified version of `FluentValue` that can implement `Encodable` and `Decodable`. Converted /// to a `FluentValue` by the emitter to be used in diagnostic translation. @@ -229,7 +233,7 @@ pub struct Diagnostic { pub span: MultiSpan, pub children: Vec, pub suggestions: Result, SuggestionsDisabled>, - args: Vec>, + args: FxHashMap, DiagnosticArgValue<'static>>, /// This is not used for highlighting or rendering any error message. Rather, it can be used /// as a sort key to sort a buffer of diagnostics. By default, it is the primary span of @@ -321,7 +325,7 @@ impl Diagnostic { span: MultiSpan::new(), children: vec![], suggestions: Ok(vec![]), - args: vec![], + args: Default::default(), sort_span: DUMMY_SP, is_lint: false, } @@ -956,8 +960,11 @@ impl Diagnostic { self } - pub fn args(&self) -> &[DiagnosticArg<'static>] { - &self.args + // Exact iteration order of diagnostic arguments shouldn't make a difference to output because + // they're only used in interpolation. + #[allow(rustc::potential_query_instability)] + pub fn args<'a>(&'a self) -> impl Iterator> { + self.args.iter() } pub fn set_arg( @@ -965,7 +972,7 @@ impl Diagnostic { name: impl Into>, arg: impl IntoDiagnosticArg, ) -> &mut Self { - self.args.push((name.into(), arg.into_diagnostic_arg())); + self.args.insert(name.into(), arg.into_diagnostic_arg()); self } diff --git a/compiler/rustc_errors/src/emitter.rs b/compiler/rustc_errors/src/emitter.rs index 66fbb8f1213e0..cd6413bc3ec62 100644 --- a/compiler/rustc_errors/src/emitter.rs +++ b/compiler/rustc_errors/src/emitter.rs @@ -14,7 +14,7 @@ use rustc_span::{FileLines, SourceFile, Span}; use crate::snippet::{Annotation, AnnotationType, Line, MultilineAnnotation, Style, StyledString}; use crate::styled_buffer::StyledBuffer; -use crate::translation::Translate; +use crate::translation::{to_fluent_args, Translate}; use crate::{ CodeSuggestion, Diagnostic, DiagnosticId, DiagnosticMessage, FluentBundle, Handler, LazyFallbackBundle, Level, MultiSpan, SubDiagnostic, SubstitutionHighlight, SuggestionStyle, @@ -535,7 +535,7 @@ impl Emitter for EmitterWriter { } fn emit_diagnostic(&mut self, diag: &Diagnostic) { - let fluent_args = self.to_fluent_args(diag.args()); + let fluent_args = to_fluent_args(diag.args()); let mut children = diag.children.clone(); let (mut primary_span, suggestions) = self.primary_span_formatted(&diag, &fluent_args); diff --git a/compiler/rustc_errors/src/json.rs b/compiler/rustc_errors/src/json.rs index 1680c6accd78c..4cc7be47fc2c6 100644 --- a/compiler/rustc_errors/src/json.rs +++ b/compiler/rustc_errors/src/json.rs @@ -13,7 +13,7 @@ use rustc_span::source_map::{FilePathMapping, SourceMap}; use crate::emitter::{Emitter, HumanReadableErrorType}; use crate::registry::Registry; -use crate::translation::Translate; +use crate::translation::{to_fluent_args, Translate}; use crate::DiagnosticId; use crate::{ CodeSuggestion, FluentBundle, LazyFallbackBundle, MultiSpan, SpanLabel, SubDiagnostic, @@ -312,7 +312,7 @@ struct UnusedExterns<'a, 'b, 'c> { impl Diagnostic { fn from_errors_diagnostic(diag: &crate::Diagnostic, je: &JsonEmitter) -> Diagnostic { - let args = je.to_fluent_args(diag.args()); + let args = to_fluent_args(diag.args()); let sugg = diag.suggestions.iter().flatten().map(|sugg| { let translated_message = je.translate_message(&sugg.msg, &args); Diagnostic { diff --git a/compiler/rustc_errors/src/translation.rs b/compiler/rustc_errors/src/translation.rs index 4f407badb3f9e..c2ec2526a4aea 100644 --- a/compiler/rustc_errors/src/translation.rs +++ b/compiler/rustc_errors/src/translation.rs @@ -4,6 +4,27 @@ use rustc_data_structures::sync::Lrc; use rustc_error_messages::FluentArgs; use std::borrow::Cow; +/// Convert diagnostic arguments (a rustc internal type that exists to implement +/// `Encodable`/`Decodable`) into `FluentArgs` which is necessary to perform translation. +/// +/// Typically performed once for each diagnostic at the start of `emit_diagnostic` and then +/// passed around as a reference thereafter. +pub fn to_fluent_args<'iter, 'arg: 'iter>( + iter: impl Iterator>, +) -> FluentArgs<'arg> { + let mut args = if let Some(size) = iter.size_hint().1 { + FluentArgs::with_capacity(size) + } else { + FluentArgs::new() + }; + + for (k, v) in iter { + args.set(k.clone(), v.clone()); + } + + args +} + pub trait Translate { /// Return `FluentBundle` with localized diagnostics for the locale requested by the user. If no /// language was requested by the user then this will be `None` and `fallback_fluent_bundle` @@ -15,15 +36,6 @@ pub trait Translate { /// unavailable for the requested locale. fn fallback_fluent_bundle(&self) -> &FluentBundle; - /// Convert diagnostic arguments (a rustc internal type that exists to implement - /// `Encodable`/`Decodable`) into `FluentArgs` which is necessary to perform translation. - /// - /// Typically performed once for each diagnostic at the start of `emit_diagnostic` and then - /// passed around as a reference thereafter. - fn to_fluent_args<'arg>(&self, args: &[DiagnosticArg<'arg>]) -> FluentArgs<'arg> { - FromIterator::from_iter(args.iter().cloned()) - } - /// Convert `DiagnosticMessage`s to a string, performing translation if necessary. fn translate_messages( &self, diff --git a/src/librustdoc/passes/check_code_block_syntax.rs b/src/librustdoc/passes/check_code_block_syntax.rs index 14a38a760d238..2e651b5387419 100644 --- a/src/librustdoc/passes/check_code_block_syntax.rs +++ b/src/librustdoc/passes/check_code_block_syntax.rs @@ -1,8 +1,9 @@ //! Validates syntax inside Rust code blocks (\`\`\`rust). use rustc_data_structures::sync::{Lock, Lrc}; use rustc_errors::{ - emitter::Emitter, translation::Translate, Applicability, Diagnostic, Handler, - LazyFallbackBundle, + emitter::Emitter, + translation::{to_fluent_args, Translate}, + Applicability, Diagnostic, Handler, LazyFallbackBundle, }; use rustc_parse::parse_stream_from_source_str; use rustc_session::parse::ParseSess; @@ -193,7 +194,7 @@ impl Emitter for BufferEmitter { fn emit_diagnostic(&mut self, diag: &Diagnostic) { let mut buffer = self.buffer.borrow_mut(); - let fluent_args = self.to_fluent_args(diag.args()); + let fluent_args = to_fluent_args(diag.args()); let translated_main_message = self.translate_message(&diag.message[0].0, &fluent_args); buffer.messages.push(format!("error from rustc: {}", translated_main_message)); From b4ac26289f17a5779d4318fb63436d94aebbf5ea Mon Sep 17 00:00:00 2001 From: David Wood Date: Mon, 3 Oct 2022 14:09:05 +0100 Subject: [PATCH 18/27] errors: `AddToDiagnostic::add_to_diagnostic_with` `AddToDiagnostic::add_to_diagnostic_with` is similar to the previous `AddToDiagnostic::add_to_diagnostic` but takes a function that can be used by the caller to modify diagnostic messages originating from the subdiagnostic (such as performing translation eagerly). `add_to_diagnostic` now just calls `add_to_diagnostic_with` with an empty closure. Signed-off-by: David Wood --- compiler/rustc_ast_lowering/src/errors.rs | 15 +++++-- compiler/rustc_ast_passes/src/errors.rs | 12 +++-- compiler/rustc_errors/src/diagnostic.rs | 19 ++++++-- compiler/rustc_infer/src/errors/mod.rs | 33 +++++++++++--- .../src/errors/note_and_explain.rs | 9 +++- compiler/rustc_lint/src/errors.rs | 15 +++++-- compiler/rustc_macros/src/diagnostics/mod.rs | 4 +- .../src/diagnostics/subdiagnostic.rs | 45 ++++++++++++------- compiler/rustc_query_system/src/error.rs | 7 ++- .../ui-fulldeps/internal-lints/diagnostics.rs | 12 +++-- .../internal-lints/diagnostics.stderr | 8 ++-- 11 files changed, 129 insertions(+), 50 deletions(-) diff --git a/compiler/rustc_ast_lowering/src/errors.rs b/compiler/rustc_ast_lowering/src/errors.rs index 63ff64b00bed6..c6c85ffa84dd7 100644 --- a/compiler/rustc_ast_lowering/src/errors.rs +++ b/compiler/rustc_ast_lowering/src/errors.rs @@ -1,4 +1,7 @@ -use rustc_errors::{fluent, AddToDiagnostic, Applicability, Diagnostic, DiagnosticArgFromDisplay}; +use rustc_errors::{ + fluent, AddToDiagnostic, Applicability, Diagnostic, DiagnosticArgFromDisplay, + SubdiagnosticMessage, +}; use rustc_macros::{Diagnostic, Subdiagnostic}; use rustc_span::{symbol::Ident, Span, Symbol}; @@ -19,7 +22,10 @@ pub struct UseAngleBrackets { } impl AddToDiagnostic for UseAngleBrackets { - fn add_to_diagnostic(self, diag: &mut Diagnostic) { + fn add_to_diagnostic_with(self, diag: &mut Diagnostic, _: F) + where + F: Fn(&mut Diagnostic, SubdiagnosticMessage) -> SubdiagnosticMessage, + { diag.multipart_suggestion( fluent::ast_lowering::use_angle_brackets, vec![(self.open_param, String::from("<")), (self.close_param, String::from(">"))], @@ -69,7 +75,10 @@ pub enum AssocTyParenthesesSub { } impl AddToDiagnostic for AssocTyParenthesesSub { - fn add_to_diagnostic(self, diag: &mut Diagnostic) { + fn add_to_diagnostic_with(self, diag: &mut Diagnostic, _: F) + where + F: Fn(&mut Diagnostic, SubdiagnosticMessage) -> SubdiagnosticMessage, + { match self { Self::Empty { parentheses_span } => diag.multipart_suggestion( fluent::ast_lowering::remove_parentheses, diff --git a/compiler/rustc_ast_passes/src/errors.rs b/compiler/rustc_ast_passes/src/errors.rs index 035f0ce1cbc42..ba2ed24fc08fc 100644 --- a/compiler/rustc_ast_passes/src/errors.rs +++ b/compiler/rustc_ast_passes/src/errors.rs @@ -1,6 +1,6 @@ //! Errors emitted by ast_passes. -use rustc_errors::{fluent, AddToDiagnostic, Applicability, Diagnostic}; +use rustc_errors::{fluent, AddToDiagnostic, Applicability, Diagnostic, SubdiagnosticMessage}; use rustc_macros::{Diagnostic, Subdiagnostic}; use rustc_span::{Span, Symbol}; @@ -17,7 +17,10 @@ pub struct ForbiddenLet { } impl AddToDiagnostic for ForbiddenLetReason { - fn add_to_diagnostic(self, diag: &mut Diagnostic) { + fn add_to_diagnostic_with(self, diag: &mut Diagnostic, _: F) + where + F: Fn(&mut Diagnostic, SubdiagnosticMessage) -> SubdiagnosticMessage, + { match self { Self::GenericForbidden => {} Self::NotSupportedOr(span) => { @@ -228,7 +231,10 @@ pub struct ExternBlockSuggestion { } impl AddToDiagnostic for ExternBlockSuggestion { - fn add_to_diagnostic(self, diag: &mut Diagnostic) { + fn add_to_diagnostic_with(self, diag: &mut Diagnostic, _: F) + where + F: Fn(&mut Diagnostic, SubdiagnosticMessage) -> SubdiagnosticMessage, + { let start_suggestion = if let Some(abi) = self.abi { format!("extern \"{}\" {{", abi) } else { diff --git a/compiler/rustc_errors/src/diagnostic.rs b/compiler/rustc_errors/src/diagnostic.rs index a267bf6c0b4d1..2a6dd6da415cf 100644 --- a/compiler/rustc_errors/src/diagnostic.rs +++ b/compiler/rustc_errors/src/diagnostic.rs @@ -203,9 +203,20 @@ impl IntoDiagnosticArg for ast::token::TokenKind { /// `#[derive(Subdiagnostic)]` -- see [rustc_macros::Subdiagnostic]. #[cfg_attr(bootstrap, rustc_diagnostic_item = "AddSubdiagnostic")] #[cfg_attr(not(bootstrap), rustc_diagnostic_item = "AddToDiagnostic")] -pub trait AddToDiagnostic { +pub trait AddToDiagnostic +where + Self: Sized, +{ /// Add a subdiagnostic to an existing diagnostic. - fn add_to_diagnostic(self, diag: &mut Diagnostic); + fn add_to_diagnostic(self, diag: &mut Diagnostic) { + self.add_to_diagnostic_with(diag, |_, m| m); + } + + /// Add a subdiagnostic to an existing diagnostic where `f` is invoked on every message used + /// (to optionally perform eager translation). + fn add_to_diagnostic_with(self, diag: &mut Diagnostic, f: F) + where + F: Fn(&mut Diagnostic, SubdiagnosticMessage) -> SubdiagnosticMessage; } /// Trait implemented by lint types. This should not be implemented manually. Instead, use @@ -921,8 +932,8 @@ impl Diagnostic { self } - /// Add a subdiagnostic from a type that implements `Subdiagnostic` - see - /// [rustc_macros::Subdiagnostic]. + /// Add a subdiagnostic from a type that implements `Subdiagnostic` (see + /// [rustc_macros::Subdiagnostic]). pub fn subdiagnostic(&mut self, subdiagnostic: impl AddToDiagnostic) -> &mut Self { subdiagnostic.add_to_diagnostic(self); self diff --git a/compiler/rustc_infer/src/errors/mod.rs b/compiler/rustc_infer/src/errors/mod.rs index 85b877652c6aa..500900d3d4a74 100644 --- a/compiler/rustc_infer/src/errors/mod.rs +++ b/compiler/rustc_infer/src/errors/mod.rs @@ -1,6 +1,7 @@ use hir::GenericParamKind; use rustc_errors::{ - fluent, AddToDiagnostic, Applicability, DiagnosticMessage, DiagnosticStyledString, MultiSpan, + fluent, AddToDiagnostic, Applicability, Diagnostic, DiagnosticMessage, DiagnosticStyledString, + MultiSpan, SubdiagnosticMessage, }; use rustc_hir as hir; use rustc_hir::{FnRetTy, Ty}; @@ -229,7 +230,10 @@ pub enum RegionOriginNote<'a> { } impl AddToDiagnostic for RegionOriginNote<'_> { - fn add_to_diagnostic(self, diag: &mut rustc_errors::Diagnostic) { + fn add_to_diagnostic_with(self, diag: &mut Diagnostic, _: F) + where + F: Fn(&mut Diagnostic, SubdiagnosticMessage) -> SubdiagnosticMessage, + { let mut label_or_note = |span, msg: DiagnosticMessage| { let sub_count = diag.children.iter().filter(|d| d.span.is_dummy()).count(); let expanded_sub_count = diag.children.iter().filter(|d| !d.span.is_dummy()).count(); @@ -290,7 +294,10 @@ pub enum LifetimeMismatchLabels { } impl AddToDiagnostic for LifetimeMismatchLabels { - fn add_to_diagnostic(self, diag: &mut rustc_errors::Diagnostic) { + fn add_to_diagnostic_with(self, diag: &mut Diagnostic, _: F) + where + F: Fn(&mut Diagnostic, SubdiagnosticMessage) -> SubdiagnosticMessage, + { match self { LifetimeMismatchLabels::InRet { param_span, ret_span, span, label_var1 } => { diag.span_label(param_span, fluent::infer::declared_different); @@ -340,7 +347,10 @@ pub struct AddLifetimeParamsSuggestion<'a> { } impl AddToDiagnostic for AddLifetimeParamsSuggestion<'_> { - fn add_to_diagnostic(self, diag: &mut rustc_errors::Diagnostic) { + fn add_to_diagnostic_with(self, diag: &mut Diagnostic, _: F) + where + F: Fn(&mut Diagnostic, SubdiagnosticMessage) -> SubdiagnosticMessage, + { let mut mk_suggestion = || { let ( hir::Ty { kind: hir::TyKind::Rptr(lifetime_sub, _), .. }, @@ -439,7 +449,10 @@ pub struct IntroducesStaticBecauseUnmetLifetimeReq { } impl AddToDiagnostic for IntroducesStaticBecauseUnmetLifetimeReq { - fn add_to_diagnostic(mut self, diag: &mut rustc_errors::Diagnostic) { + fn add_to_diagnostic_with(mut self, diag: &mut Diagnostic, _: F) + where + F: Fn(&mut Diagnostic, SubdiagnosticMessage) -> SubdiagnosticMessage, + { self.unmet_requirements .push_span_label(self.binding_span, fluent::infer::msl_introduces_static); diag.span_note(self.unmet_requirements, fluent::infer::msl_unmet_req); @@ -451,7 +464,10 @@ pub struct ImplNote { } impl AddToDiagnostic for ImplNote { - fn add_to_diagnostic(self, diag: &mut rustc_errors::Diagnostic) { + fn add_to_diagnostic_with(self, diag: &mut Diagnostic, _: F) + where + F: Fn(&mut Diagnostic, SubdiagnosticMessage) -> SubdiagnosticMessage, + { match self.impl_span { Some(span) => diag.span_note(span, fluent::infer::msl_impl_note), None => diag.note(fluent::infer::msl_impl_note), @@ -466,7 +482,10 @@ pub enum TraitSubdiag { // FIXME(#100717) used in `Vec` so requires eager translation/list support impl AddToDiagnostic for TraitSubdiag { - fn add_to_diagnostic(self, diag: &mut rustc_errors::Diagnostic) { + fn add_to_diagnostic_with(self, diag: &mut Diagnostic, _: F) + where + F: Fn(&mut Diagnostic, SubdiagnosticMessage) -> SubdiagnosticMessage, + { match self { TraitSubdiag::Note { span } => { diag.span_note(span, "this has an implicit `'static` lifetime requirement"); diff --git a/compiler/rustc_infer/src/errors/note_and_explain.rs b/compiler/rustc_infer/src/errors/note_and_explain.rs index 7f54918f73614..201a3c7100cc8 100644 --- a/compiler/rustc_infer/src/errors/note_and_explain.rs +++ b/compiler/rustc_infer/src/errors/note_and_explain.rs @@ -1,5 +1,7 @@ use crate::infer::error_reporting::nice_region_error::find_anon_type; -use rustc_errors::{self, fluent, AddToDiagnostic, IntoDiagnosticArg}; +use rustc_errors::{ + self, fluent, AddToDiagnostic, Diagnostic, IntoDiagnosticArg, SubdiagnosticMessage, +}; use rustc_middle::ty::{self, TyCtxt}; use rustc_span::{symbol::kw, Span}; @@ -159,7 +161,10 @@ impl RegionExplanation<'_> { } impl AddToDiagnostic for RegionExplanation<'_> { - fn add_to_diagnostic(self, diag: &mut rustc_errors::Diagnostic) { + fn add_to_diagnostic_with(self, diag: &mut Diagnostic, _: F) + where + F: Fn(&mut Diagnostic, SubdiagnosticMessage) -> SubdiagnosticMessage, + { if let Some(span) = self.desc.span { diag.span_note(span, fluent::infer::region_explanation); } else { diff --git a/compiler/rustc_lint/src/errors.rs b/compiler/rustc_lint/src/errors.rs index 880f3fbd00e60..97d012fb611d0 100644 --- a/compiler/rustc_lint/src/errors.rs +++ b/compiler/rustc_lint/src/errors.rs @@ -1,4 +1,7 @@ -use rustc_errors::{fluent, AddToDiagnostic, ErrorGuaranteed, Handler, IntoDiagnostic}; +use rustc_errors::{ + fluent, AddToDiagnostic, Diagnostic, ErrorGuaranteed, Handler, IntoDiagnostic, + SubdiagnosticMessage, +}; use rustc_macros::{Diagnostic, Subdiagnostic}; use rustc_session::lint::Level; use rustc_span::{Span, Symbol}; @@ -23,7 +26,10 @@ pub enum OverruledAttributeSub { } impl AddToDiagnostic for OverruledAttributeSub { - fn add_to_diagnostic(self, diag: &mut rustc_errors::Diagnostic) { + fn add_to_diagnostic_with(self, diag: &mut Diagnostic, _: F) + where + F: Fn(&mut Diagnostic, SubdiagnosticMessage) -> SubdiagnosticMessage, + { match self { OverruledAttributeSub::DefaultSource { id } => { diag.note(fluent::lint::default_source); @@ -88,7 +94,10 @@ pub struct RequestedLevel { } impl AddToDiagnostic for RequestedLevel { - fn add_to_diagnostic(self, diag: &mut rustc_errors::Diagnostic) { + fn add_to_diagnostic_with(self, diag: &mut Diagnostic, _: F) + where + F: Fn(&mut Diagnostic, SubdiagnosticMessage) -> SubdiagnosticMessage, + { diag.note(fluent::lint::requested_level); diag.set_arg( "level", diff --git a/compiler/rustc_macros/src/diagnostics/mod.rs b/compiler/rustc_macros/src/diagnostics/mod.rs index 4166816b5e3c7..f98cc66e9e93e 100644 --- a/compiler/rustc_macros/src/diagnostics/mod.rs +++ b/compiler/rustc_macros/src/diagnostics/mod.rs @@ -9,7 +9,7 @@ use diagnostic::{DiagnosticDerive, LintDiagnosticDerive}; pub(crate) use fluent::fluent_messages; use proc_macro2::TokenStream; use quote::format_ident; -use subdiagnostic::SubdiagnosticDerive; +use subdiagnostic::SubdiagnosticDeriveBuilder; use synstructure::Structure; /// Implements `#[derive(Diagnostic)]`, which allows for errors to be specified as a struct, @@ -155,5 +155,5 @@ pub fn lint_diagnostic_derive(s: Structure<'_>) -> TokenStream { /// diag.subdiagnostic(RawIdentifierSuggestion { span, applicability, ident }); /// ``` pub fn session_subdiagnostic_derive(s: Structure<'_>) -> TokenStream { - SubdiagnosticDerive::new(s).into_tokens() + SubdiagnosticDeriveBuilder::new().into_tokens(s) } diff --git a/compiler/rustc_macros/src/diagnostics/subdiagnostic.rs b/compiler/rustc_macros/src/diagnostics/subdiagnostic.rs index 9a2588513f06d..ef17dbd0426fe 100644 --- a/compiler/rustc_macros/src/diagnostics/subdiagnostic.rs +++ b/compiler/rustc_macros/src/diagnostics/subdiagnostic.rs @@ -15,19 +15,19 @@ use syn::{spanned::Spanned, Attribute, Meta, MetaList, MetaNameValue, NestedMeta use synstructure::{BindingInfo, Structure, VariantInfo}; /// The central struct for constructing the `add_to_diagnostic` method from an annotated struct. -pub(crate) struct SubdiagnosticDerive<'a> { - structure: Structure<'a>, +pub(crate) struct SubdiagnosticDeriveBuilder { diag: syn::Ident, + f: syn::Ident, } -impl<'a> SubdiagnosticDerive<'a> { - pub(crate) fn new(structure: Structure<'a>) -> Self { +impl SubdiagnosticDeriveBuilder { + pub(crate) fn new() -> Self { let diag = format_ident!("diag"); - Self { structure, diag } + let f = format_ident!("f"); + Self { diag, f } } - pub(crate) fn into_tokens(self) -> TokenStream { - let SubdiagnosticDerive { mut structure, diag } = self; + pub(crate) fn into_tokens<'a>(self, mut structure: Structure<'a>) -> TokenStream { let implementation = { let ast = structure.ast(); let span = ast.span().unwrap(); @@ -53,8 +53,8 @@ impl<'a> SubdiagnosticDerive<'a> { structure.bind_with(|_| synstructure::BindStyle::Move); let variants_ = structure.each_variant(|variant| { - let mut builder = SubdiagnosticDeriveBuilder { - diag: &diag, + let mut builder = SubdiagnosticDeriveVariantBuilder { + parent: &self, variant, span, fields: build_field_mapping(variant), @@ -72,9 +72,17 @@ impl<'a> SubdiagnosticDerive<'a> { } }; + let diag = &self.diag; + let f = &self.f; let ret = structure.gen_impl(quote! { gen impl rustc_errors::AddToDiagnostic for @Self { - fn add_to_diagnostic(self, #diag: &mut rustc_errors::Diagnostic) { + fn add_to_diagnostic_with<__F>(self, #diag: &mut rustc_errors::Diagnostic, #f: __F) + where + __F: Fn( + &mut rustc_errors::Diagnostic, + rustc_errors::SubdiagnosticMessage + ) -> rustc_errors::SubdiagnosticMessage, + { use rustc_errors::{Applicability, IntoDiagnosticArg}; #implementation } @@ -88,9 +96,9 @@ impl<'a> SubdiagnosticDerive<'a> { /// for the final generated method. This is a separate struct to `SubdiagnosticDerive` /// only to be able to destructure and split `self.builder` and the `self.structure` up to avoid a /// double mut borrow later on. -struct SubdiagnosticDeriveBuilder<'a> { +struct SubdiagnosticDeriveVariantBuilder<'parent, 'a> { /// The identifier to use for the generated `DiagnosticBuilder` instance. - diag: &'a syn::Ident, + parent: &'parent SubdiagnosticDeriveBuilder, /// Info for the current variant (or the type if not an enum). variant: &'a VariantInfo<'a>, @@ -112,7 +120,7 @@ struct SubdiagnosticDeriveBuilder<'a> { has_suggestion_parts: bool, } -impl<'a> HasFieldMap for SubdiagnosticDeriveBuilder<'a> { +impl<'parent, 'a> HasFieldMap for SubdiagnosticDeriveVariantBuilder<'parent, 'a> { fn get_field_binding(&self, field: &String) -> Option<&TokenStream> { self.fields.get(field) } @@ -156,7 +164,7 @@ impl<'a> FromIterator<&'a SubdiagnosticKind> for KindsStatistics { } } -impl<'a> SubdiagnosticDeriveBuilder<'a> { +impl<'parent, 'a> SubdiagnosticDeriveVariantBuilder<'parent, 'a> { fn identify_kind(&mut self) -> Result, DiagnosticDeriveError> { let mut kind_slugs = vec![]; @@ -187,7 +195,7 @@ impl<'a> SubdiagnosticDeriveBuilder<'a> { let ast = binding.ast(); assert_eq!(ast.attrs.len(), 0, "field with attribute used as diagnostic arg"); - let diag = &self.diag; + let diag = &self.parent.diag; let ident = ast.ident.as_ref().unwrap(); // strip `r#` prefix, if present let ident = format_ident!("{}", ident); @@ -442,11 +450,14 @@ impl<'a> SubdiagnosticDeriveBuilder<'a> { let span_field = self.span_field.value_ref(); - let diag = &self.diag; + let diag = &self.parent.diag; + let f = &self.parent.f; let mut calls = TokenStream::new(); for (kind, slug) in kind_slugs { + let message = format_ident!("__message"); + calls.extend(quote! { let #message = #f(#diag, rustc_errors::fluent::#slug.into()); }); + let name = format_ident!("{}{}", if span_field.is_some() { "span_" } else { "" }, kind); - let message = quote! { rustc_errors::fluent::#slug }; let call = match kind { SubdiagnosticKind::Suggestion { suggestion_kind, applicability, code } => { let applicability = applicability diff --git a/compiler/rustc_query_system/src/error.rs b/compiler/rustc_query_system/src/error.rs index 8602a4cf5aef2..debdf9dbf4403 100644 --- a/compiler/rustc_query_system/src/error.rs +++ b/compiler/rustc_query_system/src/error.rs @@ -1,4 +1,4 @@ -use rustc_errors::AddToDiagnostic; +use rustc_errors::{AddToDiagnostic, Diagnostic, SubdiagnosticMessage}; use rustc_macros::{Diagnostic, Subdiagnostic}; use rustc_session::Limit; use rustc_span::{Span, Symbol}; @@ -9,7 +9,10 @@ pub struct CycleStack { } impl AddToDiagnostic for CycleStack { - fn add_to_diagnostic(self, diag: &mut rustc_errors::Diagnostic) { + fn add_to_diagnostic_with(self, diag: &mut Diagnostic, _: F) + where + F: Fn(&mut Diagnostic, SubdiagnosticMessage) -> SubdiagnosticMessage, + { diag.span_note(self.span, &format!("...which requires {}...", self.desc)); } } diff --git a/src/test/ui-fulldeps/internal-lints/diagnostics.rs b/src/test/ui-fulldeps/internal-lints/diagnostics.rs index 9a5100ce17f53..462f5e7849849 100644 --- a/src/test/ui-fulldeps/internal-lints/diagnostics.rs +++ b/src/test/ui-fulldeps/internal-lints/diagnostics.rs @@ -13,7 +13,7 @@ extern crate rustc_span; use rustc_errors::{ AddToDiagnostic, IntoDiagnostic, Diagnostic, DiagnosticBuilder, - ErrorGuaranteed, Handler, fluent + ErrorGuaranteed, Handler, fluent, SubdiagnosticMessage, }; use rustc_macros::{Diagnostic, Subdiagnostic}; use rustc_span::Span; @@ -52,7 +52,10 @@ impl<'a> IntoDiagnostic<'a, ErrorGuaranteed> for TranslatableInIntoDiagnostic { pub struct UntranslatableInAddToDiagnostic; impl AddToDiagnostic for UntranslatableInAddToDiagnostic { - fn add_to_diagnostic(self, diag: &mut Diagnostic) { + fn add_to_diagnostic_with(self, diag: &mut Diagnostic, _: F) + where + F: Fn(&mut Diagnostic, SubdiagnosticMessage) -> SubdiagnosticMessage, + { diag.note("untranslatable diagnostic"); //~^ ERROR diagnostics should be created using translatable messages } @@ -61,7 +64,10 @@ impl AddToDiagnostic for UntranslatableInAddToDiagnostic { pub struct TranslatableInAddToDiagnostic; impl AddToDiagnostic for TranslatableInAddToDiagnostic { - fn add_to_diagnostic(self, diag: &mut Diagnostic) { + fn add_to_diagnostic_with(self, diag: &mut Diagnostic, _: F) + where + F: Fn(&mut Diagnostic, SubdiagnosticMessage) -> SubdiagnosticMessage, + { diag.note(fluent::compiletest::note); } } diff --git a/src/test/ui-fulldeps/internal-lints/diagnostics.stderr b/src/test/ui-fulldeps/internal-lints/diagnostics.stderr index f5f92ac1e7fbe..ac820a79db274 100644 --- a/src/test/ui-fulldeps/internal-lints/diagnostics.stderr +++ b/src/test/ui-fulldeps/internal-lints/diagnostics.stderr @@ -11,13 +11,13 @@ LL | #![deny(rustc::untranslatable_diagnostic)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: diagnostics should be created using translatable messages - --> $DIR/diagnostics.rs:56:14 + --> $DIR/diagnostics.rs:59:14 | LL | diag.note("untranslatable diagnostic"); | ^^^^ error: diagnostics should only be created in `IntoDiagnostic`/`AddToDiagnostic` impls - --> $DIR/diagnostics.rs:70:25 + --> $DIR/diagnostics.rs:76:25 | LL | let _diag = handler.struct_err(fluent::compiletest::example); | ^^^^^^^^^^ @@ -29,13 +29,13 @@ LL | #![deny(rustc::diagnostic_outside_of_impl)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: diagnostics should only be created in `IntoDiagnostic`/`AddToDiagnostic` impls - --> $DIR/diagnostics.rs:73:25 + --> $DIR/diagnostics.rs:79:25 | LL | let _diag = handler.struct_err("untranslatable diagnostic"); | ^^^^^^^^^^ error: diagnostics should be created using translatable messages - --> $DIR/diagnostics.rs:73:25 + --> $DIR/diagnostics.rs:79:25 | LL | let _diag = handler.struct_err("untranslatable diagnostic"); | ^^^^^^^^^^ From 540b203bf9fe05e572f1baa938317d4c10df3528 Mon Sep 17 00:00:00 2001 From: David Wood Date: Mon, 3 Oct 2022 14:14:51 +0100 Subject: [PATCH 19/27] errors: `DiagnosticMessage::Eager` Add variant of `DiagnosticMessage` for eagerly translated messages (messages in the target language which don't need translated by the emitter during emission). Also adds `eager_subdiagnostic` function which is intended to be invoked by the diagnostic derive for subdiagnostic fields which are marked as needing eager translation. Signed-off-by: David Wood --- compiler/rustc_error_messages/src/lib.rs | 29 +++++++++++++++++++++++- compiler/rustc_errors/src/diagnostic.rs | 19 +++++++++++++++- compiler/rustc_errors/src/lib.rs | 11 +++++++++ compiler/rustc_errors/src/translation.rs | 4 +++- 4 files changed, 60 insertions(+), 3 deletions(-) diff --git a/compiler/rustc_error_messages/src/lib.rs b/compiler/rustc_error_messages/src/lib.rs index 18be60975e4eb..560d6c7c69506 100644 --- a/compiler/rustc_error_messages/src/lib.rs +++ b/compiler/rustc_error_messages/src/lib.rs @@ -276,6 +276,18 @@ pub enum SubdiagnosticMessage { /// Non-translatable diagnostic message. // FIXME(davidtwco): can a `Cow<'static, str>` be used here? Str(String), + /// Translatable message which has already been translated eagerly. + /// + /// Some diagnostics have repeated subdiagnostics where the same interpolated variables would + /// be instantiated multiple times with different values. As translation normally happens + /// immediately prior to emission, after the diagnostic and subdiagnostic derive logic has run, + /// the setting of diagnostic arguments in the derived code will overwrite previous variable + /// values and only the final value will be set when translation occurs - resulting in + /// incorrect diagnostics. Eager translation results in translation for a subdiagnostic + /// happening immediately after the subdiagnostic derive's logic has been run. This variant + /// stores messages which have been translated eagerly. + // FIXME(#100717): can a `Cow<'static, str>` be used here? + Eager(String), /// Identifier of a Fluent message. Instances of this variant are generated by the /// `Subdiagnostic` derive. FluentIdentifier(FluentId), @@ -303,8 +315,20 @@ impl> From for SubdiagnosticMessage { #[rustc_diagnostic_item = "DiagnosticMessage"] pub enum DiagnosticMessage { /// Non-translatable diagnostic message. - // FIXME(davidtwco): can a `Cow<'static, str>` be used here? + // FIXME(#100717): can a `Cow<'static, str>` be used here? Str(String), + /// Translatable message which has already been translated eagerly. + /// + /// Some diagnostics have repeated subdiagnostics where the same interpolated variables would + /// be instantiated multiple times with different values. As translation normally happens + /// immediately prior to emission, after the diagnostic and subdiagnostic derive logic has run, + /// the setting of diagnostic arguments in the derived code will overwrite previous variable + /// values and only the final value will be set when translation occurs - resulting in + /// incorrect diagnostics. Eager translation results in translation for a subdiagnostic + /// happening immediately after the subdiagnostic derive's logic has been run. This variant + /// stores messages which have been translated eagerly. + // FIXME(#100717): can a `Cow<'static, str>` be used here? + Eager(String), /// Identifier for a Fluent message (with optional attribute) corresponding to the diagnostic /// message. /// @@ -323,6 +347,7 @@ impl DiagnosticMessage { pub fn with_subdiagnostic_message(&self, sub: SubdiagnosticMessage) -> Self { let attr = match sub { SubdiagnosticMessage::Str(s) => return DiagnosticMessage::Str(s), + SubdiagnosticMessage::Eager(s) => return DiagnosticMessage::Eager(s), SubdiagnosticMessage::FluentIdentifier(id) => { return DiagnosticMessage::FluentIdentifier(id, None); } @@ -331,6 +356,7 @@ impl DiagnosticMessage { match self { DiagnosticMessage::Str(s) => DiagnosticMessage::Str(s.clone()), + DiagnosticMessage::Eager(s) => DiagnosticMessage::Eager(s.clone()), DiagnosticMessage::FluentIdentifier(id, _) => { DiagnosticMessage::FluentIdentifier(id.clone(), Some(attr)) } @@ -379,6 +405,7 @@ impl Into for DiagnosticMessage { fn into(self) -> SubdiagnosticMessage { match self { DiagnosticMessage::Str(s) => SubdiagnosticMessage::Str(s), + DiagnosticMessage::Eager(s) => SubdiagnosticMessage::Eager(s), DiagnosticMessage::FluentIdentifier(id, None) => { SubdiagnosticMessage::FluentIdentifier(id) } diff --git a/compiler/rustc_errors/src/diagnostic.rs b/compiler/rustc_errors/src/diagnostic.rs index 2a6dd6da415cf..3e0840caaa693 100644 --- a/compiler/rustc_errors/src/diagnostic.rs +++ b/compiler/rustc_errors/src/diagnostic.rs @@ -939,6 +939,23 @@ impl Diagnostic { self } + /// Add a subdiagnostic from a type that implements `Subdiagnostic` (see + /// [rustc_macros::Subdiagnostic]). Performs eager translation of any translatable messages + /// used in the subdiagnostic, so suitable for use with repeated messages (i.e. re-use of + /// interpolated variables). + pub fn eager_subdiagnostic( + &mut self, + handler: &crate::Handler, + subdiagnostic: impl AddToDiagnostic, + ) -> &mut Self { + subdiagnostic.add_to_diagnostic_with(self, |diag, msg| { + let args = diag.args(); + let msg = diag.subdiagnostic_message_to_diagnostic_message(msg); + handler.eagerly_translate(msg, args) + }); + self + } + pub fn set_span>(&mut self, sp: S) -> &mut Self { self.span = sp.into(); if let Some(span) = self.span.primary_span() { @@ -994,7 +1011,7 @@ impl Diagnostic { /// Helper function that takes a `SubdiagnosticMessage` and returns a `DiagnosticMessage` by /// combining it with the primary message of the diagnostic (if translatable, otherwise it just /// passes the user's string along). - fn subdiagnostic_message_to_diagnostic_message( + pub(crate) fn subdiagnostic_message_to_diagnostic_message( &self, attr: impl Into, ) -> DiagnosticMessage { diff --git a/compiler/rustc_errors/src/lib.rs b/compiler/rustc_errors/src/lib.rs index 94a493992e593..61c9abbb0d691 100644 --- a/compiler/rustc_errors/src/lib.rs +++ b/compiler/rustc_errors/src/lib.rs @@ -598,6 +598,17 @@ impl Handler { } } + /// Translate `message` eagerly with `args`. + pub fn eagerly_translate<'a>( + &self, + message: DiagnosticMessage, + args: impl Iterator>, + ) -> SubdiagnosticMessage { + let inner = self.inner.borrow(); + let args = crate::translation::to_fluent_args(args); + SubdiagnosticMessage::Eager(inner.emitter.translate_message(&message, &args).to_string()) + } + // This is here to not allow mutation of flags; // as of this writing it's only used in tests in librustc_middle. pub fn can_emit_warnings(&self) -> bool { diff --git a/compiler/rustc_errors/src/translation.rs b/compiler/rustc_errors/src/translation.rs index c2ec2526a4aea..a7737b467b75b 100644 --- a/compiler/rustc_errors/src/translation.rs +++ b/compiler/rustc_errors/src/translation.rs @@ -55,7 +55,9 @@ pub trait Translate { ) -> Cow<'_, str> { trace!(?message, ?args); let (identifier, attr) = match message { - DiagnosticMessage::Str(msg) => return Cow::Borrowed(&msg), + DiagnosticMessage::Str(msg) | DiagnosticMessage::Eager(msg) => { + return Cow::Borrowed(&msg); + } DiagnosticMessage::FluentIdentifier(identifier, attr) => (identifier, attr), }; From 291a4736d9300a9be79a5f105c54a06a2a4f1d1b Mon Sep 17 00:00:00 2001 From: David Wood Date: Mon, 3 Oct 2022 14:24:17 +0100 Subject: [PATCH 20/27] macros: `#[subdiagnostic(eager)]` Add support for `eager` argument to the `subdiagnostic` attribute which generates a call to `eager_subdiagnostic`. Signed-off-by: David Wood --- .../src/diagnostics/diagnostic.rs | 13 ++-- .../src/diagnostics/diagnostic_builder.rs | 71 ++++++++++++++----- .../session-diagnostic/diagnostic-derive.rs | 47 ++++++++++++ .../diagnostic-derive.stderr | 42 ++++++++++- 4 files changed, 151 insertions(+), 22 deletions(-) diff --git a/compiler/rustc_macros/src/diagnostics/diagnostic.rs b/compiler/rustc_macros/src/diagnostics/diagnostic.rs index 406a56cd4ae3e..84c57b3f64e18 100644 --- a/compiler/rustc_macros/src/diagnostics/diagnostic.rs +++ b/compiler/rustc_macros/src/diagnostics/diagnostic.rs @@ -10,27 +10,31 @@ use synstructure::Structure; /// The central struct for constructing the `into_diagnostic` method from an annotated struct. pub(crate) struct DiagnosticDerive<'a> { structure: Structure<'a>, - handler: syn::Ident, builder: DiagnosticDeriveBuilder, } impl<'a> DiagnosticDerive<'a> { pub(crate) fn new(diag: syn::Ident, handler: syn::Ident, structure: Structure<'a>) -> Self { Self { - builder: DiagnosticDeriveBuilder { diag, kind: DiagnosticDeriveKind::Diagnostic }, - handler, + builder: DiagnosticDeriveBuilder { + diag, + kind: DiagnosticDeriveKind::Diagnostic { handler }, + }, structure, } } pub(crate) fn into_tokens(self) -> TokenStream { - let DiagnosticDerive { mut structure, handler, mut builder } = self; + let DiagnosticDerive { mut structure, mut builder } = self; let implementation = builder.each_variant(&mut structure, |mut builder, variant| { let preamble = builder.preamble(&variant); let body = builder.body(&variant); let diag = &builder.parent.diag; + let DiagnosticDeriveKind::Diagnostic { handler } = &builder.parent.kind else { + unreachable!() + }; let init = match builder.slug.value_ref() { None => { span_err(builder.span, "diagnostic slug not specified") @@ -56,6 +60,7 @@ impl<'a> DiagnosticDerive<'a> { } }); + let DiagnosticDeriveKind::Diagnostic { handler } = &builder.kind else { unreachable!() }; structure.gen_impl(quote! { gen impl<'__diagnostic_handler_sess, G> rustc_errors::IntoDiagnostic<'__diagnostic_handler_sess, G> diff --git a/compiler/rustc_macros/src/diagnostics/diagnostic_builder.rs b/compiler/rustc_macros/src/diagnostics/diagnostic_builder.rs index 9e88dc7a913a2..df4e309086fd2 100644 --- a/compiler/rustc_macros/src/diagnostics/diagnostic_builder.rs +++ b/compiler/rustc_macros/src/diagnostics/diagnostic_builder.rs @@ -17,9 +17,9 @@ use syn::{ use synstructure::{BindingInfo, Structure, VariantInfo}; /// What kind of diagnostic is being derived - a fatal/error/warning or a lint? -#[derive(Copy, Clone, PartialEq, Eq)] +#[derive(Clone, PartialEq, Eq)] pub(crate) enum DiagnosticDeriveKind { - Diagnostic, + Diagnostic { handler: syn::Ident }, LintDiagnostic, } @@ -340,18 +340,15 @@ impl<'a> DiagnosticDeriveVariantBuilder<'a> { let diag = &self.parent.diag; let meta = attr.parse_meta()?; - if let Meta::Path(_) = meta { - let ident = &attr.path.segments.last().unwrap().ident; - let name = ident.to_string(); - let name = name.as_str(); - match name { - "skip_arg" => { - // Don't need to do anything - by virtue of the attribute existing, the - // `set_arg` call will not be generated. - return Ok(quote! {}); - } - "primary_span" => match self.parent.kind { - DiagnosticDeriveKind::Diagnostic => { + let ident = &attr.path.segments.last().unwrap().ident; + let name = ident.to_string(); + match (&meta, name.as_str()) { + // Don't need to do anything - by virtue of the attribute existing, the + // `set_arg` call will not be generated. + (Meta::Path(_), "skip_arg") => return Ok(quote! {}), + (Meta::Path(_), "primary_span") => { + match self.parent.kind { + DiagnosticDeriveKind::Diagnostic { .. } => { report_error_if_not_applied_to_span(attr, &info)?; return Ok(quote! { @@ -363,10 +360,50 @@ impl<'a> DiagnosticDeriveVariantBuilder<'a> { diag.help("the `primary_span` field attribute is not valid for lint diagnostics") }) } - }, - "subdiagnostic" => return Ok(quote! { #diag.subdiagnostic(#binding); }), - _ => {} + } + } + (Meta::Path(_), "subdiagnostic") => { + return Ok(quote! { #diag.subdiagnostic(#binding); }); + } + (Meta::NameValue(_), "subdiagnostic") => { + throw_invalid_attr!(attr, &meta, |diag| { + diag.help("`eager` is the only supported nested attribute for `subdiagnostic`") + }) + } + (Meta::List(MetaList { ref nested, .. }), "subdiagnostic") => { + if nested.len() != 1 { + throw_invalid_attr!(attr, &meta, |diag| { + diag.help( + "`eager` is the only supported nested attribute for `subdiagnostic`", + ) + }) + } + + let handler = match &self.parent.kind { + DiagnosticDeriveKind::Diagnostic { handler } => handler, + DiagnosticDeriveKind::LintDiagnostic => { + throw_invalid_attr!(attr, &meta, |diag| { + diag.help("eager subdiagnostics are not supported on lints") + }) + } + }; + + let nested_attr = nested.first().expect("pop failed for single element list"); + match nested_attr { + NestedMeta::Meta(meta @ Meta::Path(_)) + if meta.path().segments.last().unwrap().ident.to_string().as_str() + == "eager" => + { + return Ok(quote! { #diag.eager_subdiagnostic(#handler, #binding); }); + } + _ => { + throw_invalid_nested_attr!(attr, nested_attr, |diag| { + diag.help("`eager` is the only supported nested attribute for `subdiagnostic`") + }) + } + } } + _ => (), } let (subdiag, slug) = self.parse_subdiag_attribute(attr)?; diff --git a/src/test/ui-fulldeps/session-diagnostic/diagnostic-derive.rs b/src/test/ui-fulldeps/session-diagnostic/diagnostic-derive.rs index 1dc71abc104f9..460af07a55967 100644 --- a/src/test/ui-fulldeps/session-diagnostic/diagnostic-derive.rs +++ b/src/test/ui-fulldeps/session-diagnostic/diagnostic-derive.rs @@ -678,3 +678,50 @@ enum ExampleEnum { struct RawIdentDiagnosticArg { pub r#type: String, } + +#[derive(Diagnostic)] +#[diag(compiletest::example)] +struct SubdiagnosticBad { + #[subdiagnostic(bad)] +//~^ ERROR `#[subdiagnostic(bad)]` is not a valid attribute + note: Note, +} + +#[derive(Diagnostic)] +#[diag(compiletest::example)] +struct SubdiagnosticBadStr { + #[subdiagnostic = "bad"] +//~^ ERROR `#[subdiagnostic = ...]` is not a valid attribute + note: Note, +} + +#[derive(Diagnostic)] +#[diag(compiletest::example)] +struct SubdiagnosticBadTwice { + #[subdiagnostic(bad, bad)] +//~^ ERROR `#[subdiagnostic(...)]` is not a valid attribute + note: Note, +} + +#[derive(Diagnostic)] +#[diag(compiletest::example)] +struct SubdiagnosticBadLitStr { + #[subdiagnostic("bad")] +//~^ ERROR `#[subdiagnostic("...")]` is not a valid attribute + note: Note, +} + +#[derive(LintDiagnostic)] +#[diag(compiletest::example)] +struct SubdiagnosticEagerLint { + #[subdiagnostic(eager)] +//~^ ERROR `#[subdiagnostic(...)]` is not a valid attribute + note: Note, +} + +#[derive(Diagnostic)] +#[diag(compiletest::example)] +struct SubdiagnosticEagerCorrect { + #[subdiagnostic(eager)] + note: Note, +} diff --git a/src/test/ui-fulldeps/session-diagnostic/diagnostic-derive.stderr b/src/test/ui-fulldeps/session-diagnostic/diagnostic-derive.stderr index 167198b47f20f..7a42d618707ad 100644 --- a/src/test/ui-fulldeps/session-diagnostic/diagnostic-derive.stderr +++ b/src/test/ui-fulldeps/session-diagnostic/diagnostic-derive.stderr @@ -533,6 +533,46 @@ LL | #[label] | = help: `#[label]` and `#[suggestion]` can only be applied to fields +error: `#[subdiagnostic(bad)]` is not a valid attribute + --> $DIR/diagnostic-derive.rs:685:21 + | +LL | #[subdiagnostic(bad)] + | ^^^ + | + = help: `eager` is the only supported nested attribute for `subdiagnostic` + +error: `#[subdiagnostic = ...]` is not a valid attribute + --> $DIR/diagnostic-derive.rs:693:5 + | +LL | #[subdiagnostic = "bad"] + | ^^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: `eager` is the only supported nested attribute for `subdiagnostic` + +error: `#[subdiagnostic(...)]` is not a valid attribute + --> $DIR/diagnostic-derive.rs:701:5 + | +LL | #[subdiagnostic(bad, bad)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: `eager` is the only supported nested attribute for `subdiagnostic` + +error: `#[subdiagnostic("...")]` is not a valid attribute + --> $DIR/diagnostic-derive.rs:709:21 + | +LL | #[subdiagnostic("bad")] + | ^^^^^ + | + = help: `eager` is the only supported nested attribute for `subdiagnostic` + +error: `#[subdiagnostic(...)]` is not a valid attribute + --> $DIR/diagnostic-derive.rs:717:5 + | +LL | #[subdiagnostic(eager)] + | ^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: eager subdiagnostics are not supported on lints + error: cannot find attribute `nonsense` in this scope --> $DIR/diagnostic-derive.rs:55:3 | @@ -607,7 +647,7 @@ LL | arg: impl IntoDiagnosticArg, | ^^^^^^^^^^^^^^^^^ required by this bound in `DiagnosticBuilder::<'a, G>::set_arg` = note: this error originates in the derive macro `Diagnostic` (in Nightly builds, run with -Z macro-backtrace for more info) -error: aborting due to 75 previous errors +error: aborting due to 80 previous errors Some errors have detailed explanations: E0277, E0425. For more information about an error, try `rustc --explain E0277`. From 113e94369cb72a98648c12c263475821bbff7d9c Mon Sep 17 00:00:00 2001 From: David Wood Date: Mon, 3 Oct 2022 14:26:29 +0100 Subject: [PATCH 21/27] query_system: finish migration Using eager translation, migrate the remaining repeated cycle stack diagnostic. Signed-off-by: David Wood --- .../locales/en-US/query_system.ftl | 2 ++ compiler/rustc_query_system/src/error.rs | 15 ++++----------- compiler/rustc_query_system/src/lib.rs | 2 +- 3 files changed, 7 insertions(+), 12 deletions(-) diff --git a/compiler/rustc_error_messages/locales/en-US/query_system.ftl b/compiler/rustc_error_messages/locales/en-US/query_system.ftl index b914ba52a7353..870e824039cb6 100644 --- a/compiler/rustc_error_messages/locales/en-US/query_system.ftl +++ b/compiler/rustc_error_messages/locales/en-US/query_system.ftl @@ -12,6 +12,8 @@ query_system_cycle_usage = cycle used when {$usage} query_system_cycle_stack_single = ...which immediately requires {$stack_bottom} again +query_system_cycle_stack_middle = ...which requires {$desc}... + query_system_cycle_stack_multiple = ...which again requires {$stack_bottom}, completing the cycle query_system_cycle_recursive_ty_alias = type aliases cannot be recursive diff --git a/compiler/rustc_query_system/src/error.rs b/compiler/rustc_query_system/src/error.rs index debdf9dbf4403..1e74e0e299099 100644 --- a/compiler/rustc_query_system/src/error.rs +++ b/compiler/rustc_query_system/src/error.rs @@ -1,22 +1,15 @@ -use rustc_errors::{AddToDiagnostic, Diagnostic, SubdiagnosticMessage}; use rustc_macros::{Diagnostic, Subdiagnostic}; use rustc_session::Limit; use rustc_span::{Span, Symbol}; +#[derive(Subdiagnostic)] +#[note(query_system::cycle_stack_middle)] pub struct CycleStack { + #[primary_span] pub span: Span, pub desc: String, } -impl AddToDiagnostic for CycleStack { - fn add_to_diagnostic_with(self, diag: &mut Diagnostic, _: F) - where - F: Fn(&mut Diagnostic, SubdiagnosticMessage) -> SubdiagnosticMessage, - { - diag.span_note(self.span, &format!("...which requires {}...", self.desc)); - } -} - #[derive(Copy, Clone)] pub enum HandleCycleError { Error, @@ -56,7 +49,7 @@ pub struct Cycle { #[primary_span] pub span: Span, pub stack_bottom: String, - #[subdiagnostic] + #[subdiagnostic(eager)] pub cycle_stack: Vec, #[subdiagnostic] pub stack_count: StackCount, diff --git a/compiler/rustc_query_system/src/lib.rs b/compiler/rustc_query_system/src/lib.rs index 8f6da73d1f2d2..f47760e9ae6c8 100644 --- a/compiler/rustc_query_system/src/lib.rs +++ b/compiler/rustc_query_system/src/lib.rs @@ -4,7 +4,7 @@ #![feature(min_specialization)] #![feature(extern_types)] #![allow(rustc::potential_query_instability)] -// #![deny(rustc::untranslatable_diagnostic)] +#![deny(rustc::untranslatable_diagnostic)] #![deny(rustc::diagnostic_outside_of_impl)] #[macro_use] From 7e20929e55c5c76bc1466568da746cea4171bc71 Mon Sep 17 00:00:00 2001 From: David Wood Date: Mon, 3 Oct 2022 14:28:02 +0100 Subject: [PATCH 22/27] macros: separate suggestion fmt'ing and emission Diagnostic derives have previously had to take special care when ordering the generated code so that fields were not used after a move. This is unlikely for most fields because a field is either annotated with a subdiagnostic attribute and is thus likely a `Span` and copiable, or is a argument, in which case it is only used once by `set_arg` anyway. However, format strings for code in suggestions can result in fields being used after being moved if not ordered carefully. As a result, the derive currently puts `set_arg` calls last (just before emission), such as: ```rust let diag = { /* create diagnostic */ }; diag.span_suggestion_with_style( span, fluent::crate::slug, format!("{}", __binding_0), Applicability::Unknown, SuggestionStyle::ShowAlways ); /* + other subdiagnostic additions */ diag.set_arg("foo", __binding_0); /* + other `set_arg` calls */ diag.emit(); ``` For eager translation, this doesn't work, as the message being translated eagerly can assume that all arguments are available - so arguments _must_ be set first. Format strings for suggestion code are now separated into two parts - an initialization line that performs the formatting into a variable, and a usage in the subdiagnostic addition. By separating these parts, the initialization can happen before arguments are set, preserving the desired order so that code compiles, while still enabling arguments to be set before subdiagnostics are added. ```rust let diag = { /* create diagnostic */ }; let __code_0 = format!("{}", __binding_0); /* + other formatting */ diag.set_arg("foo", __binding_0); /* + other `set_arg` calls */ diag.span_suggestion_with_style( span, fluent::crate::slug, __code_0, Applicability::Unknown, SuggestionStyle::ShowAlways ); /* + other subdiagnostic additions */ diag.emit(); ``` Signed-off-by: David Wood --- .../src/diagnostics/diagnostic_builder.rs | 6 ++- .../src/diagnostics/subdiagnostic.rs | 50 ++++++++++++++----- .../rustc_macros/src/diagnostics/utils.rs | 48 ++++++++++++++---- .../session-diagnostic/diagnostic-derive.rs | 24 +++++++++ 4 files changed, 105 insertions(+), 23 deletions(-) diff --git a/compiler/rustc_macros/src/diagnostics/diagnostic_builder.rs b/compiler/rustc_macros/src/diagnostics/diagnostic_builder.rs index df4e309086fd2..017bf2365d62b 100644 --- a/compiler/rustc_macros/src/diagnostics/diagnostic_builder.rs +++ b/compiler/rustc_macros/src/diagnostics/diagnostic_builder.rs @@ -426,7 +426,8 @@ impl<'a> DiagnosticDeriveVariantBuilder<'a> { SubdiagnosticKind::Suggestion { suggestion_kind, applicability: static_applicability, - code, + code_field, + code_init, } => { let (span_field, mut applicability) = self.span_and_applicability_of_ty(info)?; @@ -440,10 +441,11 @@ impl<'a> DiagnosticDeriveVariantBuilder<'a> { let style = suggestion_kind.to_suggestion_style(); Ok(quote! { + #code_init #diag.span_suggestion_with_style( #span_field, rustc_errors::fluent::#slug, - #code, + #code_field, #applicability, #style ); diff --git a/compiler/rustc_macros/src/diagnostics/subdiagnostic.rs b/compiler/rustc_macros/src/diagnostics/subdiagnostic.rs index ef17dbd0426fe..3d4c3ab9fd7c9 100644 --- a/compiler/rustc_macros/src/diagnostics/subdiagnostic.rs +++ b/compiler/rustc_macros/src/diagnostics/subdiagnostic.rs @@ -5,7 +5,7 @@ use crate::diagnostics::error::{ DiagnosticDeriveError, }; use crate::diagnostics::utils::{ - build_field_mapping, report_error_if_not_applied_to_applicability, + build_field_mapping, new_code_ident, report_error_if_not_applied_to_applicability, report_error_if_not_applied_to_span, FieldInfo, FieldInnerTy, FieldMap, HasFieldMap, SetOnce, SpannedOption, SubdiagnosticKind, }; @@ -57,6 +57,7 @@ impl SubdiagnosticDeriveBuilder { parent: &self, variant, span, + formatting_init: TokenStream::new(), fields: build_field_mapping(variant), span_field: None, applicability: None, @@ -105,6 +106,9 @@ struct SubdiagnosticDeriveVariantBuilder<'parent, 'a> { /// Span for the entire type. span: proc_macro::Span, + /// Initialization of format strings for code suggestions. + formatting_init: TokenStream, + /// Store a map of field name to its corresponding field. This is built on construction of the /// derive builder. fields: FieldMap, @@ -230,7 +234,7 @@ impl<'parent, 'a> SubdiagnosticDeriveVariantBuilder<'parent, 'a> { }; let generated = self - .generate_field_code_inner(kind_stats, attr, info) + .generate_field_code_inner(kind_stats, attr, info, inner_ty.will_iterate()) .unwrap_or_else(|v| v.to_compile_error()); inner_ty.with(binding, generated) @@ -243,13 +247,18 @@ impl<'parent, 'a> SubdiagnosticDeriveVariantBuilder<'parent, 'a> { kind_stats: KindsStatistics, attr: &Attribute, info: FieldInfo<'_>, + clone_suggestion_code: bool, ) -> Result { let meta = attr.parse_meta()?; match meta { Meta::Path(path) => self.generate_field_code_inner_path(kind_stats, attr, info, path), - Meta::List(list @ MetaList { .. }) => { - self.generate_field_code_inner_list(kind_stats, attr, info, list) - } + Meta::List(list @ MetaList { .. }) => self.generate_field_code_inner_list( + kind_stats, + attr, + info, + list, + clone_suggestion_code, + ), _ => throw_invalid_attr!(attr, &meta), } } @@ -353,6 +362,7 @@ impl<'parent, 'a> SubdiagnosticDeriveVariantBuilder<'parent, 'a> { attr: &Attribute, info: FieldInfo<'_>, list: MetaList, + clone_suggestion_code: bool, ) -> Result { let span = attr.span().unwrap(); let ident = &list.path.segments.last().unwrap().ident; @@ -390,7 +400,8 @@ impl<'parent, 'a> SubdiagnosticDeriveVariantBuilder<'parent, 'a> { match nested_name { "code" => { let formatted_str = self.build_format(&value.value(), value.span()); - code.set_once(formatted_str, span); + let code_field = new_code_ident(); + code.set_once((code_field, formatted_str), span); } _ => throw_invalid_nested_attr!(attr, &nested_attr, |diag| { diag.help("`code` is the only valid nested attribute") @@ -398,14 +409,20 @@ impl<'parent, 'a> SubdiagnosticDeriveVariantBuilder<'parent, 'a> { } } - let Some((code, _)) = code else { + let Some((code_field, formatted_str)) = code.value() else { span_err(span, "`#[suggestion_part(...)]` attribute without `code = \"...\"`") .emit(); return Ok(quote! {}); }; let binding = info.binding; - Ok(quote! { suggestions.push((#binding, #code)); }) + self.formatting_init.extend(quote! { let #code_field = #formatted_str; }); + let code_field = if clone_suggestion_code { + quote! { #code_field.clone() } + } else { + quote! { #code_field } + }; + Ok(quote! { suggestions.push((#binding, #code_field)); }) } _ => throw_invalid_attr!(attr, &Meta::List(list), |diag| { let mut span_attrs = vec![]; @@ -459,7 +476,14 @@ impl<'parent, 'a> SubdiagnosticDeriveVariantBuilder<'parent, 'a> { let name = format_ident!("{}{}", if span_field.is_some() { "span_" } else { "" }, kind); let call = match kind { - SubdiagnosticKind::Suggestion { suggestion_kind, applicability, code } => { + SubdiagnosticKind::Suggestion { + suggestion_kind, + applicability, + code_init, + code_field, + } => { + self.formatting_init.extend(code_init); + let applicability = applicability .value() .map(|a| quote! { #a }) @@ -468,8 +492,7 @@ impl<'parent, 'a> SubdiagnosticDeriveVariantBuilder<'parent, 'a> { if let Some(span) = span_field { let style = suggestion_kind.to_suggestion_style(); - - quote! { #diag.#name(#span, #message, #code, #applicability, #style); } + quote! { #diag.#name(#span, #message, #code_field, #applicability, #style); } } else { span_err(self.span, "suggestion without `#[primary_span]` field").emit(); quote! { unreachable!(); } @@ -510,6 +533,7 @@ impl<'parent, 'a> SubdiagnosticDeriveVariantBuilder<'parent, 'a> { } } }; + calls.extend(call); } @@ -521,11 +545,13 @@ impl<'parent, 'a> SubdiagnosticDeriveVariantBuilder<'parent, 'a> { .map(|binding| self.generate_field_set_arg(binding)) .collect(); + let formatting_init = &self.formatting_init; Ok(quote! { #init + #formatting_init #attr_args - #calls #plain_args + #calls }) } } diff --git a/compiler/rustc_macros/src/diagnostics/utils.rs b/compiler/rustc_macros/src/diagnostics/utils.rs index 162699c286872..5e1cd842b9bbb 100644 --- a/compiler/rustc_macros/src/diagnostics/utils.rs +++ b/compiler/rustc_macros/src/diagnostics/utils.rs @@ -4,6 +4,7 @@ use crate::diagnostics::error::{ use proc_macro::Span; use proc_macro2::TokenStream; use quote::{format_ident, quote, ToTokens}; +use std::cell::RefCell; use std::cmp::Ordering; use std::collections::{BTreeSet, HashMap}; use std::fmt; @@ -14,6 +15,19 @@ use synstructure::{BindStyle, BindingInfo, VariantInfo}; use super::error::invalid_nested_attr; +thread_local! { + pub static CODE_IDENT_COUNT: RefCell = RefCell::new(0); +} + +/// Returns an ident of the form `__code_N` where `N` is incremented once with every call. +pub(crate) fn new_code_ident() -> syn::Ident { + CODE_IDENT_COUNT.with(|count| { + let ident = format_ident!("__code_{}", *count.borrow()); + *count.borrow_mut() += 1; + ident + }) +} + /// Checks whether the type name of `ty` matches `name`. /// /// Given some struct at `a::b::c::Foo`, this will return true for `c::Foo`, `b::c::Foo`, or @@ -142,6 +156,15 @@ impl<'ty> FieldInnerTy<'ty> { unreachable!(); } + /// Returns `true` if `FieldInnerTy::with` will result in iteration for this inner type (i.e. + /// that cloning might be required for values moved in the loop body). + pub(crate) fn will_iterate(&self) -> bool { + match self { + FieldInnerTy::Vec(..) => true, + FieldInnerTy::Option(..) | FieldInnerTy::None => false, + } + } + /// Returns `Option` containing inner type if there is one. pub(crate) fn inner_type(&self) -> Option<&'ty Type> { match self { @@ -434,7 +457,12 @@ pub(super) enum SubdiagnosticKind { Suggestion { suggestion_kind: SuggestionKind, applicability: SpannedOption, - code: TokenStream, + /// Identifier for variable used for formatted code, e.g. `___code_0`. Enables separation + /// of formatting and diagnostic emission so that `set_arg` calls can happen in-between.. + code_field: syn::Ident, + /// Initialization logic for `code_field`'s variable, e.g. + /// `let __formatted_code = /* whatever */;` + code_init: TokenStream, }, /// `#[multipart_suggestion{,_short,_hidden,_verbose}]` MultipartSuggestion { @@ -469,7 +497,8 @@ impl SubdiagnosticKind { SubdiagnosticKind::Suggestion { suggestion_kind, applicability: None, - code: TokenStream::new(), + code_field: new_code_ident(), + code_init: TokenStream::new(), } } else if let Some(suggestion_kind) = name.strip_prefix("multipart_suggestion").and_then(|s| s.parse().ok()) @@ -548,9 +577,10 @@ impl SubdiagnosticKind { }; match (nested_name, &mut kind) { - ("code", SubdiagnosticKind::Suggestion { .. }) => { + ("code", SubdiagnosticKind::Suggestion { code_field, .. }) => { let formatted_str = fields.build_format(&value.value(), value.span()); - code.set_once(formatted_str, span); + let code_init = quote! { let #code_field = #formatted_str; }; + code.set_once(code_init, span); } ( "applicability", @@ -582,13 +612,13 @@ impl SubdiagnosticKind { } match kind { - SubdiagnosticKind::Suggestion { code: ref mut code_field, .. } => { - *code_field = if let Some((code, _)) = code { - code + SubdiagnosticKind::Suggestion { ref code_field, ref mut code_init, .. } => { + *code_init = if let Some(init) = code.value() { + init } else { span_err(span, "suggestion without `code = \"...\"`").emit(); - quote! { "" } - } + quote! { let #code_field: String = unreachable!(); } + }; } SubdiagnosticKind::Label | SubdiagnosticKind::Note diff --git a/src/test/ui-fulldeps/session-diagnostic/diagnostic-derive.rs b/src/test/ui-fulldeps/session-diagnostic/diagnostic-derive.rs index 460af07a55967..e873c36e0b39a 100644 --- a/src/test/ui-fulldeps/session-diagnostic/diagnostic-derive.rs +++ b/src/test/ui-fulldeps/session-diagnostic/diagnostic-derive.rs @@ -725,3 +725,27 @@ struct SubdiagnosticEagerCorrect { #[subdiagnostic(eager)] note: Note, } + +// Check that formatting of `correct` in suggestion doesn't move the binding for that field, making +// the `set_arg` call a compile error; and that isn't worked around by moving the `set_arg` call +// after the `span_suggestion` call - which breaks eager translation. + +#[derive(Subdiagnostic)] +#[suggestion_short( + parser::use_instead, + applicability = "machine-applicable", + code = "{correct}" +)] +pub(crate) struct SubdiagnosticWithSuggestion { + #[primary_span] + span: Span, + invalid: String, + correct: String, +} + +#[derive(Diagnostic)] +#[diag(compiletest::example)] +struct SubdiagnosticEagerSuggestion { + #[subdiagnostic(eager)] + sub: SubdiagnosticWithSuggestion, +} From fbac1f288bb797ee239f69eafc3dca558f1ea817 Mon Sep 17 00:00:00 2001 From: David Wood Date: Mon, 3 Oct 2022 16:10:34 +0100 Subject: [PATCH 23/27] macros: simplify field ordering in diag derive Following the approach taken in earlier commits to separate formatting initialization from use in the subdiagnostic derive, simplify the diagnostic derive by removing the field-ordering logic that previously solved this problem. Signed-off-by: David Wood --- .../src/diagnostics/diagnostic.rs | 5 +- .../src/diagnostics/diagnostic_builder.rs | 75 ++++++++----------- .../rustc_macros/src/diagnostics/utils.rs | 60 +-------------- 3 files changed, 38 insertions(+), 102 deletions(-) diff --git a/compiler/rustc_macros/src/diagnostics/diagnostic.rs b/compiler/rustc_macros/src/diagnostics/diagnostic.rs index 84c57b3f64e18..8cf307df5a565 100644 --- a/compiler/rustc_macros/src/diagnostics/diagnostic.rs +++ b/compiler/rustc_macros/src/diagnostics/diagnostic.rs @@ -52,8 +52,10 @@ impl<'a> DiagnosticDerive<'a> { } }; + let formatting_init = &builder.formatting_init; quote! { #init + #formatting_init #preamble #body #diag @@ -101,9 +103,10 @@ impl<'a> LintDiagnosticDerive<'a> { let body = builder.body(&variant); let diag = &builder.parent.diag; - + let formatting_init = &builder.formatting_init; quote! { #preamble + #formatting_init #body #diag } diff --git a/compiler/rustc_macros/src/diagnostics/diagnostic_builder.rs b/compiler/rustc_macros/src/diagnostics/diagnostic_builder.rs index 017bf2365d62b..dcbe89251cb36 100644 --- a/compiler/rustc_macros/src/diagnostics/diagnostic_builder.rs +++ b/compiler/rustc_macros/src/diagnostics/diagnostic_builder.rs @@ -5,9 +5,9 @@ use crate::diagnostics::error::{ DiagnosticDeriveError, }; use crate::diagnostics::utils::{ - bind_style_of_field, build_field_mapping, report_error_if_not_applied_to_span, - report_type_error, should_generate_set_arg, type_is_unit, type_matches_path, FieldInfo, - FieldInnerTy, FieldMap, HasFieldMap, SetOnce, SpannedOption, SubdiagnosticKind, + build_field_mapping, report_error_if_not_applied_to_span, report_type_error, + should_generate_set_arg, type_is_unit, type_matches_path, FieldInfo, FieldInnerTy, FieldMap, + HasFieldMap, SetOnce, SpannedOption, SubdiagnosticKind, }; use proc_macro2::{Ident, Span, TokenStream}; use quote::{format_ident, quote}; @@ -40,6 +40,9 @@ pub(crate) struct DiagnosticDeriveVariantBuilder<'parent> { /// The parent builder for the entire type. pub parent: &'parent DiagnosticDeriveBuilder, + /// Initialization of format strings for code suggestions. + pub formatting_init: TokenStream, + /// Span of the struct or the enum variant. pub span: proc_macro::Span, @@ -88,19 +91,7 @@ impl DiagnosticDeriveBuilder { } } - for variant in structure.variants_mut() { - // First, change the binding style of each field based on the code that will be - // generated for the field - e.g. `set_arg` calls needs by-move bindings, whereas - // `set_primary_span` only needs by-ref. - variant.bind_with(|bi| bind_style_of_field(bi.ast()).0); - - // Then, perform a stable sort on bindings which generates code for by-ref bindings - // before code generated for by-move bindings. Any code generated for the by-ref - // bindings which creates a reference to the by-move fields will happen before the - // by-move bindings move those fields and make them inaccessible. - variant.bindings_mut().sort_by_cached_key(|bi| bind_style_of_field(bi.ast())); - } - + structure.bind_with(|_| synstructure::BindStyle::Move); let variants = structure.each_variant(|variant| { let span = match structure.ast().data { syn::Data::Struct(..) => span, @@ -112,6 +103,7 @@ impl DiagnosticDeriveBuilder { parent: &self, span, field_map: build_field_mapping(variant), + formatting_init: TokenStream::new(), slug: None, code: None, }; @@ -143,16 +135,14 @@ impl<'a> DiagnosticDeriveVariantBuilder<'a> { /// Generates calls to `span_label` and similar functions based on the attributes on fields or /// calls to `set_arg` when no attributes are present. - /// - /// Expects use of `Self::each_variant` which will have sorted bindings so that by-ref bindings - /// (which may create references to by-move bindings) have their code generated first - - /// necessary as code for suggestions uses formatting machinery and the value of other fields - /// (any given field can be referenced multiple times, so must be accessed through a borrow); - /// and when passing fields to `add_subdiagnostic` or `set_arg` for Fluent, fields must be - /// accessed by-move. pub fn body<'s>(&mut self, variant: &VariantInfo<'s>) -> TokenStream { let mut body = quote! {}; - for binding in variant.bindings() { + // Generate `set_arg` calls first.. + for binding in variant.bindings().iter().filter(|bi| should_generate_set_arg(bi.ast())) { + body.extend(self.generate_field_code(binding)); + } + // ..and then subdiagnostic additions. + for binding in variant.bindings().iter().filter(|bi| !should_generate_set_arg(bi.ast())) { body.extend(self.generate_field_attrs_code(binding)); } body @@ -274,24 +264,27 @@ impl<'a> DiagnosticDeriveVariantBuilder<'a> { } } - fn generate_field_attrs_code(&mut self, binding_info: &BindingInfo<'_>) -> TokenStream { + fn generate_field_code(&mut self, binding_info: &BindingInfo<'_>) -> TokenStream { + let diag = &self.parent.diag; + let field = binding_info.ast(); let field_binding = &binding_info.binding; - if should_generate_set_arg(&field) { - let diag = &self.parent.diag; - let ident = field.ident.as_ref().unwrap(); - // strip `r#` prefix, if present - let ident = format_ident!("{}", ident); - return quote! { - #diag.set_arg( - stringify!(#ident), - #field_binding - ); - }; + let ident = field.ident.as_ref().unwrap(); + let ident = format_ident!("{}", ident); // strip `r#` prefix, if present + + quote! { + #diag.set_arg( + stringify!(#ident), + #field_binding + ); } + } + + fn generate_field_attrs_code(&mut self, binding_info: &BindingInfo<'_>) -> TokenStream { + let field = binding_info.ast(); + let field_binding = &binding_info.binding; - let needs_move = bind_style_of_field(&field).is_move(); let inner_ty = FieldInnerTy::from_type(&field.ty); field @@ -304,10 +297,8 @@ impl<'a> DiagnosticDeriveVariantBuilder<'a> { let (binding, needs_destructure) = if needs_clone { // `primary_span` can accept a `Vec` so don't destructure that. (quote! { #field_binding.clone() }, false) - } else if needs_move { - (quote! { #field_binding }, true) } else { - (quote! { *#field_binding }, true) + (quote! { #field_binding }, true) }; let generated_code = self @@ -440,8 +431,8 @@ impl<'a> DiagnosticDeriveVariantBuilder<'a> { .unwrap_or_else(|| quote! { rustc_errors::Applicability::Unspecified }); let style = suggestion_kind.to_suggestion_style(); + self.formatting_init.extend(code_init); Ok(quote! { - #code_init #diag.span_suggestion_with_style( #span_field, rustc_errors::fluent::#slug, @@ -490,7 +481,7 @@ impl<'a> DiagnosticDeriveVariantBuilder<'a> { // If `ty` is `Span` w/out applicability, then use `Applicability::Unspecified`. ty @ Type::Path(..) if type_matches_path(ty, &["rustc_span", "Span"]) => { let binding = &info.binding.binding; - Ok((quote!(*#binding), None)) + Ok((quote!(#binding), None)) } // If `ty` is `(Span, Applicability)` then return tokens accessing those. Type::Tuple(tup) => { diff --git a/compiler/rustc_macros/src/diagnostics/utils.rs b/compiler/rustc_macros/src/diagnostics/utils.rs index 5e1cd842b9bbb..4fd4adc511267 100644 --- a/compiler/rustc_macros/src/diagnostics/utils.rs +++ b/compiler/rustc_macros/src/diagnostics/utils.rs @@ -5,13 +5,12 @@ use proc_macro::Span; use proc_macro2::TokenStream; use quote::{format_ident, quote, ToTokens}; use std::cell::RefCell; -use std::cmp::Ordering; use std::collections::{BTreeSet, HashMap}; use std::fmt; use std::str::FromStr; use syn::{spanned::Spanned, Attribute, Field, Meta, Type, TypeTuple}; use syn::{MetaList, MetaNameValue, NestedMeta, Path}; -use synstructure::{BindStyle, BindingInfo, VariantInfo}; +use synstructure::{BindingInfo, VariantInfo}; use super::error::invalid_nested_attr; @@ -650,65 +649,8 @@ impl quote::IdentFragment for SubdiagnosticKind { } } -/// Wrapper around `synstructure::BindStyle` which implements `Ord`. -#[derive(PartialEq, Eq)] -pub(super) struct OrderedBindStyle(pub(super) BindStyle); - -impl OrderedBindStyle { - /// Is `BindStyle::Move` or `BindStyle::MoveMut`? - pub(super) fn is_move(&self) -> bool { - matches!(self.0, BindStyle::Move | BindStyle::MoveMut) - } -} - -impl Ord for OrderedBindStyle { - fn cmp(&self, other: &Self) -> Ordering { - match (self.is_move(), other.is_move()) { - // If both `self` and `other` are the same, then ordering is equal. - (true, true) | (false, false) => Ordering::Equal, - // If `self` is not a move then it should be considered less than `other` (so that - // references are sorted first). - (false, _) => Ordering::Less, - // If `self` is a move then it must be greater than `other` (again, so that references - // are sorted first). - (true, _) => Ordering::Greater, - } - } -} - -impl PartialOrd for OrderedBindStyle { - fn partial_cmp(&self, other: &Self) -> Option { - Some(self.cmp(other)) - } -} - /// Returns `true` if `field` should generate a `set_arg` call rather than any other diagnostic /// call (like `span_label`). pub(super) fn should_generate_set_arg(field: &Field) -> bool { field.attrs.is_empty() } - -/// Returns `true` if `field` needs to have code generated in the by-move branch of the -/// generated derive rather than the by-ref branch. -pub(super) fn bind_style_of_field(field: &Field) -> OrderedBindStyle { - let generates_set_arg = should_generate_set_arg(field); - let is_multispan = type_matches_path(&field.ty, &["rustc_errors", "MultiSpan"]); - // FIXME(davidtwco): better support for one field needing to be in the by-move and - // by-ref branches. - let is_subdiagnostic = field - .attrs - .iter() - .map(|attr| attr.path.segments.last().unwrap().ident.to_string()) - .any(|attr| attr == "subdiagnostic"); - - // `set_arg` calls take their argument by-move.. - let needs_move = generates_set_arg - // If this is a `MultiSpan` field then it needs to be moved to be used by any - // attribute.. - || is_multispan - // If this a `#[subdiagnostic]` then it needs to be moved as the other diagnostic is - // unlikely to be `Copy`.. - || is_subdiagnostic; - - OrderedBindStyle(if needs_move { BindStyle::Move } else { BindStyle::Ref }) -} From da638b3a9f32974c41439be6834670d7a438faa3 Mon Sep 17 00:00:00 2001 From: Andrew Brown Date: Tue, 27 Sep 2022 11:50:47 -0700 Subject: [PATCH 24/27] Allow compiling the `wasm32-wasi` std library with atomics The issue #102157 demonstrates how currently the `-Z build-std` option will fail when re-compiling the standard library with `RUSTFLAGS` like `RUSTFLAGS="-C target-feature=+atomics,+bulk-memory -C link-args=--shared-memory"`. This change attempts to resolve those build issues by depending on the the WebAssembly `futex` module and providing an implementation for `env_lock`. Fixes #102157. --- library/std/src/sys/wasi/mod.rs | 3 +++ library/std/src/sys/wasi/os.rs | 11 ++++++++--- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/library/std/src/sys/wasi/mod.rs b/library/std/src/sys/wasi/mod.rs index 683a07a34dcf9..c8c47763a340b 100644 --- a/library/std/src/sys/wasi/mod.rs +++ b/library/std/src/sys/wasi/mod.rs @@ -25,6 +25,9 @@ pub mod cmath; pub mod env; pub mod fd; pub mod fs; +#[allow(unused)] +#[path = "../wasm/atomics/futex.rs"] +pub mod futex; pub mod io; #[path = "../unsupported/locks/mod.rs"] pub mod locks; diff --git a/library/std/src/sys/wasi/os.rs b/library/std/src/sys/wasi/os.rs index cab2887dfcf14..b340061cb3d10 100644 --- a/library/std/src/sys/wasi/os.rs +++ b/library/std/src/sys/wasi/os.rs @@ -24,10 +24,15 @@ mod libc { } } -#[cfg(not(target_feature = "atomics"))] pub unsafe fn env_lock() -> impl Any { - // No need for a lock if we're single-threaded, but this function will need - // to get implemented for multi-threaded scenarios + cfg_if::cfg_if! { + if #[cfg(target_feature = "atomics")] { + todo!() + } else { + // No need for a lock if we're single-threaded, but this function will need + // to get implemented for multi-threaded scenarios + } + } } pub fn errno() -> i32 { From 9530ba0fe2118d9a60e16861faad29ec5803d3f9 Mon Sep 17 00:00:00 2001 From: Andrew Brown Date: Thu, 29 Sep 2022 11:17:15 -0700 Subject: [PATCH 25/27] Implement `env_lock` with `RwLock` Copying the approach of the Unix target, this change uses the standard `RwLock` to protect against concurrent access of libc's environment. This locking is only enabled when WebAssembly's `atomics` feature is also enabled. --- library/std/src/sys/wasi/os.rs | 35 ++++++++++++++++++++++------------ 1 file changed, 23 insertions(+), 12 deletions(-) diff --git a/library/std/src/sys/wasi/os.rs b/library/std/src/sys/wasi/os.rs index b340061cb3d10..cffe1e32308b6 100644 --- a/library/std/src/sys/wasi/os.rs +++ b/library/std/src/sys/wasi/os.rs @@ -1,11 +1,11 @@ #![deny(unsafe_op_in_unsafe_fn)] -use crate::any::Any; use crate::error::Error as StdError; use crate::ffi::{CStr, OsStr, OsString}; use crate::fmt; use crate::io; use crate::marker::PhantomData; +use crate::ops::Drop; use crate::os::wasi::prelude::*; use crate::path::{self, PathBuf}; use crate::str; @@ -24,13 +24,24 @@ mod libc { } } -pub unsafe fn env_lock() -> impl Any { - cfg_if::cfg_if! { - if #[cfg(target_feature = "atomics")] { - todo!() - } else { - // No need for a lock if we're single-threaded, but this function will need - // to get implemented for multi-threaded scenarios +cfg_if::cfg_if! { + if #[cfg(target_feature = "atomics")] { + // Access to the environment must be protected by a lock in multi-threaded scenarios. + use crate::sync::{PoisonError, RwLock}; + static ENV_LOCK: RwLock<()> = RwLock::new(()); + pub fn env_read_lock() -> impl Drop { + ENV_LOCK.read().unwrap_or_else(PoisonError::into_inner) + } + pub fn env_write_lock() -> impl Drop { + ENV_LOCK.write().unwrap_or_else(PoisonError::into_inner) + } + } else { + // No need for a lock if we are single-threaded. + pub fn env_read_lock() -> impl Drop { + () + } + pub fn env_write_lock() -> impl Drop { + () } } } @@ -149,7 +160,7 @@ impl Iterator for Env { pub fn env() -> Env { unsafe { - let _guard = env_lock(); + let _guard = env_read_lock(); let mut environ = libc::environ; let mut result = Vec::new(); if !environ.is_null() { @@ -180,7 +191,7 @@ pub fn env() -> Env { pub fn getenv(k: &OsStr) -> Option { let s = run_with_cstr(k.as_bytes(), |k| unsafe { - let _guard = env_lock(); + let _guard = env_read_lock(); Ok(libc::getenv(k.as_ptr()) as *const libc::c_char) }) .ok()?; @@ -194,7 +205,7 @@ pub fn getenv(k: &OsStr) -> Option { pub fn setenv(k: &OsStr, v: &OsStr) -> io::Result<()> { run_with_cstr(k.as_bytes(), |k| { run_with_cstr(v.as_bytes(), |v| unsafe { - let _guard = env_lock(); + let _guard = env_write_lock(); cvt(libc::setenv(k.as_ptr(), v.as_ptr(), 1)).map(drop) }) }) @@ -202,7 +213,7 @@ pub fn setenv(k: &OsStr, v: &OsStr) -> io::Result<()> { pub fn unsetenv(n: &OsStr) -> io::Result<()> { run_with_cstr(n.as_bytes(), |nbuf| unsafe { - let _guard = env_lock(); + let _guard = env_write_lock(); cvt(libc::unsetenv(nbuf.as_ptr())).map(drop) }) } From b63b02f8728259c3b67d1f6ca7d1845892b86a9e Mon Sep 17 00:00:00 2001 From: Michael Howell Date: Mon, 10 Oct 2022 11:37:19 -0700 Subject: [PATCH 26/27] rustdoc: remove unneeded `
` wrapper from sidebar DOM When this was added, the sidebar had a bit more complex style. It can be removed, now. --- src/librustdoc/html/render/mod.rs | 23 +++++----------- src/librustdoc/html/static/css/rustdoc.css | 5 ++-- src/librustdoc/html/static/js/main.js | 26 ++++++++----------- src/test/rustdoc-gui/headings.goml | 4 +-- src/test/rustdoc-gui/sidebar.goml | 20 +++++++------- .../strip-enum-variant.no-not-shown.html | 2 +- src/test/rustdoc/strip-enum-variant.rs | 2 +- 7 files changed, 35 insertions(+), 47 deletions(-) diff --git a/src/librustdoc/html/render/mod.rs b/src/librustdoc/html/render/mod.rs index 4bbb322d3701c..5e4226042ec72 100644 --- a/src/librustdoc/html/render/mod.rs +++ b/src/librustdoc/html/render/mod.rs @@ -1853,12 +1853,12 @@ fn print_sidebar(cx: &Context<'_>, it: &clean::Item, buffer: &mut Buffer) { buffer.write_str("
"); if it.is_crate() { - write!(buffer, "
    "); + write!(buffer, "
      "); if let Some(ref version) = cx.cache().crate_version { write!(buffer, "
    • Version {}
      • ", Escape(version)); } write!(buffer, "
    • All Items
      • "); - buffer.write_str("
"); + buffer.write_str(""); } match *it.kind { @@ -2259,7 +2259,7 @@ fn extract_for_impl_name(item: &clean::Item, cx: &Context<'_>) -> Option<(String } /// Don't call this function directly!!! Use `print_sidebar_title` or `print_sidebar_block` instead! -fn print_sidebar_title_inner(buf: &mut Buffer, id: &str, title: &str) { +fn print_sidebar_title(buf: &mut Buffer, id: &str, title: &str) { write!( buf, "

\ @@ -2269,25 +2269,18 @@ fn print_sidebar_title_inner(buf: &mut Buffer, id: &str, title: &str) { ); } -fn print_sidebar_title(buf: &mut Buffer, id: &str, title: &str) { - buf.push_str("
"); - print_sidebar_title_inner(buf, id, title); - buf.push_str("
"); -} - fn print_sidebar_block( buf: &mut Buffer, id: &str, title: &str, items: impl Iterator, ) { - buf.push_str("
"); - print_sidebar_title_inner(buf, id, title); - buf.push_str("
    "); + print_sidebar_title(buf, id, title); + buf.push_str("
      "); for item in items { write!(buf, "
    • {}
      • ", item); } - buf.push_str("
"); + buf.push_str(""); } fn sidebar_trait(cx: &Context<'_>, buf: &mut Buffer, it: &clean::Item, t: &clean::Trait) { @@ -2676,9 +2669,7 @@ pub(crate) fn sidebar_module_like(buf: &mut Buffer, item_sections_in_use: FxHash write!( buf, "
\ -
\ -
    {}
\ -
\ +
    {}
\
", sidebar ); diff --git a/src/librustdoc/html/static/css/rustdoc.css b/src/librustdoc/html/static/css/rustdoc.css index eb64147d90677..386977c6f8e79 100644 --- a/src/librustdoc/html/static/css/rustdoc.css +++ b/src/librustdoc/html/static/css/rustdoc.css @@ -501,13 +501,14 @@ img { width: 100px; } -.block ul, .block li { +ul.block, .block li { padding: 0; margin: 0; list-style: none; } .block a, +.sidebar h3 a, h2.location a { display: block; padding: 0.25rem; @@ -769,7 +770,7 @@ h2.small-section-header > .anchor { text-decoration: underline; } -.block a.current.crate { font-weight: 500; } +.crate.block a.current { font-weight: 500; } /* In most contexts we use `overflow-wrap: anywhere` to ensure that we can wrap as much as needed on mobile (see diff --git a/src/librustdoc/html/static/js/main.js b/src/librustdoc/html/static/js/main.js index 359dd41b13fed..0180c0ead8d39 100644 --- a/src/librustdoc/html/static/js/main.js +++ b/src/librustdoc/html/static/js/main.js @@ -442,12 +442,10 @@ function loadCss(cssFileName) { return; } - const div = document.createElement("div"); - div.className = "block " + shortty; const h3 = document.createElement("h3"); h3.innerHTML = `${longty}`; - div.appendChild(h3); const ul = document.createElement("ul"); + ul.className = "block " + shortty; for (const item of filtered) { const name = item[0]; @@ -473,8 +471,8 @@ function loadCss(cssFileName) { li.appendChild(link); ul.appendChild(li); } - div.appendChild(ul); - sidebar.appendChild(div); + sidebar.appendChild(h3); + sidebar.appendChild(ul); } if (sidebar) { @@ -592,27 +590,25 @@ function loadCss(cssFileName) { return; } // Draw a convenient sidebar of known crates if we have a listing - const div = document.createElement("div"); - div.className = "block crate"; - div.innerHTML = "

Crates

"; + const h3 = document.createElement("h3"); + h3.innerHTML = "Crates"; const ul = document.createElement("ul"); - div.appendChild(ul); + ul.className = "block crate"; for (const crate of window.ALL_CRATES) { - let klass = "crate"; - if (window.rootPath !== "./" && crate === window.currentCrate) { - klass += " current"; - } const link = document.createElement("a"); link.href = window.rootPath + crate + "/index.html"; - link.className = klass; + if (window.rootPath !== "./" && crate === window.currentCrate) { + link.className = "current"; + } link.textContent = crate; const li = document.createElement("li"); li.appendChild(link); ul.appendChild(li); } - sidebarElems.appendChild(div); + sidebarElems.appendChild(h3); + sidebarElems.appendChild(ul); } diff --git a/src/test/rustdoc-gui/headings.goml b/src/test/rustdoc-gui/headings.goml index 3e97bb78c7818..9a77d8bbd1541 100644 --- a/src/test/rustdoc-gui/headings.goml +++ b/src/test/rustdoc-gui/headings.goml @@ -106,8 +106,8 @@ assert-css: ("h6#sub-heading-for-enum-impl-item-doc", {"border-bottom-width": "0 assert-css: ("h6#sub-sub-heading-for-enum-impl-item-doc", {"font-size": "14px"}) assert-css: ("h6#sub-sub-heading-for-enum-impl-item-doc", {"border-bottom-width": "0px"}) -assert-text: (".sidebar .mod h3", "Modules") -assert-css: (".sidebar .mod h3", {"border-bottom-width": "0px"}, ALL) +assert-text: ("//ul[@class='block mod']/preceding-sibling::h3", "Modules") +assert-css: ("//ul[@class='block mod']/preceding-sibling::h3", {"border-bottom-width": "0px"}, ALL) goto: "file://" + |DOC_PATH| + "/test_docs/union.HeavilyDocumentedUnion.html" diff --git a/src/test/rustdoc-gui/sidebar.goml b/src/test/rustdoc-gui/sidebar.goml index ad1fb6df89ae9..54193234af9dd 100644 --- a/src/test/rustdoc-gui/sidebar.goml +++ b/src/test/rustdoc-gui/sidebar.goml @@ -12,7 +12,7 @@ assert-count: (".sidebar .location", 1) assert-text: ("#all-types", "All Items") assert-css: ("#all-types", {"color": "rgb(53, 109, 164)"}) // We check that we have the crates list and that the "current" on is "test_docs". -assert-text: (".sidebar-elems .crate > ul > li > a.current", "test_docs") +assert-text: (".sidebar-elems ul.crate > li > a.current", "test_docs") // And we're also supposed to have the list of items in the current module. assert-text: (".sidebar-elems section ul > li:nth-child(1)", "Re-exports") assert-text: (".sidebar-elems section ul > li:nth-child(2)", "Modules") @@ -41,21 +41,21 @@ assert-property: ("html", {"scrollTop": "0"}) // We now go back to the crate page to click on the "lib2" crate link. goto: "file://" + |DOC_PATH| + "/test_docs/index.html" assert-property: (".sidebar", {"clientWidth": "200"}) -assert-css: (".sidebar-elems .crate > ul > li:first-child > a", {"color": "rgb(53, 109, 164)"}) -click: ".sidebar-elems .crate > ul > li:first-child > a" +assert-css: (".sidebar-elems ul.crate > li:first-child > a", {"color": "rgb(53, 109, 164)"}) +click: ".sidebar-elems ul.crate > li:first-child > a" // PAGE: lib2/index.html goto: "file://" + |DOC_PATH| + "/lib2/index.html" assert-property: (".sidebar", {"clientWidth": "200"}) assert-text: (".sidebar > .location", "Crate lib2") // We check that we have the crates list and that the "current" on is now "lib2". -assert-text: (".sidebar-elems .crate > ul > li > a.current", "lib2") +assert-text: (".sidebar-elems ul.crate > li > a.current", "lib2") // We now go to the "foobar" function page. -assert-text: (".sidebar-elems > section .block ul > li:nth-child(1)", "Modules") -assert-text: (".sidebar-elems > section .block ul > li:nth-child(2)", "Structs") -assert-text: (".sidebar-elems > section .block ul > li:nth-child(3)", "Traits") -assert-text: (".sidebar-elems > section .block ul > li:nth-child(4)", "Functions") -assert-text: (".sidebar-elems > section .block ul > li:nth-child(5)", "Type Definitions") +assert-text: (".sidebar-elems > section ul.block > li:nth-child(1)", "Modules") +assert-text: (".sidebar-elems > section ul.block > li:nth-child(2)", "Structs") +assert-text: (".sidebar-elems > section ul.block > li:nth-child(3)", "Traits") +assert-text: (".sidebar-elems > section ul.block > li:nth-child(4)", "Functions") +assert-text: (".sidebar-elems > section ul.block > li:nth-child(5)", "Type Definitions") assert-text: ("#functions + .item-table .item-left > a", "foobar") click: "#functions + .item-table .item-left > a" @@ -90,7 +90,7 @@ assert-property: (".sidebar-elems section .block li > a", {"offsetHeight": 29}) // appropriate anchor in index.html. goto: "file://" + |DOC_PATH| + "/test_docs/struct.Foo.html" assert-property: (".sidebar", {"clientWidth": "200"}) -click: ".block.mod h3 a" +click: "//ul[@class='block mod']/preceding-sibling::h3/a" // PAGE: index.html assert-css: ("#modules", {"background-color": "rgb(253, 255, 211)"}) diff --git a/src/test/rustdoc/strip-enum-variant.no-not-shown.html b/src/test/rustdoc/strip-enum-variant.no-not-shown.html index c4ee1a9911436..782198956a0f3 100644 --- a/src/test/rustdoc/strip-enum-variant.no-not-shown.html +++ b/src/test/rustdoc/strip-enum-variant.no-not-shown.html @@ -1 +1 @@ - \ No newline at end of file + \ No newline at end of file diff --git a/src/test/rustdoc/strip-enum-variant.rs b/src/test/rustdoc/strip-enum-variant.rs index f82ffdfeda58f..8753a7dc613a8 100644 --- a/src/test/rustdoc/strip-enum-variant.rs +++ b/src/test/rustdoc/strip-enum-variant.rs @@ -3,7 +3,7 @@ // @!has - '//code' 'NotShown' // @has - '//code' '// some variants omitted' // Also check that `NotShown` isn't displayed in the sidebar. -// @snapshot no-not-shown - '//*[@class="sidebar-elems"]/section/*[@class="block"][1]/ul' +// @snapshot no-not-shown - '//*[@class="sidebar-elems"]/section/*[@class="block"][1]' pub enum MyThing { Shown, #[doc(hidden)] From 44f466cb08650a721d06dece85385506633d55fc Mon Sep 17 00:00:00 2001 From: Michael Howell Date: Mon, 10 Oct 2022 17:53:27 -0700 Subject: [PATCH 27/27] Remove outdated comment --- src/librustdoc/html/render/mod.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/librustdoc/html/render/mod.rs b/src/librustdoc/html/render/mod.rs index 5e4226042ec72..1e162bf314b87 100644 --- a/src/librustdoc/html/render/mod.rs +++ b/src/librustdoc/html/render/mod.rs @@ -2258,7 +2258,6 @@ fn extract_for_impl_name(item: &clean::Item, cx: &Context<'_>) -> Option<(String } } -/// Don't call this function directly!!! Use `print_sidebar_title` or `print_sidebar_block` instead! fn print_sidebar_title(buf: &mut Buffer, id: &str, title: &str) { write!( buf,