Skip to content

Conversation

cpetig
Copy link

@cpetig cpetig commented Aug 1, 2024

Resolves #181

@cpetig
Copy link
Author

cpetig commented Aug 1, 2024

Updated version of MR #304 with the conflicts resolved

@jeffparsons
Copy link

Question from the peanut gallery:

Has this change been approved in principle already — i.e. is it just the implementation that's awaiting review?

@lukewagner
Copy link
Member

Yes, I think I've heard broad agreement on this feature. But it is a good question of why not just merge the PR now, given that it's emoji-gated (so that it's clear it's not implemented everywhere) and not controversial. That's probably a good idea, so I'll do that next week unless anyone suggests otherwise.

@lukewagner
Copy link
Member

Rereading the PR just now, it seems like we'd want a low (but relaxable in the future) upper bound on the fixed length so that bindings generators don't have to think about rare pathological cases (similar to what we did for flags). Maybe 16?

@jeffparsons
Copy link

I'd argue for at least 64 to support the largest variants of SHA-2, SHA-3 and BLAKE3, which are all 512 bits, as list<u8, 64>.

I know this is a very narrow use case, but most of the others that are on my radar are smaller arrays, e.g., IPv6, UUID, coordinates and color values for "graphics stuff", etc.

@lukewagner
Copy link
Member

Yes, that's a good point. Given that I'm overdue from merging after my previous comment and this is a minor refinement, I'll merge this PR now as-is and file a new PR to add the length limits in a bit, and we can agree on 64 or whatever else there.

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.

Fixed length arrays
4 participants