-
Notifications
You must be signed in to change notification settings - Fork 13.4k
Raise alignment limit from 2^15 to 2^31 - 1 #43097
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -60,6 +60,13 @@ struct AlignContainsPacked { | |
b: Packed, | ||
} | ||
|
||
// The align limit was originally smaller (2^15). | ||
// Check that it works with big numbers. | ||
#[repr(align(0x10000))] | ||
struct AlignLarge { | ||
stuff: [u8; 0x10000], | ||
} | ||
|
||
impl Align16 { | ||
// return aligned type | ||
pub fn new(i: i32) -> Align16 { | ||
|
@@ -193,4 +200,14 @@ pub fn main() { | |
assert_eq!(mem::align_of_val(&a.b), 1); | ||
assert_eq!(mem::size_of_val(&a), 16); | ||
assert!(is_aligned_to(&a, 16)); | ||
|
||
let mut arr = [0; 0x10000]; | ||
arr[0] = 132; | ||
let large = AlignLarge { | ||
stuff: arr, | ||
}; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Try using There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good point, I thought an upstream change might result in that but I didn't think to avoid it with |
||
assert_eq!(large.stuff[0], 132); | ||
assert_eq!(mem::align_of::<AlignLarge>(), 0x10000); | ||
assert_eq!(mem::align_of_val(&large), 0x10000); | ||
assert!(is_aligned_to(&large, 0x10000)); | ||
} |
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should keep the check here though (and update it for 31 bits).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this struct care about the 31 bit limit? I've done the checking for that closer to the parsing. The previous limit here was due to the limitations of using just 1 nibble, but now it has no such limit.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The parsing is for the user-facing value. Here we must protect any internal guarantees.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fair enough. Will fix.