Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add [extern_without_abi] lint #13404

Closed
wants to merge 1 commit into from

Conversation

CBSpeir
Copy link
Contributor

@CBSpeir CBSpeir commented Sep 16, 2024

Fixes #13372

The [extern_without_abi] lint emits when extern is not followed by an ABI. For example:

// EMIT
extern fn foo() {}

// EMIT
extern {
    fn bar();
}

// NO EMIT
extern "C" fn baz() {}

// NO EMIT
extern "C" {
    fn foo_bar();
}

changelog: Add [extern_without_abi] lint

@rustbot
Copy link
Collaborator

rustbot commented Sep 16, 2024

r? @Centri3

rustbot has assigned @Centri3.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Sep 16, 2024
Copy link
Member

@Centri3 Centri3 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall looks good to me 👍

clippy_lints/src/extern_without_abi.rs Outdated Show resolved Hide resolved
clippy_lints/src/extern_without_abi.rs Outdated Show resolved Hide resolved
clippy_lints/src/extern_without_abi.rs Outdated Show resolved Hide resolved
clippy_lints/src/extern_without_abi.rs Outdated Show resolved Hide resolved
tests/ui/extern_without_abi.rs Outdated Show resolved Hide resolved
@bors
Copy link
Contributor

bors commented Sep 22, 2024

☔ The latest upstream changes (presumably #13322) made this pull request unmergeable. Please resolve the merge conflicts.

@bors
Copy link
Contributor

bors commented Sep 22, 2024

☔ The latest upstream changes (presumably #13440) made this pull request unmergeable. Please resolve the merge conflicts.

@CBSpeir
Copy link
Contributor Author

CBSpeir commented Sep 23, 2024

Thank you for reviewing the PR. It should include the changes you requested. I went ahead and squashed and merged the changes with the latest from master.

@bors
Copy link
Contributor

bors commented Oct 13, 2024

☔ The latest upstream changes (presumably #13334) made this pull request unmergeable. Please resolve the merge conflicts.

@CBSpeir CBSpeir force-pushed the extern-without-abi branch from 515cc2b to 7df214f Compare October 13, 2024 22:02
@bors
Copy link
Contributor

bors commented Oct 15, 2024

☔ The latest upstream changes (presumably #13395) made this pull request unmergeable. Please resolve the merge conflicts.

@CBSpeir CBSpeir force-pushed the extern-without-abi branch from 7df214f to f81518a Compare October 15, 2024 17:24
@CBSpeir CBSpeir force-pushed the extern-without-abi branch from f81518a to b7c4490 Compare October 15, 2024 17:50
@CBSpeir
Copy link
Contributor Author

CBSpeir commented Oct 15, 2024

In addition to fixing the merge conflicts, I moved is_from_proc_macro test to last since it seems to be the most expensive. Also made a minor change to a string literal to improve readability.

@y21
Copy link
Member

y21 commented Oct 31, 2024

There's the missing_abi rustc lint that sounds like the same as what this adds, or is there a difference? rust-lang/rust#132397 makes that rustc lint warn-by-default

@CBSpeir
Copy link
Contributor Author

CBSpeir commented Nov 1, 2024

I guess missing_abi already checks for this, and therefore this PR isn't needed.

There is one difference, this lint emits a diagnostic with a suggestion. I don't believe missing_abi does.

@CBSpeir CBSpeir closed this Nov 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Force explicit abi
5 participants