core: key registration macro identifiers on __LINE__, not type spelling - #31
Open
Yaraslaut wants to merge 2 commits into
Open
core: key registration macro identifiers on __LINE__, not type spelling#31Yaraslaut wants to merge 2 commits into
Yaraslaut wants to merge 2 commits into
Conversation
BRIDGE_REGISTER_MODEL/BRIDGE_REGISTER_ACTION built their generated static registrar names by token-pasting the model/action type directly 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, with the error pointing at the macro expansion rather than the call site. Key the generated names on __LINE__ instead (via the standard two-level CAT indirection so it expands before pasting). There is exactly one registration per macro invocation, so __LINE__ is equally unique, and it no longer depends on how the type is written. Unqualified registrations keep compiling unchanged. Fixes #21 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! |
BRIDGE_REGISTER_MODEL/BRIDGE_REGISTER_ACTION keyed their generated anonymous-namespace registrar variables on __LINE__ to avoid pasting a namespace-qualified or template type into the identifier (issue #21). __LINE__ is only unique within a single physical file, though: two different headers that each invoke one of these macros on the same line number produce the same generated identifier once both are transitively #include'd into one translation unit, which is a hard redefinition error since C++ unnamed namespaces are per-TU, not per-file. This is exactly what broke the WASM demo build in CI (multiple bank model headers, each registering on the same line number, pulled into one autogen TU). Switch the key from __LINE__ to __COUNTER__, which increments monotonically across the whole translation unit regardless of which file expands it, so it cannot collide this way. Add tests/test_registration_same_line.cpp (with companion headers issue21_same_line_{a,b}.hpp, whose BRIDGE_REGISTER_MODEL invocations are deliberately pinned to the same physical line number) as a regression test; confirmed it fails with the redefinition error under __LINE__ and passes under __COUNTER__. Updated docs/spec/core/registry.md with the naming-scheme rationale. 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_ACTIONtoken-pasted the model/action type directly into the generated static registrar name, which only works for bare identifiers — a namespace-qualified type (e.g.app::models::Report) pastes:characters into the token and fails to compile.__LINE__instead (via the standard two-levelCAT/CAT_indirection macro), matching the fix suggested in the issue. There is exactly one registration per invocation, so__LINE__is equally unique, and unqualified registrations keep compiling unchanged.Test plan
tests/test_registration_qualified_types.cpp, registering a namespace-qualified model/action exactly as described in the issue; confirmed it fails to compile onmasterand compiles + passes with the fix../build/tests/morph_tests— all 812 test cases / 8286 assertions pass.Closes #21
🤖 Generated with Claude Code