Skip to content
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++: Rework hex literals parsing #644

Merged
merged 1 commit into from
Jun 2, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions examples/example_host.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why this? it's still noexcept

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

evmc::bytes32 get_block_hash(int64_t number) const noexcept final
{
const int64_t current_block_number = get_tx_context().block_number;
Expand Down
65 changes: 12 additions & 53 deletions include/evmc/evmc.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

#include <evmc/evmc.h>
#include <evmc/helpers.h>
#include <evmc/hex.hpp>

#include <functional>
#include <initializer_list>
Expand Down Expand Up @@ -280,69 +281,27 @@ inline constexpr bytes32::operator bool() const noexcept

namespace literals
{
namespace internal
{
constexpr int from_hex(char c) noexcept
{
return (c >= 'a' && c <= 'f') ? c - ('a' - 10) :
(c >= 'A' && c <= 'F') ? c - ('A' - 10) :
c - '0';
}

constexpr uint8_t byte(const char* s, size_t i) noexcept
{
return static_cast<uint8_t>((from_hex(s[2 * i]) << 4) | from_hex(s[2 * i + 1]));
}

/// 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.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this comment still valid?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will the .value() result in terminate in case of std::nullopt?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately, clang-tidy also notice this and report warning bugprone-exception-escape.

/// TODO(c++20): Use consteval.
template <typename T>
T from_hex(const char*) noexcept;

template <>
constexpr bytes32 from_hex<bytes32>(const char* s) noexcept
constexpr T parse(std::string_view s) noexcept
{
return {
{{byte(s, 0), byte(s, 1), byte(s, 2), byte(s, 3), byte(s, 4), byte(s, 5), byte(s, 6),
byte(s, 7), byte(s, 8), byte(s, 9), byte(s, 10), byte(s, 11), byte(s, 12), byte(s, 13),
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 from_hex<T>(s).value();
}

template <>
constexpr address from_hex<address>(const char* s) noexcept
{
return {
{{byte(s, 0), byte(s, 1), byte(s, 2), byte(s, 3), byte(s, 4), byte(s, 5), byte(s, 6),
byte(s, 7), byte(s, 8), byte(s, 9), byte(s, 10), byte(s, 11), byte(s, 12), byte(s, 13),
byte(s, 14), byte(s, 15), byte(s, 16), byte(s, 17), byte(s, 18), byte(s, 19)}}};
}

template <typename T, char... c>
constexpr T from_literal() noexcept
{
constexpr auto size = sizeof...(c);
constexpr char literal[] = {c...};

static_assert(size > 2 && literal[0] == '0' && literal[1] == 'x',
"literal must be in hexadecimal notation");
static_assert(size == 2 * sizeof(T) + 2, "literal must match the result type size");

return from_hex<T>(&literal[2]);
}
} // namespace internal

/// Literal for evmc::address.
template <char... c>
constexpr address operator""_address() noexcept
constexpr address operator""_address(const char* s) noexcept
{
return internal::from_literal<address, c...>();
return parse<address>(s);
}

/// Literal for evmc::bytes32.
template <char... c>
constexpr bytes32 operator""_bytes32() noexcept
constexpr bytes32 operator""_bytes32(const char* s) noexcept
{
return internal::from_literal<bytes32, c...>();
return parse<bytes32>(s);
}
} // namespace literals

Expand Down
4 changes: 4 additions & 0 deletions test/unittests/cpp_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -362,6 +362,10 @@ TEST(cpp, literals)
static_assert(zero_address == evmc::address{});
static_assert(zero_hash == evmc::bytes32{});

static_assert(0x00_address == 0x0000000000000000000000000000000000000000_address);
static_assert(0x01_address == 0x0000000000000000000000000000000000000001_address);
static_assert(0xf101_address == 0x000000000000000000000000000000000000f101_address);

EXPECT_EQ(0x0000000000000000000000000000000000000000_address, evmc::address{});
EXPECT_EQ(0x0000000000000000000000000000000000000000000000000000000000000000_bytes32,
evmc::bytes32{});
Expand Down