Skip to content

Allow transmute-ing for tuples with [u8; <const N: usize>] #110438

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

Closed
wants to merge 1 commit into from

Conversation

JulianKnodt
Copy link
Contributor

@JulianKnodt JulianKnodt commented Apr 17, 2023

This allows transmuting tuples, but currently only supports u8 or other single byte types, because I did not add any way to factor additions.

Why this is desired is it will allow for concatenation of arrays "without copying", (although likely they will have to be copied to be adjacent in memory?). As opposed to before which handled multiplication of layouts within arrays which can be thought of as reshaping, this allows for stacking arrays.

For example, if I have:
transmute( ( [u32; N], [u32; M] ) ) == [u32; N + M], it is equivalent to 4N + 4M = 4 (N + M). I do not currently factor the 4 out.

This also doesn't handle subtractions, since I didn't want to have to think about underflow (not sure if that would be desired in the future. It is also unclear how to handle N + N = 2N, which I suspect likely wouldn't be handled.

I think this PR still lacks what I hope would be complete functionality, but is enough to ask for feedback on how to implement the missing pieces.

@rustbot
Copy link
Collaborator

rustbot commented Apr 17, 2023

r? @cjgillot

(rustbot has picked a reviewer for you, use r? to override)

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Apr 17, 2023

fn concat_tuple<const W: usize, const H: usize>(
v: ([u8; W], [u8; H]),
) -> [u8; W + H] {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is that even defined to have equivalent layout? 😅

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's not

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🙈 oh no, I could probably implement this soundly for types that have #[repr(C)], that also contain no additional padding and alignment constraints?

Copy link
Member

@the8472 the8472 Apr 18, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tuples are laid out according to the default representation.

That's repr(Rust), which means their field order isn't defined. So you can do a concat if they're homogenous and have no padding but the order wouldn't be defined according to spec. The current layout algorithm doesn't touch homogenous structs but that's not guaranteed.
But it does reorder them if W != H.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(there is also -Zrandomize-layout, repr(Rust) really can have a "weird" layout).

I could probably implement this soundly for types that have #[repr(C)], that also contain no additional padding and alignment constraints?

Technically — yes. However, I'm very concerned with the maintenance cost, it seems like those special cases make improvements in const eval/generics harder, while also not being the desired end-goal of their work.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, so what I'm saying is this could instead be implemented on ADTs, which are #[repr(C)] instead (and other constraints on alignment and padding). The ultimate point of this PR is to allow for concatenating arrays by making them contiguous in memory, then (possibly) transmuting them together, so it seems that going out of the way to make transmuting work is probably not the right path

@JulianKnodt JulianKnodt deleted the tuple_add branch May 31, 2023 18:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants