-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
Allow ValTree::try_to_raw_bytes
on u8
array
#99358
Conversation
r? @nagisa (rust-highfive has picked a reviewer for you, use r? to override) |
// build-pass | ||
//[mir] compile-flags: -Zunpretty=mir |
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.
IMO a separate test should be added. It is not super clear from looking at #70408 why this unpretty is necessary or why it is relevant to what’s described in the issue.
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.
I can split it out, I just didn't want to duplicate the file.
b2567ce
to
db98ffd
Compare
.collect::<Vec<_>>(); | ||
|
||
return Some(tcx.arena.alloc_from_iter(leafs.into_iter())); |
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.
is it possible to not collect as an intermediate step but just pass the iterator directly to the arena?
@@ -0,0 +1,14 @@ | |||
// build-pass | |||
// compile-flags: -Zunpretty=mir |
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.
if you can reproduce with -Zdump-mir=main
or -Zdump-mir=function_with_bytes
, then you could turn this into a mir opt test instead of a ui test with mir output.
r? @oli-obk |
db98ffd
to
91e91d8
Compare
@rustbot ready Converted to mir-opt test and addressed comments. |
}, | ||
_ => {} | ||
// `[u8; N]` can be interpreted as raw bytes | ||
ty::Array(array_ty, _) if *array_ty == tcx.types.u8 => {} |
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.
That's odd... why is this one not behind a reference?
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.
Is the caller peeling references? But only for Sized pointers?
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.
No, the old implementation just used to never hit this path because for [u8, _]
we got the raw bytes of the allocation: 705d818#diff-b56d8c5fa3d7803c5b0e19e1a8854d85c302974484c4c20bf51c76a19556f4a7L1459 -- it was rewritten to use valtree.try_to_raw_bytes
but I guess never tested to hit this ICE.
@bors r+ |
…laumeGomez Rollup of 7 pull requests Successful merges: - rust-lang#94247 (Fix slice::ChunksMut aliasing) - rust-lang#99358 (Allow `ValTree::try_to_raw_bytes` on `u8` array) - rust-lang#99651 (Deeply deny fn and raw ptrs in const generics) - rust-lang#99710 (lint: add bad opt access internal lint) - rust-lang#99717 (Add some comments to the docs issue template to clarify) - rust-lang#99728 (Clean up HIR-based lifetime resolution) - rust-lang#99812 (Fix headings colors) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
Fixes #99325
cc @b-naber I think who touched this last in 705d818