-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
Use read_unaligned instead of read in transmute_copy #55052
Conversation
Closes: rust-lang#55044 This change could result in performance regression on non-x86 platforms. Alternative would be to update `transmute_copy` with alignment requirements.
(rust_highfive has picked a reviewer for you, use r? to override) |
This seems like the right thing to do to me… |
@withoutboats |
Ping from triage @withoutboats / @rust-lang/libs: This PR requires your review. |
@rfcbot fcp merge I would personally agree that these semantics are correct for this function, not requiring the alignments match up. I'd like to make sure others agree, though |
Team member @alexcrichton has proposed to merge this. The next step is review by the rest of the tagged teams: No concerns currently listed. Once a majority of reviewers approve (and none object), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up! See this document for info about what commands tagged team members can give me. |
If we're worried about the performance we can always compare the source and target alignments and call read/read_unaligned as appropriate. It'll all get constant folded out. |
🔔 This is now entering its final comment period, as per the review above. 🔔 |
@bors: r+ |
📌 Commit 4210cca has been approved by |
⌛ Testing commit 4210cca with merge 90b4a012ad212427dd1c0e3ab72c2ea24bfd0013... |
💔 Test failed - status-travis |
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
Use read_unaligned instead of read in transmute_copy Closes: #55044 This change could result in performance regression on non-x86 platforms. (but it also can fix some of UB which lurks in existing programs) An alternative would be to update `transmute_copy` documentation with alignment requirements.
☀️ Test successful - status-appveyor, status-travis |
Tested on commit rust-lang/rust@c4371c8. Direct link to PR: <rust-lang/rust#55052> 💔 rls on linux: test-pass → test-fail (cc @nrc @Xanewok, @rust-lang/infra).
The RLS test failure is:
|
Closes: #55044
This change could result in performance regression on non-x86 platforms. (but it also can fix some of UB which lurks in existing programs) An alternative would be to update
transmute_copy
documentation with alignment requirements.