-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
[RFC] Explicit ABI in extern
#3722
Open
m-ou-se
wants to merge
3
commits into
rust-lang:master
Choose a base branch
from
m-ou-se:explicit-abi
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
3 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains 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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,86 @@ | ||
- Feature Name: `explicit_abi` | ||
- Start Date: 2024-10-30 | ||
- RFC PR: [rust-lang/rfcs#3722](https://github.com/rust-lang/rfcs/pull/3722) | ||
- Rust Issue: [rust-lang/rust#0000](https://github.com/rust-lang/rust/issues/0000) | ||
|
||
# Summary | ||
|
||
Disallow `extern` without an explicit ABI in a new edition. Write `extern "C"` (or another ABI) instead of just `extern`. | ||
|
||
```diff | ||
- extern { … } | ||
+ extern "C" { … } | ||
|
||
- extern fn foo() { … } | ||
+ extern "C" fn foo() { … } | ||
``` | ||
|
||
# Motivation | ||
|
||
Originally, `"C"` was a very reasable default for `extern`. | ||
However, with work ongoing to add other ABIs to Rust, it is no longer obvious that `"C"` should forever stay the default. | ||
|
||
By making the ABI explicit, it becomes much clearer that `"C"` is just one of the possible choices, rather than the "standard" way for external functions. | ||
Removing the default makes it easier to add a new ABI on equal footing as `"C"`. | ||
|
||
Right now, "extern", "FFI" and "C" are somewhat used interchangeably in Rust. For example, this is the diagnostic when using a `String` in an `extern` function: | ||
|
||
``` | ||
warning: `extern` fn uses type `String`, which is not FFI-safe | ||
--> src/main.rs:1:16 | ||
| | ||
1 | extern fn a(s: String) {} | ||
| ^^^^^^ not FFI-safe | ||
| | ||
= help: consider adding a `#[repr(C)]` or `#[repr(transparent)]` attribute to this struct | ||
= note: this struct has unspecified layout | ||
= note: `#[warn(improper_ctypes_definitions)]` on by default | ||
``` | ||
|
||
If another future ABI will support `String`, this error should make it clearer that the problem is not that `String` doesn't support FFI, but rather that the `"C"` ABI doesn't support `String`. | ||
This would be easier if there was actually a `"C"` token to point at in the source code. E.g.: | ||
|
||
``` | ||
warning: `extern` fn uses type `String`, which is not supported by the "C" ABI | ||
--> src/main.rs:1:16 | ||
| | ||
1 | extern "C" fn a(s: String) {} | ||
| --- ^^^^^^ String type not supported by this ABI | ||
| | | ||
| the "C" ABI does not support this type | ||
``` | ||
|
||
It would also make it clearer that swapping `"C"` for another ABI might be an option. | ||
|
||
# Guilde-level explanation | ||
|
||
Up to the previous edition, `extern` without an explicit ABI was equivalent to `extern "C"`. | ||
In the new edition, writing `extern` without an ABI is an error. | ||
Instead, you must write `extern "C"` explicitly. | ||
|
||
# Automatic migration | ||
|
||
Automatic migration (for `cargo fix --edition`) is trivial: Insert `"C"` after `extern` if there is no ABI. | ||
|
||
# Drawbacks | ||
|
||
- This is a breaking change and needs to be done in a new edition. | ||
|
||
# Prior art | ||
|
||
This was proposed before Rust 1.0 in 2015 in [RFC 697](https://github.com/rust-lang/rfcs/pull/697). | ||
It was not accepted at the time, because "C" seemed like the only resonable default. | ||
It was later closed because it'd be a backwards incompatible change, and editions were not yet invented. | ||
|
||
# Unresolved questions | ||
|
||
- ~~In which edition do we make this change?~~ | ||
- It's too late for the 2024 edition: https://github.com/rust-lang/rfcs/pull/3722#issuecomment-2447333966 | ||
- ~~Do we warn about `extern` without an explicit ABI in previous editions?~~ | ||
- Yes, with separate FCP: https://github.com/rust-lang/rfcs/pull/3722#issuecomment-2447719047 | ||
|
||
# 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. | ||
That is, however, a separate discussion; it might also be reasonable to never have a default ABI again. |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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. :)
There was a problem hiding this comment.
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.