Emit specific warning to clarify that #[no_mangle] should not be applied on foreign statics or functions

[asquared31415]

Foreign statics and foreign functions should not have #[no_mangle] applied, as it does nothing to the name and has some extra hidden behavior that is normally unwanted. There was an existing warning for this, but it says the attribute is only allowed on "statics or functions", which to the user can be confusing.

This PR adds a specific version of the unused #[no_mangle] warning that explains that the target is a foreign static or function and that they do not need the attribute.

Fixes #78989

[rust-highfive]

r? @matthewjasper

(rust-highfive has picked a reviewer for you, use r? to override)

[Mark-Simulacrum]

Can you say more about "may have additional undesired exporting effects"? I would naively expect that there is no additional behavior on extern statics/functions with no_mangle applied, in which case dropping that seems reasonable.

I don't think we should indicate an attribute is unused if it actually has a semantic effect (which the new diagnostic implies).

[asquared31415]

This PR has two goals.

1. To clarify that a fn or static within a foreign extern block are not the same as a fn or static outside and clarify that because of this they may not have #[no_mangle] applied.
2. To explain to experienced FFI users why the attribute is entirely redundant for its stated purpose, and why its hidden effects on normal fns and statics (which do not happen for foreign items) don't make sense to apply.

I believe that this PR accomplishes goal 1 well with the current error message ( `#[no_mangle]` should not be applied to a foreign [static | fn]), the span that suggests removal of the attribute, and the span explaining that the item is a foreign [static | fn].
The PR currently attempts to accomplish number two via the help text, but it is admittedly a little unclear, and could confuse users that just needed part 1 and didn't need a more in depth explanation.

Attached is a file that has examples of the current behavior of #[no_mangle] on non-extern fns and statics. It can be summarized as follows:

• Without #[no_mangle], functions and statics are never exported and always mangled.
• With #[no_mangle], functions and statics are always exported and never mangled.

To the best of my knowledge, the behavior of #[no_mangle] causing symbols to be made global (regardless of rust's visibility rules) is not well explained except for some discussions on IRLO and other blog posts that explore the behavior.
extern blocks already use the symbol name as declared and import those symbols. It does not make sense for an already un-mangled symbol declaration to be un-mangled again. It also does not make sense for those imports to additionally be exported by #[no_mangle]'s effects (and I would expect possible linker errors if they were).

I think that in attempting to clarify that the hidden effects make no sense and do not happen, I have made the help text for the lint more confusing. It definitely needs a rewording, but I would ideally like to keep something to the same effect there for the power users that understand the full implications of how #[no_mangle] works but may not be aware that it doesn't work on foreign items.

[asquared31415]

Ah the file I referenced did not attach, github didn't like the extension, here it is:
main.txt

[Mark-Simulacrum]

Hm,I believe that no_mangle does not reexport extern static/fns (which are in some sense imports, as you say), and so no_mangle has no effect whatsoever on them, right?

The warning text in this PR is:

warning: `#[no_mangle]` should not be applied to 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: foreign symbol names are always preserved and the `#[no_mangle]` attribute may have additional undesired exporting effects.

I would suggest changing the main message and the note text to something like the following, presuming that it is accurate and makes sense in your eyes as well?

warning: `#[no_mangle]` has no effect on foreign statics
  --> $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 by default

[asquared31415]

That updated text looks good.

With regards to exporting, I suppose what I was trying to do was contrast the behavior on foreign items with the behavior on everything else and explain why that is unintuitive. However since it #[no_mangle] on foreign items is a no-op and doesn't make sense anyway because they don't use name mangling, it isn't really needed.

[Mark-Simulacrum]

@bors r+ rollup

Thanks! This looks good to me. I decided to skip a check in with T-lang since this seems like an obvious bug fix and we're just adding a lint here, so we can easily back this out if it shows up more often than expected or there's concerns raised at any point.

[bors]

📌 Commit fc125a5 has been approved by Mark-Simulacrum