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

From: Christian Mazakas (christian.mazakas_at_[hidden])
Date: 2025-04-30 22:58:00

Alright, I'm going to cut to the chase here and say that we should
emphatically accept Boost.OpenMethod.

I was actually quite surprised by the library when I tried it out. I've
learned about multi-methods somewhat in the past but I've never really had
a use for them in the applications I've written. That being said, their
applicability is obvious and this library makes it really, really nice to
do.

I think this library should be accepted into Boost because it's a very
strong return to form for us, in the sense that this library really
demonstrates how many language features you can emulate via a library which
has always been a spot Boost has occupied. In the ideal world, this would
have language support but this library does a good job of showing us what
that hypothetical world would be like.

Plus, the author did their due diligence and has reported overhead, even
down to estimated cycles. I didn't verify anything on my actual machine but
if we assume the author is speaking in good faith, I think the library is
acceptable from a performance perspective and this doesn't need to be a
point of contention.

I don't expect this library to be used in production or adopted overnight
by companies. But I think it still belongs in Boost because it's
interesting and it's a really solid solution to a common problem. I'd put
this library in line with other experimental libraries like Yap, HOF,
Spirit, Lambda2, etc. yomm2, the library this is derived from, seems
decently popular on Github and it has contributions from others which is a
good point in its favor. So, maybe companies are using it. Technically,
I've shipped Spirit code but I don't expect Spirit to be a popular choice
at many companies where I've been.

I do have technical feedback.

First and foremost, I'd say this library can be dramatically pared down in
its interface. What I mean by this, I'm left wondering if we could replace
a lot of the customization points with just a set of common choices for
RTTI and error handling. For example:

    struct dynamic_policy
        : boost::openmethod::default_policy::fork<dynamic_policy>::replace<
              boost::openmethod::policies::extern_vptr,
              boost::openmethod::policies::vptr_vector<
                  dynamic_policy,
boost::openmethod::policies::indirect_vptr>> {};

Committing this code to any project I've worked on would've left most of my
coworkers absolutely checked out.

And because this policy is used in the indirect_vptr example, showcasing
how to handle dynamic loading, I was left wondering, shouldn't the library
just be exposing this stuff already for me? Why is there an example
teaching me how to build it? dyn_vptr seems important enough that I
shouldn't have to build it.

This applies similarly to the RTTI and error handling stuff. Why do I need
custom RTTI? Why would I want custom RTTI? What's the library not doing for
me that I should be considering doing myself?

As far as error handling goes,
> "When an error is encountered, the program is terminated by a call to
abort"

This absolutely should be changed, even if it did make me chuckle at first.
It's because "abort" is a scary word and I think most readers would drop
the library instantly if they read that "when an error occurs, we abort
your program". I realize now that this library means: "we internally use
asserts".

But I still think this is a bad default because the example code in the
Error Handling section immediately shows you how to author the thing you
actually want: exceptions. This is where the complexity of the library
ramps up a lot as now you're introduced to facets and policies.

I think there's maybe a more sane path to error handling here which is to
use exceptions by default and then maybe something like an assert version
which works in debug or release and then finally a "we just do UB". Kind of
like an invalid static_cast<> vs dynamic_cast<>. I don't know how viable
these approaches are but I think there's a more sane default we can take
and I really don't think this needs to be offered so prominently in the
interfaces.

The use of trailing return seems to really mess up the asciidoc code
examples, especially because it's for stuff like `auto main() -> int` which
is tough to defend in and of itself, even as someone who's a huge trailing
return fan.

I had a lot of trouble trying to actually construct a virtual_ptr myself
and the reference docs were not helpful because they don't have any
examples.

The Overview section probably belongs at the top.

Do we need BOOST_OPENMETHOD_OVERRIDER? It seems like in the code examples,
just calling `next()` is sufficient. This code is already so entrenched in
macros and I'm not sure another one helps its case.

I'm not sure about BOOST_OPENMETHOD_GUIDE either. It seemed like it was
needed so we can add overriding functions to types in a different namespace
without opening it. I think this is actually something we don't want to
encourage for reasons mentioned by Andrzej about this being a potential
footgun. I think forcing users to have to open up the namespace is a good
thing and kind of mimics the orphan rule in Rust, which is: you own either
the trait or the type.

Otherwise, I think the library is very enjoyable. I think a lot of the
stuff it offers seems extraneous on first glance and I'd much rather have
the library just offer me a nice set of tools to pick and choose from
instead of me "getting to" build my own solution. But overall, a really
solid approach to a ubiquitous problem and I'd love to see how this library
evolves in Boost.

- Christian