[AIX] Ignore linting on repr(C) structs with repr(packed) or repr(align(n)) #138206
Conversation
…gn(n)) 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.
|
r? @SparrowLii rustbot has assigned @SparrowLii. Use |
rustbot
added
S-waiting-on-review
|
Does AIX have any docs for these layout rules? Might be useful to link to them in a comment if there are. |
|
@jieyouxu Thanks for asking. There are some documentation in https://www.ibm.com/docs/en/xl-c-and-cpp-aix/16.1?topic=data-using-alignment-modes#alignment and https://www.ibm.com/docs/en/xl-c-and-cpp-aix/16.1?topic=data-using-alignment-modes. The layout rules are also described in https://github.com/rust-lang/rust/blob/master/compiler/rustc_lint/src/types.rs#L748. |
|
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 😸 |
|
Ping. |
| @@ -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() | |||
There was a problem hiding this comment.
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()))
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Thanks @SparrowLii for the review and Thanks @workingjubilee for the reply - yes, this should only affect structures annotated with repr(C).
|
Approving, but the correct fix is actually llvm/llvm-project#133599 |
bors
added
S-waiting-on-bors
|
@bors rollup |
…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)). 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.
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
|
⌛ Testing commit f86a71d with merge 46424fb5054f211ec836c5c03159f92e46bb35ac... |
|
☀️ Test successful - checks-actions |
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 differencesShow 2 test diffsAdditionally, 2 doctest diffs were found. These are ignored, as they are noisy. Job group index Job duration changes
How to interpret the job duration changes?Job durations can vary a lot, based on the actual runner instance |
|
Finished benchmarking commit (46424fb): comparison URL. Overall result: no relevant changes - no action needed@rustbot label: -perf-regression Instruction countThis benchmark run did not return any relevant results for this metric. Max RSS (memory usage)This benchmark run did not return any relevant results for this metric. CyclesThis benchmark run did not return any relevant results for this metric. Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 777.758s -> 776.657s (-0.14%) |
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.