core: key registration macro identifiers on __LINE__, not type spelling

docs/spec/core/registry.md

@@ -469,6 +469,20 @@ variables).
 
 ## Registration macros
 
+Both macros below name their generated anonymous-namespace variable by pasting
+a fixed prefix onto `__COUNTER__` (via the two-level `BRIDGE_DETAIL_CAT`/
+`BRIDGE_DETAIL_CAT_` indirection needed to force macro expansion before the
+paste), not onto the spelling of `M`/`A`. Pasting the type directly (the
+original approach) breaks for namespace-qualified or template types —
+`app::models::Report` pastes `::` into the identifier. `__LINE__` was tried as
+a replacement key but is only unique *within a single physical file*; two
+different headers that each invoke one of these macros on the same line
+number produce the same identifier once both are transitively `#include`d
+into one translation unit, which is a hard redefinition error because C++
+unnamed namespaces are per-TU, not per-file. `__COUNTER__` increments
+monotonically across the whole translation unit regardless of which file
+expands it, so it cannot collide this way.
+
 ### `BRIDGE_REGISTER_MODEL(M, NAME)`
 
 Specialises `ModelTraits<M>` and registers a factory at static-init time.

include/morph/core/registry.hpp

@@ -381,6 +381,21 @@ bool registerActionExecutorOnce(std::string_view modelId, std::string_view actio
 
 // NOLINTBEGIN(bugprone-macro-parentheses)
 
+// Keying the generated registrar names on `__COUNTER__` (rather than on the type spelling)
+// keeps them valid identifiers regardless of how `M`/`A` are written. A namespace-qualified
+// or template type (e.g. `app::models::Report`) would otherwise be pasted directly into the
+// identifier, producing invalid tokens like `bridge_model_reg_app::models::Report`.
+// `__LINE__` was tried first, but it is only unique *within a single physical file*: two
+// headers that each invoke this macro on the same line number (e.g. `card_model.hpp:38` and
+// `payment_model.hpp:38`) produce the same identifier once both are `#include`d into one
+// translation unit, and since C++ unnamed namespaces are per-TU (not per-file), that is a
+// hard redefinition error. `__COUNTER__` increments monotonically for the entire translation
+// unit regardless of which file expands it, so it cannot collide this way. The extra
+// indirection (`_CAT` calling `_CAT_`) is required so `__COUNTER__` is expanded to its numeric
+// value before the paste, rather than being pasted as the literal text "__COUNTER__".
+#define BRIDGE_DETAIL_CAT_(a, b) a##b
+#define BRIDGE_DETAIL_CAT(a, b) BRIDGE_DETAIL_CAT_(a, b)
+
 /// @brief Registers model type @p M with the string id @p NAME.
 ///
 /// Specialises `morph::model::ModelTraits<M>` and registers a factory with the
@@ -394,7 +409,8 @@ bool registerActionExecutorOnce(std::string_view modelId, std::string_view actio
         static constexpr std::string_view typeId() noexcept { return NAME; }                             \
     };                                                                                                   \
     namespace {                                                                                          \
-    [[maybe_unused]] const bool bridge_model_reg_##M = morph::model::detail::registerModelOnce<M>(NAME); \
+    [[maybe_unused]] const bool BRIDGE_DETAIL_CAT(bridge_model_reg_, __COUNTER__) =                      \
+        morph::model::detail::registerModelOnce<M>(NAME);                                                \
     }
 
 /// @brief Registers action type @p A (for model @p M) with the string id @p NAME.
@@ -475,9 +491,9 @@ bool registerActionExecutorOnce(std::string_view modelId, std::string_view actio
         }                                                                                                      \
     };                                                                                                         \
     namespace {                                                                                                \
-    [[maybe_unused]] const bool bridge_action_reg_##M##_##A =                                                  \
+    [[maybe_unused]] const bool BRIDGE_DETAIL_CAT(bridge_action_reg_, __COUNTER__) =                           \
         morph::model::detail::registerActionOnce<M, A>(morph::model::ModelTraits<M>::typeId(), NAME);          \
-    [[maybe_unused]] const bool bridge_action_exec_reg_##M##_##A =                                             \
+    [[maybe_unused]] const bool BRIDGE_DETAIL_CAT(bridge_action_exec_reg_, __COUNTER__) =                      \
         morph::model::detail::registerActionExecutorOnce<M, A>(morph::model::ModelTraits<M>::typeId(), NAME);  \
     }
 /// @endcond

tests/CMakeLists.txt

@@ -18,6 +18,8 @@ add_executable(morph_tests
     test_observability.cpp
     test_backend_extra.cpp
     test_registry_extra.cpp
+    test_registration_qualified_types.cpp
+    test_registration_same_line.cpp
     test_bridge_local.cpp
     test_bridge_remote.cpp
     test_bridge_execute_json.cpp

tests/issue21_same_line_a.hpp

@@ -0,0 +1,19 @@
+// SPDX-License-Identifier: Apache-2.0
+
+// Companion header for test_registration_same_line.cpp -- see that file for the scenario this
+// reproduces. The BRIDGE_REGISTER_MODEL invocation below must stay on the exact same physical
+// line number as the one in issue21_same_line_b.hpp for the two headers to exercise the bug.
+
+#pragma once
+
+#include <morph/core/registry.hpp>
+
+namespace issue21::sameline {
+
+struct WidgetA {
+    int execute(int x) { return x + 1; }
+};
+
+}  // namespace issue21::sameline
+
+BRIDGE_REGISTER_MODEL(issue21::sameline::WidgetA, "Issue21_SameLine_WidgetA")

tests/issue21_same_line_b.hpp

@@ -0,0 +1,19 @@
+// SPDX-License-Identifier: Apache-2.0
+
+// Companion header for test_registration_same_line.cpp -- see that file for the scenario this
+// reproduces. The BRIDGE_REGISTER_MODEL invocation below must stay on the exact same physical
+// line number as the one in issue21_same_line_a.hpp for the two headers to exercise the bug.
+
+#pragma once
+
+#include <morph/core/registry.hpp>
+
+namespace issue21::sameline {
+
+struct WidgetB {
+    int execute(int x) { return x + 2; }
+};
+
+}  // namespace issue21::sameline
+
+BRIDGE_REGISTER_MODEL(issue21::sameline::WidgetB, "Issue21_SameLine_WidgetB")

tests/test_registration_qualified_types.cpp

@@ -0,0 +1,50 @@
+// SPDX-License-Identifier: Apache-2.0
+
+// Regression test for issue #21: BRIDGE_REGISTER_MODEL / BRIDGE_REGISTER_ACTION built the name
+// of their generated static registrar by token-pasting the model/action type onto a fixed
+// prefix (`bridge_model_reg_##M`, `bridge_action_reg_##M##_##A`). That only produces a valid
+// identifier when both arguments are bare identifiers -- a namespace-qualified type pastes ':'
+// characters into the token and fails to compile. This file registers a namespace-qualified
+// model and action exactly as the issue describes; it must compile (and the runtime dispatch
+// below must succeed) for the fix to be verified.
+
+#include <morph/core/bridge.hpp>
+#include <morph/core/registry.hpp>
+
+#include <catch2/catch_test_macros.hpp>
+#include <string>
+#include <string_view>
+
+namespace issue21::models {
+
+struct ReportCreateResult {
+    bool ok = false;
+};
+
+struct ReportCreate {
+    int id = 0;
+};
+
+class Report {
+public:
+    ReportCreateResult execute(const ReportCreate& action) { return ReportCreateResult{.ok = action.id > 0}; }
+};
+
+}  // namespace issue21::models
+
+// Namespace-qualified model and action -- this is exactly the case that used to fail to
+// compile (the generated identifiers `bridge_model_reg_issue21::models::Report` and
+// `bridge_action_reg_issue21::models::Report_issue21::models::ReportCreate` are not valid
+// tokens).
+BRIDGE_REGISTER_MODEL(issue21::models::Report, "Issue21_Report")
+BRIDGE_REGISTER_ACTION(issue21::models::Report, issue21::models::ReportCreate, "Issue21_Create")
+
+TEST_CASE("BRIDGE_REGISTER_MODEL/BRIDGE_REGISTER_ACTION compile and register namespace-qualified types",
+          "[registry][issue21]") {
+    auto holder = morph::model::detail::ModelRegistryFactory::instance().create("Issue21_Report");
+    REQUIRE(holder != nullptr);
+
+    auto result = morph::model::detail::ActionDispatcher::instance().dispatch("Issue21_Report", "Issue21_Create",
+                                                                               *holder, R"({"id":5})");
+    REQUIRE(result == R"({"ok":true})");
+}

tests/test_registration_same_line.cpp

@@ -0,0 +1,26 @@
+// SPDX-License-Identifier: Apache-2.0
+
+// Regression test for issue #21 (follow-up): keying the generated registrar variable name on
+// `__LINE__` fixed the original namespace-qualified-type bug but introduced a new one --
+// `__LINE__` is only unique within a single physical file, so two different headers that each
+// invoke BRIDGE_REGISTER_MODEL/BRIDGE_REGISTER_ACTION on the same line number produce the same
+// generated identifier once both are `#include`d into one translation unit. Since C++ unnamed
+// namespaces are per-translation-unit (not per-file), that is a hard redefinition error -- this
+// is exactly what broke the WASM demo build (multiple bank model headers, each registering on
+// the same line number, all pulled into one autogen TU). issue21_same_line_a.hpp and
+// issue21_same_line_b.hpp each invoke BRIDGE_REGISTER_MODEL on line 19; this file must compile
+// (and both models must be independently registered) for the fix (`__COUNTER__` instead of
+// `__LINE__`) to be verified.
+
+#include "issue21_same_line_a.hpp"
+#include "issue21_same_line_b.hpp"
+
+#include <catch2/catch_test_macros.hpp>
+
+TEST_CASE("BRIDGE_REGISTER_MODEL on the same line number in two different headers does not collide",
+          "[registry][issue21]") {
+    auto holderA = morph::model::detail::ModelRegistryFactory::instance().create("Issue21_SameLine_WidgetA");
+    auto holderB = morph::model::detail::ModelRegistryFactory::instance().create("Issue21_SameLine_WidgetB");
+    REQUIRE(holderA != nullptr);
+    REQUIRE(holderB != nullptr);
+}