From 0b5d6ae5dbbbe6b05a85bdcccc8aedbb96feedf4 Mon Sep 17 00:00:00 2001 From: mejrs <> Date: Thu, 5 Jan 2023 03:39:07 +0100 Subject: [PATCH 01/13] Improve fluent error messages --- compiler/rustc_errors/src/error.rs | 134 +++++++++++++++++++++++ compiler/rustc_errors/src/lib.rs | 5 + compiler/rustc_errors/src/translation.rs | 108 +++++++----------- 3 files changed, 180 insertions(+), 67 deletions(-) create mode 100644 compiler/rustc_errors/src/error.rs diff --git a/compiler/rustc_errors/src/error.rs b/compiler/rustc_errors/src/error.rs new file mode 100644 index 0000000000000..e1c8dcd3bd396 --- /dev/null +++ b/compiler/rustc_errors/src/error.rs @@ -0,0 +1,134 @@ +use rustc_error_messages::{ + fluent_bundle::resolver::errors::{ReferenceKind, ResolverError}, + FluentArgs, FluentError, +}; +use std::borrow::Cow; +use std::error::Error; +use std::fmt; + +#[derive(Debug)] +pub enum TranslateError<'args> { + One { + id: &'args Cow<'args, str>, + args: &'args FluentArgs<'args>, + kind: TranslateErrorKind<'args>, + }, + Two { + primary: Box>, + fallback: Box>, + }, +} + +impl<'args> TranslateError<'args> { + pub fn message(id: &'args Cow<'args, str>, args: &'args FluentArgs<'args>) -> Self { + Self::One { id, args, kind: TranslateErrorKind::MessageMissing } + } + pub fn primary(id: &'args Cow<'args, str>, args: &'args FluentArgs<'args>) -> Self { + Self::One { id, args, kind: TranslateErrorKind::PrimaryBundleMissing } + } + pub fn attribute( + id: &'args Cow<'args, str>, + args: &'args FluentArgs<'args>, + attr: &'args str, + ) -> Self { + Self::One { id, args, kind: TranslateErrorKind::AttributeMissing { attr } } + } + pub fn value(id: &'args Cow<'args, str>, args: &'args FluentArgs<'args>) -> Self { + Self::One { id, args, kind: TranslateErrorKind::ValueMissing } + } + + pub fn fluent( + id: &'args Cow<'args, str>, + args: &'args FluentArgs<'args>, + errs: Vec, + ) -> Self { + Self::One { id, args, kind: TranslateErrorKind::Fluent { errs } } + } + + pub fn and(self, fallback: TranslateError<'args>) -> TranslateError<'args> { + Self::Two { primary: Box::new(self), fallback: Box::new(fallback) } + } +} + +#[derive(Debug)] +pub enum TranslateErrorKind<'args> { + MessageMissing, + PrimaryBundleMissing, + AttributeMissing { attr: &'args str }, + ValueMissing, + Fluent { errs: Vec }, +} + +impl fmt::Display for TranslateError<'_> { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + use TranslateErrorKind::*; + + match self { + Self::One { id, args, kind } => { + writeln!(f, "\nfailed while formatting fluent string `{id}`: ")?; + match kind { + MessageMissing => writeln!(f, "message was missing")?, + PrimaryBundleMissing => writeln!(f, "the primary bundle was missing")?, + AttributeMissing { attr } => writeln!(f, "the attribute `{attr}` was missing")?, + ValueMissing => writeln!(f, "the value was missing")?, + Fluent { errs } => { + for err in errs { + match err { + FluentError::ResolverError(ResolverError::Reference( + ReferenceKind::Message { id, .. } + | ReferenceKind::Variable { id, .. }, + )) => { + if args.iter().any(|(arg_id, _)| arg_id == id) { + writeln!( + f, + "argument `{id}` exists but was not referenced correctly" + )?; + writeln!(f, "help: try using `{{${id}}}` instead")?; + } else { + writeln!( + f, + "the fluent string has an argument `{id}` that was not found." + )?; + let vars: Vec<&str> = + args.iter().map(|(a, _v)| a).collect(); + match &*vars { + [] => writeln!(f, "help: no arguments are available")?, + [one] => writeln!( + f, + "help: the argument `{one}` is available" + )?, + [first, middle @ .., last] => { + write!(f, "help: the arguments `{first}`")?; + for a in middle { + write!(f, ", `{a}`")?; + } + writeln!(f, " and `{last}` are available")?; + } + } + } + } + _ => writeln!(f, "{err}")?, + } + } + } + } + } + // If someone cares about primary bundles, they'll probably notice it's missing + // regardless or will be using `debug_assertions` + // so we skip the arm below this one to avoid confusing the regular user. + Self::Two { primary: box Self::One { kind: PrimaryBundleMissing, .. }, fallback } => { + fmt::Display::fmt(fallback, f)?; + } + Self::Two { primary, fallback } => { + writeln!( + f, + "first, fluent formatting using the primary bundle failed:\n {primary}\n \ + while attempting to recover by using the fallback bundle instead, another error occurred:\n{fallback}" + )?; + } + } + Ok(()) + } +} + +impl Error for TranslateError<'_> {} diff --git a/compiler/rustc_errors/src/lib.rs b/compiler/rustc_errors/src/lib.rs index 136c360201e61..6f411c5174a04 100644 --- a/compiler/rustc_errors/src/lib.rs +++ b/compiler/rustc_errors/src/lib.rs @@ -11,6 +11,10 @@ #![feature(never_type)] #![feature(result_option_inspect)] #![feature(rustc_attrs)] +#![feature(yeet_expr)] +#![feature(try_blocks)] +#![feature(box_patterns)] +#![feature(error_reporter)] #![allow(incomplete_features)] #[macro_use] @@ -55,6 +59,7 @@ mod diagnostic; mod diagnostic_builder; mod diagnostic_impls; pub mod emitter; +pub mod error; pub mod json; mod lock; pub mod registry; diff --git a/compiler/rustc_errors/src/translation.rs b/compiler/rustc_errors/src/translation.rs index afd660ff1bf1f..26c86cdaf4567 100644 --- a/compiler/rustc_errors/src/translation.rs +++ b/compiler/rustc_errors/src/translation.rs @@ -1,11 +1,10 @@ +use crate::error::TranslateError; use crate::snippet::Style; use crate::{DiagnosticArg, DiagnosticMessage, FluentBundle}; use rustc_data_structures::sync::Lrc; -use rustc_error_messages::{ - fluent_bundle::resolver::errors::{ReferenceKind, ResolverError}, - FluentArgs, FluentError, -}; +use rustc_error_messages::FluentArgs; use std::borrow::Cow; +use std::error::Report; /// Convert diagnostic arguments (a rustc internal type that exists to implement /// `Encodable`/`Decodable`) into `FluentArgs` which is necessary to perform translation. @@ -63,75 +62,50 @@ pub trait Translate { } DiagnosticMessage::FluentIdentifier(identifier, attr) => (identifier, attr), }; + let translate_with_bundle = + |bundle: &'a FluentBundle| -> Result, TranslateError<'_>> { + let message = bundle + .get_message(identifier) + .ok_or(TranslateError::message(identifier, args))?; + let value = match attr { + Some(attr) => message + .get_attribute(attr) + .ok_or(TranslateError::attribute(identifier, args, attr))? + .value(), + None => message.value().ok_or(TranslateError::value(identifier, args))?, + }; + debug!(?message, ?value); - let translate_with_bundle = |bundle: &'a FluentBundle| -> Option<(Cow<'_, str>, Vec<_>)> { - let message = bundle.get_message(identifier)?; - let value = match attr { - Some(attr) => message.get_attribute(attr)?.value(), - None => message.value()?, + let mut errs = vec![]; + let translated = bundle.format_pattern(value, Some(args), &mut errs); + debug!(?translated, ?errs); + if errs.is_empty() { + Ok(translated) + } else { + Err(TranslateError::fluent(identifier, args, errs)) + } }; - debug!(?message, ?value); - - let mut errs = vec![]; - let translated = bundle.format_pattern(value, Some(args), &mut errs); - debug!(?translated, ?errs); - Some((translated, errs)) - }; - self.fluent_bundle() - .and_then(|bundle| translate_with_bundle(bundle)) - // If `translate_with_bundle` returns `None` with the primary bundle, this is likely - // just that the primary bundle doesn't contain the message being translated, so - // proceed to the fallback bundle. - // - // However, when errors are produced from translation, then that means the translation - // is broken (e.g. `{$foo}` exists in a translation but `foo` isn't provided). - // - // In debug builds, assert so that compiler devs can spot the broken translation and - // fix it.. - .inspect(|(_, errs)| { - debug_assert!( - errs.is_empty(), - "identifier: {:?}, attr: {:?}, args: {:?}, errors: {:?}", - identifier, - attr, - args, - errs - ); - }) - // ..otherwise, for end users, an error about this wouldn't be useful or actionable, so - // just hide it and try with the fallback bundle. - .filter(|(_, errs)| errs.is_empty()) - .or_else(|| translate_with_bundle(self.fallback_fluent_bundle())) - .map(|(translated, errs)| { - // Always bail out for errors with the fallback bundle. + let ret: Result, TranslateError<'_>> = try { + match self.fluent_bundle().map(|b| translate_with_bundle(b)) { + // The primary bundle was present and translation succeeded + Some(Ok(t)) => t, - let mut help_messages = vec![]; + // Always yeet out for errors on debug + Some(Err(primary)) if cfg!(debug_assertions) => do yeet primary, - if !errs.is_empty() { - for error in &errs { - match error { - FluentError::ResolverError(ResolverError::Reference( - ReferenceKind::Message { id, .. }, - )) if args.iter().any(|(arg_id, _)| arg_id == id) => { - help_messages.push(format!("Argument `{id}` exists but was not referenced correctly. Try using `{{${id}}}` instead")); - } - _ => {} - } - } + // If `translate_with_bundle` returns `Err` with the primary bundle, this is likely + // just that the primary bundle doesn't contain the message being translated or + // something else went wrong) so proceed to the fallback bundle. + Some(Err(primary)) => translate_with_bundle(self.fallback_fluent_bundle()) + .map_err(|fallback| primary.and(fallback))?, - panic!( - "Encountered errors while formatting message for `{identifier}`\n\ - help: {}\n\ - attr: `{attr:?}`\n\ - args: `{args:?}`\n\ - errors: `{errs:?}`", - help_messages.join("\nhelp: ") - ); - } - - translated - }) + // The primary bundle is missing, proceed to the fallback bundle + None => translate_with_bundle(self.fallback_fluent_bundle()) + .map_err(|fallback| TranslateError::primary(identifier, args).and(fallback))?, + } + }; + ret.map_err(Report::new) .expect("failed to find message in primary or fallback fluent bundles") } } From 9fd744b3e32817db3dcc3210fee67bfd0bcc11fe Mon Sep 17 00:00:00 2001 From: Jacob Kiesel Date: Sat, 7 Jan 2023 11:05:33 -0700 Subject: [PATCH 02/13] add tests for div_duration_* functions --- library/core/tests/time.rs | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/library/core/tests/time.rs b/library/core/tests/time.rs index a05128de471ab..2975c81f8fec9 100644 --- a/library/core/tests/time.rs +++ b/library/core/tests/time.rs @@ -173,6 +173,32 @@ fn div() { assert_eq!(Duration::new(99, 999_999_000) / 100, Duration::new(0, 999_999_990)); } +#[test] +fn div_duration_f32() { + assert_eq!(Duration::ZERO.div_duration_f32(Duration::MAX), 0.0); + assert_eq!(Duration::MAX.div_duration_f32(Duration::ZERO), f32::INFINITY); + assert_eq!((Duration::SECOND * 2).div_duration_f32(Duration::SECOND), 2.0); + assert!(Duration::ZERO.div_duration_f32(Duration::ZERO).is_nan()); + // These tests demonstrate it doesn't panic with extreme values. + // Accuracy of the computed value is not a huge concern, we know floats don't work well + // at these extremes. + assert!((Duration::MAX).div_duration_f32(Duration::NANOSECOND) > 10.0f32.powf(28.0)); + assert!((Duration::NANOSECOND).div_duration_f32(Duration::MAX) < 0.1); +} + +#[test] +fn div_duration_f64() { + assert_eq!(Duration::ZERO.div_duration_f64(Duration::MAX), 0.0); + assert_eq!(Duration::MAX.div_duration_f64(Duration::ZERO), f64::INFINITY); + assert_eq!((Duration::SECOND * 2).div_duration_f64(Duration::SECOND), 2.0); + assert!(Duration::ZERO.div_duration_f64(Duration::ZERO).is_nan()); + // These tests demonstrate it doesn't panic with extreme values. + // Accuracy of the computed value is not a huge concern, we know floats don't work well + // at these extremes. + assert!((Duration::MAX).div_duration_f64(Duration::NANOSECOND) > 10.0f64.powf(28.0)); + assert!((Duration::NANOSECOND).div_duration_f64(Duration::MAX) < 0.1); +} + #[test] fn checked_div() { assert_eq!(Duration::new(2, 0).checked_div(2), Some(Duration::new(1, 0))); From eae615dfdd9222a49182e59cc15bd47da1d536b6 Mon Sep 17 00:00:00 2001 From: yukang Date: Sun, 8 Jan 2023 05:02:04 +0800 Subject: [PATCH 03/13] Remove unnecessary lseek syscall when using std::fs::read --- library/std/src/fs.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/library/std/src/fs.rs b/library/std/src/fs.rs index f357d505fe89c..6ddd5c28cc2d3 100644 --- a/library/std/src/fs.rs +++ b/library/std/src/fs.rs @@ -250,7 +250,9 @@ pub fn read>(path: P) -> io::Result> { fn inner(path: &Path) -> io::Result> { let mut file = File::open(path)?; let mut bytes = Vec::new(); - file.read_to_end(&mut bytes)?; + let size = file.metadata().map(|m| m.len()).unwrap_or(0); + bytes.reserve(size as usize); + io::default_read_to_end(&mut file, &mut bytes)?; Ok(bytes) } inner(path.as_ref()) @@ -289,7 +291,9 @@ pub fn read_to_string>(path: P) -> io::Result { fn inner(path: &Path) -> io::Result { let mut file = File::open(path)?; let mut string = String::new(); - file.read_to_string(&mut string)?; + let size = file.metadata().map(|m| m.len()).unwrap_or(0); + string.reserve(size as usize); + io::default_read_to_string(&mut file, &mut string)?; Ok(string) } inner(path.as_ref()) From 262ff86138730c1eb65f3ec39dd9c93222ed77e7 Mon Sep 17 00:00:00 2001 From: mejrs <> Date: Sun, 8 Jan 2023 23:35:43 +0100 Subject: [PATCH 04/13] Make translate_message return result and add tests --- compiler/rustc_errors/src/emitter.rs | 11 +- compiler/rustc_errors/src/error.rs | 7 +- compiler/rustc_errors/src/json.rs | 9 +- compiler/rustc_errors/src/lib.rs | 12 +- compiler/rustc_errors/src/tests.rs | 183 ++++++++++++++++++ compiler/rustc_errors/src/translation.rs | 15 +- .../passes/lint/check_code_block_syntax.rs | 4 +- 7 files changed, 224 insertions(+), 17 deletions(-) create mode 100644 compiler/rustc_errors/src/tests.rs diff --git a/compiler/rustc_errors/src/emitter.rs b/compiler/rustc_errors/src/emitter.rs index 0ca200abe19fb..7f01df321010b 100644 --- a/compiler/rustc_errors/src/emitter.rs +++ b/compiler/rustc_errors/src/emitter.rs @@ -28,6 +28,7 @@ use rustc_error_messages::{FluentArgs, SpanLabel}; use rustc_span::hygiene::{ExpnKind, MacroKind}; use std::borrow::Cow; use std::cmp::{max, min, Reverse}; +use std::error::Report; use std::io::prelude::*; use std::io::{self, IsTerminal}; use std::iter; @@ -250,7 +251,7 @@ pub trait Emitter: Translate { let mut primary_span = diag.span.clone(); let suggestions = diag.suggestions.as_deref().unwrap_or(&[]); if let Some((sugg, rest)) = suggestions.split_first() { - let msg = self.translate_message(&sugg.msg, fluent_args); + let msg = self.translate_message(&sugg.msg, fluent_args).map_err(Report::new).unwrap(); if rest.is_empty() && // ^ if there is only one suggestion // don't display multi-suggestions as labels @@ -1325,7 +1326,7 @@ impl EmitterWriter { // very *weird* formats // see? for (text, style) in msg.iter() { - let text = self.translate_message(text, args); + let text = self.translate_message(text, args).map_err(Report::new).unwrap(); let lines = text.split('\n').collect::>(); if lines.len() > 1 { for (i, line) in lines.iter().enumerate() { @@ -1387,7 +1388,7 @@ impl EmitterWriter { label_width += 2; } for (text, _) in msg.iter() { - let text = self.translate_message(text, args); + let text = self.translate_message(text, args).map_err(Report::new).unwrap(); // Account for newlines to align output to its label. for (line, text) in normalize_whitespace(&text).lines().enumerate() { buffer.append( @@ -2301,7 +2302,9 @@ impl FileWithAnnotatedLines { hi.col_display += 1; } - let label = label.as_ref().map(|m| emitter.translate_message(m, args).to_string()); + let label = label.as_ref().map(|m| { + emitter.translate_message(m, args).map_err(Report::new).unwrap().to_string() + }); if lo.line != hi.line { let ml = MultilineAnnotation { diff --git a/compiler/rustc_errors/src/error.rs b/compiler/rustc_errors/src/error.rs index e1c8dcd3bd396..ec0a2fe8cd8d0 100644 --- a/compiler/rustc_errors/src/error.rs +++ b/compiler/rustc_errors/src/error.rs @@ -65,11 +65,14 @@ impl fmt::Display for TranslateError<'_> { match self { Self::One { id, args, kind } => { - writeln!(f, "\nfailed while formatting fluent string `{id}`: ")?; + writeln!(f, "failed while formatting fluent string `{id}`: ")?; match kind { MessageMissing => writeln!(f, "message was missing")?, PrimaryBundleMissing => writeln!(f, "the primary bundle was missing")?, - AttributeMissing { attr } => writeln!(f, "the attribute `{attr}` was missing")?, + AttributeMissing { attr } => { + writeln!(f, "the attribute `{attr}` was missing")?; + writeln!(f, "help: add `.{attr} = `")?; + } ValueMissing => writeln!(f, "the value was missing")?, Fluent { errs } => { for err in errs { diff --git a/compiler/rustc_errors/src/json.rs b/compiler/rustc_errors/src/json.rs index a37073d8fa32a..dc38b8725ad1e 100644 --- a/compiler/rustc_errors/src/json.rs +++ b/compiler/rustc_errors/src/json.rs @@ -24,6 +24,7 @@ use rustc_data_structures::sync::Lrc; use rustc_error_messages::FluentArgs; use rustc_span::hygiene::ExpnData; use rustc_span::Span; +use std::error::Report; use std::io::{self, Write}; use std::path::Path; use std::sync::{Arc, Mutex}; @@ -321,7 +322,8 @@ impl Diagnostic { fn from_errors_diagnostic(diag: &crate::Diagnostic, je: &JsonEmitter) -> Diagnostic { let args = to_fluent_args(diag.args()); let sugg = diag.suggestions.iter().flatten().map(|sugg| { - let translated_message = je.translate_message(&sugg.msg, &args); + let translated_message = + je.translate_message(&sugg.msg, &args).map_err(Report::new).unwrap(); Diagnostic { message: translated_message.to_string(), code: None, @@ -411,7 +413,10 @@ impl DiagnosticSpan { Self::from_span_etc( span.span, span.is_primary, - span.label.as_ref().map(|m| je.translate_message(m, args)).map(|m| m.to_string()), + span.label + .as_ref() + .map(|m| je.translate_message(m, args).unwrap()) + .map(|m| m.to_string()), suggestion, je, ) diff --git a/compiler/rustc_errors/src/lib.rs b/compiler/rustc_errors/src/lib.rs index 6f411c5174a04..0b5bfdc8e8985 100644 --- a/compiler/rustc_errors/src/lib.rs +++ b/compiler/rustc_errors/src/lib.rs @@ -46,6 +46,7 @@ use rustc_span::{Loc, Span}; use std::any::Any; use std::borrow::Cow; +use std::error::Report; use std::fmt; use std::hash::Hash; use std::num::NonZeroUsize; @@ -65,6 +66,8 @@ mod lock; pub mod registry; mod snippet; mod styled_buffer; +#[cfg(test)] +mod tests; pub mod translation; pub use diagnostic_builder::IntoDiagnostic; @@ -627,7 +630,14 @@ impl Handler { ) -> SubdiagnosticMessage { let inner = self.inner.borrow(); let args = crate::translation::to_fluent_args(args); - SubdiagnosticMessage::Eager(inner.emitter.translate_message(&message, &args).to_string()) + SubdiagnosticMessage::Eager( + inner + .emitter + .translate_message(&message, &args) + .map_err(Report::new) + .unwrap() + .to_string(), + ) } // This is here to not allow mutation of flags; diff --git a/compiler/rustc_errors/src/tests.rs b/compiler/rustc_errors/src/tests.rs new file mode 100644 index 0000000000000..47220b59bd447 --- /dev/null +++ b/compiler/rustc_errors/src/tests.rs @@ -0,0 +1,183 @@ +use crate::error::{TranslateError, TranslateErrorKind}; +use crate::fluent_bundle::*; +use crate::translation::Translate; +use crate::FluentBundle; +use rustc_data_structures::sync::Lrc; +use rustc_error_messages::fluent_bundle::resolver::errors::{ReferenceKind, ResolverError}; +use rustc_error_messages::langid; +use rustc_error_messages::DiagnosticMessage; + +struct Dummy { + bundle: FluentBundle, +} + +impl Translate for Dummy { + fn fluent_bundle(&self) -> Option<&Lrc> { + None + } + + fn fallback_fluent_bundle(&self) -> &FluentBundle { + &self.bundle + } +} + +fn make_dummy(ftl: &'static str) -> Dummy { + let resource = FluentResource::try_new(ftl.into()).expect("Failed to parse an FTL string."); + + let langid_en = langid!("en-US"); + let mut bundle = FluentBundle::new(vec![langid_en]); + + bundle.add_resource(resource).expect("Failed to add FTL resources to the bundle."); + + Dummy { bundle } +} + +#[test] +fn wellformed_fluent() { + let dummy = make_dummy("mir_build_borrow_of_moved_value = borrow of moved value + .label = value moved into `{$name}` here + .occurs_because_label = move occurs because `{$name}` has type `{$ty}` which does not implement the `Copy` trait + .value_borrowed_label = value borrowed here after move + .suggestion = borrow this binding in the pattern to avoid moving the value"); + + let mut args = FluentArgs::new(); + args.set("name", "Foo"); + args.set("ty", "std::string::String"); + { + let message = DiagnosticMessage::FluentIdentifier( + "mir_build_borrow_of_moved_value".into(), + Some("suggestion".into()), + ); + + assert_eq!( + dummy.translate_message(&message, &args).unwrap(), + "borrow this binding in the pattern to avoid moving the value" + ); + } + + { + let message = DiagnosticMessage::FluentIdentifier( + "mir_build_borrow_of_moved_value".into(), + Some("value_borrowed_label".into()), + ); + + assert_eq!( + dummy.translate_message(&message, &args).unwrap(), + "value borrowed here after move" + ); + } + + { + let message = DiagnosticMessage::FluentIdentifier( + "mir_build_borrow_of_moved_value".into(), + Some("occurs_because_label".into()), + ); + + assert_eq!( + dummy.translate_message(&message, &args).unwrap(), + "move occurs because `\u{2068}Foo\u{2069}` has type `\u{2068}std::string::String\u{2069}` which does not implement the `Copy` trait" + ); + + { + let message = DiagnosticMessage::FluentIdentifier( + "mir_build_borrow_of_moved_value".into(), + Some("label".into()), + ); + + assert_eq!( + dummy.translate_message(&message, &args).unwrap(), + "value moved into `\u{2068}Foo\u{2069}` here" + ); + } + } +} + +#[test] +fn misformed_fluent() { + let dummy = make_dummy("mir_build_borrow_of_moved_value = borrow of moved value + .label = value moved into `{name}` here + .occurs_because_label = move occurs because `{$oops}` has type `{$ty}` which does not implement the `Copy` trait + .suggestion = borrow this binding in the pattern to avoid moving the value"); + + let mut args = FluentArgs::new(); + args.set("name", "Foo"); + args.set("ty", "std::string::String"); + { + let message = DiagnosticMessage::FluentIdentifier( + "mir_build_borrow_of_moved_value".into(), + Some("value_borrowed_label".into()), + ); + + let err = dummy.translate_message(&message, &args).unwrap_err(); + assert!( + matches!( + &err, + TranslateError::Two { + primary: box TranslateError::One { + kind: TranslateErrorKind::PrimaryBundleMissing, + .. + }, + fallback: box TranslateError::One { + kind: TranslateErrorKind::AttributeMissing { attr: "value_borrowed_label" }, + .. + } + } + ), + "{err:#?}" + ); + assert_eq!( + format!("{err}"), + "failed while formatting fluent string `mir_build_borrow_of_moved_value`: \nthe attribute `value_borrowed_label` was missing\nhelp: add `.value_borrowed_label = `\n" + ); + } + + { + let message = DiagnosticMessage::FluentIdentifier( + "mir_build_borrow_of_moved_value".into(), + Some("label".into()), + ); + + let err = dummy.translate_message(&message, &args).unwrap_err(); + if let TranslateError::Two { + primary: box TranslateError::One { kind: TranslateErrorKind::PrimaryBundleMissing, .. }, + fallback: box TranslateError::One { kind: TranslateErrorKind::Fluent { errs }, .. }, + } = &err + && let [FluentError::ResolverError(ResolverError::Reference( + ReferenceKind::Message { id, .. } + | ReferenceKind::Variable { id, .. }, + ))] = &**errs + && id == "name" + {} else { + panic!("{err:#?}") + }; + assert_eq!( + format!("{err}"), + "failed while formatting fluent string `mir_build_borrow_of_moved_value`: \nargument `name` exists but was not referenced correctly\nhelp: try using `{$name}` instead\n" + ); + } + + { + let message = DiagnosticMessage::FluentIdentifier( + "mir_build_borrow_of_moved_value".into(), + Some("occurs_because_label".into()), + ); + + let err = dummy.translate_message(&message, &args).unwrap_err(); + if let TranslateError::Two { + primary: box TranslateError::One { kind: TranslateErrorKind::PrimaryBundleMissing, .. }, + fallback: box TranslateError::One { kind: TranslateErrorKind::Fluent { errs }, .. }, + } = &err + && let [FluentError::ResolverError(ResolverError::Reference( + ReferenceKind::Message { id, .. } + | ReferenceKind::Variable { id, .. }, + ))] = &**errs + && id == "oops" + {} else { + panic!("{err:#?}") + }; + assert_eq!( + format!("{err}"), + "failed while formatting fluent string `mir_build_borrow_of_moved_value`: \nthe fluent string has an argument `oops` that was not found.\nhelp: the arguments `name` and `ty` are available\n" + ); + } +} diff --git a/compiler/rustc_errors/src/translation.rs b/compiler/rustc_errors/src/translation.rs index 26c86cdaf4567..addfc9726ca44 100644 --- a/compiler/rustc_errors/src/translation.rs +++ b/compiler/rustc_errors/src/translation.rs @@ -45,7 +45,10 @@ pub trait Translate { args: &FluentArgs<'_>, ) -> Cow<'_, str> { Cow::Owned( - messages.iter().map(|(m, _)| self.translate_message(m, args)).collect::(), + messages + .iter() + .map(|(m, _)| self.translate_message(m, args).map_err(Report::new).unwrap()) + .collect::(), ) } @@ -54,11 +57,11 @@ pub trait Translate { &'a self, message: &'a DiagnosticMessage, args: &'a FluentArgs<'_>, - ) -> Cow<'_, str> { + ) -> Result, TranslateError<'_>> { trace!(?message, ?args); let (identifier, attr) = match message { DiagnosticMessage::Str(msg) | DiagnosticMessage::Eager(msg) => { - return Cow::Borrowed(msg); + return Ok(Cow::Borrowed(msg)); } DiagnosticMessage::FluentIdentifier(identifier, attr) => (identifier, attr), }; @@ -86,7 +89,7 @@ pub trait Translate { } }; - let ret: Result, TranslateError<'_>> = try { + try { match self.fluent_bundle().map(|b| translate_with_bundle(b)) { // The primary bundle was present and translation succeeded Some(Ok(t)) => t, @@ -104,8 +107,6 @@ pub trait Translate { None => translate_with_bundle(self.fallback_fluent_bundle()) .map_err(|fallback| TranslateError::primary(identifier, args).and(fallback))?, } - }; - ret.map_err(Report::new) - .expect("failed to find message in primary or fallback fluent bundles") + } } } diff --git a/src/librustdoc/passes/lint/check_code_block_syntax.rs b/src/librustdoc/passes/lint/check_code_block_syntax.rs index 5aa4f238b2d14..7158355ffdacc 100644 --- a/src/librustdoc/passes/lint/check_code_block_syntax.rs +++ b/src/librustdoc/passes/lint/check_code_block_syntax.rs @@ -156,7 +156,9 @@ impl Emitter for BufferEmitter { let mut buffer = self.buffer.borrow_mut(); let fluent_args = to_fluent_args(diag.args()); - let translated_main_message = self.translate_message(&diag.message[0].0, &fluent_args); + let translated_main_message = self + .translate_message(&diag.message[0].0, &fluent_args) + .unwrap_or_else(|e| panic!("{e}")); buffer.messages.push(format!("error from rustc: {}", translated_main_message)); if diag.is_error() { From 4c0c32c895df28b762a61958b21cbe4d68f60238 Mon Sep 17 00:00:00 2001 From: mejrs <> Date: Mon, 9 Jan 2023 00:23:27 +0100 Subject: [PATCH 05/13] Fix tests --- compiler/rustc_errors/src/tests.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/compiler/rustc_errors/src/tests.rs b/compiler/rustc_errors/src/tests.rs index 47220b59bd447..52103e4609770 100644 --- a/compiler/rustc_errors/src/tests.rs +++ b/compiler/rustc_errors/src/tests.rs @@ -25,6 +25,11 @@ fn make_dummy(ftl: &'static str) -> Dummy { let resource = FluentResource::try_new(ftl.into()).expect("Failed to parse an FTL string."); let langid_en = langid!("en-US"); + + #[cfg(parallel_compiler)] + let mut bundle = FluentBundle::new_concurrent(vec![langid_en]); + + #[cfg(not(parallel_compiler))] let mut bundle = FluentBundle::new(vec![langid_en]); bundle.add_resource(resource).expect("Failed to add FTL resources to the bundle."); From 36ee66c6c5e5e5f1a132faf28c6a5d28e950a2ba Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Tue, 3 Jan 2023 03:43:11 +0000 Subject: [PATCH 06/13] Check impl's where clauses in consider_impl_candidate in experimental solver --- .../src/solve/project_goals.rs | 10 ++++++++-- .../src/solve/trait_goals.rs | 17 +++++++++++++---- 2 files changed, 21 insertions(+), 6 deletions(-) diff --git a/compiler/rustc_trait_selection/src/solve/project_goals.rs b/compiler/rustc_trait_selection/src/solve/project_goals.rs index b50f42c4d9416..3d649bea19ddf 100644 --- a/compiler/rustc_trait_selection/src/solve/project_goals.rs +++ b/compiler/rustc_trait_selection/src/solve/project_goals.rs @@ -131,8 +131,14 @@ impl<'tcx> assembly::GoalKind<'tcx> for ProjectionPredicate<'tcx> { else { return }; - - let nested_goals = obligations.into_iter().map(|o| o.into()).collect(); + let where_clause_bounds = tcx + .predicates_of(impl_def_id) + .instantiate(tcx, impl_substs) + .predicates + .into_iter() + .map(|pred| goal.with(tcx, pred)); + + let nested_goals = obligations.into_iter().map(|o| o.into()).chain(where_clause_bounds).collect(); let Ok(trait_ref_certainty) = acx.cx.evaluate_all(acx.infcx, nested_goals) else { return }; let Some(assoc_def) = fetch_eligible_assoc_item_def( diff --git a/compiler/rustc_trait_selection/src/solve/trait_goals.rs b/compiler/rustc_trait_selection/src/solve/trait_goals.rs index 10b45a77dabb0..c69cc39acb53c 100644 --- a/compiler/rustc_trait_selection/src/solve/trait_goals.rs +++ b/compiler/rustc_trait_selection/src/solve/trait_goals.rs @@ -71,7 +71,9 @@ impl<'tcx> assembly::GoalKind<'tcx> for TraitPredicate<'tcx> { goal: Goal<'tcx, TraitPredicate<'tcx>>, impl_def_id: DefId, ) { - let impl_trait_ref = acx.cx.tcx.bound_impl_trait_ref(impl_def_id).unwrap(); + let tcx = acx.cx.tcx; + + let impl_trait_ref = tcx.bound_impl_trait_ref(impl_def_id).unwrap(); let drcx = DeepRejectCtxt { treat_obligation_params: TreatParams::AsPlaceholder }; if iter::zip(goal.predicate.trait_ref.substs, impl_trait_ref.skip_binder().substs) .any(|(goal, imp)| !drcx.generic_args_may_unify(goal, imp)) @@ -81,7 +83,7 @@ impl<'tcx> assembly::GoalKind<'tcx> for TraitPredicate<'tcx> { acx.infcx.probe(|_| { let impl_substs = acx.infcx.fresh_substs_for_item(DUMMY_SP, impl_def_id); - let impl_trait_ref = impl_trait_ref.subst(acx.cx.tcx, impl_substs); + let impl_trait_ref = impl_trait_ref.subst(tcx, impl_substs); let Ok(InferOk { obligations, .. }) = acx .infcx @@ -92,8 +94,15 @@ impl<'tcx> assembly::GoalKind<'tcx> for TraitPredicate<'tcx> { else { return }; - - let nested_goals = obligations.into_iter().map(|o| o.into()).collect(); + let where_clause_bounds = tcx + .predicates_of(impl_def_id) + .instantiate(tcx, impl_substs) + .predicates + .into_iter() + .map(|pred| goal.with(tcx, pred)); + + let nested_goals = + obligations.into_iter().map(|o| o.into()).chain(where_clause_bounds).collect(); let Ok(certainty) = acx.cx.evaluate_all(acx.infcx, nested_goals) else { return }; acx.try_insert_candidate(CandidateSource::Impl(impl_def_id), certainty); From 2855794257e36ec7393f3b7d8f4b99e5776f550f Mon Sep 17 00:00:00 2001 From: Nilstrieb <48135649+Nilstrieb@users.noreply.github.com> Date: Thu, 1 Dec 2022 18:57:53 +0100 Subject: [PATCH 07/13] Use newtype for unused generic parameters --- .../rustc_const_eval/src/interpret/util.rs | 3 +- compiler/rustc_metadata/src/rmeta/encoder.rs | 2 +- compiler/rustc_metadata/src/rmeta/mod.rs | 6 +-- compiler/rustc_middle/src/query/mod.rs | 2 +- compiler/rustc_middle/src/ty/instance.rs | 38 ++++++++++++++++++- compiler/rustc_middle/src/ty/mod.rs | 2 +- compiler/rustc_middle/src/ty/parameterized.rs | 1 + compiler/rustc_middle/src/ty/query.rs | 4 +- compiler/rustc_monomorphize/src/errors.rs | 4 +- .../rustc_monomorphize/src/polymorphize.rs | 36 +++++++++--------- 10 files changed, 65 insertions(+), 33 deletions(-) diff --git a/compiler/rustc_const_eval/src/interpret/util.rs b/compiler/rustc_const_eval/src/interpret/util.rs index e4f716c31945c..a61d3ab40a5ca 100644 --- a/compiler/rustc_const_eval/src/interpret/util.rs +++ b/compiler/rustc_const_eval/src/interpret/util.rs @@ -40,12 +40,11 @@ where let index = index .try_into() .expect("more generic parameters than can fit into a `u32`"); - let is_used = unused_params.contains(index).map_or(true, |unused| !unused); // Only recurse when generic parameters in fns, closures and generators // are used and require substitution. // Just in case there are closures or generators within this subst, // recurse. - if is_used && subst.needs_subst() { + if unused_params.is_used(index) && subst.needs_subst() { return subst.visit_with(self); } } diff --git a/compiler/rustc_metadata/src/rmeta/encoder.rs b/compiler/rustc_metadata/src/rmeta/encoder.rs index 0d9f216700fb1..bdc4ae391f043 100644 --- a/compiler/rustc_metadata/src/rmeta/encoder.rs +++ b/compiler/rustc_metadata/src/rmeta/encoder.rs @@ -1429,7 +1429,7 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> { let instance = ty::InstanceDef::Item(ty::WithOptConstParam::unknown(def_id.to_def_id())); let unused = tcx.unused_generic_params(instance); - if !unused.is_empty() { + if !unused.all_used() { record!(self.tables.unused_generic_params[def_id.to_def_id()] <- unused); } } diff --git a/compiler/rustc_metadata/src/rmeta/mod.rs b/compiler/rustc_metadata/src/rmeta/mod.rs index 26a41f633fffa..bf9be714daf7e 100644 --- a/compiler/rustc_metadata/src/rmeta/mod.rs +++ b/compiler/rustc_metadata/src/rmeta/mod.rs @@ -13,7 +13,7 @@ use rustc_hir::def::{CtorKind, DefKind}; use rustc_hir::def_id::{CrateNum, DefId, DefIndex, DefPathHash, StableCrateId}; use rustc_hir::definitions::DefKey; use rustc_hir::lang_items::LangItem; -use rustc_index::bit_set::{BitSet, FiniteBitSet}; +use rustc_index::bit_set::BitSet; use rustc_index::vec::IndexVec; use rustc_middle::metadata::ModChild; use rustc_middle::middle::codegen_fn_attrs::CodegenFnAttrs; @@ -22,7 +22,7 @@ use rustc_middle::middle::resolve_lifetime::ObjectLifetimeDefault; use rustc_middle::mir; use rustc_middle::ty::fast_reject::SimplifiedType; use rustc_middle::ty::query::Providers; -use rustc_middle::ty::{self, ReprOptions, Ty}; +use rustc_middle::ty::{self, ReprOptions, Ty, UnusedGenericParams}; use rustc_middle::ty::{DeducedParamAttrs, GeneratorDiagnosticData, ParameterizedOverTcx, TyCtxt}; use rustc_serialize::opaque::FileEncoder; use rustc_session::config::SymbolManglingVersion; @@ -384,7 +384,7 @@ define_tables! { trait_item_def_id: Table, inherent_impls: Table>, expn_that_defined: Table>, - unused_generic_params: Table>>, + unused_generic_params: Table>, params_in_repr: Table>>, repr_options: Table>, // `def_keys` and `def_path_hashes` represent a lazy version of a diff --git a/compiler/rustc_middle/src/query/mod.rs b/compiler/rustc_middle/src/query/mod.rs index 37db2274f678f..076ce1bdb3486 100644 --- a/compiler/rustc_middle/src/query/mod.rs +++ b/compiler/rustc_middle/src/query/mod.rs @@ -1839,7 +1839,7 @@ rustc_queries! { desc { "getting codegen unit `{sym}`" } } - query unused_generic_params(key: ty::InstanceDef<'tcx>) -> FiniteBitSet { + query unused_generic_params(key: ty::InstanceDef<'tcx>) -> UnusedGenericParams { cache_on_disk_if { key.def_id().is_local() } desc { |tcx| "determining which generic parameters are unused by `{}`", diff --git a/compiler/rustc_middle/src/ty/instance.rs b/compiler/rustc_middle/src/ty/instance.rs index 35d369ffc891c..4ee4d7caec1f3 100644 --- a/compiler/rustc_middle/src/ty/instance.rs +++ b/compiler/rustc_middle/src/ty/instance.rs @@ -6,6 +6,7 @@ use rustc_errors::ErrorGuaranteed; use rustc_hir::def::Namespace; use rustc_hir::def_id::{CrateNum, DefId}; use rustc_hir::lang_items::LangItem; +use rustc_index::bit_set::FiniteBitSet; use rustc_macros::HashStable; use rustc_middle::ty::normalize_erasing_regions::NormalizationError; use rustc_span::Symbol; @@ -711,7 +712,7 @@ fn polymorphize<'tcx>( } InternalSubsts::for_item(tcx, def_id, |param, _| { - let is_unused = unused.contains(param.index).unwrap_or(false); + let is_unused = unused.is_unused(param.index); debug!("polymorphize: param={:?} is_unused={:?}", param, is_unused); match param.kind { // Upvar case: If parameter is a type parameter.. @@ -733,7 +734,7 @@ fn polymorphize<'tcx>( // Simple case: If parameter is a const or type parameter.. ty::GenericParamDefKind::Const { .. } | ty::GenericParamDefKind::Type { .. } if // ..and is within range and unused.. - unused.contains(param.index).unwrap_or(false) => + unused.is_unused(param.index) => // ..then use the identity for this parameter. tcx.mk_param_from_def(param), @@ -774,3 +775,36 @@ fn needs_fn_once_adapter_shim( (ty::ClosureKind::FnMut | ty::ClosureKind::FnOnce, _) => Err(()), } } + +// Set bits represent unused generic parameters. +// An empty set indicates that all parameters are used. +#[derive(Debug, Copy, Clone, Eq, PartialEq, Decodable, Encodable, HashStable)] +pub struct UnusedGenericParams(FiniteBitSet); + +impl UnusedGenericParams { + pub fn new_all_unused(amount: u32) -> Self { + let mut bitset = FiniteBitSet::new_empty(); + bitset.set_range(0..amount); + Self(bitset) + } + + pub fn new_all_used() -> Self { + Self(FiniteBitSet::new_empty()) + } + + pub fn mark_used(&mut self, idx: u32) { + self.0.clear(idx); + } + + pub fn is_unused(&self, idx: u32) -> bool { + self.0.contains(idx).unwrap_or(false) + } + + pub fn is_used(&self, idx: u32) -> bool { + !self.is_unused(idx) + } + + pub fn all_used(&self) -> bool { + self.0.is_empty() + } +} diff --git a/compiler/rustc_middle/src/ty/mod.rs b/compiler/rustc_middle/src/ty/mod.rs index f01d74539a12e..fa571d480b646 100644 --- a/compiler/rustc_middle/src/ty/mod.rs +++ b/compiler/rustc_middle/src/ty/mod.rs @@ -86,7 +86,7 @@ pub use self::context::{ tls, CtxtInterners, DeducedParamAttrs, FreeRegionInfo, GlobalCtxt, Lift, OnDiskCache, TyCtxt, TyCtxtFeed, }; -pub use self::instance::{Instance, InstanceDef, ShortInstance}; +pub use self::instance::{Instance, InstanceDef, ShortInstance, UnusedGenericParams}; pub use self::list::List; pub use self::parameterized::ParameterizedOverTcx; pub use self::rvalue_scopes::RvalueScopes; diff --git a/compiler/rustc_middle/src/ty/parameterized.rs b/compiler/rustc_middle/src/ty/parameterized.rs index a21e3961cb627..72f451985796b 100644 --- a/compiler/rustc_middle/src/ty/parameterized.rs +++ b/compiler/rustc_middle/src/ty/parameterized.rs @@ -60,6 +60,7 @@ trivially_parameterized_over_tcx! { ty::ImplPolarity, ty::ReprOptions, ty::TraitDef, + ty::UnusedGenericParams, ty::Visibility, ty::adjustment::CoerceUnsizedInfo, ty::fast_reject::SimplifiedType, diff --git a/compiler/rustc_middle/src/ty/query.rs b/compiler/rustc_middle/src/ty/query.rs index 642900d3ab429..9d4ee22a7273b 100644 --- a/compiler/rustc_middle/src/ty/query.rs +++ b/compiler/rustc_middle/src/ty/query.rs @@ -34,7 +34,7 @@ use crate::ty::layout::TyAndLayout; use crate::ty::subst::{GenericArg, SubstsRef}; use crate::ty::util::AlwaysRequiresDrop; use crate::ty::GeneratorDiagnosticData; -use crate::ty::{self, CrateInherentImpls, ParamEnvAnd, Ty, TyCtxt}; +use crate::ty::{self, CrateInherentImpls, ParamEnvAnd, Ty, TyCtxt, UnusedGenericParams}; use rustc_ast as ast; use rustc_ast::expand::allocator::AllocatorKind; use rustc_attr as attr; @@ -50,7 +50,7 @@ use rustc_hir::def_id::{CrateNum, DefId, DefIdMap, DefIdSet, LocalDefId}; use rustc_hir::hir_id::OwnerId; use rustc_hir::lang_items::{LangItem, LanguageItems}; use rustc_hir::{Crate, ItemLocalId, TraitCandidate}; -use rustc_index::{bit_set::FiniteBitSet, vec::IndexVec}; +use rustc_index::vec::IndexVec; use rustc_session::config::{EntryFnType, OptLevel, OutputFilenames, SymbolManglingVersion}; use rustc_session::cstore::{CrateDepKind, CrateSource}; use rustc_session::cstore::{ExternCrate, ForeignModule, LinkagePreference, NativeLib}; diff --git a/compiler/rustc_monomorphize/src/errors.rs b/compiler/rustc_monomorphize/src/errors.rs index aa3227cac2de4..5233cfb21203b 100644 --- a/compiler/rustc_monomorphize/src/errors.rs +++ b/compiler/rustc_monomorphize/src/errors.rs @@ -32,13 +32,13 @@ pub struct TypeLengthLimit { pub type_length: usize, } -pub struct UnusedGenericParams { +pub struct UnusedGenericParamsHint { pub span: Span, pub param_spans: Vec, pub param_names: Vec, } -impl IntoDiagnostic<'_> for UnusedGenericParams { +impl IntoDiagnostic<'_> for UnusedGenericParamsHint { #[track_caller] fn into_diagnostic( self, diff --git a/compiler/rustc_monomorphize/src/polymorphize.rs b/compiler/rustc_monomorphize/src/polymorphize.rs index 703ed09a254a9..60fbdf2fc7a6e 100644 --- a/compiler/rustc_monomorphize/src/polymorphize.rs +++ b/compiler/rustc_monomorphize/src/polymorphize.rs @@ -6,7 +6,6 @@ //! for their size, offset of a field, etc.). use rustc_hir::{def::DefKind, def_id::DefId, ConstContext}; -use rustc_index::bit_set::FiniteBitSet; use rustc_middle::mir::{ self, visit::{TyContext, Visitor}, @@ -17,12 +16,12 @@ use rustc_middle::ty::{ query::Providers, subst::SubstsRef, visit::{TypeSuperVisitable, TypeVisitable, TypeVisitor}, - Const, Ty, TyCtxt, + Const, Ty, TyCtxt, UnusedGenericParams, }; use rustc_span::symbol::sym; use std::ops::ControlFlow; -use crate::errors::UnusedGenericParams; +use crate::errors::UnusedGenericParamsHint; /// Provide implementations of queries relating to polymorphization analysis. pub fn provide(providers: &mut Providers) { @@ -36,16 +35,16 @@ pub fn provide(providers: &mut Providers) { fn unused_generic_params<'tcx>( tcx: TyCtxt<'tcx>, instance: ty::InstanceDef<'tcx>, -) -> FiniteBitSet { +) -> UnusedGenericParams { if !tcx.sess.opts.unstable_opts.polymorphize { // If polymorphization disabled, then all parameters are used. - return FiniteBitSet::new_empty(); + return UnusedGenericParams::new_all_used(); } let def_id = instance.def_id(); // Exit early if this instance should not be polymorphized. if !should_polymorphize(tcx, def_id, instance) { - return FiniteBitSet::new_empty(); + return UnusedGenericParams::new_all_used(); } let generics = tcx.generics_of(def_id); @@ -53,14 +52,13 @@ fn unused_generic_params<'tcx>( // Exit early when there are no parameters to be unused. if generics.count() == 0 { - return FiniteBitSet::new_empty(); + return UnusedGenericParams::new_all_used(); } // Create a bitset with N rightmost ones for each parameter. let generics_count: u32 = generics.count().try_into().expect("more generic parameters than can fit into a `u32`"); - let mut unused_parameters = FiniteBitSet::::new_empty(); - unused_parameters.set_range(0..generics_count); + let mut unused_parameters = UnusedGenericParams::new_all_unused(generics_count); debug!(?unused_parameters, "(start)"); mark_used_by_default_parameters(tcx, def_id, generics, &mut unused_parameters); @@ -78,7 +76,7 @@ fn unused_generic_params<'tcx>( debug!(?unused_parameters, "(end)"); // Emit errors for debugging and testing if enabled. - if !unused_parameters.is_empty() { + if !unused_parameters.all_used() { emit_unused_generic_params_error(tcx, def_id, generics, &unused_parameters); } @@ -136,13 +134,13 @@ fn mark_used_by_default_parameters<'tcx>( tcx: TyCtxt<'tcx>, def_id: DefId, generics: &'tcx ty::Generics, - unused_parameters: &mut FiniteBitSet, + unused_parameters: &mut UnusedGenericParams, ) { match tcx.def_kind(def_id) { DefKind::Closure | DefKind::Generator => { for param in &generics.params { debug!(?param, "(closure/gen)"); - unused_parameters.clear(param.index); + unused_parameters.mark_used(param.index); } } DefKind::Mod @@ -178,7 +176,7 @@ fn mark_used_by_default_parameters<'tcx>( for param in &generics.params { debug!(?param, "(other)"); if let ty::GenericParamDefKind::Lifetime = param.kind { - unused_parameters.clear(param.index); + unused_parameters.mark_used(param.index); } } } @@ -196,7 +194,7 @@ fn emit_unused_generic_params_error<'tcx>( tcx: TyCtxt<'tcx>, def_id: DefId, generics: &'tcx ty::Generics, - unused_parameters: &FiniteBitSet, + unused_parameters: &UnusedGenericParams, ) { let base_def_id = tcx.typeck_root_def_id(def_id); if !tcx.has_attr(base_def_id, sym::rustc_polymorphize_error) { @@ -213,7 +211,7 @@ fn emit_unused_generic_params_error<'tcx>( let mut next_generics = Some(generics); while let Some(generics) = next_generics { for param in &generics.params { - if unused_parameters.contains(param.index).unwrap_or(false) { + if unused_parameters.is_unused(param.index) { debug!(?param); let def_span = tcx.def_span(param.def_id); param_spans.push(def_span); @@ -224,14 +222,14 @@ fn emit_unused_generic_params_error<'tcx>( next_generics = generics.parent.map(|did| tcx.generics_of(did)); } - tcx.sess.emit_err(UnusedGenericParams { span: fn_span, param_spans, param_names }); + tcx.sess.emit_err(UnusedGenericParamsHint { span: fn_span, param_spans, param_names }); } /// Visitor used to aggregate generic parameter uses. struct MarkUsedGenericParams<'a, 'tcx> { tcx: TyCtxt<'tcx>, def_id: DefId, - unused_parameters: &'a mut FiniteBitSet, + unused_parameters: &'a mut UnusedGenericParams, } impl<'a, 'tcx> MarkUsedGenericParams<'a, 'tcx> { @@ -244,7 +242,7 @@ impl<'a, 'tcx> MarkUsedGenericParams<'a, 'tcx> { debug!(?self.unused_parameters, ?unused); for (i, arg) in substs.iter().enumerate() { let i = i.try_into().unwrap(); - if !unused.contains(i).unwrap_or(false) { + if unused.is_used(i) { arg.visit_with(self); } } @@ -308,7 +306,7 @@ impl<'a, 'tcx> TypeVisitor<'tcx> for MarkUsedGenericParams<'a, 'tcx> { match c.kind() { ty::ConstKind::Param(param) => { debug!(?param); - self.unused_parameters.clear(param.index); + self.unused_parameters.mark_used(param.index); ControlFlow::CONTINUE } ty::ConstKind::Unevaluated(ty::UnevaluatedConst { def, substs }) From a4b859e35f2e055e6866d58e72477e4cf256afa0 Mon Sep 17 00:00:00 2001 From: Nilstrieb <48135649+Nilstrieb@users.noreply.github.com> Date: Thu, 1 Dec 2022 22:00:19 +0100 Subject: [PATCH 08/13] Delete unused polymorphization code --- .../rustc_monomorphize/src/polymorphize.rs | 47 +------------------ 1 file changed, 1 insertion(+), 46 deletions(-) diff --git a/compiler/rustc_monomorphize/src/polymorphize.rs b/compiler/rustc_monomorphize/src/polymorphize.rs index 60fbdf2fc7a6e..c8fc69eb856ab 100644 --- a/compiler/rustc_monomorphize/src/polymorphize.rs +++ b/compiler/rustc_monomorphize/src/polymorphize.rs @@ -340,55 +340,10 @@ impl<'a, 'tcx> TypeVisitor<'tcx> for MarkUsedGenericParams<'a, 'tcx> { } ty::Param(param) => { debug!(?param); - self.unused_parameters.clear(param.index); + self.unused_parameters.mark_used(param.index); ControlFlow::CONTINUE } _ => ty.super_visit_with(self), } } } - -/// Visitor used to check if a generic parameter is used. -struct HasUsedGenericParams<'a> { - unused_parameters: &'a FiniteBitSet, -} - -impl<'a, 'tcx> TypeVisitor<'tcx> for HasUsedGenericParams<'a> { - type BreakTy = (); - - #[instrument(level = "debug", skip(self))] - fn visit_const(&mut self, c: Const<'tcx>) -> ControlFlow { - if !c.has_non_region_param() { - return ControlFlow::CONTINUE; - } - - match c.kind() { - ty::ConstKind::Param(param) => { - if self.unused_parameters.contains(param.index).unwrap_or(false) { - ControlFlow::CONTINUE - } else { - ControlFlow::BREAK - } - } - _ => c.super_visit_with(self), - } - } - - #[instrument(level = "debug", skip(self))] - fn visit_ty(&mut self, ty: Ty<'tcx>) -> ControlFlow { - if !ty.has_non_region_param() { - return ControlFlow::CONTINUE; - } - - match ty.kind() { - ty::Param(param) => { - if self.unused_parameters.contains(param.index).unwrap_or(false) { - ControlFlow::CONTINUE - } else { - ControlFlow::BREAK - } - } - _ => ty.super_visit_with(self), - } - } -} From f7bc68bb4edd1974f7c4aa6113902132429c00e8 Mon Sep 17 00:00:00 2001 From: yukang Date: Wed, 11 Jan 2023 00:19:27 +0800 Subject: [PATCH 09/13] use with_capacity in read read_to_string --- library/std/src/fs.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/library/std/src/fs.rs b/library/std/src/fs.rs index 6ddd5c28cc2d3..5c5ef0b1125a0 100644 --- a/library/std/src/fs.rs +++ b/library/std/src/fs.rs @@ -249,9 +249,8 @@ pub struct DirBuilder { pub fn read>(path: P) -> io::Result> { fn inner(path: &Path) -> io::Result> { let mut file = File::open(path)?; - let mut bytes = Vec::new(); let size = file.metadata().map(|m| m.len()).unwrap_or(0); - bytes.reserve(size as usize); + let mut bytes = Vec::with_capacity(size as usize); io::default_read_to_end(&mut file, &mut bytes)?; Ok(bytes) } @@ -290,9 +289,8 @@ pub fn read>(path: P) -> io::Result> { pub fn read_to_string>(path: P) -> io::Result { fn inner(path: &Path) -> io::Result { let mut file = File::open(path)?; - let mut string = String::new(); let size = file.metadata().map(|m| m.len()).unwrap_or(0); - string.reserve(size as usize); + let mut string = String::with_capacity(size as usize); io::default_read_to_string(&mut file, &mut string)?; Ok(string) } From aca2f88d1e0905329971ee7dfd1d596490d0ca0b Mon Sep 17 00:00:00 2001 From: Kyle Huey Date: Tue, 10 Jan 2023 22:34:09 -0800 Subject: [PATCH 10/13] Disable "split dwarf inlining" by default. This matches clang's behavior and makes split-debuginfo behave as expected (i.e. actually split the debug info). Fixes #106592 --- compiler/rustc_session/src/options.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/rustc_session/src/options.rs b/compiler/rustc_session/src/options.rs index 043a60a1c5310..c9da7b046220a 100644 --- a/compiler/rustc_session/src/options.rs +++ b/compiler/rustc_session/src/options.rs @@ -1570,7 +1570,7 @@ options! { /// o/w tests have closure@path span_free_formats: bool = (false, parse_bool, [UNTRACKED], "exclude spans when debug-printing compiler state (default: no)"), - split_dwarf_inlining: bool = (true, parse_bool, [TRACKED], + split_dwarf_inlining: bool = (false, parse_bool, [TRACKED], "provide minimal debug info in the object/executable to facilitate online \ symbolication/stack traces in the absence of .dwo/.dwp files when using Split DWARF"), split_dwarf_kind: SplitDwarfKind = (SplitDwarfKind::Split, parse_split_dwarf_kind, [TRACKED], From d031befe7928fd218c959568c12b25c4b6e04efa Mon Sep 17 00:00:00 2001 From: Boxy Date: Wed, 11 Jan 2023 12:02:14 +0000 Subject: [PATCH 11/13] a --- triagebot.toml | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/triagebot.toml b/triagebot.toml index bee0371d36ef3..f129d433f0e19 100644 --- a/triagebot.toml +++ b/triagebot.toml @@ -249,6 +249,11 @@ trigger_files = [ [autolabel."S-waiting-on-review"] new_pr = true +[autolabel."WG-trait-system-refactor"] +trigger_files = [ + "compiler/rustc_trait_selection/solve" +] + [notify-zulip."I-prioritize"] zulip_stream = 245100 # #t-compiler/wg-prioritization/alerts topic = "#{number} {title}" @@ -344,7 +349,7 @@ cc = ["@BoxyUwU"] [mentions."compiler/rustc_trait_selection/src/solve/"] message = "Some changes occurred to the core trait solver" -cc = ["@lcnr", "@compiler-errors"] +cc = ["@rust-lang/initiative-trait-system-refactor"] [mentions."compiler/rustc_trait_selection/src/traits/engine.rs"] message = """ From cce2f5f7726875d3784988f5e54aae7f4421ba85 Mon Sep 17 00:00:00 2001 From: klensy Date: Wed, 11 Jan 2023 15:45:52 +0300 Subject: [PATCH 12/13] fix typo LocalItemId -> ItemLocalId --- compiler/rustc_hir/src/hir_id.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/compiler/rustc_hir/src/hir_id.rs b/compiler/rustc_hir/src/hir_id.rs index 3b4add0cf4d40..404abe2b068cb 100644 --- a/compiler/rustc_hir/src/hir_id.rs +++ b/compiler/rustc_hir/src/hir_id.rs @@ -148,7 +148,7 @@ rustc_index::newtype_index! { /// that is, within a `hir::Item`, `hir::TraitItem`, or `hir::ImplItem`. There is no /// guarantee that the numerical value of a given `ItemLocalId` corresponds to /// the node's position within the owning item in any way, but there is a - /// guarantee that the `LocalItemId`s within an owner occupy a dense range of + /// guarantee that the `ItemLocalId`s within an owner occupy a dense range of /// integers starting at zero, so a mapping that maps all or most nodes within /// an "item-like" to something else can be implemented by a `Vec` instead of a /// tree or hash map. @@ -161,7 +161,7 @@ impl ItemLocalId { pub const INVALID: ItemLocalId = ItemLocalId::MAX; } -// Safety: Ord is implement as just comparing the LocalItemId's numerical +// Safety: Ord is implement as just comparing the ItemLocalId's numerical // values and these are not changed by (de-)serialization. unsafe impl StableOrd for ItemLocalId {} From 5135eac5f7ccb129b6cb52a4c570dd11009f874d Mon Sep 17 00:00:00 2001 From: Yuki Omoto Date: Thu, 12 Jan 2023 00:17:48 +0900 Subject: [PATCH 13/13] Add log-backtrace option to show backtraces along with logging --- Cargo.lock | 1 + compiler/rustc_driver/src/lib.rs | 12 +++++- compiler/rustc_interface/src/tests.rs | 1 + compiler/rustc_log/Cargo.toml | 1 + compiler/rustc_log/src/lib.rs | 58 ++++++++++++++++++++++++++- compiler/rustc_session/src/options.rs | 2 + tests/rustdoc-ui/z-help.stdout | 1 + tests/ui/attributes/log-backtrace.rs | 9 +++++ 8 files changed, 81 insertions(+), 4 deletions(-) create mode 100644 tests/ui/attributes/log-backtrace.rs diff --git a/Cargo.lock b/Cargo.lock index 2a88152b5194a..b0f050a85d2f1 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4273,6 +4273,7 @@ version = "0.0.0" dependencies = [ "rustc_span", "tracing", + "tracing-core", "tracing-subscriber", "tracing-tree", ] diff --git a/compiler/rustc_driver/src/lib.rs b/compiler/rustc_driver/src/lib.rs index 3cbe0052359b3..6c2c1839554e6 100644 --- a/compiler/rustc_driver/src/lib.rs +++ b/compiler/rustc_driver/src/lib.rs @@ -231,6 +231,8 @@ fn run_compiler( registry: diagnostics_registry(), }; + init_rustc_env_logger_with_backtrace_option(&config.opts.unstable_opts.log_backtrace); + match make_input(config.opts.error_format, &matches.free) { Err(reported) => return Err(reported), Ok(Some((input, input_file_path))) => { @@ -1299,7 +1301,14 @@ pub fn install_ice_hook() { /// This allows tools to enable rust logging without having to magically match rustc's /// tracing crate version. pub fn init_rustc_env_logger() { - if let Err(error) = rustc_log::init_rustc_env_logger() { + init_rustc_env_logger_with_backtrace_option(&None); +} + +/// This allows tools to enable rust logging without having to magically match rustc's +/// tracing crate version. In contrast to `init_rustc_env_logger` it allows you to +/// choose a target module you wish to show backtraces along with its logging. +pub fn init_rustc_env_logger_with_backtrace_option(backtrace_target: &Option) { + if let Err(error) = rustc_log::init_rustc_env_logger_with_backtrace_option(backtrace_target) { early_error(ErrorOutputType::default(), &error.to_string()); } } @@ -1365,7 +1374,6 @@ mod signal_handler { pub fn main() -> ! { let start_time = Instant::now(); let start_rss = get_resident_set_size(); - init_rustc_env_logger(); signal_handler::install(); let mut callbacks = TimePassesCallbacks::default(); install_ice_hook(); diff --git a/compiler/rustc_interface/src/tests.rs b/compiler/rustc_interface/src/tests.rs index a3b9891ee64e9..07b28cc86cee1 100644 --- a/compiler/rustc_interface/src/tests.rs +++ b/compiler/rustc_interface/src/tests.rs @@ -748,6 +748,7 @@ fn test_unstable_options_tracking_hash() { tracked!(link_only, true); tracked!(llvm_plugins, vec![String::from("plugin_name")]); tracked!(location_detail, LocationDetail { file: true, line: false, column: false }); + tracked!(log_backtrace, Some("filter".to_string())); tracked!(maximal_hir_to_mir_coverage, true); tracked!(merge_functions, Some(MergeFunctions::Disabled)); tracked!(mir_emit_retag, true); diff --git a/compiler/rustc_log/Cargo.toml b/compiler/rustc_log/Cargo.toml index 3c50827c1abc3..7f955b0a75090 100644 --- a/compiler/rustc_log/Cargo.toml +++ b/compiler/rustc_log/Cargo.toml @@ -7,6 +7,7 @@ edition = "2021" tracing = "0.1.28" tracing-subscriber = { version = "0.3.3", default-features = false, features = ["fmt", "env-filter", "smallvec", "parking_lot", "ansi"] } tracing-tree = "0.2.0" +tracing-core = "0.1.28" [dev-dependencies] rustc_span = { path = "../rustc_span" } diff --git a/compiler/rustc_log/src/lib.rs b/compiler/rustc_log/src/lib.rs index 4cac88aff640e..fc1cabd2de951 100644 --- a/compiler/rustc_log/src/lib.rs +++ b/compiler/rustc_log/src/lib.rs @@ -45,16 +45,34 @@ use std::env::{self, VarError}; use std::fmt::{self, Display}; use std::io::{self, IsTerminal}; +use tracing_core::{Event, Subscriber}; use tracing_subscriber::filter::{Directive, EnvFilter, LevelFilter}; +use tracing_subscriber::fmt::{ + format::{self, FormatEvent, FormatFields}, + FmtContext, +}; use tracing_subscriber::layer::SubscriberExt; pub fn init_rustc_env_logger() -> Result<(), Error> { - init_env_logger("RUSTC_LOG") + init_rustc_env_logger_with_backtrace_option(&None) +} + +pub fn init_rustc_env_logger_with_backtrace_option( + backtrace_target: &Option, +) -> Result<(), Error> { + init_env_logger_with_backtrace_option("RUSTC_LOG", backtrace_target) } /// In contrast to `init_rustc_env_logger` this allows you to choose an env var /// other than `RUSTC_LOG`. pub fn init_env_logger(env: &str) -> Result<(), Error> { + init_env_logger_with_backtrace_option(env, &None) +} + +pub fn init_env_logger_with_backtrace_option( + env: &str, + backtrace_target: &Option, +) -> Result<(), Error> { let filter = match env::var(env) { Ok(env) => EnvFilter::new(env), _ => EnvFilter::default().add_directive(Directive::from(LevelFilter::WARN)), @@ -88,11 +106,47 @@ pub fn init_env_logger(env: &str) -> Result<(), Error> { let layer = layer.with_thread_ids(true).with_thread_names(true); let subscriber = tracing_subscriber::Registry::default().with(filter).with(layer); - tracing::subscriber::set_global_default(subscriber).unwrap(); + match backtrace_target { + Some(str) => { + let fmt_layer = tracing_subscriber::fmt::layer() + .with_writer(io::stderr) + .without_time() + .event_format(BacktraceFormatter { backtrace_target: str.to_string() }); + let subscriber = subscriber.with(fmt_layer); + tracing::subscriber::set_global_default(subscriber).unwrap(); + } + None => { + tracing::subscriber::set_global_default(subscriber).unwrap(); + } + }; Ok(()) } +struct BacktraceFormatter { + backtrace_target: String, +} + +impl FormatEvent for BacktraceFormatter +where + S: Subscriber + for<'a> tracing_subscriber::registry::LookupSpan<'a>, + N: for<'a> FormatFields<'a> + 'static, +{ + fn format_event( + &self, + _ctx: &FmtContext<'_, S, N>, + mut writer: format::Writer<'_>, + event: &Event<'_>, + ) -> fmt::Result { + let target = event.metadata().target(); + if !target.contains(&self.backtrace_target) { + return Ok(()); + } + let backtrace = std::backtrace::Backtrace::capture(); + writeln!(writer, "stack backtrace: \n{:?}", backtrace) + } +} + pub fn stdout_isatty() -> bool { io::stdout().is_terminal() } diff --git a/compiler/rustc_session/src/options.rs b/compiler/rustc_session/src/options.rs index 4c23606768819..399fa735807cb 100644 --- a/compiler/rustc_session/src/options.rs +++ b/compiler/rustc_session/src/options.rs @@ -1411,6 +1411,8 @@ options! { "what location details should be tracked when using caller_location, either \ `none`, or a comma separated list of location details, for which \ valid options are `file`, `line`, and `column` (default: `file,line,column`)"), + log_backtrace: Option = (None, parse_opt_string, [TRACKED], + "add a backtrace along with logging"), ls: bool = (false, parse_bool, [UNTRACKED], "list the symbols defined by a library crate (default: no)"), macro_backtrace: bool = (false, parse_bool, [UNTRACKED], diff --git a/tests/rustdoc-ui/z-help.stdout b/tests/rustdoc-ui/z-help.stdout index 43f30f3d6e800..4bdecdc1b7944 100644 --- a/tests/rustdoc-ui/z-help.stdout +++ b/tests/rustdoc-ui/z-help.stdout @@ -76,6 +76,7 @@ -Z llvm-plugins=val -- a list LLVM plugins to enable (space separated) -Z llvm-time-trace=val -- generate JSON tracing data file from LLVM data (default: no) -Z location-detail=val -- what location details should be tracked when using caller_location, either `none`, or a comma separated list of location details, for which valid options are `file`, `line`, and `column` (default: `file,line,column`) + -Z log-backtrace=val -- add a backtrace along with logging -Z ls=val -- list the symbols defined by a library crate (default: no) -Z macro-backtrace=val -- show macro backtraces (default: no) -Z maximal-hir-to-mir-coverage=val -- save as much information as possible about the correspondence between MIR and HIR as source scopes (default: no) diff --git a/tests/ui/attributes/log-backtrace.rs b/tests/ui/attributes/log-backtrace.rs new file mode 100644 index 0000000000000..3979d2001fc59 --- /dev/null +++ b/tests/ui/attributes/log-backtrace.rs @@ -0,0 +1,9 @@ +// run-pass +// +// This test makes sure that log-backtrace option doesn't give a compilation error. +// +// dont-check-compiler-stdout +// dont-check-compiler-stderr +// rustc-env:RUSTC_LOG=info +// compile-flags: -Zlog-backtrace=rustc_metadata::creader +fn main() {}