Skip to content

Commit

Permalink
refactor: replace hand-rolled lexicalCast (#4473)
Browse files Browse the repository at this point in the history
Replace hand-rolled code with std::from_chars for better
maintainability.

The C++ std::from_chars function is intended to be as fast as possible,
so it is unlikely to be slower than the code it replaces. This change is
a net gain because it reduces the amount of hand-rolled code.
  • Loading branch information
dangell7 authored Jun 27, 2023
1 parent beba871 commit 534a365
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 131 deletions.
13 changes: 5 additions & 8 deletions src/ripple/app/rdb/backend/detail/impl/Node.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -518,7 +518,7 @@ getHashByIndex(soci::session& session, LedgerIndex ledgerIndex)

std::string sql =
"SELECT LedgerHash FROM Ledgers INDEXED BY SeqLedger WHERE LedgerSeq='";
sql.append(beast::lexicalCastThrow<std::string>(ledgerIndex));
sql.append(std::to_string(ledgerIndex));
sql.append("';");

std::string hash;
Expand Down Expand Up @@ -581,9 +581,9 @@ getHashesByIndex(
{
std::string sql =
"SELECT LedgerSeq,LedgerHash,PrevHash FROM Ledgers WHERE LedgerSeq >= ";
sql.append(beast::lexicalCastThrow<std::string>(minSeq));
sql.append(std::to_string(minSeq));
sql.append(" AND LedgerSeq <= ");
sql.append(beast::lexicalCastThrow<std::string>(maxSeq));
sql.append(std::to_string(maxSeq));
sql.append(";");

std::uint64_t ls;
Expand Down Expand Up @@ -761,8 +761,7 @@ transactionsSQL(
boost::format("SELECT %s FROM AccountTransactions "
"WHERE Account = '%s' %s %s LIMIT %u, %u;") %
selection % toBase58(options.account) % maxClause % minClause %
beast::lexicalCastThrow<std::string>(options.offset) %
beast::lexicalCastThrow<std::string>(numberOfResults));
options.offset % numberOfResults);
else
sql = boost::str(
boost::format(
Expand All @@ -775,9 +774,7 @@ transactionsSQL(
"LIMIT %u, %u;") %
selection % toBase58(options.account) % maxClause % minClause %
(descending ? "DESC" : "ASC") % (descending ? "DESC" : "ASC") %
(descending ? "DESC" : "ASC") %
beast::lexicalCastThrow<std::string>(options.offset) %
beast::lexicalCastThrow<std::string>(numberOfResults));
(descending ? "DESC" : "ASC") % options.offset % numberOfResults);
JLOG(j.trace()) << "txSQL query: " << sql;
return sql;
}
Expand Down
140 changes: 17 additions & 123 deletions src/ripple/beast/core/LexicalCast.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,120 +23,19 @@
#include <algorithm>
#include <cassert>
#include <cerrno>
#include <charconv>
#include <cstdlib>
#include <iostream>
#include <iterator>
#include <limits>
#include <string>
#include <type_traits>
#include <typeinfo>
#include <utility>

#include <boost/predef.h>

namespace beast {

namespace detail {

#if BOOST_COMP_MSVC
#pragma warning(push)
#pragma warning(disable : 4800)
#pragma warning(disable : 4804)
#endif

template <class Int, class FwdIt, class Accumulator>
bool
parse_integral(Int& num, FwdIt first, FwdIt last, Accumulator accumulator)
{
num = 0;

if (first == last)
return false;

while (first != last)
{
auto const c = *first++;
if (c < '0' || c > '9')
return false;
if (!accumulator(num, Int(c - '0')))
return false;
}

return true;
}

template <class Int, class FwdIt>
bool
parse_negative_integral(Int& num, FwdIt first, FwdIt last)
{
Int limit_value = std::numeric_limits<Int>::min() / 10;
Int limit_digit = std::numeric_limits<Int>::min() % 10;

if (limit_digit < 0)
limit_digit = -limit_digit;

return parse_integral<Int>(
num, first, last, [limit_value, limit_digit](Int& value, Int digit) {
assert((digit >= 0) && (digit <= 9));
if (value < limit_value ||
(value == limit_value && digit > limit_digit))
return false;
value = (value * 10) - digit;
return true;
});
}

template <class Int, class FwdIt>
bool
parse_positive_integral(Int& num, FwdIt first, FwdIt last)
{
Int limit_value = std::numeric_limits<Int>::max() / 10;
Int limit_digit = std::numeric_limits<Int>::max() % 10;

return parse_integral<Int>(
num, first, last, [limit_value, limit_digit](Int& value, Int digit) {
assert((digit >= 0) && (digit <= 9));
if (value > limit_value ||
(value == limit_value && digit > limit_digit))
return false;
value = (value * 10) + digit;
return true;
});
}

template <class IntType, class FwdIt>
bool
parseSigned(IntType& result, FwdIt first, FwdIt last)
{
static_assert(
std::is_signed<IntType>::value,
"You may only call parseSigned with a signed integral type.");

if (first != last && *first == '-')
return parse_negative_integral(result, first + 1, last);

if (first != last && *first == '+')
return parse_positive_integral(result, first + 1, last);

return parse_positive_integral(result, first, last);
}

template <class UIntType, class FwdIt>
bool
parseUnsigned(UIntType& result, FwdIt first, FwdIt last)
{
static_assert(
std::is_unsigned<UIntType>::value,
"You may only call parseUnsigned with an unsigned integral type.");

if (first != last && *first == '+')
return parse_positive_integral(result, first + 1, last);

return parse_positive_integral(result, first, last);
}

//------------------------------------------------------------------------------

// These specializatons get called by the non-member functions to do the work
template <class Out, class In>
struct LexicalCast;
Expand All @@ -148,15 +47,15 @@ struct LexicalCast<std::string, In>
explicit LexicalCast() = default;

template <class Arithmetic = In>
std::enable_if_t<std::is_arithmetic<Arithmetic>::value, bool>
std::enable_if_t<std::is_arithmetic_v<Arithmetic>, bool>
operator()(std::string& out, Arithmetic in)
{
out = std::to_string(in);
return true;
}

template <class Enumeration = In>
std::enable_if_t<std::is_enum<Enumeration>::value, bool>
std::enable_if_t<std::is_enum_v<Enumeration>, bool>
operator()(std::string& out, Enumeration in)
{
out = std::to_string(
Expand All @@ -172,21 +71,24 @@ struct LexicalCast<Out, std::string>
explicit LexicalCast() = default;

static_assert(
std::is_integral<Out>::value,
std::is_integral_v<Out>,
"beast::LexicalCast can only be used with integral types");

template <class Integral = Out>
std::enable_if_t<std::is_unsigned<Integral>::value, bool>
std::enable_if_t<
std::is_integral_v<Integral> && !std::is_same_v<Integral, bool>,
bool>
operator()(Integral& out, std::string const& in) const
{
return parseUnsigned(out, in.begin(), in.end());
}
auto first = in.data();
auto last = in.data() + in.size();

template <class Integral = Out>
std::enable_if_t<std::is_signed<Integral>::value, bool>
operator()(Integral& out, std::string const& in) const
{
return parseSigned(out, in.begin(), in.end());
if (first != last && *first == '+')
++first;

auto ret = std::from_chars(first, last, out);

return ret.ec == std::errc() && ret.ptr == last;
}

bool
Expand Down Expand Up @@ -242,10 +144,6 @@ struct LexicalCast<Out, char*>
}
};

#if BOOST_COMP_MSVC
#pragma warning(pop)
#endif

} // namespace detail

//------------------------------------------------------------------------------
Expand Down Expand Up @@ -278,9 +176,7 @@ template <class Out, class In>
Out
lexicalCastThrow(In in)
{
Out out;

if (lexicalCastChecked(out, in))
if (Out out; lexicalCastChecked(out, in))
return out;

throw BadLexicalCast();
Expand All @@ -295,9 +191,7 @@ template <class Out, class In>
Out
lexicalCast(In in, Out defaultValue = Out())
{
Out out;

if (lexicalCastChecked(out, in))
if (Out out; lexicalCastChecked(out, in))
return out;

return defaultValue;
Expand Down

0 comments on commit 534a365

Please sign in to comment.