Conversation
jswrenn
force-pushed
the
self-hygiene
branch
5 times, most recently
from
d555cae to
e3c5276
Compare
|
Unfortunately, with the added zerocopy/zerocopy-derive/src/lib.rs Lines 320 to 327 in e3c5276 This is the test case that exhibits the ICE: zerocopy/zerocopy-derive/tests/struct_known_layout.rs Lines 50 to 59 in ac7b8e8 The problematic quality here is the lifetimes in the trailing field. Lifetimes in leading fields are unproblematic. Not yet sure how to work around this. |
Hot take: what if we don't work around it? The likelihood of someone encountering this in practice is extremely low since our MSRV is so old. If someone does, we can explain that it's a compiler bug and thus not really our fault. I'd want to figure out what toolchain fixed this ICE. If it's a recent toolchain, then I wouldn't want to go this route, but if it's like 1.58 or something, then I'm okay with it. |
|
The last problematic rustc version is 1.61, released May 19, 2022. |
Static lifetimes are also unproblematic. |
jswrenn
force-pushed
the
self-hygiene
branch
4 times, most recently
from
87046ff to
2a40051
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## v0.8.x #2127 +/- ##
=======================================
Coverage 87.42% 87.42%
=======================================
Files 16 16
Lines 6115 6115
=======================================
Hits 5346 5346
Misses 769 769 ☔ View full report in Codecov by Sentry. |
jswrenn
force-pushed
the
self-hygiene
branch
5 times, most recently
from
b4e7382 to
c7cfc8d
Compare
| #(::zerocopy::util::macro_util::core_reexport::mem::MaybeUninit< | ||
| <#ident #ty_generics as | ||
| ::zerocopy::util::macro_util::Field<#leading_field_indices> | ||
| >::Type | ||
| >,)* |
There was a problem hiding this comment.
Comment here explaining why we indirect via Field rather than just using the field types directly?
joshlf
left a comment
joshlf
left a comment
There was a problem hiding this comment.
One more change requested, but otherwise LGTM.
This commit revises the `KnownLayout` derive on `repr(C)` target structs to
preserve the correct resolution of `Self` when constructing
`__ZerocopyKnownLayoutMaybeUninit`. This type contains a `MaybeUninit` version
of each of the target struct's field types. Previously, it achieved this by
simply copying the tokens corresponding to field types from the definition of
the target struct into the definition of `__ZerocopyKnownLayoutMaybeUninit`
However, on types like this:
#[repr(C)]
struct Struct([u8; Self::N]);
…this approach is insufficient. Pasted into `__ZerocopyKnownLayoutMaybeUninit`,
`Self` ceases to refer to the target struct and instead refers to
`__ZerocopyKnownLayoutMaybeUninit`.
To preserve `Self` hygiene, this commit defines a struct for projecting the
field types of the target struct based on their index:
pub unsafe trait Field<Index> {
type Type: ?Sized;
}
…then implements it for each of the field types of the target struct; e.g.:
struct Index<const N: usize>;
impl Field<Index<0>> for Struct {
type Type = [u8; Self::N];
}
With this, the fields of `__ZerocopyKnownLayoutMaybeUninit` can be defined
hygienically; e.g., as `<Struct as Field<0>>::Type`.
Fixes #2116
This commit revises the `KnownLayout` derive on `repr(C)` target structs to
preserve the correct resolution of `Self` when constructing
`__ZerocopyKnownLayoutMaybeUninit`. This type contains a `MaybeUninit` version
of each of the target struct's field types. Previously, it achieved this by
simply copying the tokens corresponding to field types from the definition of
the target struct into the definition of `__ZerocopyKnownLayoutMaybeUninit`
However, on types like this:
#[repr(C)]
struct Struct([u8; Self::N]);
…this approach is insufficient. Pasted into `__ZerocopyKnownLayoutMaybeUninit`,
`Self` ceases to refer to the target struct and instead refers to
`__ZerocopyKnownLayoutMaybeUninit`.
To preserve `Self` hygiene, this commit defines a struct for projecting the
field types of the target struct based on their index:
pub unsafe trait Field<Index> {
type Type: ?Sized;
}
…then implements it for each of the field types of the target struct; e.g.:
struct Index<const N: usize>;
impl Field<Index<0>> for Struct {
type Type = [u8; Self::N];
}
With this, the fields of `__ZerocopyKnownLayoutMaybeUninit` can be defined
hygienically; e.g., as `<Struct as Field<0>>::Type`.
Fixes google#2116
This commit revises the `KnownLayout` derive on `repr(C)` target structs to
preserve the correct resolution of `Self` when constructing
`__ZerocopyKnownLayoutMaybeUninit`. This type contains a `MaybeUninit` version
of each of the target struct's field types. Previously, it achieved this by
simply copying the tokens corresponding to field types from the definition of
the target struct into the definition of `__ZerocopyKnownLayoutMaybeUninit`
However, on types like this:
#[repr(C)]
struct Struct([u8; Self::N]);
…this approach is insufficient. Pasted into `__ZerocopyKnownLayoutMaybeUninit`,
`Self` ceases to refer to the target struct and instead refers to
`__ZerocopyKnownLayoutMaybeUninit`.
To preserve `Self` hygiene, this commit defines a struct for projecting the
field types of the target struct based on their index:
pub unsafe trait Field<Index> {
type Type: ?Sized;
}
…then implements it for each of the field types of the target struct; e.g.:
struct Index<const N: usize>;
impl Field<Index<0>> for Struct {
type Type = [u8; Self::N];
}
With this, the fields of `__ZerocopyKnownLayoutMaybeUninit` can be defined
hygienically; e.g., as `<Struct as Field<0>>::Type`.
Fixes #2116
Co-authored-by: Jack Wrenn <[email protected]>
Originally introduced in #2127, this trait is used to ensure that field types which mention `Self` resolve correctly (always to the original type, and never to generated utility types). gherrit-pr-id: Gf808298c8219fddf1b406e7bb4357a4f3bb40b82
3 participants
This commit revises the
KnownLayoutderive onrepr(C)target structs to preserve the correct resolution ofSelfwhen constructing__ZerocopyKnownLayoutMaybeUninit. This type contains aMaybeUninitversion of each of the target struct's field types. Previously, it achieved this by simply copying the tokens corresponding to field types from the definition of the target struct into the definition of__ZerocopyKnownLayoutMaybeUninitHowever, on types like this:
…this approach is insufficient. Pasted into
__ZerocopyKnownLayoutMaybeUninit,Selfceases to refer to the target struct and instead refers to__ZerocopyKnownLayoutMaybeUninit.To preserve
Selfhygiene, this commit defines a struct for projecting the field types of the target struct based on their index:…then implements it for each of the field types of the target struct; e.g.:
With this, the fields of
__ZerocopyKnownLayoutMaybeUninitcan be defined hygienically; e.g., as<Struct as Field<0>>::Type.Fixes #2116