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
Add support for long double
#75
Conversation
Need to ensure that all write serializers support this type.
@@ -138,10 +138,12 @@ namespace leap { | |||
|
|||
virtual void WriteFloat(float value) { WriteByteArray(&value, sizeof(float)); } | |||
virtual void WriteFloat(double value) { WriteByteArray(&value, sizeof(double)); } | |||
virtual void WriteFloat(long double value) { WriteByteArray(&value, sizeof(double)); } |
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.
Should that be sizeof(long double)
?
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.
For correctness, yes. I'm surprised that the unit test didn't catch this one. I'll add a test that does catch it, then fix this spot.
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.
Looks like this wasn't getting caught because sizeof(double) == sizeof(long double)
on my particular build, and also, the build server actually does catch this failure case correctly. Easy enough to fix.
01070cc
to
e444ad4
Compare
c35c2a9
to
4293a7b
Compare
In |
Actually there's a lot more to it than that--I have to add a new |
1db8030
to
3437574
Compare
Clang warns that you should add explicit braces to avoid a dangling else in |
Oh clang. |
You are missing a case statement for |
Need to ensure that all write serializers support this type.