-
Notifications
You must be signed in to change notification settings - Fork 13.5k
Prototype VaList
proposal
#141980
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
base: master
Are you sure you want to change the base?
Prototype VaList
proposal
#141980
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1585,6 +1585,12 @@ impl<'tcx> TyCtxt<'tcx> { | |
flags.insert(ReprFlags::IS_LINEAR); | ||
} | ||
|
||
if self.is_lang_item(did.to_def_id(), LangItem::VaList) | ||
&& !flags.contains(ReprFlags::IS_TRANSPARENT) | ||
{ | ||
flags.insert(ReprFlags::PASS_INDIRECTLY); | ||
} | ||
Comment on lines
+1588
to
+1592
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So randomly in the middle of Also, in the discussion the argument was that this is necessary when va_list is an array -- but I can't see that reflected here at all. Given that the actual source of the issue is array decay, shouldn't we just fix the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is where the (very-ABI-affecting) I think @folkertdev has suggested before something along the lines of a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah I think if we do go for this (which I am not convinced we should), then it should be its own attribute, not magically connected to the lang item. |
||
|
||
ReprOptions { int: size, align: max_align, pack: min_pack, flags, field_shuffle_seed } | ||
} | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I only see this as having an effect on x86-64 and aarch64, but doesn't the argument-passing algorithm already enforce this by making sure that VaList gets passed via Memory?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This method is only needed by the code in
rustc_target::callconv
to ensure thatVaList
getsPassMode::Indirect { on_stack: false, .. }
.repr_options_of_def
sets thePASS_INDIRECTLY
ReprFlag
forVaList
when it needs to be passed indirectly for non-rustic ABIs, however there is no way to directly accessReprFlag
s from the calling convention code. There is already a methodis_transparent
onTyAbiInterface
to expose theIS_TRANSPARENT
ReprFlag
, so I used a similar pattern to exposePASS_INDIRECTLY
to the calling convention code. Without this method, the System V x86-64 ABI would passVaList
withPassMode::Indirect { on_stack: true, .. }
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And just to finish the thought: the difference is important (only) in an FFI context: a foreign
extern "C" { fn(ap: VaList); }
would expectap
to be passed asPassMode::Indirect { on_stack: false, .. }
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
huh, fascinating.