doc: forward-declare the registry instead of including preamble.hpp - #93
Open
jll63 wants to merge 5 commits into
Open
doc: forward-declare the registry instead of including preamble.hpp#93jll63 wants to merge 5 commits into
jll63 wants to merge 5 commits into
Conversation
Overriding BOOST_OPENMETHOD_DEFAULT_REGISTRY forced users to learn the header
layering: include a "pre-core" header (preamble.hpp or default_registry.hpp),
define the registry, #define the macro, then include <boost/openmethod.hpp>.
ref_headers.adoc codified that as a "Pre-Core Headers" section, and
custom_rtti.adoc told the reader not to include <boost/openmethod.hpp> yet.
None of it is necessary. Every use of the macro in the headers is a name-only
context - a default template argument, an alias, a deduction guide, or a member
typedef inside a template - so the registry can be forward-declared before the
include and defined after it, where <boost/openmethod.hpp> has already supplied
registry, default_registry, indirect_registry and the five stock policies:
struct my_registry;
#define BOOST_OPENMETHOD_DEFAULT_REGISTRY my_registry
#include <boost/openmethod.hpp>
#include <boost/openmethod/policies/vptr_map.hpp>
struct my_registry
: boost::openmethod::default_registry::with<
boost::openmethod::policies::vptr_map<>> {};
A registry the library provides needs no declaration at all: core.hpp includes
default_registry.hpp *before* it tests the macro, so #define plus the include is
the whole recipe. preamble.hpp and default_registry.hpp now appear nowhere
outside include/.
Three rules, all documented: the registry must be complete before the first
construct that instantiates it (a hard error otherwise, never silent); it must
name a class declared with the same class-key as the definition (a mismatch is
MSVC C4099, an error under /W4 /WX); and the name must be qualified if it could
also be found in namespace boost::openmethod - `registry` in particular, which
otherwise resolves to boost::openmethod::registry and fails with "missing
template arguments".
Migrated the 17 tests that override the default, plus test_policies.cpp.
test_capture_errors.hpp now includes <boost/openmethod.hpp>; test_core.cpp drops
a redundant preamble.hpp; dynamic_loading/registry.hpp loses its whole #ifndef
fallback, which only existed because the alias preceded the include.
The two custom_rtti step-by-step examples get a tag::setup[] region so
tag::policy[] no longer opens with an include. ref_headers.adoc renames
"Pre-Core Headers" to "Policy Headers" and demotes preamble.hpp and
default_registry.hpp to "Headers Included by Other Headers".
registries_and_policies.adoc becomes the canonical home of the recipe; its
reference to <boost/openmethod/registry.hpp>, a file that does not exist, is
fixed. shared_libraries.adoc's my_registry.hpp listing gains the #define and
include it was missing - as written it never actually overrode the default.
Removed doc/modules/ROOT/examples/deferred_custom_rtti.cpp: an orphan no page
included, whose virtual-inheritance and dynamic_cast_ref coverage
test_custom_rtti_virtual_base.cpp already provides.
Also dropped three stray "end::" tags with no opener, and the dead Quickbook
markers in examples/static_rtti.cpp.
Closes boostorg#90
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
An automated preview of the documentation is available at https://93.openmethod.prtest3.cppalliance.org/libs/openmethod/doc/html/index.html If more commits are pushed to the pull request, the docs will rebuild at the same URL. 2026-08-24 00:51:12 UTC |
The doc comment added for BOOST_OPENMETHOD_DEFAULT_REGISTRY ended a `//!` line with a backslash, making it a line continuation. GCC's -Wcomment (in -Wall) rejects that under -Werror, so every GCC job on both CIs failed while every clang, MSVC and Xcode job passed. Put the #define on one line. Verified by reproducing the exact diagnostic with the pre-fix header and confirming g++ -Wall -Wextra -Werror accepts the fixed one. Restore the coverage lost with deferred_custom_rtti.cpp. That example was the only place combining deferred_static_rtti with virtual bases and dynamic_cast_ref: test_custom_rtti_virtual_base.cpp has the virtual bases and the cast but derives from policies::rtti, and test_custom_rtti_deferred.cpp had neither. Add Bat and Owl, virtually derived, plus the dynamic_cast_ref the cast now needs - `requires_dynamic_cast` is true exactly when static_cast is ill-formed, so these overriders reach the policy hook. Confirmed by sabotaging dynamic_cast_ref and watching the test fail. Name core.hpp, not <boost/openmethod.hpp>, as the point where the macro is read. initialize.hpp, inplace_vptr.hpp and every interop/*.hpp also include core.hpp, so a TU that includes one of those before the #define binds silently to default_registry. Four places still said otherwise. test_capture_errors.hpp now owns the whole recipe - declaration, #define, include and test_registry - instead of the seven tests repeating it. There is no ordering left for a caller to get wrong, which is what the previous version's comment had claimed without being able to guarantee it. Those tests no longer name the macro, so test/CMakeLists.txt would have handed them the shared PCH and silently defeated the override; teach its scan to follow the header too. Documentation fixes from the same review: - ref_headers.adoc claimed every stock policy has its own header; indirect_vptr, runtime_checks and deferred_static_rtti live in preamble.hpp. Say so, and note runtime_checks joins default_registry under BOOST_OPENMETHOD_ENABLE_RUNTIME_CHECKS. - custom_rtti.adoc pointed at a declaration "at the top of the file" that sits below the classes listing, and its Deferred RTTI section never showed the declaration step at all - example 2's setup and registry tags were unrendered. - error_handling.adoc's listing opened with five lines of registry boilerplate its prose never introduced. - shared_libraries.adoc's my_registry.hpp is a complete header now, so give it the include guard the animals.hpp it is compared to has. - CLAUDE.md cited a line number that had already moved, pointed at greps in a section that has none, miscounted the files carrying the macro, and described an unqualified `registry` as resolving silently when it is a hard error. Remove doc/modules/ROOT/examples/static_rtti.cpp: an orphan no page references, left marker-free by the previous commit, duplicating snippets/static_rtti.cpp - which exercises the same policy, classes and dispatch, is the copy the docs render, and asserts more. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Finishes the deduplication started for the capture-diagnostics tests. The three test_class_registration_*.cpp files repeated the same six-line prologue and the same `default_registry::with<runtime_checks, throw_error_handler>` definition verbatim; test_checked_registry.hpp now owns the whole recipe, so including it first is all they do and there is no ordering left to get wrong. Teach the PCH scan in test/CMakeLists.txt about the new header, as with test_capture_errors.hpp: those files no longer name the macro, and a force-included PCH would precede the #define and silently bind the macros to default_registry. Verified from build.ninja that all three stay off the shared PCH, and that defeating the override makes the tests fail rather than pass quietly - they assert BOOST_CHECK_THROW(..., missing_class), which needs runtime_checks and throw_error_handler. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… fix/new-global-registry-override # Conflicts: # doc/modules/ROOT/examples/deferred_custom_rtti.cpp # doc/modules/ROOT/examples/static_rtti.cpp
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #93 +/- ##
===========================================
- Coverage 94.97% 92.94% -2.04%
===========================================
Files 99 22 -77
Lines 4322 1615 -2707
Branches 2138 500 -1638
===========================================
- Hits 4105 1501 -2604
+ Misses 160 66 -94
+ Partials 57 48 -9
... and 78 files with indirect coverage changes Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
The codecov report counted test/ alongside include/, so the only red check on this PR was three unreachable lines in test_custom_rtti_deferred.cpp: the cast_aux null return and the Bat/Owl delegation-to-base branches, which exist to make the custom cast well-formed for every pair but that no call reaches. Test code is not the thing the coverage target is measuring. The exclusion was already in the file, commented out, but spelled `test/**/*` as the boost-ci sample ships it. That spelling does not do what it looks like: codecov compiles it to (?s:test/.*/[^\/]*)\Z, which requires a second slash, so it matches only the subdirectories of test/ and leaves every top-level test/test_*.cpp counted - which is where nearly all the test code is. `test/**` compiles to (?s:test/.*)\Z and matches at any depth. Verified against the paths codecov actually reports, which are repo-relative despite boost-ci building inside the superproject. Comment records the trap and the validate endpoint. Co-Authored-By: Claude Opus 5 <noreply@anthropic.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
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.
Closes #90.
Overriding
BOOST_OPENMETHOD_DEFAULT_REGISTRYforced users to learn the header layering: include a "pre-core" header (preamble.hppordefault_registry.hpp), define the registry,#definethe macro, then include<boost/openmethod.hpp>.ref_headers.adoccodified that as a "Pre-Core Headers" section, andcustom_rtti.adoctold the reader not to include<boost/openmethod.hpp>yet.None of it is necessary. Every use of the macro in the headers is a name-only context — a default template argument, an alias, a deduction guide, or a member typedef inside a template — so the registry can be forward-declared before the include and defined after it:
core.hppincludesdefault_registry.hppbefore it tests the macro, so by the time the definition is reachedregistry,default_registry,indirect_registryand the five stock policies are all complete. A registry the library provides needs no declaration at all —#defineplus the include is the whole recipe.preamble.hppanddefault_registry.hppnow appear nowhere outsideinclude/.Rules, all documented
BOOST_OPENMETHOD*macro,use_classes,method,virtual_ptr,inplace_vptr_base,initialize(), or the registry-sharing macros). Violating this is a hard error, never silent./W4 /WX.namespace boost::openmethod.#define BOOST_OPENMETHOD_DEFAULT_REGISTRY registryresolves toboost::openmethod::registryand fails withmissing template arguments;::registryworks.What changed
BOOST_OPENMETHOD_DEFAULT_REGISTRYdoc comment with both recipe shapes; fixed four other "before includingcore.hpp" comments.test_policies.cpp.test_capture_errors.hppnow includes<boost/openmethod.hpp>;test_core.cppdrops a redundantpreamble.hpp;dynamic_loading/registry.hpploses its whole#ifndeffallback, which existed only because the alias preceded the include.custom_rttistep-by-step examples get atag::setup[]region sotag::policy[]no longer opens with an include.ref_headers.adocrenames "Pre-Core Headers" to "Policy Headers" and demotespreamble.hpp/default_registry.hppto "Headers Included by Other Headers".registries_and_policies.adocbecomes the canonical home of the recipe, and its reference to<boost/openmethod/registry.hpp>— a file that does not exist — is fixed.shared_libraries.adoc'smy_registry.hpplisting gains the#defineand include it was missing; as written it never actually overrode the default.Drive-by cleanups
Removed
doc/modules/ROOT/examples/deferred_custom_rtti.cpp: an orphan no page included, whose virtual-inheritance anddynamic_cast_refcoveragetest_custom_rtti_virtual_base.cppalready provides. Also dropped three strayend::tags with no opener and the dead Quickbook markers inexamples/static_rtti.cpp.Verification
-Werror: 149/149 pass.dynamic_loadingvariants (one supplies the macro on the command line) and bothimplicit_shared_librariesvariants./W4 /WX: the three declspec paths inimplicit_shared_libraries/custom_registryand the migrated tests compile clean. No C4099 — every declaration and definition usesstruct.Note for anyone with an existing build tree: the glob in
examples/CMakeLists.txthas noCONFIGURE_DEPENDS, so a manualcmakere-run is needed to drop the deleted example's target.🤖 Generated with Claude Code