Skip to content

Conversation

@folkertdev
Copy link
Contributor

I noticed while browsing LLVM source that we use an incorrect VaList definition for the musl hexagon target.

relevant links

cc target maintainer @androm3da can you confirm that this looks OK? In particular the #[rustc_pass_indirectly_in_non_rustic_abis] attribute is used to simulate pointer decay (like if the struct were wrapped in a 1-element array in C). The clang comment suggests that the Tag is wrapped in such a single-element array, but I haven't actually been able to confirm it.

For stabilizing c_variadic (on the hexagon targets) we will also need a custom va_arg implementation to mirror the one in clang in va_arg.rs. Would you be able to contribute one?

r? @workingjubilee

@folkertdev folkertdev added the F-c_variadic `#![feature(c_variadic)]` label Dec 13, 2025
@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Dec 13, 2025
@rustbot
Copy link
Collaborator

rustbot commented Dec 13, 2025

workingjubilee is currently at their maximum review capacity.
They may take a while to respond.

@androm3da
Copy link
Contributor

can you confirm that this looks OK?

Thanks for the PR - I will give it a look!

@androm3da
Copy link
Contributor

For stabilizing c_variadic (on the hexagon targets) we will also need a custom va_arg implementation to mirror the one in clang in va_arg.rs. Would you be able to contribute one?

Yeah - can do.

@folkertdev
Copy link
Contributor Author

For stabilizing c_variadic (on the hexagon targets) we will also need a custom va_arg implementation to mirror the one in clang in va_arg.rs. Would you be able to contribute one?

Yeah - can do.

Nice, the relevant test here is ./x test tests/run-make/c-link-to-rust-va-list-fn --target hexagon-unknown-linux-musl.

@androm3da
Copy link
Contributor

For stabilizing c_variadic (on the hexagon targets) we will also need a custom va_arg implementation to mirror the one in clang in va_arg.rs. Would you be able to contribute one?

Yeah - can do.

Nice, the relevant test here is ./x test tests/run-make/c-link-to-rust-va-list-fn --target hexagon-unknown-linux-musl.

Thanks! It passes with the changes in androm3da@5e7eca0 - now sent as three PRs: #150008, #150009, #150010

Copy link
Contributor

@androm3da androm3da left a comment

Choose a reason for hiding this comment

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

Comment on lines 34 to 35
//
// The LLVM `BuiltinVaListKind` enumerates the `va_list` variations that LLVM supports.
Copy link
Member

Choose a reason for hiding this comment

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

This is incorrect: this is clang's library code.

You are trying to leave a note here that says "you can look at this to see what kind of support a C compiler uses". So just do that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, fixed that

@workingjubilee
Copy link
Member

Thanks. This otherwise seems fine!

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented Dec 16, 2025

📌 Commit 023f38f 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 Dec 16, 2025
bors added a commit that referenced this pull request Dec 16, 2025
…uwer

Rollup of 11 pull requests

Successful merges:

 - #147939 (Make `const BorrowMut` require `const Borrow` and make `const Fn` require `const FnMut`)
 - #149734 (Mirror GCC 9.5.0)
 - #149767 (Tidying up tests/ui/issues 33 tests [4/N])
 - #149804 (chore: fix some minor issues in the comments)
 - #149967 (custom `VaList` layout for Hexagon)
 - #150025 (dont create unnecessary `DefId`s under mgca)
 - #150032 (Use annotate-snippet as default emitter on stable)
 - #150033 (Add try_as_dyn and try_as_dyn_mut)
 - #150042 (rustc-dev-guide subtree update)
 - #150063 (Remove deny of manual-let-else)
 - #150064 (std: io: error: Add comment for UEFI unpacked repr use)

Failed merges:

 - #150044 (Avoid unhelpful suggestion when crate name is invalid)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit ef2c71c into rust-lang:main Dec 17, 2025
11 checks passed
@rustbot rustbot added this to the 1.94.0 milestone Dec 17, 2025
rust-timer added a commit that referenced this pull request Dec 17, 2025
Rollup merge of #149967 - folkertdev:va-list-hexagon, r=workingjubilee

custom `VaList` layout for Hexagon

I noticed while browsing LLVM source that we use an incorrect `VaList` definition for the musl hexagon target.

relevant links

- https://github.com/llvm/llvm-project/blob/0cdc1b6dd4a870fc41d4b15ad97e0001882aba58/clang/include/clang/Basic/TargetInfo.h#L333
- https://github.com/llvm/llvm-project/blob/0cdc1b6dd4a870fc41d4b15ad97e0001882aba58/clang/lib/CodeGen/Targets/Hexagon.cpp#L407-L417

cc target maintainer `@androm3da` can you confirm that this looks OK? In particular the `#[rustc_pass_indirectly_in_non_rustic_abis]` attribute is used to simulate pointer decay (like if the struct were wrapped in a 1-element array in C). The clang comment suggests that the Tag is wrapped in such a single-element array, but I haven't actually been able to confirm it.

For stabilizing `c_variadic` (on the hexagon targets) we will also need a custom `va_arg` implementation to mirror the one in `clang` in [va_arg.rs](https://github.com/rust-lang/rust/blob/main/compiler/rustc_codegen_llvm/src/va_arg.rs). Would you be able to contribute one?

r? `@workingjubilee`
github-actions bot pushed a commit to rust-lang/rustc-dev-guide that referenced this pull request Dec 17, 2025
…uwer

Rollup of 11 pull requests

Successful merges:

 - rust-lang/rust#147939 (Make `const BorrowMut` require `const Borrow` and make `const Fn` require `const FnMut`)
 - rust-lang/rust#149734 (Mirror GCC 9.5.0)
 - rust-lang/rust#149767 (Tidying up tests/ui/issues 33 tests [4/N])
 - rust-lang/rust#149804 (chore: fix some minor issues in the comments)
 - rust-lang/rust#149967 (custom `VaList` layout for Hexagon)
 - rust-lang/rust#150025 (dont create unnecessary `DefId`s under mgca)
 - rust-lang/rust#150032 (Use annotate-snippet as default emitter on stable)
 - rust-lang/rust#150033 (Add try_as_dyn and try_as_dyn_mut)
 - rust-lang/rust#150042 (rustc-dev-guide subtree update)
 - rust-lang/rust#150063 (Remove deny of manual-let-else)
 - rust-lang/rust#150064 (std: io: error: Add comment for UEFI unpacked repr use)

Failed merges:

 - rust-lang/rust#150044 (Avoid unhelpful suggestion when crate name is invalid)

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

F-c_variadic `#![feature(c_variadic)]` S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-libs Relevant to the library team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants