Introduce TypedValue to fizzy::test #658
Conversation
Codecov Report
@@ Coverage Diff @@
## master #658 +/- ##
=======================================
Coverage 98.38% 98.39%
=======================================
Files 69 71 +2
Lines 9724 9765 +41
=======================================
+ Hits 9567 9608 +41
Misses 157 157
Flags with carried forward coverage won't be shown. Click here to find out more.
|
chfast
force-pushed
the
typed_value
branch
2 times, most recently
from
997929e
to
cd75e9f
Compare
| /// No validation is possible, so the correctness is on the user only. | ||
| constexpr TypedValue(ValType ty, Value v) noexcept : value{v}, type{ty} {} | ||
|
|
||
| constexpr TypedValue(int32_t v) noexcept : TypedValue{ValType::i32, v} {} |
There was a problem hiding this comment.
This is in my options minimal set of constructors. Problem here is that on macOS uint64_t is defined differently than on Linux. On macOS the literals of long type are ambiguous, e.g. TypedValue(0x100000000) and it must be explicitly converted to uint64_t as TypedValue(0x100000000_u64).
| static_assert(i32.type == ValType::i32); | ||
| static_assert(i32.value.i64 == uint32_t(-1)); | ||
| EXPECT_EQ(i32.type, ValType::i32); | ||
| EXPECT_EQ(i32.value.i64, uint32_t(-1)); |
There was a problem hiding this comment.
Why have both sattic_assert and EXPECT_EQ?
There was a problem hiding this comment.
Performing check in constexpr and non-contexpr context. This first one does not give code coverage.
test/utils/typed_value.hpp
Outdated
| constexpr TypedValue(float v) noexcept : TypedValue{ValType::f32, v} {} | ||
| constexpr TypedValue(double v) noexcept : TypedValue{ValType::f64, v} {} | ||
|
|
||
| constexpr operator Value() const noexcept { return value; } |
There was a problem hiding this comment.
I don't know any more. Some future PRs propose to drop it.
There was a problem hiding this comment.
It was used ones, I dropped it. It is always easy to drop the type information by .value.
There was a problem hiding this comment.
Yeah, I was concerned that with it can be misleading e.g.
TypedValue v{1_u64};
if (v == Value{1_i32}) // true
|
|
||
| TEST(typed_value, construct_contexpr) | ||
| { | ||
| constexpr TypedValue i32{int32_t{-1}}; |
There was a problem hiding this comment.
Nit: some value literals are defined with {} others with ().
Are _i32 literals merged yet? Can't we use them here?
There was a problem hiding this comment.
Ah I got it, it was intentional, because somewhere conversion is happening.
No description provided.