Skip to content

Conversation

DiuDiu777
Copy link
Contributor

I observed that PR146521 submitted two weeks ago resolved some documentation issues related to VaListImpl, similar to the previous PR136969.

This PR specifically adds requirements about argument availability for VaListImpl::arg, and also adds safety descriptions to the three associated intrinsic APIs.

@rustbot
Copy link
Collaborator

rustbot commented Sep 23, 2025

Some changes occurred to the intrinsics. Make sure the CTFE / Miri interpreter
gets adapted for the changes, if necessary.

cc @rust-lang/miri, @RalfJung, @oli-obk, @lcnr

@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 Sep 23, 2025
@rustbot
Copy link
Collaborator

rustbot commented Sep 23, 2025

r? @tgross35

rustbot has assigned @tgross35.
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

Copy link
Contributor

@tgross35 tgross35 left a comment

Choose a reason for hiding this comment

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

Two small nits and please squash, then r=me. Thanks for the fixes!

View changes since this review

Comment on lines 3331 to 3333
/// You must check the following invariants before you call this function:
///
/// - `ap` must not be used to access variable arguments after this call
Copy link
Contributor

Choose a reason for hiding this comment

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

This can be simplified to just:

/// `ap` must not be used to access variable arguments after this call

///
/// - has a type that is ABI-compatible with the type `T`
/// - has a value that is a properly initialized value of type `T`
/// - there is a next variable argument available (i.e., the number of arguments already read from `ap` is less than the total number passed)
Copy link
Contributor

Choose a reason for hiding this comment

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

Please rewrap to 100 chars (same in intrinsics/mod.rs)

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 3, 2025
@DiuDiu777
Copy link
Contributor Author

Done! r? @tgross35

@rustbot
Copy link
Collaborator

rustbot commented Oct 4, 2025

Requested reviewer is already assigned to this pull request.

Please choose another assignee.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. 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.

4 participants