c-variadic: add checks for windows i686 - #162905
folkertdev wants to merge 2 commits into
Conversation
rustbot
added
S-waiting-on-review
|
|
|
r? me |
|
Just to link this together, the change is llvm/llvm-project#218017 which gives |
There was a problem hiding this comment.
The I686_GNU and I686_MSVC revisions assembly are (as expected) identical apart from I686_GNU having frame pointers (which only makes a difference for two instructions). I think it would be better to merge the assembly lines where possible using the I686 check prefix you've already defined. One option is that the assembly is identical AFAICT if -Cforce-frame-pointers is passed as a compile flag for the I686_MSVC, which should be fine to add as this test isn't testing frame pointers.
| #[lang = "va_arg_safe"] | ||
| pub unsafe trait VaArgSafe {} | ||
|
|
||
| unsafe impl VaArgSafe for i32 {} | ||
| unsafe impl VaArgSafe for i64 {} | ||
| #[cfg(target_pointer_width = "64")] | ||
| unsafe impl VaArgSafe for i128 {} | ||
| unsafe impl VaArgSafe for f64 {} | ||
| unsafe impl<T> VaArgSafe for *const T {} | ||
|
|
||
| #[repr(transparent)] | ||
| struct VaListInner { | ||
| ptr: *const c_void, | ||
| } | ||
|
|
||
| #[repr(transparent)] | ||
| #[lang = "va_list"] | ||
| pub struct VaList<'a> { | ||
| inner: VaListInner, | ||
| _marker: PhantomData<&'a mut ()>, | ||
| } | ||
|
|
||
| #[rustc_intrinsic] | ||
| #[rustc_nounwind] | ||
| pub const unsafe fn va_arg<T: VaArgSafe>(ap: &mut VaList<'_>) -> T; |
There was a problem hiding this comment.
These seem to be duplicated across lots of c-variadic tests: seems like it might be better to move them to minicore? (OK if you don't want to do that in this PR.)
There was a problem hiding this comment.
Well we could duplicate all of VaList and VaArgSafe in minicore now and move it out of these tests. We didn't want to do that while it was unstable (and so might change and minicore might diverge), at least for VaList that should not be the case any more.
rustbot
added
S-waiting-on-author
|
Reminder, once the PR becomes ready for a review, use |
16da235 to
4cba8e5
Compare
| #[lang = "va_arg_safe"] | ||
| pub unsafe trait VaArgSafe {} | ||
|
|
||
| unsafe impl VaArgSafe for i32 {} | ||
| unsafe impl VaArgSafe for i64 {} | ||
| #[cfg(target_pointer_width = "64")] | ||
| unsafe impl VaArgSafe for i128 {} | ||
| unsafe impl VaArgSafe for f64 {} | ||
| unsafe impl<T> VaArgSafe for *const T {} | ||
|
|
||
| #[repr(transparent)] | ||
| struct VaListInner { | ||
| ptr: *const c_void, | ||
| } | ||
|
|
||
| #[repr(transparent)] | ||
| #[lang = "va_list"] | ||
| pub struct VaList<'a> { | ||
| inner: VaListInner, | ||
| _marker: PhantomData<&'a mut ()>, | ||
| } | ||
|
|
||
| #[rustc_intrinsic] | ||
| #[rustc_nounwind] | ||
| pub const unsafe fn va_arg<T: VaArgSafe>(ap: &mut VaList<'_>) -> T; |
There was a problem hiding this comment.
Well we could duplicate all of VaList and VaArgSafe in minicore now and move it out of these tests. We didn't want to do that while it was unstable (and so might change and minicore might diverge), at least for VaList that should not be the case any more.
rustbot
added
S-waiting-on-review
4 participants
tracking issue: #44930
Turns out I missed these earlier. That's sort of OK because the clang implementation is not target-specific for types that we allow. But for
f128some changes are needed tova_argon x86 so I'd rather lock this all down.The diff is easier to read as individual commits: a file got moved but that is not obvious from the global diff github shows.
r? tgross35 or @beetrees