From 676f14baa0eb0291e946e6c134f832277c5236b8 Mon Sep 17 00:00:00 2001 From: Philipp Hansch Date: Fri, 27 Sep 2019 07:09:12 +0200 Subject: [PATCH 1/8] Add custom ICE message that points to Clippy repo This utilizes https://github.com/rust-lang/rust/pull/60584 by setting our own `panic_hook` and pointing to our own issue tracker instead of the rustc issue tracker. This also adds a new internal lint to test the ICE message. **Potential downsides** * This essentially copies rustc's `report_ice` function as `report_clippy_ice`. I think that's how it's meant to be implemented, but maybe @jonas-schievink could have a look as well =) The downside of more-or-less copying this function is that we have to maintain it as well now. The original function can be found [here][original]. * `driver` now depends directly on `rustc` and `rustc_errors` Closes #2734 [original]: https://github.com/rust-lang/rust/blob/59367b074f1523353dddefa678ffe3cac9fd4e50/src/librustc_driver/lib.rs#L1185 --- Cargo.toml | 1 + clippy_lints/src/lib.rs | 2 + clippy_lints/src/utils/internal_lints.rs | 39 +++++++++++++++ src/driver.rs | 63 +++++++++++++++++++++++- tests/ui/custom_ice_message.rs | 5 ++ tests/ui/custom_ice_message.stderr | 11 +++++ 6 files changed, 120 insertions(+), 1 deletion(-) create mode 100644 tests/ui/custom_ice_message.rs create mode 100644 tests/ui/custom_ice_message.stderr diff --git a/Cargo.toml b/Cargo.toml index dabc4a62e6d6..6b757ac0a2fc 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -40,6 +40,7 @@ semver = "0.9" rustc_tools_util = { version = "0.2.0", path = "rustc_tools_util"} git2 = { version = "0.10", optional = true } tempfile = { version = "3.1.0", optional = true } +lazy_static = "1.0" [dev-dependencies] cargo_metadata = "0.9.0" diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 07fe9f474653..c1113daadd81 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -963,6 +963,7 @@ pub fn register_plugins(store: &mut lint::LintStore, sess: &Session, conf: &Conf let array_size_threshold = conf.array_size_threshold; store.register_late_pass(move || box large_stack_arrays::LargeStackArrays::new(array_size_threshold)); store.register_early_pass(|| box as_conversions::AsConversions); + store.register_early_pass(|| box utils::internal_lints::ProduceIce); store.register_group(true, "clippy::restriction", Some("clippy_restriction"), vec![ LintId::of(&arithmetic::FLOAT_ARITHMETIC), @@ -1057,6 +1058,7 @@ pub fn register_plugins(store: &mut lint::LintStore, sess: &Session, conf: &Conf LintId::of(&utils::internal_lints::COMPILER_LINT_FUNCTIONS), LintId::of(&utils::internal_lints::LINT_WITHOUT_LINT_PASS), LintId::of(&utils::internal_lints::OUTER_EXPN_EXPN_DATA), + LintId::of(&utils::internal_lints::PRODUCE_ICE), ]); store.register_group(true, "clippy::all", Some("clippy"), vec![ diff --git a/clippy_lints/src/utils/internal_lints.rs b/clippy_lints/src/utils/internal_lints.rs index 5bf105582216..cb75720f1203 100644 --- a/clippy_lints/src/utils/internal_lints.rs +++ b/clippy_lints/src/utils/internal_lints.rs @@ -11,9 +11,11 @@ use rustc::lint::{EarlyContext, EarlyLintPass, LateContext, LateLintPass, LintAr use rustc::{declare_lint_pass, declare_tool_lint, impl_lint_pass}; use rustc_data_structures::fx::{FxHashMap, FxHashSet}; use rustc_errors::Applicability; +use syntax::ast; use syntax::ast::{Crate as AstCrate, ItemKind, Name}; use syntax::source_map::Span; use syntax_pos::symbol::SymbolStr; +use syntax::visit::FnKind; declare_clippy_lint! { /// **What it does:** Checks for various things we like to keep tidy in clippy. @@ -99,6 +101,24 @@ declare_clippy_lint! { "using `cx.outer_expn().expn_data()` instead of `cx.outer_expn_data()`" } +declare_clippy_lint! { + /// **What it does:** Not an actual lint. This lint is only meant for testing our customized internal compiler + /// error message by calling `panic`. + /// + /// **Why is this bad?** ICE in large quantities can damage your teeth + /// + /// **Known problems:** None + /// + /// **Example:** + /// Bad: + /// ```rust,ignore + /// 🍦🍦🍦🍦🍦 + /// ``` + pub PRODUCE_ICE, + internal, + "this message should not appear anywhere as we ICE before and don't emit the lint" +} + declare_lint_pass!(ClippyLintsInternal => [CLIPPY_LINTS_INTERNAL]); impl EarlyLintPass for ClippyLintsInternal { @@ -302,3 +322,22 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for OuterExpnDataPass { } } } + +declare_lint_pass!(ProduceIce => [PRODUCE_ICE]); + +impl EarlyLintPass for ProduceIce { + fn check_fn(&mut self, _: &EarlyContext<'_>, fn_kind: FnKind<'_>, _: &ast::FnDecl, _: Span, _: ast::NodeId) { + if is_trigger_fn(fn_kind) { + panic!("Testing the ICE message"); + } + } +} + +fn is_trigger_fn(fn_kind: FnKind<'_>) -> bool { + match fn_kind { + FnKind::ItemFn(ident, ..) | FnKind::Method(ident, ..) => { + ident.name.as_str() == "should_trigger_an_ice_in_clippy" + }, + FnKind::Closure(..) => false, + } +} diff --git a/src/driver.rs b/src/driver.rs index 0b14e1d5a5ab..76aaae3747b8 100644 --- a/src/driver.rs +++ b/src/driver.rs @@ -5,13 +5,21 @@ // FIXME: switch to something more ergonomic here, once available. // (Currently there is no way to opt into sysroot crates without `extern crate`.) #[allow(unused_extern_crates)] +extern crate rustc; +#[allow(unused_extern_crates)] extern crate rustc_driver; #[allow(unused_extern_crates)] +extern crate rustc_errors; +#[allow(unused_extern_crates)] extern crate rustc_interface; +use rustc::ty::TyCtxt; use rustc_interface::interface; use rustc_tools_util::*; +use lazy_static::lazy_static; +use std::borrow::Cow; +use std::panic; use std::path::{Path, PathBuf}; use std::process::{exit, Command}; @@ -221,9 +229,62 @@ You can use tool lints to allow or deny lints from your code, eg.: ); } +const BUG_REPORT_URL: &str = "https://github.com/rust-lang/rust-clippy/issues/new"; + +lazy_static! { + static ref ICE_HOOK: Box) + Sync + Send + 'static> = { + let hook = panic::take_hook(); + panic::set_hook(Box::new(|info| report_clippy_ice(info, BUG_REPORT_URL))); + hook + }; +} + +fn report_clippy_ice(info: &panic::PanicInfo<'_>, bug_report_url: &str) { + // Invoke our ICE handler, which prints the actual panic message and optionally a backtrace + (*ICE_HOOK)(info); + + // Separate the output with an empty line + eprintln!(); + + let emitter = Box::new(rustc_errors::emitter::EmitterWriter::stderr( + rustc_errors::ColorConfig::Auto, + None, + false, + false, + None, + false, + )); + let handler = rustc_errors::Handler::with_emitter(true, None, emitter); + + // a .span_bug or .bug call has already printed what + // it wants to print. + if !info.payload().is::() { + let d = rustc_errors::Diagnostic::new(rustc_errors::Level::Bug, "unexpected panic"); + handler.emit_diagnostic(&d); + handler.abort_if_errors_and_should_abort(); + } + + let xs: Vec> = vec![ + "the compiler unexpectedly panicked. this is a bug.".into(), + format!("we would appreciate a bug report: {}", bug_report_url).into(), + format!("rustc {}", option_env!("CFG_VERSION").unwrap_or("unknown_version")).into(), + ]; + + for note in &xs { + handler.note_without_error(¬e); + } + + // If backtraces are enabled, also print the query stack + let backtrace = std::env::var_os("RUST_BACKTRACE").map(|x| &x != "0").unwrap_or(false); + + if backtrace { + TyCtxt::try_print_query_stack(); + } +} + pub fn main() { rustc_driver::init_rustc_env_logger(); - rustc_driver::install_ice_hook(); + lazy_static::initialize(&ICE_HOOK); exit( rustc_driver::catch_fatal_errors(move || { use std::env; diff --git a/tests/ui/custom_ice_message.rs b/tests/ui/custom_ice_message.rs new file mode 100644 index 000000000000..8ad5ebba7f3a --- /dev/null +++ b/tests/ui/custom_ice_message.rs @@ -0,0 +1,5 @@ +#![deny(clippy::internal)] + +fn should_trigger_an_ice_in_clippy() {} + +fn main() {} diff --git a/tests/ui/custom_ice_message.stderr b/tests/ui/custom_ice_message.stderr new file mode 100644 index 000000000000..85c9f42a2de3 --- /dev/null +++ b/tests/ui/custom_ice_message.stderr @@ -0,0 +1,11 @@ +thread 'rustc' panicked at 'Testing the ICE message', clippy_lints/src/utils/internal_lints.rs:333:13 +note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace. + +error: internal compiler error: unexpected panic + +note: the compiler unexpectedly panicked. this is a bug. + +note: we would appreciate a bug report: https://github.com/rust-lang/rust-clippy/issues/new + +note: rustc unknown_version + From fc57c84abeee862fea08d823077f56ebab809ea7 Mon Sep 17 00:00:00 2001 From: Philipp Hansch Date: Fri, 27 Sep 2019 07:25:16 +0200 Subject: [PATCH 2/8] Use Clippy version in ICE message --- src/driver.rs | 4 +++- tests/ui/custom_ice_message.stderr | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/driver.rs b/src/driver.rs index 76aaae3747b8..50c821c182d7 100644 --- a/src/driver.rs +++ b/src/driver.rs @@ -264,10 +264,12 @@ fn report_clippy_ice(info: &panic::PanicInfo<'_>, bug_report_url: &str) { handler.abort_if_errors_and_should_abort(); } + let version_info = rustc_tools_util::get_version_info!(); + let xs: Vec> = vec![ "the compiler unexpectedly panicked. this is a bug.".into(), format!("we would appreciate a bug report: {}", bug_report_url).into(), - format!("rustc {}", option_env!("CFG_VERSION").unwrap_or("unknown_version")).into(), + format!("Clippy version: {}", version_info).into(), ]; for note in &xs { diff --git a/tests/ui/custom_ice_message.stderr b/tests/ui/custom_ice_message.stderr index 85c9f42a2de3..cacc41bb5dfd 100644 --- a/tests/ui/custom_ice_message.stderr +++ b/tests/ui/custom_ice_message.stderr @@ -7,5 +7,5 @@ note: the compiler unexpectedly panicked. this is a bug. note: we would appreciate a bug report: https://github.com/rust-lang/rust-clippy/issues/new -note: rustc unknown_version +note: Clippy version: clippy 0.0.212 (68ff8b19 2019-09-26) From 36c6a18217f27491c57b1b078c24f881cde92c57 Mon Sep 17 00:00:00 2001 From: Philipp Hansch Date: Tue, 8 Oct 2019 21:23:57 +0200 Subject: [PATCH 3/8] Update custom ICE function with latest rustc --- src/driver.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/driver.rs b/src/driver.rs index 50c821c182d7..ffa9b663ec42 100644 --- a/src/driver.rs +++ b/src/driver.rs @@ -280,7 +280,7 @@ fn report_clippy_ice(info: &panic::PanicInfo<'_>, bug_report_url: &str) { let backtrace = std::env::var_os("RUST_BACKTRACE").map(|x| &x != "0").unwrap_or(false); if backtrace { - TyCtxt::try_print_query_stack(); + TyCtxt::try_print_query_stack(&handler); } } From 5029dfe403c35e63352f9e1f10eb62ee2851fa90 Mon Sep 17 00:00:00 2001 From: Philipp Hansch Date: Tue, 8 Oct 2019 21:25:10 +0200 Subject: [PATCH 4/8] Use exec_env to set backtrace level and normalize output --- tests/ui/custom_ice_message.rs | 4 ++++ tests/ui/custom_ice_message.stderr | 4 ++-- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/tests/ui/custom_ice_message.rs b/tests/ui/custom_ice_message.rs index 8ad5ebba7f3a..7a5d5ada53d8 100644 --- a/tests/ui/custom_ice_message.rs +++ b/tests/ui/custom_ice_message.rs @@ -1,3 +1,7 @@ +// exec-env:RUST_BACKTRACE=0 +// normalize-stderr-test: "Clippy version: .*" -> "Clippy version: foo" +// normalize-stderr-test: "internal_lints.rs.*" -> "internal_lints.rs:1:1" + #![deny(clippy::internal)] fn should_trigger_an_ice_in_clippy() {} diff --git a/tests/ui/custom_ice_message.stderr b/tests/ui/custom_ice_message.stderr index cacc41bb5dfd..aa2793a4cd4f 100644 --- a/tests/ui/custom_ice_message.stderr +++ b/tests/ui/custom_ice_message.stderr @@ -1,4 +1,4 @@ -thread 'rustc' panicked at 'Testing the ICE message', clippy_lints/src/utils/internal_lints.rs:333:13 +thread 'rustc' panicked at 'Testing the ICE message', clippy_lints/src/utils/internal_lints.rs:1:1 note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace. error: internal compiler error: unexpected panic @@ -7,5 +7,5 @@ note: the compiler unexpectedly panicked. this is a bug. note: we would appreciate a bug report: https://github.com/rust-lang/rust-clippy/issues/new -note: Clippy version: clippy 0.0.212 (68ff8b19 2019-09-26) +note: Clippy version: foo From ad6d8a470048a53eecfa73b32d2470d7dea44a5a Mon Sep 17 00:00:00 2001 From: Philipp Hansch Date: Tue, 8 Oct 2019 21:35:12 +0200 Subject: [PATCH 5/8] Make triggering this lint less likely :paperclip: --- clippy_lints/src/utils/internal_lints.rs | 2 +- tests/ui/custom_ice_message.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/clippy_lints/src/utils/internal_lints.rs b/clippy_lints/src/utils/internal_lints.rs index cb75720f1203..aa62bb461871 100644 --- a/clippy_lints/src/utils/internal_lints.rs +++ b/clippy_lints/src/utils/internal_lints.rs @@ -336,7 +336,7 @@ impl EarlyLintPass for ProduceIce { fn is_trigger_fn(fn_kind: FnKind<'_>) -> bool { match fn_kind { FnKind::ItemFn(ident, ..) | FnKind::Method(ident, ..) => { - ident.name.as_str() == "should_trigger_an_ice_in_clippy" + ident.name.as_str() == "it_looks_like_you_are_trying_to_kill_clippy" }, FnKind::Closure(..) => false, } diff --git a/tests/ui/custom_ice_message.rs b/tests/ui/custom_ice_message.rs index 7a5d5ada53d8..1ceecf38032a 100644 --- a/tests/ui/custom_ice_message.rs +++ b/tests/ui/custom_ice_message.rs @@ -4,6 +4,6 @@ #![deny(clippy::internal)] -fn should_trigger_an_ice_in_clippy() {} +fn it_looks_like_you_are_trying_to_kill_clippy() {} fn main() {} From 21dee8f529442e07df82fe95353d14013ca3e6d4 Mon Sep 17 00:00:00 2001 From: Philipp Hansch Date: Wed, 9 Oct 2019 07:45:05 +0200 Subject: [PATCH 6/8] Use rustc_env instead of exec_env for test --- tests/ui/custom_ice_message.rs | 4 ++-- tests/ui/custom_ice_message.stderr | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/ui/custom_ice_message.rs b/tests/ui/custom_ice_message.rs index 1ceecf38032a..2f58fbce30bf 100644 --- a/tests/ui/custom_ice_message.rs +++ b/tests/ui/custom_ice_message.rs @@ -1,6 +1,6 @@ -// exec-env:RUST_BACKTRACE=0 +// rustc-env:RUST_BACKTRACE=0 // normalize-stderr-test: "Clippy version: .*" -> "Clippy version: foo" -// normalize-stderr-test: "internal_lints.rs.*" -> "internal_lints.rs:1:1" +// normalize-stderr-test: "internal_lints.rs:\d*:\d*" -> "internal_lints.rs" #![deny(clippy::internal)] diff --git a/tests/ui/custom_ice_message.stderr b/tests/ui/custom_ice_message.stderr index aa2793a4cd4f..817e48724337 100644 --- a/tests/ui/custom_ice_message.stderr +++ b/tests/ui/custom_ice_message.stderr @@ -1,4 +1,4 @@ -thread 'rustc' panicked at 'Testing the ICE message', clippy_lints/src/utils/internal_lints.rs:1:1 +thread 'rustc' panicked at 'Testing the ICE message', clippy_lints/src/utils/internal_lints.rs note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace. error: internal compiler error: unexpected panic From 44eec0884d198020e72f2f28b522bcfa16a1830a Mon Sep 17 00:00:00 2001 From: Philipp Hansch Date: Wed, 9 Oct 2019 20:51:45 +0200 Subject: [PATCH 7/8] Feed the dog --- src/driver.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/driver.rs b/src/driver.rs index ffa9b663ec42..fda304afcbe1 100644 --- a/src/driver.rs +++ b/src/driver.rs @@ -277,7 +277,7 @@ fn report_clippy_ice(info: &panic::PanicInfo<'_>, bug_report_url: &str) { } // If backtraces are enabled, also print the query stack - let backtrace = std::env::var_os("RUST_BACKTRACE").map(|x| &x != "0").unwrap_or(false); + let backtrace = std::env::var_os("RUST_BACKTRACE").map_or(false, |x| &x != "0"); if backtrace { TyCtxt::try_print_query_stack(&handler); From c1ccba005ff89a1a3b7972adfde222c593d7851d Mon Sep 17 00:00:00 2001 From: Philipp Hansch Date: Fri, 15 Nov 2019 08:53:21 +0100 Subject: [PATCH 8/8] fmt --- clippy_lints/src/utils/internal_lints.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clippy_lints/src/utils/internal_lints.rs b/clippy_lints/src/utils/internal_lints.rs index aa62bb461871..ea7ea5818cd3 100644 --- a/clippy_lints/src/utils/internal_lints.rs +++ b/clippy_lints/src/utils/internal_lints.rs @@ -14,8 +14,8 @@ use rustc_errors::Applicability; use syntax::ast; use syntax::ast::{Crate as AstCrate, ItemKind, Name}; use syntax::source_map::Span; -use syntax_pos::symbol::SymbolStr; use syntax::visit::FnKind; +use syntax_pos::symbol::SymbolStr; declare_clippy_lint! { /// **What it does:** Checks for various things we like to keep tidy in clippy.