Fix clang-tidy violations in coroutine primitives

- NOLINT the compiler-mandated coroutine protocol names (promise_type,
  await_ready, ...) that conflict with readability-identifier-naming and
  readability-convert-member-functions-to-static
- Add [[nodiscard]] to handle(), done(), await_ready()
- Replace std::lock_guard with std::scoped_lock const
- Pass CoroTaskRunner name by value and std::move it; default-init
  runCount_ in-class
- Drop unused <coroutine> include from JobQueue.h; include
  instrumentation.h directly in JobQueueAwaiter.h
- CoroTask_test: kN constant naming, const locals, file-level NOLINT for
  cppcoreguidelines-avoid-capturing-lambda-coroutines (lifetimes are
  gated and joined)
This commit is contained in:
Pratik Mankawde
2026-07-27 11:30:16 +01:00
parent 8d4ea00453
commit 9bddc54722
5 changed files with 63 additions and 28 deletions

View File

@@ -152,6 +152,10 @@ template <>
class CoroTask<void>
{
public:
// The C++ coroutine protocol mandates these names (promise_type,
// initial_suspend, await_ready, ...) and instance-callable awaiter
// methods, which conflict with the project naming/static conventions.
// NOLINTBEGIN(readability-identifier-naming, readability-convert-member-functions-to-static)
struct promise_type;
using Handle = std::coroutine_handle<promise_type>;
@@ -260,6 +264,7 @@ public:
exception_ = std::current_exception();
}
};
// NOLINTEND(readability-identifier-naming, readability-convert-member-functions-to-static)
/**
* Default constructor. Creates an empty (null handle) task.
@@ -313,7 +318,7 @@ public:
/**
* @return The underlying coroutine_handle
*/
Handle
[[nodiscard]] Handle
handle() const
{
return handle_;
@@ -322,7 +327,7 @@ public:
/**
* @return true if the coroutine has run to completion (or thrown)
*/
bool
[[nodiscard]] bool
done() const
{
return handle_ && handle_.done();
@@ -334,7 +339,8 @@ public:
* Always false. This task is lazy, so co_await always suspends
* the caller to set up the continuation link.
*/
bool
// NOLINTBEGIN(readability-identifier-naming, readability-convert-member-functions-to-static)
[[nodiscard]] bool
await_ready() const noexcept
{
return false;
@@ -368,6 +374,7 @@ public:
if (auto& ep = handle_.promise().exception_)
std::rethrow_exception(ep);
}
// NOLINTEND(readability-identifier-naming, readability-convert-member-functions-to-static)
private:
// Exclusively-owned coroutine handle. Null after move or default
@@ -477,6 +484,10 @@ class CoroTask
"CoroTask<T> requires T to be move-constructible");
public:
// The C++ coroutine protocol mandates these names (promise_type,
// initial_suspend, await_ready, ...) and instance-callable awaiter
// methods, which conflict with the project naming/static conventions.
// NOLINTBEGIN(readability-identifier-naming, readability-convert-member-functions-to-static)
struct promise_type;
using Handle = std::coroutine_handle<promise_type>;
@@ -578,6 +589,7 @@ public:
result_.template emplace<2>(std::current_exception());
}
};
// NOLINTEND(readability-identifier-naming, readability-convert-member-functions-to-static)
/**
* Default constructor. Creates an empty (null handle) task.
@@ -631,7 +643,7 @@ public:
/**
* @return The underlying coroutine_handle
*/
Handle
[[nodiscard]] Handle
handle() const
{
return handle_;
@@ -640,7 +652,7 @@ public:
/**
* @return true if the coroutine has run to completion (or thrown)
*/
bool
[[nodiscard]] bool
done() const
{
return handle_ && handle_.done();
@@ -651,7 +663,8 @@ public:
/**
* Always false. co_await always suspends to set up continuation.
*/
bool
// NOLINTBEGIN(readability-identifier-naming, readability-convert-member-functions-to-static)
[[nodiscard]] bool
await_ready() const noexcept
{
return false;
@@ -689,6 +702,7 @@ public:
std::rethrow_exception(*ep);
return std::get<1>(std::move(result));
}
// NOLINTEND(readability-identifier-naming, readability-convert-member-functions-to-static)
private:
// Exclusively-owned coroutine handle. Null after move or default

View File

@@ -93,8 +93,8 @@ inline JobQueue::CoroTaskRunner::CoroTaskRunner(
CreateT,
JobQueue& jq,
JobType type,
std::string const& name)
: jq_(jq), type_(type), name_(name), runCount_(0)
std::string name)
: jq_(jq), type_(type), name_(std::move(name))
{
}
@@ -145,7 +145,7 @@ inline JobQueue::CoroTaskRunner::~CoroTaskRunner()
inline void
JobQueue::CoroTaskRunner::onSuspend()
{
std::lock_guard lock(jq_.mutex_);
std::scoped_lock const lock(jq_.mutex_);
++jq_.nSuspend_;
}
@@ -155,7 +155,7 @@ JobQueue::CoroTaskRunner::onSuspend()
inline void
JobQueue::CoroTaskRunner::onUndoSuspend()
{
std::lock_guard lock(jq_.mutex_);
std::scoped_lock const lock(jq_.mutex_);
--jq_.nSuspend_;
}
@@ -173,6 +173,10 @@ JobQueue::CoroTaskRunner::suspend()
* Custom awaiter for suspend(). Always suspends (await_ready
* returns false) and increments nSuspend_ in await_suspend().
*/
// The C++ coroutine protocol mandates these awaiter names and
// instance-callable methods, which conflict with the project
// naming/static conventions.
// NOLINTBEGIN(readability-identifier-naming, readability-convert-member-functions-to-static)
struct SuspendAwaiter
{
CoroTaskRunner& runner_; // The runner that owns this coroutine.
@@ -180,7 +184,7 @@ JobQueue::CoroTaskRunner::suspend()
/**
* Always returns false so the coroutine suspends.
*/
bool
[[nodiscard]] bool
await_ready() const noexcept
{
return false;
@@ -201,6 +205,7 @@ JobQueue::CoroTaskRunner::suspend()
{
}
};
// NOLINTEND(readability-identifier-naming, readability-convert-member-functions-to-static)
return SuspendAwaiter{*this};
}
@@ -218,11 +223,15 @@ JobQueue::CoroTaskRunner::suspend()
inline auto
JobQueue::CoroTaskRunner::yieldAndPost()
{
// The C++ coroutine protocol mandates these awaiter names and
// instance-callable methods, which conflict with the project
// naming/static conventions.
// NOLINTBEGIN(readability-identifier-naming, readability-convert-member-functions-to-static)
struct YieldPostAwaiter
{
CoroTaskRunner& runner_;
bool
[[nodiscard]] bool
await_ready() const noexcept
{
return false;
@@ -264,6 +273,7 @@ JobQueue::CoroTaskRunner::yieldAndPost()
{
}
};
// NOLINTEND(readability-identifier-naming, readability-convert-member-functions-to-static)
return YieldPostAwaiter{*this};
}
@@ -278,7 +288,7 @@ inline bool
JobQueue::CoroTaskRunner::post()
{
{
std::lock_guard lk(mutexRun_);
std::scoped_lock const lk(mutexRun_);
++runCount_;
}
@@ -289,7 +299,7 @@ JobQueue::CoroTaskRunner::post()
}
// The coroutine will not run. Undo the runCount_ increment.
std::lock_guard lk(mutexRun_);
std::scoped_lock const lk(mutexRun_);
--runCount_;
cv_.notify_all();
return false;
@@ -314,12 +324,12 @@ inline void
JobQueue::CoroTaskRunner::resume()
{
{
std::lock_guard lock(jq_.mutex_);
std::scoped_lock const lock(jq_.mutex_);
--jq_.nSuspend_;
}
auto saved = detail::getLocalValues().release();
detail::getLocalValues().reset(&lvs_);
std::lock_guard lock(mutex_);
std::scoped_lock const lock(mutex_);
XRPL_ASSERT(
task_.handle() && !task_.done(),
"xrpl::JobQueue::CoroTaskRunner::resume : task handle is valid and not done");
@@ -372,7 +382,7 @@ JobQueue::CoroTaskRunner::resume()
// frame destruction triggers runner cleanup.
[[maybe_unused]] auto completed = std::move(task_);
}
std::lock_guard lk(mutexRun_);
std::scoped_lock const lk(mutexRun_);
--runCount_;
cv_.notify_all();
}
@@ -398,7 +408,7 @@ JobQueue::CoroTaskRunner::expectEarlyExit()
{
if (!finished_)
{
std::lock_guard lock(jq_.mutex_);
std::scoped_lock const lock(jq_.mutex_);
--jq_.nSuspend_;
finished_ = true;
}

View File

@@ -25,7 +25,6 @@
#include <atomic>
#include <chrono>
#include <condition_variable>
#include <coroutine>
#include <cstdint>
#include <functional>
#include <map>
@@ -372,7 +371,7 @@ public:
// enqueuing a second resume while the first is still running.
// A bool would be clobbered: R2.post() sets true, then R1's
// cleanup sets false — losing the fact that R2 is still pending.
int runCount_;
int runCount_ = 0;
// Serializes all coroutine resume() calls, preventing concurrent
// execution of the coroutine body on multiple threads. Handles the
@@ -449,7 +448,7 @@ public:
* @param type Job type for scheduling priority
* @param name Human-readable name for logging
*/
CoroTaskRunner(CreateT, JobQueue&, JobType, std::string const&);
CoroTaskRunner(CreateT, JobQueue&, JobType, std::string);
CoroTaskRunner(CoroTaskRunner const&) = delete;
CoroTaskRunner&
@@ -937,7 +936,7 @@ JobQueue::postCoroTask(JobType t, std::string const& name, F&& f)
return nullptr;
{
std::lock_guard lock(mutex_);
std::scoped_lock const lock(mutex_);
++nSuspend_;
}

View File

@@ -1,5 +1,6 @@
#pragma once
#include <xrpl/beast/utility/instrumentation.h>
#include <xrpl/core/JobQueue.h>
#include <coroutine>
@@ -154,7 +155,11 @@ struct JobQueueAwaiter
/**
* Always returns false so the coroutine suspends.
*/
bool
// The C++ coroutine protocol mandates these awaiter names and
// instance-callable methods, which conflict with the project
// naming/static conventions.
// NOLINTBEGIN(readability-identifier-naming, readability-convert-member-functions-to-static)
[[nodiscard]] bool
await_ready() const noexcept
{
return false;
@@ -201,6 +206,7 @@ struct JobQueueAwaiter
await_resume() const noexcept
{
}
// NOLINTEND(readability-identifier-naming, readability-convert-member-functions-to-static)
};
} // namespace xrpl

View File

@@ -19,6 +19,10 @@
#include <string>
#include <vector>
// Tests intentionally capture state in coroutine lambdas; lifetimes are
// controlled by Gate synchronization and join() before scope exit.
// NOLINTBEGIN(cppcoreguidelines-avoid-capturing-lambda-coroutines)
namespace xrpl::test {
/**
@@ -96,7 +100,7 @@ public:
void
signal()
{
std::lock_guard lk(mutex_);
std::scoped_lock const lk(mutex_);
signaled_ = true;
cv_.notify_all();
}
@@ -253,8 +257,8 @@ public:
auto& jq = env.app().getJobQueue();
static int const N = 4;
std::array<std::shared_ptr<JobQueue::CoroTaskRunner>, N> a;
static constexpr int kN = 4;
std::array<std::shared_ptr<JobQueue::CoroTaskRunner>, kN> a;
LocalValue<int> lv(-1);
BEAST_EXPECT(*lv == -1);
@@ -270,7 +274,7 @@ public:
return;
BEAST_EXPECT(*lv == -1);
for (int i = 0; i < N; ++i)
for (int i = 0; i < kN; ++i)
{
jq.postCoroTask(
JtClient,
@@ -514,7 +518,7 @@ public:
JtClient, "CoroTaskTest", [rp = &result, gp = &g](auto) -> CoroTask<void> {
auto add = [](int a, int b) -> CoroTask<int> { co_return a + b; };
auto mul = [add](int a, int b) -> CoroTask<int> {
int sum = co_await add(a, b);
int const sum = co_await add(a, b);
co_return sum * 2;
};
*rp = co_await mul(3, 4);
@@ -573,3 +577,5 @@ public:
BEAST_DEFINE_TESTSUITE(CoroTask, core, xrpl);
} // namespace xrpl::test
// NOLINTEND(cppcoreguidelines-avoid-capturing-lambda-coroutines)