[AIX] Ignore linting on repr(C) structs with repr(packed) or repr(align(n)) by amy-kwan · Pull Request #138206 · rust-lang/rust (original) (raw)

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 andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Conversation17 Commits1 Checks6 Files changed

Conversation

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters

[ Show hidden characters]({{ revealButtonHref }})

This PR updates the lint added in 9b40bd7 to ignore repr(C) structs that also have repr(packed) or repr(align(n)).

As these representations can be modifiers on repr(C), it is assumed that users that add these should know what they are doing, and thus the the lint should not warn on the respective structs. For example, for the time being, using repr(packed) and manually padding a repr(C) struct can be done to correctly align struct members on AIX.

…gn(n))

This PR updates the lint added in 9b40bd7 to ignore repr(C) structs that also have repr(packed) or repr(align(n)).

r? @SparrowLii

rustbot has assigned @SparrowLii.
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

rustbot added S-waiting-on-review

Status: Awaiting review from the assignee but also interested parties.

T-compiler

Relevant to the compiler team, which will review and decide on the PR/issue.

labels

Mar 8, 2025

Does AIX have any docs for these layout rules? Might be useful to link to them in a comment if there are.

FYI @workingjubilee in case you are also interested in reviewing this, as you reviewed my first patch for the AIX lint.

Ah okay, nevermind, it's above the diff 😸

Choose a reason for hiding this comment

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

This looks reasonable, but I'm not familair with lints, so r? compiler

@@ -1638,6 +1638,9 @@ impl ImproperCTypesDefinitions {
return true;
} else if let Adt(adt_def, _) = ty.kind()
&& adt_def.is_struct()
&& adt_def.repr().c()
&& !adt_def.repr().packed()
&& adt_def.repr().align.is_none()

Choose a reason for hiding this comment

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

Does this affect the strcuts without repr(C)? I guess this should be like this:

&& adt_def.is_struct()
&& (!adt_def.repr().c() || (!adt_def.repr().packed() && adt_def.repr().align.is_none()))

Choose a reason for hiding this comment

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

That is incorrect, this should only matter for repr(C). The && means that repr(C) must be satisfied first before we concern ourselves with the rest, which is correct.

Choose a reason for hiding this comment

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

Thanks @SparrowLii for the review and Thanks @workingjubilee for the reply - yes, this should only affect structures annotated with repr(C).

📌 Commit f86a71d has been approved by workingjubilee

It is now in the queue for this repository.

bors added S-waiting-on-bors

Status: Waiting on bors to run and complete tests. Bors will change the label on completion.

and removed S-waiting-on-review

Status: Awaiting review from the assignee but also interested parties.

labels

Mar 29, 2025

jhpratt added a commit to jhpratt/rust that referenced this pull request

Mar 30, 2025

…er-align-ignore-packed-align, r=workingjubilee

[AIX] Ignore linting on repr(C) structs with repr(packed) or repr(align(n))

This PR updates the lint added in 9b40bd7 to ignore repr(C) structs that also have repr(packed) or repr(align(n)).

bors added a commit to rust-lang-ci/rust that referenced this pull request

Mar 30, 2025

Rollup of 5 pull requests

Successful merges:

rust-lang#137836 (Set target_vendor = "openwrt" on mips64-openwrt-linux-musl)
rust-lang#138206 ([AIX] Ignore linting on repr(C) structs with repr(packed) or repr(align(n)))
rust-lang#139044 (bootstrap: Avoid cloning change-id list)
rust-lang#139092 (Move fd into std::sys)
rust-lang#139111 (Properly document FakeReads)

r? @ghost @rustbot modify labels: rollup

What is this?This is an experimental post-merge analysis report that shows differences in test outcomes between the merged PR and its parent PR.

Comparing 45b40a7 (parent) -> 46424fb (this PR)

Test differences

Show 2 test diffs

Additionally, 2 doctest diffs were found. These are ignored, as they are noisy.

Job group index

Job duration changes

x86_64-apple-1: 6789.9s -> 7383.6s (8.7%)
dist-x86_64-msvc: 5766.2s -> 6243.9s (8.3%)
x86_64-msvc-ext3: 7607.9s -> 8208.2s (7.9%)
aarch64-apple: 3577.0s -> 3836.9s (7.3%)
i686-gnu-nopt-2: 6345.7s -> 6621.7s (4.4%)
dist-x86_64-mingw: 7487.0s -> 7742.9s (3.4%)
x86_64-gnu-llvm-18-1: 5239.9s -> 5397.2s (3.0%)
i686-mingw-1: 7232.1s -> 7415.4s (2.5%)
i686-gnu-nopt-1: 8230.7s -> 8431.0s (2.4%)
dist-various-1: 4265.7s -> 4355.6s (2.1%) How to interpret the job duration changes?

Job durations can vary a lot, based on the actual runner instance
that executed the job, system noise, invalidated caches, etc. The table above is provided
mostly for t-infra members, for simpler debugging of potential CI slow-downs.