Skip to content

Conversation

dvdhrm
Copy link
Contributor

@dvdhrm dvdhrm commented Jul 21, 2023

Include the computed alignment of the violating field when rejecting transparent types with non-trivially aligned ZSTs.

ZST member fields in transparent types must have an alignment of 1 (to ensure it does not raise the layout requirements of the transparent field). The current error message looks like this:

 LL | struct Foobar(u32, [u32; 0]);
    |                    ^^^^^^^^ has alignment larger than 1

This patch changes the report to include the alignment of the violating field:

 LL | struct Foobar(u32, [u32; 0]);
    |                    ^^^^^^^^ has alignment of 4, which is larger than 1

In case of unknown alignments, it will yield:

 LL | struct Foobar(u32, [u32; 0]);
    |                    ^^^^^^^^ may have alignment larger than 1

This allows developers to get a better grasp why a specific field is rejected. Knowing the alignment of the violating field makes it easier to judge where that alignment-requirement originates, and thus hopefully provide better hints on how to mitigate the problem.

This idea was proposed in 2022 in #98071 as part of a bigger change. This commit simply extracts this error-message change, to decouple it from the other diagnostic improvements.

(Originally proposed by @compiler-errors in #98071)

@rustbot
Copy link
Collaborator

rustbot commented Jul 21, 2023

r? @jackh726

(rustbot has picked a reviewer for you, use r? to override)

@rustbot 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 Jul 21, 2023
@rust-log-analyzer

This comment has been minimized.

@dvdhrm
Copy link
Contributor Author

dvdhrm commented Jul 21, 2023

(tidy failed; fixed incorrect code formatting)

@rust-log-analyzer

This comment has been minimized.

Include the computed alignment of the violating field when rejecting
transparent types with non-trivially aligned ZSTs.

ZST member fields in transparent types must have an alignment of 1 (to
ensure it does not raise the layout requirements of the transparent
field). The current error message looks like this:

 LL | struct Foobar(u32, [u32; 0]);
    |                    ^^^^^^^^ has alignment larger than 1

This patch changes the report to include the alignment of the violating
field:

 LL | struct Foobar(u32, [u32; 0]);
    |                    ^^^^^^^^ has alignment of 4, which is larger than 1

In case of unknown alignments, it will yield:

 LL | struct Foobar<T>(u32, [T; 0]);
    |                       ^^^^^^ may have alignment larger than 1

This allows developers to get a better grasp why a specific field is
rejected. Knowing the alignment of the violating field makes it easier
to judge where that alignment-requirement originates, and thus hopefully
provide better hints on how to mitigate the problem.

This idea was proposed in 2022 in rust-lang#98071 as part of a bigger change.
This commit simply extracts this error-message change, to decouple it
from the other diagnostic improvements.
@dvdhrm
Copy link
Contributor Author

dvdhrm commented Jul 21, 2023

(fixed test assuming Align32 means 4-byte align, rather than 32-byte)

@jackh726
Copy link
Member

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented Jul 21, 2023

📌 Commit b0dadff has been approved by jackh726

It is now in the queue for this repository.

@bors 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 Jul 21, 2023
bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 21, 2023
…iaskrgr

Rollup of 4 pull requests

Successful merges:

 - rust-lang#113887 (new solver: add a separate cache for coherence)
 - rust-lang#113910 (Add FnPtr ty to SMIR)
 - rust-lang#113913 (error/E0691: include alignment in error message)
 - rust-lang#113914 (rustc_target: drop duplicate code)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 4a90553 into rust-lang:master Jul 21, 2023
@rustbot rustbot added this to the 1.73.0 milestone Jul 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants