struct AlignedVec: Verify alignment for T
#1369
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Note that I have not verified if any element type/chunk type combinations could fail the requirement in practice beyond a cursory review of some instantiations. Among them, |
kkysen
changed the title
struct AlignedVec: Verify alignment for T
kkysen
approved these changes
Nov 11, 2024
There was a problem hiding this comment.
Thanks for finding this! I guess we forgot to check that alignment requirement. Could you just adjust the comments a bit and fix that typo in sufficiently?
Also, verifying this in the constructor is indeed fine. We already do that for a similar size/alignment check.
Turning the pointer of an aligned chunk into a reference to the overlay element type requires us to verify the element type's alignment invariant on the address. This check need not happen dynamically if we can provide a stronger static assertion. The use of `AlignedVec` is over-aligning the storage with regards to the elements. This guarantees that all aligned block addresses are at least as aligned as required for elements. With this commit we validate that only vectors with appropriate element types can be instantiated. This verifies the constructor instead of the type parameters due to simplicity. Turning the violation into a compile error may be beneficial during development but should not be strictly necessary. Passing through a constructor is a dominator code path of the creation of a reference from its storage (at least right now).
kkysen
approved these changes
Nov 11, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Turning the pointer of an aligned chunk into a reference to the overlay element type requires us to verify the element type's alignment invariant on the address. This check need not happen dynamically if we can provide a stronger static assertion.
The use of
AlignedVecis over-aligning the storage with regards to the elements. This guarantees that all aligned block addresses are at least as aligned as required for elements. With this commit we validate that only vectors with appropriate element types can be instantiated. This verifies the constructor instead of the type parameters due to simplicity. Turning the violation into a compile error may be beneficial during development but should not be strictly necessary. Passing through a constructor is a dominator code path of the creation of a reference from its storage (at least right now).