Add allow-by-default lint on implicit ABI in extern function pointers and items #76219
Conversation
Mark-Simulacrum
added
I-nominated
T-lang
|
Nominating for discussion at the next lang team meeting. I suspect we'll want a crater run as well, but I would like CI to pass before kicking off a try run -- I suspect I may have missed some tests and that may need behavioral modifications. |
Mark-Simulacrum
added
S-waiting-on-author
Mark-Simulacrum
force-pushed
the
extern-require-abi
branch
4 times, most recently
from
857c768
to
8f26f25
Compare
Mark-Simulacrum
removed
the
relnotes
Mark-Simulacrum
force-pushed
the
extern-require-abi
branch
5 times, most recently
from
e4eba6b
to
f0c6496
Compare
|
@bors try |
|
⌛ Trying commit f0c6496ddbd277ddbb7ae5f0e687e351441786b2 with merge 05c8870159e4f328578a970e125aa76d24f03b56... |
|
☀️ Try build successful - checks-actions, checks-azure |
|
@craterbot check |
|
👌 Experiment ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more |
craterbot
added
S-waiting-on-crater
|
🚧 Experiment ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more |
|
🚨 Experiment 🆘 Can someone from the infra team check in on this? @rust-lang/infra |
|
@craterbot retry |
|
🛠️ Experiment ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more |
|
🚧 Experiment ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more |
|
🎉 Experiment
|
crlf0710
added
the
S-waiting-on-review
JohnCSimon
added
S-waiting-on-review
|
@Mark-Simulacrum you can rebase and resolve conflicts again we can push this forward |
|
I got confused first by the update of the tests because it made it seem like this was a deny-by-default. I don't see any reason not to merge this. r=me after rebase with the lint as allow-by-default. |
| @@ -297,14 +297,16 @@ impl<'hir> LoweringContext<'_, 'hir> { | |||
| ); | |||
| let sig = hir::FnSig { | |||
| decl, | |||
| header: this.lower_fn_header(header), | |||
| header: this.lower_fn_header(header, span, id), | |||
There was a problem hiding this comment.
I think you might want to use fn_sig_span instead here.
Mark-Simulacrum
force-pushed
the
extern-require-abi
branch
from
6654fc8
to
ba8874e
Compare
|
☔ The latest upstream changes (presumably #80960) made this pull request unmergeable. Please resolve the merge conflicts. |
Mark-Simulacrum
force-pushed
the
extern-require-abi
branch
from
ba8874e
to
4614671
Compare
|
@bors r=estebank rollup=iffy p=1 (prone to merge conflicts) |
|
📌 Commit 4614671 has been approved by |
bors
added
S-waiting-on-bors
|
☀️ Test successful - checks-actions |
This adds a new lint, missing_abi, which lints on omitted ABIs on extern blocks, function declarations, and function pointers.
It is currently not emitting the best possible diagnostics -- we need to track the span of "extern" at least or do some heuristic searching based on the available spans -- but seems good enough for an initial pass than can be expanded in future PRs.
This is a pretty large PR, but mostly due to updating a large number of tests to include ABIs; I can split that into a separate PR if it would be helpful, but test updates are already in dedicated commits.