Improve diagnostic for E0691 #98071
Improve diagnostic for E0691 #98071
Conversation
rustbot
added
the
T-compiler
|
r? @wesleywiser (rust-highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
This comment has been minimized.
This comment has been minimized.
lukas-code
force-pushed
the
repr-transparent-zero-array
branch
2 times, most recently
from
21c81a5
to
40fa282
Compare
lukas-code
force-pushed
the
repr-transparent-zero-array
branch
from
40fa282
to
bd3d1ae
Compare
| let array_len = match ty.kind() { | ||
| Array(_, len) => len.try_eval_usize(tcx, param_env), | ||
| _ => None, | ||
| }; | ||
| let zst = array_len == Some(0) || layout.map_or(false, |layout| layout.is_zst()); |
There was a problem hiding this comment.
Wait, doesn't this change the actual typechecking behavior?
There was a problem hiding this comment.
well, i guess not necessarily, because the align check below will still fail?
I would rather move this is-array check to line 1348, like if (zst || zero_sized_array) && align != Some(1) so we don't need to edit this logic, just in case?
There was a problem hiding this comment.
I've split zero_sized_array from zst, but the logic stays the same, otherwise we would get #98064 again.
There was a problem hiding this comment.
Also with a more complex zero-sized but non-trivially-aligned field, we still get the same wrong diagnostic:
struct ComplexZst<A, B> {
a: [A; 0],
b: [B; 0],
}
#[repr(transparent)]
struct Aligned<T, A, B> {
align: ComplexZst<A, B>,
value: T,
}error[E0690]: transparent struct needs at most one non-zero-sized field, but has 2
--> src/lib.rs:7:1
|
7 | struct Aligned<T, A, B> {
| ^^^^^^^^^^^^^^^^^^^^^^^ needs at most one non-zero-sized field, but has 2
8 | align: ComplexZst<A, B>,
| ----------------------- this field is non-zero-sized
9 | value: T,
| -------- this field is non-zero-sized
For more information about this error, try `rustc --explain E0690`.
Is it worth trying to fix this? Should there be something like ty.is_zst(tcx, param_env)? I am not really experienced with rustc internals.
|
I found a kind of related thing in #98104. |
rustbot
added
S-waiting-on-author
|
☔ The latest upstream changes (presumably #98206) made this pull request unmergeable. Please resolve the merge conflicts. |
|
@lukas-code |
|
@lukas-code @rustbot label: +S-inactive |
rustbot
added
the
S-inactive
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 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.
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.
error/E0691: include alignment in error message
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:
```text
LL | struct Foobar(u32, [u32; 0]);
| ^^^^^^^^ has alignment larger than 1
```
This patch changes the report to include the alignment of the violating field:
```text
LL | struct Foobar(u32, [u32; 0]);
| ^^^^^^^^ has alignment of 4, which is larger than 1
```
In case of unknown alignments, it will yield:
```text
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 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.
(Originally proposed by `@compiler-errors` in rust-lang#98071)
fixes #98064