C++: Rework hex literals parsing #644
Conversation
Codecov Report
@@ Coverage Diff @@
## master #644 +/- ##
==========================================
- Coverage 92.86% 92.68% -0.19%
==========================================
Files 23 23
Lines 3546 3526 -20
Branches 375 376 +1
==========================================
- Hits 3293 3268 -25
- Misses 144 147 +3
- Partials 109 111 +2 |
chfast
force-pushed
the
hex_inline
branch
3 times, most recently
from
f09a7da
to
a476d6e
Compare
chfast
force-pushed
the
hex_literals
branch
2 times, most recently
from
4fa9b27
to
67ad485
Compare
include/evmc/evmc.hpp
Outdated
| byte(s, 14), byte(s, 15), byte(s, 16), byte(s, 17), byte(s, 18), byte(s, 19), byte(s, 20), | ||
| byte(s, 21), byte(s, 22), byte(s, 23), byte(s, 24), byte(s, 25), byte(s, 26), byte(s, 27), | ||
| byte(s, 28), byte(s, 29), byte(s, 30), byte(s, 31)}}}; | ||
| return (s == "0") ? T{} : from_hex<T>(s).value(); |
There was a problem hiding this comment.
Oh nice, from_hex is constexpr noexcept.
Is the remove_prefix really constexpr in it though?
There was a problem hiding this comment.
Yes, because this is string_view so it just moves the pointer and decreases size. https://en.cppreference.com/w/cpp/string/basic_string_view/remove_prefix.
chfast
force-pushed
the
hex_to_T
branch
5 times, most recently
from
95805a2
to
a65e4a9
Compare
| /// Converts a raw literal into value of type T. | ||
| /// | ||
| /// This function is expected to be used on literals in constexpr context only. | ||
| /// In case the input is invalid the std::terminate() is called. |
There was a problem hiding this comment.
Will the .value() result in terminate in case of std::nullopt?
There was a problem hiding this comment.
The .value() throws an exception which will call std::terminate() because the function is noexcept. This is not great if the execution happens at runtime (can happen also for literals) but better than UB with *from_hex(). This is going to be corrected with consteval in C++20 (then execution must happen at compile time).
There was a problem hiding this comment.
Unfortunately, clang-tidy also notice this and report warning bugprone-exception-escape.
| @@ -138,6 +138,7 @@ class ExampleHost : public evmc::Host | |||
|
|
|||
| evmc_tx_context get_tx_context() const noexcept final { return tx_context; } | |||
|
|
|||
| // NOLINTNEXTLINE(bugprone-exception-escape) | |||
There was a problem hiding this comment.
Requires #649.