Cleanup code using move semantics

This commit is contained in:
seelabs
2020-04-10 14:07:31 -04:00
committed by manojsdoshi
parent 421417ab07
commit 6d28f2a8d9
25 changed files with 132 additions and 116 deletions

View File

@@ -499,7 +499,7 @@ Ledger::rawInsert(std::shared_ptr<SLE> const& sle)
Serializer ss;
sle->add(ss);
auto item = std::make_shared<SHAMapItem const>(sle->key(), std::move(ss));
// VFALCO NOTE addGiveItem should take ownership
// addGiveItem should take ownership
if (!stateMap_->addGiveItem(std::move(item), false, false))
LogicError("Ledger::rawInsert: key already exists");
}
@@ -510,7 +510,7 @@ Ledger::rawReplace(std::shared_ptr<SLE> const& sle)
Serializer ss;
sle->add(ss);
auto item = std::make_shared<SHAMapItem const>(sle->key(), std::move(ss));
// VFALCO NOTE updateGiveItem should take ownership
// updateGiveItem should take ownership
if (!stateMap_->updateGiveItem(std::move(item), false, false))
LogicError("Ledger::rawReplace: key not found");
}

View File

@@ -29,7 +29,7 @@ LedgerReplay::LedgerReplay(
{
for (auto const& item : replay_->txMap())
{
auto const txPair = replay_->txRead(item.key());
auto txPair = replay_->txRead(item.key()); // non-const so can be moved
auto const txIndex = (*txPair.second)[sfTransactionIndex];
orderedTxns_.emplace(txIndex, std::move(txPair.first));
}

View File

@@ -245,7 +245,7 @@ FeeVoteImpl::doVoting(
auto tItem = std::make_shared<SHAMapItem>(txID, s.peekData());
if (!initialPosition->addGiveItem(tItem, true, false))
if (!initialPosition->addGiveItem(std::move(tItem), true, false))
{
JLOG(journal_.warn()) << "Ledger already had fee change";
}

View File

@@ -2405,8 +2405,8 @@ NetworkOPsImp::getTxsAccount(
auto bound = [&ret, &app](
std::uint32_t ledger_index,
std::string const& status,
Blob const& rawTxn,
Blob const& rawMeta) {
Blob&& rawTxn,
Blob&& rawMeta) {
convertBlobsToTxResult(ret, ledger_index, status, rawTxn, rawMeta, app);
};
@@ -2444,8 +2444,8 @@ NetworkOPsImp::getTxsAccountB(
auto bound = [&ret](
std::uint32_t ledgerIndex,
std::string const& status,
Blob const& rawTxn,
Blob const& rawMeta) {
Blob&& rawTxn,
Blob&& rawMeta) {
ret.emplace_back(std::move(rawTxn), std::move(rawMeta), ledgerIndex);
};

View File

@@ -65,11 +65,9 @@ accountTxPage(
DatabaseCon& connection,
AccountIDCache const& idCache,
std::function<void(std::uint32_t)> const& onUnsavedLedger,
std::function<void(
std::uint32_t,
std::string const&,
Blob const&,
Blob const&)> const& onTransaction,
std::function<
void(std::uint32_t, std::string const&, Blob&&, Blob&&)> const&
onTransaction,
AccountID const& account,
std::int32_t minLedger,
std::int32_t maxLedger,
@@ -247,11 +245,21 @@ accountTxPage(
if (rawMeta.size() == 0)
onUnsavedLedger(ledgerSeq.value_or(0));
// `rawData` and `rawMeta` will be used after they are moved.
// That's OK.
onTransaction(
rangeCheckedCast<std::uint32_t>(ledgerSeq.value_or(0)),
*status,
rawData,
rawMeta);
std::move(rawData),
std::move(rawMeta));
// Note some callbacks will move the data, some will not. Clear
// them so code doesn't depend on if the data was actually moved
// or not. The code will be more efficient if `rawData` and
// `rawMeta` don't have to allocate in `convert`, so don't
// refactor my moving these variables into loop scope.
rawData.clear();
rawMeta.clear();
--numberOfResults;
}
}

View File

@@ -47,11 +47,9 @@ accountTxPage(
DatabaseCon& connection,
AccountIDCache const& idCache,
std::function<void(std::uint32_t)> const& onUnsavedLedger,
std::function<void(
std::uint32_t,
std::string const&,
Blob const&,
Blob const&)> const& onTransaction,
std::function<
void(std::uint32_t, std::string const&, Blob&&, Blob&&)> const&
onTransaction,
AccountID const& account,
std::int32_t minLedger,
std::int32_t maxLedger,

View File

@@ -1552,7 +1552,7 @@ setup_TxQ(Config const& config)
std::unique_ptr<TxQ>
make_TxQ(TxQ::Setup const& setup, beast::Journal j)
{
return std::make_unique<TxQ>(setup, std::move(j));
return std::make_unique<TxQ>(setup, j);
}
} // namespace ripple

View File

@@ -39,7 +39,8 @@ ValidatorKeys::ValidatorKeys(Config const& config, beast::Journal j)
if (config.exists(SECTION_VALIDATOR_TOKEN))
{
if (auto const token = loadValidatorToken(
// token is non-const so it can be moved from
if (auto token = loadValidatorToken(
config.section(SECTION_VALIDATOR_TOKEN).lines()))
{
auto const pk =

View File

@@ -183,7 +183,7 @@ ValidatorList::load(
// Config listed keys never expire
if (it.second)
it.first->second.expiration = TimeKeeper::time_point::max();
it.first->second.list.emplace_back(std::move(*id));
it.first->second.list.emplace_back(*id);
it.first->second.available = true;
++count;
}

View File

@@ -812,7 +812,7 @@ Shard::storeSQLite(
session
<< (STTx::getMetaSQLInsertReplaceHeader() +
item.first->getMetaSQL(
seq, sqlEscape(std::move(s.modData()))) +
seq, sqlEscape(s.modData())) +
';');
}
}

View File

@@ -809,7 +809,8 @@ OverlayImpl::crawlShards(bool pubKey, std::uint32_t hops)
for_each([&](std::shared_ptr<PeerImp> const& peer) {
if (auto psi = peer->getPeerShardInfo())
{
for (auto const& e : *psi)
// e is non-const so it may be moved from
for (auto& e : *psi)
{
auto it{peerShardInfo.find(e.first)};
if (it != peerShardInfo.end())
@@ -898,7 +899,8 @@ OverlayImpl::getOverlayInfo()
{
auto version{sp->getVersion()};
if (!version.empty())
pv[jss::version] = std::move(version);
// Could move here if Json::value supported moving from strings
pv[jss::version] = version;
}
std::uint32_t minSeq, maxSeq;

View File

@@ -322,7 +322,8 @@ PeerImp::json()
std::string name{getName()};
if (!name.empty())
ret[jss::name] = std::move(name);
// Could move here if Json::Value supported moving from a string
ret[jss::name] = name;
}
ret[jss::load] = usage_.balance();

View File

@@ -423,7 +423,7 @@ std::uint64_t const STAmount::uRateOne = getRate(STAmount(1), STAmount(1));
void
STAmount::setIssue(Issue const& issue)
{
mIssue = std::move(issue);
mIssue = issue;
mIsNative = isXRP(*this);
}

View File

@@ -148,10 +148,7 @@ insertDeliveredAmount(
auto const getCloseTime = [&info] { return info.closeTime; };
auto amt = getDeliveredAmount(
getLedgerIndex,
getCloseTime,
std::move(serializedTx),
transactionMeta);
getLedgerIndex, getCloseTime, serializedTx, transactionMeta);
if (amt)
{
meta[jss::delivered_amount] =
@@ -182,10 +179,7 @@ getDeliveredAmount(
return context.ledgerMaster.getCloseTimeBySeq(getLedgerIndex());
};
return getDeliveredAmount(
getLedgerIndex,
getCloseTime,
std::move(serializedTx),
transactionMeta);
getLedgerIndex, getCloseTime, serializedTx, transactionMeta);
}
return {};

View File

@@ -424,40 +424,43 @@ ShardArchiveHandler::complete(path dstPath)
}
}
auto wrapper = jobCounter_.wrap([=, dstPath = std::move(dstPath)](Job&) {
if (isStopping())
return;
// Make lambdas mutable captured vars can be moved from
auto wrapper =
jobCounter_.wrap([=, dstPath = std::move(dstPath)](Job&) mutable {
if (isStopping())
return;
// If not synced then defer and retry
auto const mode{app_.getOPs().getOperatingMode()};
if (mode != OperatingMode::FULL)
{
std::lock_guard lock(m_);
timer_.expires_from_now(static_cast<std::chrono::seconds>(
(static_cast<std::size_t>(OperatingMode::FULL) -
static_cast<std::size_t>(mode)) *
10));
// If not synced then defer and retry
auto const mode{app_.getOPs().getOperatingMode()};
if (mode != OperatingMode::FULL)
{
std::lock_guard lock(m_);
timer_.expires_from_now(static_cast<std::chrono::seconds>(
(static_cast<std::size_t>(OperatingMode::FULL) -
static_cast<std::size_t>(mode)) *
10));
auto wrapper =
timerCounter_.wrap([=, dstPath = std::move(dstPath)](
boost::system::error_code const& ec) {
if (ec != boost::asio::error::operation_aborted)
complete(std::move(dstPath));
});
auto wrapper = timerCounter_.wrap(
[=, dstPath = std::move(dstPath)](
boost::system::error_code const& ec) mutable {
if (ec != boost::asio::error::operation_aborted)
complete(std::move(dstPath));
});
if (!wrapper)
onClosureFailed(
"failed to wrap closure for operating mode timer", lock);
if (!wrapper)
onClosureFailed(
"failed to wrap closure for operating mode timer",
lock);
else
timer_.async_wait(*wrapper);
}
else
timer_.async_wait(*wrapper);
}
else
{
process(dstPath);
std::lock_guard lock(m_);
removeAndProceed(lock);
}
});
{
process(dstPath);
std::lock_guard lock(m_);
removeAndProceed(lock);
}
});
if (!wrapper)
{

View File

@@ -335,11 +335,7 @@ struct transactionPreProcessResult
transactionPreProcessResult() = delete;
transactionPreProcessResult(transactionPreProcessResult const&) = delete;
transactionPreProcessResult(transactionPreProcessResult&& rhs)
: first(std::move(rhs.first)) // VS2013 won't default this.
, second(std::move(rhs.second))
{
}
transactionPreProcessResult(transactionPreProcessResult&& rhs) = default;
transactionPreProcessResult&
operator=(transactionPreProcessResult const&) = delete;

View File

@@ -165,12 +165,12 @@ public:
// save a copy if you have a temporary anyway
bool
updateGiveItem(
std::shared_ptr<SHAMapItem const> const&,
std::shared_ptr<SHAMapItem const>,
bool isTransaction,
bool hasMeta);
bool
addGiveItem(
std::shared_ptr<SHAMapItem const> const&,
std::shared_ptr<SHAMapItem const>,
bool isTransaction,
bool hasMeta);

View File

@@ -263,11 +263,11 @@ public:
operator=(const SHAMapTreeNode&) = delete;
SHAMapTreeNode(
std::shared_ptr<SHAMapItem const> const& item,
std::shared_ptr<SHAMapItem const> item,
TNType type,
std::uint32_t seq);
SHAMapTreeNode(
std::shared_ptr<SHAMapItem const> const& item,
std::shared_ptr<SHAMapItem const> item,
TNType type,
std::uint32_t seq,
SHAMapHash const& hash);
@@ -292,7 +292,7 @@ public: // public only to SHAMap
std::shared_ptr<SHAMapItem const> const&
peekItem() const;
bool
setItem(std::shared_ptr<SHAMapItem const> const& i, TNType type);
setItem(std::shared_ptr<SHAMapItem const> i, TNType type);
std::string
getString(SHAMapNodeID const&) const override;

View File

@@ -697,7 +697,7 @@ SHAMap::delItem(uint256 const& id)
bool
SHAMap::addGiveItem(
std::shared_ptr<SHAMapItem const> const& item,
std::shared_ptr<SHAMapItem const> item,
bool isTransaction,
bool hasMeta)
{
@@ -732,7 +732,8 @@ SHAMap::addGiveItem(
auto inner = std::static_pointer_cast<SHAMapInnerNode>(node);
int branch = nodeID.selectBranch(tag);
assert(inner->isEmptyBranch(branch));
auto newNode = std::make_shared<SHAMapTreeNode>(item, type, seq_);
auto newNode =
std::make_shared<SHAMapTreeNode>(std::move(item), type, seq_);
inner->setChild(branch, newNode);
}
else
@@ -762,12 +763,13 @@ SHAMap::addGiveItem(
assert(node->isInner());
std::shared_ptr<SHAMapTreeNode> newNode =
std::make_shared<SHAMapTreeNode>(item, type, seq_);
std::make_shared<SHAMapTreeNode>(std::move(item), type, seq_);
assert(newNode->isValid() && newNode->isLeaf());
auto inner = std::static_pointer_cast<SHAMapInnerNode>(node);
inner->setChild(b1, newNode);
newNode = std::make_shared<SHAMapTreeNode>(otherItem, type, seq_);
newNode =
std::make_shared<SHAMapTreeNode>(std::move(otherItem), type, seq_);
assert(newNode->isValid() && newNode->isLeaf());
inner->setChild(b2, newNode);
}
@@ -799,7 +801,7 @@ SHAMap::getHash() const
bool
SHAMap::updateGiveItem(
std::shared_ptr<SHAMapItem const> const& item,
std::shared_ptr<SHAMapItem const> item,
bool isTransaction,
bool hasMeta)
{
@@ -827,7 +829,7 @@ SHAMap::updateGiveItem(
node = unshareNode(std::move(node), nodeID);
if (!node->setItem(
item,
std::move(item),
!isTransaction ? SHAMapTreeNode::tnACCOUNT_STATE
: (hasMeta ? SHAMapTreeNode::tnTRANSACTION_MD
: SHAMapTreeNode::tnTRANSACTION_NM)))
@@ -1005,7 +1007,9 @@ SHAMap::walkSubTree(bool doWrite, NodeObjectType t, std::uint32_t seq)
// save our place and work on this node
stack.emplace(std::move(node), branch);
// The semantics of this changes when we move to c++-20
// Right now no move will occur; With c++-20 child will
// be moved from.
node = std::static_pointer_cast<SHAMapInnerNode>(
std::move(child));
pos = 0;

View File

@@ -485,11 +485,13 @@ SHAMap::getNodeFat(
std::tie(node, nodeID, depth) = stack.top();
stack.pop();
// Add this node to the reply
Serializer s;
node->addRaw(s, snfWIRE);
nodeIDs.push_back(nodeID);
rawNodes.push_back(std::move(s.peekData()));
{
// Add this node to the reply
Serializer s;
node->addRaw(s, snfWIRE);
nodeIDs.push_back(nodeID);
rawNodes.push_back(std::move(s.modData()));
}
if (node->isInner())
{
@@ -522,7 +524,7 @@ SHAMap::getNodeFat(
Serializer ns;
childNode->addRaw(ns, snfWIRE);
nodeIDs.push_back(childID);
rawNodes.push_back(std::move(ns.peekData()));
rawNodes.push_back(std::move(ns.modData()));
}
}
}

View File

@@ -56,23 +56,23 @@ SHAMapTreeNode::clone(std::uint32_t seq) const
}
SHAMapTreeNode::SHAMapTreeNode(
std::shared_ptr<SHAMapItem const> const& item,
std::shared_ptr<SHAMapItem const> item,
TNType type,
std::uint32_t seq)
: SHAMapAbstractNode(type, seq), mItem(item)
: SHAMapAbstractNode(type, seq), mItem(std::move(item))
{
assert(item->peekData().size() >= 12);
assert(mItem->peekData().size() >= 12);
updateHash();
}
SHAMapTreeNode::SHAMapTreeNode(
std::shared_ptr<SHAMapItem const> const& item,
std::shared_ptr<SHAMapItem const> item,
TNType type,
std::uint32_t seq,
SHAMapHash const& hash)
: SHAMapAbstractNode(type, seq, hash), mItem(item)
: SHAMapAbstractNode(type, seq, hash), mItem(std::move(item))
{
assert(item->peekData().size() >= 12);
assert(mItem->peekData().size() >= 12);
}
std::shared_ptr<SHAMapAbstractNode>
@@ -105,9 +105,9 @@ SHAMapAbstractNode::make(
s.peekData());
if (hashValid)
return std::make_shared<SHAMapTreeNode>(
item, tnTRANSACTION_NM, seq, hash);
std::move(item), tnTRANSACTION_NM, seq, hash);
return std::make_shared<SHAMapTreeNode>(
item, tnTRANSACTION_NM, seq);
std::move(item), tnTRANSACTION_NM, seq);
}
else if (type == 1)
{
@@ -125,8 +125,9 @@ SHAMapAbstractNode::make(
auto item = std::make_shared<SHAMapItem const>(u, s.peekData());
if (hashValid)
return std::make_shared<SHAMapTreeNode>(
item, tnACCOUNT_STATE, seq, hash);
return std::make_shared<SHAMapTreeNode>(item, tnACCOUNT_STATE, seq);
std::move(item), tnACCOUNT_STATE, seq, hash);
return std::make_shared<SHAMapTreeNode>(
std::move(item), tnACCOUNT_STATE, seq);
}
else if (type == 2)
{
@@ -185,9 +186,9 @@ SHAMapAbstractNode::make(
auto item = std::make_shared<SHAMapItem const>(u, s.peekData());
if (hashValid)
return std::make_shared<SHAMapTreeNode>(
item, tnTRANSACTION_MD, seq, hash);
std::move(item), tnTRANSACTION_MD, seq, hash);
return std::make_shared<SHAMapTreeNode>(
item, tnTRANSACTION_MD, seq);
std::move(item), tnTRANSACTION_MD, seq);
}
}
@@ -214,9 +215,9 @@ SHAMapAbstractNode::make(
sha512Half(rawNode), s.peekData());
if (hashValid)
return std::make_shared<SHAMapTreeNode>(
item, tnTRANSACTION_NM, seq, hash);
std::move(item), tnTRANSACTION_NM, seq, hash);
return std::make_shared<SHAMapTreeNode>(
item, tnTRANSACTION_NM, seq);
std::move(item), tnTRANSACTION_NM, seq);
}
else if (safe_cast<HashPrefix>(prefix) == HashPrefix::leafNode)
{
@@ -236,8 +237,9 @@ SHAMapAbstractNode::make(
auto item = std::make_shared<SHAMapItem const>(u, s.peekData());
if (hashValid)
return std::make_shared<SHAMapTreeNode>(
item, tnACCOUNT_STATE, seq, hash);
return std::make_shared<SHAMapTreeNode>(item, tnACCOUNT_STATE, seq);
std::move(item), tnACCOUNT_STATE, seq, hash);
return std::make_shared<SHAMapTreeNode>(
std::move(item), tnACCOUNT_STATE, seq);
}
else if (safe_cast<HashPrefix>(prefix) == HashPrefix::innerNode)
{
@@ -274,9 +276,9 @@ SHAMapAbstractNode::make(
auto item = std::make_shared<SHAMapItem const>(txID, s.peekData());
if (hashValid)
return std::make_shared<SHAMapTreeNode>(
item, tnTRANSACTION_MD, seq, hash);
std::move(item), tnTRANSACTION_MD, seq, hash);
return std::make_shared<SHAMapTreeNode>(
item, tnTRANSACTION_MD, seq);
std::move(item), tnTRANSACTION_MD, seq);
}
else
{
@@ -459,10 +461,10 @@ SHAMapTreeNode::addRaw(Serializer& s, SHANodeFormat format) const
}
bool
SHAMapTreeNode::setItem(std::shared_ptr<SHAMapItem const> const& i, TNType type)
SHAMapTreeNode::setItem(std::shared_ptr<SHAMapItem const> i, TNType type)
{
mType = type;
mItem = i;
mItem = std::move(i);
assert(isLeaf());
assert(mSeq != 0);
return updateHash();

View File

@@ -183,11 +183,13 @@ public:
Serializer s;
st.add(s);
std::string const m(static_cast<char const*>(s.data()), s.size());
// m is non-const so it can be moved from
std::string m(static_cast<char const*>(s.data()), s.size());
if (auto r = deserializeManifest(std::move(m)))
return std::move(*r);
Throw<std::runtime_error>("Could not create a revocation manifest");
return *deserializeManifest(std::move(m)); // Silence compiler warning.
return *deserializeManifest(
std::string{}); // Silence compiler warning.
}
Manifest
@@ -223,11 +225,14 @@ public:
Serializer s;
st.add(s);
std::string const m(static_cast<char const*>(s.data()), s.size());
std::string m(
static_cast<char const*>(s.data()),
s.size()); // non-const so can be moved
if (auto r = deserializeManifest(std::move(m)))
return std::move(*r);
Throw<std::runtime_error>("Could not create a manifest");
return *deserializeManifest(std::move(m)); // Silence compiler warning.
return *deserializeManifest(
std::string{}); // Silence compiler warning.
}
Manifest

View File

@@ -146,7 +146,7 @@ struct Regression_test : public beast::unit_test::suite
auto secp256r1Sig = std::make_unique<STTx>(*(jt.stx));
auto pubKeyBlob = strUnHex(secp256r1PubKey);
assert(pubKeyBlob); // Hex for public key must be valid
secp256r1Sig->setFieldVL(sfSigningPubKey, std::move(*pubKeyBlob));
secp256r1Sig->setFieldVL(sfSigningPubKey, *pubKeyBlob);
jt.stx.reset(secp256r1Sig.release());
env(jt, ter(temINVALID));

View File

@@ -48,7 +48,7 @@ Account::Account(
Account
Account::fromCache(std::string name, KeyType type)
{
auto const p = std::make_pair(name, type);
auto p = std::make_pair(name, type); // non-const so it can be moved from
auto const iter = cache_.find(p);
if (iter != cache_.end())
return iter->second;

View File

@@ -151,7 +151,7 @@ class View_test : public beast::unit_test::suite
BEAST_EXPECT(seq(v.read(k(3))) == 3);
auto s = copy(v.read(k(2)));
seq(s, 4);
ledger->rawReplace(std::move(s));
ledger->rawReplace(s);
BEAST_EXPECT(seq(v.read(k(2))) == 4);
ledger->rawErase(sle(2));
BEAST_EXPECT(!v.exists(k(2)));