From fc125a52ec3a342c91ca399cafaeb0504a253991 Mon Sep 17 00:00:00 2001
From: asquared31415 <34665709+asquared31415@users.noreply.github.com>
Date: Sun, 29 Aug 2021 20:08:40 -0400
Subject: [PATCH] emit specific warning to clarify that foreign items can't
 have no_mangle remove extra commented code Deduplicate some diagnostics code
 add code symbols, machine applicable suggestion clarify error message

---
 compiler/rustc_passes/src/check_attr.rs    | 30 ++++++++++++++++
 src/test/ui/extern/extern-no-mangle.rs     | 30 ++++++++++++++++
 src/test/ui/extern/extern-no-mangle.stderr | 42 ++++++++++++++++++++++
 3 files changed, 102 insertions(+)
 create mode 100644 src/test/ui/extern/extern-no-mangle.rs
 create mode 100644 src/test/ui/extern/extern-no-mangle.stderr

diff --git a/compiler/rustc_passes/src/check_attr.rs b/compiler/rustc_passes/src/check_attr.rs
index d3dac35d2c9e5..fd438bdc9005a 100644
--- a/compiler/rustc_passes/src/check_attr.rs
+++ b/compiler/rustc_passes/src/check_attr.rs
@@ -1331,6 +1331,36 @@ impl CheckAttrVisitor<'tcx> {
             Target::Field | Target::Arm | Target::MacroDef => {
                 self.inline_attr_str_error_with_macro_def(hir_id, attr, "no_mangle");
             }
+            // FIXME: #[no_mangle] was previously allowed on non-functions/statics, this should be an error
+            // The error should specify that the item that is wrong is specifically a *foreign* fn/static
+            // otherwise the error seems odd
+            Target::ForeignFn | Target::ForeignStatic => {
+                let foreign_item_kind = match target {
+                    Target::ForeignFn => "function",
+                    Target::ForeignStatic => "static",
+                    _ => unreachable!(),
+                };
+                self.tcx.struct_span_lint_hir(UNUSED_ATTRIBUTES, hir_id, attr.span, |lint| {
+                    lint.build(&format!(
+                        "`#[no_mangle]` has no effect on a foreign {}",
+                        foreign_item_kind
+                    ))
+                    .warn(
+                        "this was previously accepted by the compiler but is \
+                            being phased out; it will become a hard error in \
+                            a future release!",
+                    )
+                    .span_label(*span, format!("foreign {}", foreign_item_kind))
+                    .note("symbol names in extern blocks are not mangled")
+                    .span_suggestion(
+                        attr.span,
+                        "remove this attribute",
+                        String::new(),
+                        Applicability::MachineApplicable,
+                    )
+                    .emit();
+                });
+            }
             _ => {
                 // FIXME: #[no_mangle] was previously allowed on non-functions/statics and some
                 // crates used this, so only emit a warning.
diff --git a/src/test/ui/extern/extern-no-mangle.rs b/src/test/ui/extern/extern-no-mangle.rs
new file mode 100644
index 0000000000000..ab7c9824af039
--- /dev/null
+++ b/src/test/ui/extern/extern-no-mangle.rs
@@ -0,0 +1,30 @@
+#![warn(unused_attributes)]
+
+// Tests that placing the #[no_mangle] attribute on a foreign fn or static emits
+// a specialized warning.
+// The previous warning only talks about a "function or static" but foreign fns/statics
+// are also not allowed to have #[no_mangle]
+
+// build-pass
+
+extern "C" {
+    #[no_mangle]
+    //~^ WARNING `#[no_mangle]` has no effect on a foreign static
+    //~^^ WARNING this was previously accepted by the compiler
+    pub static FOO: u8;
+
+    #[no_mangle]
+    //~^ WARNING `#[no_mangle]` has no effect on a foreign function
+    //~^^ WARNING this was previously accepted by the compiler
+    pub fn bar();
+}
+
+fn no_new_warn() {
+    // Should emit the generic "not a function or static" warning
+    #[no_mangle]
+    //~^ WARNING attribute should be applied to a free function, impl method or static
+    //~^^ WARNING this was previously accepted by the compiler
+    let x = 0_u8;
+}
+
+fn main() {}
diff --git a/src/test/ui/extern/extern-no-mangle.stderr b/src/test/ui/extern/extern-no-mangle.stderr
new file mode 100644
index 0000000000000..b56428141141d
--- /dev/null
+++ b/src/test/ui/extern/extern-no-mangle.stderr
@@ -0,0 +1,42 @@
+warning: attribute should be applied to a free function, impl method or static
+  --> $DIR/extern-no-mangle.rs:24:5
+   |
+LL |     #[no_mangle]
+   |     ^^^^^^^^^^^^
+...
+LL |     let x = 0_u8;
+   |     ------------- not a free function, impl method or static
+   |
+note: the lint level is defined here
+  --> $DIR/extern-no-mangle.rs:1:9
+   |
+LL | #![warn(unused_attributes)]
+   |         ^^^^^^^^^^^^^^^^^
+   = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
+
+warning: `#[no_mangle]` has no effect on a foreign static
+  --> $DIR/extern-no-mangle.rs:11:5
+   |
+LL |     #[no_mangle]
+   |     ^^^^^^^^^^^^ help: remove this attribute
+...
+LL |     pub static FOO: u8;
+   |     ------------------- foreign static
+   |
+   = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
+   = note: symbol names in extern blocks are not mangled
+
+warning: `#[no_mangle]` has no effect on a foreign function
+  --> $DIR/extern-no-mangle.rs:16:5
+   |
+LL |     #[no_mangle]
+   |     ^^^^^^^^^^^^ help: remove this attribute
+...
+LL |     pub fn bar();
+   |     ------------- foreign function
+   |
+   = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
+   = note: symbol names in extern blocks are not mangled
+
+warning: 3 warnings emitted
+