avoid move ctor and assing operator in cpp generated code (#36)

This commit is contained in:
Ivan FB 2026-05-21 16:38:13 +02:00 committed by GitHub
parent 5e6e58e7d1
commit c7cf46bdea
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 54 additions and 73 deletions

View File

@ -7,9 +7,9 @@ int main() {
auto ctx = MyTimerCtx::create(TimerConfig{"cpp-demo"});
std::cout << "[1] Context created\n";
auto versionFuture = ctx.versionAsync();
auto echo1Future = ctx.echoAsync(EchoRequest{"hello from C++", 200});
auto echo2Future = ctx.echoAsync(EchoRequest{"second C++ request", 50});
auto versionFuture = ctx->versionAsync();
auto echo1Future = ctx->echoAsync(EchoRequest{"hello from C++", 200});
auto echo2Future = ctx->echoAsync(EchoRequest{"second C++ request", 50});
auto version = versionFuture.get();
std::cout << "[2] Version: " << version << "\n";
@ -29,7 +29,7 @@ int main() {
std::optional<int64_t>(3)
};
auto complexFuture = ctx.complexAsync(complexReq);
auto complexFuture = ctx->complexAsync(complexReq);
auto complex = complexFuture.get();
std::cout << "[5] Complex: summary=" << complex.summary
<< ", itemCount=" << complex.itemCount
@ -55,7 +55,7 @@ int main() {
/*jitter*/ std::optional<int64_t>(250),
};
auto scheduleFuture = ctx.scheduleAsync(job, retry, schedule);
auto scheduleFuture = ctx->scheduleAsync(job, retry, schedule);
auto scheduleRes = scheduleFuture.get();
std::cout << "[6] Schedule (3 complex params): jobId=" << scheduleRes.jobId
<< ", willRunCount=" << scheduleRes.willRunCount

View File

@ -661,7 +661,7 @@ inline std::vector<std::uint8_t> ffi_call_(std::function<int(FFICallback, void*)
class MyTimerCtx {
public:
static MyTimerCtx create(const TimerConfig& config, std::chrono::milliseconds timeout = std::chrono::seconds{30}) {
static std::unique_ptr<MyTimerCtx> create(const TimerConfig& config, std::chrono::milliseconds timeout = std::chrono::seconds{30}) {
const auto ffi_req_ = MyTimerCreateCtorReq{config};
const auto ffi_req_bytes_ = encodeCborFFI(ffi_req_);
const auto ffi_raw_ = ffi_call_([&](FFICallback cb, void* ud) {
@ -671,28 +671,25 @@ public:
const auto addr_str = decodeCborFFI<std::string>(ffi_raw_);
try {
const auto addr = std::stoull(addr_str);
return MyTimerCtx(reinterpret_cast<void*>(static_cast<uintptr_t>(addr)), timeout);
return std::unique_ptr<MyTimerCtx>(new MyTimerCtx(reinterpret_cast<void*>(static_cast<uintptr_t>(addr)), timeout));
} catch (const std::exception&) {
throw std::runtime_error("FFI create returned non-numeric address: " + addr_str);
}
}
static std::future<MyTimerCtx> createAsync(const TimerConfig& config, std::chrono::milliseconds timeout = std::chrono::seconds{30}) {
static std::future<std::unique_ptr<MyTimerCtx>> createAsync(const TimerConfig& config, std::chrono::milliseconds timeout = std::chrono::seconds{30}) {
return std::async(std::launch::async, [config, timeout]() { return create(config, timeout); });
}
// Rule of Five: because this class owns a raw resource (the my_timer
// context pointer freed in the destructor), the compiler-generated copy
// and move special members would do the wrong thing — copies would
// double-free, and a default move would leave both objects pointing at
// the same context. So we define all five special members explicitly:
// 1. destructor — releases the context.
// 2. copy constructor — deleted; contexts are not copyable.
// 3. copy assignment — deleted; same reason.
// 4. move constructor — transfers ownership, nulls the source.
// 5. move assignment — destroys the current context, then
// transfers ownership from `other`.
// See: https://en.cppreference.com/w/cpp/language/rule_of_three
// Special-member policy: this class owns a my_timer context, which in
// turn owns the library's worker thread(s) and internal state. Moving
// such an object out from under a caller silently tears that state
// down and is easy to misuse (e.g. storing in a container that
// relocates its elements). It also has no clean analogue in the other
// binding languages we generate. So copies and moves are both
// deleted; ownership is transferred via MyTimerCtx::create returning a
// std::unique_ptr<MyTimerCtx>. The destructor still releases the
// context.
~MyTimerCtx() {
if (ptr_) {
my_timer_destroy(ptr_);
@ -702,19 +699,8 @@ public:
MyTimerCtx(const MyTimerCtx&) = delete;
MyTimerCtx& operator=(const MyTimerCtx&) = delete;
MyTimerCtx(MyTimerCtx&& other) noexcept : ptr_(other.ptr_), timeout_(other.timeout_) {
other.ptr_ = nullptr;
}
MyTimerCtx& operator=(MyTimerCtx&& other) noexcept {
if (this != &other) {
if (ptr_) my_timer_destroy(ptr_);
ptr_ = other.ptr_;
timeout_ = other.timeout_;
other.ptr_ = nullptr;
}
return *this;
}
MyTimerCtx(MyTimerCtx&&) = delete;
MyTimerCtx& operator=(MyTimerCtx&&) = delete;
EchoResponse echo(const EchoRequest& req) const {
const auto ffi_req_ = MyTimerEchoReq{req};

View File

@ -282,7 +282,14 @@ proc generateCppHeader*(
# path anyway since it carries the CBOR-encoded ctx address. Discard the
# synchronous return and yield 0 from the lambda; the address comes back
# through the callback's CBOR text-string payload.
lines.add(" static $1 create($2) {" % [ctxTypeName, ctorParamsWithTimeout])
# `create` returns std::unique_ptr<Ctx> rather than a Ctx by value: the
# context owns library threads, so we forbid copy/move on the class
# itself (see ContextRuleOf5Tpl) and hand out ownership through a
# smart pointer that callers can move, store in containers, etc.
lines.add(
" static std::unique_ptr<$1> create($2) {" %
[ctxTypeName, ctorParamsWithTimeout]
)
lines.add(" const auto ffi_req_ = $1;" % [reqInit])
lines.add(" const auto ffi_req_bytes_ = encodeCborFFI(ffi_req_);")
lines.add(" const auto ffi_raw_ = ffi_call_([&](FFICallback cb, void* ud) {")
@ -295,8 +302,9 @@ proc generateCppHeader*(
lines.add(" const auto addr_str = decodeCborFFI<std::string>(ffi_raw_);")
lines.add(" try {")
lines.add(" const auto addr = std::stoull(addr_str);")
# Use `new` directly (not std::make_unique) so the ctor can stay private.
lines.add(
" return $1(reinterpret_cast<void*>(static_cast<uintptr_t>(addr)), timeout);" %
" return std::unique_ptr<$1>(new $1(reinterpret_cast<void*>(static_cast<uintptr_t>(addr)), timeout));" %
[ctxTypeName]
)
lines.add(" } catch (const std::exception&) {")
@ -318,7 +326,7 @@ proc generateCppHeader*(
else:
"timeout"
lines.add(
" static std::future<$1> createAsync($2) {" %
" static std::future<std::unique_ptr<$1>> createAsync($2) {" %
[ctxTypeName, ctorParamsWithTimeout]
)
lines.add(

View File

@ -1,15 +1,12 @@
// Rule of Five: because this class owns a raw resource (the {{LIB}}
// context pointer freed in the destructor), the compiler-generated copy
// and move special members would do the wrong thing — copies would
// double-free, and a default move would leave both objects pointing at
// the same context. So we define all five special members explicitly:
// 1. destructor — releases the context.
// 2. copy constructor — deleted; contexts are not copyable.
// 3. copy assignment — deleted; same reason.
// 4. move constructor — transfers ownership, nulls the source.
// 5. move assignment — destroys the current context, then
// transfers ownership from `other`.
// See: https://en.cppreference.com/w/cpp/language/rule_of_three
// Special-member policy: this class owns a {{LIB}} context, which in
// turn owns the library's worker thread(s) and internal state. Moving
// such an object out from under a caller silently tears that state
// down and is easy to misuse (e.g. storing in a container that
// relocates its elements). It also has no clean analogue in the other
// binding languages we generate. So copies and moves are both
// deleted; ownership is transferred via {{CTX}}::create returning a
// std::unique_ptr<{{CTX}}>. The destructor still releases the
// context.
~{{CTX}}() {
if (ptr_) {
{{LIB}}_destroy(ptr_);
@ -19,16 +16,5 @@
{{CTX}}(const {{CTX}}&) = delete;
{{CTX}}& operator=(const {{CTX}}&) = delete;
{{CTX}}({{CTX}}&& other) noexcept : ptr_(other.ptr_), timeout_(other.timeout_) {
other.ptr_ = nullptr;
}
{{CTX}}& operator=({{CTX}}&& other) noexcept {
if (this != &other) {
if (ptr_) {{LIB}}_destroy(ptr_);
ptr_ = other.ptr_;
timeout_ = other.timeout_;
other.ptr_ = nullptr;
}
return *this;
}
{{CTX}}({{CTX}}&&) = delete;
{{CTX}}& operator=({{CTX}}&&) = delete;

View File

@ -11,6 +11,7 @@
#include <atomic>
#include <chrono>
#include <future>
#include <memory>
#include <string>
#include <thread>
#include <vector>
@ -19,7 +20,7 @@
namespace {
MyTimerCtx makeCtx(const std::string& name = "e2e") {
std::unique_ptr<MyTimerCtx> makeCtx(const std::string& name = "e2e") {
return MyTimerCtx::create(TimerConfig{name});
}
@ -34,19 +35,19 @@ TEST(TimerE2E, CreateAndDestroy) {
TEST(TimerE2E, VersionSync) {
auto ctx = makeCtx("version-sync");
const auto v = ctx.version();
const auto v = ctx->version();
EXPECT_EQ(v, "nim-timer v0.1.0");
}
TEST(TimerE2E, VersionAsync) {
auto ctx = makeCtx("version-async");
auto fut = ctx.versionAsync();
auto fut = ctx->versionAsync();
EXPECT_EQ(fut.get(), "nim-timer v0.1.0");
}
TEST(TimerE2E, EchoRoundTripsMessageAndTimerName) {
auto ctx = makeCtx("echo-ctx");
const auto resp = ctx.echo(EchoRequest{"hello", 10});
const auto resp = ctx->echo(EchoRequest{"hello", 10});
EXPECT_EQ(resp.echoed, "hello");
EXPECT_EQ(resp.timerName, "echo-ctx");
}
@ -56,7 +57,7 @@ TEST(TimerE2E, EchoHonoursDelay) {
constexpr int delayMs = 150;
const auto start = std::chrono::steady_clock::now();
const auto resp = ctx.echo(EchoRequest{"waited", delayMs});
const auto resp = ctx->echo(EchoRequest{"waited", delayMs});
const auto elapsed = std::chrono::duration_cast<std::chrono::milliseconds>(
std::chrono::steady_clock::now() - start).count();
@ -68,9 +69,9 @@ TEST(TimerE2E, EchoHonoursDelay) {
TEST(TimerE2E, ConcurrentAsyncCallsAreIndependent) {
auto ctx = makeCtx("concurrent");
auto f1 = ctx.echoAsync(EchoRequest{"one", 80});
auto f2 = ctx.echoAsync(EchoRequest{"two", 40});
auto f3 = ctx.echoAsync(EchoRequest{"three", 20});
auto f1 = ctx->echoAsync(EchoRequest{"one", 80});
auto f2 = ctx->echoAsync(EchoRequest{"two", 40});
auto f3 = ctx->echoAsync(EchoRequest{"three", 20});
const auto r3 = f3.get();
const auto r2 = f2.get();
@ -93,7 +94,7 @@ TEST(TimerE2E, ComplexWithOptionalNotePresent) {
std::optional<int64_t>(2),
};
const auto resp = ctx.complex(req);
const auto resp = ctx->complex(req);
EXPECT_EQ(resp.itemCount, 2);
EXPECT_TRUE(resp.hasNote);
EXPECT_NE(resp.summary.find("note=a note"), std::string::npos)
@ -111,7 +112,7 @@ TEST(TimerE2E, ComplexWithOptionalNoteAbsent) {
std::nullopt,
};
const auto resp = ctx.complex(req);
const auto resp = ctx->complex(req);
EXPECT_EQ(resp.itemCount, 0);
EXPECT_FALSE(resp.hasNote);
EXPECT_NE(resp.summary.find("note=<none>"), std::string::npos)
@ -124,8 +125,8 @@ TEST(TimerE2E, IndependentContextsKeepTheirOwnState) {
auto ctxA = makeCtx("alpha");
auto ctxB = makeCtx("beta");
const auto rA = ctxA.echo(EchoRequest{"x", 5});
const auto rB = ctxB.echo(EchoRequest{"x", 5});
const auto rA = ctxA->echo(EchoRequest{"x", 5});
const auto rB = ctxB->echo(EchoRequest{"x", 5});
EXPECT_EQ(rA.timerName, "alpha");
EXPECT_EQ(rB.timerName, "beta");