Skip to content

Conversation

@azdle
Copy link

@azdle azdle commented Nov 26, 2025

Any type that implements From<Vec<T>>, Into<Vec<T>> and Deref<Target = Vec<T>> can be used for a WIT type list<T> via the with option.


This was simpler than I originally expected, by omitting the type names, I can just let the compiler fall back to calling <Vec<T> as Into<Vec<T>>::into when no custom type is used, allowing the codegen to not need to track the custom type.

Resolves #1441

@azdle azdle marked this pull request as draft November 26, 2025 20:41
@azdle azdle force-pushed the support-list-types branch from 153b870 to b3b75d8 Compare November 26, 2025 20:52
Any type that implements `From<Vec<T>>`, `Into<Vec<T>>` can be used for
a WIT type `list<T>` via the `with` option.
@azdle azdle force-pushed the support-list-types branch from b3b75d8 to f41ad1f Compare November 26, 2025 21:03
@azdle
Copy link
Author

azdle commented Nov 26, 2025

Status (mostly as a reminder to myself):

  • I ended up removing the impl for Deref<Target = Vec<T>> and the test passes, but I do actually need that impl in my real application. The test probably needs to test more, I'm just not sure exactly what yet.
  • When I don't have the deref impl the errors the compiler spits out are not the most helpful, I want to see if I can find some way to require that trait explicitly to get better errors.

Copy link
Member

@alexcrichton alexcrichton left a comment

Choose a reason for hiding this comment

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

Would this work for your use case with Bytes? Receiving a Bytes from the outside world would be Bytes: From<Vec<u8>> which is the best we can do, but sending a Bytes to the outside world would unnecessarily require a copy since the shared-reference of Bytes would be copied into a uniquely owned Vec<u8>

@azdle
Copy link
Author

azdle commented Dec 1, 2025

but sending a Bytes to the outside world would unnecessarily require a copy since the shared-reference of Bytes would be copied into a uniquely owned Vec

I'm not sure what shared reference you're talking about here.

Just to make sure we're on the same page, the Bytes type was talking about in the issue was struct Bytes(Vec<u8>); in the first code block, not a Bytes from the bytes crate. I didn't use the bytes crate because exactly for the reason that it would cause a fully copy of the data. [edit: Actually, this may not be true.]

@azdle
Copy link
Author

azdle commented Dec 1, 2025

I didn't use the bytes crate because exactly for the reason that it would cause a fully copy of the data.

I was assuming this, but looking at the bytes crate. I'm not sure this is actually true. It sounds like it's capable of converting a Bytes into a BytesMut (https://docs.rs/bytes/latest/bytes/struct.BytesMut.html#method.from) and BytesMut into a Vec<u8> (https://docs.rs/bytes/latest/src/bytes/bytes_mut.rs.html#1727-1760) without copying, as long as it's all unique.

I tried swapping out my Bytes type for the bytes crate Bytes and it all just worked. Though, I can't be sure if it's doing a full copy or not, but by my reading it shouldn't be.

@alexcrichton
Copy link
Member

I can ask perhaps a different question, are you looking for zero-copy when you pass a Bytes to an imported function? This PR, for example, does not enable that. Conversion from Bytes to Vec<u8> I believe always performs a copy. I'm less sure about BytesMut, but reading the code it looks like the answer is "sometimes".

@azdle
Copy link
Author

azdle commented Dec 4, 2025

Sorry if I added to the confusion by looking into the bytes crate types. But my original intent wasn't specifically about the Bytes and/or BytesMut types from the bytes crate. It was only about supporting newtype wrappers. The Bytes type in #1441 was just a newtype wrapper around a Vec<u8>.

When I started implementing this I just figured there was no reason to limit this to straight newtype wrappers and support anything that impl From<Vec<u8>> and Into<Vec<u8>>.

@azdle
Copy link
Author

azdle commented Dec 4, 2025

At the risk of adding to the confusion, I decided to dig into what the bytes crate does in this case.

As long as you're not doing anything 'interesting' with the Bytes you can roundtrip through it without causing a full copy of the data.

When converting from Vec<u8> to Bytes, it will:

  1. Transform the Vec into a boxed slice.
  2. Turn that into a Bytes with a 'promotable' vtable.

Then, as long as the Bytes hasn't been cloned (or otherwise turned into a KIND_ARC), it will:

  1. Call the into_vec from the promotable vtable.
  2. Call the inner promote function. (Or here)
  3. Rebuild the original Vec from its parts.

Though there is a call to ptr::copy, it's called with ptr and buf equal to each other since our Bytes is pointing to the start of the original Vec, which I have to imagine doesn't cause any actual copying to happen.

[Edit: Because I was curious, my assumption is essentially correct, at least on my system / version of glibc. As long as there's more than 512 bytes of data it'll do a check on src and dst and realize it doesn't need to do anything. At 512 or below it just copies into registers and back. Which could be not ideal in some cases, but probably isn't that big of a deal, though that's getting beyond my experience to know for sure.]

But again, this PR isn't specifically about the bytes crate's Bytes, I just happened to name my newtype struct Bytes in the original issue.


I also pushed more test cases that I wrote to prove to myself that you weren't implying something else, so that's unrelated and I plan to remove the changes around non-canonical lists because that definitely does have a copying problem that I'm not sure how I'd solve.

Copy link
Member

@alexcrichton alexcrichton left a comment

Choose a reason for hiding this comment

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

Ah ok I see now makes sense, thanks for clarifying!

While this wasn't really the intended purpose of the with key I think it's ok to go ahead and support this. I don't see any problems with it for example per se, and it's a convenient feature to have too.

Thanks for the PR again! With green CI I'd be happy to merge

test-helpers = { path = '../test-helpers' }
# For use with the custom attributes test
serde_json = "1"
bytes = "*"
Copy link
Member

Choose a reason for hiding this comment

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

Mind putting a version requirement here instead of just using *?

uwriteln!(self.src, "{result}.push(e{tmp});");
uwriteln!(self.src, "}}");
results.push(result);
results.push(format!("<_ as From<Vec<_>>>::from({result})"));
Copy link
Member

Choose a reason for hiding this comment

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

From CI I believe this is giong to want to use {vec} instead of Vec

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

rust: Support custom list types

2 participants