More align_offset things by oli-obk · Pull Request #44537 · rust-lang/rust (original) (raw)
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 andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Conversation15 Commits1 Checks0 Files changed
Conversation
This file contains hidden or 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
[ Show hidden characters]({{ revealButtonHref }})
(rust_highfive has picked a reviewer for you, use r? to override)
| let mut offset; |
|---|
| if align > 0 { |
| offset = cmp::min(usize_bytes - align, len); |
| let mut offset = unsafe { align_offset(ptr as *const _, usize_bytes) }; |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be using the inherent methods added in this PR?
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The compiler complained that it doesn't know the unstable feature (on stage0). This would become a mess of cfgs, so I opted to not do this here.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds like a stage0 inherent method on pointers which does the old method via casts and not(stage0) uses the intrinsic?
| /// remove me after the next release |
|---|
| pub fn align_offset(self, align: usize) -> usize { |
| unsafe { |
| intrinsics::align_offset(self as *const _, align) |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks the same as the above function? I think the #[cfg] in the intrinsics module should be enough?
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It differs in the stability. I couldn't use the function within core without giving core the corresponding feature, which then complained about an unknown feature.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure I understand this? Can't the stage0 shim in the intrinsics module be just as unstable as the real intrinsic in non-stage0?
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I must have misread the error messages I got. It works now without duplicating the inherent methods. The intrinsics are still duplicated.
📌 Commit 1462cab has been approved by alexcrichton
frewsxcv added a commit to frewsxcv/rust that referenced this pull request
☔ The latest upstream changes (presumably #43964) made this pull request unmergeable. Please resolve the merge conflicts.
bors added a commit that referenced this pull request
Rollup of 19 pull requests
- Successful merges: #44273, #44356, #44395, #44531, #44537, #44542, #44560, #44567, #44574, #44577, #44586, #44589, #44590, #44593, #44598, #44606, #44609, #44616, #44631
- Failed merges:
📌 Commit 2787a28 has been approved by alexcrichton
alexcrichton added a commit to alexcrichton/rust that referenced this pull request
bors added a commit that referenced this pull request
alexcrichton added a commit to alexcrichton/rust that referenced this pull request
bors added a commit that referenced this pull request
Rollup of 11 pull requests
- Successful merges: #44364, #44466, #44537, #44548, #44640, #44651, #44657, #44661, #44668, #44671, #44675
- Failed merges:
Mark-Simulacrum added a commit to Mark-Simulacrum/rust that referenced this pull request
make memrchr use align_offset
I hope I did not screw that up...
Cc @oli-obk who authored the original rust-lang#44537
bors added a commit that referenced this pull request
make memrchr use align_offset
I hope I did not screw that up...