Skip to content

Conversation

mbyx
Copy link
Contributor

@mbyx mbyx commented Aug 11, 2025

Description

Adds a struct for padding types.

Sources

Checklist

  • Relevant tests in libc-test/semver have been updated
  • No placeholder or unstable values like *LAST or *MAX are
    included (see #3131)
  • Tested locally (cd libc-test && cargo test --target mytarget);
    especially relevant for platforms that may not be checked in CI

src/types.rs Outdated
/// while providing `Default`.
#[repr(transparent)]
#[derive(Clone, Copy)]
pub struct Padding<T: Copy>(MaybeUninit<T>);
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think we need to export this, pub(crate) should be fine

@tgross35
Copy link
Contributor

Fyi you can delete the template in the PR body if it's not relevant

@mbyx mbyx force-pushed the libc-padding-struct branch 2 times, most recently from 8cdddd4 to 1daca12 Compare August 11, 2025 05:10
src/lib.rs Outdated
pub use new::*;

mod types;
pub use types::*;
Copy link
Contributor

Choose a reason for hiding this comment

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

There shouldn't be anything to export

@tgross35
Copy link
Contributor

LGTM with the last change, any reason this is marked as a draft?

@tgross35 tgross35 added the stable-nominated This PR should be considered for cherry-pick to libc's stable release branch label Aug 11, 2025
@mbyx
Copy link
Contributor Author

mbyx commented Aug 11, 2025

It seems like ctest still fails with just these changes.

@mbyx mbyx force-pushed the libc-padding-struct branch from 1daca12 to f72c749 Compare August 11, 2025 05:14
@tgross35
Copy link
Contributor

 error: struct `Padding` is never constructed
  --> src/types.rs:9:19
   |
 9 | pub(crate) struct Padding<T: Copy>(MaybeUninit<T>);
   |                   ^^^^^^^
   |

This bit? It just needs an allow(unused) (I don't see a ctest-specific failure)

@mbyx mbyx force-pushed the libc-padding-struct branch from f72c749 to b20e865 Compare August 11, 2025 05:19
@tgross35
Copy link
Contributor

For style check, this will need an update at

const SKIP_PREFIXES: &[&str] = &[

@mbyx mbyx force-pushed the libc-padding-struct branch 3 times, most recently from 8dd85bd to fa9d83e Compare August 11, 2025 05:37
@mbyx mbyx marked this pull request as ready for review August 11, 2025 05:40
src/types.rs Outdated
Comment on lines 3 to 4
use core::clone::Clone;
use core::default::Default;
use core::marker::Copy;
use core::mem::MaybeUninit;
use core::prelude::rust_2024::derive;
Copy link
Contributor

Choose a reason for hiding this comment

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

If you need to import all of these, just add Default to the prelude macro and add use crate::prelude::* here.

This adds a bit of trickiness: there is no crate::prelude in some cases

libc/src/lib.rs

Lines 143 to 145 in c0071cc

} else {
// non-supported targets: empty...
}
, so the import would fail in this module. To avoid that problem, move mod types into the prelude as well.

@mbyx mbyx force-pushed the libc-padding-struct branch 2 times, most recently from 6d3757e to 14167c4 Compare August 11, 2025 05:50
src/types.rs Outdated
//! Platform-agnostic support types.
use core::mem::MaybeUninit;
use core::prelude::rust_2024::derive;
Copy link
Contributor

Choose a reason for hiding this comment

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

Sigh I was thinking this was already in the prelude, sorry I keep steering you wrong here.

Add this to prelude! instead, that's consistent. Also it will need to be ::core::prelude::v1::derive like https://github.com/rust-lang/libc/blob/c0071cc9f9385da6352e40824cfd77334e85f72d/src/macros.rs#L117C15-L117C43, the 2024 prelude isn't available until way after our MSRV.

@mbyx mbyx force-pushed the libc-padding-struct branch from 14167c4 to 546a941 Compare August 11, 2025 05:56
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.

Thanks for all the cycles, looks great :)

@tgross35 tgross35 enabled auto-merge August 11, 2025 05:58
@tgross35 tgross35 added this pull request to the merge queue Aug 11, 2025
Merged via the queue into rust-lang:main with commit bbafd45 Aug 11, 2025
48 of 52 checks passed
tgross35 pushed a commit to tgross35/rust-libc that referenced this pull request Sep 19, 2025
(backport <rust-lang#4632>)
(cherry picked from commit bbafd45)
@tgross35 tgross35 mentioned this pull request Sep 19, 2025
github-merge-queue bot pushed a commit that referenced this pull request Sep 19, 2025
(backport <#4632>)
(cherry picked from commit bbafd45)
github-merge-queue bot pushed a commit that referenced this pull request Sep 19, 2025
(backport <#4632>)
(cherry picked from commit bbafd45)
@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 Sep 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.

3 participants