Add new lint no_mangle_with_rust_abi #10369
Conversation
|
r? @llogiq (rustbot has picked a reviewer for you, use r? to override) |
rustbot
added
the
S-waiting-on-review
nindalf
changed the title
nindalf
force-pushed
the
no_mangle_lint
branch
2 times, most recently
from
2dd9898
to
eaafb8e
Compare
| use rustc_session::{declare_lint_pass, declare_tool_lint}; | ||
| use rustc_target::spec::abi::Abi; | ||
|
|
||
| declare_clippy_lint! { |
There was a problem hiding this comment.
Does it make sense to move this to src/attrs.rs or let it remain in it's own file?
There was a problem hiding this comment.
Feel free to leave in its own file.
| span, | ||
| &format!("attribute #[no_mangle] set on a Rust ABI function"), | ||
| "try", | ||
| format!(r#"extern "C" {}"#, snippet_with_applicability(cx, span, "..", &mut applicability)), |
There was a problem hiding this comment.
@llogiq I'm not sure about the best way to accomplish this. I have the entire span available here - pub fn example(), which needs to be modified to pub extern "C" fn example(). Is there an elegant way to do this that doesn't involve string splitting?
I didn't find anything relevant in the Span docs.
There was a problem hiding this comment.
Ended up using String::split_once("fn")
nindalf
force-pushed
the
no_mangle_lint
branch
3 times, most recently
from
b845985
to
46c251b
Compare
| @@ -84,7 +84,7 @@ pub unsafe fn mutates_static() -> usize { | |||
| } | |||
|
|
|||
| #[no_mangle] | |||
| pub fn unmangled(i: bool) -> bool { | |||
| pub extern "C" fn unmangled(i: bool) -> bool { | |||
There was a problem hiding this comment.
Just applying this lint. Helps us avoid adding noise to tests/ui/must_use_candidates.fixed
|
|
||
| let mut applicability = Applicability::MachineApplicable; | ||
| let snippet = snippet_with_applicability(cx, fn_sig.span, "..", &mut applicability); | ||
| let suggestion = snippet.split_once("fn") |
There was a problem hiding this comment.
Is using String::split_once acceptable?
nindalf
force-pushed
the
no_mangle_lint
branch
from
46c251b
to
d7bf2d4
Compare
nindalf
changed the title
nindalf
force-pushed
the
no_mangle_lint
branch
2 times, most recently
from
36ded2a
to
07922c0
Compare
nindalf
changed the title
#[no_mangle] attribute to C ABI
nindalf
changed the title
#[no_mangle] attribute to C ABI
nindalf
force-pushed
the
no_mangle_lint
branch
2 times, most recently
from
38e3a15
to
2b431ca
Compare
| if let Some(ident) = attr.ident() | ||
| && ident.name == rustc_span::sym::no_mangle | ||
| && fn_sig.header.abi == Abi::Rust | ||
| && !snippet.contains("extern"){ |
There was a problem hiding this comment.
Is there a better way to do this that doesn't involve string search? Can we search on the span directly? I wasn't able to find this information in the Function Header.
nindalf
force-pushed
the
no_mangle_lint
branch
3 times, most recently
from
dc1a37b
to
d7a8d17
Compare
|
☔ The latest upstream changes (presumably #10392) made this pull request unmergeable. Please resolve the merge conflicts. |
nindalf
force-pushed
the
no_mangle_lint
branch
2 times, most recently
from
7bbff8d
to
ee64b43
Compare
nindalf
force-pushed
the
no_mangle_lint
branch
from
ee64b43
to
00c294a
Compare
|
Thank you for writing this lint! Sorry it took me so long to review, I had a lot of stuff on my desk after returning from London. @bors r+ |
|
☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test |
Fixes issue #10347
This PR adds a new lint
no_mangle_with_rust_abithat suggests converting a function to the C ABI to if the function has the#[no_mangle]attribute andAbi == 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.changelog: [
no_mangle_with_rust_abi]: add lint that converts Rust ABI functions with the#[no_mangle]attribute to C ABI