[RFC] Explicit ABI in extern by m-ou-se · Pull Request #3722 · rust-lang/rfcs (original) (raw)
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
[ Show hidden characters]({{ revealButtonHref }})
jdonszelmann, samueltardieu, golddranks, Expurple, kennytm, jieyouxu, repnop, RustyYato, scottmcm, tgross35, and 18 more reacted with thumbs up emoji zopsicle reacted with thumbs down emoji jdonszelmann and GrayJack reacted with hooray emoji joshtriplett, burdges, bjorn3, jeffparsons, GrayJack, and Ltrlg reacted with heart emoji joshtriplett and GrayJack reacted with eyes emoji
Relevant to the language team, which will review and decide on the RFC.
Proposals relating to defaults / provided definitions
Indicates that an issue has been nominated for prioritizing at the next lang team meeting.
Area: The 2024 edition
Changes of edition-relevance not targeted for a specific edition.
Relevant to the edition team, which will review and decide on this RFC.
labels
Very much a fan of making this change. As noted in the RFC, it's very late for the 2024 edition. That said, I would personally not object to trying for 2024 if we can get the necessary migration lint in place.
One way or another, I personally think T-lang should approve this RFC, with the guidance of "whenever it's ready", and defer to those wrangling the edition to decide whether this happens in 2024 or 2027.
@rustbot labels -A-edition-2024
This makes sense to me as a language matter also.
With the edition hat on, though, and after consulting with the rest of the edition team, it's unfortunately just too late to accept a new language item like this for Rust 2024. This is due to the testing and release timeline, where we are in it, and how these kind of changes affect that.
Let's make sure it makes the next edition, assuming the rest of the lang team likes this also.
Relevant to the edition team, which will review and decide on this RFC.
label
The lang team discussed this in our meeting today and everyone was in favor of the change. Since it's too late for 2024 Edition, this RFC is not time sensitive, but we might as well approve it for the next edition.
Meanwhile, there is an existing missing_abi lint that we should upgrade to warn-by-default in all editions. We should do that in a separate FCP. (There was some discussion of rolling this out starting with 2024 – but that should not happen as part of the initial edition release, or it would fall under the testing and documentation requirements that we already said it was too late to do for 2024.)
@rfcbot fcp merge
Team member @tmandry has proposed to merge this. The next step is review by the rest of the tagged team members:
No concerns currently listed.
Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!
cc @rust-lang/lang-advisors: FCP proposed for lang, please feel free to register concerns.
See this document for info about what commands tagged team members can give me.
Relevant to the edition team, which will review and decide on this RFC.
label
| # Future possibilities |
|---|
| In the future, we might want to add a new default ABI. |
| For example, if `extern "stable-rust-abi"` becomes a thing and e.g. dynamically linking Rust from Rust becomes very popular, it might make sense to make that the default when writing `extern fn` without an ABI. |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think, if there is a default, it should be anything other than "C" because it raises the stakes of getting the edition wrong.
It would mean that getting the edition parameter wrong causes a breakage in ABI. Which is fine if it's caught by an error but this is not guaranteed, it could also just lead to silent UB, segfaults, etc.
There is some such flags in Rust already, but flags have been removed with it affecting the ABI being one of the reasons.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure. The future possibilities are a separate discussion if/when they are proposed. Definitely not part of what I'm proposing now. :)
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is easy to read anything in the future possibilities section as an endorsement of that idea. It would be nice to either remove that paragraph or incorporate the known issues into the text.
Also I doubt that dynamically linking Rust to Rust will become more popular than interfacing with the non-Rust world, which (mostly) goes via the "C" ABI.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is easy to read anything in the future possibilities section as an endorsement of that idea.
This is something that we really, really should try to avert in the RFC process. Accepting an RFC is not any kind of endorsement or directional approval of things in the future possibilities section.
I like being explicit here. The burden of also saying "C" is small compared to also having to have said extern, so might as well -- especially as more people actually want "C-unwind".
@rfcbot reviewed
I agree that we probably shouldn't change to a new default for some time, if ever.
@rfcbot reviewed
@rustbot labels -I-lang-nominated
We discussed this in lang triage today, resulting in tmandry's proposed FCP above, which sounds right to me.
Indicates that an issue has been nominated for prioritizing at the next lang team meeting.
label
Meanwhile, there is an existing
missing_abilint that we should upgrade to warn-by-default in all editions. We should do that in a separate FCP.
This PR makes the lint warn-by-default: rust-lang/rust#132397
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request
…rrors
Improve missing_abi lint
This is for the migration lint for rust-lang/rfcs#3722
It is not yet marked as an edition migration lint, because Edition2027 doesn't exist yet.
The lint now includes a machine applicable suggestion:
warning: extern declarations without an explicit ABI are deprecated
--> src/main.rs:3:1
|
3 | extern fn a() {}
| ^^^^^^ help: explicitly specify the C ABI: `extern "C"`
|workingjubilee added a commit to workingjubilee/rustc that referenced this pull request
…rrors
Improve missing_abi lint
This is for the migration lint for rust-lang/rfcs#3722
It is not yet marked as an edition migration lint, because Edition2027 doesn't exist yet.
The lint now includes a machine applicable suggestion:
warning: extern declarations without an explicit ABI are deprecated
--> src/main.rs:3:1
|
3 | extern fn a() {}
| ^^^^^^ help: explicitly specify the C ABI: `extern "C"`
|rust-timer added a commit to rust-lang-ci/rust that referenced this pull request
Rollup merge of rust-lang#132357 - m-ou-se:explicit-abi, r=compiler-errors
Improve missing_abi lint
This is for the migration lint for rust-lang/rfcs#3722
It is not yet marked as an edition migration lint, because Edition2027 doesn't exist yet.
The lint now includes a machine applicable suggestion:
warning: extern declarations without an explicit ABI are deprecated
--> src/main.rs:3:1
|
3 | extern fn a() {}
| ^^^^^^ help: explicitly specify the C ABI: `extern "C"`
|
🔔 This is now entering its final comment period, as per the review above. 🔔
| - This is a breaking change and needs to be done in a new edition. |
| # Prior art |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Other prior art: rustfmt converts extern {} to extern "C" {}, see the docs.
The final comment period, with a disposition to merge, as per the review above, is now complete.
As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.
This will be merged soon.
bors added a commit to rust-lang-ci/rust that referenced this pull request
Make missing_abi lint warn-by-default.
This makes the missing_abi lint warn-by-default, as suggested here: rust-lang/rfcs#3722 (comment)
This needs a lang FCP.
jhpratt added a commit to jhpratt/rust that referenced this pull request
Make missing_abi lint warn-by-default.
This makes the missing_abi lint warn-by-default, as suggested here: rust-lang/rfcs#3722 (comment)
This needs a lang FCP.
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request
Rollup merge of rust-lang#132397 - m-ou-se:warn-missing-abi, r=Nadrieril
Make missing_abi lint warn-by-default.
This makes the missing_abi lint warn-by-default, as suggested here: rust-lang/rfcs#3722 (comment)
This needs a lang FCP.
flip1995 pushed a commit to flip1995/rust-clippy that referenced this pull request
Make missing_abi lint warn-by-default.
This makes the missing_abi lint warn-by-default, as suggested here: rust-lang/rfcs#3722 (comment)
This needs a lang FCP.
antoyo pushed a commit to rust-lang/rustc_codegen_gcc that referenced this pull request
Make missing_abi lint warn-by-default.
This makes the missing_abi lint warn-by-default, as suggested here: rust-lang/rfcs#3722 (comment)
This needs a lang FCP.
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request
…iscross,nadrieril
Add explicit_extern_abis Feature and Enforce Explicit ABIs
The unstable explicit_extern_abis feature is introduced, requiring explicit ABIs in extern blocks. Hard errors will be enforced with this feature enabled in a future edition.
Update rust-lang#134986
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request
Rollup merge of rust-lang#135340 - obeis:explicit-extern-abis, r=traviscross,nadrieril
Add explicit_extern_abis Feature and Enforce Explicit ABIs
The unstable explicit_extern_abis feature is introduced, requiring explicit ABIs in extern blocks. Hard errors will be enforced with this feature enabled in a future edition.
Update rust-lang#134986
github-actions bot pushed a commit to rust-lang/rustc-dev-guide that referenced this pull request
…adrieril
Add explicit_extern_abis Feature and Enforce Explicit ABIs
The unstable explicit_extern_abis feature is introduced, requiring explicit ABIs in extern blocks. Hard errors will be enforced with this feature enabled in a future edition.
Update #134986