register_classes: record every ancestor, follow no alias, scan once - #99
Open
jll63 wants to merge 3 commits into
Open
register_classes: record every ancestor, follow no alias, scan once#99jll63 wants to merge 3 commits into
jll63 wants to merge 3 commits into
Conversation
|
An automated preview of the documentation is available at https://99.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-09-05 14:48:16 UTC |
Fixes from the review of ebe8cf1 (register classes by reflection). The records worked from a per-registration view of the program: a class was recorded with its nearest registered ancestors among the classes *this* scan found, so a derived class and an intermediate base registered by different registrations - a library and a plugin, each scanning its own namespace - lost the edge between them, and the intermediate's overriders were silently skipped for the derived class. The n x n matrix and the nearest-ancestor reduction behind this also exhausted the default -fconstexpr-ops-limit on a chain of sixty classes. Each class is now recorded with every registered class above it - the shape initialize already consumes, deriving the direct bases itself - and an ancestor the scan does not declare, in a namespace it does not enter or a specialization of a class template (which members_of never yields), gets a record of its own. The scan itself: - never goes through an alias. A namespace alias to an enclosing namespace was an infinite recursion, one into a namespace under boost defeated the exclusion, and an aliased namespace was scanned shallower than the same one reached directly. A class an alias named before the scan reached its declaring scope was never walked into, and its nested classes were lost, depending on scan order; - does not ask is_complete_type of an aliased specialization of a class template: the question instantiates it, and a `using Edge = std::pair<Node, Node>` over a Node defined elsewhere made a C++17-valid translation unit fail to compile; - pushes the classes it declares without a search, and only those with bases. It was quadratic in the number of classes merely visible in the translation unit, and the default scan stopped compiling at about five hundred of them; - runs once per registration, not once per target registry; - treats repeated inheritance consistently: a class is registered under the bases it reaches unambiguously through public paths - Repeated : Left, Right, both deriving from Animal, is registered under both - where it was dropped or kept depending on the depth of the hierarchy, and the reference promised the former. A method's return type is no longer a root: std::ostream& armed the covariant-return check in initialize, which then aborted on an overrider returning std::ostringstream&. A covariant return type is registered like any other class, when it derives from a root or is listed, as in C++17. An incomplete listed class is a static_assert, not an exception thrown from constant evaluation that named no class. The explicit_class_registration policy is gone. All it did was make register_classes skip a registry, silently dropping the classes listed for it, and nothing in the tree exercised it: the error tests and snippets that carried it never call the macro, so they withhold their class under either standard as they did. Build: the CMake probe passed its -std flag through CMAKE_REQUIRED_FLAGS, which try_compile puts before the flag it derives from CMAKE_CXX_STANDARD (CMP0067), so any configure that set the standard failed with a false FATAL_ERROR. The reflection options are now directory-scoped, so the shared-library tests get them too, and those tests register by reflection like the rest of the suite, which puts the per-program inline registrar across module boundaries and under hidden visibility. BOOST_OPENMETHOD_EXPECT_REFLECTION makes a target that quietly falls back to C++17 fail to build. The CI reflection job builds test/dynamic_loading. Docs: the scan runs at the registrar's point of instantiation, the end of the translation unit on every current compiler, so "reflection sees only what precedes it" stated the opposite of what happens. The reason to put the macro last is the standard's: a selected class or method declared after the registrar makes the program ill-formed, no diagnostic required. The five-line @code block in the register_classes reference is restored. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JDLWpTRETW5kfEgK9b2fgF
The register_classes reference mentioned the requirement in its title suffix and in a sentence at the very end of a long description. A WARNING admonition now opens the description, on the class page and on the BOOST_OPENMETHOD_REGISTER_CLASSES page, which folds in its "expands to a no-op" paragraph. @attention was the first choice; MrDocs drops it silently, paragraph and all. CLAUDE.md's list of doc-comment markup traps gains that one. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JDLWpTRETW5kfEgK9b2fgF
Four small items the review listed below its cap: - CLAUDE.md's list of CMake options gains BOOST_OPENMETHOD_ENABLE_REFLECTION. - reflection_group::size mirrors the Size template parameter as a static constexpr member, instead of a data member the constructor set to the same value. - The three-line trailer pasted into 45 tests - the reflection registration and the comment explaining why it comes last - is one macro, BOOST_OPENMETHOD_TEST_REGISTER_CLASSES, in test_classes.hpp, which carries the explanation once. - The CMake reflection probe compiles config/has_reflection.cpp, the file b2's probe uses, instead of a second copy of it embedded as a string; as a static library, since the file has no main. The CMP0067 save-and-restore around the probe covers the target type too. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JDLWpTRETW5kfEgK9b2fgF
jll63
force-pushed
the
fix/reflection
branch
from
September 5, 2026 14:43
3387a0b to
e50e1e1
Compare
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.
(Written by Claude Code, on behalf of @jll63.)
Fixes from a review of #94 (register classes by reflection in C++26), plus two small follow-ups.
The records
register_classesrecorded each class with its nearest registered ancestors among the classes that one scan found. Two consequences:plugin::C : lib::B : core::Awas recorded asC -> A, and the overriders onBwere silently skipped forC.-fconstexpr-ops-limiton a chain of sixty classes, or two hundred classes under one root.Each class is now recorded with every registered class above it - the shape
initializealready consumes, deriving the direct bases itself - and an ancestor the scan does not declare (in a namespace it does not enter, or a specialization of a class template, whichmembers_ofnever yields) gets a record of its own. A registration stands on its own, whatever another one registers alongside it.The scan
boostdefeated the exclusion, and an aliased namespace was scanned shallower than the same one reached directly. A class an alias named before the scan reached its declaring scope was never walked into, and its nested classes were lost - depending on scan order.is_complete_typeof an aliased specialization of a class template. The question instantiates it, andusing Edge = std::pair<Node, Node>over aNodedefined elsewhere made a translation unit that is valid C++17 fail to compile.^^::scan stopped compiling at about five hundred of them. Declared classes are pushed without a search, and only those with base classes: a class with none can only be a root, and roots come from the methods and the class list.Repeated : Left, Right, both deriving fromAnimal, is registered under both - where it was dropped or kept depending on the depth of the hierarchy, and the reference promised the former.Roots and diagnostics
std::ostream&armed the covariant-return check ininitialize, which then aborted on an overrider returningstd::ostringstream&. A covariant return type is registered like any other class: when it derives from a root, or when it is listed, as in C++17.static_assert("a listed class must be complete"), not an exception thrown from constant evaluation that named no class.explicit_class_registrationis removedAll it did was make
register_classesskip a registry - silently dropping the classes listed for it - and nothing in the tree exercised it: the error tests and snippets that carried it never call the macro, so they withhold their class under either standard, as before.Build and CI
-stdflag throughCMAKE_REQUIRED_FLAGS, whichtry_compileputs before the flag it derives fromCMAKE_CXX_STANDARD(CMP0067): any configure that set the standard - a preset, a toolchain file, a consuming project - failed both candidates and stopped at a falseFATAL_ERROR. The probe now compilesconfig/has_reflection.cpp, the file b2 probes, with the standard variables taken out of its way.inlineregistrar across module boundaries and under hidden visibility.BOOST_OPENMETHOD_EXPECT_REFLECTIONmakes a target that quietly falls back to C++17 fail to build. The CI reflection job buildstest/dynamic_loading.Docs
register_classesandBOOST_OPENMETHOD_REGISTER_CLASSESreference pages open with a WARNING block stating the C++26 requirement. (@attentionis silently dropped by MrDocs; CLAUDE.md's list of doc-comment traps gains that.)register_classesreference is restored.Tests
New fixtures in
test_reflection.cppfor each of the above: namespace aliases, an alias scanned before the declaration, two registrations over disjoint namespaces, a return type instd, a specialization in a base list, an alias to a specialization over an incomplete class, and a chain of 60 / 200 classes under one root at the default constexpr budget. A compile-fail test for the incomplete listed class. The trailer pasted into 45 tests is one macro,BOOST_OPENMETHOD_TEST_REGISTER_CLASSES().Verified locally: the whole suite under GCC 16
-std=c++26 -freflection(151/151, warnings as errors, examples and snippets included), the C++17 suite with shared libraries (156/156), b2 withtoolset=gcc-16 cxxstd=26, and the rendered docs.🤖 Generated with Claude Code
https://claude.ai/code/session_01JDLWpTRETW5kfEgK9b2fgF