Skip to content

Revert "Stabilize extended_varargs_abi_support"#136897

Merged
bors merged 2 commits into
rust-lang:masterfrom
workingjubilee:revert-unfcped-stab
Feb 12, 2025
Merged

Revert "Stabilize extended_varargs_abi_support"#136897
bors merged 2 commits into
rust-lang:masterfrom
workingjubilee:revert-unfcped-stab

Conversation

@workingjubilee

@workingjubilee workingjubilee commented Feb 12, 2025 •
edited
Loading

Copy link
Copy Markdown
Member

I cannot find an FCP for this, despite it being a stabilization PR which normally means we do an FCP of some kind? It would seem reasonable for either compiler or lang to have FCPed it? I am thus opening a revert PR, which mostly-cleanly applies, so that we can later actually land this properly with a stability report and FCP.

@rustbot

rustbot commented Feb 12, 2025

Copy link
Copy Markdown
Collaborator

r? @estebank

rustbot has assigned @estebank.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Feb 12, 2025
@workingjubilee

workingjubilee commented Feb 12, 2025 •
edited
Loading

Copy link
Copy Markdown
Member Author

Note that even if this is accepted I intend to almost immediately relaunch the stabilization process here for the set of ABIs for which it is simply and obviously consistent to do this for... which is basically everything except extern "system". I would not recommend backporting such a diff to our current beta, however...

@workingjubilee workingjubilee added beta-nominated Nominated for backporting to the compiler in the beta channel. I-compiler-nominated Nominated for discussion during a compiler team meeting. T-lang Relevant to the language team I-lang-nominated Nominated for discussion during a lang team meeting. and removed T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Feb 12, 2025
@workingjubilee workingjubilee mentioned this pull request Feb 12, 2025
Closed
3 tasks
WaffleLapkin
WaffleLapkin approved these changes Feb 12, 2025
View reviewed changes

@WaffleLapkin WaffleLapkin left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

r=me with/without nitpick

Comment thread compiler/rustc_feature/src/accepted.rs
(accepted, expr_fragment_specifier_2024, "1.83.0", Some(123742)),
/// Allows arbitrary expressions in key-value attributes at parse time.
(accepted, extended_key_value_attributes, "1.54.0", Some(78835)),
/// Allows using `efiapi`, `aapcs`, `sysv64` and `win64` as calling

@WaffleLapkin WaffleLapkin Feb 12, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

fun how this doesn't even mention "system"...

Comment thread library/std/src/lib.rs Outdated
@workingjubilee @WaffleLapkin
library: amend revert of extended_varargs_abi_support for beta diff
cafa646 
And leave a comment on the unusual `cfg_attr`

Co-authored-by: waffle <waffle.lapkin@gmail.com>
@WaffleLapkin

WaffleLapkin commented Feb 12, 2025

Copy link
Copy Markdown
Member

@bors r+

@bors

bors commented Feb 12, 2025

Copy link
Copy Markdown
Collaborator

📌 Commit cafa646 has been approved by WaffleLapkin

It is now in the queue for this repository.

@bors 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 Feb 12, 2025
@WaffleLapkin

WaffleLapkin commented Feb 12, 2025

Copy link
Copy Markdown
Member

@bors p=5

this needs to be beta backported before Friday, so let's try to merge this as soon as possible

@bors

bors commented Feb 12, 2025

Copy link
Copy Markdown
Collaborator

⌛ Testing commit cafa646 with merge 021fb9c...

@apiraino apiraino mentioned this pull request Feb 12, 2025
Closed
@jieyouxu jieyouxu added the F-extended_varargs_abi_support `#![feature(extended_varargs_abi_support)]` label Feb 12, 2025
@RalfJung

RalfJung commented Feb 12, 2025 •
edited
Loading

Copy link
Copy Markdown
Member

Note that even if this is accepted I intend to almost immediately relaunch the stabilization process here for the set of ABIs for which it is simply and obviously consistent to do this for... which is basically everything except extern "system". I would not recommend backporting such a diff to our current beta, however...

Maybe "system" should just return false in fn supports_varargs ?

@ChrisDenton

ChrisDenton commented Feb 12, 2025

Copy link
Copy Markdown
Member

That would work as quick fix but ideally it should return the same as whatever calling convention system is aliasing, no? As should C in theory, albeit I think all C calling conventions must support varags.

@bors

bors commented Feb 12, 2025

Copy link
Copy Markdown
Collaborator

☀️ Test successful - checks-actions
Approved by: WaffleLapkin
Pushing 021fb9c to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Feb 12, 2025
@bors bors merged commit 021fb9c into rust-lang:master Feb 12, 2025
@rustbot rustbot added this to the 1.86.0 milestone Feb 12, 2025
@Mark-Simulacrum Mark-Simulacrum added the beta-accepted Accepted for backporting to the compiler in the beta channel. label Feb 12, 2025
@Mark-Simulacrum

Mark-Simulacrum commented Feb 12, 2025

Copy link
Copy Markdown
Member

I'll unilaterally beta accept this - reverting a stabilization feels like obviously required to backport.

@cuviper

cuviper commented Feb 12, 2025

Copy link
Copy Markdown
Member

Note, we should include the reference revert too: rust-lang/reference#1734

@WaffleLapkin WaffleLapkin mentioned this pull request Feb 12, 2025
Closed
@workingjubilee workingjubilee deleted the revert-unfcped-stab branch February 12, 2025 17:28
@cuviper cuviper mentioned this pull request Feb 13, 2025
Merged
@cuviper cuviper modified the milestones: 1.86.0, 1.85.0 Feb 13, 2025
@cuviper cuviper removed the beta-nominated Nominated for backporting to the compiler in the beta channel. label Feb 13, 2025
This was referenced Feb 13, 2025
Merged
Closed
@apiraino

apiraino commented Feb 20, 2025

Copy link
Copy Markdown
Contributor

I'll remove the nominations since this was the sensible thing to do (T-lang left a comment here)

@rustbot label -I-compiler-nominated -I-lang-nominated

@rustbot rustbot removed I-compiler-nominated Nominated for discussion during a compiler team meeting. I-lang-nominated Nominated for discussion during a lang team meeting. labels Feb 20, 2025
@RalfJung RalfJung mentioned this pull request Jul 17, 2025
Merged
@yotamofek yotamofek mentioned this pull request Nov 27, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@WaffleLapkin WaffleLapkin WaffleLapkin approved these changes

Assignees

@estebank estebank

Labels

beta-accepted Accepted for backporting to the compiler in the beta channel. F-extended_varargs_abi_support `#![feature(extended_varargs_abi_support)]` merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-lang Relevant to the language team

Projects

None yet

Milestone

1.85.0

Development

Successfully merging this pull request may close these issues.