Skip to content

Commit

Permalink
Address reviewer's feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
@gregtatcam
gregtatcam committed Oct 7, 2024
1 parent 98d419f commit 3b7861d
Show file tree
Hide file tree
Showing 4 changed files with 87 additions and 103 deletions.
5 changes: 5 additions & 0 deletions include/xrpl/basics/Number.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,11 @@ class Number
static constexpr Number
lowest() noexcept;

/** Conversions to Number are implicit and conversions away from Number
* are explicit. This design encourages and facilitates the use of Number
* as the preferred type for floating point arithmetic as it makes
* "mixed mode" more convenient, e.g. MPTAmount + Number.
*/
explicit operator XRPAmount() const; // round to nearest, even on tie
explicit operator MPTAmount() const; // round to nearest, even on tie
explicit operator rep() const; // round to nearest, even on tie
Expand Down
13 changes: 3 additions & 10 deletions include/xrpl/protocol/Asset.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,9 @@ class Asset
public:
Asset() = default;

/** Conversions to Asset are implicit and conversions to specific issue
* type are explicit. This design facilitates the use of Asset.
*/
Asset(Issue const& issue) : issue_(issue)
{
}
Expand All @@ -56,16 +59,6 @@ class Asset
{
}

explicit operator Issue() const
{
return get<Issue>();
}

explicit operator MPTIssue() const
{
return get<MPTIssue>();
}

AccountID const&
getIssuer() const;

Expand Down
23 changes: 23 additions & 0 deletions include/xrpl/protocol/detail/STVar.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,10 @@
#include <xrpl/protocol/SField.h>
#include <xrpl/protocol/STBase.h>
#include <xrpl/protocol/Serializer.h>
#include <concepts>
#include <cstddef>
#include <cstdint>
#include <type_traits>
#include <typeinfo>
#include <utility>

Expand All @@ -44,6 +46,19 @@ struct nonPresentObject_t
extern defaultObject_t defaultObject;
extern nonPresentObject_t nonPresentObject;

// Concept to constrain STVar constructors, which
// instantiate ST* types from SerializedTypeID
// clang-format off
template <typename... Args>
concept ValidConstructSTArgs =
(std::is_same_v<
std::tuple<std::remove_cvref_t<Args>...>,
std::tuple<SField>> ||
std::is_same_v<
std::tuple<std::remove_cvref_t<Args>...>,
std::tuple<SerialIter, SField>>);
// clang-format on

// "variant" that can hold any type of serialized object
// and includes a small-object allocation optimization.
class STVar
Expand Down Expand Up @@ -131,6 +146,14 @@ class STVar
p_ = new (&d_) T(std::forward<Args>(args)...);
}

/** Construct requested Serializable Type according to id.
* The variadic args are: (SField), or (SerialIter, SField).
* depth is ignored in former case.
*/
template <typename... Args>
requires ValidConstructSTArgs<Args...> void
constructST(SerializedTypeID id, int depth, Args&&... arg);

bool
on_heap() const
{
Expand Down
149 changes: 56 additions & 93 deletions src/libxrpl/protocol/STVar.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -115,147 +115,110 @@ STVar::STVar(SerialIter& sit, SField const& name, int depth)
{
if (depth > 10)
Throw<std::runtime_error>("Maximum nesting depth of STVar exceeded");
switch (name.fieldType)
{
case STI_NOTPRESENT:
construct<STBase>(name);
return;
case STI_UINT8:
construct<STUInt8>(sit, name);
return;
case STI_UINT16:
construct<STUInt16>(sit, name);
return;
case STI_UINT32:
construct<STUInt32>(sit, name);
return;
case STI_UINT64:
construct<STUInt64>(sit, name);
return;
case STI_AMOUNT:
construct<STAmount>(sit, name);
return;
case STI_UINT128:
construct<STUInt128>(sit, name);
return;
case STI_UINT160:
construct<STUInt160>(sit, name);
return;
case STI_UINT192:
construct<STUInt192>(sit, name);
return;
case STI_UINT256:
construct<STUInt256>(sit, name);
return;
case STI_VECTOR256:
construct<STVector256>(sit, name);
return;
case STI_VL:
construct<STBlob>(sit, name);
return;
case STI_ACCOUNT:
construct<STAccount>(sit, name);
return;
case STI_PATHSET:
construct<STPathSet>(sit, name);
return;
case STI_OBJECT:
construct<STObject>(sit, name, depth);
return;
case STI_ARRAY:
construct<STArray>(sit, name, depth);
return;
case STI_ISSUE:
construct<STIssue>(sit, name);
return;
case STI_XCHAIN_BRIDGE:
construct<STXChainBridge>(sit, name);
return;
case STI_CURRENCY:
construct<STCurrency>(sit, name);
return;
default:
Throw<std::runtime_error>("Unknown object type");
}
constructST(name.fieldType, depth, sit, name);
}

STVar::STVar(SerializedTypeID id, SField const& name)
{
assert((id == STI_NOTPRESENT) || (id == name.fieldType));
constructST(id, 0, name);
}

void
STVar::destroy()
{
if (on_heap())
delete p_;
else
p_->~STBase();

p_ = nullptr;
}

template <typename... Args>
requires ValidConstructSTArgs<Args...> void
STVar::constructST(SerializedTypeID id, int depth, Args&&... args)
{
auto constructWithDepth = [&]<typename T>() {
if constexpr (std::is_same_v<
std::tuple<std::remove_cvref_t<Args>...>,
std::tuple<SField>>)
construct<T>(std::forward<Args>(args)...);
else if constexpr (std::is_same_v<
std::tuple<std::remove_cvref_t<Args>...>,
std::tuple<SerialIter, SField>>)
construct<T>(std::forward<Args>(args)..., depth);
else
static_assert(false, "Invalid STVar constructor arguments");
};

switch (id)
{
case STI_NOTPRESENT:
construct<STBase>(name);
case STI_NOTPRESENT: {
// Last argument is always SField
SField const& field =
std::get<sizeof...(args) - 1>(std::forward_as_tuple(args...));
construct<STBase>(field);
return;
}
case STI_UINT8:
construct<STUInt8>(name);
construct<STUInt8>(std::forward<Args>(args)...);
return;
case STI_UINT16:
construct<STUInt16>(name);
construct<STUInt16>(std::forward<Args>(args)...);
return;
case STI_UINT32:
construct<STUInt32>(name);
construct<STUInt32>(std::forward<Args>(args)...);
return;
case STI_UINT64:
construct<STUInt64>(name);
construct<STUInt64>(std::forward<Args>(args)...);
return;
case STI_AMOUNT:
construct<STAmount>(name);
construct<STAmount>(std::forward<Args>(args)...);
return;
case STI_UINT128:
construct<STUInt128>(name);
construct<STUInt128>(std::forward<Args>(args)...);
return;
case STI_UINT160:
construct<STUInt160>(name);
construct<STUInt160>(std::forward<Args>(args)...);
return;
case STI_UINT192:
construct<STUInt192>(name);
construct<STUInt192>(std::forward<Args>(args)...);
return;
case STI_UINT256:
construct<STUInt256>(name);
construct<STUInt256>(std::forward<Args>(args)...);
return;
case STI_VECTOR256:
construct<STVector256>(name);
construct<STVector256>(std::forward<Args>(args)...);
return;
case STI_VL:
construct<STBlob>(name);
construct<STBlob>(std::forward<Args>(args)...);
return;
case STI_ACCOUNT:
construct<STAccount>(name);
construct<STAccount>(std::forward<Args>(args)...);
return;
case STI_PATHSET:
construct<STPathSet>(name);
construct<STPathSet>(std::forward<Args>(args)...);
return;
case STI_OBJECT:
construct<STObject>(name);
constructWithDepth.template operator()<STObject>();
return;
case STI_ARRAY:
construct<STArray>(name);
constructWithDepth.template operator()<STArray>();
return;
case STI_ISSUE:
construct<STIssue>(name);
construct<STIssue>(std::forward<Args>(args)...);
return;
case STI_XCHAIN_BRIDGE:
construct<STXChainBridge>(name);
construct<STXChainBridge>(std::forward<Args>(args)...);
return;
case STI_CURRENCY:
construct<STCurrency>(name);
construct<STCurrency>(std::forward<Args>(args)...);
return;
default:
Throw<std::runtime_error>("Unknown object type");
}
}

void
STVar::destroy()
{
if (on_heap())
delete p_;
else
p_->~STBase();

p_ = nullptr;
}

} // namespace detail
} // namespace ripple

0 comments on commit 3b7861d

Please sign in to comment.