From 113e8dfb7293cc070214b42541781b2eeac25ae1 Mon Sep 17 00:00:00 2001 From: Charles Lew Date: Sat, 22 Oct 2022 18:48:20 +0800 Subject: [PATCH 01/14] Port `dead_code` lints to be translatable. --- .../locales/en-US/passes.ftl | 34 ++++ compiler/rustc_errors/src/diagnostic_impls.rs | 32 ++++ compiler/rustc_errors/src/lib.rs | 2 +- compiler/rustc_passes/src/dead.rs | 175 +++++++++--------- compiler/rustc_passes/src/errors.rs | 80 +++++++- ...lone-debug-dead-code-in-the-same-struct.rs | 2 +- ...-debug-dead-code-in-the-same-struct.stderr | 2 +- .../multiple-dead-codes-in-the-same-struct.rs | 2 +- ...tiple-dead-codes-in-the-same-struct.stderr | 2 +- .../ui/lint/dead-code/tuple-struct-field.rs | 2 +- .../lint/dead-code/tuple-struct-field.stderr | 2 +- 11 files changed, 236 insertions(+), 99 deletions(-) diff --git a/compiler/rustc_error_messages/locales/en-US/passes.ftl b/compiler/rustc_error_messages/locales/en-US/passes.ftl index 4bc6bd9fb2207..a5b002fa3579e 100644 --- a/compiler/rustc_error_messages/locales/en-US/passes.ftl +++ b/compiler/rustc_error_messages/locales/en-US/passes.ftl @@ -671,3 +671,37 @@ passes_missing_const_err = attributes `#[rustc_const_unstable]` and `#[rustc_const_stable]` require the function or method to be `const` .help = make the function or method const .label = attribute specified here + +passes_dead_codes = + { $multiple -> + *[true] multiple {$descr}s are + [false] { $num -> + [one] {$descr} {$name_list} is + *[other] {$descr}s {$name_list} are + } + } never {$participle} + +passes_change_fields_to_be_of_unit_type = + consider changing the { $num -> + [one] field + *[other] fields + } to be of unit type to suppress this warning + while preserving the field numbering, or remove the { $num -> + [one] field + *[other] fields + } + +passes_parent_info = + {$num -> + [one] {$descr} + *[other] {$descr}s + } in this {$parent_descr} + +passes_ignored_derived_impls = + `{$name}` has {$trait_list_len -> + [one] a derived impl + *[other] derived impls + } for the {$trait_list_len -> + [one] trait {$trait_list}, but this is + *[other] traits {$trait_list}, but these are + } intentionally ignored during dead code analysis diff --git a/compiler/rustc_errors/src/diagnostic_impls.rs b/compiler/rustc_errors/src/diagnostic_impls.rs index 7640b2919f78b..f6fe9192b45ca 100644 --- a/compiler/rustc_errors/src/diagnostic_impls.rs +++ b/compiler/rustc_errors/src/diagnostic_impls.rs @@ -11,6 +11,7 @@ use rustc_target::abi::TargetDataLayoutErrors; use rustc_target::spec::{PanicStrategy, SplitDebuginfo, StackProtector, TargetTriple}; use std::borrow::Cow; use std::fmt; +use std::fmt::Write; use std::num::ParseIntError; use std::path::{Path, PathBuf}; @@ -170,6 +171,37 @@ impl IntoDiagnosticArg for Level { } } +#[derive(Clone)] +pub struct DiagnosticSymbolList(Vec); + +impl From> for DiagnosticSymbolList { + fn from(v: Vec) -> Self { + DiagnosticSymbolList(v) + } +} + +impl IntoDiagnosticArg for DiagnosticSymbolList { + fn into_diagnostic_arg(self) -> DiagnosticArgValue<'static> { + // FIXME: replace the logic here with a real list formatter + let symbols = match &self.0[..] { + [symbol] => format!("`{symbol}`"), + [symbol, last] => { + format!("`{symbol}` and `{last}`",) + } + [symbols @ .., last] => { + let mut result = String::new(); + for symbol in symbols { + write!(result, "`{symbol}`, ").unwrap(); + } + write!(result, "and `{last}`").unwrap(); + result + } + [] => unreachable!(), + }; + DiagnosticArgValue::Str(Cow::Owned(symbols)) + } +} + impl IntoDiagnostic<'_, !> for TargetDataLayoutErrors<'_> { fn into_diagnostic(self, handler: &Handler) -> DiagnosticBuilder<'_, !> { let mut diag; diff --git a/compiler/rustc_errors/src/lib.rs b/compiler/rustc_errors/src/lib.rs index 0963ea71f8023..2c8a70981bc72 100644 --- a/compiler/rustc_errors/src/lib.rs +++ b/compiler/rustc_errors/src/lib.rs @@ -376,7 +376,7 @@ pub use diagnostic::{ DiagnosticStyledString, IntoDiagnosticArg, SubDiagnostic, }; pub use diagnostic_builder::{DiagnosticBuilder, EmissionGuarantee, Noted}; -pub use diagnostic_impls::DiagnosticArgFromDisplay; +pub use diagnostic_impls::{DiagnosticArgFromDisplay, DiagnosticSymbolList}; use std::backtrace::Backtrace; /// A handler deals with errors and other compiler output. diff --git a/compiler/rustc_passes/src/dead.rs b/compiler/rustc_passes/src/dead.rs index 6a97ad3fe86e2..9157c8279a5c2 100644 --- a/compiler/rustc_passes/src/dead.rs +++ b/compiler/rustc_passes/src/dead.rs @@ -4,7 +4,7 @@ use itertools::Itertools; use rustc_data_structures::fx::{FxHashMap, FxHashSet}; -use rustc_errors::{pluralize, Applicability, MultiSpan}; +use rustc_errors::MultiSpan; use rustc_hir as hir; use rustc_hir::def::{CtorOf, DefKind, Res}; use rustc_hir::def_id::{DefId, LocalDefId}; @@ -18,7 +18,10 @@ use rustc_session::lint; use rustc_span::symbol::{sym, Symbol}; use std::mem; -use crate::errors::UselessAssignment; +use crate::errors::{ + ChangeFieldsToBeOfUnitType, IgnoredDerivedImpls, MultipleDeadCodes, ParentInfo, + UselessAssignment, +}; // Any local node that may call something in its body block should be // explored. For example, if it's a live Node::Item that is a @@ -698,99 +701,89 @@ impl<'tcx> DeadVisitor<'tcx> { parent_item: Option, is_positional: bool, ) { - if let Some(&first_id) = dead_codes.first() { - let tcx = self.tcx; - let names: Vec<_> = dead_codes - .iter() - .map(|&def_id| tcx.item_name(def_id.to_def_id()).to_string()) - .collect(); - let spans: Vec<_> = dead_codes - .iter() - .map(|&def_id| match tcx.def_ident_span(def_id) { - Some(s) => s.with_ctxt(tcx.def_span(def_id).ctxt()), - None => tcx.def_span(def_id), + let Some(&first_id) = dead_codes.first() else { + return; + }; + let tcx = self.tcx; + let names: Vec<_> = + dead_codes.iter().map(|&def_id| tcx.item_name(def_id.to_def_id())).collect(); + let spans: Vec<_> = dead_codes + .iter() + .map(|&def_id| match tcx.def_ident_span(def_id) { + Some(s) => s.with_ctxt(tcx.def_span(def_id).ctxt()), + None => tcx.def_span(def_id), + }) + .collect(); + + let descr = tcx.def_kind(first_id).descr(first_id.to_def_id()); + let num = dead_codes.len(); + let multiple = num > 6; + let name_list = names.into(); + + let lint = if is_positional { + lint::builtin::UNUSED_TUPLE_STRUCT_FIELDS + } else { + lint::builtin::DEAD_CODE + }; + + let parent_info = if let Some(parent_item) = parent_item { + let parent_descr = tcx.def_kind(parent_item).descr(parent_item.to_def_id()); + Some(ParentInfo { + num, + descr, + parent_descr, + span: tcx.def_ident_span(parent_item).unwrap(), + }) + } else { + None + }; + + let encl_def_id = parent_item.unwrap_or(first_id); + let ignored_derived_impls = + if let Some(ign_traits) = self.ignored_derived_traits.get(&encl_def_id) { + let trait_list = ign_traits + .iter() + .map(|(trait_id, _)| self.tcx.item_name(*trait_id)) + .collect::>(); + let trait_list_len = trait_list.len(); + Some(IgnoredDerivedImpls { + name: self.tcx.item_name(encl_def_id.to_def_id()), + trait_list: trait_list.into(), + trait_list_len, }) - .collect(); - - let descr = tcx.def_kind(first_id).descr(first_id.to_def_id()); - let span_len = dead_codes.len(); - let names = match &names[..] { - _ if span_len > 6 => String::new(), - [name] => format!("`{name}` "), - [names @ .., last] => { - format!( - "{} and `{last}` ", - names.iter().map(|name| format!("`{name}`")).join(", ") - ) - } - [] => unreachable!(), + } else { + None }; - let msg = format!( - "{these}{descr}{s} {names}{are} never {participle}", - these = if span_len > 6 { "multiple " } else { "" }, - s = pluralize!(span_len), - are = pluralize!("is", span_len), - ); - - tcx.struct_span_lint_hir( - if is_positional { - lint::builtin::UNUSED_TUPLE_STRUCT_FIELDS - } else { - lint::builtin::DEAD_CODE - }, - tcx.hir().local_def_id_to_hir_id(first_id), - MultiSpan::from_spans(spans.clone()), - msg, - |err| { - if is_positional { - err.multipart_suggestion( - &format!( - "consider changing the field{s} to be of unit type to \ - suppress this warning while preserving the field \ - numbering, or remove the field{s}", - s = pluralize!(span_len) - ), - spans.iter().map(|sp| (*sp, "()".to_string())).collect(), - // "HasPlaceholders" because applying this fix by itself isn't - // enough: All constructor calls have to be adjusted as well - Applicability::HasPlaceholders, - ); - } - if let Some(parent_item) = parent_item { - let parent_descr = tcx.def_kind(parent_item).descr(parent_item.to_def_id()); - err.span_label( - tcx.def_ident_span(parent_item).unwrap(), - format!("{descr}{s} in this {parent_descr}", s = pluralize!(span_len)), - ); - } + let diag = if is_positional { + MultipleDeadCodes::UnusedTupleStructFields { + multiple, + num, + descr, + participle, + name_list, + change_fields_suggestion: ChangeFieldsToBeOfUnitType { num, spans: spans.clone() }, + parent_info, + ignored_derived_impls, + } + } else { + MultipleDeadCodes::DeadCodes { + multiple, + num, + descr, + participle, + name_list, + parent_info, + ignored_derived_impls, + } + }; - let encl_def_id = parent_item.unwrap_or(first_id); - if let Some(ign_traits) = self.ignored_derived_traits.get(&encl_def_id) { - let traits_str = ign_traits - .iter() - .map(|(trait_id, _)| format!("`{}`", self.tcx.item_name(*trait_id))) - .collect::>() - .join(" and "); - let plural_s = pluralize!(ign_traits.len()); - let article = if ign_traits.len() > 1 { "" } else { "a " }; - let is_are = if ign_traits.len() > 1 { "these are" } else { "this is" }; - let msg = format!( - "`{}` has {}derived impl{} for the trait{} {}, but {} \ - intentionally ignored during dead code analysis", - self.tcx.item_name(encl_def_id.to_def_id()), - article, - plural_s, - plural_s, - traits_str, - is_are - ); - err.note(&msg); - } - err - }, - ); - } + self.tcx.emit_spanned_lint( + lint, + tcx.hir().local_def_id_to_hir_id(first_id), + MultiSpan::from_spans(spans.clone()), + diag, + ); } fn warn_dead_fields_and_variants( diff --git a/compiler/rustc_passes/src/errors.rs b/compiler/rustc_passes/src/errors.rs index adaaf5392425b..d39d7629b287f 100644 --- a/compiler/rustc_passes/src/errors.rs +++ b/compiler/rustc_passes/src/errors.rs @@ -4,12 +4,16 @@ use std::{ }; use rustc_ast::Label; -use rustc_errors::{error_code, Applicability, ErrorGuaranteed, IntoDiagnostic, MultiSpan}; +use rustc_errors::{ + error_code, Applicability, DiagnosticSymbolList, ErrorGuaranteed, IntoDiagnostic, MultiSpan, +}; use rustc_hir::{self as hir, ExprKind, Target}; use rustc_macros::{Diagnostic, LintDiagnostic, Subdiagnostic}; use rustc_middle::ty::{MainDefinition, Ty}; use rustc_span::{Span, Symbol, DUMMY_SP}; +use rustc_errors::{pluralize, AddToDiagnostic, Diagnostic, SubdiagnosticMessage}; + use crate::lang_items::Duplicate; #[derive(LintDiagnostic)] @@ -1449,3 +1453,77 @@ pub struct MissingConstErr { #[label] pub const_span: Span, } + +#[derive(LintDiagnostic)] +pub enum MultipleDeadCodes<'tcx> { + #[diag(passes_dead_codes)] + DeadCodes { + multiple: bool, + num: usize, + descr: &'tcx str, + participle: &'tcx str, + name_list: DiagnosticSymbolList, + #[subdiagnostic] + parent_info: Option>, + #[subdiagnostic] + ignored_derived_impls: Option, + }, + #[diag(passes_dead_codes)] + UnusedTupleStructFields { + multiple: bool, + num: usize, + descr: &'tcx str, + participle: &'tcx str, + name_list: DiagnosticSymbolList, + #[subdiagnostic] + change_fields_suggestion: ChangeFieldsToBeOfUnitType, + #[subdiagnostic] + parent_info: Option>, + #[subdiagnostic] + ignored_derived_impls: Option, + }, +} + +#[derive(Subdiagnostic)] +#[label(passes_parent_info)] +pub struct ParentInfo<'tcx> { + pub num: usize, + pub descr: &'tcx str, + pub parent_descr: &'tcx str, + #[primary_span] + pub span: Span, +} + +#[derive(Subdiagnostic)] +#[note(passes_ignored_derived_impls)] +pub struct IgnoredDerivedImpls { + pub name: Symbol, + pub trait_list: DiagnosticSymbolList, + pub trait_list_len: usize, +} + +pub struct ChangeFieldsToBeOfUnitType { + pub num: usize, + pub spans: Vec, +} + +// FIXME: Replace this impl with a derive. +impl AddToDiagnostic for ChangeFieldsToBeOfUnitType { + fn add_to_diagnostic_with(self, diag: &mut Diagnostic, _: F) + where + F: Fn(&mut Diagnostic, SubdiagnosticMessage) -> SubdiagnosticMessage, + { + diag.multipart_suggestion( + &format!( + "consider changing the field{s} to be of unit type to \ + suppress this warning while preserving the field \ + numbering, or remove the field{s}", + s = pluralize!(self.num) + ), + self.spans.iter().map(|sp| (*sp, "()".to_string())).collect(), + // "HasPlaceholders" because applying this fix by itself isn't + // enough: All constructor calls have to be adjusted as well + Applicability::HasPlaceholders, + ); + } +} diff --git a/src/test/ui/derives/clone-debug-dead-code-in-the-same-struct.rs b/src/test/ui/derives/clone-debug-dead-code-in-the-same-struct.rs index 15d06817577ea..6ab1fb7b039bd 100644 --- a/src/test/ui/derives/clone-debug-dead-code-in-the-same-struct.rs +++ b/src/test/ui/derives/clone-debug-dead-code-in-the-same-struct.rs @@ -3,7 +3,7 @@ #[derive(Debug)] pub struct Whatever { pub field0: (), - field1: (), //~ ERROR fields `field1`, `field2`, `field3` and `field4` are never read + field1: (), //~ ERROR fields `field1`, `field2`, `field3`, and `field4` are never read field2: (), field3: (), field4: (), diff --git a/src/test/ui/derives/clone-debug-dead-code-in-the-same-struct.stderr b/src/test/ui/derives/clone-debug-dead-code-in-the-same-struct.stderr index 512b870fa4b6c..7f4f78cebc918 100644 --- a/src/test/ui/derives/clone-debug-dead-code-in-the-same-struct.stderr +++ b/src/test/ui/derives/clone-debug-dead-code-in-the-same-struct.stderr @@ -1,4 +1,4 @@ -error: fields `field1`, `field2`, `field3` and `field4` are never read +error: fields `field1`, `field2`, `field3`, and `field4` are never read --> $DIR/clone-debug-dead-code-in-the-same-struct.rs:6:5 | LL | pub struct Whatever { diff --git a/src/test/ui/lint/dead-code/multiple-dead-codes-in-the-same-struct.rs b/src/test/ui/lint/dead-code/multiple-dead-codes-in-the-same-struct.rs index e3935cf9149bb..2003e1e293a58 100644 --- a/src/test/ui/lint/dead-code/multiple-dead-codes-in-the-same-struct.rs +++ b/src/test/ui/lint/dead-code/multiple-dead-codes-in-the-same-struct.rs @@ -7,7 +7,7 @@ struct Bar { b: usize, //~ ERROR field `b` is never read #[deny(dead_code)] c: usize, //~ ERROR fields `c` and `e` are never read - d: usize, //~ WARN fields `d`, `f` and `g` are never read + d: usize, //~ WARN fields `d`, `f`, and `g` are never read #[deny(dead_code)] e: usize, f: usize, diff --git a/src/test/ui/lint/dead-code/multiple-dead-codes-in-the-same-struct.stderr b/src/test/ui/lint/dead-code/multiple-dead-codes-in-the-same-struct.stderr index c0f1ed38f6de3..0e5c78a716797 100644 --- a/src/test/ui/lint/dead-code/multiple-dead-codes-in-the-same-struct.stderr +++ b/src/test/ui/lint/dead-code/multiple-dead-codes-in-the-same-struct.stderr @@ -1,4 +1,4 @@ -warning: fields `d`, `f` and `g` are never read +warning: fields `d`, `f`, and `g` are never read --> $DIR/multiple-dead-codes-in-the-same-struct.rs:10:5 | LL | struct Bar { diff --git a/src/test/ui/lint/dead-code/tuple-struct-field.rs b/src/test/ui/lint/dead-code/tuple-struct-field.rs index b15d706368633..14fb30be949dc 100644 --- a/src/test/ui/lint/dead-code/tuple-struct-field.rs +++ b/src/test/ui/lint/dead-code/tuple-struct-field.rs @@ -11,7 +11,7 @@ struct SingleUnused(i32, [u8; LEN], String); //~| HELP: consider changing the field to be of unit type struct MultipleUnused(i32, f32, String, u8); -//~^ ERROR: fields `0`, `1`, `2` and `3` are never read +//~^ ERROR: fields `0`, `1`, `2`, and `3` are never read //~| NOTE: fields in this struct //~| HELP: consider changing the fields to be of unit type diff --git a/src/test/ui/lint/dead-code/tuple-struct-field.stderr b/src/test/ui/lint/dead-code/tuple-struct-field.stderr index ca0989f5b987f..b8ad5cbe4e977 100644 --- a/src/test/ui/lint/dead-code/tuple-struct-field.stderr +++ b/src/test/ui/lint/dead-code/tuple-struct-field.stderr @@ -16,7 +16,7 @@ help: consider changing the field to be of unit type to suppress this warning wh LL | struct SingleUnused(i32, (), String); | ~~ -error: fields `0`, `1`, `2` and `3` are never read +error: fields `0`, `1`, `2`, and `3` are never read --> $DIR/tuple-struct-field.rs:13:23 | LL | struct MultipleUnused(i32, f32, String, u8); From 3af058ec947b91590fe3ffd47a9bb2d690d2cb9d Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Fri, 28 Oct 2022 11:22:33 +0200 Subject: [PATCH 02/14] libtest: run all tests in their own thread, if supported by the host --- library/test/src/lib.rs | 58 +++++++++---------- library/test/src/options.rs | 7 --- library/test/src/tests.rs | 26 +++------ .../libtest-json/output-default.json | 2 +- .../libtest-json/output-stdout-success.json | 4 +- .../test-attrs/test-thread-capture.run.stdout | 2 +- .../test-thread-nocapture.run.stderr | 2 +- 7 files changed, 42 insertions(+), 59 deletions(-) diff --git a/library/test/src/lib.rs b/library/test/src/lib.rs index 141f16d17f022..a822df38e5758 100644 --- a/library/test/src/lib.rs +++ b/library/test/src/lib.rs @@ -40,7 +40,7 @@ pub mod test { cli::{parse_opts, TestOpts}, filter_tests, helpers::metrics::{Metric, MetricMap}, - options::{Concurrent, Options, RunIgnored, RunStrategy, ShouldPanic}, + options::{Options, RunIgnored, RunStrategy, ShouldPanic}, run_test, test_main, test_main_static, test_result::{TestResult, TrFailed, TrFailedMsg, TrIgnored, TrOk}, time::{TestExecTime, TestTimeOptions}, @@ -85,7 +85,7 @@ use event::{CompletedTest, TestEvent}; use helpers::concurrency::get_concurrency; use helpers::exit_code::get_exit_code; use helpers::shuffle::{get_shuffle_seed, shuffle_tests}; -use options::{Concurrent, RunStrategy}; +use options::RunStrategy; use test_result::*; use time::TestExecTime; @@ -235,6 +235,19 @@ where join_handle: Option>, } + impl RunningTest { + fn join(self, completed_test: &mut CompletedTest) { + if let Some(join_handle) = self.join_handle { + if let Err(_) = join_handle.join() { + if let TrOk = completed_test.result { + completed_test.result = + TrFailedMsg("panicked after reporting success".to_string()); + } + } + } + } + } + // Use a deterministic hasher type TestMap = HashMap>; @@ -328,10 +341,10 @@ where let (id, test) = remaining.pop_front().unwrap(); let event = TestEvent::TeWait(test.desc.clone()); notify_about_test_event(event)?; - let join_handle = - run_test(opts, !opts.run_tests, id, test, run_strategy, tx.clone(), Concurrent::No); - assert!(join_handle.is_none()); - let completed_test = rx.recv().unwrap(); + let join_handle = run_test(opts, !opts.run_tests, id, test, run_strategy, tx.clone()); + // Wait for the test to complete. + let mut completed_test = rx.recv().unwrap(); + RunningTest { join_handle }.join(&mut completed_test); let event = TestEvent::TeResult(completed_test); notify_about_test_event(event)?; @@ -345,15 +358,8 @@ where let event = TestEvent::TeWait(desc.clone()); notify_about_test_event(event)?; //here no pad - let join_handle = run_test( - opts, - !opts.run_tests, - id, - test, - run_strategy, - tx.clone(), - Concurrent::Yes, - ); + let join_handle = + run_test(opts, !opts.run_tests, id, test, run_strategy, tx.clone()); running_tests.insert(id, RunningTest { join_handle }); timeout_queue.push_back(TimeoutEntry { id, desc, timeout }); pending += 1; @@ -385,14 +391,7 @@ where let mut completed_test = res.unwrap(); let running_test = running_tests.remove(&completed_test.id).unwrap(); - if let Some(join_handle) = running_test.join_handle { - if let Err(_) = join_handle.join() { - if let TrOk = completed_test.result { - completed_test.result = - TrFailedMsg("panicked after reporting success".to_string()); - } - } - } + running_test.join(&mut completed_test); let event = TestEvent::TeResult(completed_test); notify_about_test_event(event)?; @@ -405,8 +404,10 @@ where for (id, b) in filtered_benchs { let event = TestEvent::TeWait(b.desc.clone()); notify_about_test_event(event)?; - run_test(opts, false, id, b, run_strategy, tx.clone(), Concurrent::No); - let completed_test = rx.recv().unwrap(); + let join_handle = run_test(opts, false, id, b, run_strategy, tx.clone()); + // Wait for the test to complete. + let mut completed_test = rx.recv().unwrap(); + RunningTest { join_handle }.join(&mut completed_test); let event = TestEvent::TeResult(completed_test); notify_about_test_event(event)?; @@ -480,7 +481,6 @@ pub fn run_test( test: TestDescAndFn, strategy: RunStrategy, monitor_ch: Sender, - concurrency: Concurrent, ) -> Option> { let TestDescAndFn { desc, testfn } = test; @@ -498,7 +498,6 @@ pub fn run_test( struct TestRunOpts { pub strategy: RunStrategy, pub nocapture: bool, - pub concurrency: Concurrent, pub time: Option, } @@ -509,7 +508,6 @@ pub fn run_test( testfn: Box Result<(), String> + Send>, opts: TestRunOpts, ) -> Option> { - let concurrency = opts.concurrency; let name = desc.name.clone(); let runtest = move || match opts.strategy { @@ -536,7 +534,7 @@ pub fn run_test( // the test synchronously, regardless of the concurrency // level. let supports_threads = !cfg!(target_os = "emscripten") && !cfg!(target_family = "wasm"); - if concurrency == Concurrent::Yes && supports_threads { + if supports_threads { let cfg = thread::Builder::new().name(name.as_slice().to_owned()); let mut runtest = Arc::new(Mutex::new(Some(runtest))); let runtest2 = runtest.clone(); @@ -557,7 +555,7 @@ pub fn run_test( } let test_run_opts = - TestRunOpts { strategy, nocapture: opts.nocapture, concurrency, time: opts.time_options }; + TestRunOpts { strategy, nocapture: opts.nocapture, time: opts.time_options }; match testfn { DynBenchFn(benchfn) => { diff --git a/library/test/src/options.rs b/library/test/src/options.rs index baf36b5f1d85e..75ec0b616e193 100644 --- a/library/test/src/options.rs +++ b/library/test/src/options.rs @@ -1,12 +1,5 @@ //! Enums denoting options for test execution. -/// Whether to execute tests concurrently or not -#[derive(Copy, Clone, Debug, PartialEq, Eq)] -pub enum Concurrent { - Yes, - No, -} - /// Number of times to run a benchmarked function #[derive(Clone, PartialEq, Eq)] pub enum BenchMode { diff --git a/library/test/src/tests.rs b/library/test/src/tests.rs index b54be64efcfed..7b2e6707f9d11 100644 --- a/library/test/src/tests.rs +++ b/library/test/src/tests.rs @@ -102,7 +102,7 @@ pub fn do_not_run_ignored_tests() { testfn: DynTestFn(Box::new(f)), }; let (tx, rx) = channel(); - run_test(&TestOpts::new(), false, TestId(0), desc, RunStrategy::InProcess, tx, Concurrent::No); + run_test(&TestOpts::new(), false, TestId(0), desc, RunStrategy::InProcess, tx); let result = rx.recv().unwrap().result; assert_ne!(result, TrOk); } @@ -125,7 +125,7 @@ pub fn ignored_tests_result_in_ignored() { testfn: DynTestFn(Box::new(f)), }; let (tx, rx) = channel(); - run_test(&TestOpts::new(), false, TestId(0), desc, RunStrategy::InProcess, tx, Concurrent::No); + run_test(&TestOpts::new(), false, TestId(0), desc, RunStrategy::InProcess, tx); let result = rx.recv().unwrap().result; assert_eq!(result, TrIgnored); } @@ -150,7 +150,7 @@ fn test_should_panic() { testfn: DynTestFn(Box::new(f)), }; let (tx, rx) = channel(); - run_test(&TestOpts::new(), false, TestId(0), desc, RunStrategy::InProcess, tx, Concurrent::No); + run_test(&TestOpts::new(), false, TestId(0), desc, RunStrategy::InProcess, tx); let result = rx.recv().unwrap().result; assert_eq!(result, TrOk); } @@ -175,7 +175,7 @@ fn test_should_panic_good_message() { testfn: DynTestFn(Box::new(f)), }; let (tx, rx) = channel(); - run_test(&TestOpts::new(), false, TestId(0), desc, RunStrategy::InProcess, tx, Concurrent::No); + run_test(&TestOpts::new(), false, TestId(0), desc, RunStrategy::InProcess, tx); let result = rx.recv().unwrap().result; assert_eq!(result, TrOk); } @@ -205,7 +205,7 @@ fn test_should_panic_bad_message() { testfn: DynTestFn(Box::new(f)), }; let (tx, rx) = channel(); - run_test(&TestOpts::new(), false, TestId(0), desc, RunStrategy::InProcess, tx, Concurrent::No); + run_test(&TestOpts::new(), false, TestId(0), desc, RunStrategy::InProcess, tx); let result = rx.recv().unwrap().result; assert_eq!(result, TrFailedMsg(failed_msg.to_string())); } @@ -239,7 +239,7 @@ fn test_should_panic_non_string_message_type() { testfn: DynTestFn(Box::new(f)), }; let (tx, rx) = channel(); - run_test(&TestOpts::new(), false, TestId(0), desc, RunStrategy::InProcess, tx, Concurrent::No); + run_test(&TestOpts::new(), false, TestId(0), desc, RunStrategy::InProcess, tx); let result = rx.recv().unwrap().result; assert_eq!(result, TrFailedMsg(failed_msg)); } @@ -267,15 +267,7 @@ fn test_should_panic_but_succeeds() { testfn: DynTestFn(Box::new(f)), }; let (tx, rx) = channel(); - run_test( - &TestOpts::new(), - false, - TestId(0), - desc, - RunStrategy::InProcess, - tx, - Concurrent::No, - ); + run_test(&TestOpts::new(), false, TestId(0), desc, RunStrategy::InProcess, tx); let result = rx.recv().unwrap().result; assert_eq!( result, @@ -306,7 +298,7 @@ fn report_time_test_template(report_time: bool) -> Option { let test_opts = TestOpts { time_options, ..TestOpts::new() }; let (tx, rx) = channel(); - run_test(&test_opts, false, TestId(0), desc, RunStrategy::InProcess, tx, Concurrent::No); + run_test(&test_opts, false, TestId(0), desc, RunStrategy::InProcess, tx); let exec_time = rx.recv().unwrap().exec_time; exec_time } @@ -345,7 +337,7 @@ fn time_test_failure_template(test_type: TestType) -> TestResult { let test_opts = TestOpts { time_options: Some(time_options), ..TestOpts::new() }; let (tx, rx) = channel(); - run_test(&test_opts, false, TestId(0), desc, RunStrategy::InProcess, tx, Concurrent::No); + run_test(&test_opts, false, TestId(0), desc, RunStrategy::InProcess, tx); let result = rx.recv().unwrap().result; result diff --git a/src/test/run-make-fulldeps/libtest-json/output-default.json b/src/test/run-make-fulldeps/libtest-json/output-default.json index 63342abc6ef71..ad22b66eda69f 100644 --- a/src/test/run-make-fulldeps/libtest-json/output-default.json +++ b/src/test/run-make-fulldeps/libtest-json/output-default.json @@ -2,7 +2,7 @@ { "type": "test", "event": "started", "name": "a" } { "type": "test", "name": "a", "event": "ok" } { "type": "test", "event": "started", "name": "b" } -{ "type": "test", "name": "b", "event": "failed", "stdout": "thread 'main' panicked at 'assertion failed: false', f.rs:9:5\nnote: run with `RUST_BACKTRACE=1` environment variable to display a backtrace\n" } +{ "type": "test", "name": "b", "event": "failed", "stdout": "thread 'b' panicked at 'assertion failed: false', f.rs:9:5\nnote: run with `RUST_BACKTRACE=1` environment variable to display a backtrace\n" } { "type": "test", "event": "started", "name": "c" } { "type": "test", "name": "c", "event": "ok" } { "type": "test", "event": "started", "name": "d" } diff --git a/src/test/run-make-fulldeps/libtest-json/output-stdout-success.json b/src/test/run-make-fulldeps/libtest-json/output-stdout-success.json index 8f19114460e89..ec98172eb1c4e 100644 --- a/src/test/run-make-fulldeps/libtest-json/output-stdout-success.json +++ b/src/test/run-make-fulldeps/libtest-json/output-stdout-success.json @@ -2,9 +2,9 @@ { "type": "test", "event": "started", "name": "a" } { "type": "test", "name": "a", "event": "ok", "stdout": "print from successful test\n" } { "type": "test", "event": "started", "name": "b" } -{ "type": "test", "name": "b", "event": "failed", "stdout": "thread 'main' panicked at 'assertion failed: false', f.rs:9:5\nnote: run with `RUST_BACKTRACE=1` environment variable to display a backtrace\n" } +{ "type": "test", "name": "b", "event": "failed", "stdout": "thread 'b' panicked at 'assertion failed: false', f.rs:9:5\nnote: run with `RUST_BACKTRACE=1` environment variable to display a backtrace\n" } { "type": "test", "event": "started", "name": "c" } -{ "type": "test", "name": "c", "event": "ok", "stdout": "thread 'main' panicked at 'assertion failed: false', f.rs:15:5\n" } +{ "type": "test", "name": "c", "event": "ok", "stdout": "thread 'c' panicked at 'assertion failed: false', f.rs:15:5\n" } { "type": "test", "event": "started", "name": "d" } { "type": "test", "name": "d", "event": "ignored", "message": "msg" } { "type": "suite", "event": "failed", "passed": 2, "failed": 1, "ignored": 1, "measured": 0, "filtered_out": 0, "exec_time": $TIME } diff --git a/src/test/ui/test-attrs/test-thread-capture.run.stdout b/src/test/ui/test-attrs/test-thread-capture.run.stdout index c712a78afb0f1..513c8cf2add00 100644 --- a/src/test/ui/test-attrs/test-thread-capture.run.stdout +++ b/src/test/ui/test-attrs/test-thread-capture.run.stdout @@ -10,7 +10,7 @@ fee fie foe fum -thread 'main' panicked at 'explicit panic', $DIR/test-thread-capture.rs:32:5 +thread 'thready_fail' panicked at 'explicit panic', $DIR/test-thread-capture.rs:32:5 note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace diff --git a/src/test/ui/test-attrs/test-thread-nocapture.run.stderr b/src/test/ui/test-attrs/test-thread-nocapture.run.stderr index 0a12a052819c9..8c905d1af8572 100644 --- a/src/test/ui/test-attrs/test-thread-nocapture.run.stderr +++ b/src/test/ui/test-attrs/test-thread-nocapture.run.stderr @@ -1,2 +1,2 @@ -thread 'main' panicked at 'explicit panic', $DIR/test-thread-nocapture.rs:32:5 +thread 'thready_fail' panicked at 'explicit panic', $DIR/test-thread-nocapture.rs:32:5 note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace From 03e4c76dcffc35c660bbea602993828328fda7b2 Mon Sep 17 00:00:00 2001 From: Amanieu d'Antras Date: Wed, 2 Nov 2022 18:47:58 +0000 Subject: [PATCH 03/14] asm: Work around LLVM bug on AArch64 Upstream issue: https://github.com/llvm/llvm-project/issues/58384 LLVM gets confused if we assign a 32-bit value to a 64-bit register, so pass the 32-bit register name to LLVM in that case. --- compiler/rustc_codegen_llvm/src/asm.rs | 57 ++++++++++++++++++++++++-- src/test/ui/asm/aarch64/llvm-58384.rs | 16 ++++++++ 2 files changed, 70 insertions(+), 3 deletions(-) create mode 100644 src/test/ui/asm/aarch64/llvm-58384.rs diff --git a/compiler/rustc_codegen_llvm/src/asm.rs b/compiler/rustc_codegen_llvm/src/asm.rs index 017513721b75b..2e65b1b34608a 100644 --- a/compiler/rustc_codegen_llvm/src/asm.rs +++ b/compiler/rustc_codegen_llvm/src/asm.rs @@ -496,6 +496,44 @@ fn xmm_reg_index(reg: InlineAsmReg) -> Option { } } +/// If the register is an AArch64 integer register then return its index. +fn a64_reg_index(reg: InlineAsmReg) -> Option { + match reg { + InlineAsmReg::AArch64(AArch64InlineAsmReg::x0) => Some(0), + InlineAsmReg::AArch64(AArch64InlineAsmReg::x1) => Some(1), + InlineAsmReg::AArch64(AArch64InlineAsmReg::x2) => Some(2), + InlineAsmReg::AArch64(AArch64InlineAsmReg::x3) => Some(3), + InlineAsmReg::AArch64(AArch64InlineAsmReg::x4) => Some(4), + InlineAsmReg::AArch64(AArch64InlineAsmReg::x5) => Some(5), + InlineAsmReg::AArch64(AArch64InlineAsmReg::x6) => Some(6), + InlineAsmReg::AArch64(AArch64InlineAsmReg::x7) => Some(7), + InlineAsmReg::AArch64(AArch64InlineAsmReg::x8) => Some(8), + InlineAsmReg::AArch64(AArch64InlineAsmReg::x9) => Some(9), + InlineAsmReg::AArch64(AArch64InlineAsmReg::x10) => Some(10), + InlineAsmReg::AArch64(AArch64InlineAsmReg::x11) => Some(11), + InlineAsmReg::AArch64(AArch64InlineAsmReg::x12) => Some(12), + InlineAsmReg::AArch64(AArch64InlineAsmReg::x13) => Some(13), + InlineAsmReg::AArch64(AArch64InlineAsmReg::x14) => Some(14), + InlineAsmReg::AArch64(AArch64InlineAsmReg::x15) => Some(15), + InlineAsmReg::AArch64(AArch64InlineAsmReg::x16) => Some(16), + InlineAsmReg::AArch64(AArch64InlineAsmReg::x17) => Some(17), + InlineAsmReg::AArch64(AArch64InlineAsmReg::x18) => Some(18), + // x19 is reserved + InlineAsmReg::AArch64(AArch64InlineAsmReg::x20) => Some(20), + InlineAsmReg::AArch64(AArch64InlineAsmReg::x21) => Some(21), + InlineAsmReg::AArch64(AArch64InlineAsmReg::x22) => Some(22), + InlineAsmReg::AArch64(AArch64InlineAsmReg::x23) => Some(23), + InlineAsmReg::AArch64(AArch64InlineAsmReg::x24) => Some(24), + InlineAsmReg::AArch64(AArch64InlineAsmReg::x25) => Some(25), + InlineAsmReg::AArch64(AArch64InlineAsmReg::x26) => Some(26), + InlineAsmReg::AArch64(AArch64InlineAsmReg::x27) => Some(27), + InlineAsmReg::AArch64(AArch64InlineAsmReg::x28) => Some(28), + // x29 is reserved + InlineAsmReg::AArch64(AArch64InlineAsmReg::x30) => Some(30), + _ => None, + } +} + /// If the register is an AArch64 vector register then return its index. fn a64_vreg_index(reg: InlineAsmReg) -> Option { match reg { @@ -526,6 +564,22 @@ fn reg_to_llvm(reg: InlineAsmRegOrRegClass, layout: Option<&TyAndLayout<'_>>) -> 'x' }; format!("{{{}mm{}}}", class, idx) + } else if let Some(idx) = a64_reg_index(reg) { + let class = if let Some(layout) = layout { + match layout.size.bytes() { + 8 => 'x', + _ => 'w', + } + } else { + // We use i32 as the type for discarded outputs + 'w' + }; + if class == 'x' && reg == InlineAsmReg::AArch64(AArch64InlineAsmReg::x30) { + // LLVM doesn't recognize x30. use lr instead. + "{lr}".to_string() + } else { + format!("{{{}{}}}", class, idx) + } } else if let Some(idx) = a64_vreg_index(reg) { let class = if let Some(layout) = layout { match layout.size.bytes() { @@ -541,9 +595,6 @@ fn reg_to_llvm(reg: InlineAsmRegOrRegClass, layout: Option<&TyAndLayout<'_>>) -> 'q' }; format!("{{{}{}}}", class, idx) - } else if reg == InlineAsmReg::AArch64(AArch64InlineAsmReg::x30) { - // LLVM doesn't recognize x30 - "{lr}".to_string() } else if reg == InlineAsmReg::Arm(ArmInlineAsmReg::r14) { // LLVM doesn't recognize r14 "{lr}".to_string() diff --git a/src/test/ui/asm/aarch64/llvm-58384.rs b/src/test/ui/asm/aarch64/llvm-58384.rs new file mode 100644 index 0000000000000..308f789082959 --- /dev/null +++ b/src/test/ui/asm/aarch64/llvm-58384.rs @@ -0,0 +1,16 @@ +// only-aarch64 +// run-pass +// needs-asm-support + +// Test that we properly work around this LLVM issue: +// https://github.com/llvm/llvm-project/issues/58384 + +use std::arch::asm; + +fn main() { + let a: i32; + unsafe { + asm!("", inout("x0") 435 => a); + } + assert_eq!(a, 435); +} From dfab01b6e0ee60dda61b2c51ac76850c5e79b550 Mon Sep 17 00:00:00 2001 From: Collin Baker Date: Fri, 21 Oct 2022 14:38:56 -0400 Subject: [PATCH 04/14] Remove std's transitive dependency on cfg-if 0.1 After rust-lang/rust#101946 this completes the move to cfg-if 1.0 by: * Updating getrandom 0.1.14->0.1.16 * Updating panic_abort, panic_unwind, and unwind to cfg-if 1.0 --- Cargo.lock | 16 ++++++++-------- library/panic_abort/Cargo.toml | 2 +- library/panic_unwind/Cargo.toml | 2 +- library/unwind/Cargo.toml | 2 +- 4 files changed, 11 insertions(+), 11 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 33b1299976f3e..f79392cffdb39 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1526,11 +1526,11 @@ dependencies = [ [[package]] name = "getrandom" -version = "0.1.14" +version = "0.1.16" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7abc8dd8451921606d809ba32e95b6111925cd2906060d2dcc29c070220503eb" +checksum = "8fc3cb4d91f53b50155bdcfd23f6a4c39ae1969c2ae85982b135750cccaf5fce" dependencies = [ - "cfg-if 0.1.10", + "cfg-if 1.0.0", "libc", "wasi 0.9.0+wasi-snapshot-preview1", ] @@ -2478,7 +2478,7 @@ name = "panic_abort" version = "0.0.0" dependencies = [ "alloc", - "cfg-if 0.1.10", + "cfg-if 1.0.0", "compiler_builtins", "core", "libc", @@ -2489,7 +2489,7 @@ name = "panic_unwind" version = "0.0.0" dependencies = [ "alloc", - "cfg-if 0.1.10", + "cfg-if 1.0.0", "compiler_builtins", "core", "libc", @@ -2817,7 +2817,7 @@ version = "0.7.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "6a6b1679d49b24bbfe0c803429aa1874472f50d9b363131f0e89fc356b544d03" dependencies = [ - "getrandom 0.1.14", + "getrandom 0.1.16", "libc", "rand_chacha 0.2.2", "rand_core 0.5.1", @@ -2861,7 +2861,7 @@ version = "0.5.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "90bde5296fc891b0cef12a6d03ddccc162ce7b2aff54160af9338f8d40df6d19" dependencies = [ - "getrandom 0.1.14", + "getrandom 0.1.16", ] [[package]] @@ -5357,7 +5357,7 @@ name = "unwind" version = "0.0.0" dependencies = [ "cc", - "cfg-if 0.1.10", + "cfg-if 1.0.0", "compiler_builtins", "core", "libc", diff --git a/library/panic_abort/Cargo.toml b/library/panic_abort/Cargo.toml index 46183d1ad0066..e6ea2b1849b4c 100644 --- a/library/panic_abort/Cargo.toml +++ b/library/panic_abort/Cargo.toml @@ -13,7 +13,7 @@ doc = false [dependencies] alloc = { path = "../alloc" } -cfg-if = { version = "0.1.8", features = ['rustc-dep-of-std'] } +cfg-if = { version = "1.0", features = ['rustc-dep-of-std'] } core = { path = "../core" } libc = { version = "0.2", default-features = false } compiler_builtins = "0.1.0" diff --git a/library/panic_unwind/Cargo.toml b/library/panic_unwind/Cargo.toml index d720cc7bcbdf1..85386976d639a 100644 --- a/library/panic_unwind/Cargo.toml +++ b/library/panic_unwind/Cargo.toml @@ -17,4 +17,4 @@ core = { path = "../core" } libc = { version = "0.2", default-features = false } unwind = { path = "../unwind" } compiler_builtins = "0.1.0" -cfg-if = "0.1.8" +cfg-if = "1.0" diff --git a/library/unwind/Cargo.toml b/library/unwind/Cargo.toml index 69fce8d7795c1..32c4a7eb5c18c 100644 --- a/library/unwind/Cargo.toml +++ b/library/unwind/Cargo.toml @@ -17,7 +17,7 @@ doc = false core = { path = "../core" } libc = { version = "0.2.79", features = ['rustc-dep-of-std'], default-features = false } compiler_builtins = "0.1.0" -cfg-if = "0.1.8" +cfg-if = "1.0" [build-dependencies] cc = "1.0.69" From 3583f2758b438a9274b6b7f00bf4e586058e55d9 Mon Sep 17 00:00:00 2001 From: Boxy Date: Thu, 3 Nov 2022 18:52:08 +0000 Subject: [PATCH 05/14] Cleanups --- compiler/rustc_hir_typeck/src/method/mod.rs | 3 +- compiler/rustc_hir_typeck/src/method/probe.rs | 1 - .../rustc_hir_typeck/src/method/suggest.rs | 136 +++++------------- compiler/rustc_middle/src/traits/mod.rs | 9 +- compiler/rustc_middle/src/ty/sty.rs | 7 + .../src/traits/on_unimplemented.rs | 1 + 6 files changed, 56 insertions(+), 101 deletions(-) diff --git a/compiler/rustc_hir_typeck/src/method/mod.rs b/compiler/rustc_hir_typeck/src/method/mod.rs index a1278edefbb71..2c7b3bbf31c20 100644 --- a/compiler/rustc_hir_typeck/src/method/mod.rs +++ b/compiler/rustc_hir_typeck/src/method/mod.rs @@ -55,8 +55,7 @@ pub enum MethodError<'tcx> { // not-in-scope traits which may work. PrivateMatch(DefKind, DefId, Vec), - // Found a `Self: Sized` bound where `Self` is a trait object, also the caller may have - // forgotten to import a trait. + // Found a `Self: Sized` bound where `Self` is a trait object. IllegalSizedBound(Vec, bool, Span), // Found a match, but the return type is wrong diff --git a/compiler/rustc_hir_typeck/src/method/probe.rs b/compiler/rustc_hir_typeck/src/method/probe.rs index 28aa2302f882f..e88701685bc6d 100644 --- a/compiler/rustc_hir_typeck/src/method/probe.rs +++ b/compiler/rustc_hir_typeck/src/method/probe.rs @@ -1019,7 +1019,6 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> { let out_of_scope_traits = match self.pick_core() { Some(Ok(p)) => vec![p.item.container_id(self.tcx)], - //Some(Ok(p)) => p.iter().map(|p| p.item.container().id()).collect(), Some(Err(MethodError::Ambiguity(v))) => v .into_iter() .map(|source| match source { diff --git a/compiler/rustc_hir_typeck/src/method/suggest.rs b/compiler/rustc_hir_typeck/src/method/suggest.rs index 6c21ed902d007..04ecd2757b427 100644 --- a/compiler/rustc_hir_typeck/src/method/suggest.rs +++ b/compiler/rustc_hir_typeck/src/method/suggest.rs @@ -248,7 +248,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { match error { MethodError::NoMatch(NoMatchData { - static_candidates: mut static_sources, + mut static_candidates, unsatisfied_predicates, out_of_scope_traits, lev_candidate, @@ -288,9 +288,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { if generics.len() > 0 { let mut autoderef = self.autoderef(span, actual); let candidate_found = autoderef.any(|(ty, _)| { - if let ty::Adt(adt_deref, _) = ty.kind() { + if let ty::Adt(adt_def, _) = ty.kind() { self.tcx - .inherent_impls(adt_deref.did()) + .inherent_impls(adt_def.did()) .iter() .filter_map(|def_id| self.associated_value(*def_id, item_name)) .count() @@ -348,15 +348,12 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { } let ty_span = match actual.kind() { - ty::Param(param_type) => { - let generics = self.tcx.generics_of(self.body_id.owner.to_def_id()); - let type_param = generics.type_param(param_type, self.tcx); - Some(self.tcx.def_span(type_param.def_id)) - } + ty::Param(param_type) => Some( + param_type.span_from_generics(self.tcx, self.body_id.owner.to_def_id()), + ), ty::Adt(def, _) if def.did().is_local() => Some(tcx.def_span(def.did())), _ => None, }; - if let Some(span) = ty_span { err.span_label( span, @@ -386,7 +383,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { let mut custom_span_label = false; - if !static_sources.is_empty() { + if !static_candidates.is_empty() { err.note( "found the following associated functions; to be used as methods, \ functions must have a `self` parameter", @@ -394,9 +391,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { err.span_label(span, "this is an associated function, not a method"); custom_span_label = true; } - if static_sources.len() == 1 { + if static_candidates.len() == 1 { let ty_str = - if let Some(CandidateSource::Impl(impl_did)) = static_sources.get(0) { + if let Some(CandidateSource::Impl(impl_did)) = static_candidates.get(0) { // When the "method" is resolved through dereferencing, we really want the // original type that has the associated function for accurate suggestions. // (#61411) @@ -422,9 +419,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { err.help(&format!("try with `{}::{}`", ty_str, item_name,)); } - report_candidates(span, &mut err, &mut static_sources, sugg_span); - } else if static_sources.len() > 1 { - report_candidates(span, &mut err, &mut static_sources, sugg_span); + report_candidates(span, &mut err, &mut static_candidates, sugg_span); + } else if static_candidates.len() > 1 { + report_candidates(span, &mut err, &mut static_candidates, sugg_span); } let mut bound_spans = vec![]; @@ -496,24 +493,18 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { if let (ty::Param(_), ty::PredicateKind::Trait(p)) = (self_ty.kind(), parent_pred.kind().skip_binder()) { + let hir = self.tcx.hir(); let node = match p.trait_ref.self_ty().kind() { ty::Param(_) => { // Account for `fn` items like in `issue-35677.rs` to // suggest restricting its type params. - let did = self.tcx.hir().body_owner_def_id(hir::BodyId { - hir_id: self.body_id, - }); - Some( - self.tcx - .hir() - .get(self.tcx.hir().local_def_id_to_hir_id(did)), - ) + let parent_body = + hir.body_owner(hir::BodyId { hir_id: self.body_id }); + Some(hir.get(parent_body)) + } + ty::Adt(def, _) => { + def.did().as_local().map(|def_id| hir.get_by_def_id(def_id)) } - ty::Adt(def, _) => def.did().as_local().map(|def_id| { - self.tcx - .hir() - .get(self.tcx.hir().local_def_id_to_hir_id(def_id)) - }), _ => None, }; if let Some(hir::Node::Item(hir::Item { kind, .. })) = node { @@ -605,7 +596,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { .iter() .filter_map(|(p, parent, c)| c.as_ref().map(|c| (p, parent, c))) .filter_map(|(p, parent, c)| match c.code() { - ObligationCauseCode::ImplDerivedObligation(ref data) => { + ObligationCauseCode::ImplDerivedObligation(data) => { Some((&data.derived, p, parent, data.impl_def_id, data)) } _ => None, @@ -620,22 +611,6 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { match self.tcx.hir().get_if_local(impl_def_id) { // Unmet obligation comes from a `derive` macro, point at it once to // avoid multiple span labels pointing at the same place. - Some(Node::Item(hir::Item { - kind: hir::ItemKind::Trait(..), - ident, - .. - })) if matches!( - ident.span.ctxt().outer_expn_data().kind, - ExpnKind::Macro(MacroKind::Derive, _) - ) => - { - let span = ident.span.ctxt().outer_expn_data().call_site; - let mut spans: MultiSpan = span.into(); - spans.push_span_label(span, derive_msg); - let entry = spanned_predicates.entry(spans); - entry.or_insert_with(|| (path, tr_self_ty, Vec::new())).2.push(p); - } - Some(Node::Item(hir::Item { kind: hir::ItemKind::Impl(hir::Impl { of_trait, self_ty, .. }), .. @@ -659,34 +634,6 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { entry.or_insert_with(|| (path, tr_self_ty, Vec::new())).2.push(p); } - // Unmet obligation coming from a `trait`. - Some(Node::Item(hir::Item { - kind: hir::ItemKind::Trait(..), - ident, - span: item_span, - .. - })) if !matches!( - ident.span.ctxt().outer_expn_data().kind, - ExpnKind::Macro(MacroKind::Derive, _) - ) => - { - if let Some(pred) = parent_p { - // Done to add the "doesn't satisfy" `span_label`. - let _ = format_pred(*pred); - } - skip_list.insert(p); - let mut spans = if cause.span != *item_span { - let mut spans: MultiSpan = cause.span.into(); - spans.push_span_label(cause.span, unsatisfied_msg); - spans - } else { - ident.span.into() - }; - spans.push_span_label(ident.span, "in this trait"); - let entry = spanned_predicates.entry(spans); - entry.or_insert_with(|| (path, tr_self_ty, Vec::new())).2.push(p); - } - // Unmet obligation coming from an `impl`. Some(Node::Item(hir::Item { kind: @@ -695,19 +642,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { }), span: item_span, .. - })) if !matches!( - self_ty.span.ctxt().outer_expn_data().kind, - ExpnKind::Macro(MacroKind::Derive, _) - ) && !matches!( - of_trait.as_ref().map(|t| t - .path - .span - .ctxt() - .outer_expn_data() - .kind), - Some(ExpnKind::Macro(MacroKind::Derive, _)) - ) => - { + })) => { let sized_pred = unsatisfied_predicates.iter().any(|(pred, _, _)| { match pred.kind().skip_binder() { @@ -759,7 +694,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { let entry = spanned_predicates.entry(spans); entry.or_insert_with(|| (path, tr_self_ty, Vec::new())).2.push(p); } - _ => {} + Some(_) => unreachable!(), + None => (), } } let mut spanned_predicates: Vec<_> = spanned_predicates.into_iter().collect(); @@ -863,7 +799,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { .on_unimplemented_note(trait_ref, &obligation); (message, label) }) - .unwrap_or((None, None)) + .unwrap() } else { (None, None) }; @@ -972,7 +908,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { // If the method name is the name of a field with a function or closure type, // give a helping note that it has to be called as `(x.f)(...)`. if let SelfSource::MethodCall(expr) = source { - if !self.suggest_field_call(span, rcvr_ty, expr, item_name, &mut err) + if !self.suggest_calling_field_as_fn(span, rcvr_ty, expr, item_name, &mut err) && lev_candidate.is_none() && !custom_span_label { @@ -982,10 +918,10 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { label_span_not_found(&mut err); } - // Don't suggest (for example) `expr.field.method()` if `expr.method()` - // doesn't exist due to unsatisfied predicates. + // Don't suggest (for example) `expr.field.clone()` if `expr.clone()` + // can't be called due to `typeof(expr): Clone` not holding. if unsatisfied_predicates.is_empty() { - self.check_for_field_method(&mut err, source, span, actual, item_name); + self.suggest_calling_method_on_field(&mut err, source, span, actual, item_name); } self.check_for_inner_self(&mut err, source, span, actual, item_name); @@ -1007,7 +943,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { source, out_of_scope_traits, &unsatisfied_predicates, - &static_sources, + &static_candidates, unsatisfied_bounds, ); } @@ -1146,7 +1082,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { None } - fn suggest_field_call( + /// Suggest calling a field with a type that implements the `Fn*` traits instead of a method with + /// the same name as the field i.e. `(a.my_fn_ptr)(10)` instead of `a.my_fn_ptr(10)`. + fn suggest_calling_field_as_fn( &self, span: Span, rcvr_ty: Ty<'tcx>, @@ -1408,7 +1346,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { false } - fn check_for_field_method( + /// Suggest calling a method on a field i.e. `a.field.bar()` instead of `a.bar()` + fn suggest_calling_method_on_field( &self, err: &mut Diagnostic, source: SelfSource<'tcx>, @@ -2021,7 +1960,10 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { ) { let mut alt_rcvr_sugg = false; if let (SelfSource::MethodCall(rcvr), false) = (source, unsatisfied_bounds) { - debug!(?span, ?item_name, ?rcvr_ty, ?rcvr); + debug!( + "suggest_traits_to_import: span={:?}, item_name={:?}, rcvr_ty={:?}, rcvr={:?}", + span, item_name, rcvr_ty, rcvr + ); let skippable = [ self.tcx.lang_items().clone_trait(), self.tcx.lang_items().deref_trait(), @@ -2060,7 +2002,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { // suggestions are generally misleading (see #94218). break; } - _ => {} + Err(_) => (), } for (rcvr_ty, pre) in &[ diff --git a/compiler/rustc_middle/src/traits/mod.rs b/compiler/rustc_middle/src/traits/mod.rs index e73d44bbb36c3..07ee758b32c1f 100644 --- a/compiler/rustc_middle/src/traits/mod.rs +++ b/compiler/rustc_middle/src/traits/mod.rs @@ -203,13 +203,20 @@ pub struct UnifyReceiverContext<'tcx> { pub substs: SubstsRef<'tcx>, } -#[derive(Clone, Debug, PartialEq, Eq, Hash, Lift, Default)] +#[derive(Clone, PartialEq, Eq, Hash, Lift, Default)] pub struct InternedObligationCauseCode<'tcx> { /// `None` for `ObligationCauseCode::MiscObligation` (a common case, occurs ~60% of /// the time). `Some` otherwise. code: Option>>, } +impl<'tcx> std::fmt::Debug for InternedObligationCauseCode<'tcx> { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + let cause: &ObligationCauseCode<'_> = self; + cause.fmt(f) + } +} + impl<'tcx> ObligationCauseCode<'tcx> { #[inline(always)] fn into(self) -> InternedObligationCauseCode<'tcx> { diff --git a/compiler/rustc_middle/src/ty/sty.rs b/compiler/rustc_middle/src/ty/sty.rs index cf420bafeb12f..5f108bf0ef306 100644 --- a/compiler/rustc_middle/src/ty/sty.rs +++ b/compiler/rustc_middle/src/ty/sty.rs @@ -20,6 +20,7 @@ use rustc_hir::def_id::DefId; use rustc_index::vec::Idx; use rustc_macros::HashStable; use rustc_span::symbol::{kw, sym, Symbol}; +use rustc_span::Span; use rustc_target::abi::VariantIdx; use rustc_target::spec::abi; use std::borrow::Cow; @@ -1282,6 +1283,12 @@ impl<'tcx> ParamTy { pub fn to_ty(self, tcx: TyCtxt<'tcx>) -> Ty<'tcx> { tcx.mk_ty_param(self.index, self.name) } + + pub fn span_from_generics(&self, tcx: TyCtxt<'tcx>, item_with_generics: DefId) -> Span { + let generics = tcx.generics_of(item_with_generics); + let type_param = generics.type_param(self, tcx); + tcx.def_span(type_param.def_id) + } } #[derive(Copy, Clone, Hash, TyEncodable, TyDecodable, Eq, PartialEq, Ord, PartialOrd)] diff --git a/compiler/rustc_trait_selection/src/traits/on_unimplemented.rs b/compiler/rustc_trait_selection/src/traits/on_unimplemented.rs index 4a4f34b768059..fb062ea71c4ce 100644 --- a/compiler/rustc_trait_selection/src/traits/on_unimplemented.rs +++ b/compiler/rustc_trait_selection/src/traits/on_unimplemented.rs @@ -27,6 +27,7 @@ pub struct OnUnimplementedDirective { } #[derive(Default)] +/// For the `#[rustc_on_unimplemented]` attribute pub struct OnUnimplementedNote { pub message: Option, pub label: Option, From a777c46dff4fdb54e29a19e273d8677d485232e6 Mon Sep 17 00:00:00 2001 From: Charles Lew Date: Fri, 4 Nov 2022 03:02:09 +0800 Subject: [PATCH 06/14] Use `derive(Subdiagnostic)` for `ChangeFieldsToBeOfUnitType`. --- .../locales/en-US/passes.ftl | 3 +-- compiler/rustc_passes/src/errors.rs | 26 +++---------------- 2 files changed, 4 insertions(+), 25 deletions(-) diff --git a/compiler/rustc_error_messages/locales/en-US/passes.ftl b/compiler/rustc_error_messages/locales/en-US/passes.ftl index a5b002fa3579e..a7cb9f14e8247 100644 --- a/compiler/rustc_error_messages/locales/en-US/passes.ftl +++ b/compiler/rustc_error_messages/locales/en-US/passes.ftl @@ -685,8 +685,7 @@ passes_change_fields_to_be_of_unit_type = consider changing the { $num -> [one] field *[other] fields - } to be of unit type to suppress this warning - while preserving the field numbering, or remove the { $num -> + } to be of unit type to suppress this warning while preserving the field numbering, or remove the { $num -> [one] field *[other] fields } diff --git a/compiler/rustc_passes/src/errors.rs b/compiler/rustc_passes/src/errors.rs index d39d7629b287f..83a51bcd097a8 100644 --- a/compiler/rustc_passes/src/errors.rs +++ b/compiler/rustc_passes/src/errors.rs @@ -12,8 +12,6 @@ use rustc_macros::{Diagnostic, LintDiagnostic, Subdiagnostic}; use rustc_middle::ty::{MainDefinition, Ty}; use rustc_span::{Span, Symbol, DUMMY_SP}; -use rustc_errors::{pluralize, AddToDiagnostic, Diagnostic, SubdiagnosticMessage}; - use crate::lang_items::Duplicate; #[derive(LintDiagnostic)] @@ -1502,28 +1500,10 @@ pub struct IgnoredDerivedImpls { pub trait_list_len: usize, } +#[derive(Subdiagnostic)] +#[multipart_suggestion(passes_change_fields_to_be_of_unit_type, applicability = "has-placeholders")] pub struct ChangeFieldsToBeOfUnitType { pub num: usize, + #[suggestion_part(code = "()")] pub spans: Vec, } - -// FIXME: Replace this impl with a derive. -impl AddToDiagnostic for ChangeFieldsToBeOfUnitType { - fn add_to_diagnostic_with(self, diag: &mut Diagnostic, _: F) - where - F: Fn(&mut Diagnostic, SubdiagnosticMessage) -> SubdiagnosticMessage, - { - diag.multipart_suggestion( - &format!( - "consider changing the field{s} to be of unit type to \ - suppress this warning while preserving the field \ - numbering, or remove the field{s}", - s = pluralize!(self.num) - ), - self.spans.iter().map(|sp| (*sp, "()".to_string())).collect(), - // "HasPlaceholders" because applying this fix by itself isn't - // enough: All constructor calls have to be adjusted as well - Applicability::HasPlaceholders, - ); - } -} From 4c80f50fc6dce4e35eebe1beaadd5fc3ecfe6f52 Mon Sep 17 00:00:00 2001 From: Jhonny Bill Mena Date: Sun, 30 Oct 2022 15:38:37 -0400 Subject: [PATCH 07/14] UPDATE - Complete link.rs migration to new diagnostics infraestructure --- compiler/rustc_codegen_ssa/src/back/link.rs | 123 ++++++----------- compiler/rustc_codegen_ssa/src/errors.rs | 130 ++++++++++++++++++ .../locales/en-US/codegen_ssa.ftl | 51 +++++++ compiler/rustc_errors/src/diagnostic_impls.rs | 4 +- 4 files changed, 227 insertions(+), 81 deletions(-) diff --git a/compiler/rustc_codegen_ssa/src/back/link.rs b/compiler/rustc_codegen_ssa/src/back/link.rs index 0dc0dee862c74..4c58d0b53f08d 100644 --- a/compiler/rustc_codegen_ssa/src/back/link.rs +++ b/compiler/rustc_codegen_ssa/src/back/link.rs @@ -919,29 +919,17 @@ fn link_natively<'a>( ) .is_some(); - sess.note_without_error("`link.exe` returned an unexpected error"); + sess.emit_note(errors::LinkExeUnexpectedError); if is_vs_installed && has_linker { // the linker is broken - sess.note_without_error( - "the Visual Studio build tools may need to be repaired \ - using the Visual Studio installer", - ); - sess.note_without_error( - "or a necessary component may be missing from the \ - \"C++ build tools\" workload", - ); + sess.emit_note(errors::RepairVSBuildTools); + sess.emit_note(errors::MissingCppBuildToolComponent); } else if is_vs_installed { // the linker is not installed - sess.note_without_error( - "in the Visual Studio installer, ensure the \ - \"C++ build tools\" workload is selected", - ); + sess.emit_note(errors::SelectCppBuildToolWorkload); } else { // visual studio is not installed - sess.note_without_error( - "you may need to install Visual Studio build tools with the \ - \"C++ build tools\" workload", - ); + sess.emit_note(errors::VisualStudioNotInstalled); } } } @@ -954,35 +942,20 @@ fn link_natively<'a>( Err(e) => { let linker_not_found = e.kind() == io::ErrorKind::NotFound; - let mut linker_error = { - if linker_not_found { - sess.struct_err(&format!("linker `{}` not found", linker_path.display())) - } else { - sess.struct_err(&format!( - "could not exec the linker `{}`", - linker_path.display() - )) - } - }; - - linker_error.note(&e.to_string()); - - if !linker_not_found { - linker_error.note(&format!("{:?}", &cmd)); + if linker_not_found { + sess.emit_err(errors::LinkerNotFound { linker_path, error: e }); + } else { + sess.emit_err(errors::UnableToExeLinker { + linker_path, + error: e, + command_formatted: format!("{:?}", &cmd), + }); } - linker_error.emit(); - if sess.target.is_like_msvc && linker_not_found { - sess.note_without_error( - "the msvc targets depend on the msvc linker \ - but `link.exe` was not found", - ); - sess.note_without_error( - "please ensure that Visual Studio 2017 or later, or Build Tools \ - for Visual Studio were installed with the Visual C++ option.", - ); - sess.note_without_error("VS Code is a different product, and is not sufficient."); + sess.emit_note(errors::MsvcMissingLinker); + sess.emit_note(errors::CheckInstalledVisualStudio); + sess.emit_note(errors::UnsufficientVSCodeProduct); } sess.abort_if_errors(); } @@ -1007,15 +980,13 @@ fn link_natively<'a>( if !prog.status.success() { let mut output = prog.stderr.clone(); output.extend_from_slice(&prog.stdout); - sess.struct_warn(&format!( - "processing debug info with `dsymutil` failed: {}", - prog.status - )) - .note(&escape_string(&output)) - .emit(); + sess.emit_warning(errors::ProcessingDymutilFailed { + status: prog.status, + output: escape_string(&output), + }); } } - Err(e) => sess.fatal(&format!("unable to run `dsymutil`: {}", e)), + Err(error) => sess.emit_fatal(errors::UnableToRunDsymutil { error }), } } @@ -1092,21 +1063,21 @@ fn strip_symbols_with_external_utility<'a>( if !prog.status.success() { let mut output = prog.stderr.clone(); output.extend_from_slice(&prog.stdout); - sess.struct_warn(&format!( - "stripping debug info with `{}` failed: {}", - util, prog.status - )) - .note(&escape_string(&output)) - .emit(); + sess.emit_warning(errors::StrippingDebuInfoFailed { + util, + status: prog.status, + output: escape_string(&output), + }); } } - Err(e) => sess.fatal(&format!("unable to run `{}`: {}", util, e)), + Err(error) => sess.emit_fatal(errors::UnableToRun { util, error }), } } fn escape_string(s: &[u8]) -> String { match str::from_utf8(s) { Ok(s) => s.to_owned(), + // FIXME: return a type that can conform to IntoDiagnosticArg Err(_) => format!("Non-UTF-8 output: {}", s.escape_ascii()), } } @@ -1251,7 +1222,7 @@ pub fn linker_and_flavor(sess: &Session) -> (PathBuf, LinkerFlavor) { )), (Some(linker), None) => { let stem = linker.file_stem().and_then(|stem| stem.to_str()).unwrap_or_else(|| { - sess.fatal("couldn't extract file stem from specified linker") + sess.emit_fatal(errors::LinkerFileStem); }); let flavor = if stem == "emcc" { @@ -1378,13 +1349,9 @@ fn print_native_static_libs(sess: &Session, all_native_libs: &[NativeLib]) { }) .collect(); if !lib_args.is_empty() { - sess.note_without_error( - "Link against the following native artifacts when linking \ - against this static library. The order and any duplication \ - can be significant on some platforms.", - ); + sess.emit_note(errors::StaticLibraryNativeArtifacts); // Prefix for greppability - sess.note_without_error(&format!("native-static-libs: {}", &lib_args.join(" "))); + sess.emit_note(errors::NativeStaticLibs { arguments: lib_args.join(" ") }); } } @@ -1688,14 +1655,14 @@ fn add_link_script(cmd: &mut dyn Linker, sess: &Session, tmpdir: &Path, crate_ty match (crate_type, &sess.target.link_script) { (CrateType::Cdylib | CrateType::Executable, Some(script)) => { if !sess.target.linker_flavor.is_gnu() { - sess.fatal("can only use link script when linking with GNU-like linker"); + sess.emit_fatal(errors::LinkScriptUnavailable); } let file_name = ["rustc", &sess.target.llvm_target, "linkfile.ld"].join("-"); let path = tmpdir.join(file_name); - if let Err(e) = fs::write(&path, script.as_ref()) { - sess.fatal(&format!("failed to write link script to {}: {}", path.display(), e)); + if let Err(error) = fs::write(&path, script.as_ref()) { + sess.emit_fatal(errors::LinkScriptWriteFailure { path, error }); } cmd.arg("--script"); @@ -1841,8 +1808,8 @@ fn add_linked_symbol_object( let path = tmpdir.join("symbols.o"); let result = std::fs::write(&path, file.write().unwrap()); - if let Err(e) = result { - sess.fatal(&format!("failed to write {}: {}", path.display(), e)); + if let Err(error) = result { + sess.emit_fatal(errors::FailedToWrite { path, error }); } cmd.add_object(&path); } @@ -2299,14 +2266,10 @@ fn collect_natvis_visualizers( visualizer_paths.push(visualizer_out_file); } Err(error) => { - sess.warn( - format!( - "Unable to write debugger visualizer file `{}`: {} ", - visualizer_out_file.display(), - error - ) - .as_str(), - ); + sess.emit_warning(errors::UnableToWriteDebuggerVisualizer { + path: visualizer_out_file, + error, + }); } }; } @@ -2641,7 +2604,7 @@ fn add_upstream_rust_crates<'a>( || !codegen_results.crate_info.is_no_builtins.contains(&cnum); let mut archive = archive_builder_builder.new_archive_builder(sess); - if let Err(e) = archive.add_archive( + if let Err(error) = archive.add_archive( cratepath, Box::new(move |f| { if f == METADATA_FILENAME { @@ -2681,7 +2644,7 @@ fn add_upstream_rust_crates<'a>( false }), ) { - sess.fatal(&format!("failed to build archive from rlib: {}", e)); + sess.emit_fatal(errors::RlibArchiveBuildFailure { error }); } if archive.build(&dst) { link_upstream(&dst); @@ -2919,7 +2882,7 @@ fn add_gcc_ld_path(cmd: &mut dyn Linker, sess: &Session, flavor: LinkerFlavor) { } } } else { - sess.fatal("option `-Z gcc-ld` is used even though linker flavor is not gcc"); + sess.emit_fatal(errors::OptionGccOnly); } } } diff --git a/compiler/rustc_codegen_ssa/src/errors.rs b/compiler/rustc_codegen_ssa/src/errors.rs index ebb531f1c43a5..71fac123725cf 100644 --- a/compiler/rustc_codegen_ssa/src/errors.rs +++ b/compiler/rustc_codegen_ssa/src/errors.rs @@ -354,3 +354,133 @@ impl IntoDiagnostic<'_> for LinkingFailed<'_> { diag } } + +#[derive(Diagnostic)] +#[diag(codegen_ssa_link_exe_unexpected_error)] +pub struct LinkExeUnexpectedError; + +#[derive(Diagnostic)] +#[diag(codegen_ssa_repair_vs_build_tools)] +pub struct RepairVSBuildTools; + +#[derive(Diagnostic)] +#[diag(codegen_ssa_missing_cpp_build_tool_component)] +pub struct MissingCppBuildToolComponent; + +#[derive(Diagnostic)] +#[diag(codegen_ssa_select_cpp_build_tool_workload)] +pub struct SelectCppBuildToolWorkload; + +#[derive(Diagnostic)] +#[diag(codegen_ssa_visual_studio_not_installed)] +pub struct VisualStudioNotInstalled; + +#[derive(Diagnostic)] +#[diag(codegen_ssa_linker_not_found)] +#[note] +pub struct LinkerNotFound { + pub linker_path: PathBuf, + pub error: Error, +} + +#[derive(Diagnostic)] +#[diag(codegen_ssa_unable_to_exe_linker)] +#[note] +#[note(command_note)] +pub struct UnableToExeLinker { + pub linker_path: PathBuf, + pub error: Error, + pub command_formatted: String, +} + +#[derive(Diagnostic)] +#[diag(codegen_ssa_msvc_missing_linker)] +pub struct MsvcMissingLinker; + +#[derive(Diagnostic)] +#[diag(codegen_ssa_check_installed_visual_studio)] +pub struct CheckInstalledVisualStudio; + +#[derive(Diagnostic)] +#[diag(codegen_ssa_unsufficient_vs_code_product)] +pub struct UnsufficientVSCodeProduct; + +#[derive(Diagnostic)] +#[diag(codegen_ssa_processing_dymutil_failed)] +#[note] +pub struct ProcessingDymutilFailed { + pub status: ExitStatus, + pub output: String, +} + +#[derive(Diagnostic)] +#[diag(codegen_ssa_unable_to_run_dsymutil)] +#[note] +pub struct UnableToRunDsymutil { + pub error: Error, +} + +#[derive(Diagnostic)] +#[diag(codegen_ssa_stripping_debu_info_failed)] +#[note] +pub struct StrippingDebuInfoFailed<'a> { + pub util: &'a str, + pub status: ExitStatus, + pub output: String, +} + +#[derive(Diagnostic)] +#[diag(codegen_ssa_unable_to_run)] +pub struct UnableToRun<'a> { + pub util: &'a str, + pub error: Error, +} + +#[derive(Diagnostic)] +#[diag(codegen_ssa_linker_file_stem)] +pub struct LinkerFileStem; + +#[derive(Diagnostic)] +#[diag(codegen_ssa_static_library_native_artifacts)] +pub struct StaticLibraryNativeArtifacts; + +#[derive(Diagnostic)] +#[diag(codegen_ssa_native_static_libs)] +pub struct NativeStaticLibs { + pub arguments: String, +} + +#[derive(Diagnostic)] +#[diag(codegen_ssa_link_script_unavailable)] +pub struct LinkScriptUnavailable; + +#[derive(Diagnostic)] +#[diag(codegen_ssa_link_script_write_failure)] +pub struct LinkScriptWriteFailure { + pub path: PathBuf, + pub error: Error, +} + +#[derive(Diagnostic)] +#[diag(codegen_ssa_failed_to_write)] +pub struct FailedToWrite { + pub path: PathBuf, + pub error: Error, +} + +#[derive(Diagnostic)] +#[diag(codegen_ssa_unable_to_write_debugger_visualizer)] +pub struct UnableToWriteDebuggerVisualizer { + pub path: PathBuf, + pub error: Error, +} + +#[derive(Diagnostic)] +#[diag(codegen_ssa_rlib_archive_build_failure)] +pub struct RlibArchiveBuildFailure { + pub error: Error, +} + +#[derive(Diagnostic)] +#[diag(codegen_ssa_option_gcc_only)] +pub struct OptionGccOnly; diff --git a/compiler/rustc_error_messages/locales/en-US/codegen_ssa.ftl b/compiler/rustc_error_messages/locales/en-US/codegen_ssa.ftl index 966a421bcf08c..2e5c72ee6452a 100644 --- a/compiler/rustc_error_messages/locales/en-US/codegen_ssa.ftl +++ b/compiler/rustc_error_messages/locales/en-US/codegen_ssa.ftl @@ -119,3 +119,54 @@ codegen_ssa_thorin_object_read = {$error} codegen_ssa_thorin_object_write = {$error} codegen_ssa_thorin_gimli_read = {$error} codegen_ssa_thorin_gimli_write = {$error} + +codegen_ssa_link_exe_unexpected_error = `link.exe` returned an unexpected error + +codegen_ssa_repair_vs_build_tools = the Visual Studio build tools may need to be repaired using the Visual Studio installer + +codegen_ssa_missing_cpp_build_tool_component = or a necessary component may be missing from the "C++ build tools" workload + +codegen_ssa_select_cpp_build_tool_workload = in the Visual Studio installer, ensure the "C++ build tools" workload is selected + +codegen_ssa_visual_studio_not_installed = you may need to install Visual Studio build tools with the "C++ build tools" workload + +codegen_ssa_linker_not_found = linker `{$linker_path}` not found + .note = {$error} + +codegen_ssa_unable_to_exe_linker = could not exec the linker `{$linker_path}` + .note = {$error} + .command_note = {$command_formatted} + +codegen_ssa_msvc_missing_linker = the msvc targets depend on the msvc linker but `link.exe` was not found + +codegen_ssa_check_installed_visual_studio = please ensure that Visual Studio 2017 or later, or Build Tools for Visual Studio were installed with the Visual C++ option. + +codegen_ssa_unsufficient_vs_code_product = VS Code is a different product, and is not sufficient. + +codegen_ssa_processing_dymutil_failed = processing debug info with `dsymutil` failed: {$status} + .note = {$output} + +codegen_ssa_unable_to_run_dsymutil = unable to run `dsymutil`: {$error} + +codegen_ssa_stripping_debu_info_failed = stripping debug info with `{$util}` failed: {$status} + .note = {$output} + +codegen_ssa_unable_to_run = unable to run `{$util}`: {$error} + +codegen_ssa_linker_file_stem = couldn't extract file stem from specified linker + +codegen_ssa_static_library_native_artifacts = Link against the following native artifacts when linking against this static library. The order and any duplication can be significant on some platforms. + +codegen_ssa_native_static_libs = native-static-libs: {$arguments} + +codegen_ssa_link_script_unavailable = can only use link script when linking with GNU-like linker + +codegen_ssa_link_script_write_failure = failed to write link script to {$path}: {$error} + +codegen_ssa_failed_to_write = failed to write {$path}: {$error} + +codegen_ssa_unable_to_write_debugger_visualizer = Unable to write debugger visualizer file `{$path}`: {$error} + +codegen_ssa_rlib_archive_build_failure = failed to build archive from rlib: {$error} + +codegen_ssa_option_gcc_only = option `-Z gcc-ld` is used even though linker flavor is not gcc diff --git a/compiler/rustc_errors/src/diagnostic_impls.rs b/compiler/rustc_errors/src/diagnostic_impls.rs index 7640b2919f78b..4f32e236b2dbb 100644 --- a/compiler/rustc_errors/src/diagnostic_impls.rs +++ b/compiler/rustc_errors/src/diagnostic_impls.rs @@ -13,6 +13,7 @@ use std::borrow::Cow; use std::fmt; use std::num::ParseIntError; use std::path::{Path, PathBuf}; +use std::process::ExitStatus; pub struct DiagnosticArgFromDisplay<'a>(pub &'a dyn fmt::Display); @@ -66,7 +67,8 @@ into_diagnostic_arg_using_display!( ParseIntError, StackProtector, &TargetTriple, - SplitDebuginfo + SplitDebuginfo, + ExitStatus, ); impl IntoDiagnosticArg for bool { From 1f4c5a624fd7db58b32d6d9cfa063fbfe0a340a7 Mon Sep 17 00:00:00 2001 From: Jhonny Bill Mena Date: Mon, 31 Oct 2022 01:36:32 -0400 Subject: [PATCH 08/14] ADD - ExtractBundledLibsError. Migrated extract_bundled_libs to translatable diagnostics --- .../rustc_codegen_ssa/src/back/archive.rs | 59 +++++++++++++------ compiler/rustc_codegen_ssa/src/back/link.rs | 2 +- compiler/rustc_codegen_ssa/src/errors.rs | 48 +++++++++++++++ .../locales/en-US/codegen_ssa.ftl | 8 +++ 4 files changed, 98 insertions(+), 19 deletions(-) diff --git a/compiler/rustc_codegen_ssa/src/back/archive.rs b/compiler/rustc_codegen_ssa/src/back/archive.rs index bb76ca5d2b941..b45fa2476875b 100644 --- a/compiler/rustc_codegen_ssa/src/back/archive.rs +++ b/compiler/rustc_codegen_ssa/src/back/archive.rs @@ -6,11 +6,12 @@ use rustc_span::symbol::Symbol; use object::read::archive::ArchiveFile; -use std::fmt::Display; use std::fs::File; use std::io; use std::path::{Path, PathBuf}; +use crate::errors::{ExtractBundledLibsError, ExtractBundledLibsErrorKind::*}; + pub trait ArchiveBuilderBuilder { fn new_archive_builder<'a>(&self, sess: &'a Session) -> Box + 'a>; @@ -28,32 +29,54 @@ pub trait ArchiveBuilderBuilder { is_direct_dependency: bool, ) -> PathBuf; - fn extract_bundled_libs( - &self, - rlib: &Path, + fn extract_bundled_libs<'a>( + &'a self, + rlib: &'a Path, outdir: &Path, bundled_lib_file_names: &FxHashSet, - ) -> Result<(), String> { - let message = |msg: &str, e: &dyn Display| format!("{} '{}': {}", msg, &rlib.display(), e); + ) -> Result<(), ExtractBundledLibsError<'_>> { let archive_map = unsafe { - Mmap::map(File::open(rlib).map_err(|e| message("failed to open file", &e))?) - .map_err(|e| message("failed to mmap file", &e))? + Mmap::map(File::open(rlib).map_err(|e| ExtractBundledLibsError { + kind: OpenFile, + rlib, + error: e.to_string(), + })?) + .map_err(|e| ExtractBundledLibsError { + kind: MmapFile, + rlib, + error: e.to_string(), + })? }; - let archive = ArchiveFile::parse(&*archive_map) - .map_err(|e| message("failed to parse archive", &e))?; + let archive = ArchiveFile::parse(&*archive_map).map_err(|e| ExtractBundledLibsError { + kind: ParseArchive, + rlib, + error: e.to_string(), + })?; for entry in archive.members() { - let entry = entry.map_err(|e| message("failed to read entry", &e))?; - let data = entry - .data(&*archive_map) - .map_err(|e| message("failed to get data from archive member", &e))?; - let name = std::str::from_utf8(entry.name()) - .map_err(|e| message("failed to convert name", &e))?; + let entry = entry.map_err(|e| ExtractBundledLibsError { + kind: ReadEntry, + rlib, + error: e.to_string(), + })?; + let data = entry.data(&*archive_map).map_err(|e| ExtractBundledLibsError { + kind: ArchiveMember, + rlib, + error: e.to_string(), + })?; + let name = std::str::from_utf8(entry.name()).map_err(|e| ExtractBundledLibsError { + kind: ConvertName, + rlib, + error: e.to_string(), + })?; if !bundled_lib_file_names.contains(&Symbol::intern(name)) { continue; // We need to extract only native libraries. } - std::fs::write(&outdir.join(&name), data) - .map_err(|e| message("failed to write file", &e))?; + std::fs::write(&outdir.join(&name), data).map_err(|e| ExtractBundledLibsError { + kind: WriteFile, + rlib, + error: e.to_string(), + })?; } Ok(()) } diff --git a/compiler/rustc_codegen_ssa/src/back/link.rs b/compiler/rustc_codegen_ssa/src/back/link.rs index 4c58d0b53f08d..1277294b63419 100644 --- a/compiler/rustc_codegen_ssa/src/back/link.rs +++ b/compiler/rustc_codegen_ssa/src/back/link.rs @@ -2447,7 +2447,7 @@ fn add_upstream_rust_crates<'a>( let rlib = &src.rlib.as_ref().unwrap().0; archive_builder_builder .extract_bundled_libs(rlib, tmpdir, &bundled_libs) - .unwrap_or_else(|e| sess.fatal(e)); + .unwrap_or_else(|e| sess.emit_fatal(e)); } let mut last = (None, NativeLibKind::Unspecified, None); diff --git a/compiler/rustc_codegen_ssa/src/errors.rs b/compiler/rustc_codegen_ssa/src/errors.rs index 71fac123725cf..0a2532ccc470e 100644 --- a/compiler/rustc_codegen_ssa/src/errors.rs +++ b/compiler/rustc_codegen_ssa/src/errors.rs @@ -484,3 +484,51 @@ pub struct RlibArchiveBuildFailure { #[derive(Diagnostic)] #[diag(codegen_ssa_option_gcc_only)] pub struct OptionGccOnly; + +pub struct ExtractBundledLibsError<'a> { + pub kind: ExtractBundledLibsErrorKind, + pub rlib: &'a Path, + pub error: String, +} + +pub enum ExtractBundledLibsErrorKind { + OpenFile, + MmapFile, + ParseArchive, + ReadEntry, + ArchiveMember, + ConvertName, + WriteFile, +} + +impl IntoDiagnostic<'_, !> for ExtractBundledLibsError<'_> { + fn into_diagnostic(self, handler: &'_ Handler) -> DiagnosticBuilder<'_, !> { + let mut diag = match self.kind { + ExtractBundledLibsErrorKind::OpenFile => { + handler.struct_fatal(fluent::codegen_ssa_extract_bundled_libs_open_file) + } + ExtractBundledLibsErrorKind::MmapFile => { + handler.struct_fatal(fluent::codegen_ssa_extract_bundled_libs_mmap_file) + } + ExtractBundledLibsErrorKind::ParseArchive => { + handler.struct_fatal(fluent::codegen_ssa_extract_bundled_libs_parse_archive) + } + ExtractBundledLibsErrorKind::ReadEntry => { + handler.struct_fatal(fluent::codegen_ssa_extract_bundled_libs_read_entry) + } + ExtractBundledLibsErrorKind::ArchiveMember => { + handler.struct_fatal(fluent::codegen_ssa_extract_bundled_libs_archive_member) + } + ExtractBundledLibsErrorKind::ConvertName => { + handler.struct_fatal(fluent::codegen_ssa_extract_bundled_libs_convert_name) + } + ExtractBundledLibsErrorKind::WriteFile => { + handler.struct_fatal(fluent::codegen_ssa_extract_bundled_libs_write_file) + } + }; + + diag.set_arg("rlib", self.rlib); + diag.set_arg("error", self.error); + diag + } +} diff --git a/compiler/rustc_error_messages/locales/en-US/codegen_ssa.ftl b/compiler/rustc_error_messages/locales/en-US/codegen_ssa.ftl index 2e5c72ee6452a..a31e1658f5f00 100644 --- a/compiler/rustc_error_messages/locales/en-US/codegen_ssa.ftl +++ b/compiler/rustc_error_messages/locales/en-US/codegen_ssa.ftl @@ -170,3 +170,11 @@ codegen_ssa_unable_to_write_debugger_visualizer = Unable to write debugger visua codegen_ssa_rlib_archive_build_failure = failed to build archive from rlib: {$error} codegen_ssa_option_gcc_only = option `-Z gcc-ld` is used even though linker flavor is not gcc + +codegen_ssa_extract_bundled_libs_open_file = failed to open file '{$rlib}': {$error} +codegen_ssa_extract_bundled_libs_mmap_file = failed to mmap file '{$rlib}': {$error} +codegen_ssa_extract_bundled_libs_parse_archive = failed to parse archive '{$rlib}': {$error} +codegen_ssa_extract_bundled_libs_read_entry = failed to read entry '{$rlib}': {$error} +codegen_ssa_extract_bundled_libs_archive_member = failed to get data from archive member '{$rlib}': {$error} +codegen_ssa_extract_bundled_libs_convert_name = failed to convert name '{$rlib}': {$error} +codegen_ssa_extract_bundled_libs_write_file = failed to write file '{$rlib}': {$error} From 2678765d08df633b4804d1ba03e090c3bed878bb Mon Sep 17 00:00:00 2001 From: Jhonny Bill Mena Date: Mon, 31 Oct 2022 01:51:58 -0400 Subject: [PATCH 09/14] FIX - Migrate missing errors in link.rs --- compiler/rustc_codegen_ssa/src/back/link.rs | 8 ++++---- compiler/rustc_codegen_ssa/src/errors.rs | 13 +++++++++++++ .../locales/en-US/codegen_ssa.ftl | 4 ++++ 3 files changed, 21 insertions(+), 4 deletions(-) diff --git a/compiler/rustc_codegen_ssa/src/back/link.rs b/compiler/rustc_codegen_ssa/src/back/link.rs index 1277294b63419..1e6a2b6ecaa6d 100644 --- a/compiler/rustc_codegen_ssa/src/back/link.rs +++ b/compiler/rustc_codegen_ssa/src/back/link.rs @@ -2776,14 +2776,14 @@ fn add_apple_sdk(cmd: &mut dyn Linker, sess: &Session, flavor: LinkerFlavor) { ("arm", "watchos") => "watchos", (_, "macos") => "macosx", _ => { - sess.err(&format!("unsupported arch `{}` for os `{}`", arch, os)); + sess.emit_err(errors::UnsupportedArch { arch, os }); return; } }; let sdk_root = match get_apple_sdk_root(sdk_name) { Ok(s) => s, Err(e) => { - sess.err(&e); + sess.emit_err(e); return; } }; @@ -2799,7 +2799,7 @@ fn add_apple_sdk(cmd: &mut dyn Linker, sess: &Session, flavor: LinkerFlavor) { } } -fn get_apple_sdk_root(sdk_name: &str) -> Result { +fn get_apple_sdk_root(sdk_name: &str) -> Result> { // Following what clang does // (https://github.com/llvm/llvm-project/blob/ // 296a80102a9b72c3eda80558fb78a3ed8849b341/clang/lib/Driver/ToolChains/Darwin.cpp#L1661-L1678) @@ -2849,7 +2849,7 @@ fn get_apple_sdk_root(sdk_name: &str) -> Result { match res { Ok(output) => Ok(output.trim().to_string()), - Err(e) => Err(format!("failed to get {} SDK path: {}", sdk_name, e)), + Err(error) => Err(errors::AppleSdkRootError::SdkPath { sdk_name, error }), } } diff --git a/compiler/rustc_codegen_ssa/src/errors.rs b/compiler/rustc_codegen_ssa/src/errors.rs index 0a2532ccc470e..35eae30c4ba9d 100644 --- a/compiler/rustc_codegen_ssa/src/errors.rs +++ b/compiler/rustc_codegen_ssa/src/errors.rs @@ -532,3 +532,16 @@ impl IntoDiagnostic<'_, !> for ExtractBundledLibsError<'_> { diag } } + +#[derive(Diagnostic)] +#[diag(codegen_ssa_unsupported_arch)] +pub struct UnsupportedArch<'a> { + pub arch: &'a str, + pub os: &'a str, +} + +#[derive(Diagnostic)] +pub enum AppleSdkRootError<'a> { + #[diag(codegen_ssa_apple_sdk_error_sdk_path)] + SdkPath { sdk_name: &'a str, error: Error }, +} diff --git a/compiler/rustc_error_messages/locales/en-US/codegen_ssa.ftl b/compiler/rustc_error_messages/locales/en-US/codegen_ssa.ftl index a31e1658f5f00..ad0d758210175 100644 --- a/compiler/rustc_error_messages/locales/en-US/codegen_ssa.ftl +++ b/compiler/rustc_error_messages/locales/en-US/codegen_ssa.ftl @@ -178,3 +178,7 @@ codegen_ssa_extract_bundled_libs_read_entry = failed to read entry '{$rlib}': {$ codegen_ssa_extract_bundled_libs_archive_member = failed to get data from archive member '{$rlib}': {$error} codegen_ssa_extract_bundled_libs_convert_name = failed to convert name '{$rlib}': {$error} codegen_ssa_extract_bundled_libs_write_file = failed to write file '{$rlib}': {$error} + +codegen_ssa_unsupported_arch = unsupported arch `{$arch}` for os `{$os}` + +codegen_ssa_apple_sdk_error_sdk_path = failed to get {$sdk_name} SDK path: {error} From 28491a7b36a717e42081fc6ee788433feccb72e6 Mon Sep 17 00:00:00 2001 From: Jhonny Bill Mena Date: Thu, 3 Nov 2022 01:53:06 -0400 Subject: [PATCH 10/14] UPDATE - address PR Comments FIX - StrippingDebugInfoFailed typo DELETE - unneeded FIXME comment UPDATE - only declare the error with ExtractBundledLibsError as an enum and use the Diagnostic derive macro --- .../rustc_codegen_ssa/src/back/archive.rs | 56 ++++++---------- compiler/rustc_codegen_ssa/src/back/link.rs | 3 +- compiler/rustc_codegen_ssa/src/errors.rs | 64 ++++++------------- 3 files changed, 40 insertions(+), 83 deletions(-) diff --git a/compiler/rustc_codegen_ssa/src/back/archive.rs b/compiler/rustc_codegen_ssa/src/back/archive.rs index b45fa2476875b..9113ddab048ab 100644 --- a/compiler/rustc_codegen_ssa/src/back/archive.rs +++ b/compiler/rustc_codegen_ssa/src/back/archive.rs @@ -10,7 +10,7 @@ use std::fs::File; use std::io; use std::path::{Path, PathBuf}; -use crate::errors::{ExtractBundledLibsError, ExtractBundledLibsErrorKind::*}; +use crate::errors::ExtractBundledLibsError; pub trait ArchiveBuilderBuilder { fn new_archive_builder<'a>(&self, sess: &'a Session) -> Box + 'a>; @@ -35,48 +35,30 @@ pub trait ArchiveBuilderBuilder { outdir: &Path, bundled_lib_file_names: &FxHashSet, ) -> Result<(), ExtractBundledLibsError<'_>> { - let archive_map = unsafe { - Mmap::map(File::open(rlib).map_err(|e| ExtractBundledLibsError { - kind: OpenFile, - rlib, - error: e.to_string(), - })?) - .map_err(|e| ExtractBundledLibsError { - kind: MmapFile, - rlib, - error: e.to_string(), - })? - }; - let archive = ArchiveFile::parse(&*archive_map).map_err(|e| ExtractBundledLibsError { - kind: ParseArchive, - rlib, - error: e.to_string(), - })?; + let archive_map = + unsafe { + Mmap::map(File::open(rlib).map_err(|e| ExtractBundledLibsError::OpenFile { + rlib, + error: e.to_string(), + })?) + .map_err(|e| ExtractBundledLibsError::MmapFile { rlib, error: e.to_string() })? + }; + let archive = ArchiveFile::parse(&*archive_map) + .map_err(|e| ExtractBundledLibsError::ParseArchive { rlib, error: e.to_string() })?; for entry in archive.members() { - let entry = entry.map_err(|e| ExtractBundledLibsError { - kind: ReadEntry, - rlib, - error: e.to_string(), - })?; - let data = entry.data(&*archive_map).map_err(|e| ExtractBundledLibsError { - kind: ArchiveMember, - rlib, - error: e.to_string(), - })?; - let name = std::str::from_utf8(entry.name()).map_err(|e| ExtractBundledLibsError { - kind: ConvertName, - rlib, - error: e.to_string(), + let entry = entry + .map_err(|e| ExtractBundledLibsError::ReadEntry { rlib, error: e.to_string() })?; + let data = entry.data(&*archive_map).map_err(|e| { + ExtractBundledLibsError::ArchiveMember { rlib, error: e.to_string() } })?; + let name = std::str::from_utf8(entry.name()) + .map_err(|e| ExtractBundledLibsError::ConvertName { rlib, error: e.to_string() })?; if !bundled_lib_file_names.contains(&Symbol::intern(name)) { continue; // We need to extract only native libraries. } - std::fs::write(&outdir.join(&name), data).map_err(|e| ExtractBundledLibsError { - kind: WriteFile, - rlib, - error: e.to_string(), - })?; + std::fs::write(&outdir.join(&name), data) + .map_err(|e| ExtractBundledLibsError::WriteFile { rlib, error: e.to_string() })?; } Ok(()) } diff --git a/compiler/rustc_codegen_ssa/src/back/link.rs b/compiler/rustc_codegen_ssa/src/back/link.rs index 1e6a2b6ecaa6d..6f0a8d0a54cba 100644 --- a/compiler/rustc_codegen_ssa/src/back/link.rs +++ b/compiler/rustc_codegen_ssa/src/back/link.rs @@ -1063,7 +1063,7 @@ fn strip_symbols_with_external_utility<'a>( if !prog.status.success() { let mut output = prog.stderr.clone(); output.extend_from_slice(&prog.stdout); - sess.emit_warning(errors::StrippingDebuInfoFailed { + sess.emit_warning(errors::StrippingDebugInfoFailed { util, status: prog.status, output: escape_string(&output), @@ -1077,7 +1077,6 @@ fn strip_symbols_with_external_utility<'a>( fn escape_string(s: &[u8]) -> String { match str::from_utf8(s) { Ok(s) => s.to_owned(), - // FIXME: return a type that can conform to IntoDiagnosticArg Err(_) => format!("Non-UTF-8 output: {}", s.escape_ascii()), } } diff --git a/compiler/rustc_codegen_ssa/src/errors.rs b/compiler/rustc_codegen_ssa/src/errors.rs index 35eae30c4ba9d..265f466f2ca37 100644 --- a/compiler/rustc_codegen_ssa/src/errors.rs +++ b/compiler/rustc_codegen_ssa/src/errors.rs @@ -423,7 +423,7 @@ pub struct UnableToRunDsymutil { #[derive(Diagnostic)] #[diag(codegen_ssa_stripping_debu_info_failed)] #[note] -pub struct StrippingDebuInfoFailed<'a> { +pub struct StrippingDebugInfoFailed<'a> { pub util: &'a str, pub status: ExitStatus, pub output: String, @@ -485,52 +485,28 @@ pub struct RlibArchiveBuildFailure { #[diag(codegen_ssa_option_gcc_only)] pub struct OptionGccOnly; -pub struct ExtractBundledLibsError<'a> { - pub kind: ExtractBundledLibsErrorKind, - pub rlib: &'a Path, - pub error: String, -} +#[derive(Diagnostic)] +pub enum ExtractBundledLibsError<'a> { + #[diag(codegen_ssa_extract_bundled_libs_open_file)] + OpenFile { rlib: &'a Path, error: String }, -pub enum ExtractBundledLibsErrorKind { - OpenFile, - MmapFile, - ParseArchive, - ReadEntry, - ArchiveMember, - ConvertName, - WriteFile, -} + #[diag(codegen_ssa_extract_bundled_libs_mmap_file)] + MmapFile { rlib: &'a Path, error: String }, -impl IntoDiagnostic<'_, !> for ExtractBundledLibsError<'_> { - fn into_diagnostic(self, handler: &'_ Handler) -> DiagnosticBuilder<'_, !> { - let mut diag = match self.kind { - ExtractBundledLibsErrorKind::OpenFile => { - handler.struct_fatal(fluent::codegen_ssa_extract_bundled_libs_open_file) - } - ExtractBundledLibsErrorKind::MmapFile => { - handler.struct_fatal(fluent::codegen_ssa_extract_bundled_libs_mmap_file) - } - ExtractBundledLibsErrorKind::ParseArchive => { - handler.struct_fatal(fluent::codegen_ssa_extract_bundled_libs_parse_archive) - } - ExtractBundledLibsErrorKind::ReadEntry => { - handler.struct_fatal(fluent::codegen_ssa_extract_bundled_libs_read_entry) - } - ExtractBundledLibsErrorKind::ArchiveMember => { - handler.struct_fatal(fluent::codegen_ssa_extract_bundled_libs_archive_member) - } - ExtractBundledLibsErrorKind::ConvertName => { - handler.struct_fatal(fluent::codegen_ssa_extract_bundled_libs_convert_name) - } - ExtractBundledLibsErrorKind::WriteFile => { - handler.struct_fatal(fluent::codegen_ssa_extract_bundled_libs_write_file) - } - }; + #[diag(codegen_ssa_extract_bundled_libs_parse_archive)] + ParseArchive { rlib: &'a Path, error: String }, - diag.set_arg("rlib", self.rlib); - diag.set_arg("error", self.error); - diag - } + #[diag(codegen_ssa_extract_bundled_libs_read_entry)] + ReadEntry { rlib: &'a Path, error: String }, + + #[diag(codegen_ssa_extract_bundled_libs_archive_member)] + ArchiveMember { rlib: &'a Path, error: String }, + + #[diag(codegen_ssa_extract_bundled_libs_convert_name)] + ConvertName { rlib: &'a Path, error: String }, + + #[diag(codegen_ssa_extract_bundled_libs_write_file)] + WriteFile { rlib: &'a Path, error: String }, } #[derive(Diagnostic)] From 540c3f94d71879f413a151bc8c83c20c10b386dc Mon Sep 17 00:00:00 2001 From: Jhonny Bill Mena Date: Fri, 4 Nov 2022 01:16:16 -0400 Subject: [PATCH 11/14] UPDATE - accept dyn error and make Box conform to IntoDiagnosticArg --- .../rustc_codegen_ssa/src/back/archive.rs | 29 +++++++++---------- compiler/rustc_codegen_ssa/src/errors.rs | 14 ++++----- compiler/rustc_errors/src/diagnostic_impls.rs | 1 + 3 files changed, 22 insertions(+), 22 deletions(-) diff --git a/compiler/rustc_codegen_ssa/src/back/archive.rs b/compiler/rustc_codegen_ssa/src/back/archive.rs index 9113ddab048ab..18789d00fd3a4 100644 --- a/compiler/rustc_codegen_ssa/src/back/archive.rs +++ b/compiler/rustc_codegen_ssa/src/back/archive.rs @@ -35,30 +35,29 @@ pub trait ArchiveBuilderBuilder { outdir: &Path, bundled_lib_file_names: &FxHashSet, ) -> Result<(), ExtractBundledLibsError<'_>> { - let archive_map = - unsafe { - Mmap::map(File::open(rlib).map_err(|e| ExtractBundledLibsError::OpenFile { - rlib, - error: e.to_string(), - })?) - .map_err(|e| ExtractBundledLibsError::MmapFile { rlib, error: e.to_string() })? - }; + let archive_map = unsafe { + Mmap::map( + File::open(rlib) + .map_err(|e| ExtractBundledLibsError::OpenFile { rlib, error: Box::new(e) })?, + ) + .map_err(|e| ExtractBundledLibsError::MmapFile { rlib, error: Box::new(e) })? + }; let archive = ArchiveFile::parse(&*archive_map) - .map_err(|e| ExtractBundledLibsError::ParseArchive { rlib, error: e.to_string() })?; + .map_err(|e| ExtractBundledLibsError::ParseArchive { rlib, error: Box::new(e) })?; for entry in archive.members() { let entry = entry - .map_err(|e| ExtractBundledLibsError::ReadEntry { rlib, error: e.to_string() })?; - let data = entry.data(&*archive_map).map_err(|e| { - ExtractBundledLibsError::ArchiveMember { rlib, error: e.to_string() } - })?; + .map_err(|e| ExtractBundledLibsError::ReadEntry { rlib, error: Box::new(e) })?; + let data = entry + .data(&*archive_map) + .map_err(|e| ExtractBundledLibsError::ArchiveMember { rlib, error: Box::new(e) })?; let name = std::str::from_utf8(entry.name()) - .map_err(|e| ExtractBundledLibsError::ConvertName { rlib, error: e.to_string() })?; + .map_err(|e| ExtractBundledLibsError::ConvertName { rlib, error: Box::new(e) })?; if !bundled_lib_file_names.contains(&Symbol::intern(name)) { continue; // We need to extract only native libraries. } std::fs::write(&outdir.join(&name), data) - .map_err(|e| ExtractBundledLibsError::WriteFile { rlib, error: e.to_string() })?; + .map_err(|e| ExtractBundledLibsError::WriteFile { rlib, error: Box::new(e) })?; } Ok(()) } diff --git a/compiler/rustc_codegen_ssa/src/errors.rs b/compiler/rustc_codegen_ssa/src/errors.rs index 265f466f2ca37..36c94462b0b3e 100644 --- a/compiler/rustc_codegen_ssa/src/errors.rs +++ b/compiler/rustc_codegen_ssa/src/errors.rs @@ -488,25 +488,25 @@ pub struct OptionGccOnly; #[derive(Diagnostic)] pub enum ExtractBundledLibsError<'a> { #[diag(codegen_ssa_extract_bundled_libs_open_file)] - OpenFile { rlib: &'a Path, error: String }, + OpenFile { rlib: &'a Path, error: Box }, #[diag(codegen_ssa_extract_bundled_libs_mmap_file)] - MmapFile { rlib: &'a Path, error: String }, + MmapFile { rlib: &'a Path, error: Box }, #[diag(codegen_ssa_extract_bundled_libs_parse_archive)] - ParseArchive { rlib: &'a Path, error: String }, + ParseArchive { rlib: &'a Path, error: Box }, #[diag(codegen_ssa_extract_bundled_libs_read_entry)] - ReadEntry { rlib: &'a Path, error: String }, + ReadEntry { rlib: &'a Path, error: Box }, #[diag(codegen_ssa_extract_bundled_libs_archive_member)] - ArchiveMember { rlib: &'a Path, error: String }, + ArchiveMember { rlib: &'a Path, error: Box }, #[diag(codegen_ssa_extract_bundled_libs_convert_name)] - ConvertName { rlib: &'a Path, error: String }, + ConvertName { rlib: &'a Path, error: Box }, #[diag(codegen_ssa_extract_bundled_libs_write_file)] - WriteFile { rlib: &'a Path, error: String }, + WriteFile { rlib: &'a Path, error: Box }, } #[derive(Diagnostic)] diff --git a/compiler/rustc_errors/src/diagnostic_impls.rs b/compiler/rustc_errors/src/diagnostic_impls.rs index 4f32e236b2dbb..83a3211c63b88 100644 --- a/compiler/rustc_errors/src/diagnostic_impls.rs +++ b/compiler/rustc_errors/src/diagnostic_impls.rs @@ -59,6 +59,7 @@ into_diagnostic_arg_using_display!( i128, u128, std::io::Error, + std::boxed::Box, std::num::NonZeroU32, hir::Target, Edition, From b7360fa23f6e029770acae390b47e828f1f08347 Mon Sep 17 00:00:00 2001 From: clubby789 Date: Fri, 4 Nov 2022 12:25:40 +0000 Subject: [PATCH 12/14] Give a specific lint for unsafety not being inherited --- .../rustc_mir_transform/src/check_unsafety.rs | 44 +++++++++++++++---- 1 file changed, 35 insertions(+), 9 deletions(-) diff --git a/compiler/rustc_mir_transform/src/check_unsafety.rs b/compiler/rustc_mir_transform/src/check_unsafety.rs index 959fcf8d89e86..80e1246ec6bf3 100644 --- a/compiler/rustc_mir_transform/src/check_unsafety.rs +++ b/compiler/rustc_mir_transform/src/check_unsafety.rs @@ -1,3 +1,4 @@ +use hir::{BlockCheckMode, ExprKind, Node}; use rustc_data_structures::fx::FxHashSet; use rustc_errors::struct_span_err; use rustc_hir as hir; @@ -517,24 +518,49 @@ pub fn check_unsafety(tcx: TyCtxt<'_>, def_id: LocalDefId) { for &UnsafetyViolation { source_info, lint_root, kind, details } in violations.iter() { let (description, note) = details.description_and_note(); - // Report an error. - let unsafe_fn_msg = - if unsafe_op_in_unsafe_fn_allowed(tcx, lint_root) { " function or" } else { "" }; - match kind { UnsafetyViolationKind::General => { // once - struct_span_err!( + // Mutable statics always require an unsafe block + let unsafe_fn_msg = if unsafe_op_in_unsafe_fn_allowed(tcx, lint_root) + && details != UnsafetyViolationDetails::UseOfMutableStatic + { + " function or" + } else { + "" + }; + + let mut err = struct_span_err!( tcx.sess, source_info.span, E0133, "{} is unsafe and requires unsafe{} block", description, unsafe_fn_msg, - ) - .span_label(source_info.span, description) - .note(note) - .emit(); + ); + err.span_label(source_info.span, description).note(note); + let note_non_inherited = tcx.hir().parent_iter(lint_root).find(|(id, node)| { + if let Node::Expr(block) = node + && let ExprKind::Block(block, _) = block.kind + && let BlockCheckMode::UnsafeBlock(_) = block.rules { + true + } + else if let Some(sig) = tcx.hir().fn_sig_by_hir_id(*id) + && sig.header.is_unsafe() { + true + } else { + false + } + }); + if let Some((id, _)) = note_non_inherited { + let span = tcx.hir().span(id); + err.span_label( + tcx.sess.source_map().guess_head_span(span), + "items do not inherit unsafety from separate enclosing items", + ); + } + + err.emit(); } UnsafetyViolationKind::UnsafeFn => tcx.struct_span_lint_hir( UNSAFE_OP_IN_UNSAFE_FN, From 28819cbb7e7ae05dc83e285ca936ac16876fd701 Mon Sep 17 00:00:00 2001 From: clubby789 Date: Fri, 4 Nov 2022 12:57:42 +0000 Subject: [PATCH 13/14] Formatting changes + add UI test --- .../rustc_mir_transform/src/check_unsafety.rs | 19 +++++++------- src/test/ui/unsafe/unsafe-not-inherited.rs | 26 +++++++++++++++++++ .../ui/unsafe/unsafe-not-inherited.stderr | 24 +++++++++++++++++ 3 files changed, 59 insertions(+), 10 deletions(-) create mode 100644 src/test/ui/unsafe/unsafe-not-inherited.rs create mode 100644 src/test/ui/unsafe/unsafe-not-inherited.stderr diff --git a/compiler/rustc_mir_transform/src/check_unsafety.rs b/compiler/rustc_mir_transform/src/check_unsafety.rs index 80e1246ec6bf3..269d9f3b102c1 100644 --- a/compiler/rustc_mir_transform/src/check_unsafety.rs +++ b/compiler/rustc_mir_transform/src/check_unsafety.rs @@ -1,10 +1,10 @@ -use hir::{BlockCheckMode, ExprKind, Node}; use rustc_data_structures::fx::FxHashSet; use rustc_errors::struct_span_err; use rustc_hir as hir; use rustc_hir::def_id::{DefId, LocalDefId}; use rustc_hir::hir_id::HirId; use rustc_hir::intravisit; +use rustc_hir::{BlockCheckMode, ExprKind, Node}; use rustc_middle::mir::visit::{MutatingUseContext, PlaceContext, Visitor}; use rustc_middle::mir::*; use rustc_middle::ty::query::Providers; @@ -521,10 +521,7 @@ pub fn check_unsafety(tcx: TyCtxt<'_>, def_id: LocalDefId) { match kind { UnsafetyViolationKind::General => { // once - // Mutable statics always require an unsafe block - let unsafe_fn_msg = if unsafe_op_in_unsafe_fn_allowed(tcx, lint_root) - && details != UnsafetyViolationDetails::UseOfMutableStatic - { + let unsafe_fn_msg = if unsafe_op_in_unsafe_fn_allowed(tcx, lint_root) { " function or" } else { "" @@ -542,12 +539,14 @@ pub fn check_unsafety(tcx: TyCtxt<'_>, def_id: LocalDefId) { let note_non_inherited = tcx.hir().parent_iter(lint_root).find(|(id, node)| { if let Node::Expr(block) = node && let ExprKind::Block(block, _) = block.kind - && let BlockCheckMode::UnsafeBlock(_) = block.rules { - true - } + && let BlockCheckMode::UnsafeBlock(_) = block.rules + { + true + } else if let Some(sig) = tcx.hir().fn_sig_by_hir_id(*id) - && sig.header.is_unsafe() { - true + && sig.header.is_unsafe() + { + true } else { false } diff --git a/src/test/ui/unsafe/unsafe-not-inherited.rs b/src/test/ui/unsafe/unsafe-not-inherited.rs new file mode 100644 index 0000000000000..6d797caa0f94d --- /dev/null +++ b/src/test/ui/unsafe/unsafe-not-inherited.rs @@ -0,0 +1,26 @@ +#![allow(unused, dead_code)] + +static mut FOO: u64 = 0; + +fn static_mod() { + unsafe {static BAR: u64 = FOO;} + //~^ ERROR: use of mutable static is unsafe + //~| NOTE: use of mutable static + //~| NOTE: mutable statics can be mutated by multiple threads + //~| NOTE: items do not inherit unsafety +} + +unsafe fn unsafe_call() {} +fn foo() { + unsafe { + //~^ NOTE: items do not inherit unsafety + fn bar() { + unsafe_call(); + //~^ ERROR: call to unsafe function + //~| NOTE: call to unsafe function + //~| NOTE: consult the function's documentation + } + } +} + +fn main() {} diff --git a/src/test/ui/unsafe/unsafe-not-inherited.stderr b/src/test/ui/unsafe/unsafe-not-inherited.stderr new file mode 100644 index 0000000000000..3bc5ca5c9d151 --- /dev/null +++ b/src/test/ui/unsafe/unsafe-not-inherited.stderr @@ -0,0 +1,24 @@ +error[E0133]: use of mutable static is unsafe and requires unsafe function or block + --> $DIR/unsafe-not-inherited.rs:6:31 + | +LL | unsafe {static BAR: u64 = FOO;} + | ------ ^^^ use of mutable static + | | + | items do not inherit unsafety from separate enclosing items + | + = note: mutable statics can be mutated by multiple threads: aliasing violations or data races will cause undefined behavior + +error[E0133]: call to unsafe function is unsafe and requires unsafe function or block + --> $DIR/unsafe-not-inherited.rs:18:13 + | +LL | unsafe { + | ------ items do not inherit unsafety from separate enclosing items +... +LL | unsafe_call(); + | ^^^^^^^^^^^^^ call to unsafe function + | + = note: consult the function's documentation for information on how to avoid undefined behavior + +error: aborting due to 2 previous errors + +For more information about this error, try `rustc --explain E0133`. From 18511ba1cb6199aae13d7c8dd987e328259a4702 Mon Sep 17 00:00:00 2001 From: yukang Date: Fri, 4 Nov 2022 16:56:48 +0800 Subject: [PATCH 14/14] test tidy should not count untracked paths towards entries limit --- Cargo.lock | 1 + src/tools/tidy/Cargo.toml | 1 + src/tools/tidy/src/ui_tests.rs | 59 +++++++++++++++++++--------------- 3 files changed, 35 insertions(+), 26 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index dab693419a95d..cea17ecc3d409 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4919,6 +4919,7 @@ name = "tidy" version = "0.1.0" dependencies = [ "cargo_metadata 0.14.0", + "ignore", "lazy_static", "regex", "walkdir", diff --git a/src/tools/tidy/Cargo.toml b/src/tools/tidy/Cargo.toml index 471d78a2922a0..8a6fbaecd6980 100644 --- a/src/tools/tidy/Cargo.toml +++ b/src/tools/tidy/Cargo.toml @@ -9,6 +9,7 @@ cargo_metadata = "0.14" regex = "1" lazy_static = "1" walkdir = "2" +ignore = "0.4.18" [[bin]] name = "rust-tidy" diff --git a/src/tools/tidy/src/ui_tests.rs b/src/tools/tidy/src/ui_tests.rs index c600f99c2c4bf..b4ee87bb41068 100644 --- a/src/tools/tidy/src/ui_tests.rs +++ b/src/tools/tidy/src/ui_tests.rs @@ -2,6 +2,8 @@ //! - the number of entries in each directory must be less than `ENTRY_LIMIT` //! - there are no stray `.stderr` files +use ignore::Walk; +use ignore::WalkBuilder; use std::fs; use std::path::Path; @@ -11,34 +13,39 @@ const ROOT_ENTRY_LIMIT: usize = 948; const ISSUES_ENTRY_LIMIT: usize = 2117; fn check_entries(path: &Path, bad: &mut bool) { - let dirs = walkdir::WalkDir::new(&path.join("test/ui")) - .into_iter() - .filter_entry(|e| e.file_type().is_dir()); - for dir in dirs { - if let Ok(dir) = dir { - let dir_path = dir.path(); + for dir in Walk::new(&path.join("test/ui")) { + if let Ok(entry) = dir { + if entry.file_type().map(|ft| ft.is_dir()).unwrap_or(false) { + let dir_path = entry.path(); + // Use special values for these dirs. + let is_root = path.join("test/ui") == dir_path; + let is_issues_dir = path.join("test/ui/issues") == dir_path; + let limit = if is_root { + ROOT_ENTRY_LIMIT + } else if is_issues_dir { + ISSUES_ENTRY_LIMIT + } else { + ENTRY_LIMIT + }; - // Use special values for these dirs. - let is_root = path.join("test/ui") == dir_path; - let is_issues_dir = path.join("test/ui/issues") == dir_path; - let limit = if is_root { - ROOT_ENTRY_LIMIT - } else if is_issues_dir { - ISSUES_ENTRY_LIMIT - } else { - ENTRY_LIMIT - }; + let count = WalkBuilder::new(&dir_path) + .max_depth(Some(1)) + .build() + .into_iter() + .collect::>() + .len() + - 1; // remove the dir itself - let count = std::fs::read_dir(dir_path).unwrap().count(); - if count > limit { - tidy_error!( - bad, - "following path contains more than {} entries, \ - you should move the test to some relevant subdirectory (current: {}): {}", - limit, - count, - dir_path.display() - ); + if count > limit { + tidy_error!( + bad, + "following path contains more than {} entries, \ + you should move the test to some relevant subdirectory (current: {}): {}", + limit, + count, + dir_path.display() + ); + } } } }