Skip to content

Commit

Permalink
Rollup merge of rust-lang#87965 - m-ou-se:non-fmt-panic-external, r=e…
Browse files Browse the repository at this point in the history
…stebank

Silence non_fmt_panic from external macros.

This stops the non_fmt_panic lint from triggering if a macro from another crate is entirely responsible. In those cases there's nothing that the current crate can/should do.

See also rust-lang#87621 (comment)
  • Loading branch information
Manishearth authored Aug 12, 2021
2 parents 0c4e37a + a6da55c commit 2d27976
Show file tree
Hide file tree
Showing 4 changed files with 19 additions and 14 deletions.
15 changes: 13 additions & 2 deletions compiler/rustc_lint/src/non_fmt_panic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ use crate::{LateContext, LateLintPass, LintContext};
use rustc_ast as ast;
use rustc_errors::{pluralize, Applicability};
use rustc_hir as hir;
use rustc_middle::lint::in_external_macro;
use rustc_middle::ty;
use rustc_parse_format::{ParseMode, Parser, Piece};
use rustc_session::lint::FutureIncompatibilityReason;
Expand Down Expand Up @@ -75,6 +76,11 @@ fn check_panic<'tcx>(cx: &LateContext<'tcx>, f: &'tcx hir::Expr<'tcx>, arg: &'tc

let (span, panic, symbol_str) = panic_call(cx, f);

if in_external_macro(cx.sess(), span) {
// Nothing that can be done about it in the current crate.
return;
}

// Find the span of the argument to `panic!()`, before expansion in the
// case of `panic!(some_macro!())`.
// We don't use source_callsite(), because this `panic!(..)` might itself
Expand Down Expand Up @@ -152,6 +158,13 @@ fn check_panic_str<'tcx>(
return;
}

let (span, _, _) = panic_call(cx, f);

if in_external_macro(cx.sess(), span) && in_external_macro(cx.sess(), arg.span) {
// Nothing that can be done about it in the current crate.
return;
}

let fmt_span = arg.span.source_callsite();

let (snippet, style) = match cx.sess().parse_sess.source_map().span_to_snippet(fmt_span) {
Expand All @@ -167,8 +180,6 @@ fn check_panic_str<'tcx>(
Parser::new(fmt.as_ref(), style, snippet.clone(), false, ParseMode::Format);
let n_arguments = (&mut fmt_parser).filter(|a| matches!(a, Piece::NextArgument(_))).count();

let (span, _, _) = panic_call(cx, f);

if n_arguments > 0 && fmt_parser.errors.is_empty() {
let arg_spans: Vec<_> = match &fmt_parser.arg_places[..] {
[] => vec![fmt_span],
Expand Down
3 changes: 3 additions & 0 deletions src/test/ui/auxiliary/fancy-panic.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
#[macro_export]
macro_rules! fancy_panic {
() => {
panic!("{}");
};
($msg:expr) => {
panic!($msg)
};
Expand Down
4 changes: 2 additions & 2 deletions src/test/ui/non-fmt-panic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,8 @@ fn main() {
fancy_panic::fancy_panic!("test {} 123");
//~^ WARN panic message contains an unused formatting placeholder

fancy_panic::fancy_panic!(S);
//~^ WARN panic message is not a string literal
fancy_panic::fancy_panic!(); // OK
fancy_panic::fancy_panic!(S); // OK

macro_rules! a {
() => { 123 };
Expand Down
11 changes: 1 addition & 10 deletions src/test/ui/non-fmt-panic.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -180,15 +180,6 @@ LL | fancy_panic::fancy_panic!("test {} 123");
|
= note: this message is not used as a format string when given without arguments, but will be in Rust 2021

warning: panic message is not a string literal
--> $DIR/non-fmt-panic.rs:29:31
|
LL | fancy_panic::fancy_panic!(S);
| ^
|
= note: this usage of panic!() is deprecated; it will be a hard error in Rust 2021
= note: for more information, see <https://doc.rust-lang.org/nightly/edition-guide/rust-2021/panic-macro-consistency.html>

warning: panic message is not a string literal
--> $DIR/non-fmt-panic.rs:36:12
|
Expand Down Expand Up @@ -285,5 +276,5 @@ help: or use std::panic::panic_any instead
LL | std::panic::panic_any(123);
| ~~~~~~~~~~~~~~~~~~~~~~ ~

warning: 20 warnings emitted
warning: 19 warnings emitted

0 comments on commit 2d27976

Please sign in to comment.