Skip to content

variadic functions: remove list of supported ABIs from error #142464

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

Merged
merged 1 commit into from
Jun 14, 2025

Conversation

RalfJung
Copy link
Member

I think this list is problematic for multiple reasons:

  • It is bound to go out-of-date as it is in a very different place from where we actually define which functions support varagrs (fn supports_varargs).
  • Many of the ABIs we list only work on some targets; it makes no sense to mention "aapcs" as a possible ABI when building for x86_64. (This led to a lot of confusion in Variadic functions don't allow "system" on non-x86 Windows #110505 where the author thought they should use "cdecl" and then were promptly told that "cdecl" is not a legal ABI on their target.)
  • Typically, when the programmer wrote extern "foobar", it is because they need the "foobar" ABI. It is of little use to tell them that there are other ABIs with which varargs would work.

Cc @workingjubilee

@rustbot
Copy link
Collaborator

rustbot commented Jun 13, 2025

r? @davidtwco

rustbot has assigned @davidtwco.
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. labels Jun 13, 2025
@rust-log-analyzer

This comment has been minimized.

@RalfJung RalfJung force-pushed the variadic-fn-abi-error branch from 6106df6 to 963fdbc Compare June 13, 2025 16:10
@workingjubilee
Copy link
Member

Yeah, I didn't really understand this suggestion, in retrospect, I just didn't pause to question it.

r=me with CI passing.

@RalfJung
Copy link
Member Author

@bors r=workingjubilee

@bors
Copy link
Collaborator

bors commented Jun 13, 2025

📌 Commit 963fdbc has been approved by workingjubilee

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 Jun 13, 2025
jhpratt added a commit to jhpratt/rust that referenced this pull request Jun 14, 2025
…workingjubilee

variadic functions: remove list of supported ABIs from error

I think this list is problematic for multiple reasons:
- It is bound to go out-of-date as it is in a very different place from where we actually define which functions support varagrs (`fn supports_varargs`).
- Many of the ABIs we list only work on some targets; it makes no sense to mention "aapcs" as a possible ABI when building for x86_64. (This led to a lot of confusion in rust-lang#110505 where the author thought they should use "cdecl" and then were promptly told that "cdecl" is not a legal ABI on their target.)
- Typically, when the programmer wrote `extern "foobar"`, it is because they need the "foobar" ABI. It is of little use to tell them that there are other ABIs with which varargs would work.

Cc `@workingjubilee`
bors added a commit that referenced this pull request Jun 14, 2025
Rollup of 9 pull requests

Successful merges:

 - #140593 (Temporary lifetime extension through tuple struct and tuple variant constructors)
 - #141399 ([rustdoc] Give more information into extracted doctest information)
 - #141493 (Delegate `<SocketAddr as Debug>` to `ByteStr`)
 - #141811 (Unimplement unsized_locals)
 - #142243 (float tests: deduplicate min, max, and rounding tests)
 - #142464 (variadic functions: remove list of supported ABIs from error)
 - #142477 (Fix incorrect suggestion when calling an associated type with a type anchor)
 - #142484 (Remove unneeded lifetime bound from signature of BTreeSet::extract_if)
 - #142489 (Update the `compiler-builtins` subtree)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit fe54c3a into rust-lang:master Jun 14, 2025
10 checks passed
@rustbot rustbot added this to the 1.89.0 milestone Jun 14, 2025
rust-timer added a commit that referenced this pull request Jun 14, 2025
Rollup merge of #142464 - RalfJung:variadic-fn-abi-error, r=workingjubilee

variadic functions: remove list of supported ABIs from error

I think this list is problematic for multiple reasons:
- It is bound to go out-of-date as it is in a very different place from where we actually define which functions support varagrs (`fn supports_varargs`).
- Many of the ABIs we list only work on some targets; it makes no sense to mention "aapcs" as a possible ABI when building for x86_64. (This led to a lot of confusion in #110505 where the author thought they should use "cdecl" and then were promptly told that "cdecl" is not a legal ABI on their target.)
- Typically, when the programmer wrote `extern "foobar"`, it is because they need the "foobar" ABI. It is of little use to tell them that there are other ABIs with which varargs would work.

Cc ``@workingjubilee``
@RalfJung RalfJung deleted the variadic-fn-abi-error branch June 14, 2025 12:53
RalfJung pushed a commit to RalfJung/miri that referenced this pull request Jun 15, 2025
Rollup of 9 pull requests

Successful merges:

 - rust-lang/rust#140593 (Temporary lifetime extension through tuple struct and tuple variant constructors)
 - rust-lang/rust#141399 ([rustdoc] Give more information into extracted doctest information)
 - rust-lang/rust#141493 (Delegate `<SocketAddr as Debug>` to `ByteStr`)
 - rust-lang/rust#141811 (Unimplement unsized_locals)
 - rust-lang/rust#142243 (float tests: deduplicate min, max, and rounding tests)
 - rust-lang/rust#142464 (variadic functions: remove list of supported ABIs from error)
 - rust-lang/rust#142477 (Fix incorrect suggestion when calling an associated type with a type anchor)
 - rust-lang/rust#142484 (Remove unneeded lifetime bound from signature of BTreeSet::extract_if)
 - rust-lang/rust#142489 (Update the `compiler-builtins` subtree)

r? `@ghost`
`@rustbot` modify labels: rollup
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants