-
Notifications
You must be signed in to change notification settings - Fork 140
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
Move all endianness/byte-order CPP into one module #659
Conversation
, whenBigEndian | ||
) where | ||
|
||
data ByteOrder |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shall we reuse GHC.ByteOrder.ByteOrder
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It makes sense to do so. It can wait until I make a pass at removing the various old-ghc cruft, though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I mean, we can already reuse the type itself?..
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's added in base-4.11
while we currently have base >= 4.9
. I guess there is a compatibility package ghc-byteorder
but that seems like re-arranging deck chairs on the Titanic since we will discontinue support for such old ghc versions at our convenience.
This isn't a battle worth fighting, though.
|
||
module Data.ByteString.Utils.ByteOrder | ||
( ByteOrder(..) | ||
, hostByteOrder |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the name intentionally different from targetByteOrder
for cross-compilation purposes?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since bytestring
is not a compiler the notion of "target" doesn't really make sense. The name in base
is confusing.
(cherry picked from commit 161780a)
I can't be bothered to remember the right combination of
#include ...
and#ifdef
to check this. So I'd like to remove the need entirely.