about summary refs log tree commit diff
diff options
context:
space:
mode:
authorZack M. Davis <code@zackmdavis.net>2018-01-15 18:32:18 -0800
committerZack M. Davis <code@zackmdavis.net>2018-01-16 00:31:43 -0800
commit661e03383c99c577db3791e0846e165fe5c21510 (patch)
tree685ae254733200fb71da4ae182344772484c865c
parent79a521bb9a8ace1a6663578a4c409906adde620d (diff)
downloadrust-661e03383c99c577db3791e0846e165fe5c21510.tar.gz
rust-661e03383c99c577db3791e0846e165fe5c21510.zip
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.
-rw-r--r--src/librustc_lint/builtin.rs16
-rw-r--r--src/test/ui/lint/suggestions.rs10
-rw-r--r--src/test/ui/lint/suggestions.stderr32
3 files changed, 42 insertions, 16 deletions
diff --git a/src/librustc_lint/builtin.rs b/src/librustc_lint/builtin.rs
index c5c27c92ab4..de55710bdf3 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 3789b6dfc8b..dfcaede1402 100644
--- a/src/test/ui/lint/suggestions.rs
+++ b/src/test/ui/lint/suggestions.rs
@@ -24,6 +24,16 @@ pub fn defiant<T>(_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 701a9522218..8b30f552d37 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