From 37f4535d49e40931e793b2873cc14e9869b6b6b1 Mon Sep 17 00:00:00 2001 From: mental Date: Wed, 1 Jan 2020 14:33:53 +0200 Subject: [PATCH 01/18] Make use $crate a hard error --- src/librustc_resolve/build_reduced_graph.rs | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/src/librustc_resolve/build_reduced_graph.rs b/src/librustc_resolve/build_reduced_graph.rs index 911de5d2174e6..b563e6e14e8f8 100644 --- a/src/librustc_resolve/build_reduced_graph.rs +++ b/src/librustc_resolve/build_reduced_graph.rs @@ -472,12 +472,7 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> { self.r .session - .struct_span_warn(item.span, "`$crate` may not be imported") - .note( - "`use $crate;` was erroneously allowed and \ - will become a hard error in a future release", - ) - .emit(); + .struct_span_err(item.span, "`$crate` may not be imported"); } } From 72ee3622d77c21bf84d2324f638f8d7911398923 Mon Sep 17 00:00:00 2001 From: mental Date: Wed, 1 Jan 2020 14:44:19 +0200 Subject: [PATCH 02/18] Forgot an .emit() --- src/librustc_resolve/build_reduced_graph.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/librustc_resolve/build_reduced_graph.rs b/src/librustc_resolve/build_reduced_graph.rs index b563e6e14e8f8..64e0a27c5848c 100644 --- a/src/librustc_resolve/build_reduced_graph.rs +++ b/src/librustc_resolve/build_reduced_graph.rs @@ -472,7 +472,8 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> { self.r .session - .struct_span_err(item.span, "`$crate` may not be imported"); + .struct_span_err(item.span, "`$crate` may not be imported") + .emit(); } } From 3323f1e57d64e98da05dcc90b56e0f2cde89653e Mon Sep 17 00:00:00 2001 From: mental Date: Thu, 2 Jan 2020 08:38:59 +0200 Subject: [PATCH 03/18] Update src/test/ui --- .../ui/dollar-crate/dollar-crate-is-keyword.rs | 4 +--- .../dollar-crate/dollar-crate-is-keyword.stderr | 16 ++++++---------- src/test/ui/imports/import-crate-var.rs | 3 +-- src/test/ui/imports/import-crate-var.stderr | 3 +-- 4 files changed, 9 insertions(+), 17 deletions(-) diff --git a/src/test/ui/dollar-crate/dollar-crate-is-keyword.rs b/src/test/ui/dollar-crate/dollar-crate-is-keyword.rs index 6deaeb8117956..318c4c3974bb2 100644 --- a/src/test/ui/dollar-crate/dollar-crate-is-keyword.rs +++ b/src/test/ui/dollar-crate/dollar-crate-is-keyword.rs @@ -6,10 +6,8 @@ macro_rules! m { struct $crate {} //~ ERROR expected identifier, found reserved identifier `$crate` } - use $crate; // OK - //~^ WARN `$crate` may not be imported + use $crate; // ERROR `$crate` may not be imported use $crate as $crate; //~ ERROR expected identifier, found reserved identifier `$crate` - //~^ WARN `$crate` may not be imported } } diff --git a/src/test/ui/dollar-crate/dollar-crate-is-keyword.stderr b/src/test/ui/dollar-crate/dollar-crate-is-keyword.stderr index 5d4f39086cee6..6f3cf9a55c744 100644 --- a/src/test/ui/dollar-crate/dollar-crate-is-keyword.stderr +++ b/src/test/ui/dollar-crate/dollar-crate-is-keyword.stderr @@ -8,7 +8,7 @@ LL | m!(); | ----- in this macro invocation error: expected identifier, found reserved identifier `$crate` - --> $DIR/dollar-crate-is-keyword.rs:11:23 + --> $DIR/dollar-crate-is-keyword.rs:10:23 | LL | use $crate as $crate; | ^^^^^^ expected identifier, found reserved identifier @@ -16,27 +16,23 @@ LL | use $crate as $crate; LL | m!(); | ----- in this macro invocation -warning: `$crate` may not be imported +error: `$crate` may not be imported --> $DIR/dollar-crate-is-keyword.rs:9:9 | -LL | use $crate; // OK +LL | use $crate; // ERROR `$crate` may not be imported | ^^^^^^^^^^^ ... LL | m!(); | ----- in this macro invocation - | - = note: `use $crate;` was erroneously allowed and will become a hard error in a future release -warning: `$crate` may not be imported - --> $DIR/dollar-crate-is-keyword.rs:11:9 +error: `$crate` may not be imported + --> $DIR/dollar-crate-is-keyword.rs:10:9 | LL | use $crate as $crate; | ^^^^^^^^^^^^^^^^^^^^^ ... LL | m!(); | ----- in this macro invocation - | - = note: `use $crate;` was erroneously allowed and will become a hard error in a future release -error: aborting due to 2 previous errors +error: aborting due to 4 previous errors diff --git a/src/test/ui/imports/import-crate-var.rs b/src/test/ui/imports/import-crate-var.rs index b9d146d3735f2..ad1c34844619a 100644 --- a/src/test/ui/imports/import-crate-var.rs +++ b/src/test/ui/imports/import-crate-var.rs @@ -5,6 +5,5 @@ fn main() { m!(); - //~^ WARN `$crate` may not be imported - //~| NOTE `use $crate;` was erroneously allowed and will become a hard error + //~^ ERROR `$crate` may not be imported } diff --git a/src/test/ui/imports/import-crate-var.stderr b/src/test/ui/imports/import-crate-var.stderr index 2f8c845156a82..98f113fb4559f 100644 --- a/src/test/ui/imports/import-crate-var.stderr +++ b/src/test/ui/imports/import-crate-var.stderr @@ -1,9 +1,8 @@ -warning: `$crate` may not be imported +error: `$crate` may not be imported --> $DIR/import-crate-var.rs:7:5 | LL | m!(); | ^^^^^ | - = note: `use $crate;` was erroneously allowed and will become a hard error in a future release = note: this warning originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info) From 56b3120bf487ac6197a5af8dd8e446de98b047a8 Mon Sep 17 00:00:00 2001 From: mental Date: Thu, 2 Jan 2020 11:59:32 +0200 Subject: [PATCH 04/18] Remove // check-pass for ui/import/import-crate-var.rs --- src/test/ui/imports/import-crate-var.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/test/ui/imports/import-crate-var.rs b/src/test/ui/imports/import-crate-var.rs index ad1c34844619a..aac5a15d3e6b5 100644 --- a/src/test/ui/imports/import-crate-var.rs +++ b/src/test/ui/imports/import-crate-var.rs @@ -1,4 +1,3 @@ -// check-pass // aux-build:import_crate_var.rs #[macro_use] extern crate import_crate_var; From b97438333e4b1b73a650cf2b25ebbe4ba5c3a618 Mon Sep 17 00:00:00 2001 From: mental Date: Thu, 2 Jan 2020 12:03:04 +0200 Subject: [PATCH 05/18] Lets see if I can get these ui tests to run. --- src/test/ui/dollar-crate/dollar-crate-is-keyword.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/ui/dollar-crate/dollar-crate-is-keyword.rs b/src/test/ui/dollar-crate/dollar-crate-is-keyword.rs index 318c4c3974bb2..e1a8eda948edb 100644 --- a/src/test/ui/dollar-crate/dollar-crate-is-keyword.rs +++ b/src/test/ui/dollar-crate/dollar-crate-is-keyword.rs @@ -6,7 +6,7 @@ macro_rules! m { struct $crate {} //~ ERROR expected identifier, found reserved identifier `$crate` } - use $crate; // ERROR `$crate` may not be imported + use $crate; //~ ERROR `$crate` may not be imported use $crate as $crate; //~ ERROR expected identifier, found reserved identifier `$crate` } } From c0da63951afb127a898a503b5e80061f8aa93105 Mon Sep 17 00:00:00 2001 From: mental Date: Fri, 3 Jan 2020 11:50:12 +0200 Subject: [PATCH 06/18] Thank you Dylan and JohnTitor for blessing me. --- src/test/ui/dollar-crate/dollar-crate-is-keyword.rs | 1 + src/test/ui/dollar-crate/dollar-crate-is-keyword.stderr | 3 +-- src/test/ui/imports/import-crate-var.stderr | 5 +++-- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/test/ui/dollar-crate/dollar-crate-is-keyword.rs b/src/test/ui/dollar-crate/dollar-crate-is-keyword.rs index e1a8eda948edb..d625163dc7e1c 100644 --- a/src/test/ui/dollar-crate/dollar-crate-is-keyword.rs +++ b/src/test/ui/dollar-crate/dollar-crate-is-keyword.rs @@ -8,6 +8,7 @@ macro_rules! m { use $crate; //~ ERROR `$crate` may not be imported use $crate as $crate; //~ ERROR expected identifier, found reserved identifier `$crate` + //~^ ERROR `$crate` may not be imported } } diff --git a/src/test/ui/dollar-crate/dollar-crate-is-keyword.stderr b/src/test/ui/dollar-crate/dollar-crate-is-keyword.stderr index 6f3cf9a55c744..1efd3fa06a542 100644 --- a/src/test/ui/dollar-crate/dollar-crate-is-keyword.stderr +++ b/src/test/ui/dollar-crate/dollar-crate-is-keyword.stderr @@ -19,7 +19,7 @@ LL | m!(); error: `$crate` may not be imported --> $DIR/dollar-crate-is-keyword.rs:9:9 | -LL | use $crate; // ERROR `$crate` may not be imported +LL | use $crate; | ^^^^^^^^^^^ ... LL | m!(); @@ -35,4 +35,3 @@ LL | m!(); | ----- in this macro invocation error: aborting due to 4 previous errors - diff --git a/src/test/ui/imports/import-crate-var.stderr b/src/test/ui/imports/import-crate-var.stderr index 98f113fb4559f..4f98e973c52a6 100644 --- a/src/test/ui/imports/import-crate-var.stderr +++ b/src/test/ui/imports/import-crate-var.stderr @@ -1,8 +1,9 @@ error: `$crate` may not be imported - --> $DIR/import-crate-var.rs:7:5 + --> $DIR/import-crate-var.rs:6:5 | LL | m!(); | ^^^^^ | - = note: this warning originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info) + = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info) +error: aborting due to previous error From 4feeceecd14d41cb74d10f15933aba3acdd6cc91 Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Sun, 29 Dec 2019 22:10:47 +0300 Subject: [PATCH 07/18] Introduce an option for disabling deduplication of diagnostics --- src/librustc_errors/lib.rs | 12 +++++++----- src/librustc_session/options.rs | 2 ++ src/librustc_session/session.rs | 5 ++--- .../ui/consts/miri_unleashed/mutable_const2.stderr | 2 +- 4 files changed, 12 insertions(+), 9 deletions(-) diff --git a/src/librustc_errors/lib.rs b/src/librustc_errors/lib.rs index aa2865a75f9ae..999c3470e71e1 100644 --- a/src/librustc_errors/lib.rs +++ b/src/librustc_errors/lib.rs @@ -329,6 +329,8 @@ pub struct HandlerFlags { /// show macro backtraces even for non-local macros. /// (rustc: see `-Z external-macro-backtrace`) pub external_macro_backtrace: bool, + /// If true, identical diagnostics are reported only once. + pub deduplicate_diagnostics: bool, } impl Drop for HandlerInner { @@ -736,16 +738,16 @@ impl HandlerInner { self.emitted_diagnostic_codes.insert(code.clone()); } - let diagnostic_hash = { + let already_emitted = |this: &mut Self| { use std::hash::Hash; let mut hasher = StableHasher::new(); diagnostic.hash(&mut hasher); - hasher.finish() + let diagnostic_hash = hasher.finish(); + !this.emitted_diagnostics.insert(diagnostic_hash) }; - // Only emit the diagnostic if we haven't already emitted an equivalent - // one: - if self.emitted_diagnostics.insert(diagnostic_hash) { + // Only emit the diagnostic if we haven't already emitted an equivalent one. + if !(self.flags.deduplicate_diagnostics && already_emitted(self)) { self.emitter.emit_diagnostic(diagnostic); if diagnostic.is_error() { self.deduplicated_err_count += 1; diff --git a/src/librustc_session/options.rs b/src/librustc_session/options.rs index 7e44ef7d4a433..656c1b019b2e2 100644 --- a/src/librustc_session/options.rs +++ b/src/librustc_session/options.rs @@ -946,4 +946,6 @@ options! {DebuggingOptions, DebuggingSetter, basic_debugging_options, insert_sideeffect: bool = (false, parse_bool, [TRACKED], "fix undefined behavior when a thread doesn't eventually make progress \ (such as entering an empty infinite loop) by inserting llvm.sideeffect"), + deduplicate_diagnostics: Option = (None, parse_opt_bool, [UNTRACKED], + "deduplicate identical diagnostics"), } diff --git a/src/librustc_session/session.rs b/src/librustc_session/session.rs index 28acbd5713f12..7375c5e88c028 100644 --- a/src/librustc_session/session.rs +++ b/src/librustc_session/session.rs @@ -943,12 +943,11 @@ pub fn build_session_with_source_map( let cap_lints_allow = sopts.lint_cap.map_or(false, |cap| cap == lint::Allow); let can_emit_warnings = !(warnings_allow || cap_lints_allow); - let treat_err_as_bug = sopts.debugging_opts.treat_err_as_bug; let dont_buffer_diagnostics = sopts.debugging_opts.dont_buffer_diagnostics; let report_delayed_bugs = sopts.debugging_opts.report_delayed_bugs; - let external_macro_backtrace = sopts.debugging_opts.external_macro_backtrace; + let deduplicate_diagnostics = sopts.debugging_opts.deduplicate_diagnostics.unwrap_or(true); let write_dest = match diagnostics_output { DiagnosticOutput::Default => None, @@ -964,7 +963,7 @@ pub fn build_session_with_source_map( report_delayed_bugs, dont_buffer_diagnostics, external_macro_backtrace, - ..Default::default() + deduplicate_diagnostics, }, ); diff --git a/src/test/ui/consts/miri_unleashed/mutable_const2.stderr b/src/test/ui/consts/miri_unleashed/mutable_const2.stderr index 2212b7d75d21e..3493b7c54c43b 100644 --- a/src/test/ui/consts/miri_unleashed/mutable_const2.stderr +++ b/src/test/ui/consts/miri_unleashed/mutable_const2.stderr @@ -10,7 +10,7 @@ error: internal compiler error: mutable allocation in constant LL | const MUTABLE_BEHIND_RAW: *mut i32 = &UnsafeCell::new(42) as *const _ as *mut _; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -thread 'rustc' panicked at 'no errors encountered even though `delay_span_bug` issued', src/librustc_errors/lib.rs:345:17 +thread 'rustc' panicked at 'no errors encountered even though `delay_span_bug` issued', src/librustc_errors/lib.rs:347:17 note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace. error: internal compiler error: unexpected panic From 1370bbcf0d52c480ae3da29c0b2c5c68480c2c15 Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Sun, 29 Dec 2019 23:07:23 +0300 Subject: [PATCH 08/18] rustdoc: Respect diagnostic debugging options --- src/librustc_session/config.rs | 13 ++++++++++++- src/librustc_session/session.rs | 15 +-------------- src/librustdoc/config.rs | 7 +------ src/librustdoc/core.rs | 29 ++++++++--------------------- src/librustdoc/lib.rs | 18 ++++-------------- 5 files changed, 26 insertions(+), 56 deletions(-) diff --git a/src/librustc_session/config.rs b/src/librustc_session/config.rs index 164803d92b127..b6b22e298ca62 100644 --- a/src/librustc_session/config.rs +++ b/src/librustc_session/config.rs @@ -20,7 +20,7 @@ use rustc_span::source_map::{FileName, FilePathMapping}; use rustc_span::symbol::{sym, Symbol}; use rustc_errors::emitter::HumanReadableErrorType; -use rustc_errors::{ColorConfig, FatalError, Handler}; +use rustc_errors::{ColorConfig, FatalError, Handler, HandlerFlags}; use getopts; @@ -597,6 +597,17 @@ impl DebuggingOptions { pub fn ui_testing(&self) -> bool { self.ui_testing.unwrap_or(false) } + + pub fn diagnostic_handler_flags(&self, can_emit_warnings: bool) -> HandlerFlags { + HandlerFlags { + can_emit_warnings, + treat_err_as_bug: self.treat_err_as_bug, + dont_buffer_diagnostics: self.dont_buffer_diagnostics, + report_delayed_bugs: self.report_delayed_bugs, + external_macro_backtrace: self.external_macro_backtrace, + deduplicate_diagnostics: self.deduplicate_diagnostics.unwrap_or(true), + } + } } // The type of entry function, so users can have their own entry functions diff --git a/src/librustc_session/session.rs b/src/librustc_session/session.rs index 7375c5e88c028..15911db46fe0b 100644 --- a/src/librustc_session/session.rs +++ b/src/librustc_session/session.rs @@ -941,13 +941,7 @@ pub fn build_session_with_source_map( .last() .unwrap_or(false); let cap_lints_allow = sopts.lint_cap.map_or(false, |cap| cap == lint::Allow); - let can_emit_warnings = !(warnings_allow || cap_lints_allow); - let treat_err_as_bug = sopts.debugging_opts.treat_err_as_bug; - let dont_buffer_diagnostics = sopts.debugging_opts.dont_buffer_diagnostics; - let report_delayed_bugs = sopts.debugging_opts.report_delayed_bugs; - let external_macro_backtrace = sopts.debugging_opts.external_macro_backtrace; - let deduplicate_diagnostics = sopts.debugging_opts.deduplicate_diagnostics.unwrap_or(true); let write_dest = match diagnostics_output { DiagnosticOutput::Default => None, @@ -957,14 +951,7 @@ pub fn build_session_with_source_map( let diagnostic_handler = rustc_errors::Handler::with_emitter_and_flags( emitter, - rustc_errors::HandlerFlags { - can_emit_warnings, - treat_err_as_bug, - report_delayed_bugs, - dont_buffer_diagnostics, - external_macro_backtrace, - deduplicate_diagnostics, - }, + sopts.debugging_opts.diagnostic_handler_flags(can_emit_warnings), ); build_session_(sopts, local_crate_source_file, diagnostic_handler, source_map, lint_caps) diff --git a/src/librustdoc/config.rs b/src/librustdoc/config.rs index 25a892062fcbb..55df2a5e6e71e 100644 --- a/src/librustdoc/config.rs +++ b/src/librustdoc/config.rs @@ -269,12 +269,7 @@ impl Options { let codegen_options = build_codegen_options(matches, error_format); let debugging_options = build_debugging_options(matches, error_format); - let diag = new_handler( - error_format, - None, - debugging_options.treat_err_as_bug, - debugging_options.ui_testing(), - ); + let diag = new_handler(error_format, None, &debugging_options); // check for deprecated options check_deprecated_options(&matches, &diag); diff --git a/src/librustdoc/core.rs b/src/librustdoc/core.rs index bd19eeecf7726..c48523961de05 100644 --- a/src/librustdoc/core.rs +++ b/src/librustdoc/core.rs @@ -35,7 +35,7 @@ use crate::html::render::RenderInfo; use crate::passes; -pub use rustc::session::config::{CodegenOptions, Input, Options}; +pub use rustc::session::config::{CodegenOptions, DebuggingOptions, Input, Options}; pub use rustc::session::search_paths::SearchPath; pub type ExternalPaths = FxHashMap, clean::TypeKind)>; @@ -170,12 +170,8 @@ impl<'tcx> DocContext<'tcx> { pub fn new_handler( error_format: ErrorOutputType, source_map: Option>, - treat_err_as_bug: Option, - ui_testing: bool, + debugging_opts: &DebuggingOptions, ) -> errors::Handler { - // rustdoc doesn't override (or allow to override) anything from this that is relevant here, so - // stick to the defaults - let sessopts = Options::default(); let emitter: Box = match error_format { ErrorOutputType::HumanReadable(kind) => { let (short, color_config) = kind.unzip(); @@ -184,34 +180,25 @@ pub fn new_handler( color_config, source_map.map(|cm| cm as _), short, - sessopts.debugging_opts.teach, - sessopts.debugging_opts.terminal_width, + debugging_opts.teach, + debugging_opts.terminal_width, false, ) - .ui_testing(ui_testing), + .ui_testing(debugging_opts.ui_testing), ) } ErrorOutputType::Json { pretty, json_rendered } => { let source_map = source_map.unwrap_or_else(|| { - Lrc::new(source_map::SourceMap::new(sessopts.file_path_mapping())) + Lrc::new(source_map::SourceMap::new(source_map::FilePathMapping::empty())) }); Box::new( JsonEmitter::stderr(None, source_map, pretty, json_rendered, false) - .ui_testing(ui_testing), + .ui_testing(debugging_opts.ui_testing), ) } }; - errors::Handler::with_emitter_and_flags( - emitter, - errors::HandlerFlags { - can_emit_warnings: true, - treat_err_as_bug, - report_delayed_bugs: false, - external_macro_backtrace: false, - ..Default::default() - }, - ) + errors::Handler::with_emitter_and_flags(emitter, debugging_opts.diagnostic_handler_flags(true)) } pub fn run_core(options: RustdocOptions) -> (clean::Crate, RenderInfo, RenderOptions) { diff --git a/src/librustdoc/lib.rs b/src/librustdoc/lib.rs index 998b28b8807d3..c8a32306194df 100644 --- a/src/librustdoc/lib.rs +++ b/src/librustdoc/lib.rs @@ -445,12 +445,7 @@ fn main_args(args: &[String]) -> i32 { } fn main_options(options: config::Options) -> i32 { - let diag = core::new_handler( - options.error_format, - None, - options.debugging_options.treat_err_as_bug, - options.debugging_options.ui_testing(), - ); + let diag = core::new_handler(options.error_format, None, &options.debugging_options); match (options.should_test, options.markdown_input()) { (true, true) => return markdown::test(options, &diag), @@ -463,12 +458,7 @@ fn main_options(options: config::Options) -> i32 { // need to move these items separately because we lose them by the time the closure is called, // but we can't crates the Handler ahead of time because it's not Send - let diag_opts = ( - options.error_format, - options.debugging_options.treat_err_as_bug, - options.debugging_options.ui_testing(), - options.edition, - ); + let diag_opts = (options.error_format, options.edition, options.debugging_options.clone()); let show_coverage = options.show_coverage; rust_input(options, move |out| { if show_coverage { @@ -479,8 +469,8 @@ fn main_options(options: config::Options) -> i32 { let Output { krate, renderinfo, renderopts } = out; info!("going to format"); - let (error_format, treat_err_as_bug, ui_testing, edition) = diag_opts; - let diag = core::new_handler(error_format, None, treat_err_as_bug, ui_testing); + let (error_format, edition, debugging_options) = diag_opts; + let diag = core::new_handler(error_format, None, &debugging_options); match html::render::run(krate, renderopts, renderinfo, &diag, edition) { Ok(_) => rustc_driver::EXIT_SUCCESS, Err(e) => { From 6bd6a204754658173be37f77985abf052bc33b6d Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Sun, 29 Dec 2019 23:53:35 +0300 Subject: [PATCH 09/18] Add a test --- .../ui/deduplicate-diagnostics.deduplicate.stderr | 8 ++++++++ .../ui/deduplicate-diagnostics.duplicate.stderr | 14 ++++++++++++++ src/test/ui/deduplicate-diagnostics.rs | 8 ++++++++ 3 files changed, 30 insertions(+) create mode 100644 src/test/ui/deduplicate-diagnostics.deduplicate.stderr create mode 100644 src/test/ui/deduplicate-diagnostics.duplicate.stderr create mode 100644 src/test/ui/deduplicate-diagnostics.rs diff --git a/src/test/ui/deduplicate-diagnostics.deduplicate.stderr b/src/test/ui/deduplicate-diagnostics.deduplicate.stderr new file mode 100644 index 0000000000000..1acfce506229f --- /dev/null +++ b/src/test/ui/deduplicate-diagnostics.deduplicate.stderr @@ -0,0 +1,8 @@ +error: cannot find derive macro `Unresolved` in this scope + --> $DIR/deduplicate-diagnostics.rs:4:10 + | +LL | #[derive(Unresolved)] + | ^^^^^^^^^^ + +error: aborting due to previous error + diff --git a/src/test/ui/deduplicate-diagnostics.duplicate.stderr b/src/test/ui/deduplicate-diagnostics.duplicate.stderr new file mode 100644 index 0000000000000..325da3b5d915b --- /dev/null +++ b/src/test/ui/deduplicate-diagnostics.duplicate.stderr @@ -0,0 +1,14 @@ +error: cannot find derive macro `Unresolved` in this scope + --> $DIR/deduplicate-diagnostics.rs:4:10 + | +LL | #[derive(Unresolved)] + | ^^^^^^^^^^ + +error: cannot find derive macro `Unresolved` in this scope + --> $DIR/deduplicate-diagnostics.rs:4:10 + | +LL | #[derive(Unresolved)] + | ^^^^^^^^^^ + +error: aborting due to 2 previous errors + diff --git a/src/test/ui/deduplicate-diagnostics.rs b/src/test/ui/deduplicate-diagnostics.rs new file mode 100644 index 0000000000000..4a1f503d757a5 --- /dev/null +++ b/src/test/ui/deduplicate-diagnostics.rs @@ -0,0 +1,8 @@ +// revisions: duplicate deduplicate +//[duplicate] compile-flags: -Z deduplicate-diagnostics=no + +#[derive(Unresolved)] //~ ERROR cannot find derive macro `Unresolved` in this scope + //[duplicate]~| ERROR cannot find derive macro `Unresolved` in this scope +struct S; + +fn main() {} From 5bf810599306fd880b0946ecb3e1ec37ca72762f Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Fri, 3 Jan 2020 18:03:11 +0400 Subject: [PATCH 10/18] Address review comments + Fix rebase --- src/librustc_errors/lib.rs | 3 ++- src/librustdoc/core.rs | 4 ++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/librustc_errors/lib.rs b/src/librustc_errors/lib.rs index 999c3470e71e1..2279ed8595408 100644 --- a/src/librustc_errors/lib.rs +++ b/src/librustc_errors/lib.rs @@ -746,7 +746,8 @@ impl HandlerInner { !this.emitted_diagnostics.insert(diagnostic_hash) }; - // Only emit the diagnostic if we haven't already emitted an equivalent one. + // Only emit the diagnostic if we've been asked to deduplicate and + // haven't already emitted an equivalent diagnostic. if !(self.flags.deduplicate_diagnostics && already_emitted(self)) { self.emitter.emit_diagnostic(diagnostic); if diagnostic.is_error() { diff --git a/src/librustdoc/core.rs b/src/librustdoc/core.rs index c48523961de05..5f2ce34afeecf 100644 --- a/src/librustdoc/core.rs +++ b/src/librustdoc/core.rs @@ -184,7 +184,7 @@ pub fn new_handler( debugging_opts.terminal_width, false, ) - .ui_testing(debugging_opts.ui_testing), + .ui_testing(debugging_opts.ui_testing()), ) } ErrorOutputType::Json { pretty, json_rendered } => { @@ -193,7 +193,7 @@ pub fn new_handler( }); Box::new( JsonEmitter::stderr(None, source_map, pretty, json_rendered, false) - .ui_testing(debugging_opts.ui_testing), + .ui_testing(debugging_opts.ui_testing()), ) } }; From 5b7134a30e2788912cf230887cf7b2fc68711b71 Mon Sep 17 00:00:00 2001 From: mental Date: Fri, 3 Jan 2020 17:48:10 +0200 Subject: [PATCH 11/18] Apply suggestions from code review Co-Authored-By: Yuki Okushi From c818f4c88e6ef6ba83341b122901bfd390f89c8c Mon Sep 17 00:00:00 2001 From: Yuki Okushi Date: Fri, 3 Jan 2020 18:25:49 +0900 Subject: [PATCH 12/18] try to fix ui errors --- src/test/ui/dollar-crate/dollar-crate-is-keyword.stderr | 1 + src/test/ui/imports/import-crate-var.stderr | 1 + 2 files changed, 2 insertions(+) diff --git a/src/test/ui/dollar-crate/dollar-crate-is-keyword.stderr b/src/test/ui/dollar-crate/dollar-crate-is-keyword.stderr index 1efd3fa06a542..f5a5f13f9121e 100644 --- a/src/test/ui/dollar-crate/dollar-crate-is-keyword.stderr +++ b/src/test/ui/dollar-crate/dollar-crate-is-keyword.stderr @@ -35,3 +35,4 @@ LL | m!(); | ----- in this macro invocation error: aborting due to 4 previous errors + diff --git a/src/test/ui/imports/import-crate-var.stderr b/src/test/ui/imports/import-crate-var.stderr index 4f98e973c52a6..85f15ad4648a7 100644 --- a/src/test/ui/imports/import-crate-var.stderr +++ b/src/test/ui/imports/import-crate-var.stderr @@ -7,3 +7,4 @@ LL | m!(); = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info) error: aborting due to previous error + From 92acdc8b892e0cd94f101272760a815aae3258fc Mon Sep 17 00:00:00 2001 From: Stein Somers Date: Thu, 2 Jan 2020 12:33:07 +0100 Subject: [PATCH 13/18] Tweak and extend internal documentation, including debug asserts. Co-Authored-By: Robin Kruppe --- src/liballoc/collections/btree/map.rs | 4 +-- src/liballoc/collections/btree/node.rs | 50 +++++++++++++++++++------- 2 files changed, 39 insertions(+), 15 deletions(-) diff --git a/src/liballoc/collections/btree/map.rs b/src/liballoc/collections/btree/map.rs index fa8aae04011ed..302c2bcd5e4a3 100644 --- a/src/liballoc/collections/btree/map.rs +++ b/src/liballoc/collections/btree/map.rs @@ -493,7 +493,7 @@ impl BTreeMap { BTreeMap { root: node::Root::shared_empty_root(), length: 0 } } - /// Clears the map, removing all values. + /// Clears the map, removing all elements. /// /// # Examples /// @@ -2605,7 +2605,7 @@ impl<'a, K: Ord, V> OccupiedEntry<'a, K, V> { // Handle underflow let mut cur_node = small_leaf.forget_type(); - while cur_node.len() < node::CAPACITY / 2 { + while cur_node.len() < node::MIN_LEN { match handle_underfull_node(cur_node) { AtRoot => break, EmptyParent(_) => unreachable!(), diff --git a/src/liballoc/collections/btree/node.rs b/src/liballoc/collections/btree/node.rs index 0a2849f6e3961..260e51d635dbb 100644 --- a/src/liballoc/collections/btree/node.rs +++ b/src/liballoc/collections/btree/node.rs @@ -308,15 +308,15 @@ impl Root { /// `Leaf`, the `NodeRef` points to a leaf node, when this is `Internal` the /// `NodeRef` points to an internal node, and when this is `LeafOrInternal` the /// `NodeRef` could be pointing to either type of node. -/// Note that in case of a leaf node, this might still be the shared root! Only turn -/// this into a `LeafNode` reference if you know it is not a root! Shared references -/// must be dereferencable *for the entire size of their pointee*, so `&InternalNode` -/// pointing to the shared root is UB. -/// Turning this into a `NodeHeader` is always safe. +/// Note that in case of a leaf node, this might still be the shared root! +/// Only turn this into a `LeafNode` reference if you know it is not the shared root! +/// Shared references must be dereferencable *for the entire size of their pointee*, +/// so '&LeafNode` or `&InternalNode` pointing to the shared root is undefined behavior. +/// Turning this into a `NodeHeader` reference is always safe. pub struct NodeRef { height: usize, node: NonNull>, - // This is null unless the borrow type is `Mut` + // `root` is null unless the borrow type is `Mut` root: *const Root, _marker: PhantomData<(BorrowType, Type)>, } @@ -370,8 +370,13 @@ impl NodeRef { NodeRef { height: self.height, node: self.node, root: self.root, _marker: PhantomData } } - /// Assert that this is indeed a proper leaf node, and not the shared root. + /// Exposes the leaf "portion" of any leaf or internal node that is not the shared root. + /// If the node is a leaf, this function simply opens up its data. + /// If the node is an internal node, so not a leaf, it does have all the data a leaf has + /// (header, keys and values), and this function exposes that. + /// See `NodeRef` on why the node may not be a shared root. unsafe fn as_leaf(&self) -> &LeafNode { + debug_assert!(!self.is_shared_root()); self.node.as_ref() } @@ -379,14 +384,19 @@ impl NodeRef { unsafe { &*(self.node.as_ptr() as *const NodeHeader) } } + /// Returns whether the node is the shared, empty root. pub fn is_shared_root(&self) -> bool { self.as_header().is_shared_root() } + /// Borrows a view into the keys stored in the node. + /// Works on all possible nodes, including the shared root. pub fn keys(&self) -> &[K] { self.reborrow().into_key_slice() } + /// Borrows a view into the values stored in the node. + /// The caller must ensure that the node is not the shared root. fn vals(&self) -> &[V] { self.reborrow().into_val_slice() } @@ -491,16 +501,24 @@ impl<'a, K, V, Type> NodeRef, K, V, Type> { NodeRef { height: self.height, node: self.node, root: self.root, _marker: PhantomData } } + /// Exposes the leaf "portion" of any leaf or internal node for writing. + /// If the node is a leaf, this function simply opens up its data. + /// If the node is an internal node, so not a leaf, it does have all the data a leaf has + /// (header, keys and values), and this function exposes that. + /// /// Returns a raw ptr to avoid asserting exclusive access to the entire node. + /// This also implies you can invoke this member on the shared root, but the resulting pointer + /// might not be properly aligned and definitely would not allow accessing keys and values. fn as_leaf_mut(&mut self) -> *mut LeafNode { - // We are mutable, so we cannot be the shared root, so accessing this as a leaf is okay. self.node.as_ptr() } + /// The caller must ensure that the node is not the shared root. fn keys_mut(&mut self) -> &mut [K] { unsafe { self.reborrow_mut().into_key_slice_mut() } } + /// The caller must ensure that the node is not the shared root. fn vals_mut(&mut self) -> &mut [V] { unsafe { self.reborrow_mut().into_val_slice_mut() } } @@ -551,9 +569,10 @@ impl<'a, K: 'a, V: 'a, Type> NodeRef, K, V, Type> { } } + /// The caller must ensure that the node is not the shared root. fn into_val_slice(self) -> &'a [V] { debug_assert!(!self.is_shared_root()); - // We cannot be the shared root, so `as_leaf` is okay + // We cannot be the shared root, so `as_leaf` is okay. unsafe { slice::from_raw_parts(MaybeUninit::first_ptr(&self.as_leaf().vals), self.len()) } } @@ -587,6 +606,7 @@ impl<'a, K: 'a, V: 'a, Type> NodeRef, K, V, Type> { } } + /// The caller must ensure that the node is not the shared root. fn into_val_slice_mut(mut self) -> &'a mut [V] { debug_assert!(!self.is_shared_root()); unsafe { @@ -597,6 +617,7 @@ impl<'a, K: 'a, V: 'a, Type> NodeRef, K, V, Type> { } } + /// The caller must ensure that the node is not the shared root. fn into_slices_mut(mut self) -> (&'a mut [K], &'a mut [V]) { debug_assert!(!self.is_shared_root()); // We cannot use the getters here, because calling the second one @@ -655,6 +676,7 @@ impl<'a, K, V> NodeRef, K, V, marker::Internal> { // Necessary for correctness, but this is an internal module debug_assert!(edge.height == self.height - 1); debug_assert!(self.len() < CAPACITY); + debug_assert!(!self.is_shared_root()); let idx = self.len(); @@ -686,6 +708,7 @@ impl<'a, K, V> NodeRef, K, V, marker::Internal> { // Necessary for correctness, but this is an internal module debug_assert!(edge.height == self.height - 1); debug_assert!(self.len() < CAPACITY); + debug_assert!(!self.is_shared_root()); unsafe { slice_insert(self.keys_mut(), 0, key); @@ -773,6 +796,7 @@ impl<'a, K, V> NodeRef, K, V, marker::LeafOrInternal> { } } + /// The caller must ensure that the node is not the shared root. fn into_kv_pointers_mut(mut self) -> (*mut K, *mut V) { (self.keys_mut().as_mut_ptr(), self.vals_mut().as_mut_ptr()) } @@ -1116,8 +1140,8 @@ impl<'a, K, V> Handle, K, V, marker::Leaf>, marker::KV> } } - /// Removes the key/value pair pointed to by this handle, returning the edge between the - /// now adjacent key/value pairs to the left and right of this handle. + /// Removes the key/value pair pointed to by this handle and returns it, along with the edge + /// between the now adjacent key/value pairs (if any) to the left and right of this handle. pub fn remove( mut self, ) -> (Handle, K, V, marker::Leaf>, marker::Edge>, K, V) { @@ -1260,7 +1284,7 @@ impl<'a, K, V> Handle, K, V, marker::Internal>, marker:: } } - /// This removes a key/value pair from the left child and replaces it with the key/value pair + /// This removes a key/value pair from the left child and places it in the key/value storage /// pointed to by this handle while pushing the old key/value pair of this handle into the right /// child. pub fn steal_left(&mut self) { @@ -1277,7 +1301,7 @@ impl<'a, K, V> Handle, K, V, marker::Internal>, marker:: } } - /// This removes a key/value pair from the right child and replaces it with the key/value pair + /// This removes a key/value pair from the right child and places it in the key/value storage /// pointed to by this handle while pushing the old key/value pair of this handle into the left /// child. pub fn steal_right(&mut self) { From eaccda009f5891c67e554b31cfad4a52738f9b91 Mon Sep 17 00:00:00 2001 From: Adam Perry Date: Sat, 7 Dec 2019 08:37:08 -0800 Subject: [PATCH 14/18] core and std macros and panic internals use panic::Location::caller. --- src/libcore/macros/mod.rs | 12 +++--------- src/libcore/panicking.rs | 5 +++-- src/librustc_expand/build.rs | 10 ++-------- src/librustc_mir/interpret/intrinsics.rs | 2 +- src/libstd/lib.rs | 1 + src/libstd/macros.rs | 18 ++++++++++++++++-- src/libstd/panicking.rs | 24 +++++++++--------------- 7 files changed, 35 insertions(+), 37 deletions(-) diff --git a/src/libcore/macros/mod.rs b/src/libcore/macros/mod.rs index 9a52823a45474..0eb9e19423617 100644 --- a/src/libcore/macros/mod.rs +++ b/src/libcore/macros/mod.rs @@ -1,19 +1,13 @@ #[doc(include = "panic.md")] #[macro_export] -#[allow_internal_unstable(core_panic, - // FIXME(anp, eddyb) `core_intrinsics` is used here to allow calling - // the `caller_location` intrinsic, but once `#[track_caller]` is implemented, - // `panicking::{panic, panic_fmt}` can use that instead of a `Location` argument. - core_intrinsics, - const_caller_location, -)] +#[allow_internal_unstable(core_panic, track_caller)] #[stable(feature = "core", since = "1.6.0")] macro_rules! panic { () => ( $crate::panic!("explicit panic") ); ($msg:expr) => ( - $crate::panicking::panic($msg, $crate::intrinsics::caller_location()) + $crate::panicking::panic($msg) ); ($msg:expr,) => ( $crate::panic!($msg) @@ -21,7 +15,7 @@ macro_rules! panic { ($fmt:expr, $($arg:tt)+) => ( $crate::panicking::panic_fmt( $crate::format_args!($fmt, $($arg)+), - $crate::intrinsics::caller_location(), + $crate::panic::Location::caller(), ) ); } diff --git a/src/libcore/panicking.rs b/src/libcore/panicking.rs index 7ebb72e3ce7ba..61b764f2d6206 100644 --- a/src/libcore/panicking.rs +++ b/src/libcore/panicking.rs @@ -36,8 +36,9 @@ use crate::panic::{Location, PanicInfo}; // never inline unless panic_immediate_abort to avoid code // bloat at the call sites as much as possible #[cfg_attr(not(feature = "panic_immediate_abort"), inline(never))] +#[track_caller] #[lang = "panic"] // needed by codegen for panic on overflow and other `Assert` MIR terminators -pub fn panic(expr: &str, location: &Location<'_>) -> ! { +pub fn panic(expr: &str) -> ! { if cfg!(feature = "panic_immediate_abort") { unsafe { super::intrinsics::abort() } } @@ -48,7 +49,7 @@ pub fn panic(expr: &str, location: &Location<'_>) -> ! { // truncation and padding (even though none is used here). Using // Arguments::new_v1 may allow the compiler to omit Formatter::pad from the // output binary, saving up to a few kilobytes. - panic_fmt(fmt::Arguments::new_v1(&[expr], &[]), location) + panic_fmt(fmt::Arguments::new_v1(&[expr], &[]), Location::caller()) } #[cold] diff --git a/src/librustc_expand/build.rs b/src/librustc_expand/build.rs index 3375efe41f133..868f620300133 100644 --- a/src/librustc_expand/build.rs +++ b/src/librustc_expand/build.rs @@ -6,7 +6,7 @@ use syntax::ptr::P; use syntax::source_map::{respan, Spanned}; use syntax::symbol::{kw, sym, Symbol}; -use rustc_span::{Pos, Span}; +use rustc_span::Span; impl<'a> ExtCtxt<'a> { pub fn path(&self, span: Span, strs: Vec) -> ast::Path { @@ -350,16 +350,10 @@ impl<'a> ExtCtxt<'a> { } pub fn expr_fail(&self, span: Span, msg: Symbol) -> P { - let loc = self.source_map().lookup_char_pos(span.lo()); - let expr_file = self.expr_str(span, Symbol::intern(&loc.file.name.to_string())); - let expr_line = self.expr_u32(span, loc.line as u32); - let expr_col = self.expr_u32(span, loc.col.to_usize() as u32 + 1); - let expr_loc_tuple = self.expr_tuple(span, vec![expr_file, expr_line, expr_col]); - let expr_loc_ptr = self.expr_addr_of(span, expr_loc_tuple); self.expr_call_global( span, [sym::std, sym::rt, sym::begin_panic].iter().map(|s| Ident::new(*s, span)).collect(), - vec![self.expr_str(span, msg), expr_loc_ptr], + vec![self.expr_str(span, msg)], ) } diff --git a/src/librustc_mir/interpret/intrinsics.rs b/src/librustc_mir/interpret/intrinsics.rs index cc38cbbac8d65..256f7f2ab388b 100644 --- a/src/librustc_mir/interpret/intrinsics.rs +++ b/src/librustc_mir/interpret/intrinsics.rs @@ -392,7 +392,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { throw_panic!(Panic { msg, file, line, col }) } else if Some(def_id) == self.tcx.lang_items().begin_panic_fn() { assert!(args.len() == 2); - // &'static str, &(&'static str, u32, u32) + // &'static str, &core::panic::Location { &'static str, u32, u32 } let msg = args[0]; let place = self.deref_operand(args[1])?; let (file, line, col) = ( diff --git a/src/libstd/lib.rs b/src/libstd/lib.rs index 930bf397bc45b..23f82d7c2119d 100644 --- a/src/libstd/lib.rs +++ b/src/libstd/lib.rs @@ -306,6 +306,7 @@ #![feature(thread_local)] #![feature(toowned_clone_into)] #![feature(trace_macros)] +#![feature(track_caller)] #![feature(try_reserve)] #![feature(unboxed_closures)] #![feature(untagged_unions)] diff --git a/src/libstd/macros.rs b/src/libstd/macros.rs index 11850a1b5fc38..18fb0f87688de 100644 --- a/src/libstd/macros.rs +++ b/src/libstd/macros.rs @@ -4,6 +4,7 @@ //! library. Each macro is available for use when linking against the standard //! library. +#[cfg(bootstrap)] #[doc(include = "../libcore/macros/panic.md")] #[macro_export] #[stable(feature = "rust1", since = "1.0.0")] @@ -19,8 +20,21 @@ macro_rules! panic { $crate::panic!($msg) }); ($fmt:expr, $($arg:tt)+) => ({ - $crate::rt::begin_panic_fmt(&$crate::format_args!($fmt, $($arg)+), - &($crate::file!(), $crate::line!(), $crate::column!())) + $crate::rt::begin_panic_fmt(&$crate::format_args!($fmt, $($arg)+)) + }); +} + +#[cfg(not(bootstrap))] +#[doc(include = "../libcore/macros/panic.md")] +#[macro_export] +#[stable(feature = "rust1", since = "1.0.0")] +#[allow_internal_unstable(libstd_sys_internals)] +macro_rules! panic { + () => ({ $crate::panic!("explicit panic") }); + ($msg:expr) => ({ $crate::rt::begin_panic($msg) }); + ($msg:expr,) => ({ $crate::panic!($msg) }); + ($fmt:expr, $($arg:tt)+) => ({ + $crate::rt::begin_panic_fmt(&$crate::format_args!($fmt, $($arg)+)) }); } diff --git a/src/libstd/panicking.rs b/src/libstd/panicking.rs index 43230d7a2c7af..e3ce7a33a6f1f 100644 --- a/src/libstd/panicking.rs +++ b/src/libstd/panicking.rs @@ -313,17 +313,15 @@ pub fn panicking() -> bool { #[cold] // If panic_immediate_abort, inline the abort call, // otherwise avoid inlining because of it is cold path. +#[cfg_attr(not(feature = "panic_immediate_abort"), track_caller)] #[cfg_attr(not(feature = "panic_immediate_abort"), inline(never))] #[cfg_attr(feature = "panic_immediate_abort", inline)] -pub fn begin_panic_fmt(msg: &fmt::Arguments<'_>, file_line_col: &(&'static str, u32, u32)) -> ! { +pub fn begin_panic_fmt(msg: &fmt::Arguments<'_>) -> ! { if cfg!(feature = "panic_immediate_abort") { unsafe { intrinsics::abort() } } - // Just package everything into a `PanicInfo` and continue like libcore panics. - let (file, line, col) = *file_line_col; - let location = Location::internal_constructor(file, line, col); - let info = PanicInfo::internal_constructor(Some(msg), &location); + let info = PanicInfo::internal_constructor(Some(msg), Location::caller()); begin_panic_handler(&info) } @@ -372,8 +370,7 @@ pub fn begin_panic_handler(info: &PanicInfo<'_>) -> ! { let loc = info.location().unwrap(); // The current implementation always returns Some let msg = info.message().unwrap(); // The current implementation always returns Some - let file_line_col = (loc.file(), loc.line(), loc.column()); - rust_panic_with_hook(&mut PanicPayload::new(msg), info.message(), &file_line_col); + rust_panic_with_hook(&mut PanicPayload::new(msg), info.message(), loc); } /// This is the entry point of panicking for the non-format-string variants of @@ -386,7 +383,8 @@ pub fn begin_panic_handler(info: &PanicInfo<'_>) -> ! { // bloat at the call sites as much as possible #[cfg_attr(not(feature = "panic_immediate_abort"), inline(never))] #[cold] -pub fn begin_panic(msg: M, file_line_col: &(&'static str, u32, u32)) -> ! { +#[track_caller] +pub fn begin_panic(msg: M, #[cfg(bootstrap)] _: &(&str, u32, u32)) -> ! { if cfg!(feature = "panic_immediate_abort") { unsafe { intrinsics::abort() } } @@ -397,8 +395,7 @@ pub fn begin_panic(msg: M, file_line_col: &(&'static str, u32, u3 // we do start doing this, then we should propagate this allocation to // be performed in the parent of this thread instead of the thread that's // panicking. - - rust_panic_with_hook(&mut PanicPayload::new(msg), None, file_line_col); + rust_panic_with_hook(&mut PanicPayload::new(msg), None, Location::caller()); struct PanicPayload { inner: Option, @@ -436,10 +433,8 @@ pub fn begin_panic(msg: M, file_line_col: &(&'static str, u32, u3 fn rust_panic_with_hook( payload: &mut dyn BoxMeUp, message: Option<&fmt::Arguments<'_>>, - file_line_col: &(&str, u32, u32), + location: &Location<'_>, ) -> ! { - let (file, line, col) = *file_line_col; - let panics = update_panic_count(1); // If this is the third nested call (e.g., panics == 2, this is 0-indexed), @@ -456,8 +451,7 @@ fn rust_panic_with_hook( } unsafe { - let location = Location::internal_constructor(file, line, col); - let mut info = PanicInfo::internal_constructor(message, &location); + let mut info = PanicInfo::internal_constructor(message, location); HOOK_LOCK.read(); match HOOK { // Some platforms (like wasm) know that printing to stderr won't ever actually From e218da425160d4babaa46d7da1720a11ac6c02fa Mon Sep 17 00:00:00 2001 From: Adam Perry Date: Sun, 8 Dec 2019 04:51:55 -0800 Subject: [PATCH 15/18] Test cleanups to match #[track_caller] in panic!. * Removes unnecessary feature flag from track_caller test. * Tests of panic internals no longer need to explicitly construct Location. * Add #![warn(const_err)] to retain-never-const per @oli-obk. * Add track_caller test with diverging function. --- src/test/mir-opt/retain-never-const.rs | 1 + ...llow-unwind-when-calling-panic-directly.rs | 6 ++---- .../caller-location-intrinsic.rs | 6 +++--- .../diverging-caller-location.rs | 19 +++++++++++++++++++ .../track-caller-attribute.rs | 2 +- 5 files changed, 26 insertions(+), 8 deletions(-) create mode 100644 src/test/ui/rfc-2091-track-caller/diverging-caller-location.rs diff --git a/src/test/mir-opt/retain-never-const.rs b/src/test/mir-opt/retain-never-const.rs index 04394dcdf1334..8e9bae8569f19 100644 --- a/src/test/mir-opt/retain-never-const.rs +++ b/src/test/mir-opt/retain-never-const.rs @@ -6,6 +6,7 @@ #![feature(const_panic)] #![feature(never_type)] +#![warn(const_err)] struct PrintName(T); diff --git a/src/test/ui/extern/issue-64655-allow-unwind-when-calling-panic-directly.rs b/src/test/ui/extern/issue-64655-allow-unwind-when-calling-panic-directly.rs index 8727c9d1ca655..74c6e501c9136 100644 --- a/src/test/ui/extern/issue-64655-allow-unwind-when-calling-panic-directly.rs +++ b/src/test/ui/extern/issue-64655-allow-unwind-when-calling-panic-directly.rs @@ -22,7 +22,7 @@ //[thin]compile-flags: -C lto=thin //[fat]compile-flags: -C lto=fat -#![feature(core_panic, panic_internals)] +#![feature(core_panic)] // (For some reason, reproducing the LTO issue requires pulling in std // explicitly this way.) @@ -50,9 +50,7 @@ fn main() { } let _guard = Droppable; - let s = "issue-64655-allow-unwind-when-calling-panic-directly.rs"; - let location = core::panic::Location::internal_constructor(s, 17, 4); - core::panicking::panic("???", &location); + core::panicking::panic("???"); }); let wait = handle.join(); diff --git a/src/test/ui/rfc-2091-track-caller/caller-location-intrinsic.rs b/src/test/ui/rfc-2091-track-caller/caller-location-intrinsic.rs index 0a79aea376fbc..090e912c1d0ba 100644 --- a/src/test/ui/rfc-2091-track-caller/caller-location-intrinsic.rs +++ b/src/test/ui/rfc-2091-track-caller/caller-location-intrinsic.rs @@ -4,16 +4,16 @@ #[inline(never)] #[track_caller] -fn defeat_const_prop() -> &'static core::panic::Location<'static> { +fn codegen_caller_loc() -> &'static core::panic::Location<'static> { core::panic::Location::caller() } macro_rules! caller_location_from_macro { - () => (defeat_const_prop()); + () => (codegen_caller_loc()); } fn main() { - let loc = defeat_const_prop(); + let loc = codegen_caller_loc(); assert_eq!(loc.file(), file!()); assert_eq!(loc.line(), 16); assert_eq!(loc.column(), 15); diff --git a/src/test/ui/rfc-2091-track-caller/diverging-caller-location.rs b/src/test/ui/rfc-2091-track-caller/diverging-caller-location.rs new file mode 100644 index 0000000000000..1fb75ef35ffc1 --- /dev/null +++ b/src/test/ui/rfc-2091-track-caller/diverging-caller-location.rs @@ -0,0 +1,19 @@ +// run-fail + +//! This test ensures that `#[track_caller]` can be applied directly to diverging functions, as +//! the tracking issue says: https://github.com/rust-lang/rust/issues/47809#issue-292138490. +//! Because the annotated function must diverge and a panic keeps that faster than an infinite loop, +//! we don't inspect the location returned -- it would be difficult to distinguish between the +//! explicit panic and a failed assertion. That it compiles and runs is enough for this one. + +#![feature(track_caller)] + +#[track_caller] +fn doesnt_return() -> ! { + let _location = core::panic::Location::caller(); + panic!("huzzah"); +} + +fn main() { + doesnt_return(); +} diff --git a/src/test/ui/rfc-2091-track-caller/track-caller-attribute.rs b/src/test/ui/rfc-2091-track-caller/track-caller-attribute.rs index 8436ee510a5bc..76a380ed3e30d 100644 --- a/src/test/ui/rfc-2091-track-caller/track-caller-attribute.rs +++ b/src/test/ui/rfc-2091-track-caller/track-caller-attribute.rs @@ -1,6 +1,6 @@ // run-pass -#![feature(const_fn, track_caller)] +#![feature(track_caller)] use std::panic::Location; From 612c4c6c900a6e2d39df1019a794a8aa3ddf6e17 Mon Sep 17 00:00:00 2001 From: Adam Perry Date: Mon, 23 Dec 2019 19:25:09 -0800 Subject: [PATCH 16/18] Update ABI in const impls of panic_fn/begin_panic_fn. --- src/librustc_mir/const_eval/machine.rs | 3 +- src/librustc_mir/interpret/intrinsics.rs | 42 ++++--------------- .../interpret/intrinsics/caller_location.rs | 12 ++++-- src/librustc_mir/interpret/machine.rs | 1 + src/librustc_mir/interpret/terminator.rs | 2 +- src/librustc_mir/transform/const_prop.rs | 1 + 6 files changed, 22 insertions(+), 39 deletions(-) diff --git a/src/librustc_mir/const_eval/machine.rs b/src/librustc_mir/const_eval/machine.rs index a76153c19ec67..aa9c718c3eb68 100644 --- a/src/librustc_mir/const_eval/machine.rs +++ b/src/librustc_mir/const_eval/machine.rs @@ -177,6 +177,7 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter<'mir, fn find_mir_or_eval_fn( ecx: &mut InterpCx<'mir, 'tcx, Self>, + span: Span, instance: ty::Instance<'tcx>, args: &[OpTy<'tcx>], ret: Option<(PlaceTy<'tcx>, mir::BasicBlock)>, @@ -199,7 +200,7 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter<'mir, // Some functions we support even if they are non-const -- but avoid testing // that for const fn! We certainly do *not* want to actually call the fn // though, so be sure we return here. - return if ecx.hook_panic_fn(instance, args, ret)? { + return if ecx.hook_panic_fn(span, instance, args)? { Ok(None) } else { throw_unsup_format!("calling non-const function `{}`", instance) diff --git a/src/librustc_mir/interpret/intrinsics.rs b/src/librustc_mir/interpret/intrinsics.rs index 256f7f2ab388b..b075e8ea383e1 100644 --- a/src/librustc_mir/interpret/intrinsics.rs +++ b/src/librustc_mir/interpret/intrinsics.rs @@ -366,47 +366,21 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { /// Returns `true` if an intercept happened. pub fn hook_panic_fn( &mut self, + span: Span, instance: ty::Instance<'tcx>, args: &[OpTy<'tcx, M::PointerTag>], - _ret: Option<(PlaceTy<'tcx, M::PointerTag>, mir::BasicBlock)>, ) -> InterpResult<'tcx, bool> { let def_id = instance.def_id(); - if Some(def_id) == self.tcx.lang_items().panic_fn() { - // &'static str, &core::panic::Location { &'static str, u32, u32 } - assert!(args.len() == 2); + if Some(def_id) == self.tcx.lang_items().panic_fn() + || Some(def_id) == self.tcx.lang_items().begin_panic_fn() + { + // &'static str + assert!(args.len() == 1); let msg_place = self.deref_operand(args[0])?; let msg = Symbol::intern(self.read_str(msg_place)?); - - let location = self.deref_operand(args[1])?; - let (file, line, col) = ( - self.mplace_field(location, 0)?, - self.mplace_field(location, 1)?, - self.mplace_field(location, 2)?, - ); - - let file_place = self.deref_operand(file.into())?; - let file = Symbol::intern(self.read_str(file_place)?); - let line = self.read_scalar(line.into())?.to_u32()?; - let col = self.read_scalar(col.into())?.to_u32()?; - throw_panic!(Panic { msg, file, line, col }) - } else if Some(def_id) == self.tcx.lang_items().begin_panic_fn() { - assert!(args.len() == 2); - // &'static str, &core::panic::Location { &'static str, u32, u32 } - let msg = args[0]; - let place = self.deref_operand(args[1])?; - let (file, line, col) = ( - self.mplace_field(place, 0)?, - self.mplace_field(place, 1)?, - self.mplace_field(place, 2)?, - ); - - let msg_place = self.deref_operand(msg.into())?; - let msg = Symbol::intern(self.read_str(msg_place)?); - let file_place = self.deref_operand(file.into())?; - let file = Symbol::intern(self.read_str(file_place)?); - let line = self.read_scalar(line.into())?.to_u32()?; - let col = self.read_scalar(col.into())?.to_u32()?; + let span = self.find_closest_untracked_caller_location().unwrap_or(span); + let (file, line, col) = self.location_triple_for_span(span); throw_panic!(Panic { msg, file, line, col }) } else { return Ok(false); diff --git a/src/librustc_mir/interpret/intrinsics/caller_location.rs b/src/librustc_mir/interpret/intrinsics/caller_location.rs index da0a9fb4f1be5..0b07cb2f65420 100644 --- a/src/librustc_mir/interpret/intrinsics/caller_location.rs +++ b/src/librustc_mir/interpret/intrinsics/caller_location.rs @@ -9,8 +9,9 @@ use crate::interpret::{ }; impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { - /// Walks up the callstack from the intrinsic's callsite, searching for the first frame which is - /// not `#[track_caller]`. + /// Walks up the callstack from the intrinsic's callsite, searching for the first callsite in a + /// frame which is not `#[track_caller]`. If the first frame found lacks `#[track_caller]`, then + /// `None` is returned and the callsite of the function invocation itself should be used. crate fn find_closest_untracked_caller_location(&self) -> Option { let mut caller_span = None; for next_caller in self.stack.iter().rev() { @@ -54,9 +55,14 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { } pub fn alloc_caller_location_for_span(&mut self, span: Span) -> MPlaceTy<'tcx, M::PointerTag> { + let (file, line, column) = self.location_triple_for_span(span); + self.alloc_caller_location(file, line, column) + } + + pub fn location_triple_for_span(&self, span: Span) -> (Symbol, u32, u32) { let topmost = span.ctxt().outer_expn().expansion_cause().unwrap_or(span); let caller = self.tcx.sess.source_map().lookup_char_pos(topmost.lo()); - self.alloc_caller_location( + ( Symbol::intern(&caller.file.name.to_string()), caller.line as u32, caller.col_display as u32 + 1, diff --git a/src/librustc_mir/interpret/machine.rs b/src/librustc_mir/interpret/machine.rs index 3b444ac46ef26..3dc572d256d8e 100644 --- a/src/librustc_mir/interpret/machine.rs +++ b/src/librustc_mir/interpret/machine.rs @@ -139,6 +139,7 @@ pub trait Machine<'mir, 'tcx>: Sized { /// was used. fn find_mir_or_eval_fn( ecx: &mut InterpCx<'mir, 'tcx, Self>, + span: Span, instance: ty::Instance<'tcx>, args: &[OpTy<'tcx, Self::PointerTag>], ret: Option<(PlaceTy<'tcx, Self::PointerTag>, mir::BasicBlock)>, diff --git a/src/librustc_mir/interpret/terminator.rs b/src/librustc_mir/interpret/terminator.rs index 1cc22c03a05f9..7b0efd2177882 100644 --- a/src/librustc_mir/interpret/terminator.rs +++ b/src/librustc_mir/interpret/terminator.rs @@ -238,7 +238,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { | ty::InstanceDef::CloneShim(..) | ty::InstanceDef::Item(_) => { // We need MIR for this fn - let body = match M::find_mir_or_eval_fn(self, instance, args, ret, unwind)? { + let body = match M::find_mir_or_eval_fn(self, span, instance, args, ret, unwind)? { Some(body) => body, None => return Ok(()), }; diff --git a/src/librustc_mir/transform/const_prop.rs b/src/librustc_mir/transform/const_prop.rs index e1c2b494337e7..4580ae631c2f4 100644 --- a/src/librustc_mir/transform/const_prop.rs +++ b/src/librustc_mir/transform/const_prop.rs @@ -125,6 +125,7 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for ConstPropMachine { fn find_mir_or_eval_fn( _ecx: &mut InterpCx<'mir, 'tcx, Self>, + _span: Span, _instance: ty::Instance<'tcx>, _args: &[OpTy<'tcx>], _ret: Option<(PlaceTy<'tcx>, BasicBlock)>, From b76a5be18f69b79ddad8a6b72faf8ae9f2bb5e6d Mon Sep 17 00:00:00 2001 From: Adam Perry Date: Sat, 4 Jan 2020 00:49:18 -0800 Subject: [PATCH 17/18] Clean up comments in panicking infra. --- src/libstd/panicking.rs | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/src/libstd/panicking.rs b/src/libstd/panicking.rs index e3ce7a33a6f1f..599ccc809be1f 100644 --- a/src/libstd/panicking.rs +++ b/src/libstd/panicking.rs @@ -354,6 +354,9 @@ pub fn begin_panic_handler(info: &PanicInfo<'_>) -> ! { unsafe impl<'a> BoxMeUp for PanicPayload<'a> { fn take_box(&mut self) -> *mut (dyn Any + Send) { + // We do two allocations here, unfortunately. But (a) they're required with the current + // scheme, and (b) we don't handle panic + OOM properly anyway (see comment in + // begin_panic below). let contents = mem::take(self.fill()); Box::into_raw(Box::new(contents)) } @@ -363,11 +366,6 @@ pub fn begin_panic_handler(info: &PanicInfo<'_>) -> ! { } } - // We do two allocations here, unfortunately. But (a) they're - // required with the current scheme, and (b) we don't handle - // panic + OOM properly anyway (see comment in begin_panic - // below). - let loc = info.location().unwrap(); // The current implementation always returns Some let msg = info.message().unwrap(); // The current implementation always returns Some rust_panic_with_hook(&mut PanicPayload::new(msg), info.message(), loc); @@ -389,12 +387,6 @@ pub fn begin_panic(msg: M, #[cfg(bootstrap)] _: &(&str, u32, u32) unsafe { intrinsics::abort() } } - // Note that this should be the only allocation performed in this code path. - // Currently this means that panic!() on OOM will invoke this code path, - // but then again we're not really ready for panic on OOM anyway. If - // we do start doing this, then we should propagate this allocation to - // be performed in the parent of this thread instead of the thread that's - // panicking. rust_panic_with_hook(&mut PanicPayload::new(msg), None, Location::caller()); struct PanicPayload { @@ -409,6 +401,11 @@ pub fn begin_panic(msg: M, #[cfg(bootstrap)] _: &(&str, u32, u32) unsafe impl BoxMeUp for PanicPayload { fn take_box(&mut self) -> *mut (dyn Any + Send) { + // Note that this should be the only allocation performed in this code path. Currently + // this means that panic!() on OOM will invoke this code path, but then again we're not + // really ready for panic on OOM anyway. If we do start doing this, then we should + // propagate this allocation to be performed in the parent of this thread instead of the + // thread that's panicking. let data = match self.inner.take() { Some(a) => Box::new(a) as Box, None => process::abort(), From 27b25eb822d32911b73991c7fd6921fea609f825 Mon Sep 17 00:00:00 2001 From: Adam Perry Date: Fri, 3 Jan 2020 05:44:49 -0800 Subject: [PATCH 18/18] Restrict visibility of location_triple_for_span. --- src/librustc_mir/interpret/intrinsics/caller_location.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/librustc_mir/interpret/intrinsics/caller_location.rs b/src/librustc_mir/interpret/intrinsics/caller_location.rs index 0b07cb2f65420..0525108d2d129 100644 --- a/src/librustc_mir/interpret/intrinsics/caller_location.rs +++ b/src/librustc_mir/interpret/intrinsics/caller_location.rs @@ -59,7 +59,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { self.alloc_caller_location(file, line, column) } - pub fn location_triple_for_span(&self, span: Span) -> (Symbol, u32, u32) { + pub(super) fn location_triple_for_span(&self, span: Span) -> (Symbol, u32, u32) { let topmost = span.ctxt().outer_expn().expansion_cause().unwrap_or(span); let caller = self.tcx.sess.source_map().lookup_char_pos(topmost.lo()); (