Add allow-by-default lint on implicit ABI in extern function pointers and items by Mark-Simulacrum · Pull Request #76219 · rust-lang/rust

Mark-Simulacrum · 2020-09-01T22:01:08Z

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.

Mark-Simulacrum · 2020-09-01T22:04:55Z

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 · 2020-09-02T14:50:38Z

@bors try

bors · 2020-09-02T15:23:37Z

⌛ Trying commit f0c6496ddbd277ddbb7ae5f0e687e351441786b2 with merge 05c8870159e4f328578a970e125aa76d24f03b56...

bors · 2020-09-02T16:08:24Z

☀️ Try build successful - checks-actions, checks-azure
Build commit: 05c8870159e4f328578a970e125aa76d24f03b56 (05c8870159e4f328578a970e125aa76d24f03b56)

Mark-Simulacrum · 2020-09-02T16:36:43Z

@craterbot check

craterbot · 2020-09-02T16:36:47Z

👌 Experiment pr-76219 created and queued.
🤖 Automatically detected try build 05c8870159e4f328578a970e125aa76d24f03b56
🔍 You can check out the queue and this experiment's details.

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

craterbot · 2020-09-09T09:22:37Z

🚧 Experiment pr-76219 is now running

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

craterbot · 2020-09-12T00:55:48Z

🚨 Experiment pr-76219 has encountered an error: some threads returned an error
🛠️ If the error is fixed use the retry command.

🆘 Can someone from the infra team check in on this? @rust-lang/infra
ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

Mark-Simulacrum · 2020-09-12T01:20:45Z

@craterbot retry

craterbot · 2020-09-12T01:20:46Z

🛠️ Experiment pr-76219 queued again.

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

craterbot · 2020-09-12T01:52:20Z

🚧 Experiment pr-76219 is now running

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

craterbot · 2020-09-13T00:04:16Z

🎉 Experiment pr-76219 is completed!
📊 695 regressed and 1 fixed (120140 total)
📰 Open the full report.

⚠️ If you notice any spurious failure please add them to the blacklist!
ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

Dylan-DPC-zz · 2021-01-13T01:30:00Z

@Mark-Simulacrum you can rebase and resolve conflicts again we can push this forward

estebank · 2021-01-13T02:11:02Z

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.

estebank · 2021-01-13T02:16:14Z

compiler/rustc_ast_lowering/src/item.rs

I think you might want to use fn_sig_span instead here.

bors · 2021-01-13T07:43:22Z

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

Mark-Simulacrum · 2021-01-13T13:01:24Z

@bors r=estebank rollup=iffy p=1 (prone to merge conflicts)

bors · 2021-01-13T13:01:26Z

📌 Commit 4614671 has been approved by estebank

bors · 2021-01-13T13:56:20Z

⌛ Testing commit 4614671 with merge fd2df74...

bors · 2021-01-13T17:15:59Z

☀️ Test successful - checks-actions
Approved by: estebank
Pushing fd2df74 to master...

Mark-Simulacrum added I-nominated T-lang Relevant to the language team labels Sep 1, 2020

Mark-Simulacrum mentioned this pull request Sep 1, 2020

Require ABI for extern in 2021 rust-lang/lang-team#46

Closed

Mark-Simulacrum added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. relnotes Marks issues that should be documented in the release notes of the next release. labels Sep 1, 2020

Mark-Simulacrum force-pushed the extern-require-abi branch 4 times, most recently from 857c768 to 8f26f25 Compare September 2, 2020 12:28

Mark-Simulacrum removed the relnotes Marks issues that should be documented in the release notes of the next release. label Sep 2, 2020

Mark-Simulacrum force-pushed the extern-require-abi branch 5 times, most recently from e4eba6b to f0c6496 Compare September 2, 2020 14:04

craterbot added S-waiting-on-crater Status: Waiting on a crater run to be completed. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Sep 2, 2020

crlf0710 added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Dec 11, 2020

JohnCSimon added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 11, 2021

estebank reviewed Jan 13, 2021

View reviewed changes

compiler/rustc_ast_lowering/src/item.rs Outdated

Copy link

Contributor

estebank Jan 13, 2021

Choose a reason for hiding this comment

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

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 January 13, 2021 03:12

Mark-Simulacrum added 4 commits January 13, 2021 07:49

Introduce missing ABI lint on extern blocks

c4a8d7f

Update tests for extern block linting

8a3edb1

Lint on extern in item declaration position

920b5ee

Update code to account for extern ABI requirement

4614671

Mark-Simulacrum force-pushed the extern-require-abi branch from ba8874e to 4614671 Compare January 13, 2021 12:50

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 13, 2021

bors added the merged-by-bors This PR was explicitly merged by bors. label Jan 13, 2021

bors merged commit fd2df74 into rust-lang:master Jan 13, 2021

rustbot added this to the 1.51.0 milestone Jan 13, 2021

This was referenced Jan 13, 2021

Detect async visibility wrong order, async pub #76447

Merged

Implement RFC 2945: "C-unwind" ABI #76570

Merged

path trimming: ignore type aliases #78052

Merged

Add SEMICOLON_IN_EXPRESSIONS_FROM_MACROS lint #79819

Merged

Mark-Simulacrum deleted the extern-require-abi branch January 13, 2021 17:41

This was referenced Jan 13, 2021

Add lint for 2229 migrations #80629

Merged

Move some tests to more reasonable directories - 2 #80942

Merged

maijun-sec mentioned this pull request Nov 7, 2024

Force explicit abi rust-lang/rust-clippy#13372

Closed

Uh oh!

Conversation

Mark-Simulacrum commented Sep 1, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Mark-Simulacrum commented Sep 1, 2020

Uh oh!

Mark-Simulacrum commented Sep 2, 2020

Uh oh!

bors commented Sep 2, 2020

Uh oh!

bors commented Sep 2, 2020

Uh oh!

Mark-Simulacrum commented Sep 2, 2020

Uh oh!

craterbot commented Sep 2, 2020

Uh oh!

craterbot commented Sep 9, 2020

Uh oh!

craterbot commented Sep 12, 2020

Uh oh!

Mark-Simulacrum commented Sep 12, 2020

Uh oh!

craterbot commented Sep 12, 2020

Uh oh!

craterbot commented Sep 12, 2020

Uh oh!

craterbot commented Sep 13, 2020

Uh oh!

Dylan-DPC-zz commented Jan 13, 2021

Uh oh!

estebank commented Jan 13, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

estebank Jan 13, 2021

Choose a reason for hiding this comment

Uh oh!

bors commented Jan 13, 2021

Uh oh!

Mark-Simulacrum commented Jan 13, 2021

Uh oh!

bors commented Jan 13, 2021

Uh oh!

bors commented Jan 13, 2021

Uh oh!

bors commented Jan 13, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

Mark-Simulacrum commented Sep 1, 2020 •

edited

Loading

estebank commented Jan 13, 2021 •

edited

Loading