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

Would it make sense to add copy_from_slice() like functions? #63

Closed
pedrocr opened this issue Jan 15, 2017 · 3 comments
Closed

Would it make sense to add copy_from_slice() like functions? #63

pedrocr opened this issue Jan 15, 2017 · 3 comments

Comments

@pedrocr
Copy link

pedrocr commented Jan 15, 2017

I use byteorder in my rawloader crate to read binary files with specific orderings. For example here's the code to read 16bit unpacked little-endian:

https://github.com/pedrocr/rawloader/blob/8f1a100e8f8d611efabd3c14db8e5050e9f0ebbe/src/decoders/basics.rs#L376-L384

On little-endian machines this could just be a memcpy. Currently rustc isn't able to optimize loops into memcpy but maybe it would make sense to have an explicit API for this inside byteorder, the same way copy_from_slice already allows that in standard types. That way I could call a LittleEndian::read_u16s(from, to, n) function in byteorder and if the architecture matches it does a memcpy, if not does a loop or even uses a BSWAP instruction. Would that make sense as part of the scope for this crate?

@BurntSushi
Copy link
Owner

I think I'd be open to this, and it seems like it could be implemented purely in new default methods on the ByteOrder trait so that there wouldn't be any breaking changes.

What are the downsides of these APIs? Do endian encoding/decoding libraries in other languages have an analogous API?

@newpavlov
Copy link
Contributor

I don't think it can be done through existing methods in zero-cost fashion. Will it surely break backward compatibility if we add new required methods, implemented by both enums? If yes, then we'll have to introduce additional trait, something like ByteOrderSliceExt.

I will create PR with the draft shortly.

@BurntSushi
Copy link
Owner

I think we want the ByteOrder trait to be sealed, so adding new required methods should be fine.

newpavlov added a commit to newpavlov/byteorder that referenced this issue Jun 5, 2017
newpavlov added a commit to newpavlov/byteorder that referenced this issue Jun 5, 2017
BurntSushi pushed a commit that referenced this issue Jul 9, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants