From 6a3e4f13d43db42b710ee005343f54f293b044fd Mon Sep 17 00:00:00 2001 From: Jean-Louis Leroy Date: Sat, 10 Oct 2026 12:30:14 -0400 Subject: [PATCH 1/3] doc: give non-polymorphic types distinct ids in the custom RTTI examples Both examples returned `reinterpret_cast(0)` for every type that does not derive from `Node`. That looks harmless - such types never take part in dispatch - but `static_type` is also how a *method* is identified, and a `method<...>` specialization is not derived from `Node`, so it takes that branch too. `augment_methods` keys `method_map` on the method's type id, so every method in the registry lands under the same key: they merge into one, their overriders pool together, nothing dominates, and the first call reaches `fn_ambiguous`. Neither example's registry has an `error_handler` or an `output` policy, so it aborts with no message at all. Each example declares exactly one method, so nothing merges today and the examples work. It bites the reader who follows the page and adds a second method, which is the obvious next step. Demonstrated by adding a `depth` method to each example: before, abort with exit 134; after, `2 3 + 4 * = 20 depth=3`. Non-polymorphic types now get the address of a function-local static, which is what the library's own `static_rtti` policy does, and `dynamic_type` delegates to `static_type` so the two cannot disagree. The page gains a paragraph on why the placeholder is a trap, and the sentence promising a "small, dense range" of ids is narrowed to the classes that take part in dispatch - which is what `vptr_vector` indexes. Every other custom `static_type` in the tree - the `static_rtti` and `named_static_rtti` policies, and the five tests that define one - already gives non-polymorphic types distinct ids. Only the examples were wrong, which is why no test caught it. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_011Eo4q4cYTx457W7oaunqsQ --- .../examples/custom_rtti/1/custom_rtti.cpp | 6 ++++-- .../examples/custom_rtti/2/custom_rtti.cpp | 6 ++++-- doc/modules/ROOT/pages/custom_rtti.adoc | 19 +++++++++++++++---- 3 files changed, 23 insertions(+), 8 deletions(-) diff --git a/doc/modules/ROOT/examples/custom_rtti/1/custom_rtti.cpp b/doc/modules/ROOT/examples/custom_rtti/1/custom_rtti.cpp index 163efd002..7965e68de 100644 --- a/doc/modules/ROOT/examples/custom_rtti/1/custom_rtti.cpp +++ b/doc/modules/ROOT/examples/custom_rtti/1/custom_rtti.cpp @@ -61,7 +61,9 @@ struct custom_rtti : boost::openmethod::policies::rtti { if constexpr (is_polymorphic) { return reinterpret_cast(T::static_type); } else { - return reinterpret_cast(0); + static const char id = 0; + + return reinterpret_cast(&id); } } @@ -70,7 +72,7 @@ struct custom_rtti : boost::openmethod::policies::rtti { if constexpr (is_polymorphic) { return reinterpret_cast(obj.type); } else { - return reinterpret_cast(0); + return static_type(); } } }; diff --git a/doc/modules/ROOT/examples/custom_rtti/2/custom_rtti.cpp b/doc/modules/ROOT/examples/custom_rtti/2/custom_rtti.cpp index 0c154421c..1dc838450 100644 --- a/doc/modules/ROOT/examples/custom_rtti/2/custom_rtti.cpp +++ b/doc/modules/ROOT/examples/custom_rtti/2/custom_rtti.cpp @@ -57,7 +57,9 @@ struct custom_rtti : boost::openmethod::policies::deferred_static_rtti { if constexpr (is_polymorphic) { return reinterpret_cast(T::static_type); } else { - return reinterpret_cast(0); + static const char id = 0; + + return reinterpret_cast(&id); } } @@ -66,7 +68,7 @@ struct custom_rtti : boost::openmethod::policies::deferred_static_rtti { if constexpr (is_polymorphic) { return reinterpret_cast(obj.type); } else { - return reinterpret_cast(0); + return static_type(); } } }; diff --git a/doc/modules/ROOT/pages/custom_rtti.adoc b/doc/modules/ROOT/pages/custom_rtti.adoc index 14f915863..cfd0640df 100644 --- a/doc/modules/ROOT/pages/custom_rtti.adoc +++ b/doc/modules/ROOT/pages/custom_rtti.adoc @@ -116,19 +116,30 @@ not require Node to be polymorphic in the C++ sense. If we remove the virtual destructor and implement `value` as an open-method as well, the program will still work. +Note that a non-"polymorphic" type still gets an id of its own - the address of +a function-local static, as in the library's own cpp:policies::static_rtti[]. +It is tempting to return a single placeholder for all of them, since they never +take part in dispatch, but `static_type` is also what identifies a *method*: a +`method<...>` specialization is not derived from `Node`, so it takes the +non-"polymorphic" branch. Hand every method the same id and cpp:initialize[] +merges them into one, pools their overriders, and the first call fails as an +ambiguous call. With a single method in the program nothing goes wrong, which +is what makes the mistake easy to ship. + The policy is quite minimal. It does not support virtual inheritance, because it does not provide a `dynamic_cast_ref` function. It would not produce good error or trace messages, because it does not provide a `type_name` function. Instead, it relies on the `type_name` inherited from cpp:rtti::defaults[]. It -renders types as adorned integers, e.g. "type_id(2)". All non-"polymorphic" -types would be rendered the same way, as "type_id(0)". +renders types as adorned integers, e.g. "type_id(2)" for a node, and an +unwieldy address for everything else. cpp:rtti::defaults[] also provides a default implementation for `type_index`, which simply returns its argument. Now we need a policy to get a v-table pointer from an object. Our RTTI system -has an interesting property: its type ids are monotonically allocated in a -small, dense range. It means that we can use them as indexes in a vector. +has an interesting property: the ids it gives to the classes that take part in +dispatch are monotonically allocated in a small, dense range. It means that we +can use them as indexes in a vector. `vptr_vector` is perfect for that. So here is our registry: [source,c++] From 19e77ed0c955f40ebd2fd62c69d958fbca618830 Mon Sep 17 00:00:00 2001 From: Jean-Louis Leroy Date: Sat, 10 Oct 2026 12:42:03 -0400 Subject: [PATCH 2/3] doc: drop the unused stream parameter in the second custom RTTI example `call_via_ref` was copied from the first example, where `postfix` writes to the stream. The second example evaluates instead - the body is `return value(node)` - so `os` has been unused ever since, and the example does not compile under -Wall -Wextra -Werror: custom_rtti.cpp:118:51: error: unused parameter 'os' [-Werror=unused-parameter] The examples are not built with -Wextra, which is why it went unnoticed. The function sits outside every doc tag, so nothing renders it; `performance.adoc` discusses a `call_via_ref` of its own, written inline and taking the stream, and the first example keeps its two-parameter version. Both examples now compile clean at -Wall -Wextra -Werror. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_011Eo4q4cYTx457W7oaunqsQ --- doc/modules/ROOT/examples/custom_rtti/2/custom_rtti.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/modules/ROOT/examples/custom_rtti/2/custom_rtti.cpp b/doc/modules/ROOT/examples/custom_rtti/2/custom_rtti.cpp index 1dc838450..72d100b02 100644 --- a/doc/modules/ROOT/examples/custom_rtti/2/custom_rtti.cpp +++ b/doc/modules/ROOT/examples/custom_rtti/2/custom_rtti.cpp @@ -115,6 +115,6 @@ int main() { std::cout << value(e) << "\n"; // 2 3 + 4 * = 20 } -auto call_via_ref(const Node& node, std::ostream& os) { +auto call_via_ref(const Node& node) { return value(node); } From 997e6aa63570f404efff7eefb15abddbfd7ff8e2 Mon Sep 17 00:00:00 2001 From: Jean-Louis Leroy Date: Sat, 10 Oct 2026 12:52:08 -0400 Subject: [PATCH 3/3] build: compile the examples with -Wall -Wextra -Werror There is no Jamfile under doc/modules/ROOT/examples, so b2 never compiles the examples, and the CMake build - the only check they get - was passing them no warning flags at all. Mirror test/CMakeLists.txt: the flags go on under BOOST_OPENMETHOD_WARNINGS_AS_ERRORS, which the C++26 reflection job sets, and that job builds the examples because it configures the library as the root project. Add -Wextra, which the test flags lack. The block sits before the first target, so the options reach shared_libs and the step-by-step subdirectories too. Three latent defects came out, two of them only under one compiler: - A dead `double s` in both matrix examples (-Wunused-variable, in -Wall, invisible only because the examples had no flags), left over from a main() that exercised the scalar overloads. Rather than drop the variable, call the methods the examples declare: matrix * matrix (the dense/diagonal catch-all and the diagonal/diagonal override), scalar * matrix, and matrix * scalar. The examples now demonstrate the multiple dispatch they set up. - Three `expr` bindings shadowing one another along the dynamic_cast chain in slides.cpp (MSVC C4456; GCC and clang want -Wshadow, which is in neither -Wall nor -Wextra). Named for the type each one casts to, which reads better anyway. - GCC 16 reports -Wsfinae-incomplete on the friendship examples, which name virtual_ptr in a friend declaration inside Animal, before Cat is defined - the point of the example. Defaulting the registry argument runs the affinity probe, a SFINAE context. The library asks the question again under its `rechecked` tag, where the class has to be complete, and refuses an answer that has changed, so the warning is kept visible but non-fatal. It is on by default - neither -Wall nor -Wextra brings it in - so this is pre-existing, and already in the reflection job's log; only -Werror makes it fatal. Probed rather than tied to a version: only GCC 16 knows the option, and naming it on GCC 13, GCC 15 or clang is a hard error. Verified on GCC 13 (Release), GCC 16 with -freflection (the reflection job's configuration, Debug + shared libs, 203/203 tests), clang 22, and MSVC v18 /W4 /WX over all 31 examples. /we4265, which the test flags also carry, is left off: it fires in preamble.hpp, not in the examples. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_011Eo4q4cYTx457W7oaunqsQ --- doc/modules/ROOT/examples/CMakeLists.txt | 31 +++++++++++++++++++ doc/modules/ROOT/examples/matrix.cpp | 4 +++ .../ROOT/examples/matrix_intrusive_vptr.cpp | 4 +++ doc/modules/ROOT/examples/slides.cpp | 12 +++---- 4 files changed, 45 insertions(+), 6 deletions(-) diff --git a/doc/modules/ROOT/examples/CMakeLists.txt b/doc/modules/ROOT/examples/CMakeLists.txt index f922ab9eb..ce8dc834b 100644 --- a/doc/modules/ROOT/examples/CMakeLists.txt +++ b/doc/modules/ROOT/examples/CMakeLists.txt @@ -15,6 +15,37 @@ if (CMAKE_BUILD_TYPE STREQUAL "Debug") add_compile_definitions(BOOST_OPENMETHOD_ENABLE_RUNTIME_CHECKS) endif() +# Hold the examples to the test warning level, plus -Wextra. There is no +# Jamfile here, so b2 never compiles the examples and this is the only check +# they get. Set before the first target, so the options also reach shared_libs +# and the step-by-step subdirectories. +if (BOOST_OPENMETHOD_WARNINGS_AS_ERRORS) + if (CMAKE_CXX_COMPILER_ID STREQUAL "MSVC" + OR CMAKE_CXX_COMPILER_FRONTEND_VARIANT MATCHES "MSVC") + add_compile_options(/W4 /WX) + else() + add_compile_options(-Wall -Wextra -Werror) + + # friendship.cpp and friendship_across_namespaces.cpp name + # virtual_ptr in a friend declaration inside Animal, before Cat is + # defined - which is the point of the example. Defaulting the registry + # argument runs the affinity probe, a SFINAE context, so GCC 16 reports + # that the answer may change once Cat is complete. The library asks the + # question again under its `rechecked` tag, where the class has to be + # complete, and refuses an answer that has changed. -Wsfinae-incomplete + # is on by default - neither -Wall nor -Wextra brings it in - so this + # keeps the diagnostic visible without failing the build. Probed rather + # than tied to a version: only GCC 16 knows the option, and naming it on + # any other compiler is a hard error. + include(CheckCXXCompilerFlag) + check_cxx_compiler_flag( + -Wsfinae-incomplete BOOST_OPENMETHOD_HAS_WSFINAE_INCOMPLETE) + if (BOOST_OPENMETHOD_HAS_WSFINAE_INCOMPLETE) + add_compile_options(-Wno-error=sfinae-incomplete) + endif() + endif() +endif() + file(GLOB cpp_files "*.cpp") foreach (cpp ${cpp_files}) diff --git a/doc/modules/ROOT/examples/matrix.cpp b/doc/modules/ROOT/examples/matrix.cpp index c6f656640..cfdd42f68 100644 --- a/doc/modules/ROOT/examples/matrix.cpp +++ b/doc/modules/ROOT/examples/matrix.cpp @@ -150,6 +150,10 @@ auto main() -> int { cout << a << "\n"; cout << b << "\n"; + cout << a * b << "\n"; + cout << b * b << "\n"; + cout << times(s, a.rep()) << "\n"; + cout << times(b.rep(), s) << "\n"; return 0; } diff --git a/doc/modules/ROOT/examples/matrix_intrusive_vptr.cpp b/doc/modules/ROOT/examples/matrix_intrusive_vptr.cpp index 50f9e9e03..68a915b25 100644 --- a/doc/modules/ROOT/examples/matrix_intrusive_vptr.cpp +++ b/doc/modules/ROOT/examples/matrix_intrusive_vptr.cpp @@ -145,6 +145,10 @@ auto main() -> int { cout << a << "\n"; cout << b << "\n"; + cout << a * b << "\n"; + cout << b * b << "\n"; + cout << times(s, a.rep()) << "\n"; + cout << times(b.rep(), s) << "\n"; return 0; } diff --git a/doc/modules/ROOT/examples/slides.cpp b/doc/modules/ROOT/examples/slides.cpp index a93ea53b9..c0526718c 100644 --- a/doc/modules/ROOT/examples/slides.cpp +++ b/doc/modules/ROOT/examples/slides.cpp @@ -60,12 +60,12 @@ namespace typeswitch { using namespace nodes; auto to_rpn(const Node& node) -> string { - if (auto expr = dynamic_cast(&node)) { - return to_string(expr->value()); - } else if (auto expr = dynamic_cast(&node)) { - return to_rpn(expr->left) + " " + to_rpn(expr->right) + " +"; - } else if (auto expr = dynamic_cast(&node)) { - return to_rpn(expr->left) + " " + to_rpn(expr->right) + " *"; + if (auto number = dynamic_cast(&node)) { + return to_string(number->value()); + } else if (auto plus = dynamic_cast(&node)) { + return to_rpn(plus->left) + " " + to_rpn(plus->right) + " +"; + } else if (auto times = dynamic_cast(&node)) { + return to_rpn(times->left) + " " + to_rpn(times->right) + " *"; } throw runtime_error("unknown node type"); }