-
Notifications
You must be signed in to change notification settings - Fork 310
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
C++: Literals for basic types #359
Conversation
include/evmc/evmc.hpp
Outdated
constexpr uint8_t parse_hex_digit(uint8_t d) noexcept | ||
{ | ||
return static_cast<uint8_t>( | ||
(d >= '0' && d <= '9') ? d - '0' : (d >= 'a' && d <= 'f') ? d - 'a' + 10 : d - 'A' + 10); |
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.
Any chance to abort on invalid hex digits? E.g. 0x123kkx_address
?
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.
This will be C++ parser error. But I've found in C++17 you can have literals like 0x0.000000000000000000000000000000000000p0_address
so I added additional checks.
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 reckon it is not easy to add a test case (e.g. file which would not compile)?
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.
Not without creating custom testing framework in CMake.
Need to apply renaming to changelog. |
5d9a3e8
to
ed8ffa4
Compare
@@ -150,6 +150,110 @@ constexpr bytes32::operator bool() const noexcept | |||
return !is_zero(*this); | |||
} | |||
|
|||
namespace literals | |||
{ | |||
namespace internal |
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 internal stuff better go into separate header?
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.
So far I'm ok with having single header for C++ (now it's even better when helpers.hpp is gone).
No description provided.