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

Feature proposal: Support for sized arrays #21

Open
ElectronicRU opened this issue May 10, 2021 · 6 comments
Open

Feature proposal: Support for sized arrays #21

ElectronicRU opened this issue May 10, 2021 · 6 comments
Labels
enhancement New feature or request

Comments

@ElectronicRU
Copy link
Contributor

Since const generics MVP hit the stable, it should be possible to implement AsStd* for sized arrays.

The corresponding Std* type would need to be a sized array of a generated struct type with a field for underlying element's Std* representation, and a field for stride-correcting padding.

I believe the implementation should be rather straightforward, I'll gladly take on it if it is deemed worthwhile (well, I'd certainly like to use it personally).

@LPGhatguy LPGhatguy added the enhancement New feature or request label May 10, 2021
@LPGhatguy
Copy link
Owner

This would be awesome!

@ElectronicRU
Copy link
Contributor Author

I'm gonna wait until we resolve #20 to properly work on it, since the work there also applies to arrays.

@ElectronicRU
Copy link
Contributor Author

ElectronicRU commented May 11, 2021

Bleh. Poked around, and of course impl<T: Std140> AsStd140 for T and impl<T: AsStd140, const N> AsStd140 for [T; N] are conflicting, since there's no way for Rust to know if someone would implement Std140 for their [T; N].

One option to forge ahead seems to be to include the impl for [T; N] for concrete T in the derive macro (potentially extending Std140 with associated padded type), and do impl Std140 for [T; N] for primitive types manually.

Another option is to remove blanket impl for AsStd140, but I suspect this mildly breaks downstream :).

Third option, I suppose, is to add another explicit trait, e.g. Std140Convertible, that doesn't have a blanket impl, and provide blanket impls for AsStd140 for both Std140Convertible and Std140. In the derive macro, we would then derive Std140Convertible, it would have no blanket impls, so we can impl it for arrays without conflict. EDIT: oh lol, nevermind, the blanket impls would still be conflicting.

What think? Seems like only the first option is viable.

@ElectronicRU
Copy link
Contributor Author

Currently blocked by Lokathor/bytemuck#59 making it to crates.io.

@ElectronicRU
Copy link
Contributor Author

Even worse than I thought - we cannot emit impl AsStd140 for arrays because arrays are always foreign. :(

@ElectronicRU
Copy link
Contributor Author

One possible solution I see is thus:

  1. Extend AsStd140 to feature two different Std140 types: one without padding at end, one with. Padding at end is cruelly almost impossible to determine at compile time using generics (and we cannot use proc macros for arrays, because arrays are always foreign). For structs, which are logically always padded at end, these types may for simplicity be the exact same - this wastes a little bit of space but seems to align with people's expectations better anyway, see Added proper padding after structs (and matrices). #20 comments.
  2. Since we cannot have two blanket impls of AsStd140, the next best thing is to special-case the arrays in the derive macro: whenever we see a type that's [T; N], we convert it to a type [T::Std140TypePadded; N] using e.g. a generic function.

Pros:

  • Chances of actually being possible to implement.
  • Doesn't even require nightly features.
    Cons:
  • Bloats the interfaces and unpleasantly hacky.
  • Arrays will be a second-class citizens - they'll need to be wrapped in a struct for it to work.

Thoughts?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants