Lint against #[no_mangle] with the rust ABI

[bjorn3]

What it does

The lint should lint against #[no_mangle] on functions with the rust abi. See Rust-for-Linux/linux#967 for an example of this kind of issue.

Lint Name

no_mangle_with_rust_abi

Category

No response

Advantage

The Rust ABI is not stable, but in many simple cases matches enough with the C ABI that it is possible to forget to add extern "C" to a function called from C. Changes to the Rust ABI can break this at any point.

Drawbacks

No response

Example

#[no_mangle]
fn foo() {}

Should instead be written as:

#[no_mangle]
extern "C" fn foo() {}

[nindalf]

@rustbot claim

[nindalf]

@bjorn3 I've implemented this is in a PR that's waiting on review. This is what the lint does

• iff the function has no_mangle AND Abi == Abi::Rust, suggest adding extern "C" to it.
• It will not run for any of the other variants defined rustc_target::spec::abi::Abi, nor suggest any conversion other than conversion to the C ABI.

My question to you is - is it possible to have false positives on this lint, to your knowledge? Is there some possible use case for adding #[no_mangle] to a Rust ABI function?

One discussion I found on internals.rust-lang.org suggests there aren't many use cases.

[bjorn3]

I don't think there are many use cases. Maybe you could make it such that using an explicit extern "Rust" doesn't give a warning, but omitting extern entirely does? This way you can suppress the lint using extern "Rust" and at the same time indicate that this was what you actually meant.

[nindalf]

I've added an exception for extern "Rust". Suppressing this lint #[allow(no_mangle_with_rust_abi)] on an item or the whole crate is another option users have.

If we can't think of any use cases for using #[no_mangle] on a Rust ABI function, I'm leaning towards this lint being opt-out rather than opt-in.

Probably a call for the clippy maintainers to take. @llogiq does that seem fine?

[llogiq]

I'm not completely clear on the semantics here: Doesn't the ABI default to Rust, thus making no ABI specification equal to extern "Rust"? If that's right, I don't see the benefit of this lint in the general case, and it should probably be allow by default.

[bjorn3]

When using #[no_mangle] you almost always intended it to be called from C and thus must not use the Rust abi. If you did actually mean to use the Rust abi, writing an explicit extern "Rust" helps clarify this.

[ojeda]

If we can't think of any use cases for using #[no_mangle] on a Rust ABI function

The __rust_* functions are one such case in Rust for Linux, but we had to declare them due to the fact that some crate types did not generate them (i.e. maybe that changes, and therefore we could remove them and so we may not have such a case anymore) -- Rust-for-Linux/linux#967 (comment).

[bjorn3]

Note the real versions of __rg_*, __rdl_* and __rust_* are actually #[rustc_std_internal_symbol] rather than #[no_mangle]. Both attributes are identical except that #[rustc_std_internal_symbol] isn't exported from cdylib's unlike #[no_mangle]. Also having to define those __rust_* functions will hopefully go away soon.

[nindalf]

Copy pasting what the current impl of the lint does. Let me know if this works for everyone

This PR adds a new lint no_mangle_with_rust_abi that suggests converting a function to the C ABI to if the function has the #[no_mangle] attribute and Abi == Abi::Rust. It will not run for any of the other variants defined in rustc_target::spec::abi::Abi, nor suggest any conversion other than conversion to the C ABI.

Functions that explicitly opt into the Rust ABI with extern "Rust" are ignored by this lint.

[ojeda]

If that's right, I don't see the benefit of this lint in the general case, and it should probably be allow by default.

The problem is forgetting extern "C" when having written #[no_mangle].

Perhaps somebody has a common enough use case for such combination, but it would seem that the lint should be warn or deny by default (not sure what is the threshold/policy for those in Clippy), especially if the explicit extern "Rust" escape hatch is provided.

That is, projects that really use it that way can either use the escape hatch or simply allow it as needed (which are good things, because they inform the reader that was intentional), but everyone else should be informed about the (very likely) mistake they made (i.e. either assuming #[no_mangle] implied C ABI or simply forgetting to write it even if they knew).