Skip to content

crate: clean up leftovers from pre-rustc 1.19 without unions - #5351

Merged
tgross35 merged 7 commits into
rust-lang:mainfrom
dybucc:structs-to-unions
Aug 4, 2026
Merged

crate: clean up leftovers from pre-rustc 1.19 without unions#5351
tgross35 merged 7 commits into
rust-lang:mainfrom
dybucc:structs-to-unions

Conversation

@dybucc

@dybucc dybucc commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

Description

This PR tries to address issue #1020. It cleans up a bunch of old ctest skips
over fields and types that were previously (more than six years ago) not proper
unions.

Note as well that _channel_connect_attr in QNX I couldn't check even though it
seems like it may be some left over struct-as-union type. I just coulnd't
get access to the SDK in the QNX site.

Then there's an unnamed field that is itself a union in Haiku targets under
sigaction. I've not changed that one because I don't know of a way of handling
a combination of both unnamed union and unnamed field.

The same applies to cpu_topology_node_info in Haiku targets.

For details on the rest of the changes, see the patch messages.

Checklist

  • Relevant tests in libc-test/semver have been updated
  • Commit messages permalink to headers for added or changed API
  • Placeholder or unstable values like *LAST or *MAX have the standard
    doc comment
  • Tested locally (cargo test -p libc-test --target mytarget);
    especially relevant for platforms that may not be checked in CI

@rustbot label +stable-nominated

@rustbot

rustbot commented Aug 1, 2026

Copy link
Copy Markdown
Collaborator

Some changes occurred in a solarish module

cc @jclulow, @pfmooney

@rustbot rustbot added O-illumos O-solarish O-unix S-waiting-on-review stable-nominated This PR should be considered for cherry-pick to libc's stable release branch labels Aug 1, 2026
@xtqqczze

This comment was marked as outdated.

@tgross35 tgross35 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks like a great cleanup, couple trait requests.

View changes since this review

Comment thread src/unix/haiku/mod.rs Outdated
Comment thread src/unix/solarish/illumos.rs Outdated
Comment on lines +67 to +78
#[cfg_attr(any(target_arch = "x86", target_arch = "x86_64"), repr(packed(4)))]
pub struct epoll_event {
pub events: u32,
pub data: epoll_data,
}

pub union epoll_data {
pub ptr: *mut c_void,
pub fd: c_int,
pub u32: u32,
pub u64: u64,
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Illumos maintainers @jclulow @pfmooney this is a useful change but technically breaking, do you want it in 0.2?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Deciding against this, see #5351 (review)

@rustbot

rustbot commented Aug 2, 2026

Copy link
Copy Markdown
Collaborator

Reminder, once the PR becomes ready for a review, use @rustbot ready.

@dybucc
dybucc force-pushed the structs-to-unions branch from 078195b to 50d04e5 Compare August 3, 2026 06:31
@rustbot

This comment has been minimized.

@dybucc
dybucc force-pushed the structs-to-unions branch from 50d04e5 to 0978aac Compare August 3, 2026 06:31
@dybucc

dybucc commented Aug 3, 2026

Copy link
Copy Markdown
Contributor Author

@rustbot ready
@rustbot label +S-waiting-on-maintainer

@tgross35 tgross35 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM, thanks!

I actually don't think we should have the epoll_event changes in 0.2 since we opted out of backporting the fix on Linux (1d10a3f). I'll exclude the relevant commits when picking.

View changes since this review

@tgross35
tgross35 enabled auto-merge August 3, 2026 08:09
dybucc added 7 commits August 3, 2026 17:14
Remove a check in the Linux routine of build the build script that
skipped a field whose type was a `union` upstream but only a specific
field of the `union` in Rust. This is a remnant of versions prior to
Rust 1.19, where `union`s got introduced. The definition under
`unix/linux_like/mod.rs` uses a `union` for that field. The check is
redundant now.
Replace field of `epoll_event` that was once only a single field of the
upstream `union` with a proper `union`. This was done back before Rust
had received support for `union`s, so it's redundant to keep this
binding now. The build script also had a skip set in place for this one
field, and that now has also been removed.
Remove check in Linux test routine for a field of `sigevent` that was
once (pre-Rust 1.19) implemented as a `struct` instead of a `union`.
This is no more the case so the check in the build script is redundant.
Remove check for `cpuid_info` as a `union` as that dates back to the
times where Rust didn't itself have `union`s so this type had to be
implemented as a `struct`. Skipping it in libc-test's build script is
redundant. The same applies to `cpu_topology_node_info`.
Replace dummy field for `union` with an actual definition fitting that
of the upstream type. The current (pre-patch) field corresponded with
the times back when Rust didn't have `union`s. Now it's not necessary
anymore to expose only a single field of the `union`. The corresponding
skip in libc-test's build script has also been removed.
Remove a skip over `sigaction` in Hexagon QuRT as it seems the separate
fields in Rust mentioned in libc-test's build script's skip aligns well
with the (also) split fields upstream. See header file `posix/signal.h`
under `rtos/qurt/computev81` in the Hexagon SDK.
Remove skip over `sem_t` in Hexagon QuRT as it alleged to this being a
`typedef` of an anonymous `struct`, but that also happens in other
targets we test against without there being skips for their records. See
header file `posix/semaphore.h` under `rtos/qurt/computev81` in the
Hexagon SDK for details.
@tgross35
tgross35 force-pushed the structs-to-unions branch from 0978aac to de46d22 Compare August 3, 2026 21:14
@rustbot

rustbot commented Aug 3, 2026

Copy link
Copy Markdown
Collaborator

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

@tgross35
tgross35 added this pull request to the merge queue Aug 3, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Aug 3, 2026
@tgross35
tgross35 added this pull request to the merge queue Aug 4, 2026
Merged via the queue into rust-lang:main with commit 471be3d Aug 4, 2026
57 checks passed
@tgross35

tgross35 commented Aug 31, 2026

Copy link
Copy Markdown
Member

Excluding b0204f2 from the backport since that would need d255379, which isn't backported.

tgross35 pushed a commit to tgross35/rust-libc that referenced this pull request Aug 31, 2026
Remove check for `cpuid_info` as a `union` as that dates back to the
times where Rust didn't itself have `union`s so this type had to be
implemented as a `struct`. Skipping it in libc-test's build script is
redundant. The same applies to `cpu_topology_node_info`.

(backport <rust-lang#5351>)
(cherry picked from commit 12fe28d)
tgross35 pushed a commit to tgross35/rust-libc that referenced this pull request Aug 31, 2026
Replace dummy field for `union` with an actual definition fitting that
of the upstream type. The current (pre-patch) field corresponded with
the times back when Rust didn't have `union`s. Now it's not necessary
anymore to expose only a single field of the `union`. The corresponding
skip in libc-test's build script has also been removed.

(backport <rust-lang#5351>)
(cherry picked from commit 97ad377)
tgross35 pushed a commit to tgross35/rust-libc that referenced this pull request Aug 31, 2026
Remove a skip over `sigaction` in Hexagon QuRT as it seems the separate
fields in Rust mentioned in libc-test's build script's skip aligns well
with the (also) split fields upstream. See header file `posix/signal.h`
under `rtos/qurt/computev81` in the Hexagon SDK.

(backport <rust-lang#5351>)
(cherry picked from commit 96baf11)
tgross35 pushed a commit to tgross35/rust-libc that referenced this pull request Aug 31, 2026
Remove skip over `sem_t` in Hexagon QuRT as it alleged to this being a
`typedef` of an anonymous `struct`, but that also happens in other
targets we test against without there being skips for their records. See
header file `posix/semaphore.h` under `rtos/qurt/computev81` in the
Hexagon SDK for details.

(backport <rust-lang#5351>)
(cherry picked from commit 471be3d)
@tgross35

Copy link
Copy Markdown
Member

Similarly skipping 08fa94f which needs 1d10a3f

tgross35 pushed a commit to tgross35/rust-libc that referenced this pull request Aug 31, 2026
Replace field of `epoll_event` that was once only a single field of the
upstream `union` with a proper `union`. This was done back before Rust
had received support for `union`s, so it's redundant to keep this
binding now. The build script also had a skip set in place for this one
field, and that now has also been removed.

(backport <rust-lang#5351>)
(cherry picked from commit 3b624ac)
tgross35 pushed a commit to tgross35/rust-libc that referenced this pull request Aug 31, 2026
Remove check for `cpuid_info` as a `union` as that dates back to the
times where Rust didn't itself have `union`s so this type had to be
implemented as a `struct`. Skipping it in libc-test's build script is
redundant. The same applies to `cpu_topology_node_info`.

(backport <rust-lang#5351>)
(cherry picked from commit 12fe28d)
tgross35 pushed a commit to tgross35/rust-libc that referenced this pull request Aug 31, 2026
Replace dummy field for `union` with an actual definition fitting that
of the upstream type. The current (pre-patch) field corresponded with
the times back when Rust didn't have `union`s. Now it's not necessary
anymore to expose only a single field of the `union`. The corresponding
skip in libc-test's build script has also been removed.

(backport <rust-lang#5351>)
(cherry picked from commit 97ad377)
tgross35 pushed a commit to tgross35/rust-libc that referenced this pull request Aug 31, 2026
Remove a skip over `sigaction` in Hexagon QuRT as it seems the separate
fields in Rust mentioned in libc-test's build script's skip aligns well
with the (also) split fields upstream. See header file `posix/signal.h`
under `rtos/qurt/computev81` in the Hexagon SDK.

(backport <rust-lang#5351>)
(cherry picked from commit 96baf11)
tgross35 pushed a commit to tgross35/rust-libc that referenced this pull request Aug 31, 2026
Remove skip over `sem_t` in Hexagon QuRT as it alleged to this being a
`typedef` of an anonymous `struct`, but that also happens in other
targets we test against without there being skips for their records. See
header file `posix/semaphore.h` under `rtos/qurt/computev81` in the
Hexagon SDK for details.

(backport <rust-lang#5351>)
(cherry picked from commit 471be3d)
tgross35 pushed a commit to tgross35/rust-libc that referenced this pull request Aug 31, 2026
Remove check for `cpuid_info` as a `union` as that dates back to the
times where Rust didn't itself have `union`s so this type had to be
implemented as a `struct`. Skipping it in libc-test's build script is
redundant. The same applies to `cpu_topology_node_info`.

(backport <rust-lang#5351>)
(cherry picked from commit 12fe28d)
tgross35 pushed a commit to tgross35/rust-libc that referenced this pull request Aug 31, 2026
Replace dummy field for `union` with an actual definition fitting that
of the upstream type. The current (pre-patch) field corresponded with
the times back when Rust didn't have `union`s. Now it's not necessary
anymore to expose only a single field of the `union`. The corresponding
skip in libc-test's build script has also been removed.

(backport <rust-lang#5351>)
(cherry picked from commit 97ad377)
tgross35 pushed a commit to tgross35/rust-libc that referenced this pull request Aug 31, 2026
Remove a skip over `sigaction` in Hexagon QuRT as it seems the separate
fields in Rust mentioned in libc-test's build script's skip aligns well
with the (also) split fields upstream. See header file `posix/signal.h`
under `rtos/qurt/computev81` in the Hexagon SDK.

(backport <rust-lang#5351>)
(cherry picked from commit 96baf11)
tgross35 pushed a commit to tgross35/rust-libc that referenced this pull request Aug 31, 2026
Remove skip over `sem_t` in Hexagon QuRT as it alleged to this being a
`typedef` of an anonymous `struct`, but that also happens in other
targets we test against without there being skips for their records. See
header file `posix/semaphore.h` under `rtos/qurt/computev81` in the
Hexagon SDK for details.

(backport <rust-lang#5351>)
(cherry picked from commit 471be3d)
@tgross35 tgross35 mentioned this pull request Aug 31, 2026
@tgross35 tgross35 added stable-applied This PR has been cherry-picked to libc's stable release branch and removed stable-nominated This PR should be considered for cherry-pick to libc's stable release branch labels Aug 31, 2026
@tgross35

Copy link
Copy Markdown
Member

3b624ac as well, breaking change

tgross35 pushed a commit to dybucc/libc that referenced this pull request Aug 31, 2026
Remove check for `cpuid_info` as a `union` as that dates back to the
times where Rust didn't itself have `union`s so this type had to be
implemented as a `struct`. Skipping it in libc-test's build script is
redundant. The same applies to `cpu_topology_node_info`.

(backport <rust-lang#5351>)
(cherry picked from commit 12fe28d)
tgross35 pushed a commit to dybucc/libc that referenced this pull request Aug 31, 2026
Replace dummy field for `union` with an actual definition fitting that
of the upstream type. The current (pre-patch) field corresponded with
the times back when Rust didn't have `union`s. Now it's not necessary
anymore to expose only a single field of the `union`. The corresponding
skip in libc-test's build script has also been removed.

(backport <rust-lang#5351>)
(cherry picked from commit 97ad377)
tgross35 pushed a commit to dybucc/libc that referenced this pull request Aug 31, 2026
Remove a skip over `sigaction` in Hexagon QuRT as it seems the separate
fields in Rust mentioned in libc-test's build script's skip aligns well
with the (also) split fields upstream. See header file `posix/signal.h`
under `rtos/qurt/computev81` in the Hexagon SDK.

(backport <rust-lang#5351>)
(cherry picked from commit 96baf11)
tgross35 pushed a commit to dybucc/libc that referenced this pull request Aug 31, 2026
Remove skip over `sem_t` in Hexagon QuRT as it alleged to this being a
`typedef` of an anonymous `struct`, but that also happens in other
targets we test against without there being skips for their records. See
header file `posix/semaphore.h` under `rtos/qurt/computev81` in the
Hexagon SDK for details.

(backport <rust-lang#5351>)
(cherry picked from commit 471be3d)
@dybucc

dybucc commented Sep 1, 2026

Copy link
Copy Markdown
Contributor Author

Not sure if we can close #1020 already, or if we should wait until we
really near 1.0 because of the above breaking changes.

@tgross35

tgross35 commented Sep 1, 2026

Copy link
Copy Markdown
Member

Good call, I think we can close that. As long as there aren't other union-related mismatches.

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

Labels

O-illumos O-solarish O-unix S-waiting-on-maintainer stable-applied This PR has been cherry-picked to libc's stable release branch

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants