-
Notifications
You must be signed in to change notification settings - Fork 34
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
Implement floating point const and add #424
Conversation
246fdf8
to
dc41d64
Compare
Yes, makes sense.
…On Thu, Jul 16, 2020, 15:44 Andrei Maiboroda ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In lib/fizzy/value.hpp
<#424 (comment)>:
> + float f32;
+
+ Value() = default;
+
+ constexpr Value(uint64_t v) noexcept : i64{v} {}
+ constexpr Value(uint32_t v) noexcept : i64{v} {}
+ constexpr Value(int64_t v) noexcept : i64{static_cast<uint64_t>(v)} {}
+ constexpr Value(int32_t v) noexcept : i64{static_cast<uint32_t>(v)} {}
+
+ constexpr Value(float v) noexcept : f32{v} {}
+
+ constexpr operator uint64_t() const noexcept { return i64; }
+
+ /// Get the value as the given type. Handy in templates.
+ template <typename T>
+ constexpr T as() const noexcept
Can this be left without default implementation and then have
specializations for uint32_t and uint64_t similar to float?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#424 (review)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAEL7RGC67KW3EBQS7XBGMLR33725ANCNFSM4O2WT76A>
.
|
Codecov Report
@@ Coverage Diff @@
## master #424 +/- ##
==========================================
- Coverage 99.54% 99.51% -0.03%
==========================================
Files 51 52 +1
Lines 14663 14791 +128
==========================================
+ Hits 14596 14719 +123
- Misses 67 72 +5 |
21a8bbe
to
d90f329
Compare
Doesnt this need spectest expectation update in CI? |
I was expecting that, but no. What is the conditions for skipping a spec test? |
d90f329
to
accd4b2
Compare
e0b6b1c
to
c4b6629
Compare
TEST(value, constructor_from_floating_points) | ||
{ | ||
EXPECT_EQ(Value{123.456f}.f32, 123.456f); | ||
EXPECT_EQ(Value{123.456789001}.f64, 123.456789001); |
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.
Would be nice to have a test for min/max, with a large mantissa.
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.
You mean the total max/min, or the biggest mantissa only?
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 used some special values in tests.
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.
Basically something which can not be represented in f32. f64::max()
is fine, but not sure what that is actually storing, I believe a large exponent?
A symbolic milestone starting floating point support implementation.