Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Use #[derive] for Copy/Clone in s! and friends #4038

Merged
merged 2 commits into from
Nov 16, 2024
Merged

Use #[derive] for Copy/Clone in s! and friends #4038

merged 2 commits into from
Nov 16, 2024

Conversation

bossmc
Copy link
Contributor

@bossmc bossmc commented Nov 15, 2024

Description

Extracted from #3261 as a pre-requisite change.

Allows use of #[cfg] to skip items in an s! block without creating invalid, orphaned impl blocks.

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

@bossmc
Use #[derive] for Copy/Clone in s! and friends.
44c548d 
Allows use of `#[cfg]` to skip items in an `s!` block without creating
invalid, orphaned `impl` blocks.
@bossmc
Add fanotify_event_info_fid to FAM-exempt types
73ce07c
@rustbot
Copy link
Collaborator

rustbot commented Nov 15, 2024

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

@bossmc bossmc mentioned this pull request Nov 15, 2024
Merged
tgross35
tgross35 approved these changes Nov 15, 2024
View reviewed changes
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.

Ideally I'd like to backport this but that can't happen until we bump the MSRV of libc-0.2 (hopefully soon).

Also can't merge this right away due to a CI failure in recent nightly, LGTM though.

@rustbot label +stable-blocked

@bossmc
Copy link
Contributor Author

bossmc commented Nov 15, 2024

How does this change impact the MSRV? (I tried building with the currently declared 1.13.0 but that is completely broken, it can't even resolve the Cargo.toml since some versions of the cc crate have a feature named after a dependency)

@tgross35
Copy link
Contributor

For 0.2 we test down to 1.19 - do the derives work on 1.19? If so then we can backport with no problem, I was just assuming they wouldn't since we don't have this already, but could be wrong there.

There is no problem for main since that only goes down to 1.63.

@bossmc
Copy link
Contributor Author

bossmc commented Nov 15, 2024

Deriving Copy and Clone was present in Rust 1.0, my theory for why they're manually implemented in s! is to ensure that the generated code for Clone is a memcpy even in debug builds (the unoptimized implementation of Clone is a recursive call into Clone for each field, even if those fields are all Copy). As discussed in the other MR, if that is a genuine problem for developers they can chose to compile libc with optimizations even if they're compiling the rest of their crate(s) in debug mode.

I'm pretty sure this change can be ported to the 0.2 branch with the above caveat if you wished.

@tgross35 tgross35 added stable-nominated This PR should be considered for cherry-pick to libc's stable release branch and removed stable-blocked labels Nov 16, 2024
@tgross35 tgross35 enabled auto-merge November 16, 2024 05:00
@tgross35 tgross35 added this pull request to the merge queue Nov 16, 2024
@tgross35 tgross35 added stable-blocked and removed stable-nominated This PR should be considered for cherry-pick to libc's stable release branch labels Nov 16, 2024
Merged via the queue into rust-lang:main with commit 2e2e733 Nov 16, 2024
43 checks passed
@bossmc bossmc deleted the derive-copy-clone branch November 16, 2024 14:29
@tgross35 tgross35 added stable-nominated This PR should be considered for cherry-pick to libc's stable release branch and removed stable-blocked labels Nov 16, 2024
tgross35 pushed a commit to tgross35/rust-libc that referenced this pull request Nov 18, 2024
@bossmc @tgross35
Use #[derive] for Copy/Clone in s! and friends.
d528e8a 
Allows use of `#[cfg]` to skip items in an `s!` block without creating
invalid, orphaned `impl` blocks.

(backport <rust-lang#4038>)
(cherry picked from commit 44c548d)
tgross35 pushed a commit to tgross35/rust-libc that referenced this pull request Nov 18, 2024
@bossmc @tgross35
Add fanotify_event_info_fid to FAM-exempt types
ff99eb9 
(backport <rust-lang#4038>)
(cherry picked from commit 73ce07c)
@tgross35 tgross35 mentioned this pull request Nov 18, 2024
Merged
tgross35 pushed a commit to tgross35/rust-libc that referenced this pull request Nov 18, 2024
@bossmc @tgross35
Use #[derive] for Copy/Clone in s! and friends.
c43b3bc 
Allows use of `#[cfg]` to skip items in an `s!` block without creating
invalid, orphaned `impl` blocks.

(backport <rust-lang#4038>)
(cherry picked from commit 44c548d)
tgross35 pushed a commit to tgross35/rust-libc that referenced this pull request Nov 18, 2024
@bossmc @tgross35
Add fanotify_event_info_fid to FAM-exempt types
9e96ccd 
(backport <rust-lang#4038>)
(cherry picked from commit 73ce07c)
tgross35 pushed a commit to tgross35/rust-libc that referenced this pull request Nov 18, 2024
@bossmc @tgross35
Use #[derive] for Copy/Clone in s! and friends.
eb48d77 
Allows use of `#[cfg]` to skip items in an `s!` block without creating
invalid, orphaned `impl` blocks.

(backport <rust-lang#4038>)
(cherry picked from commit 44c548d)
tgross35 pushed a commit to tgross35/rust-libc that referenced this pull request Nov 18, 2024
@bossmc @tgross35
Add fanotify_event_info_fid to FAM-exempt types
717934c 
(backport <rust-lang#4038>)
(cherry picked from commit 73ce07c)
tgross35 pushed a commit to tgross35/rust-libc that referenced this pull request Nov 18, 2024
@bossmc @tgross35
Use #[derive] for Copy/Clone in s! and friends.
269e9f5 
Allows use of `#[cfg]` to skip items in an `s!` block without creating
invalid, orphaned `impl` blocks.

(backport <rust-lang#4038>)
(cherry picked from commit 44c548d)
tgross35 pushed a commit to tgross35/rust-libc that referenced this pull request Nov 18, 2024
@bossmc @tgross35
Add fanotify_event_info_fid to FAM-exempt types
70e2542 
(backport <rust-lang#4038>)
(cherry picked from commit 73ce07c)
@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 Nov 18, 2024
@theemathas theemathas mentioned this pull request Nov 27, 2024
Merged
tgross35 added a commit to tgross35/rust-libc that referenced this pull request Nov 28, 2024
@tgross35
Fix the build with rustc-dep-of-std
93cec56 
Since [1] we use derive macros rather than manually implementing `Clone`
and `Copy`. However, this caused the build in `std` to start failing
since the `core` prelude is not available. This provides the derive
macros as well as `derive` itself.

Resolve this by using complete paths. Additionally allow
`internal_features` to suppress the warning using `link_cfg`.

Link: rust-lang#4038 [1]
@tgross35 tgross35 mentioned this pull request Nov 28, 2024
Merged
tgross35 added a commit to tgross35/rust-libc that referenced this pull request Nov 28, 2024
@tgross35
Fix the build with rustc-dep-of-std
d69ad56 
Since [1] we use derive macros rather than manually implementing `Clone`
and `Copy`. However, this caused the build in `std` to start failing
since the `core` prelude is not available. This provides the derive
macros as well as `derive` itself.

Resolve this by using complete paths. Additionally allow
`internal_features` to suppress the warning using `link_cfg`, and change
to using global paths for all uses of `core`.

Link: rust-lang#4038 [1]
tgross35 added a commit to tgross35/rust-libc that referenced this pull request Nov 28, 2024
@tgross35
Fix the build with rustc-dep-of-std
4ca22fa 
Since [1] we use derive macros rather than manually implementing `Clone`
and `Copy`. However, this caused the build in `std` to start failing
since the `core` prelude is not available. This provides the derive
macros as well as `derive` itself.

Resolve this by using complete paths. Additionally allow
`internal_features` to suppress the warning using `link_cfg`, and change
to using global paths for all uses of `core`.

Link: rust-lang#4038 [1]

(backport <rust-lang#4158>)
(cherry picked from commit d69ad56)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tgross35 tgross35 tgross35 approved these changes

Assignees

@tgross35 tgross35

Labels
S-waiting-on-review stable-applied This PR has been cherry-picked to libc's stable release branch
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@bossmc @rustbot @tgross35