Re: [boost] [OpenMethod] review starts on 28th of April
<CC-ing the mailing list, since you just replied me, and answering inline> On Tue, 6 May 2025 at 01:35, Jean-Louis Leroy <jl@leroy.nyc> wrote:
Your example is very similar to https://jll63.github.io/Boost.OpenMethod/#tutorials_custom_rtti.
virtual_ptr does not try to use boost_openmethod_vptr in its constructors. That is an oversight. It is a recent invention. YOMM2 tests for the presence of a boost_openmethod_vptr instance variable in the object directly. In that context, the function is very simple and inlinable, but if it is user-defined, there is no reason to assume that. So I will make virtual_ptr consider it as well.
Regardless, the easiest way to handle your example is to put the vptr acquisition in the policy. dynamic_type returns a type_id, not a vptr. The function for that is dynamic_vptr. It is not part of a facet, eventhough facets can provide one. The dispatch mechanism is described here: https://jll63.github.io/Boost.OpenMethod/#ref_description_17
WARNING: if you try that code with the "review" branch, make sure to pull the latest changes, because they fix a bug I introduced while re-establishing checks for non-polymorphic classes.
Thus:
// ------------------------ // you
#include <cstdint>
enum class kind : std::uintptr_t { unknown, n1, n2 };
class base_node { kind k_;
protected: base_node(kind k) noexcept : k_(k) { }
public: kind getKind() const { return k_; } };
class node1 : public base_node { public: node1() noexcept : base_node(kind::n1) { } };
class node2 : public base_node { public: node2() noexcept : base_node(kind::n2) { } };
// ------------------------ // me
// Get kind from static type, as you RTTI system does not provide it as a // static member.
template<typename T> struct kind_of { static constexpr kind value = kind::unknown; };
template<> struct kind_of<node1> { static constexpr kind value = kind::n1; };
template<> struct kind_of<node2> { static constexpr kind value = kind::n2; };
#include <boost/openmethod/policies/basic_policy.hpp>
struct custom_rtti : boost::openmethod::policies::rtti { template<class Node> static constexpr bool is_polymorphic = std::is_base_of_v<base_node, Node>;
using type_id = boost::openmethod::type_id;
template<typename T> static auto static_type() { return type_id(kind_of<T>::value); }
template<typename T> static auto dynamic_type(const T& obj) { if constexpr (is_polymorphic<T>) { return type_id(obj.getKind()); } else { return kind::unknown; } } };
struct custom_policy : boost::openmethod::policies::basic_policy<custom_policy, custom_rtti> {
static auto dynamic_vptr(const base_node& b) { switch (b.getKind()) { case kind::n1: return custom_policy::static_vptr<node1>; case kind::n2: return custom_policy::static_vptr<node2>; default: return custom_policy::static_vptr<base_node>; } } };
#define BOOST_OPENMETHOD_DEFAULT_POLICY custom_policy
#include <iostream>
#include <boost/openmethod.hpp> #include <boost/openmethod/compiler.hpp>
BOOST_OPENMETHOD(process, (std::ostream&, virtual_ptr<base_node>), void);
BOOST_OPENMETHOD_OVERRIDE( process, (std::ostream & os, virtual_ptr<node1> node1), void) { os << "process node1\n"; }
BOOST_OPENMETHOD_OVERRIDE( process, (std::ostream & os, virtual_ptr<node2> node2), void) { os << "process node2\n"; }
BOOST_OPENMETHOD_CLASSES(base_node, node1, node2);
auto main() -> int { boost::openmethod::initialize();
auto a = std::make_unique<node1>(); auto b = std::make_unique<node2>();
process(std::cout, *a); process(std::cout, *b);
return 0; }
We could also take advantage of the fact that type_ids fall in a short, compact range. We can use the vptr_vector facet, but there is no need to hash the type_ids, we can use them as straight indexes. So we don't use a type_hash facet:
struct custom_policy : boost::openmethod::policies::basic_policy< custom_policy, custom_rtti, boost::openmethod::policies::vptr_vector<custom_policy>> {};
This time dynamic_vptr() is provided by a facet. Everything else stays the same.
For the reference, this is what I ended up doing: enum class kind : std::uintptr_t { unknown, n1, n2 }; class base_node { kind k_; protected: base_node(kind k) noexcept : k_(k) { } public: kind getKind() const { return k_; } }; class node1 : public base_node { public: static constexpr kind node_kind = kind::n1; node1() noexcept : base_node(node_kind) { } }; class node2 : public base_node { public: static constexpr kind node_kind = kind::n2; node2() noexcept : base_node(node_kind) { } }; constexpr const char* kind_to_string(kind k) { switch (k) { case kind::n1: return "node1"; case kind::n2: return "node2"; default: return "<unknown node type>"; } } struct custom_rtti : boost::openmethod::policies::rtti { template<class T> static constexpr bool is_polymorphic = std::is_base_of_v<base_node, T>; template<typename T> static auto static_type() -> std::uintptr_t { if constexpr ( std::is_base_of_v<base_node, T> && !std::is_same_v<base_node, T>) { return static_cast<std::uintptr_t>(T::node_kind); } else { return static_cast<std::uintptr_t>(kind::unknown); } } template<typename T> static auto dynamic_type(const T& obj) -> std::uintptr_t { if constexpr (is_polymorphic<T>) { return static_cast<std::uintptr_t>(obj.getKind()); } else { return static_cast<std::uintptr_t>(kind::unknown); } } template<class Stream> static void type_name(std::uintptr_t type, Stream& stream) { stream << kind_to_string(static_cast<kind>(type)); } }; struct custom_policy : boost::openmethod::policies::basic_policy< custom_policy, custom_rtti, boost::openmethod::policies::runtime_checks, boost::openmethod::policies::basic_error_output<custom_policy>, boost::openmethod::policies::vptr_vector<custom_policy>> {}; BOOST_OPENMETHOD_CLASSES(base_node, node1, node2, custom_policy) BOOST_OPENMETHOD(print, (virtual_<base_node>), void, custom_policy); BOOST_OPENMETHOD_OVERRIDE(print, (base_node&), void) { /* whatever */ } I've just realized that I've disabled hashing by accident, and this happened to work by accident. I think that this implicit dependency between the vptr_vector and the hashing policy is a problem - I prefer explicit dependencies.
As you see in this example, we don't have to fork policies, we can just construct them from scratch. Forking was introduced with the idea of tuning default_policy, without having to replicate it from scratch. Also the with/without setters are less verbose than the add/replace/remove ones.
Elsewhere I said that facets categories are a closed set. That is not correct. The dispatcher and the compiler look only at a closed set of facets (using if constexpr (Policy::has_facet>)). But some facets, like vptr_vector and error_handler, look for other facets in the policy, which are not necessarily referenced directly by the compiler or the dispatcher. In that sense, the set of facets is actually open.
I am going to think about your suggested alternative to the facet system. Seriously, thus it will take me a few days. In the meantime, how would you see the two example policies above, expressed with your system?
This is what it'd look like with my proposal: struct custom_rtti : openmethod::policies::rtti { template<class T> static constexpr bool is_polymorphic = std::is_base_of_v<base_node, T>; template<typename T> auto static_type() const -> std::uintptr_t { /* same definition */ } template<typename T> auto dynamic_type(const T& obj) const -> std::uintptr_t { /* same definition */ } template<class Stream> void type_name(std::uintptr_t type, Stream& stream) const { /* same definition */ } }; struct custom_policy : boost::openmethod::policies::abstract_policy { // enable runtime checks openmethod::policies::runtime_checks runtime_checks; // enable the default error output openmethod::policies::basic_error_output<> error_output; // pass the hashing policy to vptr_vector explicitly, rather than implicitly via the entire Policy // disable_hashing should be either a marker type to tell vptr_vector "I don't have any hashing available" // or implement the type_hash facet and be a no-op (I guess the 1st option is better) openmethod::policies::vptr_vector<openmethod::policies::disable_hashing> extern_vptr; }; // Now register the classes. See below for the definition of registry BOOST_OPENMETHOD_CLASSES(base_node, node1, node2, openmethod::registry<custom_policy>) BOOST_OPENMETHOD(print, (virtual_<base_node>), void, openmethod::registry<custom_policy>); BOOST_OPENMETHOD_OVERRIDE(print, (base_node&), void) { /* whatever */ } // This type needs to be provided by the library, and is in charge of creating the required static storage namespace boost::openmethod { template <class Policy> struct registry { using policy_type = Policy; static Policy policy; }; } There are likely things I'm not seeing now that will probably complicate this. If the review is favorable, I can write a PR with the proposed changes and see if it's viable.
I would love to ship a fully functional clang RTTI policy right off the bat; it is an important use-case.
J-L
Thanks, Ruben.
I don't like CRTP either. I found it very hard to explain, and failed, obviously. That's a bad sign.
struct custom_policy : boost::openmethod::policies::abstract_policy { // enable runtime checks openmethod::policies::runtime_checks runtime_checks;
// enable the default error output openmethod::policies::basic_error_output<> error_output;
// pass the hashing policy to vptr_vector explicitly, rather than implicitly via the entire Policy // disable_hashing should be either a marker type to tell vptr_vector "I don't have any hashing available" // or implement the type_hash facet and be a no-op (I guess the 1st option is better) openmethod::policies::vptr_vector<openmethod::policies::disable_hashing> extern_vptr; };
Rather than disabling hashing, I would probably have a vptr_vector and a vptr_hash_table<hash_function>, but this is a detail. The problem is that vptr_vector, vptr_hash_table, and vptr_map all need to know whether we are doing runtime checks; whether and where we are writing diagnostics on error; whether and where we are writing trace. Take vptr_hash_table: - It creates a maps from type_ids to vptrs and finds the hash factors. If it cannot find them, and if there is a error_output facet, it uses it to print a diagnostic. If it finds the factors, and there is a trace_output facet, it uses it to print the hash factors and how many attempts were needed. - If we are doing runtime checks, it also creates a second hash table (`control`) that maps type_ids to type_ids. What for? When acquiring a vptr, it checks if `control[id] == id`. If it is not the case, user forgot to register the class. Then it prints a diagnostic if the error_output facet is present. So vptr_hash_table needs to be a template like this: template< bool RuntimeChecks = false, class ErrorOutput = void, class TraceOutput = void> class vptr_hash_table {...}; But an instance of vptr_hash_table also needs to know which ErrorOutput and which TraceOutput to write to. It needs constructors. The ctor depends on the value of the template arguments. OK, we can manage that with enable_if (and some day with concepts): template< bool RuntimeChecks = false, class ErrorOutput = void, class TraceOutput = void> class vptr_hash_table { public: template< typename = std::enable_if< std::is_same_v<ErrorOutput, void>> && std::is_same_v<TraceOutput, void>> > vptr_hash_table() {...} template< typename = std::enable_if< !std::is_same_v<ErrorOutput, void>> && std::is_same_v<TraceOutput, void>> > vptr_hash_table(ErrorOutput& error_output) {...} // ditto for other combinations of ErrorOutput and TraceOutput }; When writing a policy, the constructors need to be called: struct debug_policy { // not this: // openmethod::policies::runtime_checks runtime_checks; // facets cannot see it anyway basic_error_output<> error_output; basic_trace_output<> trace_output; vptr_hash_table<true, basic_error_output<>, basic_trace_output<>> hash_table{error_output, trace_output}; // etc }; So...it is manageable, but not as simple as it looks at first sight. Unless I am making a mistake? What do you think? In the meantime, I had another idea: use CRTP but hide it. Make facets meta-functions. Something like: struct runtime_checks { template<class Policy> using fn = runtime_checks; }; template<class Stream = detail::ostderr> struct basic_error_output { template<class Policy> struct fn : basic_error_output { ... }; }; struct error_handler {}; struct vectored_error_handler { template<class Policy> struct fn : error_handler { ... }; }; template<class Hash> struct vptr_hash_table { template<class Policy> struct fn : vptr_hash_table { ... template<class Class> static auto dynamic_vptr(const Class& arg) { auto index = Hash::hash_type_id(Policy::dynamic_type(arg)); if constexpr (Policy::template has_facet<runtime_checks>) { if (index == (std::numeric_limits<std::size_t>::max)()) { if constexpr ( Policy::template has_facet<error_handler>) { unknown_class_error error; error.type = type; Policy::error(error); } abort(); } } } }; }; template<class Policy, class... Facet> struct basic_policy : Facet::template fn<Policy>... {}; struct debug : basic_policy< vptr_hash_table, runtime_checks, basic_error_output<>> { ... }; struct debug_using_std_ostream : basic_policy< vptr_hash_table, runtime_checks, basic_error_output<std::ostream>> { ... }; I can also make the with/without mechanism (which replaces add/replace/remove in the "review" branch) work with this, for those who want to use it: struct my_policy : default_policy::with<custom_rtti, vptr_vector> { ... } J-L
struct debug : basic_policy< vptr_hash_table, runtime_checks, basic_error_output<>> { ... };
Alas a bit of CRTP is needed. The above must read: struct debug : basic_policy< debug, vptr_hash_table, runtime_checks, basic_error_output<>> { ... }; Still better though... On Tue, May 6, 2025 at 7:37 PM Jean-Louis Leroy <jl@leroy.nyc> wrote:
I don't like CRTP either. I found it very hard to explain, and failed, obviously. That's a bad sign.
struct custom_policy : boost::openmethod::policies::abstract_policy { // enable runtime checks openmethod::policies::runtime_checks runtime_checks;
// enable the default error output openmethod::policies::basic_error_output<> error_output;
// pass the hashing policy to vptr_vector explicitly, rather than implicitly via the entire Policy // disable_hashing should be either a marker type to tell vptr_vector "I don't have any hashing available" // or implement the type_hash facet and be a no-op (I guess the 1st option is better) openmethod::policies::vptr_vector<openmethod::policies::disable_hashing> extern_vptr; };
Rather than disabling hashing, I would probably have a vptr_vector and a vptr_hash_table<hash_function>, but this is a detail. The problem is that vptr_vector, vptr_hash_table, and vptr_map all need to know whether we are doing runtime checks; whether and where we are writing diagnostics on error; whether and where we are writing trace.
Take vptr_hash_table: - It creates a maps from type_ids to vptrs and finds the hash factors. If it cannot find them, and if there is a error_output facet, it uses it to print a diagnostic. If it finds the factors, and there is a trace_output facet, it uses it to print the hash factors and how many attempts were needed. - If we are doing runtime checks, it also creates a second hash table (`control`) that maps type_ids to type_ids. What for? When acquiring a vptr, it checks if `control[id] == id`. If it is not the case, user forgot to register the class. Then it prints a diagnostic if the error_output facet is present.
So vptr_hash_table needs to be a template like this:
template< bool RuntimeChecks = false, class ErrorOutput = void, class TraceOutput = void> class vptr_hash_table {...};
But an instance of vptr_hash_table also needs to know which ErrorOutput and which TraceOutput to write to. It needs constructors. The ctor depends on the value of the template arguments. OK, we can manage that with enable_if (and some day with concepts):
template< bool RuntimeChecks = false, class ErrorOutput = void, class TraceOutput = void> class vptr_hash_table { public: template< typename = std::enable_if< std::is_same_v<ErrorOutput, void>> && std::is_same_v<TraceOutput, void>> > vptr_hash_table() {...}
template< typename = std::enable_if< !std::is_same_v<ErrorOutput, void>> && std::is_same_v<TraceOutput, void>> > vptr_hash_table(ErrorOutput& error_output) {...}
// ditto for other combinations of ErrorOutput and TraceOutput };
When writing a policy, the constructors need to be called:
struct debug_policy { // not this: // openmethod::policies::runtime_checks runtime_checks; // facets cannot see it anyway
basic_error_output<> error_output; basic_trace_output<> trace_output; vptr_hash_table<true, basic_error_output<>, basic_trace_output<>> hash_table{error_output, trace_output}; // etc };
So...it is manageable, but not as simple as it looks at first sight. Unless I am making a mistake? What do you think?
In the meantime, I had another idea: use CRTP but hide it. Make facets meta-functions. Something like:
struct runtime_checks { template<class Policy> using fn = runtime_checks; };
template<class Stream = detail::ostderr> struct basic_error_output { template<class Policy> struct fn : basic_error_output { ... }; };
struct error_handler {};
struct vectored_error_handler { template<class Policy> struct fn : error_handler { ... }; };
template<class Hash> struct vptr_hash_table { template<class Policy> struct fn : vptr_hash_table { ...
template<class Class> static auto dynamic_vptr(const Class& arg) { auto index = Hash::hash_type_id(Policy::dynamic_type(arg));
if constexpr (Policy::template has_facet<runtime_checks>) { if (index == (std::numeric_limits<std::size_t>::max)()) { if constexpr ( Policy::template has_facet<error_handler>) { unknown_class_error error; error.type = type; Policy::error(error); }
abort(); } } } }; };
template<class Policy, class... Facet> struct basic_policy : Facet::template fn<Policy>... {};
struct debug : basic_policy< vptr_hash_table, runtime_checks, basic_error_output<>> { ... };
struct debug_using_std_ostream : basic_policy< vptr_hash_table, runtime_checks, basic_error_output<std::ostream>> { ... };
I can also make the with/without mechanism (which replaces add/replace/remove in the "review" branch) work with this, for those who want to use it:
struct my_policy : default_policy::with<custom_rtti, vptr_vector> { ... }
J-L
On Wed, 7 May 2025 at 01:38, Jean-Louis Leroy via Boost <boost@lists.boost.org> wrote:
I don't like CRTP either. I found it very hard to explain, and failed, obviously. That's a bad sign.
struct custom_policy : boost::openmethod::policies::abstract_policy { // enable runtime checks openmethod::policies::runtime_checks runtime_checks;
// enable the default error output openmethod::policies::basic_error_output<> error_output;
// pass the hashing policy to vptr_vector explicitly, rather than implicitly via the entire Policy // disable_hashing should be either a marker type to tell vptr_vector "I don't have any hashing available" // or implement the type_hash facet and be a no-op (I guess the 1st option is better) openmethod::policies::vptr_vector<openmethod::policies::disable_hashing> extern_vptr; };
Rather than disabling hashing, I would probably have a vptr_vector and a vptr_hash_table<hash_function>, but this is a detail. The problem is that vptr_vector, vptr_hash_table, and vptr_map all need to know whether we are doing runtime checks; whether and where we are writing diagnostics on error; whether and where we are writing trace.
Take vptr_hash_table: - It creates a maps from type_ids to vptrs and finds the hash factors. If it cannot find them, and if there is a error_output facet, it uses it to print a diagnostic. If it finds the factors, and there is a trace_output facet, it uses it to print the hash factors and how many attempts were needed. - If we are doing runtime checks, it also creates a second hash table (`control`) that maps type_ids to type_ids. What for? When acquiring a vptr, it checks if `control[id] == id`. If it is not the case, user forgot to register the class. Then it prints a diagnostic if the error_output facet is present.
So vptr_hash_table needs to be a template like this:
template< bool RuntimeChecks = false, class ErrorOutput = void, class TraceOutput = void> class vptr_hash_table {...};
But an instance of vptr_hash_table also needs to know which ErrorOutput and which TraceOutput to write to. It needs constructors. The ctor depends on the value of the template arguments. OK, we can manage that with enable_if (and some day with concepts):
template< bool RuntimeChecks = false, class ErrorOutput = void, class TraceOutput = void> class vptr_hash_table { public: template< typename = std::enable_if< std::is_same_v<ErrorOutput, void>> && std::is_same_v<TraceOutput, void>> > vptr_hash_table() {...}
template< typename = std::enable_if< !std::is_same_v<ErrorOutput, void>> && std::is_same_v<TraceOutput, void>> > vptr_hash_table(ErrorOutput& error_output) {...}
// ditto for other combinations of ErrorOutput and TraceOutput };
When writing a policy, the constructors need to be called:
struct debug_policy { // not this: // openmethod::policies::runtime_checks runtime_checks; // facets cannot see it anyway
basic_error_output<> error_output; basic_trace_output<> trace_output; vptr_hash_table<true, basic_error_output<>, basic_trace_output<>> hash_table{error_output, trace_output}; // etc };
So...it is manageable, but not as simple as it looks at first sight. Unless I am making a mistake? What do you think?
I see. Well, I think that making dependencies explicit is good. Now I know what things are affected by runtime checks and logging. Maybe we can combine error output and trace output into a single logger entity - it's a well-known concept used in many other places.
In the meantime, I had another idea: use CRTP but hide it. Make facets meta-functions. Something like:
struct runtime_checks { template<class Policy> using fn = runtime_checks; };
template<class Stream = detail::ostderr> struct basic_error_output { template<class Policy> struct fn : basic_error_output { ... }; };
struct error_handler {};
struct vectored_error_handler { template<class Policy> struct fn : error_handler { ... }; };
template<class Hash> struct vptr_hash_table { template<class Policy> struct fn : vptr_hash_table { ...
template<class Class> static auto dynamic_vptr(const Class& arg) { auto index = Hash::hash_type_id(Policy::dynamic_type(arg));
if constexpr (Policy::template has_facet<runtime_checks>) { if (index == (std::numeric_limits<std::size_t>::max)()) { if constexpr ( Policy::template has_facet<error_handler>) { unknown_class_error error; error.type = type; Policy::error(error); }
abort(); } } } }; };
template<class Policy, class... Facet> struct basic_policy : Facet::template fn<Policy>... {};
struct debug : basic_policy< vptr_hash_table, runtime_checks, basic_error_output<>> { ... };
struct debug_using_std_ostream : basic_policy< vptr_hash_table, runtime_checks, basic_error_output<std::ostream>> { ... };
I think this is similar in complexity to what we have today. I don't think that CRTP is the problem, but all the static state and implicit dependencies that lead to the use of CRTP. Regards, Ruben.
participants (2)
-
Jean-Louis Leroy -
Ruben Perez