From 661e03383c99c577db3791e0846e165fe5c21510 Mon Sep 17 00:00:00 2001 From: "Zack M. Davis" Date: Mon, 15 Jan 2018 18:32:18 -0800 Subject: [PATCH] in which the private no-mangle lints receive a valued lesson in humility The incompetent fool who added these suggestions in 38e5a964f2 apparently thought it was safe to assume that, because the offending function or static was unreachable, it would therefore have not have any existing visibility modifiers, making it safe for us to unconditionally suggest inserting `pub`. This isn't true. This resolves #47383. --- src/librustc_lint/builtin.rs | 16 +++++++++------ src/test/ui/lint/suggestions.rs | 10 +++++++++ src/test/ui/lint/suggestions.stderr | 32 ++++++++++++++++++++--------- 3 files changed, 42 insertions(+), 16 deletions(-) diff --git a/src/librustc_lint/builtin.rs b/src/librustc_lint/builtin.rs index c5c27c92ab49a..de55710bdf3d0 100644 --- a/src/librustc_lint/builtin.rs +++ b/src/librustc_lint/builtin.rs @@ -1154,9 +1154,11 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for InvalidNoMangleItems { let msg = "function is marked #[no_mangle], but not exported"; let mut err = cx.struct_span_lint(PRIVATE_NO_MANGLE_FNS, it.span, msg); let insertion_span = it.span.with_hi(it.span.lo()); - err.span_suggestion(insertion_span, - "try making it public", - "pub ".to_owned()); + if it.vis == hir::Visibility::Inherited { + err.span_suggestion(insertion_span, + "try making it public", + "pub ".to_owned()); + } err.emit(); } if generics.is_type_parameterized() { @@ -1177,9 +1179,11 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for InvalidNoMangleItems { let msg = "static is marked #[no_mangle], but not exported"; let mut err = cx.struct_span_lint(PRIVATE_NO_MANGLE_STATICS, it.span, msg); let insertion_span = it.span.with_hi(it.span.lo()); - err.span_suggestion(insertion_span, - "try making it public", - "pub ".to_owned()); + if it.vis == hir::Visibility::Inherited { + err.span_suggestion(insertion_span, + "try making it public", + "pub ".to_owned()); + } err.emit(); } } diff --git a/src/test/ui/lint/suggestions.rs b/src/test/ui/lint/suggestions.rs index 3789b6dfc8b3a..dfcaede1402da 100644 --- a/src/test/ui/lint/suggestions.rs +++ b/src/test/ui/lint/suggestions.rs @@ -24,6 +24,16 @@ pub fn defiant(_t: T) {} fn rio_grande() {} // should suggest `pub` //~^ WARN function is marked +mod badlands { + // The private-no-mangle lints shouldn't suggest inserting `pub` when the + // item is already `pub` (but triggered the lint because, e.g., it's in a + // private module). (Issue #47383) + #[no_mangle] pub static DAUNTLESS: bool = true; + //~^ WARN static is marked + #[no_mangle] pub fn val_jean() {} + //~^ WARN function is marked +} + struct Equinox { warp_factor: f32, } diff --git a/src/test/ui/lint/suggestions.stderr b/src/test/ui/lint/suggestions.stderr index 701a95222183a..8b30f552d3771 100644 --- a/src/test/ui/lint/suggestions.stderr +++ b/src/test/ui/lint/suggestions.stderr @@ -1,7 +1,7 @@ warning: unnecessary parentheses around assigned value - --> $DIR/suggestions.rs:36:21 + --> $DIR/suggestions.rs:46:21 | -36 | let mut a = (1); // should suggest no `mut`, no parens +46 | let mut a = (1); // should suggest no `mut`, no parens | ^^^ help: remove these parentheses | note: lint level defined here @@ -11,17 +11,17 @@ note: lint level defined here | ^^^^^^^^^^^^^ warning: use of deprecated attribute `no_debug`: the `#[no_debug]` attribute was an experimental feature that has been deprecated due to lack of demand. See https://github.com/rust-lang/rust/issues/29721 - --> $DIR/suggestions.rs:31:1 + --> $DIR/suggestions.rs:41:1 | -31 | #[no_debug] // should suggest removal of deprecated attribute +41 | #[no_debug] // should suggest removal of deprecated attribute | ^^^^^^^^^^^ help: remove this attribute | = note: #[warn(deprecated)] on by default warning: variable does not need to be mutable - --> $DIR/suggestions.rs:36:13 + --> $DIR/suggestions.rs:46:13 | -36 | let mut a = (1); // should suggest no `mut`, no parens +46 | let mut a = (1); // should suggest no `mut`, no parens | ---^^ | | | help: remove this `mut` @@ -72,18 +72,30 @@ warning: function is marked #[no_mangle], but not exported | = note: #[warn(private_no_mangle_fns)] on by default +warning: static is marked #[no_mangle], but not exported + --> $DIR/suggestions.rs:31:18 + | +31 | #[no_mangle] pub static DAUNTLESS: bool = true; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +warning: function is marked #[no_mangle], but not exported + --> $DIR/suggestions.rs:33:18 + | +33 | #[no_mangle] pub fn val_jean() {} + | ^^^^^^^^^^^^^^^^^^^^ + warning: denote infinite loops with `loop { ... }` - --> $DIR/suggestions.rs:34:5 + --> $DIR/suggestions.rs:44:5 | -34 | while true { // should suggest `loop` +44 | while true { // should suggest `loop` | ^^^^^^^^^^ help: use `loop` | = note: #[warn(while_true)] on by default warning: the `warp_factor:` in this pattern is redundant - --> $DIR/suggestions.rs:41:23 + --> $DIR/suggestions.rs:51:23 | -41 | Equinox { warp_factor: warp_factor } => {} // should suggest shorthand +51 | Equinox { warp_factor: warp_factor } => {} // should suggest shorthand | ------------^^^^^^^^^^^^ | | | help: remove this