From 1c5d5b2e4646c864eec3e50684ce56ff4f3f33c4 Mon Sep 17 00:00:00 2001 From: Yaraslau Tamashevich Date: Sun, 12 Jul 2026 18:50:07 +0200 Subject: [PATCH 1/3] clang-tidy updates --- .clang-tidy | 17 +- CMakeLists.txt | 8 + include/morph/action_log.hpp | 10 +- include/morph/backend.hpp | 18 +- include/morph/bridge.hpp | 44 ++-- include/morph/choice.hpp | 3 +- include/morph/completion.hpp | 18 +- include/morph/datetime.hpp | 16 ++ include/morph/executor.hpp | 11 +- include/morph/file_action_log.hpp | 13 +- include/morph/forms.hpp | 6 +- include/morph/journal.hpp | 9 +- include/morph/logger.hpp | 58 ++++- include/morph/model.hpp | 4 + include/morph/network_monitor.hpp | 2 +- include/morph/offline_queue.hpp | 10 +- include/morph/rational.hpp | 67 +++-- include/morph/reconnect_coordinator.hpp | 9 +- include/morph/registry.hpp | 32 ++- include/morph/remote.hpp | 38 +-- include/morph/session.hpp | 9 +- include/morph/strand.hpp | 14 +- include/morph/sync_worker.hpp | 4 +- include/morph/wire.hpp | 2 +- src/main.cpp | 2 + tests/test_coverage_push95.cpp | 2 +- tests/test_quantity_forms.cpp | 26 +- tests/test_rational.cpp | 314 ++++++++++++------------ 28 files changed, 451 insertions(+), 315 deletions(-) diff --git a/.clang-tidy b/.clang-tidy index bca07ca..103f2fb 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -3,17 +3,22 @@ Checks: > clang-diagnostic-*, clang-analyzer-*, bugprone-*, + cppcoreguidelines-*, + misc-*, modernize-*, performance-*, + portability-*, readability-*, + cert-*, + concurrency-*, -modernize-use-trailing-return-type, -readability-magic-numbers, - -readability-named-parameter, - -bugprone-easily-swappable-parameters, - -readability-convert-member-functions-to-static, - -bugprone-empty-catch + -cppcoreguidelines-avoid-magic-numbers, + -misc-non-private-member-variables-in-classes, + -portability-avoid-pragma-once, + -misc-include-cleaner -WarningsAsErrors: "" +WarningsAsErrors: "*" HeaderFilterRegex: "include/morph/.*" @@ -37,4 +42,4 @@ CheckOptions: - key: readability-identifier-length.IgnoredVariableNames value: "^[ijk]$|^[xy]$|^lk$|^cb$|^op$|^fn$" - key: readability-identifier-length.IgnoredLoopCounterNames - value: "^[ijk]$" + value: "^[ijk]$" \ No newline at end of file diff --git a/CMakeLists.txt b/CMakeLists.txt index 8f809f3..4a5705e 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -22,6 +22,7 @@ if(MORPH_BUILD_FORMS_QML AND (NOT MORPH_BUILD_EXAMPLES OR EMSCRIPTEN)) endif() option(MORPH_BUILD_QT "Build Qt6 WebSocket backend and tests" OFF) option(MORPH_BUILD_DOCUMENTATION "Build doxygen docs" OFF) +option(MORPH_BUILD_CLANG_TIDY "Enable clang-tidy checks (warnings-as-errors)" OFF) # Both Qt consumers (the WebSocket backend and the forms QML renderer) need # Qt6 auto-detected on Windows before their find_package(Qt6 ...) calls run. @@ -31,6 +32,13 @@ endif() include(cmake/compiler_options.cmake) +# ── clang-tidy integration ───────────────────────────────────────────────── +if(MORPH_BUILD_CLANG_TIDY) + find_program(CLANG_TIDY_BIN clang-tidy REQUIRED) + set(CMAKE_CXX_CLANG_TIDY "${CLANG_TIDY_BIN};--warnings-as-errors=*;--header-filter=include/morph/.*") + message(STATUS "clang-tidy: ${CLANG_TIDY_BIN} (warnings as errors)") +endif() + # Registered before any subdirectory so demo-level add_test calls (examples/ # forms) land in the test set alongside the Catch2 suite. if(MORPH_BUILD_TESTS) diff --git a/include/morph/action_log.hpp b/include/morph/action_log.hpp index 07142fe..c9d45cb 100644 --- a/include/morph/action_log.hpp +++ b/include/morph/action_log.hpp @@ -104,6 +104,7 @@ inline LogEntry fromJson(std::string_view json) { /// items once retried successfully). Implementations range from in-memory /// (`InMemoryActionLog`) to file, SQL, or network-backed stores supplied by the /// host application. +// NOLINTBEGIN(cppcoreguidelines-special-member-functions) struct IActionLog { virtual ~IActionLog() = default; @@ -120,6 +121,7 @@ struct IActionLog { /// @return Matching entries, in append order. [[nodiscard]] virtual std::vector entries(std::string_view entityKey = {}) const = 0; }; +// NOLINTEND(cppcoreguidelines-special-member-functions) /// @brief Thread-safe in-memory implementation of `IActionLog`. /// @@ -130,7 +132,7 @@ class InMemoryActionLog : public IActionLog { /// @brief Appends @p entry, assigning a monotonically increasing `seq`. Thread-safe. /// @param entry Entry to append; `seq` is overwritten regardless of the input value. void append(LogEntry entry) override { - std::scoped_lock lock{_mtx}; + std::scoped_lock const lock{_mtx}; entry.seq = ++_nextSeq; _entries.push_back(std::move(entry)); } @@ -142,7 +144,7 @@ class InMemoryActionLog : public IActionLog { /// @param entityKey If non-empty, restricts the result to that entity's entries. /// @return Matching entries, in append order. [[nodiscard]] std::vector entries(std::string_view entityKey = {}) const override { - std::scoped_lock lock{_mtx}; + std::scoped_lock const lock{_mtx}; if (entityKey.empty()) { return _entries; } @@ -190,7 +192,7 @@ inline std::pair>& defaultActionLogState /// (existing instances keep whatever they already have). inline void setActionLog(std::shared_ptr log) { auto& [mtx, slot] = detail::defaultActionLogState(); - std::scoped_lock lock{mtx}; + std::scoped_lock const lock{mtx}; slot = std::move(log); } @@ -198,7 +200,7 @@ inline void setActionLog(std::shared_ptr log) { /// if none has been set. Thread-safe. [[nodiscard]] inline std::shared_ptr defaultActionLog() { auto& [mtx, slot] = detail::defaultActionLogState(); - std::scoped_lock lock{mtx}; + std::scoped_lock const lock{mtx}; return slot; } diff --git a/include/morph/backend.hpp b/include/morph/backend.hpp index e252b4d..759ac00 100644 --- a/include/morph/backend.hpp +++ b/include/morph/backend.hpp @@ -54,6 +54,7 @@ struct ActionCall { /// A backend owns model instances and dispatches actions against them. /// `Bridge` holds one active backend at a time and can swap it atomically /// via `Bridge::switchBackend()`. +// NOLINTBEGIN(cppcoreguidelines-special-member-functions) struct IBackend { virtual ~IBackend() = default; @@ -112,8 +113,9 @@ struct IBackend { /// `LocalBackend`) never invoke it. /// @param handler Callable invoked on the backend's transport thread after a /// successful reconnect. Pass `nullptr` to clear. - virtual void setReconnectHandler(std::function handler) { (void)handler; } + virtual void setReconnectHandler(const std::function& handler) { (void)handler; } }; +// NOLINTEND(cppcoreguidelines-special-member-functions) } // namespace detail @@ -161,7 +163,7 @@ class LocalBackend : public detail::IBackend { const std::string& /*typeId*/, std::function()> factory) override { ::morph::exec::detail::ModelId mid{_nextId.fetch_add(1) + 1}; - std::scoped_lock lock{_regMtx}; + std::scoped_lock const lock{_regMtx}; _models[mid] = factory(); return mid; } @@ -169,13 +171,13 @@ class LocalBackend : public detail::IBackend { /// @brief Removes the model with @p mid. Thread-safe. /// @param mid Id returned by a prior `registerModel()` call. void deregisterModel(::morph::exec::detail::ModelId mid) override { - std::scoped_lock lock{_regMtx}; + std::scoped_lock const lock{_regMtx}; _models.erase(mid); } /// @brief Notifies every live model that implements `IBackendChangedSink`. Thread-safe. void notifyBackendChanged() override { - std::scoped_lock lock{_regMtx}; + std::scoped_lock const lock{_regMtx}; for (auto& [modelId, holder] : _models) { if (auto* sink = dynamic_cast<::morph::model::detail::IBackendChangedSink*>(holder.get())) { sink->onBackendChanged(); @@ -200,7 +202,7 @@ class LocalBackend : public detail::IBackend { std::shared_ptr<::morph::model::detail::IModelHolder> holder; { - std::scoped_lock lock{_regMtx}; + std::scoped_lock const lock{_regMtx}; auto iter = _models.find(mid); if (iter != _models.end()) { holder = iter->second; @@ -217,7 +219,7 @@ class LocalBackend : public detail::IBackend { _strand.post(mid, [localOp = std::move(localOp), holder = std::move(holder), compState, session = std::move(session)]() mutable { try { - ::morph::session::detail::ScopedContext scoped{session}; + ::morph::session::detail::ScopedContext const scoped{session}; compState->setValue(localOp(*holder)); } catch (...) { compState->setException(std::current_exception()); @@ -231,7 +233,7 @@ class LocalBackend : public detail::IBackend { void cancelPending(const std::exception_ptr& exc) override { std::vector>>> snapshot; { - std::scoped_lock lock{_pendingMtx}; + std::scoped_lock const lock{_pendingMtx}; snapshot.swap(_pending); } for (auto& weak : snapshot) { @@ -243,7 +245,7 @@ class LocalBackend : public detail::IBackend { private: void trackPending(const std::shared_ptr<::morph::async::detail::CompletionState>>& state) { - std::scoped_lock lock{_pendingMtx}; + std::scoped_lock const lock{_pendingMtx}; std::erase_if(_pending, [](const auto& weak) { return weak.expired(); }); _pending.emplace_back(state); } diff --git a/include/morph/bridge.hpp b/include/morph/bridge.hpp index 0c58065..77bba37 100644 --- a/include/morph/bridge.hpp +++ b/include/morph/bridge.hpp @@ -79,14 +79,7 @@ class ActionExecuteRegistry { private: using Key = std::pair; - struct KeyHash { - std::size_t operator()(const Key& key) const noexcept { - std::size_t seed = std::hash{}(key.first); - seed ^= std::hash{}(key.second) + 0x9e3779b9 + (seed << 6) + (seed >> 2); - return seed; - } - }; - std::unordered_map _executors; + std::unordered_map _executors; }; inline ActionExecuteRegistry& ActionExecuteRegistry::instance() { @@ -206,7 +199,7 @@ class Bridge { auto binding = std::make_shared(); binding->typeId = std::string{::morph::model::ModelTraits::typeId()}; binding->modelFactory = [] { return ::morph::model::detail::ModelFactory::create(); }; - std::scoped_lock lock{_mtx}; + std::scoped_lock const lock{_mtx}; binding->currentId.store( loadBackend()->registerModelWithContext(binding->typeId, binding->modelFactory, binding->contextKey).v); _handlers.push_back(binding); @@ -220,7 +213,7 @@ class Bridge { /// `binding->contextKey` needs to be set before registration (see `HandlerBinding::contextKey`). /// @param binding Pre-constructed binding. Its `typeId` and `modelFactory` must be set. void registerHandler(const std::shared_ptr& binding) { - std::scoped_lock lock{_mtx}; + std::scoped_lock const lock{_mtx}; binding->currentId.store( loadBackend()->registerModelWithContext(binding->typeId, binding->modelFactory, binding->contextKey).v); _handlers.push_back(binding); @@ -235,14 +228,14 @@ class Bridge { /// /// @param session The new default. Pass `{}` to clear. void setDefaultSession(::morph::session::Context session) { - std::scoped_lock lock{_sessionMtx}; + std::scoped_lock const lock{_sessionMtx}; _defaultSession = std::move(session); } /// @brief Returns a copy of the currently installed default session. Thread-safe. /// @return Snapshot of the default `Context`. [[nodiscard]] ::morph::session::Context defaultSession() const { - std::scoped_lock lock{_sessionMtx}; + std::scoped_lock const lock{_sessionMtx}; return _defaultSession; } @@ -263,7 +256,7 @@ class Bridge { auto newShared = std::shared_ptr<::morph::backend::detail::IBackend>(std::move(newBackend)); std::shared_ptr<::morph::backend::detail::IBackend> previous; { - std::scoped_lock lock{_mtx}; + std::scoped_lock const lock{_mtx}; std::vector> live; for (auto& weak : _handlers) { @@ -298,8 +291,8 @@ class Bridge { /// complete with an error. /// @param binding Binding to remove. Must have been returned by `registerHandler()`. void deregisterHandler(const std::shared_ptr& binding) { - std::scoped_lock lock{_mtx}; - uint64_t raw = binding->currentId.load(); + std::scoped_lock const lock{_mtx}; + uint64_t const raw = binding->currentId.load(); if (raw != 0U) { loadBackend()->deregisterModel(::morph::exec::detail::ModelId{raw}); } @@ -332,7 +325,7 @@ class Bridge { using R = ::morph::model::ActionTraits::Result; auto backend = loadBackend(); - uint64_t raw = binding->currentId.load(); + uint64_t const raw = binding->currentId.load(); auto typedState = std::make_shared<::morph::async::detail::CompletionState>(); ::morph::async::Completion typed{typedState, cbExec}; @@ -372,7 +365,7 @@ class Bridge { return result; }; { - std::scoped_lock lock{_sessionMtx}; + std::scoped_lock const lock{_sessionMtx}; call.session = _defaultSession; } auto anyCompletion = backend->execute(::morph::exec::detail::ModelId{raw}, std::move(call), cbExec); @@ -386,13 +379,13 @@ class Bridge { private: std::shared_ptr<::morph::backend::detail::IBackend> loadBackend() const { - std::scoped_lock lock{_backendMtx}; + std::scoped_lock const lock{_backendMtx}; return _backend; } std::shared_ptr<::morph::backend::detail::IBackend> exchangeBackend( std::shared_ptr<::morph::backend::detail::IBackend> next) { - std::scoped_lock lock{_backendMtx}; + std::scoped_lock const lock{_backendMtx}; auto previous = std::move(_backend); _backend = std::move(next); return previous; @@ -405,10 +398,10 @@ class Bridge { // The handler is invoked on the backend's transport thread. We keep a // weak_ptr to the same shared backend so a stale callback fired after a // switchBackend is a no-op. - std::weak_ptr<::morph::backend::detail::IBackend> weakBackend{backend}; + std::weak_ptr<::morph::backend::detail::IBackend> const weakBackend{backend}; backend->setReconnectHandler([this, weakBackend] { auto pinned = weakBackend.lock(); - std::scoped_lock lock{_mtx}; + std::scoped_lock const lock{_mtx}; if (!pinned || pinned != loadBackend()) { return; // We've moved on to a different backend; ignore. } @@ -448,6 +441,7 @@ class Bridge { /// /// @tparam Model Concrete model type. template +// NOLINTNEXTLINE(cppcoreguidelines-special-member-functions) class BridgeHandler { public: /// @brief Constructs and registers the handler using the default model factory. @@ -607,6 +601,9 @@ class BridgeHandler { Bridge* bridge{nullptr}; std::shared_ptr binding; ::morph::exec::IExecutor* guiExec{nullptr}; + /// @note Keys are `std::string_view` pointing to string literals from + /// `ActionTraits::typeId()` — all call sites pass compile-time strings + /// with static storage duration, so the map's keys never dangle. std::unordered_map entries; }; @@ -710,14 +707,15 @@ class BridgeHandler { /// Placed here after BridgeHandler is fully defined so we can safely cast and call its methods. template inline void ActionExecuteRegistry::registerAction(std::string_view modelId, std::string_view actionId) { - Key key{std::string{modelId}, std::string{actionId}}; + Key const key{std::string{modelId}, std::string{actionId}}; _executors[key] = [](void* handlerVoid, std::string_view bodyJson) -> ::morph::async::Completion { auto* handler = static_cast*>(handlerVoid); auto resultState = std::make_shared<::morph::async::detail::CompletionState>(); try { Action action = ::morph::model::ActionTraits::fromJson(bodyJson); handler->template execute(std::move(action)) - .then([resultState](typename ::morph::model::ActionTraits::Result result) { + // NOLINTNEXTLINE(performance-unnecessary-value-param) — lambda captures the result by value + .then([resultState](auto result) { resultState->setValue(::morph::model::ActionTraits::resultToJson(result)); }) .onError([resultState](const std::exception_ptr& err) { resultState->setException(err); }); diff --git a/include/morph/choice.hpp b/include/morph/choice.hpp index 9088a13..c1b5dea 100644 --- a/include/morph/choice.hpp +++ b/include/morph/choice.hpp @@ -53,9 +53,10 @@ struct FixedString { /// @brief Captures a string literal. /// @param literal The literal to copy, e.g. `"ListSamples"`. - // NOLINTNEXTLINE(modernize-avoid-c-arrays) — string literals ARE C arrays. + // NOLINTNEXTLINE(modernize-avoid-c-arrays, cppcoreguidelines-avoid-c-arrays, cppcoreguidelines-pro-bounds-constant-array-index, cppcoreguidelines-pro-bounds-avoid-unchecked-container-access) — string literals ARE C arrays. consteval FixedString(const char (&literal)[N]) noexcept { for (std::size_t index = 0; index < N; ++index) { + // NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-constant-array-index, cppcoreguidelines-pro-bounds-avoid-unchecked-container-access) data[index] = literal[index]; } } diff --git a/include/morph/completion.hpp b/include/morph/completion.hpp index b64fb7c..f147f8f 100644 --- a/include/morph/completion.hpp +++ b/include/morph/completion.hpp @@ -15,6 +15,7 @@ namespace morph::async { namespace detail { +// NOLINTBEGIN(cppcoreguidelines-special-member-functions) template struct CompletionState { std::mutex mtx; @@ -29,7 +30,7 @@ struct CompletionState { void setValue(T val) { std::function callback; { - std::scoped_lock lock{mtx}; + std::scoped_lock const lock{mtx}; if (ready) { return; } @@ -50,7 +51,7 @@ struct CompletionState { void setException(const std::exception_ptr& exc) { std::function callback; { - std::scoped_lock lock{mtx}; + std::scoped_lock const lock{mtx}; if (ready) { return; } @@ -70,7 +71,7 @@ struct CompletionState { void attachThen(std::function handler) { std::function fireNow; { - std::scoped_lock lock{mtx}; + std::scoped_lock const lock{mtx}; if (ready && value) { auto savedVal = *value; fireNow = [handler = std::move(handler), savedVal]() mutable { handler(std::move(savedVal)); }; @@ -85,7 +86,7 @@ struct CompletionState { void attachOnError(std::function handler) { std::function fireNow; { - std::scoped_lock lock{mtx}; + std::scoped_lock const lock{mtx}; onErrAttached = true; if (ready && error) { auto savedErr = error; @@ -102,6 +103,7 @@ struct CompletionState { if (!ready || !error || onErrAttached) { return; } + // NOLINTBEGIN(bugprone-empty-catch) — logError may throw; we swallow to avoid noexcept-escape try { std::rethrow_exception(error); } catch (const std::exception& exc) { @@ -115,9 +117,12 @@ struct CompletionState { } catch (...) { } } + // NOLINTEND(bugprone-empty-catch) } }; +// NOLINTEND(cppcoreguidelines-special-member-functions) + } // namespace detail /// @brief Move-only handle representing the eventual result of an asynchronous operation. @@ -125,8 +130,6 @@ struct CompletionState { /// Callbacks are posted to the `IExecutor` supplied at construction time, so /// they always run on the intended thread (e.g. the GUI thread). /// -/// @tparam T Type of the success value. -/// /// @par Thread safety /// `then()` and `onError()` may be called from any thread. The registered /// callbacks are invoked via the executor, never directly from the producing thread. @@ -134,7 +137,10 @@ struct CompletionState { /// @par Orphan detection /// If a `Completion` is destroyed before an `onError()` handler is attached and /// the operation has already failed, the exception is logged as an orphan error. +/// +/// @tparam T Type of the success value. template +// NOLINTNEXTLINE(cppcoreguidelines-special-member-functions) class Completion { public: /// @brief Constructs an empty (no-op) completion. diff --git a/include/morph/datetime.hpp b/include/morph/datetime.hpp index 6eaeb0e..25e3ef2 100644 --- a/include/morph/datetime.hpp +++ b/include/morph/datetime.hpp @@ -97,6 +97,7 @@ struct DateTime { /// lowercase separators, no trailing input. /// @param text Candidate ISO-8601 string. /// @return The parsed instant, or `std::nullopt` when @p text is malformed. + // NOLINTBEGIN(cppcoreguidelines-pro-bounds-pointer-arithmetic, cppcoreguidelines-pro-bounds-avoid-unchecked-container-access) [[nodiscard]] static std::optional fromIso8601(std::string_view text) noexcept { // Callers only pass offsets within the length-checked prefix below. auto const number = [&text](std::size_t offset, std::size_t count) noexcept -> std::optional { @@ -166,6 +167,7 @@ struct DateTime { std::chrono::seconds{*second}, std::chrono::milliseconds{milliseconds}}; } + // NOLINTEND(cppcoreguidelines-pro-bounds-pointer-arithmetic, cppcoreguidelines-pro-bounds-avoid-unchecked-container-access) /// @brief Ordering on the underlying instant. /// @param other Instant to compare against. @@ -260,6 +262,18 @@ struct Timestamp { [[nodiscard]] constexpr auto operator<=>(const Timestamp& other) const noexcept = default; }; +/// @brief The signed duration between two engaged timestamps. +/// @param lhs Minuend. +/// @param rhs Subtrahend. +/// @return `lhs - rhs` as a chrono duration, or `std::nullopt` if either is empty. +[[nodiscard]] constexpr std::optional operator-(const Timestamp& lhs, + const Timestamp& rhs) noexcept { + if (!lhs.value || !rhs.value) { + return std::nullopt; + } + return *lhs.value - *rhs.value; +} + } // namespace morph::time // --------------------------------------------------------------------------- @@ -328,6 +342,7 @@ struct to_json_schema { /// renders the ISO-8601 UTC form. template <> struct std::formatter { + // NOLINTBEGIN(readability-convert-member-functions-to-static) — std::formatter requires non-static methods constexpr auto parse(std::format_parse_context& ctx) { // The parse context always contains the terminating '}' (the format // machinery's contract), so the range is never empty here. @@ -341,3 +356,4 @@ struct std::formatter { return std::format_to(ctx.out(), "{}", value.toIso8601()); } }; +// NOLINTEND(readability-convert-member-functions-to-static) diff --git a/include/morph/executor.hpp b/include/morph/executor.hpp index 8f26278..0a2f668 100644 --- a/include/morph/executor.hpp +++ b/include/morph/executor.hpp @@ -7,7 +7,6 @@ #include #include #include -#include #include #include @@ -19,6 +18,7 @@ namespace morph::exec { /// /// Concrete implementations decide *where* and *when* posted tasks run /// (thread pool, main thread, Qt event loop, …). +// NOLINTBEGIN(cppcoreguidelines-special-member-functions) struct IExecutor { virtual ~IExecutor() = default; @@ -30,12 +30,14 @@ struct IExecutor { /// @param task Callable to execute. virtual void post(std::function task) = 0; }; +// NOLINTEND(cppcoreguidelines-special-member-functions) /// @brief Multi-threaded executor backed by a fixed-size thread pool. /// /// Tasks are placed in a FIFO queue and consumed by worker threads. /// Exceptions thrown by tasks are silently caught and discarded. /// The destructor blocks until all worker threads have exited. +// NOLINTNEXTLINE(cppcoreguidelines-special-member-functions) class ThreadPoolExecutor : public IExecutor { public: /// @brief Constructs the pool with @p n worker threads. @@ -51,7 +53,7 @@ class ThreadPoolExecutor : public IExecutor { /// Remaining queued tasks that have not started are dropped. ~ThreadPoolExecutor() override { { - std::scoped_lock lock{_m}; + std::scoped_lock const lock{_m}; _stop = true; } _cv.notify_all(); @@ -64,7 +66,7 @@ class ThreadPoolExecutor : public IExecutor { /// @param task Callable to execute. Thread-safe. void post(std::function task) override { { - std::scoped_lock lock{_m}; + std::scoped_lock const lock{_m}; _q.push(std::move(task)); } _cv.notify_one(); @@ -85,6 +87,7 @@ class ThreadPoolExecutor : public IExecutor { } try { task(); + // NOLINTNEXTLINE(bugprone-empty-catch) — task exceptions are intentionally discarded } catch (...) { } } @@ -109,7 +112,7 @@ class MainThreadExecutor : public IExecutor { /// @param task Callable to execute. void post(std::function task) override { { - std::scoped_lock lock{_m}; + std::scoped_lock const lock{_m}; _q.push(std::move(task)); } _cv.notify_all(); diff --git a/include/morph/file_action_log.hpp b/include/morph/file_action_log.hpp index cfec806..30fee50 100644 --- a/include/morph/file_action_log.hpp +++ b/include/morph/file_action_log.hpp @@ -12,7 +12,7 @@ #include #include -#if defined(_WIN32) +#ifdef _WIN32 #include #else #include @@ -57,6 +57,7 @@ class FileActionLog : public IActionLog { /// valid handle or throws before completing (in which case this destructor /// never runs), and copy/move are deleted, so there is no path that could /// null it out afterwards. + // NOLINTNEXTLINE(cert-err33-c) — destructor context, can't propagate errors ~FileActionLog() override { std::fclose(_file); } FileActionLog(const FileActionLog&) = delete; @@ -67,18 +68,20 @@ class FileActionLog : public IActionLog { /// @brief Appends @p entry as one JSON line. Buffered until `flush()`. Thread-safe. /// @param entry Entry to append; `seq` is overwritten regardless of the input value. void append(LogEntry entry) override { - std::scoped_lock lock{_mtx}; + std::scoped_lock const lock{_mtx}; entry.seq = ++_nextSeq; auto line = toJson(entry); line.push_back('\n'); + // NOLINTNEXTLINE(cert-err33-c) — append-only; fwrite errors checked via subsequent fsync std::fwrite(line.data(), 1, line.size(), _file); } /// @brief Flushes stdio's buffer, then fsyncs the file descriptor. Thread-safe. void flush() override { - std::scoped_lock lock{_mtx}; + std::scoped_lock const lock{_mtx}; + // NOLINTNEXTLINE(cert-err33-c) — errors logged by caller after flush std::fflush(_file); -#if defined(_WIN32) +#ifdef _WIN32 _commit(_fileno(_file)); #else ::fsync(fileno(_file)); @@ -94,7 +97,7 @@ class FileActionLog : public IActionLog { /// @param entityKey If non-empty, restricts the result to that entity's entries. /// @return Matching entries, in on-disk (append) order. [[nodiscard]] std::vector entries(std::string_view entityKey = {}) const override { - std::scoped_lock lock{_mtx}; + std::scoped_lock const lock{_mtx}; std::ifstream in{_path}; std::vector out; std::string line; diff --git a/include/morph/forms.hpp b/include/morph/forms.hpp index 4c82ce7..3aeee8f 100644 --- a/include/morph/forms.hpp +++ b/include/morph/forms.hpp @@ -108,7 +108,7 @@ concept HasOptionalFields = requires { template [[nodiscard]] constexpr bool declaredOptional(std::string_view fieldName) noexcept { if constexpr (HasOptionalFields) { - for (std::string_view candidate : A::optionalFields) { + for (std::string_view const candidate : A::optionalFields) { if (candidate == fieldName) { return true; } @@ -122,11 +122,13 @@ template /// @brief Invokes `visitor.operator()(name, member)` for every reflected /// member of @p action (glaze pure reflection). template +// NOLINTNEXTLINE(cppcoreguidelines-missing-std-forward, cppcoreguidelines-pro-bounds-avoid-unchecked-container-access) — member-tie iteration constexpr void forEachNamedMember(A&& action, Visitor&& visitor) { using Plain = std::remove_cvref_t; constexpr auto memberCount = glz::reflect::size; auto memberTie = glz::to_tie(action); [&](std::index_sequence) { + // NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-avoid-unchecked-container-access) — index bounded by reflect::size (visitor.template operator()(glz::reflect::keys[I], glz::get_member(action, get(memberTie))), ...); }(std::make_index_sequence{}); @@ -137,6 +139,7 @@ constexpr void forEachNamedMember(A&& action, Visitor&& visitor) { /// /// Separated from `schemaJson` so the fallback path (malformed input passes /// through unchanged) is directly testable. +// NOLINTBEGIN(cppcoreguidelines-pro-bounds-avoid-unchecked-container-access) — glaze DOM requires operator[] template [[nodiscard]] std::string mergeSchemaExtras(std::string rawSchema) { // u64 number mode: the schema carries int64/uint64 bounds in $defs, which @@ -196,6 +199,7 @@ template // untestable line here (write_json of a DOM we just built cannot fail). return glz::write_json(dom).value_or(rawSchema); } +// NOLINTEND(cppcoreguidelines-pro-bounds-avoid-unchecked-container-access) } // namespace detail diff --git a/include/morph/journal.hpp b/include/morph/journal.hpp index 6ba135f..f8d6ed0 100644 --- a/include/morph/journal.hpp +++ b/include/morph/journal.hpp @@ -63,7 +63,7 @@ class SessionLog : public IActionLog { /// nothing is coalesced or dropped here. /// @param entry Entry to append; `seq` is overwritten regardless of the input value. void append(LogEntry entry) override { - std::scoped_lock lock{_mtx}; + std::scoped_lock const lock{_mtx}; entry.seq = ++_nextSeq; _all.push_back(std::move(entry)); } @@ -75,7 +75,7 @@ class SessionLog : public IActionLog { /// @param entityKey If non-empty, restricts the result to that entity's entries. /// @return Matching entries, in append order. [[nodiscard]] std::vector entries(std::string_view entityKey = {}) const override { - std::scoped_lock lock{_mtx}; + std::scoped_lock const lock{_mtx}; if (entityKey.empty()) { return _all; } @@ -106,7 +106,7 @@ class SessionLog : public IActionLog { ::morph::model::detail::ActionDispatcher& dispatcher = ::morph::model::detail::defaultDispatcher()) { std::vector remaining; { - std::scoped_lock lock{_mtx}; + std::scoped_lock const lock{_mtx}; if (!_all.empty()) { _all.pop_back(); _committedUpTo = std::min(_committedUpTo, _all.size()); @@ -132,7 +132,7 @@ class SessionLog : public IActionLog { ::morph::model::detail::ActionDispatcher& dispatcher = ::morph::model::detail::defaultDispatcher()) { std::vector pending; { - std::scoped_lock lock{_mtx}; + std::scoped_lock const lock{_mtx}; if (_committedUpTo >= _all.size()) { return; } @@ -165,6 +165,7 @@ class SessionLog : public IActionLog { lastIndexForKey.emplace(std::move(key), out.size()); out.push_back(entry); } else { + // NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-avoid-unchecked-container-access) — index from same loop out[iter->second] = entry; } } diff --git a/include/morph/logger.hpp b/include/morph/logger.hpp index b57311f..7d46d2c 100644 --- a/include/morph/logger.hpp +++ b/include/morph/logger.hpp @@ -1,6 +1,7 @@ // SPDX-License-Identifier: Apache-2.0 #pragma once +#include #include #include #include @@ -57,7 +58,7 @@ inline LogState& logState() { /// @brief Internal emit entry point used by the public level helpers. inline void log(LogLevel level, std::string_view msg) { - std::scoped_lock lock{logState().mtx}; + std::scoped_lock const lock{logState().mtx}; auto& state = logState(); if (state.sink && level >= state.minLevel) { state.sink(level, msg); @@ -74,7 +75,7 @@ inline void log(LogLevel level, std::string_view msg) { /// current minimum. Pass a no-op lambda to silence all output. /// @param logger New sink function. inline void setLogger(std::function logger) { - std::scoped_lock lock{detail::logState().mtx}; + std::scoped_lock const lock{detail::logState().mtx}; detail::logState().sink = std::move(logger); } @@ -83,19 +84,29 @@ inline void setLogger(std::function logger) { /// Messages below this level are silently dropped. Thread-safe. /// @param level Minimum level to emit. inline void setLogLevel(LogLevel level) { - std::scoped_lock lock{detail::logState().mtx}; + std::scoped_lock const lock{detail::logState().mtx}; detail::logState().minLevel = level; } /// @brief Returns the current minimum log level. Thread-safe. /// @return The active minimum level. inline LogLevel getLogLevel() { - std::scoped_lock lock{detail::logState().mtx}; + std::scoped_lock const lock{detail::logState().mtx}; return detail::logState().minLevel; } // ── Level helpers ───────────────────────────────────────────────────────────── +namespace detail { + +/// @brief Formats @p fmt with @p args and forwards to `log()`. +template +void logFormat(LogLevel level, std::format_string fmt, Args&&... args) { + log(level, std::format(fmt, std::forward(args)...)); +} + +} // namespace detail + /// @brief Logs @p msg at `LogLevel::debug`. inline void logDebug(std::string_view msg) { detail::log(LogLevel::debug, msg); } /// @brief Logs @p msg at `LogLevel::info`. @@ -105,6 +116,27 @@ inline void logWarn(std::string_view msg) { detail::log(LogLevel::warn, msg); } /// @brief Logs @p msg at `LogLevel::error`. inline void logError(std::string_view msg) { detail::log(LogLevel::error, msg); } +/// @brief Logs a formatted message at `LogLevel::debug`. +template +void logDebug(std::format_string fmt, Args&&... args) { + detail::logFormat(LogLevel::debug, fmt, std::forward(args)...); +} +/// @brief Logs a formatted message at `LogLevel::info`. +template +void logInfo(std::format_string fmt, Args&&... args) { + detail::logFormat(LogLevel::info, fmt, std::forward(args)...); +} +/// @brief Logs a formatted message at `LogLevel::warn`. +template +void logWarn(std::format_string fmt, Args&&... args) { + detail::logFormat(LogLevel::warn, fmt, std::forward(args)...); +} +/// @brief Logs a formatted message at `LogLevel::error`. +template +void logError(std::format_string fmt, Args&&... args) { + detail::logFormat(LogLevel::error, fmt, std::forward(args)...); +} + // ── Scoped override (test fixture) ──────────────────────────────────────────── /// @brief RAII helper that swaps the global logger and level for the lifetime @@ -131,10 +163,10 @@ class ScopedLoggerOverride { /// /// Use this when test code will install its own sink mid-test via /// `setLogger()` / `setLogLevel()` and just wants automatic restoration. - ScopedLoggerOverride() { - std::scoped_lock lock{detail::logState().mtx}; - _savedSink = detail::logState().sink; - _savedLevel = detail::logState().minLevel; + ScopedLoggerOverride() : _savedSink(detail::logState().sink), _savedLevel(detail::logState().minLevel) { + std::scoped_lock const lock{detail::logState().mtx}; + + } /// @brief Installs @p sink and @p level; saves whatever was there before. @@ -142,17 +174,17 @@ class ScopedLoggerOverride { /// @param sink New logger sink. Pass a no-op lambda to suppress output entirely. /// @param level New minimum level. Defaults to `debug` so every message reaches @p sink. explicit ScopedLoggerOverride(std::function sink, - LogLevel level = LogLevel::debug) { - std::scoped_lock lock{detail::logState().mtx}; - _savedSink = std::move(detail::logState().sink); - _savedLevel = detail::logState().minLevel; + LogLevel level = LogLevel::debug) : _savedSink(std::move(detail::logState().sink)), _savedLevel(detail::logState().minLevel) { + std::scoped_lock const lock{detail::logState().mtx}; + + detail::logState().sink = std::move(sink); detail::logState().minLevel = level; } /// @brief Restores the saved sink and level. ~ScopedLoggerOverride() { - std::scoped_lock lock{detail::logState().mtx}; + std::scoped_lock const lock{detail::logState().mtx}; detail::logState().sink = std::move(_savedSink); detail::logState().minLevel = _savedLevel; } diff --git a/include/morph/model.hpp b/include/morph/model.hpp index aa6a2db..343c8b6 100644 --- a/include/morph/model.hpp +++ b/include/morph/model.hpp @@ -23,6 +23,7 @@ struct ModelHolder; /// Implemented automatically by `ModelHolder` when `M` declares /// `void onBackendChanged()`. `Bridge::switchBackend()` discovers this /// capability via `dynamic_cast` without coupling to the concrete type. +// NOLINTBEGIN(cppcoreguidelines-special-member-functions) struct IBackendChangedSink { virtual ~IBackendChangedSink() = default; @@ -30,6 +31,7 @@ struct IBackendChangedSink { /// re-registered all handlers on it. virtual void onBackendChanged() = 0; }; +// NOLINTEND(cppcoreguidelines-special-member-functions) // ── Concept ─────────────────────────────────────────────────────────────────── @@ -57,6 +59,7 @@ struct BackendChangedMixin : IBackendChangedSink { /// @brief Type-erased wrapper that owns a single model instance. /// /// Used internally by backends to store heterogeneous models in a single map. +// NOLINTBEGIN(cppcoreguidelines-special-member-functions) struct IModelHolder { virtual ~IModelHolder() = default; @@ -118,6 +121,7 @@ struct IModelHolder { std::shared_ptr<::morph::journal::IActionLog> _actionLog; std::string _contextKey; }; +// NOLINTEND(cppcoreguidelines-special-member-functions) /// @brief Concrete holder that stores a `Model` by value. /// diff --git a/include/morph/network_monitor.hpp b/include/morph/network_monitor.hpp index a4ac93b..ed90085 100644 --- a/include/morph/network_monitor.hpp +++ b/include/morph/network_monitor.hpp @@ -99,7 +99,7 @@ class NetworkMonitor { /// Thread-safe. void stop() { { - std::scoped_lock lock{_mtx}; + std::scoped_lock const lock{_mtx}; if (_stopped) { return; } diff --git a/include/morph/offline_queue.hpp b/include/morph/offline_queue.hpp index 9a3ea28..a1e6ce9 100644 --- a/include/morph/offline_queue.hpp +++ b/include/morph/offline_queue.hpp @@ -29,6 +29,7 @@ struct QueueItem { /// Items accumulate while the system is offline and are replayed by `SyncWorker` /// on reconnect. The interface is intentionally minimal so that implementations /// can range from in-memory (`InMemoryOfflineQueue`) to SQLite or file-backed stores. +// NOLINTBEGIN(cppcoreguidelines-special-member-functions) struct IOfflineQueue { virtual ~IOfflineQueue() = default; @@ -52,6 +53,7 @@ struct IOfflineQueue { /// @param itemId Id returned by the corresponding `enqueue()` call. virtual void markDone(uint64_t itemId) = 0; }; +// NOLINTEND(cppcoreguidelines-special-member-functions) // ── In-memory implementation ────────────────────────────────────────────────── @@ -65,8 +67,8 @@ class InMemoryOfflineQueue : public IOfflineQueue { /// @param payload Serialised action to store. /// @return Unique id for this item. uint64_t enqueue(std::string payload) override { - std::scoped_lock lock{_mtx}; - uint64_t itemId = ++_nextId; + std::scoped_lock const lock{_mtx}; + uint64_t const itemId = ++_nextId; _items.push_back(QueueItem{.id = itemId, .payload = std::move(payload)}); return itemId; } @@ -74,7 +76,7 @@ class InMemoryOfflineQueue : public IOfflineQueue { /// @brief Returns a snapshot of all pending items. Thread-safe. /// @return Copy of all items in insertion order. std::vector drain() override { - std::scoped_lock lock{_mtx}; + std::scoped_lock const lock{_mtx}; return std::vector{_items.begin(), _items.end()}; } @@ -83,7 +85,7 @@ class InMemoryOfflineQueue : public IOfflineQueue { /// No-op if @p itemId is not found. /// @param itemId Id of the item to remove. void markDone(uint64_t itemId) override { - std::scoped_lock lock{_mtx}; + std::scoped_lock const lock{_mtx}; auto iter = std::ranges::find_if(_items, [itemId](const QueueItem& item) { return item.id == itemId; }); if (iter != _items.end()) { _items.erase(iter); diff --git a/include/morph/rational.hpp b/include/morph/rational.hpp index 5a746dd..2462917 100644 --- a/include/morph/rational.hpp +++ b/include/morph/rational.hpp @@ -122,6 +122,34 @@ struct DecimalPlaces { [[nodiscard]] constexpr auto operator<=>(const DecimalPlaces& other) const noexcept = default; }; +/// @brief Strong type for a Rational numerator. +/// +/// Prevents numerator/denominator argument swapping at construction sites. +struct Numerator { + /// @brief Raw signed integer value. + std::int64_t value{}; + + constexpr Numerator() noexcept = default; + + /// @brief Explicit so the type must be spelled out. + /// @param raw The integer to box as a numerator. + constexpr explicit Numerator(std::int64_t raw) noexcept : value{raw} {} +}; + +/// @brief Strong type for a Rational denominator. +/// +/// Prevents numerator/denominator argument swapping at construction sites. +struct Denominator { + /// @brief Raw signed integer value; must never be zero after canonicalisation. + std::int64_t value{}; + + constexpr Denominator() noexcept = default; + + /// @brief Explicit so the type must be spelled out. + /// @param raw The integer to box as a denominator. + constexpr explicit Denominator(std::int64_t raw) noexcept : value{raw} {} +}; + /// @brief Largest decimal precision the type supports: `10^18` is the /// greatest power of ten that fits in `std::int64_t`. inline constexpr std::uint32_t kMaxDecimalPlaces = 18; @@ -210,7 +238,7 @@ struct Rational { constexpr Rational(std::int64_t whole, DecimalPlaces wantedPrecision) noexcept : numerator{whole}, decimalPlaces{detail::clampDecimalPlaces(wantedPrecision.value)} {} - /// @brief Constructs from explicit numerator/denominator, then canonicalises. + /// @brief Constructs from explicit numerator/denominator, then canonicalises. /// /// A negative @p wantedDenominator flips the sign of @p wantedNumerator, /// the pair is reduced by `gcd`, and a denominator of `0` is clamped to @@ -219,10 +247,10 @@ struct Rational { /// @param wantedNumerator Signed numerator. /// @param wantedDenominator Denominator. May be negative or zero on input. /// @param wantedPrecision Decimal precision; clamped to [1, kMaxDecimalPlaces]. - constexpr Rational(std::int64_t wantedNumerator, std::int64_t wantedDenominator, + constexpr Rational(Numerator wantedNumerator, Denominator wantedDenominator, DecimalPlaces wantedPrecision) noexcept - : numerator{wantedNumerator}, - denominator{wantedDenominator}, + : numerator{wantedNumerator.value}, + denominator{wantedDenominator.value}, decimalPlaces{detail::clampDecimalPlaces(wantedPrecision.value)} { canonicalise(); } @@ -233,8 +261,8 @@ struct Rational { /// @param wantedPrecision Decimal precision; clamped to [1, kMaxDecimalPlaces]. /// @return The canonical rational, or `DivisionByZero` if @p wantedDenominator is 0. [[nodiscard]] static constexpr std::expected from( - std::int64_t wantedNumerator, std::int64_t wantedDenominator, DecimalPlaces wantedPrecision) noexcept { - if (wantedDenominator == 0) { + Numerator wantedNumerator, Denominator wantedDenominator, DecimalPlaces wantedPrecision) noexcept { + if (wantedDenominator.value == 0) { return std::unexpected(RationalError::DivisionByZero); } return Rational{wantedNumerator, wantedDenominator, wantedPrecision}; @@ -266,14 +294,14 @@ struct Rational { /// @param wantedPrecision Decimal precision of the returned value. /// @return `0/1` tagged with @p wantedPrecision. [[nodiscard]] static constexpr Rational zero(DecimalPlaces wantedPrecision) noexcept { - return Rational{0, 1, wantedPrecision}; + return Rational{Numerator{0}, Denominator{1}, wantedPrecision}; } /// @brief Canonical one (`1/1`) at the given precision. /// @param wantedPrecision Decimal precision of the returned value. /// @return `1/1` tagged with @p wantedPrecision. [[nodiscard]] static constexpr Rational one(DecimalPlaces wantedPrecision) noexcept { - return Rational{1, 1, wantedPrecision}; + return Rational{Numerator{1}, Denominator{1}, wantedPrecision}; } /// @brief The value's current decimal precision. @@ -305,7 +333,7 @@ struct Rational { /// @brief Negates. @note Negating a Rational built from `INT64_MIN` overflows. /// @return The value with the numerator's sign flipped. [[nodiscard]] constexpr Rational operator-() const noexcept { - return Rational{-numerator, denominator, decimalPlaces}; + return Rational{Numerator{-numerator}, Denominator{denominator}, decimalPlaces}; } /// @brief Multiplicative inverse. @@ -315,9 +343,9 @@ struct Rational { return std::unexpected(RationalError::DivisionByZero); } if (numerator < 0) { - return Rational{-denominator, -numerator, decimalPlaces}; + return Rational{Numerator{-denominator}, Denominator{-numerator}, decimalPlaces}; } - return Rational{denominator, numerator, decimalPlaces}; + return Rational{Numerator{denominator}, Denominator{numerator}, decimalPlaces}; } /// @brief Three-way comparison. Value-only: ignores `decimalPlaces`. @@ -419,7 +447,7 @@ struct Rational { auto leftCopy = *this; auto const reciprocalNumerator = rhs.numerator > 0 ? rhs.denominator : -rhs.denominator; auto const reciprocalDenominator = rhs.numerator > 0 ? rhs.numerator : -rhs.numerator; - leftCopy *= Rational{reciprocalNumerator, reciprocalDenominator, rhs.decimalPlaces}; + leftCopy *= Rational{Numerator{reciprocalNumerator}, Denominator{reciprocalDenominator}, rhs.decimalPlaces}; return leftCopy; } @@ -437,11 +465,11 @@ struct Rational { /// (`den == 0`, out-of-range `dp`, `INT64_MIN` components whose /// negation would overflow) instead of asserting. /// @param wire Raw values decoded from JSON. - void setWire(Wire wire) noexcept { +void setWire(Wire wire) noexcept { constexpr auto int64Min = std::numeric_limits::min(); constexpr auto negatableMin = -std::numeric_limits::max(); - *this = Rational{wire.num == int64Min ? negatableMin : wire.num, - wire.den == int64Min ? negatableMin : wire.den, + *this = Rational{Numerator{wire.num == int64Min ? negatableMin : wire.num}, + Denominator{wire.den == int64Min ? negatableMin : wire.den}, DecimalPlaces{detail::clampWireDecimalPlaces(wire.dp)}}; } @@ -490,7 +518,8 @@ static_assert(std::is_standard_layout_v); /// @param value Value to take the absolute value of. /// @return The non-negative value with the same magnitude. [[nodiscard]] constexpr Rational abs(const Rational& value) noexcept { - return value.numerator < 0 ? Rational{-value.numerator, value.denominator, value.decimalPlaces} : value; + return value.numerator < 0 ? Rational{Numerator{-value.numerator}, Denominator{value.denominator}, value.decimalPlaces} + : value; } /// @brief Rounds toward positive infinity. @@ -600,12 +629,12 @@ template concept NeedsLifting = IsExpectedRational || IsFloat; /// @brief Lifts a `Rational` to `ExpectedRational` (always succeeds). -[[nodiscard]] constexpr ExpectedRational lift(const Rational& value, DecimalPlaces) noexcept { +[[nodiscard]] constexpr ExpectedRational lift(const Rational& value, DecimalPlaces /*wantedPrecision*/) noexcept { return ExpectedRational{value}; } /// @brief Forwards an `ExpectedRational` unchanged. -[[nodiscard]] constexpr ExpectedRational lift(const ExpectedRational& value, DecimalPlaces) noexcept { +[[nodiscard]] constexpr ExpectedRational lift(const ExpectedRational& value, DecimalPlaces /*wantedPrecision*/) noexcept { return value; } @@ -762,7 +791,7 @@ template } auto const scaledNumerator = static_cast(std::llround(scaled)); - return Rational{scaledNumerator, scale, DecimalPlaces{rawPrecision}}; + return Rational{Numerator{scaledNumerator}, Denominator{scale}, DecimalPlaces{rawPrecision}}; } } // namespace detail diff --git a/include/morph/reconnect_coordinator.hpp b/include/morph/reconnect_coordinator.hpp index 7f4e828..ac020d5 100644 --- a/include/morph/reconnect_coordinator.hpp +++ b/include/morph/reconnect_coordinator.hpp @@ -53,6 +53,7 @@ struct ReconnectCoordinatorConfig { /// mirroring `SyncWorker::run()`. Calling them concurrently is safe; the second /// caller blocks. They are intended to be posted onto a worker executor by the /// host (see the wiring note in the design doc), not called on the probe thread. +// NOLINTNEXTLINE(cppcoreguidelines-special-member-functions) class ReconnectCoordinator { public: /// @brief Alias for the configuration struct. @@ -116,7 +117,7 @@ class ReconnectCoordinator { /// /// @return How the sequence ended (see `ReconnectOutcome`). ReconnectOutcome onOnline() { - std::scoped_lock lock{_mtx}; + std::scoped_lock const lock{_mtx}; for (int attempt = 1; attempt <= _cfg.maxAttempts; ++attempt) { if (!callShouldContinue()) { @@ -150,7 +151,7 @@ class ReconnectCoordinator { /// Idempotent at the policy level: safe to call when already local. Serialised /// against `onOnline()` by an internal mutex. void onOffline() { - std::scoped_lock lock{_mtx}; + std::scoped_lock const lock{_mtx}; _deps.activateLocal(); _deps.bindContext(); } @@ -173,7 +174,7 @@ class ReconnectCoordinator { } /// @brief Calls `tryReconnect`, treating a thrown exception as a failed attempt. - bool callTryReconnect() noexcept { + [[nodiscard]] bool callTryReconnect() const noexcept { try { return _deps.tryReconnect(); } catch (...) { @@ -182,7 +183,7 @@ class ReconnectCoordinator { } /// @brief Calls `shouldContinue`, treating a throw as "do not continue". - bool callShouldContinue() noexcept { + [[nodiscard]] bool callShouldContinue() const noexcept { try { return _deps.shouldContinue(); } catch (...) { diff --git a/include/morph/registry.hpp b/include/morph/registry.hpp index d5e3819..092ceed 100644 --- a/include/morph/registry.hpp +++ b/include/morph/registry.hpp @@ -34,6 +34,21 @@ struct ActionTraits; namespace detail { +/// @brief Hash functor for `std::pair` keys used by registries. +/// Shared between `ActionDispatcher` (server-side dispatch) and `ActionExecuteRegistry` +/// (client-side generic-execute) since their key types are structurally identical. +struct PairKeyHash { + /// @brief Combines the hashes of `key.first` and `key.second`. + /// @param key The pair to hash. + /// @return The combined hash value. + [[nodiscard]] std::size_t operator()(const std::pair& key) const noexcept { + std::size_t seed = std::hash{}(key.first); + // NOLINTNEXTLINE(readability-magic-numbers, cppcoreguidelines-avoid-magic-numbers) + seed ^= std::hash{}(key.second) + 0x9e3779b9 + (seed << 6) + (seed >> 2); + return seed; + } +}; + /// @brief Concept satisfied by actions that expose a `bool validate() const` member. /// /// Drives the default `ActionValidator::ready(...)` path: if the action supplies @@ -172,7 +187,7 @@ class ActionDispatcher { /// the executed action is recorded automatically after it succeeds. template void registerAction(std::string_view modelId, std::string_view actionId) { - Key key{std::string{modelId}, std::string{actionId}}; + Key const key{std::string{modelId}, std::string{actionId}}; _runners[key] = [](IModelHolder& holder, std::string_view payloadJson) { auto action = ActionTraits::fromJson(payloadJson); auto& model = holder.template into(); @@ -201,7 +216,7 @@ class ActionDispatcher { /// @brief Dispatches an action against @p holder and returns the JSON-encoded result. std::string dispatch(std::string_view modelId, std::string_view actionId, IModelHolder& holder, std::string_view payload) { - Key key{std::string{modelId}, std::string{actionId}}; + Key const key{std::string{modelId}, std::string{actionId}}; auto iter = _runners.find(key); if (iter == _runners.end()) { throw std::runtime_error("unknown action: " + key.first + "/" + key.second); @@ -225,15 +240,8 @@ class ActionDispatcher { private: using Key = std::pair; - struct KeyHash { - std::size_t operator()(const Key& key) const noexcept { - std::size_t seed = std::hash{}(key.first); - seed ^= std::hash{}(key.second) + 0x9e3779b9 + (seed << 6) + (seed >> 2); - return seed; - } - }; - std::unordered_map _runners; - std::unordered_map _coalesce; + std::unordered_map _runners; + std::unordered_map _coalesce; }; /// @brief Registry that creates `IModelHolder` instances by string type-id. @@ -353,6 +361,7 @@ bool registerActionExecutorOnce(std::string_view modelId, std::string_view actio /// @param A Concrete action type. /// @param NAME String literal used as the action type-id. /// @param ... Optional: a `morph::model::Loggable` value (defaults to `Loggable::Yes`). +// NOLINTBEGIN(cppcoreguidelines-macro-usage) — registration macros are the intended public API #define BRIDGE_REGISTER_ACTION(...) \ BRIDGE_REGISTER_ACTION_PICK(__VA_ARGS__, BRIDGE_REGISTER_ACTION_4, BRIDGE_REGISTER_ACTION_3) \ (__VA_ARGS__) @@ -417,4 +426,5 @@ bool registerActionExecutorOnce(std::string_view modelId, std::string_view actio struct morph::model::ActionValidator { \ static bool ready(const A& action) { return (FN)(action); } \ }; +// NOLINTEND(cppcoreguidelines-macro-usage) // NOLINTEND(bugprone-macro-parentheses) diff --git a/include/morph/remote.hpp b/include/morph/remote.hpp index 43de923..b4b13e0 100644 --- a/include/morph/remote.hpp +++ b/include/morph/remote.hpp @@ -71,22 +71,22 @@ class RemoteServer : public std::enable_shared_from_this { } } - /// @brief Asynchronously processes @p msg and calls @p reply with the response. + /// @brief Asynchronously processes a JSON `Envelope` and calls @p reply with the response. /// /// The message is dispatched to the worker pool. @p reply is called exactly /// once from the pool thread when processing completes. /// /// Thread-safe. Safe to call before the previous call's reply has been delivered. /// - /// @param msg Pipe-delimited protocol message. - /// @param reply Callback invoked with the response string. + /// @param msg JSON-encoded `morph::wire::Envelope` (via `wire::encode`). + /// @param reply Callback invoked with the JSON-encoded reply envelope. void handle(std::string msg, std::function reply) { auto self = shared_from_this(); _pool.post( [self, msg = std::move(msg), reply = std::move(reply)]() mutable { self->dispatchMessage(msg, reply); }); } - /// @brief Synchronously processes @p msg on the calling thread and returns the reply. + /// @brief Synchronously processes a JSON `Envelope` on the calling thread and returns the reply. /// /// Equivalent to `handle()` but never posts to the worker pool, so it is safe /// to call from a thread that *is* the worker pool — for example, from a @@ -96,8 +96,8 @@ class RemoteServer : public std::enable_shared_from_this { /// messages still post to the strand and would return before the result is /// produced. The implementation rejects `execute` to make this explicit. /// - /// @param msg Pipe-delimited control message. - /// @return Reply string the async path would have produced. + /// @param msg JSON-encoded `morph::wire::Envelope` (via `wire::encode`). + /// @return JSON-encoded reply envelope. std::string handleInline(const std::string& msg) { std::string reply; std::function capture = [&reply](std::string out) { reply = std::move(out); }; @@ -123,7 +123,7 @@ class RemoteServer : public std::enable_shared_from_this { /// provider (new registrations get no log). Thread-safe. /// @param provider Callable invoked synchronously while handling `register`. void setLogProvider(LogProvider provider) { - std::scoped_lock lock{_logProviderMtx}; + std::scoped_lock const lock{_logProviderMtx}; _logProvider = std::move(provider); } @@ -145,7 +145,7 @@ class RemoteServer : public std::enable_shared_from_this { if (!env.contextKey.empty()) { LogProvider provider; { - std::scoped_lock lock{_logProviderMtx}; + std::scoped_lock const lock{_logProviderMtx}; provider = _logProvider; } if (provider) { @@ -154,15 +154,15 @@ class RemoteServer : public std::enable_shared_from_this { } } } - ::morph::exec::detail::ModelId mid{_nextId.fetch_add(1) + 1}; + ::morph::exec::detail::ModelId const mid{_nextId.fetch_add(1) + 1}; { - std::scoped_lock lock{_regMtx}; + std::scoped_lock const lock{_regMtx}; _models[mid] = std::move(holder); } reply(::morph::wire::encode(::morph::wire::makeOk(env.callId, {}, mid.v))); } else if (env.kind == "deregister") { { - std::scoped_lock lock{_regMtx}; + std::scoped_lock const lock{_regMtx}; _models.erase(::morph::exec::detail::ModelId{env.modelId}); } reply(::morph::wire::encode(::morph::wire::makeOk(env.callId))); @@ -182,10 +182,10 @@ class RemoteServer : public std::enable_shared_from_this { reply(::morph::wire::encode(::morph::wire::makeErr("unauthorized", env.callId))); return; } - ::morph::exec::detail::ModelId mid{env.modelId}; + ::morph::exec::detail::ModelId const mid{env.modelId}; std::shared_ptr<::morph::model::detail::IModelHolder> holder; { - std::scoped_lock lock{_regMtx}; + std::scoped_lock const lock{_regMtx}; auto iter = _models.find(mid); if (iter != _models.end()) { holder = iter->second; @@ -198,7 +198,7 @@ class RemoteServer : public std::enable_shared_from_this { _strand.post(mid, [&disp = _dispatcher, env = std::move(env), holder = std::move(holder), reply = std::move(reply)]() mutable { try { - ::morph::session::detail::ScopedContext scoped{env.session}; + ::morph::session::detail::ScopedContext const scoped{env.session}; auto result = disp.dispatch(env.modelType, env.actionType, *holder, env.body); reply(::morph::wire::encode(::morph::wire::makeOk(env.callId, std::move(result)))); } catch (const std::exception& exc) { @@ -246,8 +246,8 @@ class SimulatedRemoteBackend : public detail::IBackend { /// @throws std::runtime_error if the server replies with an error. ::morph::exec::detail::ModelId registerModel( const std::string& typeId, - std::function()>) override { - return registerModelWithContext(typeId, nullptr, {}); + std::function()> /*factory*/) override { + return registerModelWithContext(typeId, {}, {}); } /// @brief Registers the model type on the server, carrying @p contextKey across @@ -261,7 +261,7 @@ class SimulatedRemoteBackend : public detail::IBackend { /// @return `ModelId` assigned by the server. /// @throws std::runtime_error if the server replies with an error. ::morph::exec::detail::ModelId registerModelWithContext( - const std::string& typeId, std::function()>, + const std::string& typeId, std::function()> /*factory*/, std::string_view contextKey) override { auto reply = ::morph::wire::decode(_server.handleInline( ::morph::wire::encode(::morph::wire::makeRegister(typeId, std::string{contextKey})))); @@ -329,7 +329,7 @@ class SimulatedRemoteBackend : public detail::IBackend { void cancelPending(const std::exception_ptr& exc) override { std::vector>>> snapshot; { - std::scoped_lock lock{_pendingMtx}; + std::scoped_lock const lock{_pendingMtx}; snapshot.swap(_pending); } for (auto& weak : snapshot) { @@ -341,7 +341,7 @@ class SimulatedRemoteBackend : public detail::IBackend { private: void trackPending(const std::shared_ptr<::morph::async::detail::CompletionState>>& state) { - std::scoped_lock lock{_pendingMtx}; + std::scoped_lock const lock{_pendingMtx}; std::erase_if(_pending, [](const auto& weak) { return weak.expired(); }); _pending.emplace_back(state); } diff --git a/include/morph/session.hpp b/include/morph/session.hpp index f7b4fa3..26cbc36 100644 --- a/include/morph/session.hpp +++ b/include/morph/session.hpp @@ -42,6 +42,7 @@ struct Context { /// Default implementation supplied by the framework is `AllowAllAuthorizer`. Real /// deployments install a custom subclass that checks principal claims, action /// permissions, rate limits, etc. +// NOLINTBEGIN(cppcoreguidelines-special-member-functions) struct IAuthorizer { virtual ~IAuthorizer() = default; @@ -51,9 +52,12 @@ struct IAuthorizer { /// @param modelType String id of the target model type. /// @param actionType String id of the action being invoked. /// @return `true` to allow dispatch, `false` to reject with `err|unauthorized`. - [[nodiscard]] virtual bool authorize(const Context& ctx, std::string_view modelType, - std::string_view actionType) const = 0; + [[nodiscard]] virtual bool authorize(const Context& ctx, + // NOLINTNEXTLINE(bugprone-easily-swappable-parameters) + std::string_view modelType, + std::string_view actionType) const = 0; }; +// NOLINTEND(cppcoreguidelines-special-member-functions) /// @brief Default authorizer that permits everything. /// @@ -66,6 +70,7 @@ struct AllowAllAuthorizer : IAuthorizer { /// @param actionType Ignored. /// @return Always `true`. [[nodiscard]] bool authorize([[maybe_unused]] const Context& ctx, + // NOLINTNEXTLINE(bugprone-easily-swappable-parameters) [[maybe_unused]] std::string_view modelType, [[maybe_unused]] std::string_view actionType) const override { return true; diff --git a/include/morph/strand.hpp b/include/morph/strand.hpp index e59f18e..37bf879 100644 --- a/include/morph/strand.hpp +++ b/include/morph/strand.hpp @@ -39,6 +39,7 @@ struct ModelIdHash { /// /// This removes the need for per-model mutexes: the model's `execute()` method /// is always called from exactly one task at a time. +// NOLINTNEXTLINE(cppcoreguidelines-special-member-functions) class StrandExecutor { public: /// @brief Constructs the strand executor wrapping @p base. @@ -65,7 +66,7 @@ class StrandExecutor { void post(ModelId key, std::function task) { std::shared_ptr strand; { - std::scoped_lock lock{_mapMtx}; + std::scoped_lock const lock{_mapMtx}; auto& slot = _strands[key]; if (!slot) { slot = std::make_shared(); @@ -75,7 +76,7 @@ class StrandExecutor { } bool schedule = false; { - std::scoped_lock lock{strand->mtx}; + std::scoped_lock const lock{strand->mtx}; strand->pending.push(std::move(task)); if (!strand->running) { strand->running = true; @@ -97,18 +98,19 @@ class StrandExecutor { void scheduleNext(const std::shared_ptr& strand, ModelId key) { { - std::scoped_lock lock{_mapMtx}; + std::scoped_lock const lock{_mapMtx}; ++_inFlight; } strand->base->post([this, strand, key] { std::function task; { - std::scoped_lock lock{strand->mtx}; + std::scoped_lock const lock{strand->mtx}; task = std::move(strand->pending.front()); strand->pending.pop(); } try { task(); + // NOLINTNEXTLINE(bugprone-empty-catch) — task exceptions are intentionally discarded } catch (...) { } // Decide "keep running vs. drain-and-erase" atomically across the @@ -121,7 +123,7 @@ class StrandExecutor { // strands could run model tasks concurrently → data race. bool more = false; { - std::scoped_lock lock{_mapMtx, strand->mtx}; + std::scoped_lock const lock{_mapMtx, strand->mtx}; more = !strand->pending.empty(); if (!more) { strand->running = false; @@ -136,7 +138,7 @@ class StrandExecutor { } // Decrement after all map access is done; wake destructor if it is waiting. { - std::scoped_lock lock{_mapMtx}; + std::scoped_lock const lock{_mapMtx}; if (--_inFlight == 0) { _cv.notify_all(); } diff --git a/include/morph/sync_worker.hpp b/include/morph/sync_worker.hpp index 5ecc7b2..bec30a7 100644 --- a/include/morph/sync_worker.hpp +++ b/include/morph/sync_worker.hpp @@ -72,8 +72,8 @@ class SyncWorker { /// /// @return Counts of successful / failed / dead-lettered replays. SyncResult run() { - std::scoped_lock runLock{_runMtx}; - bool wasStoppedBeforeRun = _stopped.exchange(false); + std::scoped_lock const runLock{_runMtx}; + bool const wasStoppedBeforeRun = _stopped.exchange(false); SyncResult result; if (wasStoppedBeforeRun) { return result; diff --git a/include/morph/wire.hpp b/include/morph/wire.hpp index 82195b6..649d8bf 100644 --- a/include/morph/wire.hpp +++ b/include/morph/wire.hpp @@ -118,7 +118,7 @@ inline std::string encode(const Envelope& env) { inline Envelope decode(std::string_view json) { Envelope env{}; if (auto errCode = glz::read_json(env, json)) { - throw std::runtime_error("envelope decode failed: " + glz::format_error(errCode, std::string{json})); + throw std::runtime_error("envelope decode failed: " + glz::format_error(errCode, json)); } return env; } diff --git a/src/main.cpp b/src/main.cpp index 1050f9a..f60e4ed 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -7,6 +7,7 @@ #include #include +// NOLINTBEGIN(misc-use-internal-linkage) struct IncrementAction { int by = 0; }; @@ -21,6 +22,7 @@ struct CounterModel { } int execute(FailingAction) { throw std::runtime_error("intentional failure from FailingAction"); } }; +// NOLINTEND(misc-use-internal-linkage) BRIDGE_REGISTER_MODEL(CounterModel, "CounterModel") BRIDGE_REGISTER_ACTION(CounterModel, IncrementAction, "IncrementAction") diff --git a/tests/test_coverage_push95.cpp b/tests/test_coverage_push95.cpp index 6a13c08..563a0ab 100644 --- a/tests/test_coverage_push95.cpp +++ b/tests/test_coverage_push95.cpp @@ -76,7 +76,7 @@ struct ControllableBackend : ::morph::backend::detail::IBackend { } void notifyBackendChanged() override {} void cancelPending(const std::exception_ptr& /*exc*/) override {} - void setReconnectHandler(std::function handler) override { reconnect = std::move(handler); } + void setReconnectHandler(const std::function& handler) override { reconnect = std::move(handler); } }; // A denying authorizer for the RemoteServer unauthorized path. diff --git a/tests/test_quantity_forms.cpp b/tests/test_quantity_forms.cpp index 4cad37f..cf36018 100644 --- a/tests/test_quantity_forms.cpp +++ b/tests/test_quantity_forms.cpp @@ -92,11 +92,11 @@ constexpr DecimalPlaces dp1{1}; constexpr DecimalPlaces dp3{3}; [[nodiscard]] Q kilograms(std::int64_t num, std::int64_t den = 1) { - return {Rational{num, den, dp3}}; + return {Rational{Numerator{num}, Denominator{den}, dp3}}; } [[nodiscard]] Q cubicMetres(std::int64_t num, std::int64_t den = 1) { - return {Rational{num, den, dp3}}; + return {Rational{Numerator{num}, Denominator{den}, dp3}}; } // Compile-time facts: unit algebra deduction and the quantity trait. @@ -191,12 +191,12 @@ BRIDGE_REGISTER_ACTION(QFLabModel, QFSchedule, "QFSchedule") // --------------------------------------------------------------------------- TEST_CASE("Quantity::Arithmetic::ExactWithPrecisionPropagation", "[quantity]") { - auto const mass = Q{Rational{26505, 10, dp1}}; + auto const mass = Q{Rational{Numerator{26505}, Denominator{10}, dp1}}; auto const volume = cubicMetres(1); auto const density = mass / volume; REQUIRE(density.hasValue()); - CHECK(*density == Rational{5301, 2, dp1}); + CHECK(*density == Rational{Numerator{5301}, Denominator{2}, dp1}); CHECK((*density).getDecimalPlaces() == dp3); // max(1, 3) propagates auto const massBack = density * volume; @@ -212,7 +212,7 @@ TEST_CASE("Quantity::Arithmetic::ExactWithPrecisionPropagation", "[quantity]") { static_assert(std::same_as>); CHECK(*scaled == Rational{6, dp3}); CHECK(*(Rational{2, dp1} * kilograms(3)) == Rational{6, dp3}); - CHECK(*(kilograms(3) / Rational{2, dp1}) == Rational{3, 2, dp3}); + CHECK(*(kilograms(3) / Rational{2, dp1}) == Rational{Numerator{3}, Denominator{2}, dp3}); } TEST_CASE("Quantity::Arithmetic::EmptyPropagates", "[quantity]") { @@ -229,7 +229,7 @@ TEST_CASE("Quantity::Arithmetic::EmptyPropagates", "[quantity]") { // Cross-unit product propagates emptiness from either side. CHECK_FALSE((Q{} * cubicMetres(2)).hasValue()); - CHECK_FALSE((Q{Rational{5, 2, dp1}} * Q{}).hasValue()); + CHECK_FALSE((Q{Rational{Numerator{5}, Denominator{2}, dp1}} * Q{}).hasValue()); } TEST_CASE("Quantity::Arithmetic::DivisionByZeroYieldsEmpty", "[quantity]") { @@ -244,7 +244,7 @@ TEST_CASE("Quantity::Comparison", "[quantity]") { CHECK(Q{} == Q{}); // Declared precisions are transparent to comparison and conversion. - PreciseMass const precise{Rational{1, 2, dp3}}; + PreciseMass const precise{Rational{Numerator{1}, Denominator{2}, dp3}}; CHECK(precise == kilograms(1, 2)); CHECK(precise < kilograms(1)); Q const widened = precise; // same unit, different declared precision @@ -255,7 +255,7 @@ TEST_CASE("Quantity::DeclaredPrecision", "[quantity]") { // fromDouble converts at the field's declared precision. auto const coarse = Q::fromDouble(2.5); REQUIRE(coarse.hasValue()); - CHECK(*coarse == Rational{5, 2, dp3}); + CHECK(*coarse == Rational{Numerator{5}, Denominator{2}, dp3}); CHECK((*coarse).getDecimalPlaces() == dp3); auto const fine = PreciseMass::fromDouble(2.5); @@ -291,7 +291,7 @@ TEST_CASE("Quantity::DeclaredPrecision", "[quantity]") { TEST_CASE("Quantity::Wire::UnitNeverTravels", "[quantity][forms]") { QFRecordMeasurement action; action.sampleId = 9; - action.density = Rational{23, 10, dp1}; + action.density = Rational{Numerator{23}, Denominator{10}, dp1}; auto const json = glz::write_json(action); REQUIRE(json.has_value()); @@ -312,7 +312,7 @@ TEST_CASE("Quantity::Wire::PartialPatchEngagesDraft", "[quantity][forms]") { REQUIRE_FALSE(glz::read_json(draft, R"({"density":{"num":5,"den":2,"dp":1}})")); CHECK(draft.sampleId == 42); // untouched by the partial patch REQUIRE(draft.density.hasValue()); - CHECK(*draft.density == Rational{5, 2, dp1}); + CHECK(*draft.density == Rational{Numerator{5}, Denominator{2}, dp1}); // Explicit null clears the field again. REQUIRE_FALSE(glz::read_json(draft, R"({"density":null})")); @@ -330,7 +330,7 @@ TEST_CASE("Forms::AllRequiredEngaged", "[forms]") { QFRecordMeasurement densityOnly; densityOnly.sampleId = 1; - densityOnly.density = Rational{23, 10, dp1}; + densityOnly.density = Rational{Numerator{23}, Denominator{10}, dp1}; CHECK(morph::forms::allRequiredEngaged(densityOnly)); // moisture is opt-out // The validator machinery picks up validate() (which delegates here). @@ -413,7 +413,7 @@ TEST_CASE("Forms::MergeSchemaExtras::MalformedInputPassesThrough", "[forms]") { TEST_CASE("Forms::DispatchQuantityActionThroughRegistry", "[forms][quantity]") { using morph::model::ActionTraits; - QFComputeDryDensity action{.massDry = Q{Rational{26505, 10, dp1}}, .volume = cubicMetres(1)}; + QFComputeDryDensity action{.massDry = Q{Rational{Numerator{26505}, Denominator{10}, dp1}}, .volume = cubicMetres(1)}; auto const payload = ActionTraits::toJson(action); auto holder = morph::model::detail::ModelFactory::create(); @@ -422,7 +422,7 @@ TEST_CASE("Forms::DispatchQuantityActionThroughRegistry", "[forms][quantity]") { auto const result = ActionTraits::resultFromJson(resultJson); REQUIRE(result.hasValue()); - CHECK(*result == Rational{5301, 2, dp1}); + CHECK(*result == Rational{Numerator{5301}, Denominator{2}, dp1}); } // --------------------------------------------------------------------------- diff --git a/tests/test_rational.cpp b/tests/test_rational.cpp index 6ff6dda..190b3a4 100644 --- a/tests/test_rational.cpp +++ b/tests/test_rational.cpp @@ -72,14 +72,14 @@ static_assert(Rational {}.numerator == 0); static_assert(Rational {}.denominator == 1); static_assert(Rational { 7, dp9 }.numerator == 7); static_assert(Rational { 7, dp9 }.denominator == 1); -static_assert(Rational { 2, 4, dp9 }.numerator == 1); -static_assert(Rational { 2, 4, dp9 }.denominator == 2); -static_assert(Rational { -1, -2, dp9 }.numerator == 1); -static_assert(Rational { -1, -2, dp9 }.denominator == 2); -static_assert(Rational { 3, -4, dp9 }.numerator == -3); -static_assert(Rational { 3, -4, dp9 }.denominator == 4); -static_assert(Rational { 5, 0, dp9 }.numerator == 5); -static_assert(Rational { 5, 0, dp9 }.denominator == 1); +static_assert(Rational { Numerator{2}, Denominator{4}, dp9 }.numerator == 1); +static_assert(Rational { Numerator{2}, Denominator{4}, dp9 }.denominator == 2); +static_assert(Rational { Numerator{-1}, Denominator{-2}, dp9 }.numerator == 1); +static_assert(Rational { Numerator{-1}, Denominator{-2}, dp9 }.denominator == 2); +static_assert(Rational { Numerator{3}, Denominator{-4}, dp9 }.numerator == -3); +static_assert(Rational { Numerator{3}, Denominator{-4}, dp9 }.denominator == 4); +static_assert(Rational { Numerator{5}, Denominator{0}, dp9 }.numerator == 5); +static_assert(Rational { Numerator{5}, Denominator{0}, dp9 }.denominator == 1); // Precision is carried and queryable at compile time. static_assert((Rational { 7, dp9 }.getDecimalPlaces()).value == 9); @@ -92,47 +92,47 @@ static_assert(Rational::one(dp9).numerator == 1); static_assert(Rational::one(dp9).denominator == 1); // From factory. -static_assert(Rational::from(1, 2, dp9).has_value()); -static_assert(!Rational::from(1, 0, dp9).has_value()); -static_assert(Rational::from(1, 0, dp9).error() == RationalError::DivisionByZero); +static_assert(Rational::from(Numerator{1}, Denominator{2}, dp9).has_value()); +static_assert(!Rational::from(Numerator{1}, Denominator{0}, dp9).has_value()); +static_assert(Rational::from(Numerator{1}, Denominator{0}, dp9).error() == RationalError::DivisionByZero); // Arithmetic. -static_assert(Rational { 1, 3, dp9 } + Rational { 1, 3, dp9 } + Rational { 1, 3, dp9 } +static_assert(Rational { Numerator{1}, Denominator{3}, dp9 } + Rational { Numerator{1}, Denominator{3}, dp9 } + Rational { Numerator{1}, Denominator{3}, dp9 } == Rational::one(dp9)); -static_assert(Rational { 1, 10, dp9 } + Rational { 2, 10, dp9 } == Rational { 3, 10, dp9 }); -static_assert(Rational { 5, 6, dp9 } - Rational { 1, 6, dp9 } == Rational { 2, 3, dp9 }); -static_assert(Rational { 2, 3, dp9 } * Rational { 3, 5, dp9 } == Rational { 2, 5, dp9 }); -static_assert((Rational { 1, 7, dp9 } * Rational { 7, 1, dp9 }) == Rational::one(dp9)); +static_assert(Rational { Numerator{1}, Denominator{10}, dp9 } + Rational { Numerator{2}, Denominator{10}, dp9 } == Rational { Numerator{3}, Denominator{10}, dp9 }); +static_assert(Rational { Numerator{5}, Denominator{6}, dp9 } - Rational { Numerator{1}, Denominator{6}, dp9 } == Rational { Numerator{2}, Denominator{3}, dp9 }); +static_assert(Rational { Numerator{2}, Denominator{3}, dp9 } * Rational { Numerator{3}, Denominator{5}, dp9 } == Rational { Numerator{2}, Denominator{5}, dp9 }); +static_assert((Rational { Numerator{1}, Denominator{7}, dp9 } * Rational { Numerator{7}, Denominator{1}, dp9 }) == Rational::one(dp9)); // Division returns expected. -static_assert((Rational { 1, 2, dp9 } / Rational { 1, 4, dp9 }).has_value()); -static_assert(*(Rational { 1, 2, dp9 } / Rational { 1, 4, dp9 }) == Rational { 2, dp9 }); -static_assert(!(Rational { 1, 2, dp9 } / Rational::zero(dp9)).has_value()); +static_assert((Rational { Numerator{1}, Denominator{2}, dp9 } / Rational { Numerator{1}, Denominator{4}, dp9 }).has_value()); +static_assert(*(Rational { Numerator{1}, Denominator{2}, dp9 } / Rational { Numerator{1}, Denominator{4}, dp9 }) == Rational { 2, dp9 }); +static_assert(!(Rational { Numerator{1}, Denominator{2}, dp9 } / Rational::zero(dp9)).has_value()); // reciprocal. -static_assert(Rational { 3, 4, dp9 }.reciprocal().has_value()); -static_assert(*Rational { 3, 4, dp9 }.reciprocal() == Rational { 4, 3, dp9 }); -static_assert(*Rational { -3, 4, dp9 }.reciprocal() == Rational { -4, 3, dp9 }); +static_assert(Rational { Numerator{3}, Denominator{4}, dp9 }.reciprocal().has_value()); +static_assert(*Rational { Numerator{3}, Denominator{4}, dp9 }.reciprocal() == Rational { Numerator{4}, Denominator{3}, dp9 }); +static_assert(*Rational { Numerator{-3}, Denominator{4}, dp9 }.reciprocal() == Rational { Numerator{-4}, Denominator{3}, dp9 }); static_assert(!Rational::zero(dp9).reciprocal().has_value()); // Comparison (value-only). -static_assert(Rational { 1, 2, dp9 } < Rational { 2, 3, dp9 }); -static_assert(Rational { -1, 2, dp9 } < Rational { 0, dp9 }); -static_assert(Rational { 2, 4, dp9 } == Rational { 1, 2, dp9 }); +static_assert(Rational { Numerator{1}, Denominator{2}, dp9 } < Rational { Numerator{2}, Denominator{3}, dp9 }); +static_assert(Rational { Numerator{-1}, Denominator{2}, dp9 } < Rational { 0, dp9 }); +static_assert(Rational { Numerator{2}, Denominator{4}, dp9 } == Rational { Numerator{1}, Denominator{2}, dp9 }); // Comparison ignores precision: same value, different precision -> equal. -static_assert(Rational { 1, 2, dp3 } == Rational { 1, 2, dp9 }); +static_assert(Rational { Numerator{1}, Denominator{2}, dp3 } == Rational { Numerator{1}, Denominator{2}, dp9 }); // Conversion helpers (free functions; ADL through Rational). -static_assert(trunc(Rational { 7, 2, dp9 }) == 3); -static_assert(trunc(Rational { -7, 2, dp9 }) == -3); -static_assert(floor(Rational { 7, 2, dp9 }) == 3); -static_assert(floor(Rational { -7, 2, dp9 }) == -4); -static_assert(ceil(Rational { 7, 2, dp9 }) == 4); -static_assert(ceil(Rational { -7, 2, dp9 }) == -3); +static_assert(trunc(Rational { Numerator{7}, Denominator{2}, dp9 }) == 3); +static_assert(trunc(Rational { Numerator{-7}, Denominator{2}, dp9 }) == -3); +static_assert(floor(Rational { Numerator{7}, Denominator{2}, dp9 }) == 3); +static_assert(floor(Rational { Numerator{-7}, Denominator{2}, dp9 }) == -4); +static_assert(ceil(Rational { Numerator{7}, Denominator{2}, dp9 }) == 4); +static_assert(ceil(Rational { Numerator{-7}, Denominator{2}, dp9 }) == -3); // Abs / unary minus. -static_assert((-Rational { 3, 4, dp9 }) == Rational { -3, 4, dp9 }); -static_assert(abs(Rational { -3, 4, dp9 }) == Rational { 3, 4, dp9 }); +static_assert((-Rational { Numerator{3}, Denominator{4}, dp9 }) == Rational { Numerator{-3}, Denominator{4}, dp9 }); +static_assert(abs(Rational { Numerator{-3}, Denominator{4}, dp9 }) == Rational { Numerator{3}, Denominator{4}, dp9 }); // Plain arithmetic result type is Rational; division wraps in expected. static_assert(std::same_as); @@ -177,25 +177,25 @@ TEST_CASE("Rational::Construction", "[rational]") CHECK(Rational { 7, dp9 }.numerator == 7); CHECK(Rational { 7, dp9 }.denominator == 1); - CHECK(Rational { 2, 4, dp9 }.numerator == 1); - CHECK(Rational { 2, 4, dp9 }.denominator == 2); + CHECK(Rational { Numerator{2}, Denominator{4}, dp9 }.numerator == 1); + CHECK(Rational { Numerator{2}, Denominator{4}, dp9 }.denominator == 2); - CHECK(Rational { -1, -2, dp9 }.numerator == 1); - CHECK(Rational { -1, -2, dp9 }.denominator == 2); - CHECK(Rational { 3, -4, dp9 }.numerator == -3); - CHECK(Rational { 3, -4, dp9 }.denominator == 4); + CHECK(Rational { Numerator{-1}, Denominator{-2}, dp9 }.numerator == 1); + CHECK(Rational { Numerator{-1}, Denominator{-2}, dp9 }.denominator == 2); + CHECK(Rational { Numerator{3}, Denominator{-4}, dp9 }.numerator == -3); + CHECK(Rational { Numerator{3}, Denominator{-4}, dp9 }.denominator == 4); - CHECK(Rational { 5, 0, dp9 }.numerator == 5); - CHECK(Rational { 5, 0, dp9 }.denominator == 1); + CHECK(Rational { Numerator{5}, Denominator{0}, dp9 }.numerator == 5); + CHECK(Rational { Numerator{5}, Denominator{0}, dp9 }.denominator == 1); } TEST_CASE("Rational::PrecisionQuery", "[rational]") { // getDecimalPlaces is public API and returns the constructed precision. - CHECK(Rational { 1, 2, dp9 }.getDecimalPlaces() == dp9); - CHECK(Rational { 1, 2, dp3 }.getDecimalPlaces() == dp3); + CHECK(Rational { Numerator{1}, Denominator{2}, dp9 }.getDecimalPlaces() == dp9); + CHECK(Rational { Numerator{1}, Denominator{2}, dp3 }.getDecimalPlaces() == dp3); CHECK(Rational { 7, dp6 }.getDecimalPlaces() == dp6); - CHECK((Rational { 1, 2, dp9 }.getDecimalPlaces()).value == 9); + CHECK((Rational { Numerator{1}, Denominator{2}, dp9 }.getDecimalPlaces()).value == 9); } TEST_CASE("Rational::PrecisionCap", "[rational]") @@ -229,16 +229,16 @@ TEST_CASE("Rational::DenominatorAlwaysPositive", "[rational]") { auto const samples = std::array { Rational { 0, dp9 }, Rational { 7, dp9 }, Rational { -7, dp9 }, - Rational { 2, 4, dp9 }, Rational { -2, 4, dp9 }, Rational { 2, -4, dp9 }, - Rational { -2, -4, dp9 }, Rational { 5, 0, dp9 }, Rational::zero(dp9), + Rational { Numerator{2}, Denominator{4}, dp9 }, Rational { Numerator{-2}, Denominator{4}, dp9 }, Rational { Numerator{2}, Denominator{-4}, dp9 }, + Rational { Numerator{-2}, Denominator{-4}, dp9 }, Rational { Numerator{5}, Denominator{0}, dp9 }, Rational::zero(dp9), Rational::one(dp9), }; for (auto const& value : samples) { CHECK(value.denominator > 0); } - auto const left = Rational { 3, 5, dp9 }; - auto const right = Rational { -7, 11, dp9 }; + auto const left = Rational { Numerator{3}, Denominator{5}, dp9 }; + auto const right = Rational { Numerator{-7}, Denominator{11}, dp9 }; CHECK((left + right).denominator > 0); CHECK((left - right).denominator > 0); CHECK((left * right).denominator > 0); @@ -260,11 +260,11 @@ TEST_CASE("Rational::DenominatorAlwaysPositive", "[rational]") TEST_CASE("Rational::FromExpected", "[rational]") { - auto const valid = Rational::from(1, 2, dp9); + auto const valid = Rational::from(Numerator{1}, Denominator{2}, dp9); REQUIRE(valid.has_value()); - CHECK(*valid == Rational { 1, 2, dp9 }); + CHECK(*valid == Rational { Numerator{1}, Denominator{2}, dp9 }); - auto const invalid = Rational::from(1, 0, dp9); + auto const invalid = Rational::from(Numerator{1}, Denominator{0}, dp9); REQUIRE_FALSE(invalid.has_value()); CHECK(invalid.error() == RationalError::DivisionByZero); } @@ -288,22 +288,22 @@ TEST_CASE("Rational::fromFloat::Roundtrip", "[rational]") { auto const value = Rational::fromFloat(0.5, dp1); REQUIRE(value.has_value()); - CHECK(*value == Rational { 1, 2, dp1 }); + CHECK(*value == Rational { Numerator{1}, Denominator{2}, dp1 }); } { auto const value = Rational::fromFloat(0.25, dp2); REQUIRE(value.has_value()); - CHECK(*value == Rational { 1, 4, dp2 }); + CHECK(*value == Rational { Numerator{1}, Denominator{4}, dp2 }); } { auto const value = Rational::fromFloat(0.1, dp1); REQUIRE(value.has_value()); - CHECK(*value == Rational { 1, 10, dp1 }); + CHECK(*value == Rational { Numerator{1}, Denominator{10}, dp1 }); } { auto const value = Rational::fromFloat(-1.75, dp2); REQUIRE(value.has_value()); - CHECK(*value == Rational { -7, 4, dp2 }); + CHECK(*value == Rational { Numerator{-7}, Denominator{4}, dp2 }); } { auto const value = Rational::fromFloat(0.0, dp6); @@ -313,12 +313,12 @@ TEST_CASE("Rational::fromFloat::Roundtrip", "[rational]") { auto const value = Rational::fromFloat(0.5F, dp1); REQUIRE(value.has_value()); - CHECK(*value == Rational { 1, 2, dp1 }); + CHECK(*value == Rational { Numerator{1}, Denominator{2}, dp1 }); } { auto const value = Rational::fromFloat(0.5L, dp1); REQUIRE(value.has_value()); - CHECK(*value == Rational { 1, 2, dp1 }); + CHECK(*value == Rational { Numerator{1}, Denominator{2}, dp1 }); } auto const negative = Rational::fromFloat(-1.75, dp2); @@ -330,15 +330,15 @@ TEST_CASE("Rational::fromFloat::Precision", "[rational]") { auto const oneThirdAt3 = Rational::fromFloat(1.0 / 3.0, dp3); REQUIRE(oneThirdAt3.has_value()); - CHECK(*oneThirdAt3 == Rational { 333, 1000, dp3 }); + CHECK(*oneThirdAt3 == Rational { Numerator{333}, Denominator{1000}, dp3 }); auto const oneThirdAt6 = Rational::fromFloat(1.0 / 3.0, dp6); REQUIRE(oneThirdAt6.has_value()); - CHECK(*oneThirdAt6 == Rational { 333333, 1000000, dp6 }); + CHECK(*oneThirdAt6 == Rational { Numerator{333333}, Denominator{1000000}, dp6 }); auto const half = Rational::fromFloat(0.5, dp6); REQUIRE(half.has_value()); - CHECK(*half == Rational { 1, 2, dp6 }); + CHECK(*half == Rational { Numerator{1}, Denominator{2}, dp6 }); } TEST_CASE("Rational::fromFloat::ErrorPaths", "[rational]") @@ -386,46 +386,46 @@ TEST_CASE("Rational::fromFloat::ErrorPaths", "[rational]") TEST_CASE("Rational::ExactArithmetic", "[rational]") { - CHECK(Rational { 1, 3, dp9 } + Rational { 1, 3, dp9 } + Rational { 1, 3, dp9 } + CHECK(Rational { Numerator{1}, Denominator{3}, dp9 } + Rational { Numerator{1}, Denominator{3}, dp9 } + Rational { Numerator{1}, Denominator{3}, dp9 } == Rational::one(dp9)); - CHECK(Rational { 1, 10, dp9 } + Rational { 2, 10, dp9 } == Rational { 3, 10, dp9 }); - CHECK(Rational { 1, 7, dp9 } * Rational { 7, dp9 } == Rational::one(dp9)); - CHECK(Rational { 5, 6, dp9 } - Rational { 1, 6, dp9 } == Rational { 2, 3, dp9 }); + CHECK(Rational { Numerator{1}, Denominator{10}, dp9 } + Rational { Numerator{2}, Denominator{10}, dp9 } == Rational { Numerator{3}, Denominator{10}, dp9 }); + CHECK(Rational { Numerator{1}, Denominator{7}, dp9 } * Rational { 7, dp9 } == Rational::one(dp9)); + CHECK(Rational { Numerator{5}, Denominator{6}, dp9 } - Rational { Numerator{1}, Denominator{6}, dp9 } == Rational { Numerator{2}, Denominator{3}, dp9 }); // Negative left operand exercises the sign handling in the cross-cancel. - CHECK(Rational { -2, 3, dp9 } * Rational { 3, 2, dp9 } == -Rational::one(dp9)); + CHECK(Rational { Numerator{-2}, Denominator{3}, dp9 } * Rational { Numerator{3}, Denominator{2}, dp9 } == -Rational::one(dp9)); - auto const tenth = Rational { 1, 10, dp9 }; - auto const fifth = Rational { 1, 5, dp9 }; - CHECK(tenth + fifth == Rational { 3, 10, dp9 }); + auto const tenth = Rational { Numerator{1}, Denominator{10}, dp9 }; + auto const fifth = Rational { Numerator{1}, Denominator{5}, dp9 }; + CHECK(tenth + fifth == Rational { Numerator{3}, Denominator{10}, dp9 }); } TEST_CASE("Rational::Comparison", "[rational]") { - CHECK(Rational { 2, 4, dp9 } == Rational { 1, 2, dp9 }); - CHECK(Rational { 1, 2, dp9 } != Rational { 1, 3, dp9 }); - CHECK(Rational { 1, 3, dp9 } < Rational { 1, 2, dp9 }); - CHECK(Rational { -1, 2, dp9 } < Rational { 1, 100, dp9 }); - CHECK(Rational { 1, 2, dp9 } > Rational { 1, 3, dp9 }); + CHECK(Rational { Numerator{2}, Denominator{4}, dp9 } == Rational { Numerator{1}, Denominator{2}, dp9 }); + CHECK(Rational { Numerator{1}, Denominator{2}, dp9 } != Rational { Numerator{1}, Denominator{3}, dp9 }); + CHECK(Rational { Numerator{1}, Denominator{3}, dp9 } < Rational { Numerator{1}, Denominator{2}, dp9 }); + CHECK(Rational { Numerator{-1}, Denominator{2}, dp9 } < Rational { Numerator{1}, Denominator{100}, dp9 }); + CHECK(Rational { Numerator{1}, Denominator{2}, dp9 } > Rational { Numerator{1}, Denominator{3}, dp9 }); CHECK(Rational { 0, dp9 } == Rational::zero(dp9)); - CHECK((Rational { 2, 4, dp9 } <=> Rational { 1, 2, dp9 }) == std::strong_ordering::equal); - CHECK((Rational { 1, 3, dp9 } <=> Rational { 1, 2, dp9 }) == std::strong_ordering::less); + CHECK((Rational { Numerator{2}, Denominator{4}, dp9 } <=> Rational { Numerator{1}, Denominator{2}, dp9 }) == std::strong_ordering::equal); + CHECK((Rational { Numerator{1}, Denominator{3}, dp9 } <=> Rational { Numerator{1}, Denominator{2}, dp9 }) == std::strong_ordering::less); // Precision-independent: same value at different precision compares equal. - CHECK(Rational { 1, 2, dp3 } == Rational { 1, 2, dp9 }); - CHECK((Rational { 1, 2, dp3 } <=> Rational { 1, 2, dp9 }) == std::strong_ordering::equal); + CHECK(Rational { Numerator{1}, Denominator{2}, dp3 } == Rational { Numerator{1}, Denominator{2}, dp9 }); + CHECK((Rational { Numerator{1}, Denominator{2}, dp3 } <=> Rational { Numerator{1}, Denominator{2}, dp9 }) == std::strong_ordering::equal); } TEST_CASE("Rational::DivisionByZero", "[rational]") { - auto const result = Rational { 1, 2, dp9 } / Rational::zero(dp9); + auto const result = Rational { Numerator{1}, Denominator{2}, dp9 } / Rational::zero(dp9); REQUIRE_FALSE(result.has_value()); CHECK(result.error() == RationalError::DivisionByZero); - auto const named = Rational { 1, 2, dp9 }.dividedBy(Rational::zero(dp9)); + auto const named = Rational { Numerator{1}, Denominator{2}, dp9 }.dividedBy(Rational::zero(dp9)); REQUIRE_FALSE(named.has_value()); CHECK(named.error() == RationalError::DivisionByZero); - auto const chained = Rational::from(1, 2, dp9).and_then( + auto const chained = Rational::from(Numerator{1}, Denominator{2}, dp9).and_then( [](Rational value) { return value.dividedBy(Rational::zero(dp9)); }); REQUIRE_FALSE(chained.has_value()); CHECK(chained.error() == RationalError::DivisionByZero); @@ -433,13 +433,13 @@ TEST_CASE("Rational::DivisionByZero", "[rational]") TEST_CASE("Rational::reciprocal", "[rational]") { - auto const reciprocal = Rational { 3, 4, dp9 }.reciprocal(); + auto const reciprocal = Rational { Numerator{3}, Denominator{4}, dp9 }.reciprocal(); REQUIRE(reciprocal.has_value()); - CHECK(*reciprocal == Rational { 4, 3, dp9 }); + CHECK(*reciprocal == Rational { Numerator{4}, Denominator{3}, dp9 }); - auto const negativeReciprocal = Rational { -3, 4, dp9 }.reciprocal(); + auto const negativeReciprocal = Rational { Numerator{-3}, Denominator{4}, dp9 }.reciprocal(); REQUIRE(negativeReciprocal.has_value()); - CHECK(*negativeReciprocal == Rational { -4, 3, dp9 }); + CHECK(*negativeReciprocal == Rational { Numerator{-4}, Denominator{3}, dp9 }); CHECK(negativeReciprocal->denominator > 0); auto const zeroReciprocal = Rational::zero(dp9).reciprocal(); @@ -449,20 +449,20 @@ TEST_CASE("Rational::reciprocal", "[rational]") TEST_CASE("Rational::Conversions", "[rational]") { - CHECK(Rational { 1, 2, dp9 }.toDouble() == Catch::Approx(0.5)); - CHECK(Rational { -3, 4, dp9 }.toDouble() == Catch::Approx(-0.75)); + CHECK(Rational { Numerator{1}, Denominator{2}, dp9 }.toDouble() == Catch::Approx(0.5)); + CHECK(Rational { Numerator{-3}, Denominator{4}, dp9 }.toDouble() == Catch::Approx(-0.75)); - CHECK(trunc(Rational { 7, 2, dp9 }) == 3); - CHECK(trunc(Rational { -7, 2, dp9 }) == -3); - CHECK(trunc(Rational { 6, 2, dp9 }) == 3); + CHECK(trunc(Rational { Numerator{7}, Denominator{2}, dp9 }) == 3); + CHECK(trunc(Rational { Numerator{-7}, Denominator{2}, dp9 }) == -3); + CHECK(trunc(Rational { Numerator{6}, Denominator{2}, dp9 }) == 3); - CHECK(floor(Rational { 7, 2, dp9 }) == 3); - CHECK(floor(Rational { -7, 2, dp9 }) == -4); - CHECK(floor(Rational { 6, 2, dp9 }) == 3); + CHECK(floor(Rational { Numerator{7}, Denominator{2}, dp9 }) == 3); + CHECK(floor(Rational { Numerator{-7}, Denominator{2}, dp9 }) == -4); + CHECK(floor(Rational { Numerator{6}, Denominator{2}, dp9 }) == 3); - CHECK(ceil(Rational { 7, 2, dp9 }) == 4); - CHECK(ceil(Rational { -7, 2, dp9 }) == -3); - CHECK(ceil(Rational { 6, 2, dp9 }) == 3); + CHECK(ceil(Rational { Numerator{7}, Denominator{2}, dp9 }) == 4); + CHECK(ceil(Rational { Numerator{-7}, Denominator{2}, dp9 }) == -3); + CHECK(ceil(Rational { Numerator{6}, Denominator{2}, dp9 }) == 3); } TEST_CASE("Rational::OverflowReduction", "[rational]") @@ -485,50 +485,50 @@ TEST_CASE("Rational::OverflowReduction", "[rational]") TEST_CASE("Rational::Mixed::ResultTypes", "[rational]") { using Expected = std::expected; - auto const oneHalf = Rational { 1, 2, dp9 }; - auto const oneQuarter = Rational { 1, 4, dp9 }; + auto const oneHalf = Rational { Numerator{1}, Denominator{2}, dp9 }; + auto const oneQuarter = Rational { Numerator{1}, Denominator{4}, dp9 }; auto const expectedHalf = Expected { oneHalf }; - CHECK(*(oneHalf + 0.25) == Rational { 3, 4, dp9 }); - CHECK(*(0.25 + oneHalf) == Rational { 3, 4, dp9 }); - CHECK(*(expectedHalf + oneQuarter) == Rational { 3, 4, dp9 }); + CHECK(*(oneHalf + 0.25) == Rational { Numerator{3}, Denominator{4}, dp9 }); + CHECK(*(0.25 + oneHalf) == Rational { Numerator{3}, Denominator{4}, dp9 }); + CHECK(*(expectedHalf + oneQuarter) == Rational { Numerator{3}, Denominator{4}, dp9 }); CHECK(*(oneHalf + expectedHalf) == Rational::one(dp9)); CHECK(*(expectedHalf + expectedHalf) == Rational::one(dp9)); - CHECK(*(expectedHalf - oneQuarter) == Rational { 1, 4, dp9 }); + CHECK(*(expectedHalf - oneQuarter) == Rational { Numerator{1}, Denominator{4}, dp9 }); CHECK(*(oneHalf * 2.0) == Rational::one(dp9)); - CHECK(*(expectedHalf / 2.0) == Rational { 1, 4, dp9 }); + CHECK(*(expectedHalf / 2.0) == Rational { Numerator{1}, Denominator{4}, dp9 }); } TEST_CASE("Rational::Mixed::ExactValuesHappy", "[rational]") { - auto const sum = Rational { 1, 2, dp9 } + 0.25; + auto const sum = Rational { Numerator{1}, Denominator{2}, dp9 } + 0.25; REQUIRE(sum.has_value()); - CHECK(*sum == Rational { 3, 4, dp9 }); + CHECK(*sum == Rational { Numerator{3}, Denominator{4}, dp9 }); auto const composed = - Rational { 1, 2, dp9 } / Rational { 2, dp9 } + Rational { 1, 4, dp9 } * 2.0; + Rational { Numerator{1}, Denominator{2}, dp9 } / Rational { 2, dp9 } + Rational { Numerator{1}, Denominator{4}, dp9 } * 2.0; REQUIRE(composed.has_value()); - CHECK(*composed == Rational { 3, 4, dp9 }); + CHECK(*composed == Rational { Numerator{3}, Denominator{4}, dp9 }); - auto const sevenHalves = Rational { 7, 2, dp9 }; + auto const sevenHalves = Rational { Numerator{7}, Denominator{2}, dp9 }; auto const two = Rational { 2, dp9 }; - auto const oneHalf = Rational { 1, 2, dp9 }; + auto const oneHalf = Rational { Numerator{1}, Denominator{2}, dp9 }; auto const expression = sevenHalves / two + oneHalf * 3.5; REQUIRE(expression.has_value()); - CHECK(*expression == Rational { 7, 2, dp9 }); + CHECK(*expression == Rational { Numerator{7}, Denominator{2}, dp9 }); } TEST_CASE("Rational::Mixed::ErrorPropagation::DivByZero", "[rational]") { - auto const result = Rational { 1, 2, dp9 } / Rational::zero(dp9) + Rational { 3, 4, dp9 }; + auto const result = Rational { Numerator{1}, Denominator{2}, dp9 } / Rational::zero(dp9) + Rational { Numerator{3}, Denominator{4}, dp9 }; REQUIRE_FALSE(result.has_value()); CHECK(result.error() == RationalError::DivisionByZero); - auto const otherSide = Rational { 1, 4, dp9 } + Rational { 1, dp9 } / Rational::zero(dp9); + auto const otherSide = Rational { Numerator{1}, Denominator{4}, dp9 } + Rational { 1, dp9 } / Rational::zero(dp9); REQUIRE_FALSE(otherSide.has_value()); CHECK(otherSide.error() == RationalError::DivisionByZero); - auto const product = (Rational { 1, dp9 } / Rational::zero(dp9)) * Rational { 7, 8, dp9 }; + auto const product = (Rational { 1, dp9 } / Rational::zero(dp9)) * Rational { Numerator{7}, Denominator{8}, dp9 }; REQUIRE_FALSE(product.has_value()); CHECK(product.error() == RationalError::DivisionByZero); } @@ -541,12 +541,12 @@ TEST_CASE("Rational::Mixed::ErrorPropagation::FloatNonFinite", "[rational]") CHECK(result.error() == RationalError::NotFinite); auto const positiveInfinity = std::numeric_limits::infinity(); - auto const product = Rational { 1, 2, dp9 } * positiveInfinity; + auto const product = Rational { Numerator{1}, Denominator{2}, dp9 } * positiveInfinity; REQUIRE_FALSE(product.has_value()); CHECK(product.error() == RationalError::NotFinite); auto const negativeInfinity = -std::numeric_limits::infinity(); - auto const difference = Rational { 1, 2, dp9 } - negativeInfinity; + auto const difference = Rational { Numerator{1}, Denominator{2}, dp9 } - negativeInfinity; REQUIRE_FALSE(difference.has_value()); CHECK(difference.error() == RationalError::NotFinite); } @@ -565,20 +565,20 @@ TEST_CASE("Rational::Mixed::ErrorPropagation::FirstErrorWins", "[rational]") TEST_CASE("Rational::Formatter::Default", "[rational]") { - CHECK(std::format("{}", Rational { 3, 4, dp9 }) == "3/4"); - CHECK(std::format("{}", Rational { 7, 1, dp9 }) == "7"); - CHECK(std::format("{}", Rational { -3, 4, dp9 }) == "-3/4"); + CHECK(std::format("{}", Rational { Numerator{3}, Denominator{4}, dp9 }) == "3/4"); + CHECK(std::format("{}", Rational { Numerator{7}, Denominator{1}, dp9 }) == "7"); + CHECK(std::format("{}", Rational { Numerator{-3}, Denominator{4}, dp9 }) == "-3/4"); CHECK(std::format("{}", Rational::zero(dp9)) == "0"); CHECK(std::format("{}", Rational::one(dp9)) == "1"); } TEST_CASE("Rational::Formatter::DelegatesToDouble", "[rational]") { - CHECK(std::format("{:.3f}", Rational { 1, 3, dp9 }) == "0.333"); - CHECK(std::format("{:.6f}", Rational { 22, 7, dp9 }) == "3.142857"); - CHECK(std::format("{:+.2e}", Rational { -1, 4, dp9 }) == "-2.50e-01"); + CHECK(std::format("{:.3f}", Rational { Numerator{1}, Denominator{3}, dp9 }) == "0.333"); + CHECK(std::format("{:.6f}", Rational { Numerator{22}, Denominator{7}, dp9 }) == "3.142857"); + CHECK(std::format("{:+.2e}", Rational { Numerator{-1}, Denominator{4}, dp9 }) == "-2.50e-01"); - auto const ratio = Rational { 1, 8, dp9 }; + auto const ratio = Rational { Numerator{1}, Denominator{8}, dp9 }; CHECK(std::format("{:.4f}", ratio) == std::format("{:.4f}", ratio.toDouble())); CHECK(std::format("{:10.4g}", ratio) == std::format("{:10.4g}", ratio.toDouble())); } @@ -589,7 +589,7 @@ TEST_CASE("Rational::Formatter::DelegatesToDouble", "[rational]") TEST_CASE("Rational::TriviallyCopyable", "[rational]") { - auto const original = Rational { -3, 4, dp9 }; + auto const original = Rational { Numerator{-3}, Denominator{4}, dp9 }; Rational copy {}; std::memcpy(©, &original, sizeof(Rational)); CHECK(copy == original); @@ -612,7 +612,7 @@ TEST_CASE("Rational::toDouble::PrecisionRoundtrip", "[rational]") CHECK(oneThird.toDouble(6) == Catch::Approx(0.333333).epsilon(1e-12)); CHECK(oneThird.toDouble(9) == Catch::Approx(oneThird.toDouble()).epsilon(1e-15)); - auto const negativeQuarter = Rational { -1, 4, dp9 }; + auto const negativeQuarter = Rational { Numerator{-1}, Denominator{4}, dp9 }; CHECK(negativeQuarter.toDouble(2) == Catch::Approx(-0.25).epsilon(1e-12)); CHECK(negativeQuarter.toDouble(1) == Catch::Approx(-0.3).epsilon(1e-12)); } @@ -621,7 +621,7 @@ TEST_CASE("Rational::CommonArithmetic::PiApproximation22Over7", "[rational]") { auto const exactQuotient = Rational { 22, dp3 } / Rational { 7, dp3 }; REQUIRE(exactQuotient.has_value()); - CHECK(*exactQuotient == Rational { 22, 7, dp3 }); + CHECK(*exactQuotient == Rational { Numerator{22}, Denominator{7}, dp3 }); CHECK(exactQuotient->toDouble() == Catch::Approx(3.143).epsilon(1e-12)); CHECK(exactQuotient->toDouble(3) == Catch::Approx(3.143).epsilon(1e-12)); CHECK(std::format("{:.3f}", *exactQuotient) == "3.143"); @@ -632,33 +632,33 @@ TEST_CASE("Rational::CommonArithmetic::PiApproximation22Over7", "[rational]") REQUIRE(divisor.has_value()); auto const liftedQuotient = *dividend / *divisor; REQUIRE(liftedQuotient.has_value()); - CHECK(*liftedQuotient == Rational { 22, 7, dp3 }); + CHECK(*liftedQuotient == Rational { Numerator{22}, Denominator{7}, dp3 }); CHECK(liftedQuotient->toDouble() == Catch::Approx(3.143).epsilon(1e-12)); auto const mixedQuotient = Rational { 22, dp3 } / 7.0; REQUIRE(mixedQuotient.has_value()); - CHECK(*mixedQuotient == Rational { 22, 7, dp3 }); + CHECK(*mixedQuotient == Rational { Numerator{22}, Denominator{7}, dp3 }); CHECK(mixedQuotient->toDouble() == Catch::Approx(3.143).epsilon(1e-12)); } TEST_CASE("Rational::FreeFunctions::AbsCeilFloorTrunc", "[rational]") { - CHECK(abs(Rational { -3, 4, dp9 }) == Rational { 3, 4, dp9 }); - CHECK(abs(Rational { 3, 4, dp9 }) == Rational { 3, 4, dp9 }); + CHECK(abs(Rational { Numerator{-3}, Denominator{4}, dp9 }) == Rational { Numerator{3}, Denominator{4}, dp9 }); + CHECK(abs(Rational { Numerator{3}, Denominator{4}, dp9 }) == Rational { Numerator{3}, Denominator{4}, dp9 }); CHECK(abs(Rational::zero(dp9)) == Rational::zero(dp9)); - CHECK(ceil(Rational { 7, 2, dp9 }) == 4); - CHECK(floor(Rational { 7, 2, dp9 }) == 3); - CHECK(trunc(Rational { 7, 2, dp9 }) == 3); + CHECK(ceil(Rational { Numerator{7}, Denominator{2}, dp9 }) == 4); + CHECK(floor(Rational { Numerator{7}, Denominator{2}, dp9 }) == 3); + CHECK(trunc(Rational { Numerator{7}, Denominator{2}, dp9 }) == 3); using std::abs; using std::ceil; using std::floor; using std::trunc; CHECK(abs(-2.5) == Catch::Approx(2.5)); - CHECK(abs(Rational { -3, 4, dp9 }) == Rational { 3, 4, dp9 }); + CHECK(abs(Rational { Numerator{-3}, Denominator{4}, dp9 }) == Rational { Numerator{3}, Denominator{4}, dp9 }); CHECK(ceil(2.1) == Catch::Approx(3.0)); - CHECK(ceil(Rational { 7, 2, dp9 }) == 4); + CHECK(ceil(Rational { Numerator{7}, Denominator{2}, dp9 }) == 4); } // --------------------------------------------------------------------------- @@ -668,31 +668,31 @@ TEST_CASE("Rational::FreeFunctions::AbsCeilFloorTrunc", "[rational]") TEST_CASE("Rational::CrossPrecision::MaxPropagates", "[rational]") { // Result precision is the max of the two operands' precisions. - CHECK((Rational { 1, 2, dp3 } + Rational { 1, 4, dp9 }).getDecimalPlaces() == dp9); - CHECK((Rational { 1, 2, dp9 } + Rational { 1, 4, dp3 }).getDecimalPlaces() == dp9); - CHECK((Rational { 1, 2, dp3 } + Rational { 1, 4, dp3 }).getDecimalPlaces() == dp3); - CHECK((Rational { 1, 2, dp3 } - Rational { 1, 4, dp9 }).getDecimalPlaces() == dp9); - CHECK((Rational { 1, 2, dp3 } * Rational { 1, 4, dp9 }).getDecimalPlaces() == dp9); + CHECK((Rational { Numerator{1}, Denominator{2}, dp3 } + Rational { Numerator{1}, Denominator{4}, dp9 }).getDecimalPlaces() == dp9); + CHECK((Rational { Numerator{1}, Denominator{2}, dp9 } + Rational { Numerator{1}, Denominator{4}, dp3 }).getDecimalPlaces() == dp9); + CHECK((Rational { Numerator{1}, Denominator{2}, dp3 } + Rational { Numerator{1}, Denominator{4}, dp3 }).getDecimalPlaces() == dp3); + CHECK((Rational { Numerator{1}, Denominator{2}, dp3 } - Rational { Numerator{1}, Denominator{4}, dp9 }).getDecimalPlaces() == dp9); + CHECK((Rational { Numerator{1}, Denominator{2}, dp3 } * Rational { Numerator{1}, Denominator{4}, dp9 }).getDecimalPlaces() == dp9); - auto const quotient = Rational { 3, 4, dp3 } / Rational { 1, 2, dp9 }; + auto const quotient = Rational { Numerator{3}, Denominator{4}, dp3 } / Rational { Numerator{1}, Denominator{2}, dp9 }; REQUIRE(quotient.has_value()); CHECK(quotient->getDecimalPlaces() == dp9); } TEST_CASE("Rational::CrossPrecision::ValueCorrectness", "[rational]") { - auto const sum = Rational { 1, 2, dp3 } + Rational { 1, 4, dp9 }; - CHECK(sum == Rational { 3, 4, dp9 }); + auto const sum = Rational { Numerator{1}, Denominator{2}, dp3 } + Rational { Numerator{1}, Denominator{4}, dp9 }; + CHECK(sum == Rational { Numerator{3}, Denominator{4}, dp9 }); - auto const difference = Rational { 2, 3, dp9 } - Rational { 1, 3, dp3 }; - CHECK(difference == Rational { 1, 3, dp9 }); + auto const difference = Rational { Numerator{2}, Denominator{3}, dp9 } - Rational { Numerator{1}, Denominator{3}, dp3 }; + CHECK(difference == Rational { Numerator{1}, Denominator{3}, dp9 }); - auto const product = Rational { 2, 3, dp3 } * Rational { 3, 2, dp9 }; + auto const product = Rational { Numerator{2}, Denominator{3}, dp3 } * Rational { Numerator{3}, Denominator{2}, dp9 }; CHECK(product == Rational::one(dp9)); - auto const quotient = Rational { 3, 4, dp3 } / Rational { 1, 2, dp9 }; + auto const quotient = Rational { Numerator{3}, Denominator{4}, dp3 } / Rational { Numerator{1}, Denominator{2}, dp9 }; REQUIRE(quotient.has_value()); - CHECK(*quotient == Rational { 3, 2, dp9 }); + CHECK(*quotient == Rational { Numerator{3}, Denominator{2}, dp9 }); } TEST_CASE("Rational::Mixed::FloatInheritsRationalPrecision", "[rational]") @@ -703,13 +703,13 @@ TEST_CASE("Rational::Mixed::FloatInheritsRationalPrecision", "[rational]") std::expected>); // 1/2 (precision 3) + 0.25 (lifted at precision 3) = 3/4 at precision 3. - auto const lifted = Rational { 1, 2, dp3 } + 0.25; + auto const lifted = Rational { Numerator{1}, Denominator{2}, dp3 } + 0.25; REQUIRE(lifted.has_value()); CHECK(lifted->getDecimalPlaces() == dp3); - CHECK(*lifted == Rational { 3, 4, dp3 }); + CHECK(*lifted == Rational { Numerator{3}, Denominator{4}, dp3 }); // Float lifts to precision 9 when the Rational operand carries 9. - auto const liftedAt9 = Rational { 1, 2, dp9 } + 0.25; + auto const liftedAt9 = Rational { Numerator{1}, Denominator{2}, dp9 } + 0.25; REQUIRE(liftedAt9.has_value()); CHECK(liftedAt9->getDecimalPlaces() == dp9); } @@ -721,9 +721,9 @@ TEST_CASE("Rational::Mixed::FloatInheritsRationalPrecision", "[rational]") TEST_CASE("Rational::Predicates::isNegative", "[rational]") { - CHECK(Rational { -1, 2, dp9 }.isNegative()); - CHECK(Rational { 1, -2, dp9 }.isNegative()); - CHECK_FALSE(Rational { 1, 2, dp9 }.isNegative()); + CHECK(Rational { Numerator{-1}, Denominator{2}, dp9 }.isNegative()); + CHECK(Rational { Numerator{1}, Denominator{-2}, dp9 }.isNegative()); + CHECK_FALSE(Rational { Numerator{1}, Denominator{2}, dp9 }.isNegative()); CHECK_FALSE(Rational::zero(dp9).isNegative()); } @@ -731,7 +731,7 @@ TEST_CASE("Rational::toDouble::OverlargePrecisionFallsBackUnrounded", "[rational { // Requested precision beyond 18 cannot be scaled in int64; the conversion // falls back to the unrounded quotient. - auto const oneThird = Rational { 1, 3, dp9 }; + auto const oneThird = Rational { Numerator{1}, Denominator{3}, dp9 }; CHECK(oneThird.toDouble(19) == Catch::Approx(1.0 / 3.0).epsilon(1e-15)); CHECK(oneThird.toDouble(99) == Catch::Approx(1.0 / 3.0).epsilon(1e-15)); } @@ -804,7 +804,7 @@ TEST_CASE("Rational::Wire::Int64MinComponentsClamp", "[rational]") TEST_CASE("Rational::Wire::RoundTrip", "[rational]") { - auto const original = Rational { 617, 50, dp2 }; + auto const original = Rational { Numerator{617}, Denominator{50}, dp2 }; auto const json = glz::write_json(original); REQUIRE(json.has_value()); CHECK(*json == R"({"num":617,"den":50,"dp":2})"); @@ -823,7 +823,7 @@ TEST_CASE("Rational::Wire::CanonicalisesOnRead", "[rational]") REQUIRE_FALSE(glz::read_json(value, R"({"num":1234,"den":100,"dp":2})")); CHECK(value.numerator == 617); CHECK(value.denominator == 50); - CHECK(value == Rational { 1234, 100, dp2 }); + CHECK(value == Rational { Numerator{1234}, Denominator{100}, dp2 }); Rational negative {}; REQUIRE_FALSE(glz::read_json(negative, R"({"num":3,"den":-4,"dp":1})")); @@ -862,7 +862,7 @@ TEST_CASE("Rational::Wire::NullableComposition", "[rational]") CHECK_FALSE(maybe.has_value()); REQUIRE_FALSE(glz::read_json(maybe, R"({"num":1,"den":3,"dp":9})")); - CHECK(maybe == std::optional { Rational { 1, 3, dp9 } }); + CHECK(maybe == std::optional { Rational { Numerator{1}, Denominator{3}, dp9 } }); auto const written = glz::write_json(maybe); REQUIRE(written.has_value()); From df5b7f5b28460e848cdc76f53cb71a123890e305 Mon Sep 17 00:00:00 2001 From: Yaraslau Tamashevich Date: Tue, 14 Jul 2026 17:13:59 +0200 Subject: [PATCH 2/3] Fix build after Numerator/Denominator strong types The clang-tidy commit introduced Numerator/Denominator strong types for the Rational constructor but left call sites unconverted and the Qt backend override stale. - Import morph::math::Numerator/Denominator in the rational and quantity form tests and wrap the remaining raw-int 3-arg Rational constructions. - Update QtWebSocketBackend::setReconnectHandler to take const std::function& to match the base class, restoring the override (Qt files are not built in the Linux jobs, so this was missed). Co-Authored-By: Claude Opus 4.8 (1M context) --- include/morph/qt/qt_websocket_backend.hpp | 2 +- src/qt/qt_websocket_backend.cpp | 4 ++-- tests/test_quantity_forms.cpp | 2 ++ tests/test_rational.cpp | 24 ++++++++++++----------- 4 files changed, 18 insertions(+), 14 deletions(-) diff --git a/include/morph/qt/qt_websocket_backend.hpp b/include/morph/qt/qt_websocket_backend.hpp index 3cf7a99..37041fd 100644 --- a/include/morph/qt/qt_websocket_backend.hpp +++ b/include/morph/qt/qt_websocket_backend.hpp @@ -123,7 +123,7 @@ class QtWebSocketBackend : public ::morph::backend::detail::IBackend { /// @brief Installs the handler `Bridge` uses to re-register handlers after a reconnect. /// @param handler Callable invoked on the Qt thread after every successful reconnect. /// Pass `nullptr` to clear. - void setReconnectHandler(std::function handler) override; + void setReconnectHandler(const std::function& handler) override; private: /// @brief Sends @p msg synchronously by blocking the Qt thread via a nested event loop. diff --git a/src/qt/qt_websocket_backend.cpp b/src/qt/qt_websocket_backend.cpp index a1f61df..7936b2b 100644 --- a/src/qt/qt_websocket_backend.cpp +++ b/src/qt/qt_websocket_backend.cpp @@ -150,8 +150,8 @@ void QtWebSocketBackend::cancelPending(const std::exception_ptr& exc) { } } -void QtWebSocketBackend::setReconnectHandler(std::function handler) { - _reconnectHandler = std::move(handler); +void QtWebSocketBackend::setReconnectHandler(const std::function& handler) { + _reconnectHandler = handler; } void QtWebSocketBackend::scheduleReconnect() { diff --git a/tests/test_quantity_forms.cpp b/tests/test_quantity_forms.cpp index cf36018..3e7f7b3 100644 --- a/tests/test_quantity_forms.cpp +++ b/tests/test_quantity_forms.cpp @@ -24,6 +24,8 @@ #include using morph::math::DecimalPlaces; +using morph::math::Denominator; +using morph::math::Numerator; using morph::math::Rational; // --------------------------------------------------------------------------- diff --git a/tests/test_rational.cpp b/tests/test_rational.cpp index 190b3a4..4d2c0b2 100644 --- a/tests/test_rational.cpp +++ b/tests/test_rational.cpp @@ -24,7 +24,9 @@ // (`.value` instead of `unbox<...>`); wire-codec cases added at the end. using morph::math::DecimalPlaces; +using morph::math::Denominator; using morph::math::kMaxDecimalPlaces; +using morph::math::Numerator; using morph::math::Rational; using morph::math::RationalError; @@ -203,14 +205,14 @@ TEST_CASE("Rational::PrecisionCap", "[rational]") CHECK(kMaxDecimalPlaces == 18); // Precision at the cap is accepted unchanged. - CHECK(Rational { 1, 2, DecimalPlaces { 18 } }.getDecimalPlaces() == DecimalPlaces { 18 }); + CHECK(Rational { Numerator{1}, Denominator{2}, DecimalPlaces { 18 } }.getDecimalPlaces() == DecimalPlaces { 18 }); // In a release build (NDEBUG) an out-of-range precision is clamped into // [1, kMaxDecimalPlaces]. A debug build asserts before reaching here, so // these clamp checks only run when NDEBUG is defined. #ifdef NDEBUG - CHECK(Rational { 1, 2, DecimalPlaces { 0 } }.getDecimalPlaces() == DecimalPlaces { 1 }); - CHECK(Rational { 1, 2, DecimalPlaces { 99 } }.getDecimalPlaces() == DecimalPlaces { 18 }); + CHECK(Rational { Numerator{1}, Denominator{2}, DecimalPlaces { 0 } }.getDecimalPlaces() == DecimalPlaces { 1 }); + CHECK(Rational { Numerator{1}, Denominator{2}, DecimalPlaces { 99 } }.getDecimalPlaces() == DecimalPlaces { 18 }); #endif } @@ -244,7 +246,7 @@ TEST_CASE("Rational::DenominatorAlwaysPositive", "[rational]") CHECK((left * right).denominator > 0); CHECK((*(left / right)).denominator > 0); - Rational mutating { 1, 2, dp9 }; + Rational mutating { Numerator{1}, Denominator{2}, dp9 }; mutating += right; CHECK(mutating.denominator > 0); mutating -= right; @@ -468,13 +470,13 @@ TEST_CASE("Rational::Conversions", "[rational]") TEST_CASE("Rational::OverflowReduction", "[rational]") { auto constexpr large = std::int64_t { std::numeric_limits::max() }; - auto const left = Rational { large, 1, dp9 }; - auto const right = Rational { 1, large, dp9 }; + auto const left = Rational { Numerator{large}, Denominator{1}, dp9 }; + auto const right = Rational { Numerator{1}, Denominator{large}, dp9 }; auto const sum = left + right; CHECK(sum.denominator == large); CHECK(sum.numerator == (large * large) + 1); - auto const cancelling = Rational { large, 7, dp9 } * Rational { 7, large, dp9 }; + auto const cancelling = Rational { Numerator{large}, Denominator{7}, dp9 } * Rational { Numerator{7}, Denominator{large}, dp9 }; CHECK(cancelling == Rational::one(dp9)); } @@ -764,8 +766,8 @@ TEST_CASE("Rational::Comparison::ExactAtInt64Extremes", "[rational]") // long-double comparison would round both to the same value and call // them equal. The 128-bit comparison must not. auto constexpr big = std::int64_t { 1 } << 62; - auto const left = Rational { big + 1, big, dp9 }; // 1 + 1/2^62 - auto const right = Rational { big, big - 1, dp9 }; // 1 + 1/(2^62-1) + auto const left = Rational { Numerator{big + 1}, Denominator{big}, dp9 }; // 1 + 1/2^62 + auto const right = Rational { Numerator{big}, Denominator{big - 1}, dp9 }; // 1 + 1/(2^62-1) CHECK(left != right); CHECK(left < right); @@ -794,7 +796,7 @@ TEST_CASE("Rational::Wire::Int64MinComponentsClamp", "[rational]") Rational numeratorCase {}; REQUIRE_FALSE(glz::read_json(numeratorCase, R"({"num":-9223372036854775808,"den":2,"dp":1})")); CHECK(numeratorCase.denominator > 0); - CHECK(numeratorCase == Rational { -std::numeric_limits::max(), 2, dp1 }); + CHECK(numeratorCase == Rational { Numerator{-std::numeric_limits::max()}, Denominator{2}, dp1 }); Rational denominatorCase {}; REQUIRE_FALSE(glz::read_json(denominatorCase, R"({"num":3,"den":-9223372036854775808,"dp":1})")); @@ -849,7 +851,7 @@ TEST_CASE("Rational::Wire::HostileInputClamps", "[rational]") TEST_CASE("Rational::Wire::MissingFieldsUseWireDefaults", "[rational]") { // Absent keys fall back to Wire's member defaults (0/1 at precision 1). - Rational value { 9, 4, dp9 }; + Rational value { Numerator{9}, Denominator{4}, dp9 }; REQUIRE_FALSE(glz::read_json(value, R"({})")); CHECK(value == Rational::zero(dp1)); CHECK(value.getDecimalPlaces() == dp1); From 5f51fa3f1ebe50801ed724fbcecd41d389322a64 Mon Sep 17 00:00:00 2001 From: Yaraslau Tamashevich Date: Tue, 14 Jul 2026 17:28:33 +0200 Subject: [PATCH 3/3] tests: cover Timestamp operator- to close patch-coverage gap The clang-tidy commit added the free operator-(Timestamp, Timestamp) returning an optional chrono duration, but no test exercised it, so codecov/patch flagged datetime.hpp:270-275 as the only uncovered lines in the diff. Add a Timestamp::Difference case covering both engaged operands (signed result) and every empty-operand branch (nullopt). Co-Authored-By: Claude Opus 4.8 (1M context) --- tests/test_datetime.cpp | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/tests/test_datetime.cpp b/tests/test_datetime.cpp index 1433960..7567e2f 100644 --- a/tests/test_datetime.cpp +++ b/tests/test_datetime.cpp @@ -151,3 +151,20 @@ TEST_CASE("Timestamp::EmptyStateAndWire", "[datetime][forms]") { REQUIRE_FALSE(glz::read_json(restored, R"({"at":null})")); CHECK_FALSE(restored.at.hasValue()); } + +TEST_CASE("Timestamp::Difference", "[datetime][forms]") { + auto const earlier = Timestamp{sample()}; + auto const later = Timestamp{sample() + std::chrono::minutes{5}}; + + // Both engaged: the signed chrono duration between the instants. + auto const delta = later - earlier; + REQUIRE(delta.has_value()); + CHECK(*delta == std::chrono::minutes{5}); + CHECK((earlier - later).value() == -std::chrono::minutes{5}); + + // Either operand empty collapses to nullopt. + Timestamp const blank; + CHECK_FALSE((later - blank).has_value()); + CHECK_FALSE((blank - later).has_value()); + CHECK_FALSE((blank - blank).has_value()); +}