From 85307b29d0dbdafa0fcf4f4396e87f04f0109a9b Mon Sep 17 00:00:00 2001 From: Scott Schurr Date: Wed, 13 Jan 2021 10:42:17 -0800 Subject: [PATCH] Add constexpr constructor for base_uint --- src/ripple/app/ledger/Ledger.cpp | 2 - src/ripple/app/ledger/impl/InboundLedgers.cpp | 1 - src/ripple/basics/base_uint.h | 129 +++++++++++++---- src/ripple/protocol/impl/Indexes.cpp | 8 +- src/ripple/protocol/impl/UintTypes.cpp | 7 +- src/ripple/shamap/impl/SHAMapSync.cpp | 1 - src/test/basics/base_uint_test.cpp | 54 +++++++ src/test/ledger/Directory_test.cpp | 6 +- src/test/rpc/AccountCurrencies_test.cpp | 1 - src/test/shamap/SHAMap_test.cpp | 137 +++++++----------- 10 files changed, 215 insertions(+), 131 deletions(-) diff --git a/src/ripple/app/ledger/Ledger.cpp b/src/ripple/app/ledger/Ledger.cpp index d1308c5890..e5b0bf3f60 100644 --- a/src/ripple/app/ledger/Ledger.cpp +++ b/src/ripple/app/ledger/Ledger.cpp @@ -1510,8 +1510,6 @@ loadByHashPostgres(uint256 const& ledgerHash, Application& app) static uint256 getHashByIndexPostgres(std::uint32_t ledgerIndex, Application& app) { - uint256 ret; - auto infos = loadLedgerInfosPostgres(ledgerIndex, app); assert(infos.size() <= 1); if (infos.size()) diff --git a/src/ripple/app/ledger/impl/InboundLedgers.cpp b/src/ripple/app/ledger/impl/InboundLedgers.cpp index b6f4dcdd96..9d04ff1eee 100644 --- a/src/ripple/app/ledger/impl/InboundLedgers.cpp +++ b/src/ripple/app/ledger/impl/InboundLedgers.cpp @@ -231,7 +231,6 @@ public: void gotStaleData(std::shared_ptr packet_ptr) override { - const uint256 uZero; Serializer s; try { diff --git a/src/ripple/basics/base_uint.h b/src/ripple/basics/base_uint.h index 3ada512b51..a6a33a5097 100644 --- a/src/ripple/basics/base_uint.h +++ b/src/ripple/basics/base_uint.h @@ -25,6 +25,7 @@ #ifndef RIPPLE_BASICS_BASE_UINT_H_INCLUDED #define RIPPLE_BASICS_BASE_UINT_H_INCLUDED +#include #include #include #include @@ -177,15 +178,94 @@ private: memcpy(data_.data(), data, bytes); } -public: - base_uint() + // Helper function to initialize a base_uint from a std::string_view. + enum class ParseResult { + okay, + badLength, + badChar, + }; + + constexpr std::pair + parseFromStringView(std::string_view sv) noexcept { - *this = beast::zero; + // Local lambda that converts a single hex char to four bits and + // ORs those bits into a uint32_t. + auto hexCharToUInt = [](char c, + std::uint32_t shift, + std::uint32_t& accum) -> ParseResult { + std::uint32_t nibble = 0xFFu; + if (c < '0' || c > 'f') + return ParseResult::badChar; + + if (c >= 'a') + nibble = static_cast(c - 'a' + 0xA); + else if (c >= 'A') + nibble = static_cast(c - 'A' + 0xA); + else if (c <= '9') + nibble = static_cast(c - '0'); + + if (nibble > 0xFu) + return ParseResult::badChar; + + accum |= (nibble << shift); + + return ParseResult::okay; + }; + + std::pair ret{ParseResult::okay, {}}; + + if (sv == "0") + { + return ret; + } + + if (sv.size() != size() * 2) + { + ret.first = ParseResult::badLength; + return ret; + } + + auto out = ret.second.data(); + + auto in = sv.begin(); + + while (in != sv.end()) + { + std::uint32_t accum = {}; + for (std::uint32_t shift : {4u, 0u, 12u, 8u, 20u, 16u, 28u, 24u}) + { + if (auto const result = hexCharToUInt(*in++, shift, accum); + result != ParseResult::okay) + { + ret.first = result; + return ret; + } + } + *out++ = accum; + } + return ret; } - base_uint(beast::Zero) + constexpr decltype(data_) + parseFromStringViewThrows(std::string_view sv) noexcept(false) + { + auto const result = parseFromStringView(sv); + if (result.first == ParseResult::badLength) + Throw("invalid length for hex string"); + + if (result.first == ParseResult::badChar) + Throw("invalid hex character"); + + return result.second; + } + +public: + constexpr base_uint() : data_{} + { + } + + constexpr base_uint(beast::Zero) : data_{} { - *this = beast::zero; } explicit base_uint(std::uint64_t b) @@ -193,6 +273,14 @@ public: *this = b; } + // This constructor is intended to be used at compile time since it might + // throw at runtime. Consider declaring this constructor consteval once + // we get to C++23. + explicit constexpr base_uint(std::string_view sv) noexcept(false) + : data_(parseFromStringViewThrows(sv)) + { + } + template < class Container, class = std::enable_if_t< @@ -225,7 +313,7 @@ public: return base_uint(data, VoidHelper()); } - int + constexpr int signum() const { for (int i = 0; i < WIDTH; i++) @@ -380,37 +468,18 @@ public: @param sv A null-terminated string of hexadecimal characters @return true if the input was parsed properly; false otherwise. */ - [[nodiscard]] bool + [[nodiscard]] constexpr bool parseHex(std::string_view sv) { - if (sv == "0") - { - zero(); - return true; - } - - if (sv.size() != bytes * 2) + auto const result = parseFromStringView(sv); + if (result.first != ParseResult::okay) return false; - auto out = data(); - - auto in = sv.begin(); - - while (in != sv.end()) - { - auto const hi = charUnHex(*in++); - auto const lo = charUnHex(*in++); - - if (hi == -1 || lo == -1) - return false; - - *out++ = static_cast((hi << 4) + lo); - } - + data_ = result.second; return true; } - [[nodiscard]] bool + [[nodiscard]] constexpr bool parseHex(const char* str) { return parseHex(std::string_view{str}); diff --git a/src/ripple/protocol/impl/Indexes.cpp b/src/ripple/protocol/impl/Indexes.cpp index 02148a5f15..6d7b7cc222 100644 --- a/src/ripple/protocol/impl/Indexes.cpp +++ b/src/ripple/protocol/impl/Indexes.cpp @@ -96,12 +96,8 @@ getBookBase(Book const& book) uint256 getQualityNext(uint256 const& uBase) { - static uint256 const nextq = []() { - uint256 x; - (void)x.parseHex( - "0000000000000000000000000000000000000000000000010000000000000000"); - return x; - }(); + static constexpr uint256 nextq( + "0000000000000000000000000000000000000000000000010000000000000000"); return uBase + nextq; } diff --git a/src/ripple/protocol/impl/UintTypes.cpp b/src/ripple/protocol/impl/UintTypes.cpp index 39e25a8136..ff2644b085 100644 --- a/src/ripple/protocol/impl/UintTypes.cpp +++ b/src/ripple/protocol/impl/UintTypes.cpp @@ -55,11 +55,8 @@ to_string(Currency const& currency) if (currency == noCurrency()) return "1"; - static Currency const sIsoBits = []() { - Currency c; - (void)c.parseHex("FFFFFFFFFFFFFFFFFFFFFFFF000000FFFFFFFFFF"); - return c; - }(); + static constexpr Currency sIsoBits( + "FFFFFFFFFFFFFFFFFFFFFFFF000000FFFFFFFFFF"); if ((currency & sIsoBits).isZero()) { diff --git a/src/ripple/shamap/impl/SHAMapSync.cpp b/src/ripple/shamap/impl/SHAMapSync.cpp index dd24e84e47..084702be9b 100644 --- a/src/ripple/shamap/impl/SHAMapSync.cpp +++ b/src/ripple/shamap/impl/SHAMapSync.cpp @@ -56,7 +56,6 @@ SHAMap::visitNodes(std::function const& function) const { while (pos < 16) { - uint256 childHash; if (!node->isEmptyBranch(pos)) { std::shared_ptr child = diff --git a/src/test/basics/base_uint_test.cpp b/src/test/basics/base_uint_test.cpp index 2ac8bbe8f6..5ed5ec4984 100644 --- a/src/test/basics/base_uint_test.cpp +++ b/src/test/basics/base_uint_test.cpp @@ -195,6 +195,60 @@ struct base_uint_test : beast::unit_test::suite BEAST_EXPECT(tmp.parseHex(s1)); BEAST_EXPECT(to_string(tmp) == s1); } + + // Constexpr constructors + { + static_assert(test96{}.signum() == 0); + static_assert(test96("0").signum() == 0); + static_assert(test96("000000000000000000000000").signum() == 0); + static_assert(test96("000000000000000000000001").signum() == 1); + static_assert(test96("800000000000000000000000").signum() == 1); + +// Everything within the #if should fail during compilation. +#if 0 + // Too few characters + static_assert(test96("00000000000000000000000").signum() == 0); + + // Too many characters + static_assert(test96("0000000000000000000000000").signum() == 0); + + // Non-hex characters + static_assert(test96("00000000000000000000000 ").signum() == 1); + static_assert(test96("00000000000000000000000/").signum() == 1); + static_assert(test96("00000000000000000000000:").signum() == 1); + static_assert(test96("00000000000000000000000@").signum() == 1); + static_assert(test96("00000000000000000000000G").signum() == 1); + static_assert(test96("00000000000000000000000`").signum() == 1); + static_assert(test96("00000000000000000000000g").signum() == 1); + static_assert(test96("00000000000000000000000~").signum() == 1); +#endif // 0 + + // Verify that constexpr base_uints interpret a string the same + // way parseHex() does. + struct StrBaseUint + { + char const* const str; + test96 tst; + + constexpr StrBaseUint(char const* s) : str(s), tst(s) + { + } + }; + constexpr StrBaseUint testCases[] = { + "000000000000000000000000", + "000000000000000000000001", + "fedcba9876543210ABCDEF91", + "19FEDCBA0123456789abcdef", + "800000000000000000000000", + "fFfFfFfFfFfFfFfFfFfFfFfF"}; + + for (StrBaseUint const& t : testCases) + { + test96 t96; + BEAST_EXPECT(t96.parseHex(t.str)); + BEAST_EXPECT(t96 == t.tst); + } + } } }; diff --git a/src/test/ledger/Directory_test.cpp b/src/test/ledger/Directory_test.cpp index 183e82d9a7..daedc3f192 100644 --- a/src/test/ledger/Directory_test.cpp +++ b/src/test/ledger/Directory_test.cpp @@ -325,12 +325,10 @@ struct Directory_test : public beast::unit_test::suite env.fund(XRP(10000), alice); env.close(); - uint256 base; - (void)base.parseHex( + constexpr uint256 base( "fb71c9aa3310141da4b01d6c744a98286af2d72ab5448d5adc0910ca0c910880"); - uint256 item; - (void)item.parseHex( + constexpr uint256 item( "bad0f021aa3b2f6754a8fe82a5779730aa0bbbab82f17201ef24900efc2c7312"); { diff --git a/src/test/rpc/AccountCurrencies_test.cpp b/src/test/rpc/AccountCurrencies_test.cpp index f1f4695900..b865aeb8eb 100644 --- a/src/test/rpc/AccountCurrencies_test.cpp +++ b/src/test/rpc/AccountCurrencies_test.cpp @@ -116,7 +116,6 @@ class AccountCurrencies_test : public beast::unit_test::suite result[fld].size() == expected.size(); for (size_t i = 0; stat && i < expected.size(); ++i) { - Currency foo; stat &= (to_string(expected[i].value().currency) == result[fld][i].asString()); diff --git a/src/test/shamap/SHAMap_test.cpp b/src/test/shamap/SHAMap_test.cpp index b7726ccc9d..182b443ce0 100644 --- a/src/test/shamap/SHAMap_test.cpp +++ b/src/test/shamap/SHAMap_test.cpp @@ -139,16 +139,15 @@ public: tests::TestNodeFamily f(journal); // h3 and h4 differ only in the leaf, same terminal node (level 19) - uint256 h1, h2, h3, h4, h5; - (void)h1.parseHex( + constexpr uint256 h1( "092891fe4ef6cee585fdc6fda0e09eb4d386363158ec3321b8123e5a772c6ca7"); - (void)h2.parseHex( + constexpr uint256 h2( "436ccbac3347baa1f1e53baeef1f43334da88f1f6d70d963b833afd6dfa289fe"); - (void)h3.parseHex( + constexpr uint256 h3( "b92891fe4ef6cee585fdc6fda1e09eb4d386363158ec3321b8123e5a772c6ca8"); - (void)h4.parseHex( + constexpr uint256 h4( "b92891fe4ef6cee585fdc6fda2e09eb4d386363158ec3321b8123e5a772c6ca8"); - (void)h5.parseHex( + constexpr uint256 h5( "a92891fe4ef6cee585fdc6fda0e09eb4d386363158ec3321b8123e5a772c6ca7"); SHAMap sMap(SHAMapType::FREE, f); @@ -223,57 +222,41 @@ public: else testcase("build/tear unbacked"); { - std::vector keys(8); - (void)keys[0].parseHex( - "b92891fe4ef6cee585fdc6fda1e09eb4d386363158ec3321b8123e5a772c6c" - "a8"); - (void)keys[1].parseHex( - "b92881fe4ef6cee585fdc6fda1e09eb4d386363158ec3321b8123e5a772c6c" - "a8"); - (void)keys[2].parseHex( - "b92691fe4ef6cee585fdc6fda1e09eb4d386363158ec3321b8123e5a772c6c" - "a8"); - (void)keys[3].parseHex( - "b92791fe4ef6cee585fdc6fda1e09eb4d386363158ec3321b8123e5a772c6c" - "a8"); - (void)keys[4].parseHex( - "b91891fe4ef6cee585fdc6fda1e09eb4d386363158ec3321b8123e5a772c6c" - "a8"); - (void)keys[5].parseHex( - "b99891fe4ef6cee585fdc6fda1e09eb4d386363158ec3321b8123e5a772c6c" - "a8"); - (void)keys[6].parseHex( - "f22891fe4ef6cee585fdc6fda1e09eb4d386363158ec3321b8123e5a772c6c" - "a8"); - (void)keys[7].parseHex( - "292891fe4ef6cee585fdc6fda1e09eb4d386363158ec3321b8123e5a772c6c" - "a8"); + constexpr std::array keys{ + uint256("b92891fe4ef6cee585fdc6fda1e09eb4d386363158ec3321b8123e" + "5a772c6ca8"), + uint256("b92881fe4ef6cee585fdc6fda1e09eb4d386363158ec3321b8123e" + "5a772c6ca8"), + uint256("b92691fe4ef6cee585fdc6fda1e09eb4d386363158ec3321b8123e" + "5a772c6ca8"), + uint256("b92791fe4ef6cee585fdc6fda1e09eb4d386363158ec3321b8123e" + "5a772c6ca8"), + uint256("b91891fe4ef6cee585fdc6fda1e09eb4d386363158ec3321b8123e" + "5a772c6ca8"), + uint256("b99891fe4ef6cee585fdc6fda1e09eb4d386363158ec3321b8123e" + "5a772c6ca8"), + uint256("f22891fe4ef6cee585fdc6fda1e09eb4d386363158ec3321b8123e" + "5a772c6ca8"), + uint256("292891fe4ef6cee585fdc6fda1e09eb4d386363158ec3321b8123e" + "5a772c6ca8")}; - std::vector hashes(8); - (void)hashes[0].parseHex( - "B7387CFEA0465759ADC718E8C42B52D2309D179B326E239EB5075C64B6281F" - "7F"); - (void)hashes[1].parseHex( - "FBC195A9592A54AB44010274163CB6BA95F497EC5BA0A8831845467FB2ECE2" - "66"); - (void)hashes[2].parseHex( - "4E7D2684B65DFD48937FFB775E20175C43AF0C94066F7D5679F51AE756795B" - "75"); - (void)hashes[3].parseHex( - "7A2F312EB203695FFD164E038E281839EEF06A1B99BFC263F3CECC6C74F93E" - "07"); - (void)hashes[4].parseHex( - "395A6691A372387A703FB0F2C6D2C405DAF307D0817F8F0E207596462B0E3A" - "3E"); - (void)hashes[5].parseHex( - "D044C0A696DE3169CC70AE216A1564D69DE96582865796142CE7D98A84D9DD" - "E4"); - (void)hashes[6].parseHex( - "76DCC77C4027309B5A91AD164083264D70B77B5E43E08AEDA5EBF943611436" - "15"); - (void)hashes[7].parseHex( - "DF4220E93ADC6F5569063A01B4DC79F8DB9553B6A3222ADE23DEA02BBE7230" - "E5"); + constexpr std::array hashes{ + uint256("B7387CFEA0465759ADC718E8C42B52D2309D179B326E239EB5075C" + "64B6281F7F"), + uint256("FBC195A9592A54AB44010274163CB6BA95F497EC5BA0A883184546" + "7FB2ECE266"), + uint256("4E7D2684B65DFD48937FFB775E20175C43AF0C94066F7D5679F51A" + "E756795B75"), + uint256("7A2F312EB203695FFD164E038E281839EEF06A1B99BFC263F3CECC" + "6C74F93E07"), + uint256("395A6691A372387A703FB0F2C6D2C405DAF307D0817F8F0E207596" + "462B0E3A3E"), + uint256("D044C0A696DE3169CC70AE216A1564D69DE96582865796142CE7D9" + "8A84D9DDE4"), + uint256("76DCC77C4027309B5A91AD164083264D70B77B5E43E08AEDA5EBF9" + "4361143615"), + uint256("DF4220E93ADC6F5569063A01B4DC79F8DB9553B6A3222ADE23DEA0" + "2BBE7230E5")}; SHAMap map(SHAMapType::FREE, f); if (!backed) @@ -303,31 +286,23 @@ public: testcase("iterate unbacked"); { - std::vector keys(8); - (void)keys[0].parseHex( - "f22891fe4ef6cee585fdc6fda1e09eb4d386363158ec3321b8123e5a772c6c" - "a8"); - (void)keys[1].parseHex( - "b99891fe4ef6cee585fdc6fda1e09eb4d386363158ec3321b8123e5a772c6c" - "a8"); - (void)keys[2].parseHex( - "b92891fe4ef6cee585fdc6fda1e09eb4d386363158ec3321b8123e5a772c6c" - "a8"); - (void)keys[3].parseHex( - "b92881fe4ef6cee585fdc6fda1e09eb4d386363158ec3321b8123e5a772c6c" - "a8"); - (void)keys[4].parseHex( - "b92791fe4ef6cee585fdc6fda1e09eb4d386363158ec3321b8123e5a772c6c" - "a8"); - (void)keys[5].parseHex( - "b92691fe4ef6cee585fdc6fda1e09eb4d386363158ec3321b8123e5a772c6c" - "a8"); - (void)keys[6].parseHex( - "b91891fe4ef6cee585fdc6fda1e09eb4d386363158ec3321b8123e5a772c6c" - "a8"); - (void)keys[7].parseHex( - "292891fe4ef6cee585fdc6fda1e09eb4d386363158ec3321b8123e5a772c6c" - "a8"); + constexpr std::array keys{ + uint256("f22891fe4ef6cee585fdc6fda1e09eb4d386363158ec3321b8123e" + "5a772c6ca8"), + uint256("b99891fe4ef6cee585fdc6fda1e09eb4d386363158ec3321b8123e" + "5a772c6ca8"), + uint256("b92891fe4ef6cee585fdc6fda1e09eb4d386363158ec3321b8123e" + "5a772c6ca8"), + uint256("b92881fe4ef6cee585fdc6fda1e09eb4d386363158ec3321b8123e" + "5a772c6ca8"), + uint256("b92791fe4ef6cee585fdc6fda1e09eb4d386363158ec3321b8123e" + "5a772c6ca8"), + uint256("b92691fe4ef6cee585fdc6fda1e09eb4d386363158ec3321b8123e" + "5a772c6ca8"), + uint256("b91891fe4ef6cee585fdc6fda1e09eb4d386363158ec3321b8123e" + "5a772c6ca8"), + uint256("292891fe4ef6cee585fdc6fda1e09eb4d386363158ec3321b8123e" + "5a772c6ca8")}; tests::TestNodeFamily tf{journal}; SHAMap map{SHAMapType::FREE, tf};