core: add MORPH_CLIENT_ONLY to suppress model-owning registrars - #37
Open
Yaraslaut wants to merge 3 commits into
Open
core: add MORPH_CLIENT_ONLY to suppress model-owning registrars#37Yaraslaut wants to merge 3 commits into
Yaraslaut wants to merge 3 commits into
Conversation
BRIDGE_REGISTER_MODEL/BRIDGE_REGISTER_ACTION emit three registrars per action. registerModelOnce's factory calls ModelFactory::create<Model>(), and registerActionOnce's runner calls Model::execute(...) on a live holder -- both are ordinary functions the compiler must fully compile into their stored closures regardless of whether those closures are ever invoked at runtime, so even a pure client that dispatches every action to a remote peer and never constructs a model locally still forces the linker to resolve the model's constructor and execute() bodies. Those routinely depend on a platform stack the client target doesn't have (a database driver, a native UI framework, an OS-specific API) -- for a browser/WASM build they don't exist at all, so the link cannot be satisfied. Add a MORPH_CLIENT_ONLY CMake option that, when ON, defines MORPH_CLIENT_ONLY on the morph target's INTERFACE (never per-consumer, since two TUs disagreeing would violate ODR) and suppresses registerModelOnce/ registerActionOnce's emission from the two macros. ModelTraits<M>/ ActionTraits<A> (type-ids, JSON codecs) are still specialised exactly as before -- only the two registrar bodies disappear. The suggested third suppression target, registerActionExecutorOnce, turned out to need different treatment than expected: it routes through BridgeHandler::execute<Action>() -> Bridge::executeVia, and executeVia unconditionally constructs an ActionCall::localOp closure that calls Model::execute directly, regardless of which backend ends up installed at runtime (only LocalBackend::execute ever invokes it; every remote backend ignores it). That closure is what actually needs the model's execute() body -- confirmed empirically by building a probe with the model's constructor/execute declared but never defined anywhere in the link: it failed exactly as expected without the guard, referencing both symbols from inside executeVia's instantiation, not from registerActionExecutorOnce. Gating executeVia's localOp on MORPH_CLIENT_ONLY (throwing instead of calling Model::execute) closes this properly, so both the typed BridgeHandler::execute<Action>() and the type-erased executeJson path work against a remote backend in a client-only build; LocalBackend must not be used in one. Verified in both directions via a new try_compile() guard (tests/CMakeLists.txt) against tests/compile_checks/client_only_no_model_link.cpp: links successfully with MORPH_CLIENT_ONLY defined, fails to link (both symbols genuinely unresolved) without it. NEVER define MORPH_CLIENT_ONLY for a process that hosts models (a server, or any Bridge running LocalBackend) -- it silently registers nothing, and the model fails at runtime with "unknown model type" (or, if LocalBackend's executeVia path is reached anyway, a clear std::logic_error) rather than at compile/link time. Closes #27 Signed-off-by: Yaraslau Tamashevich <yaraslau.tamashevich@gmail.com>
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
The client_only_no_model_link.cpp try_compile() checks currently swallow the underlying compiler/linker diagnostics on failure -- CI only shows the FATAL_ERROR summary, not why the probe actually failed. Capture OUTPUT_VARIABLE and print it so a failure (e.g. the Windows/cl-debug and Windows/cl-release failures on this PR) is self-diagnosing. Signed-off-by: Yaraslau Tamashevich <yaraslau.tamashevich@gmail.com>
MSVC's cl.exe failed the MORPH_CLIENT_ONLY_GUARD_SUPPRESSES_LINKAGE
try_compile with hard compile errors (C2143/C4430/C2059) attributed to the
BRIDGE_REGISTER_ACTION(...) call. The preceding warning names
BRIDGE_REGISTER_ACTION_PICK directly ("not enough arguments for
function-like macro invocation"), a known MSVC quirk with the
variadic-count-dispatch idiom that's otherwise benign everywhere else in
the suite -- but combined with MORPH_CLIENT_ONLY's empty
MORPH_DETAIL_REGISTER_ACTION_LOCAL expansion, it produced a hard failure on
cl.exe specifically (Clang/GCC/clang-cl were unaffected).
Call BRIDGE_REGISTER_ACTION_4 directly, bypassing the PICK dispatch, since
the guard only needs to exercise the registrar-suppression path, not the
public macro's arity-dispatch mechanism.
Signed-off-by: Yaraslau Tamashevich <yaraslau.tamashevich@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
BRIDGE_REGISTER_MODEL/BRIDGE_REGISTER_ACTIONemit two registrars (registerModelOnce,registerActionOnce) whose stored closures call the model's constructor andexecute()— compiled in regardless of whether the closure is ever invoked at runtime, so a pure remote client still forces the linker to resolve them, along with whatever platform stack they depend on (a browser/WASM build in particular has no link path for those at all).MORPH_CLIENT_ONLYCMake option that defines the macro on themorphtarget'sINTERFACE(never per-consumer — two TUs disagreeing would violate ODR) and suppresses both registrars' emission.ModelTraits/ActionTraits(type-ids, JSON codecs) are unaffected.registerActionExecutorOnce(suggested as safe, needing only declarations) actually routes throughBridge::executeVia, which unconditionally compiles anActionCall::localOpclosure callingModel::executedirectly — regardless of which backend ends up installed at runtime. Confirmed empirically with a probe (model constructor/execute()declared but never defined anywhere in the link): it failed exactly as expected, referencing both symbols from insideexecuteVia's instantiation, not fromregisterActionExecutorOnce. Fixed by gatingexecuteVia'slocalOpitself onMORPH_CLIENT_ONLY(throws instead of callingModel::execute), so both the typedBridgeHandler::execute<Action>()and the type-erasedexecuteJsonpath work against a remote backend in a client-only build.NEVERdefine this for a process that hosts models — it silently registers nothing, failing at runtime with"unknown model type"(or a clearstd::logic_errorifexecuteVia's local path is reached anyway) rather than at compile/link time.Test plan
tests/compile_checks/client_only_no_model_link.cpp+ twotry_compile()guards intests/CMakeLists.txt(mirroring the existingMORPH_REQUIRE_VETTED_HMACguard pattern), proving both directions: links successfully withMORPH_CLIENT_ONLYdefined; fails to link (both the constructor andexecute()genuinely unresolved) without it../build/tests/morph_tests— all 811 test cases / 8284 assertions pass, unmodified — confirms zero behavior change for the default (non-client-only) build.Closes #27
🤖 Generated with Claude Code