From 3aff8b338f946728481f8f41311d807893a76514 Mon Sep 17 00:00:00 2001 From: Philipp Hansch Date: Fri, 27 Sep 2019 07:09:12 +0200 Subject: [PATCH 1/6] 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 | 2 +- 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(+), 2 deletions(-) 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 7ab20320e7dc..75e7aeb626e4 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -43,11 +43,11 @@ clippy_lints = { version = "0.0.212", path = "clippy_lints" } regex = "1" semver = "0.9" rustc_tools_util = { version = "0.2.0", path = "rustc_tools_util"} +lazy_static = "1.0" [dev-dependencies] cargo_metadata = "0.8.0" compiletest_rs = { version = "0.3.23", features = ["tmp"] } -lazy_static = "1.0" clippy-mini-macro-test = { version = "0.2", path = "mini-macro" } serde = { version = "1.0", features = ["derive"] } derive-new = "0.5" diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 452e4e9787db..f57f98d276ca 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -447,6 +447,7 @@ pub fn register_plugins(reg: &mut rustc_driver::plugin::Registry<'_>, conf: &Con reg.register_late_lint_pass(box serde_api::SerdeAPI); reg.register_early_lint_pass(box utils::internal_lints::ClippyLintsInternal); + reg.register_early_lint_pass(box utils::internal_lints::ProduceIce); reg.register_late_lint_pass(box utils::internal_lints::CompilerLintFunctions::new()); reg.register_late_lint_pass(box utils::internal_lints::LintWithoutLintPass::default()); reg.register_late_lint_pass(box utils::internal_lints::OuterExpnDataPass); @@ -690,6 +691,7 @@ pub fn register_plugins(reg: &mut rustc_driver::plugin::Registry<'_>, conf: &Con utils::internal_lints::COMPILER_LINT_FUNCTIONS, utils::internal_lints::LINT_WITHOUT_LINT_PASS, utils::internal_lints::OUTER_EXPN_EXPN_DATA, + utils::internal_lints::PRODUCE_ICE, ]); reg.register_lint_group("clippy::all", Some("clippy"), vec![ diff --git a/clippy_lints/src/utils/internal_lints.rs b/clippy_lints/src/utils/internal_lints.rs index 1ec89cb93f17..0ce4ee268274 100644 --- a/clippy_lints/src/utils/internal_lints.rs +++ b/clippy_lints/src/utils/internal_lints.rs @@ -10,8 +10,10 @@ 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::visit::FnKind; use syntax_pos::symbol::LocalInternedString; declare_clippy_lint! { @@ -98,6 +100,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 { @@ -304,3 +324,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 359d2f8530cb..f36878bc0f26 100644 --- a/src/driver.rs +++ b/src/driver.rs @@ -4,13 +4,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}; @@ -245,9 +253,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 bcd02da45abd046c617aca70eef944c22a24c4ce Mon Sep 17 00:00:00 2001 From: Philipp Hansch Date: Fri, 27 Sep 2019 07:25:16 +0200 Subject: [PATCH 2/6] 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 f36878bc0f26..dc69cd43f03d 100644 --- a/src/driver.rs +++ b/src/driver.rs @@ -288,10 +288,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 04133eaf32456e5a3e7e640b3f5e4e6b58826215 Mon Sep 17 00:00:00 2001 From: Philipp Hansch Date: Tue, 8 Oct 2019 21:23:57 +0200 Subject: [PATCH 3/6] 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 dc69cd43f03d..524a31673c4d 100644 --- a/src/driver.rs +++ b/src/driver.rs @@ -304,7 +304,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 5dfb6977862741598dc6637bc3afecd80897ca87 Mon Sep 17 00:00:00 2001 From: Philipp Hansch Date: Tue, 8 Oct 2019 21:25:10 +0200 Subject: [PATCH 4/6] 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 bb4cd3a83cf349595c0a1e161dd156765d65567f Mon Sep 17 00:00:00 2001 From: Philipp Hansch Date: Tue, 8 Oct 2019 21:35:12 +0200 Subject: [PATCH 5/6] 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 0ce4ee268274..e130fe9f1112 100644 --- a/clippy_lints/src/utils/internal_lints.rs +++ b/clippy_lints/src/utils/internal_lints.rs @@ -338,7 +338,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 18149644c9a401f0803237ce83ec31730a99a214 Mon Sep 17 00:00:00 2001 From: Philipp Hansch Date: Wed, 9 Oct 2019 07:45:05 +0200 Subject: [PATCH 6/6] 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