Re: [OpenMethod] review starts on 28th of April

From: Ruben Perez (rubenperez038_at_[hidden])
Date: 2025-05-07 16:27:28

On Wed, 7 May 2025 at 01:38, Jean-Louis Leroy via Boost
<boost_at_[hidden]> 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.