Conversation
|
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @yaahc (or someone else) soon. Please see the contribution instructions for more information. |
|
Since it's now (#95016) agreed to be sound, consider adding (Or maybe it should be |
This comment has been minimized.
This comment has been minimized.
|
r? @scottmcm |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
Wow, is there really no usage of |
It doesn't surprise me, really. As the feature explanation says, it's a niche optimization path where it's extremely rare for it to be actually relevant. And the building blocks that are foundational enough to use it tend to be in |
|
Oh, I missed that before I pushed that commit. Should I just used normal |
This comment has been minimized.
This comment has been minimized.
Naw, it's fine. It's plausibly helpful for optimization, since otherwise LLVM doesn't know that the slice didn't get shorter. |
|
Another use case is to convert &[[T; N]; M] to &[T; N * M]. That's a case of array reshaping in general. |
This comment has been minimized.
This comment has been minimized.
|
🤦🏻♂️ |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Cyborus04
left a comment
There was a problem hiding this comment.
oh uh messed this one up a bit
|
As far as I know we don't want to use anything that's still I found a few more nits, and I have one extra request: Can you add So something like #[test]
#[should_panic(expect = "vec len overflow")]
fn vec_into_flattened_panics_for_capacity_overflow() {
let n = 1 << (usize::BITS / 2);
let _ = vec![[(); n]; n].into_flattened();
}which would probably go in https://github.com/rust-lang/rust/blob/master/library/alloc/tests/vec.rs And analogous ones for the two slice methods. When you're done, please comment |
This comment has been minimized.
This comment has been minimized.
|
wait i put it in the wrong file 😅 |
|
What do I do about the feature flag issue? There's other tests that use unstable features without any |
|
I think you need to allow the new feature in the tests library, like all of these ones rust/library/alloc/tests/lib.rs Lines 1 to 40 in c2afaba |
aabf8d1 to
405aaba
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
scottmcm
left a comment
There was a problem hiding this comment.
Let's see if it'll just let me commit these suggestions...
|
Ohhh, thank you! I've been confused about this all day, glad it's finally solved 😀 |
|
uh oh i think i destroyed your commit by accident, sorry. forgot to pull first |
…N]>::into_flattened`
|
Looks good! Thanks for your perseverance through all the hiccups. @bors r+ |
|
📌 Commit 06788fd has been approved by |
Rollup of 7 pull requests Successful merges: - rust-lang#95102 (Add known-bug for rust-lang#95034) - rust-lang#95579 (Add `<[[T; N]]>::flatten{_mut}`) - rust-lang#95634 (Mailmap update) - rust-lang#95705 (Promote x86_64-unknown-none target to Tier 2 and distribute build artifacts) - rust-lang#95761 (Kickstart the inner usage of `macro_metavar_expr`) - rust-lang#95782 (Windows: Increase a pipe's buffer capacity to 64kb) - rust-lang#95791 (hide an #[allow] directive from the Arc::new_cyclic doc example) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
|
Congrats on your first PR landing, @Cyborus04! |
…, r=Mark-Simulacrum Add a codegen test for `slice::from_ptr_range` I noticed back in rust-lang#95579 that this didn't optimize as well as it should. It's better now, after rust-lang#95837 changed the code in `from_ptr_range` and llvm/llvm-project#54824 was fixed in LLVM 15. So here's a test to keep it generating the good version.
Adds
flattento convert&[[T; N]]to&[T](andflatten_mutfor&mut [[T; N]]to&mut [T])