Open Erigara opened 6 months ago
I would suggest the approach should be to update the docs to remove this guarantee. The main problem is that whilst you're correct that the current implementation is not aligned to 128 bits, your proposed implementation won't be either!
You can see this in your playground link if you run it normally (i.e. not with miri) where the error if the alignments don't match displays that u128
is 8 byte aligned (i.e. only 64 bits).
So we would have two options for fixing the alignment, but both break the principle of least surprise, imo:
repr(c, align(16))
+
Aligns to 128 bits-
The alignment on some platforms will be greater than than of u128
_align: [u128; 0]
+
The alignment across all platforms will match u128
's-
The alignment is not guaranteed to be 128 bitsA separate issue to removing the docs is still whether the maintainer wishes to change the alignment to match that of u128
, which wouldn't be unreasonable. However I think the doc need to be updated to fix this issue regardless.
You can see this in your playground link if you run it normally (i.e. not with miri) where the error if the alignments don't match displays that u128 is 8 byte aligned (i.e. only 64 bits).
Yeah, i've tried to use repr(c, align(16))
initially to align to 128bits
, but i forgot that u128
alignment is tricky.
I've noticed that while docs claim that
Decimal
is 128bit aligned it's not the case which could lead to UB (playground run with miri).Either docs should be edited of fix implemented.
I can work on the fix by introducing new ZST field
_align: [u128; 0]
intoDecimal
which would force proper alignment of the type to be equal tou128
.