Switch to intx 0.8.0 endian facilities #615
Conversation
yperbasis
requested review from
AndreaLanfranchi,
canepat,
chfast and
mriccobene
yperbasis
changed the title
yperbasis
changed the title
Codecov Report
@@ Coverage Diff @@
## master #615 +/- ##
==========================================
- Coverage 81.78% 81.66% -0.12%
==========================================
Files 167 167
Lines 13966 13925 -41
==========================================
- Hits 11422 11372 -50
- Misses 2544 2553 +9
Continue to review full report at Codecov.
|
core/silkworm/common/endian.hpp
Outdated
| std::memcpy(&x, bytes, sizeof(x)); | ||
| return be::load(x); | ||
| } | ||
| inline uint16_t load_big_u16(const uint8_t* bytes) noexcept { return intx::be::unsafe::load<uint16_t>(bytes); } |
There was a problem hiding this comment.
Just wondering if it's worth to simply make aliases of intx's functions (which is apparently what we're doing here).
Less boilerplate code and more clear intention imho
Something like :
const auto load_big_u16 = intx::be::unsafe::load<uint16_t>;
const auto load_big_u32 = intx::be::unsafe::load<uint32_t>;
const auto load_big_u64 = intx::be::unsafe::load<uint64_t>;
const auto load_little_u16 = intx::le::unsafe::load<uint16_t>;
const auto load_little_u32 = intx::le::unsafe::load<uint32_t>;
const auto load_little_u64 = intx::le::unsafe::load<uint64_t>;
const auto store_big_u16 = intx::be::unsafe::store<uint16_t>;
const auto store_big_u32 = intx::be::unsafe::store<uint32_t>;
const auto store_big_u64 = intx::be::unsafe::store<uint64_t>;
There was a problem hiding this comment.
Nice! Done in fbf499e.
There was a problem hiding this comment.
Or even
template <typename T, typename = std::enable_if_t<std::is_integral_v<T> && std::is_unsigned_v<T>>>
const auto load_big = intx::be::unsafe::load<T>;
and then use it as
const auto state_mask{endian::load_big<uint16_t>(v.data()) };
I don't think load_big_u16 vs load_big<uint16_t> makes a big difference in typed keys.
There was a problem hiding this comment.
I think then it makes sense to get rid of the aliases completely and only call the intx functions directly. I can do that.
There was a problem hiding this comment.
I think then it makes sense to get rid of the aliases completely and only use the intx functions directly. I can do that.
I personally wouldn't do that. as in namespace endian we already have to/from_big_compact probably keeping aliases makes sense so we don't have to change namespaces. But is a matter of taste
There was a problem hiding this comment.
OK, then to my mind the current version with the aliases (same as the boost names, e.g. load_big_u32) is fine.
Switch to the new endian facilities introduced in release 0.8.0 of intx.