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

From: Jean-Louis Leroy (jl_at_[hidden])
Date: 2025-05-06 23:37:38

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