rust-lang / rfcs

RFCs for changes to Rust
https://rust-lang.github.io/rfcs/
Apache License 2.0
5.97k stars 1.57k forks source link

[RFC] Explicit ABI in `extern` #3722

Open m-ou-se opened 3 weeks ago

m-ou-se commented 3 weeks ago

Rendered

joshtriplett commented 3 weeks ago

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.

m-ou-se commented 3 weeks ago

Implementation, including migration lint: https://github.com/rust-lang/rust/pull/132357

traviscross commented 3 weeks ago

@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.

tmandry commented 3 weeks ago

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

rfcbot commented 3 weeks ago

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.

scottmcm commented 3 weeks ago

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.

traviscross commented 3 weeks ago

@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.

m-ou-se commented 3 weeks ago

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.

This PR makes the lint warn-by-default: https://github.com/rust-lang/rust/pull/132397

rfcbot commented 5 days ago

:bell: This is now entering its final comment period, as per the review above. :bell: