From bf135de3bc451573e9f84703cf9dda2e669bb2a7 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Fri, 7 Oct 2022 13:57:32 +1100 Subject: [PATCH 01/23] Clean up rustdoc startup. rustc's startup has several layers, including: - `interface::run_compiler` passes a closure, `f`, to `run_in_thread_pool_with_globals`, which creates a thread pool, sets up session globals, and passes `f` to `create_compiler_and_run`. - `create_compiler_and_run` creates a `Session`, a `Compiler`, sets the source map, and calls `f`. rustdoc is a bit different. - `main_args` calls `main_options` via `run_in_thread_pool_with_globals`, which (again) creates a thread pool (hardcoded to a single thread!) and sets up session globals. - `main_options` has four different paths. - The second one calls `interface::run_compiler`, which redoes the `run_in_thread_pool_with_globals`! This is bad. - The fourth one calls `interface::create_compiler_and_run`, which is reasonable. - The first and third ones don't do anything of note involving the above functions, except for some symbol interning which requires session globals. In other words, rustdoc calls into `rustc_interface` at three different levels. It's a bit confused, and feels like code where functionality has been added by different people at different times without fully understanding how the globally accessible stuff is set up. This commit tidies things up. It removes the `run_in_thread_pool_with_globals` call in `main_args`, and adjust the four paths in `main_options` as follows. - `markdown::test` calls `test::test_main`, which provides its own parallelism and so doesn't need a thread pool. It had one small use of symbol interning, which required session globals, but the commit removes this. - `doctest::run` already calls `interface::run_compiler`, so it doesn't need further adjustment. - `markdown::render` is simple but needs session globals for interning (which can't easily be removed), so it's now wrapped in `create_session_globals_then`. - The fourth path now uses `interface::run_compiler`, which is equivalent to the old `run_in_thread_pool_with_globals` + `create_compiler_and_run` pairing. --- src/librustdoc/doctest.rs | 9 ++++----- src/librustdoc/lib.rs | 14 +++++++------- src/librustdoc/markdown.rs | 5 +++-- 3 files changed, 14 insertions(+), 14 deletions(-) diff --git a/src/librustdoc/doctest.rs b/src/librustdoc/doctest.rs index f4ec60735a8dd..d9160e8abd4e3 100644 --- a/src/librustdoc/doctest.rs +++ b/src/librustdoc/doctest.rs @@ -18,7 +18,6 @@ use rustc_session::{lint, DiagnosticOutput, Session}; use rustc_span::edition::Edition; use rustc_span::source_map::SourceMap; use rustc_span::symbol::sym; -use rustc_span::Symbol; use rustc_span::{BytePos, FileName, Pos, Span, DUMMY_SP}; use rustc_target::spec::TargetTriple; use tempfile::Builder as TempFileBuilder; @@ -125,7 +124,7 @@ pub(crate) fn run(options: RustdocOptions) -> Result<(), ErrorGuaranteed> { let opts = scrape_test_config(crate_attrs); let enable_per_target_ignores = options.enable_per_target_ignores; let mut collector = Collector::new( - tcx.crate_name(LOCAL_CRATE), + tcx.crate_name(LOCAL_CRATE).to_string(), options, false, opts, @@ -909,7 +908,7 @@ pub(crate) struct Collector { rustdoc_options: RustdocOptions, use_headers: bool, enable_per_target_ignores: bool, - crate_name: Symbol, + crate_name: String, opts: GlobalTestOptions, position: Span, source_map: Option>, @@ -921,7 +920,7 @@ pub(crate) struct Collector { impl Collector { pub(crate) fn new( - crate_name: Symbol, + crate_name: String, rustdoc_options: RustdocOptions, use_headers: bool, opts: GlobalTestOptions, @@ -984,7 +983,7 @@ impl Tester for Collector { fn add_test(&mut self, test: String, config: LangString, line: usize) { let filename = self.get_filename(); let name = self.generate_name(line, &filename); - let crate_name = self.crate_name.to_string(); + let crate_name = self.crate_name.clone(); let opts = self.opts.clone(); let edition = config.edition.unwrap_or(self.rustdoc_options.edition); let rustdoc_options = self.rustdoc_options.clone(); diff --git a/src/librustdoc/lib.rs b/src/librustdoc/lib.rs index ef01b854e5a69..ce6f7e817c620 100644 --- a/src/librustdoc/lib.rs +++ b/src/librustdoc/lib.rs @@ -700,11 +700,7 @@ fn main_args(at_args: &[String]) -> MainResult { }; } }; - rustc_interface::util::run_in_thread_pool_with_globals( - options.edition, - 1, // this runs single-threaded, even in a parallel compiler - move || main_options(options), - ) + main_options(options) } fn wrap_return(diag: &rustc_errors::Handler, res: Result<(), String>) -> MainResult { @@ -749,9 +745,12 @@ fn main_options(options: config::Options) -> MainResult { (true, true) => return wrap_return(&diag, markdown::test(options)), (true, false) => return doctest::run(options), (false, true) => { + // Session globals are required for symbol interning. return wrap_return( &diag, - markdown::render(&options.input, options.render_options, options.edition), + rustc_span::create_session_globals_then(options.edition, || { + markdown::render(&options.input, options.render_options, options.edition) + }), ); } (false, false) => {} @@ -777,9 +776,10 @@ fn main_options(options: config::Options) -> MainResult { let render_options = options.render_options.clone(); let scrape_examples_options = options.scrape_examples_options.clone(); let document_private = options.render_options.document_private; + let config = core::create_config(options); - interface::create_compiler_and_run(config, |compiler| { + interface::run_compiler(config, |compiler| { let sess = compiler.session(); if sess.opts.describe_lints { diff --git a/src/librustdoc/markdown.rs b/src/librustdoc/markdown.rs index 0b557ef244e94..eb64ac455dc4c 100644 --- a/src/librustdoc/markdown.rs +++ b/src/librustdoc/markdown.rs @@ -5,7 +5,6 @@ use std::path::Path; use rustc_span::edition::Edition; use rustc_span::source_map::DUMMY_SP; -use rustc_span::Symbol; use crate::config::{Options, RenderOptions}; use crate::doctest::{Collector, GlobalTestOptions}; @@ -36,6 +35,8 @@ fn extract_leading_metadata(s: &str) -> (Vec<&str>, &str) { /// Render `input` (e.g., "foo.md") into an HTML file in `output` /// (e.g., output = "bar" => "bar/foo.html"). +/// +/// Requires session globals to be available, for symbol interning. pub(crate) fn render>( input: P, options: RenderOptions, @@ -133,7 +134,7 @@ pub(crate) fn test(options: Options) -> Result<(), String> { let mut opts = GlobalTestOptions::default(); opts.no_crate_inject = true; let mut collector = Collector::new( - Symbol::intern(&options.input.display().to_string()), + options.input.display().to_string(), options.clone(), true, opts, From c461f3a760f4a7eeef82bd1a9389d68210abaf4a Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Fri, 7 Oct 2022 15:36:44 +1100 Subject: [PATCH 02/23] Merge `main_options` into `main_args`. There is no longer any need for them to be separate. --- src/librustdoc/lib.rs | 57 ++++++++++++++++++++----------------------- 1 file changed, 27 insertions(+), 30 deletions(-) diff --git a/src/librustdoc/lib.rs b/src/librustdoc/lib.rs index ce6f7e817c620..7e0cc668d7b59 100644 --- a/src/librustdoc/lib.rs +++ b/src/librustdoc/lib.rs @@ -674,35 +674,6 @@ fn usage(argv0: &str) { /// A result type used by several functions under `main()`. type MainResult = Result<(), ErrorGuaranteed>; -fn main_args(at_args: &[String]) -> MainResult { - let args = rustc_driver::args::arg_expand_all(at_args); - - let mut options = getopts::Options::new(); - for option in opts() { - (option.apply)(&mut options); - } - let matches = match options.parse(&args[1..]) { - Ok(m) => m, - Err(err) => { - early_error(ErrorOutputType::default(), &err.to_string()); - } - }; - - // Note that we discard any distinction between different non-zero exit - // codes from `from_matches` here. - let options = match config::Options::from_matches(&matches, args) { - Ok(opts) => opts, - Err(code) => { - return if code == 0 { - Ok(()) - } else { - Err(ErrorGuaranteed::unchecked_claim_error_was_emitted()) - }; - } - }; - main_options(options) -} - fn wrap_return(diag: &rustc_errors::Handler, res: Result<(), String>) -> MainResult { match res { Ok(()) => Ok(()), @@ -733,7 +704,33 @@ fn run_renderer<'tcx, T: formats::FormatRenderer<'tcx>>( } } -fn main_options(options: config::Options) -> MainResult { +fn main_args(at_args: &[String]) -> MainResult { + let args = rustc_driver::args::arg_expand_all(at_args); + + let mut options = getopts::Options::new(); + for option in opts() { + (option.apply)(&mut options); + } + let matches = match options.parse(&args[1..]) { + Ok(m) => m, + Err(err) => { + early_error(ErrorOutputType::default(), &err.to_string()); + } + }; + + // Note that we discard any distinction between different non-zero exit + // codes from `from_matches` here. + let options = match config::Options::from_matches(&matches, args) { + Ok(opts) => opts, + Err(code) => { + return if code == 0 { + Ok(()) + } else { + Err(ErrorGuaranteed::unchecked_claim_error_was_emitted()) + }; + } + }; + let diag = core::new_handler( options.error_format, None, From d156a9092738b3f1fb5e665f532613ab06380162 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Fri, 7 Oct 2022 11:57:25 +1100 Subject: [PATCH 03/23] Inline and remove `scoped_thread`. It has a single call site, and removing it slightly improves the confusing tangle of nested closures present at startup. --- compiler/rustc_interface/src/util.rs | 24 +++++++++--------------- 1 file changed, 9 insertions(+), 15 deletions(-) diff --git a/compiler/rustc_interface/src/util.rs b/compiler/rustc_interface/src/util.rs index f7e70d355cf86..c07bfb9c23e4a 100644 --- a/compiler/rustc_interface/src/util.rs +++ b/compiler/rustc_interface/src/util.rs @@ -132,33 +132,27 @@ fn get_stack_size() -> Option { env::var_os("RUST_MIN_STACK").is_none().then_some(STACK_SIZE) } -/// Like a `thread::Builder::spawn` followed by a `join()`, but avoids the need -/// for `'static` bounds. -#[cfg(not(parallel_compiler))] -fn scoped_thread R + Send, R: Send>(cfg: thread::Builder, f: F) -> R { - // SAFETY: join() is called immediately, so any closure captures are still - // alive. - match unsafe { cfg.spawn_unchecked(f) }.unwrap().join() { - Ok(v) => v, - Err(e) => panic::resume_unwind(e), - } -} - #[cfg(not(parallel_compiler))] pub fn run_in_thread_pool_with_globals R + Send, R: Send>( edition: Edition, _threads: usize, f: F, ) -> R { + // The thread pool is a single thread in the non-parallel compiler. let mut cfg = thread::Builder::new().name("rustc".to_string()); - if let Some(size) = get_stack_size() { cfg = cfg.stack_size(size); } - let main_handler = move || rustc_span::create_session_globals_then(edition, f); + let f = move || rustc_span::create_session_globals_then(edition, f); - scoped_thread(cfg, main_handler) + // This avoids the need for `'static` bounds. + // + // SAFETY: join() is called immediately, so any closure captures are still alive. + match unsafe { cfg.spawn_unchecked(f) }.unwrap().join() { + Ok(v) => v, + Err(e) => panic::resume_unwind(e), + } } /// Creates a new thread and forwards information in thread locals to it. From 226387a4039082b2df687c645acb4bde08f2939d Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Fri, 7 Oct 2022 15:35:42 +1100 Subject: [PATCH 04/23] Reduce visibility of some functions. --- compiler/rustc_interface/src/interface.rs | 2 +- compiler/rustc_interface/src/util.rs | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/compiler/rustc_interface/src/interface.rs b/compiler/rustc_interface/src/interface.rs index 949bd02ad6839..d0555f3db0c9b 100644 --- a/compiler/rustc_interface/src/interface.rs +++ b/compiler/rustc_interface/src/interface.rs @@ -276,7 +276,7 @@ pub struct Config { pub registry: Registry, } -pub fn create_compiler_and_run(config: Config, f: impl FnOnce(&Compiler) -> R) -> R { +fn create_compiler_and_run(config: Config, f: impl FnOnce(&Compiler) -> R) -> R { crate::callbacks::setup_callbacks(); let registry = &config.registry; diff --git a/compiler/rustc_interface/src/util.rs b/compiler/rustc_interface/src/util.rs index c07bfb9c23e4a..7ec0d4d73f7a4 100644 --- a/compiler/rustc_interface/src/util.rs +++ b/compiler/rustc_interface/src/util.rs @@ -133,7 +133,7 @@ fn get_stack_size() -> Option { } #[cfg(not(parallel_compiler))] -pub fn run_in_thread_pool_with_globals R + Send, R: Send>( +pub(crate) fn run_in_thread_pool_with_globals R + Send, R: Send>( edition: Edition, _threads: usize, f: F, @@ -171,7 +171,7 @@ unsafe fn handle_deadlock() { } #[cfg(parallel_compiler)] -pub fn run_in_thread_pool_with_globals R + Send, R: Send>( +pub(crate) fn run_in_thread_pool_with_globals R + Send, R: Send>( edition: Edition, threads: usize, f: F, From c00937f2d9b2ea994e5841c5f6047a603eeffe85 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Fri, 7 Oct 2022 16:17:57 +1100 Subject: [PATCH 05/23] Inline and remove `create_compiler_and_run`. It has a single call site. --- compiler/rustc_interface/src/interface.rs | 106 +++++++++++----------- 1 file changed, 52 insertions(+), 54 deletions(-) diff --git a/compiler/rustc_interface/src/interface.rs b/compiler/rustc_interface/src/interface.rs index d0555f3db0c9b..bee2e91c74730 100644 --- a/compiler/rustc_interface/src/interface.rs +++ b/compiler/rustc_interface/src/interface.rs @@ -276,59 +276,6 @@ pub struct Config { pub registry: Registry, } -fn create_compiler_and_run(config: Config, f: impl FnOnce(&Compiler) -> R) -> R { - crate::callbacks::setup_callbacks(); - - let registry = &config.registry; - let (mut sess, codegen_backend) = util::create_session( - config.opts, - config.crate_cfg, - config.crate_check_cfg, - config.diagnostic_output, - config.file_loader, - config.input_path.clone(), - config.lint_caps, - config.make_codegen_backend, - registry.clone(), - ); - - if let Some(parse_sess_created) = config.parse_sess_created { - parse_sess_created( - &mut Lrc::get_mut(&mut sess) - .expect("create_session() should never share the returned session") - .parse_sess, - ); - } - - let temps_dir = sess.opts.unstable_opts.temps_dir.as_ref().map(|o| PathBuf::from(&o)); - - let compiler = Compiler { - sess, - codegen_backend, - input: config.input, - input_path: config.input_path, - output_dir: config.output_dir, - output_file: config.output_file, - temps_dir, - register_lints: config.register_lints, - override_queries: config.override_queries, - }; - - rustc_span::with_source_map(compiler.sess.parse_sess.clone_source_map(), move || { - let r = { - let _sess_abort_error = OnDrop(|| { - compiler.sess.finish_diagnostics(registry); - }); - - f(&compiler) - }; - - let prof = compiler.sess.prof.clone(); - prof.generic_activity("drop_compiler").run(move || drop(compiler)); - r - }) -} - // JUSTIFICATION: before session exists, only config #[allow(rustc::bad_opt_access)] pub fn run_compiler(config: Config, f: impl FnOnce(&Compiler) -> R + Send) -> R { @@ -336,7 +283,58 @@ pub fn run_compiler(config: Config, f: impl FnOnce(&Compiler) -> R + Se util::run_in_thread_pool_with_globals( config.opts.edition, config.opts.unstable_opts.threads, - || create_compiler_and_run(config, f), + || { + crate::callbacks::setup_callbacks(); + + let registry = &config.registry; + let (mut sess, codegen_backend) = util::create_session( + config.opts, + config.crate_cfg, + config.crate_check_cfg, + config.diagnostic_output, + config.file_loader, + config.input_path.clone(), + config.lint_caps, + config.make_codegen_backend, + registry.clone(), + ); + + if let Some(parse_sess_created) = config.parse_sess_created { + parse_sess_created( + &mut Lrc::get_mut(&mut sess) + .expect("create_session() should never share the returned session") + .parse_sess, + ); + } + + let temps_dir = sess.opts.unstable_opts.temps_dir.as_ref().map(|o| PathBuf::from(&o)); + + let compiler = Compiler { + sess, + codegen_backend, + input: config.input, + input_path: config.input_path, + output_dir: config.output_dir, + output_file: config.output_file, + temps_dir, + register_lints: config.register_lints, + override_queries: config.override_queries, + }; + + rustc_span::with_source_map(compiler.sess.parse_sess.clone_source_map(), move || { + let r = { + let _sess_abort_error = OnDrop(|| { + compiler.sess.finish_diagnostics(registry); + }); + + f(&compiler) + }; + + let prof = compiler.sess.prof.clone(); + prof.generic_activity("drop_compiler").run(move || drop(compiler)); + r + }) + }, ) } From 806701607285b6a61c6169797b5e22ef9a18d5de Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Fri, 7 Oct 2022 16:20:20 +1100 Subject: [PATCH 06/23] Apply `Lrc` later to `sess` and `codegen_backend`. This avoids the need for a degenerate `Lrc::get_mut` call. --- compiler/rustc_interface/src/interface.rs | 10 +++------- compiler/rustc_interface/src/util.rs | 5 ++--- 2 files changed, 5 insertions(+), 10 deletions(-) diff --git a/compiler/rustc_interface/src/interface.rs b/compiler/rustc_interface/src/interface.rs index bee2e91c74730..a697f8071b4b3 100644 --- a/compiler/rustc_interface/src/interface.rs +++ b/compiler/rustc_interface/src/interface.rs @@ -300,18 +300,14 @@ pub fn run_compiler(config: Config, f: impl FnOnce(&Compiler) -> R + Se ); if let Some(parse_sess_created) = config.parse_sess_created { - parse_sess_created( - &mut Lrc::get_mut(&mut sess) - .expect("create_session() should never share the returned session") - .parse_sess, - ); + parse_sess_created(&mut sess.parse_sess); } let temps_dir = sess.opts.unstable_opts.temps_dir.as_ref().map(|o| PathBuf::from(&o)); let compiler = Compiler { - sess, - codegen_backend, + sess: Lrc::new(sess), + codegen_backend: Lrc::new(codegen_backend), input: config.input, input_path: config.input_path, output_dir: config.output_dir, diff --git a/compiler/rustc_interface/src/util.rs b/compiler/rustc_interface/src/util.rs index 7ec0d4d73f7a4..ae7a240d81cb4 100644 --- a/compiler/rustc_interface/src/util.rs +++ b/compiler/rustc_interface/src/util.rs @@ -5,7 +5,6 @@ use rustc_codegen_ssa::traits::CodegenBackend; use rustc_data_structures::fx::{FxHashMap, FxHashSet}; #[cfg(parallel_compiler)] use rustc_data_structures::jobserver; -use rustc_data_structures::sync::Lrc; use rustc_errors::registry::Registry; #[cfg(parallel_compiler)] use rustc_middle::ty::tls; @@ -73,7 +72,7 @@ pub fn create_session( Box Box + Send>, >, descriptions: Registry, -) -> (Lrc, Lrc>) { +) -> (Session, Box) { let codegen_backend = if let Some(make_codegen_backend) = make_codegen_backend { make_codegen_backend(&sopts) } else { @@ -121,7 +120,7 @@ pub fn create_session( sess.parse_sess.config = cfg; sess.parse_sess.check_config = check_cfg; - (Lrc::new(sess), Lrc::new(codegen_backend)) + (sess, codegen_backend) } const STACK_SIZE: usize = 8 * 1024 * 1024; From 1f0463a7b5ed89cedf25842e7c2917efc9200939 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Sat, 8 Oct 2022 07:43:15 +1100 Subject: [PATCH 07/23] Replace a `spawn_unchecked` with `spawn_scoped`. --- compiler/rustc_interface/src/util.rs | 28 ++++++++++++++++------------ 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/compiler/rustc_interface/src/util.rs b/compiler/rustc_interface/src/util.rs index ae7a240d81cb4..1bbfb9a415699 100644 --- a/compiler/rustc_interface/src/util.rs +++ b/compiler/rustc_interface/src/util.rs @@ -138,20 +138,24 @@ pub(crate) fn run_in_thread_pool_with_globals R + Send, R: Send>( f: F, ) -> R { // The thread pool is a single thread in the non-parallel compiler. - let mut cfg = thread::Builder::new().name("rustc".to_string()); - if let Some(size) = get_stack_size() { - cfg = cfg.stack_size(size); - } + thread::scope(|s| { + let mut builder = thread::Builder::new().name("rustc".to_string()); + if let Some(size) = get_stack_size() { + builder = builder.stack_size(size); + } - let f = move || rustc_span::create_session_globals_then(edition, f); + // `unwrap` is ok here because `spawn_scoped` only panics if the thread + // name contains null bytes. + let r = builder + .spawn_scoped(s, move || rustc_span::create_session_globals_then(edition, f)) + .unwrap() + .join(); - // This avoids the need for `'static` bounds. - // - // SAFETY: join() is called immediately, so any closure captures are still alive. - match unsafe { cfg.spawn_unchecked(f) }.unwrap().join() { - Ok(v) => v, - Err(e) => panic::resume_unwind(e), - } + match r { + Ok(v) => v, + Err(e) => panic::resume_unwind(e), + } + }) } /// Creates a new thread and forwards information in thread locals to it. From 88bb4e4bda2e5ccafbc28ded2ef289841d5fb655 Mon Sep 17 00:00:00 2001 From: Josh Triplett Date: Sun, 9 Oct 2022 15:31:27 +0100 Subject: [PATCH 08/23] impl AsFd for io::{Stdin, Stdout, Stderr}, not the sys versions https://github.com/rust-lang/rust/pull/100892 implemented AsFd for the sys versions, rather than for the public types. Change the implementations to apply to the public types. --- library/std/src/sys/wasi/stdio.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/library/std/src/sys/wasi/stdio.rs b/library/std/src/sys/wasi/stdio.rs index bf045c7841f63..427dcf6bb0699 100644 --- a/library/std/src/sys/wasi/stdio.rs +++ b/library/std/src/sys/wasi/stdio.rs @@ -23,7 +23,7 @@ impl AsRawFd for Stdin { } } -impl AsFd for Stdin { +impl AsFd for io::Stdin { #[inline] fn as_fd(&self) -> BorrowedFd<'_> { unsafe { BorrowedFd::borrow_raw(0) } @@ -67,7 +67,7 @@ impl AsRawFd for Stdout { } } -impl AsFd for Stdout { +impl AsFd for io::Stdout { #[inline] fn as_fd(&self) -> BorrowedFd<'_> { unsafe { BorrowedFd::borrow_raw(1) } @@ -114,7 +114,7 @@ impl AsRawFd for Stderr { } } -impl AsFd for Stderr { +impl AsFd for io::Stderr { #[inline] fn as_fd(&self) -> BorrowedFd<'_> { unsafe { BorrowedFd::borrow_raw(2) } From febbf71219001cd7948c3d1e5ffa68706896d955 Mon Sep 17 00:00:00 2001 From: David Wood Date: Mon, 3 Oct 2022 15:34:00 +0100 Subject: [PATCH 09/23] 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 10/23] 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 11/23] 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 12/23] 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 13/23] 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 14/23] 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 15/23] 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 16/23] 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 d17a69e4535a73b1c556f602e97cc576ef69fadd Mon Sep 17 00:00:00 2001 From: Urgau Date: Mon, 10 Oct 2022 15:45:05 +0200 Subject: [PATCH 17/23] Fix stabilization of `feature(half_open_range_patterns)` --- compiler/rustc_feature/src/active.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/compiler/rustc_feature/src/active.rs b/compiler/rustc_feature/src/active.rs index 48c40eae662ed..5ea433e6b3d3e 100644 --- a/compiler/rustc_feature/src/active.rs +++ b/compiler/rustc_feature/src/active.rs @@ -412,8 +412,6 @@ declare_features! ( (incomplete, generic_associated_types_extended, "1.61.0", Some(95451), None), /// Allows non-trivial generic constants which have to have wfness manually propagated to callers (incomplete, generic_const_exprs, "1.56.0", Some(76560), None), - /// Allows using `..X`, `..=X`, `...X`, and `X..` as a pattern. - (active, half_open_range_patterns, "1.41.0", Some(67264), None), /// Allows using `..=X` as a patterns in slices. (active, half_open_range_patterns_in_slices, "CURRENT_RUSTC_VERSION", Some(67264), None), /// Allows `if let` guard in match arms. From ef68327de76d5f2bb6f9b2a6fa47b92fbf3ff7cc Mon Sep 17 00:00:00 2001 From: Josh Triplett Date: Mon, 10 Oct 2022 14:47:22 +0100 Subject: [PATCH 18/23] Consolidate AsFd instances for stdio types into `library/std/src/os/fd/owned.rs` --- library/std/src/os/fd/owned.rs | 52 +++++++++++++++++++++++++++++++ library/std/src/sys/unix/stdio.rs | 50 +---------------------------- library/std/src/sys/wasi/stdio.rs | 50 +---------------------------- 3 files changed, 54 insertions(+), 98 deletions(-) diff --git a/library/std/src/os/fd/owned.rs b/library/std/src/os/fd/owned.rs index 9875c389d8aaf..9d758320cfcc6 100644 --- a/library/std/src/os/fd/owned.rs +++ b/library/std/src/os/fd/owned.rs @@ -6,6 +6,7 @@ use super::raw::{AsRawFd, FromRawFd, IntoRawFd, RawFd}; use crate::fmt; use crate::fs; +use crate::io; use crate::marker::PhantomData; use crate::mem::forget; #[cfg(not(any(target_arch = "wasm32", target_env = "sgx")))] @@ -385,3 +386,54 @@ impl AsFd for Box { (**self).as_fd() } } + +#[stable(feature = "io_safety", since = "1.63.0")] +impl AsFd for io::Stdin { + #[inline] + fn as_fd(&self) -> BorrowedFd<'_> { + unsafe { BorrowedFd::borrow_raw(0) } + } +} + +#[stable(feature = "io_safety", since = "1.63.0")] +impl<'a> AsFd for io::StdinLock<'a> { + #[inline] + fn as_fd(&self) -> BorrowedFd<'_> { + // SAFETY: user code should not close stdin out from under the standard library + unsafe { BorrowedFd::borrow_raw(0) } + } +} + +#[stable(feature = "io_safety", since = "1.63.0")] +impl AsFd for io::Stdout { + #[inline] + fn as_fd(&self) -> BorrowedFd<'_> { + unsafe { BorrowedFd::borrow_raw(1) } + } +} + +#[stable(feature = "io_safety", since = "1.63.0")] +impl<'a> AsFd for io::StdoutLock<'a> { + #[inline] + fn as_fd(&self) -> BorrowedFd<'_> { + // SAFETY: user code should not close stdout out from under the standard library + unsafe { BorrowedFd::borrow_raw(1) } + } +} + +#[stable(feature = "io_safety", since = "1.63.0")] +impl AsFd for io::Stderr { + #[inline] + fn as_fd(&self) -> BorrowedFd<'_> { + unsafe { BorrowedFd::borrow_raw(2) } + } +} + +#[stable(feature = "io_safety", since = "1.63.0")] +impl<'a> AsFd for io::StderrLock<'a> { + #[inline] + fn as_fd(&self) -> BorrowedFd<'_> { + // SAFETY: user code should not close stderr out from under the standard library + unsafe { BorrowedFd::borrow_raw(2) } + } +} diff --git a/library/std/src/sys/unix/stdio.rs b/library/std/src/sys/unix/stdio.rs index 329f9433dba0e..b3626c564e86a 100644 --- a/library/std/src/sys/unix/stdio.rs +++ b/library/std/src/sys/unix/stdio.rs @@ -1,6 +1,6 @@ use crate::io::{self, IoSlice, IoSliceMut}; use crate::mem::ManuallyDrop; -use crate::os::unix::io::{AsFd, BorrowedFd, FromRawFd}; +use crate::os::unix::io::FromRawFd; use crate::sys::fd::FileDesc; pub struct Stdin(()); @@ -91,51 +91,3 @@ pub const STDIN_BUF_SIZE: usize = crate::sys_common::io::DEFAULT_BUF_SIZE; pub fn panic_output() -> Option { Some(Stderr::new()) } - -#[stable(feature = "io_safety", since = "1.63.0")] -impl AsFd for io::Stdin { - #[inline] - fn as_fd(&self) -> BorrowedFd<'_> { - unsafe { BorrowedFd::borrow_raw(libc::STDIN_FILENO) } - } -} - -#[stable(feature = "io_safety", since = "1.63.0")] -impl<'a> AsFd for io::StdinLock<'a> { - #[inline] - fn as_fd(&self) -> BorrowedFd<'_> { - unsafe { BorrowedFd::borrow_raw(libc::STDIN_FILENO) } - } -} - -#[stable(feature = "io_safety", since = "1.63.0")] -impl AsFd for io::Stdout { - #[inline] - fn as_fd(&self) -> BorrowedFd<'_> { - unsafe { BorrowedFd::borrow_raw(libc::STDOUT_FILENO) } - } -} - -#[stable(feature = "io_safety", since = "1.63.0")] -impl<'a> AsFd for io::StdoutLock<'a> { - #[inline] - fn as_fd(&self) -> BorrowedFd<'_> { - unsafe { BorrowedFd::borrow_raw(libc::STDOUT_FILENO) } - } -} - -#[stable(feature = "io_safety", since = "1.63.0")] -impl AsFd for io::Stderr { - #[inline] - fn as_fd(&self) -> BorrowedFd<'_> { - unsafe { BorrowedFd::borrow_raw(libc::STDERR_FILENO) } - } -} - -#[stable(feature = "io_safety", since = "1.63.0")] -impl<'a> AsFd for io::StderrLock<'a> { - #[inline] - fn as_fd(&self) -> BorrowedFd<'_> { - unsafe { BorrowedFd::borrow_raw(libc::STDERR_FILENO) } - } -} diff --git a/library/std/src/sys/wasi/stdio.rs b/library/std/src/sys/wasi/stdio.rs index 427dcf6bb0699..4cc0e4ed5a45a 100644 --- a/library/std/src/sys/wasi/stdio.rs +++ b/library/std/src/sys/wasi/stdio.rs @@ -4,7 +4,7 @@ use super::fd::WasiFd; use crate::io::{self, IoSlice, IoSliceMut}; use crate::mem::ManuallyDrop; use crate::os::raw; -use crate::os::wasi::io::{AsFd, AsRawFd, BorrowedFd, FromRawFd}; +use crate::os::wasi::io::{AsRawFd, FromRawFd}; pub struct Stdin; pub struct Stdout; @@ -23,22 +23,6 @@ impl AsRawFd for Stdin { } } -impl AsFd for io::Stdin { - #[inline] - fn as_fd(&self) -> BorrowedFd<'_> { - unsafe { BorrowedFd::borrow_raw(0) } - } -} - -#[stable(feature = "io_safety", since = "1.63.0")] -impl<'a> AsFd for io::StdinLock<'a> { - #[inline] - fn as_fd(&self) -> BorrowedFd<'_> { - // SAFETY: user code should not close stdin out from under the standard library - unsafe { BorrowedFd::borrow_raw(0) } - } -} - impl io::Read for Stdin { fn read(&mut self, data: &mut [u8]) -> io::Result { self.read_vectored(&mut [IoSliceMut::new(data)]) @@ -67,22 +51,6 @@ impl AsRawFd for Stdout { } } -impl AsFd for io::Stdout { - #[inline] - fn as_fd(&self) -> BorrowedFd<'_> { - unsafe { BorrowedFd::borrow_raw(1) } - } -} - -#[stable(feature = "io_safety", since = "1.63.0")] -impl<'a> AsFd for io::StdoutLock<'a> { - #[inline] - fn as_fd(&self) -> BorrowedFd<'_> { - // SAFETY: user code should not close stdout out from under the standard library - unsafe { BorrowedFd::borrow_raw(1) } - } -} - impl io::Write for Stdout { fn write(&mut self, data: &[u8]) -> io::Result { self.write_vectored(&[IoSlice::new(data)]) @@ -114,22 +82,6 @@ impl AsRawFd for Stderr { } } -impl AsFd for io::Stderr { - #[inline] - fn as_fd(&self) -> BorrowedFd<'_> { - unsafe { BorrowedFd::borrow_raw(2) } - } -} - -#[stable(feature = "io_safety", since = "1.63.0")] -impl<'a> AsFd for io::StderrLock<'a> { - #[inline] - fn as_fd(&self) -> BorrowedFd<'_> { - // SAFETY: user code should not close stderr out from under the standard library - unsafe { BorrowedFd::borrow_raw(2) } - } -} - impl io::Write for Stderr { fn write(&mut self, data: &[u8]) -> io::Result { self.write_vectored(&[IoSlice::new(data)]) From 87060c42ff2e912379af41c174885bd93f0ddfda Mon Sep 17 00:00:00 2001 From: Michael Howell Date: Tue, 11 Oct 2022 14:46:49 -0700 Subject: [PATCH 19/23] rustdoc: remove unused CSS `nav.sum` This was added in 4fd061c426902b0904c65e64a3780b21f9ab3afb, but never actually used. --- src/librustdoc/html/static/css/rustdoc.css | 1 - 1 file changed, 1 deletion(-) diff --git a/src/librustdoc/html/static/css/rustdoc.css b/src/librustdoc/html/static/css/rustdoc.css index 661aed71298d9..ec7ede010538f 100644 --- a/src/librustdoc/html/static/css/rustdoc.css +++ b/src/librustdoc/html/static/css/rustdoc.css @@ -718,7 +718,6 @@ nav.sub { .source nav.sub { margin-left: 32px; } -nav.sum { text-align: right; } nav.sub form { display: inline; } a { From 46169e66583f6f2abf9136fff2b60aabaf1f4014 Mon Sep 17 00:00:00 2001 From: Michael Howell Date: Tue, 11 Oct 2022 15:05:22 -0700 Subject: [PATCH 20/23] rustdoc: merge identical CSS selectors --- src/librustdoc/html/static/css/rustdoc.css | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/librustdoc/html/static/css/rustdoc.css b/src/librustdoc/html/static/css/rustdoc.css index ec7ede010538f..d23d88d69dc55 100644 --- a/src/librustdoc/html/static/css/rustdoc.css +++ b/src/librustdoc/html/static/css/rustdoc.css @@ -563,8 +563,6 @@ h2.location a { .rustdoc .example-wrap { display: flex; position: relative; -} -.rustdoc .example-wrap { margin-bottom: 10px; } /* For the last child of a div, the margin will be taken care of From f8048aa3a5cf1cfbc1d9778d531b3e25189016e5 Mon Sep 17 00:00:00 2001 From: Eric Huss Date: Tue, 11 Oct 2022 16:04:17 -0700 Subject: [PATCH 21/23] Update books --- src/doc/embedded-book | 2 +- src/doc/nomicon | 2 +- src/doc/reference | 2 +- src/doc/rust-by-example | 2 +- src/doc/rustc-dev-guide | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/doc/embedded-book b/src/doc/embedded-book index 4ce51cb7441a6..c533348edd69f 160000 --- a/src/doc/embedded-book +++ b/src/doc/embedded-book @@ -1 +1 @@ -Subproject commit 4ce51cb7441a6f02b5bf9b07b2eb755c21ab7954 +Subproject commit c533348edd69f11a8f4225d633a05d7093fddbf3 diff --git a/src/doc/nomicon b/src/doc/nomicon index f53bfa0569292..9c73283775466 160000 --- a/src/doc/nomicon +++ b/src/doc/nomicon @@ -1 +1 @@ -Subproject commit f53bfa056929217870a5d2df1366d2e7ba35096d +Subproject commit 9c73283775466d22208a0b28afcab44db4c0cc10 diff --git a/src/doc/reference b/src/doc/reference index a7cdac33ca735..f6ed74f582bdd 160000 --- a/src/doc/reference +++ b/src/doc/reference @@ -1 +1 @@ -Subproject commit a7cdac33ca7356ad49d5c2b5e2c5010889b33eee +Subproject commit f6ed74f582bddcec73f753eafaab3749c4f7df61 diff --git a/src/doc/rust-by-example b/src/doc/rust-by-example index 767a6bd9727a5..5e7b296d6c345 160000 --- a/src/doc/rust-by-example +++ b/src/doc/rust-by-example @@ -1 +1 @@ -Subproject commit 767a6bd9727a596d7cfdbaeee475e65b2670ea3a +Subproject commit 5e7b296d6c345addbd748f242aae28c42555c015 diff --git a/src/doc/rustc-dev-guide b/src/doc/rustc-dev-guide index 9a86c0467bbe4..7518c3445dc02 160000 --- a/src/doc/rustc-dev-guide +++ b/src/doc/rustc-dev-guide @@ -1 +1 @@ -Subproject commit 9a86c0467bbe42056f73fdf5b03fff757d7c4a9b +Subproject commit 7518c3445dc02df0d196f5f84e568d633c5141fb From c646c4d403d8c88cd3450f859fbdc34ec26d5bb5 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Sun, 9 Oct 2022 06:54:00 +0000 Subject: [PATCH 22/23] Unify tcx.constness and param env constness checks --- .../src/const_eval/fn_queries.rs | 72 ++++++++++++++----- compiler/rustc_metadata/src/rmeta/encoder.rs | 56 +++++++++++---- compiler/rustc_ty_utils/src/ty.rs | 69 +----------------- 3 files changed, 98 insertions(+), 99 deletions(-) diff --git a/compiler/rustc_const_eval/src/const_eval/fn_queries.rs b/compiler/rustc_const_eval/src/const_eval/fn_queries.rs index f1674d04f8d15..cdcebb61c2e8c 100644 --- a/compiler/rustc_const_eval/src/const_eval/fn_queries.rs +++ b/compiler/rustc_const_eval/src/const_eval/fn_queries.rs @@ -25,12 +25,10 @@ pub fn is_parent_const_impl_raw(tcx: TyCtxt<'_>, def_id: LocalDefId) -> bool { /// report whether said intrinsic has a `rustc_const_{un,}stable` attribute. Otherwise, return /// `Constness::NotConst`. fn constness(tcx: TyCtxt<'_>, def_id: DefId) -> hir::Constness { - let def_id = def_id.expect_local(); - let node = tcx.hir().get_by_def_id(def_id); - - match node { + let hir_id = tcx.hir().local_def_id_to_hir_id(def_id.expect_local()); + match tcx.hir().get(hir_id) { hir::Node::Ctor(_) => hir::Constness::Const, - hir::Node::Item(hir::Item { kind: hir::ItemKind::Impl(impl_), .. }) => impl_.constness, + hir::Node::ForeignItem(hir::ForeignItem { kind: hir::ForeignItemKind::Fn(..), .. }) => { // Intrinsics use `rustc_const_{un,}stable` attributes to indicate constness. All other // foreign items cannot be evaluated at compile-time. @@ -41,20 +39,62 @@ fn constness(tcx: TyCtxt<'_>, def_id: DefId) -> hir::Constness { }; if is_const { hir::Constness::Const } else { hir::Constness::NotConst } } - _ => { - if let Some(fn_kind) = node.fn_kind() { - if fn_kind.constness() == hir::Constness::Const { - return hir::Constness::Const; - } - // If the function itself is not annotated with `const`, it may still be a `const fn` - // if it resides in a const trait impl. - let is_const = is_parent_const_impl_raw(tcx, def_id); - if is_const { hir::Constness::Const } else { hir::Constness::NotConst } - } else { - hir::Constness::NotConst + hir::Node::TraitItem(hir::TraitItem { kind: hir::TraitItemKind::Fn(..), .. }) + if tcx.is_const_default_method(def_id) => + { + hir::Constness::Const + } + + hir::Node::Item(hir::Item { kind: hir::ItemKind::Const(..), .. }) + | hir::Node::Item(hir::Item { kind: hir::ItemKind::Static(..), .. }) + | hir::Node::TraitItem(hir::TraitItem { kind: hir::TraitItemKind::Const(..), .. }) + | hir::Node::AnonConst(_) + | hir::Node::ImplItem(hir::ImplItem { kind: hir::ImplItemKind::Const(..), .. }) + | hir::Node::ImplItem(hir::ImplItem { + kind: + hir::ImplItemKind::Fn( + hir::FnSig { + header: hir::FnHeader { constness: hir::Constness::Const, .. }, + .. + }, + .., + ), + .. + }) => hir::Constness::Const, + + hir::Node::ImplItem(hir::ImplItem { + kind: hir::ImplItemKind::Type(..) | hir::ImplItemKind::Fn(..), + .. + }) => { + let parent_hir_id = tcx.hir().get_parent_node(hir_id); + match tcx.hir().get(parent_hir_id) { + hir::Node::Item(hir::Item { + kind: hir::ItemKind::Impl(hir::Impl { constness, .. }), + .. + }) => *constness, + _ => span_bug!( + tcx.def_span(parent_hir_id.owner), + "impl item's parent node is not an impl", + ), } } + + hir::Node::Item(hir::Item { + kind: hir::ItemKind::Fn(hir::FnSig { header: hir::FnHeader { constness, .. }, .. }, ..), + .. + }) + | hir::Node::TraitItem(hir::TraitItem { + kind: + hir::TraitItemKind::Fn(hir::FnSig { header: hir::FnHeader { constness, .. }, .. }, ..), + .. + }) + | hir::Node::Item(hir::Item { + kind: hir::ItemKind::Impl(hir::Impl { constness, .. }), + .. + }) => *constness, + + _ => hir::Constness::NotConst, } } diff --git a/compiler/rustc_metadata/src/rmeta/encoder.rs b/compiler/rustc_metadata/src/rmeta/encoder.rs index 3fc10197b9129..ab4ecec2af36e 100644 --- a/compiler/rustc_metadata/src/rmeta/encoder.rs +++ b/compiler/rustc_metadata/src/rmeta/encoder.rs @@ -1059,6 +1059,43 @@ fn should_encode_const(def_kind: DefKind) -> bool { } } +fn should_encode_constness(def_kind: DefKind) -> bool { + match def_kind { + DefKind::Struct + | DefKind::Union + | DefKind::Enum + | DefKind::Trait + | DefKind::AssocTy + | DefKind::Fn + | DefKind::Const + | DefKind::Static(..) + | DefKind::Ctor(..) + | DefKind::AssocFn + | DefKind::AssocConst + | DefKind::AnonConst + | DefKind::InlineConst + | DefKind::OpaqueTy + | DefKind::ImplTraitPlaceholder + | DefKind::Impl + | DefKind::Closure + | DefKind::Generator => true, + DefKind::Variant + | DefKind::TyAlias + | DefKind::TraitAlias + | DefKind::ForeignTy + | DefKind::Field + | DefKind::TyParam + | DefKind::Mod + | DefKind::ForeignMod + | DefKind::ConstParam + | DefKind::Macro(..) + | DefKind::Use + | DefKind::LifetimeParam + | DefKind::GlobalAsm + | DefKind::ExternCrate => false, + } +} + fn should_encode_trait_impl_trait_tys<'tcx>(tcx: TyCtxt<'tcx>, def_id: DefId) -> bool { if tcx.def_kind(def_id) != DefKind::AssocFn { return false; @@ -1165,6 +1202,9 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> { { record!(self.tables.trait_impl_trait_tys[def_id] <- table); } + if should_encode_constness(def_kind) { + self.tables.constness.set(def_id.index, tcx.constness(def_id)); + } } let inherent_impls = tcx.crate_inherent_impls(()); for (def_id, implementations) in inherent_impls.inherent_impls.iter() { @@ -1192,7 +1232,6 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> { }; record!(self.tables.variant_data[def_id] <- data); - self.tables.constness.set(def_id.index, hir::Constness::Const); record_array!(self.tables.children[def_id] <- variant.fields.iter().map(|f| { assert!(f.did.is_local()); f.did.index @@ -1220,7 +1259,6 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> { }; record!(self.tables.variant_data[def_id] <- data); - self.tables.constness.set(def_id.index, hir::Constness::Const); if variant.ctor_kind == CtorKind::Fn { record!(self.tables.fn_sig[def_id] <- tcx.fn_sig(def_id)); } @@ -1284,7 +1322,6 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> { record!(self.tables.repr_options[def_id] <- adt_def.repr()); record!(self.tables.variant_data[def_id] <- data); - self.tables.constness.set(def_id.index, hir::Constness::Const); if variant.ctor_kind == CtorKind::Fn { record!(self.tables.fn_sig[def_id] <- tcx.fn_sig(def_id)); } @@ -1320,7 +1357,6 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> { } }; self.tables.asyncness.set(def_id.index, m_sig.header.asyncness); - self.tables.constness.set(def_id.index, hir::Constness::NotConst); } ty::AssocKind::Type => { self.encode_explicit_item_bounds(def_id); @@ -1345,13 +1381,6 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> { let hir::ImplItemKind::Fn(ref sig, body) = ast_item.kind else { bug!() }; self.tables.asyncness.set(def_id.index, sig.header.asyncness); record_array!(self.tables.fn_arg_names[def_id] <- self.tcx.hir().body_param_names(body)); - // Can be inside `impl const Trait`, so using sig.header.constness is not reliable - let constness = if self.tcx.is_const_fn_raw(def_id) { - hir::Constness::Const - } else { - hir::Constness::NotConst - }; - self.tables.constness.set(def_id.index, constness); } ty::AssocKind::Const | ty::AssocKind::Type => {} } @@ -1474,7 +1503,6 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> { hir::ItemKind::Fn(ref sig, .., body) => { self.tables.asyncness.set(def_id.index, sig.header.asyncness); record_array!(self.tables.fn_arg_names[def_id] <- self.tcx.hir().body_param_names(body)); - self.tables.constness.set(def_id.index, sig.header.constness); } hir::ItemKind::Macro(ref macro_def, _) => { if macro_def.macro_rules { @@ -1495,7 +1523,6 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> { hir::ItemKind::Struct(ref struct_def, _) => { let adt_def = self.tcx.adt_def(def_id); record!(self.tables.repr_options[def_id] <- adt_def.repr()); - self.tables.constness.set(def_id.index, hir::Constness::Const); // Encode def_ids for each field and method // for methods, write all the stuff get_trait_method @@ -1524,9 +1551,8 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> { is_non_exhaustive: variant.is_field_list_non_exhaustive(), }); } - hir::ItemKind::Impl(hir::Impl { defaultness, constness, .. }) => { + hir::ItemKind::Impl(hir::Impl { defaultness, .. }) => { self.tables.impl_defaultness.set(def_id.index, *defaultness); - self.tables.constness.set(def_id.index, *constness); let trait_ref = self.tcx.impl_trait_ref(def_id); if let Some(trait_ref) = trait_ref { diff --git a/compiler/rustc_ty_utils/src/ty.rs b/compiler/rustc_ty_utils/src/ty.rs index 30efbf6617598..344426a18bca5 100644 --- a/compiler/rustc_ty_utils/src/ty.rs +++ b/compiler/rustc_ty_utils/src/ty.rs @@ -133,77 +133,10 @@ fn param_env(tcx: TyCtxt<'_>, def_id: DefId) -> ty::ParamEnv<'_> { let local_did = def_id.as_local(); let hir_id = local_did.map(|def_id| tcx.hir().local_def_id_to_hir_id(def_id)); - let constness = match hir_id { - Some(hir_id) => match tcx.hir().get(hir_id) { - hir::Node::TraitItem(hir::TraitItem { kind: hir::TraitItemKind::Fn(..), .. }) - if tcx.is_const_default_method(def_id) => - { - hir::Constness::Const - } - - hir::Node::Item(hir::Item { kind: hir::ItemKind::Const(..), .. }) - | hir::Node::Item(hir::Item { kind: hir::ItemKind::Static(..), .. }) - | hir::Node::TraitItem(hir::TraitItem { - kind: hir::TraitItemKind::Const(..), .. - }) - | hir::Node::AnonConst(_) - | hir::Node::ImplItem(hir::ImplItem { kind: hir::ImplItemKind::Const(..), .. }) - | hir::Node::ImplItem(hir::ImplItem { - kind: - hir::ImplItemKind::Fn( - hir::FnSig { - header: hir::FnHeader { constness: hir::Constness::Const, .. }, - .. - }, - .., - ), - .. - }) => hir::Constness::Const, - - hir::Node::ImplItem(hir::ImplItem { - kind: hir::ImplItemKind::Type(..) | hir::ImplItemKind::Fn(..), - .. - }) => { - let parent_hir_id = tcx.hir().get_parent_node(hir_id); - match tcx.hir().get(parent_hir_id) { - hir::Node::Item(hir::Item { - kind: hir::ItemKind::Impl(hir::Impl { constness, .. }), - .. - }) => *constness, - _ => span_bug!( - tcx.def_span(parent_hir_id.owner), - "impl item's parent node is not an impl", - ), - } - } - - hir::Node::Item(hir::Item { - kind: - hir::ItemKind::Fn(hir::FnSig { header: hir::FnHeader { constness, .. }, .. }, ..), - .. - }) - | hir::Node::TraitItem(hir::TraitItem { - kind: - hir::TraitItemKind::Fn( - hir::FnSig { header: hir::FnHeader { constness, .. }, .. }, - .., - ), - .. - }) - | hir::Node::Item(hir::Item { - kind: hir::ItemKind::Impl(hir::Impl { constness, .. }), - .. - }) => *constness, - - _ => hir::Constness::NotConst, - }, - None => hir::Constness::NotConst, - }; - let unnormalized_env = ty::ParamEnv::new( tcx.intern_predicates(&predicates), traits::Reveal::UserFacing, - constness, + tcx.constness(def_id), ); let body_id = From bef8681a1837790f2745c1f6a7f8214af2fd7f5d Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Wed, 12 Oct 2022 04:17:21 +0000 Subject: [PATCH 23/23] TyAlias needs encoded constness too, for layout computation in rustdoc --- compiler/rustc_metadata/src/rmeta/encoder.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/compiler/rustc_metadata/src/rmeta/encoder.rs b/compiler/rustc_metadata/src/rmeta/encoder.rs index ab4ecec2af36e..68119598285c5 100644 --- a/compiler/rustc_metadata/src/rmeta/encoder.rs +++ b/compiler/rustc_metadata/src/rmeta/encoder.rs @@ -1078,9 +1078,9 @@ fn should_encode_constness(def_kind: DefKind) -> bool { | DefKind::ImplTraitPlaceholder | DefKind::Impl | DefKind::Closure - | DefKind::Generator => true, + | DefKind::Generator + | DefKind::TyAlias => true, DefKind::Variant - | DefKind::TyAlias | DefKind::TraitAlias | DefKind::ForeignTy | DefKind::Field