Skip to content
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

Support List(T) passthrough #436

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ErikMcClure
Copy link

This is an implementation of capnproto/capnproto#1807 for allowing List(T) in schemas. Essentially, it is treated as an AnyList parameter on the wire, but the type information is preserved in the schema, which allows the C++ version to implement a proxy List(T) over the AnyList that enforces the correct type usage.

I could not find an equivalent to capnp-c++'s AnyList class, it appears that AnyLists are simply converted to any_pointers and it is left to the user to cast them appropriately. So, this change simply replaces rejecting a List(AnyPointer) by replacing it with an any_pointer, the same way AnyList is handled. This change doesn't break any existing tests, but also does not add any tests either, because any test for this feature would have to be gated behind whatever version of capnp ends up incorporating List(T) support.

It may be possible to restrict the acceptance of List(AnyPointer) to only cases where the any_pointer is an actual parameter (which is what was done in the C++ version) but I wasn't sure how to do that.

@dwrensha
Copy link
Member

dwrensha commented Oct 3, 2023

Thanks. This looks okay to me, but I'd like to wait until upstream actually adds this feature before we add support here.

It may be possible to restrict the acceptance of List(AnyPointer) to only cases where the any_pointer is an actual parameter (which is what was done in the C++ version) but I wasn't sure how to do that.

Hm... yeah TypedAnyList may be possible in Rust via traits with associated types. But I do worry that making it work would add a lot of complexity.

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.

2 participants