[OpenMethod] Andrzej's review

From: Andrzej Krzemienski (akrzemi1_at_[hidden])
Date: 2025-05-06 22:40:38

Hi Everyone,
This is a review of the proposed Boost.OpenMethod library.

First of all, I want to thank Jean-Louis for writing and sharing this
library, for undergoing the Boost Review process, and for reviving this
interesting and important subject.

My recommendation is to *reject* the Boost.OpenMethod library at this time.

My decision is based on the design issues. I note that rejection is not a
permanent verdict, and there are known Boost libraries that were initially
rejected, and then were improved and passed the second review.

The library claims to solve two problems: the "expression problem" and the
"banana-gorilla-jungle problem". In my opinion, the first problem is not a
technical problem worth solving at all. It is more like a logical
challenge: can you provide a framework extensible in two dimensions? Maybe
I can, but so what? What real program or library benefits from this?

The second problem is real, and although the docs do not present it
adequately, during the discussion, and also via links to YOMM2
library, Jean-Louis provided a very compelling example of printing a Matrix
which is from a OO hierarchy.

I am personally not a fan of OO. I use very sparingly anything that has
`virtual` on it. For instance, for heavy matrix computations, I would never
think of using OO-hierarchies of matrices. It looks to me that OOP is
limited in a language without a garbage collection, and any attempts at
fixing it can only be limited.

The problem is real. I would call it differently: "I want a
runtime-polymorphic behavior, but I do not want to commit to all
polymorphic functions up front". The library provides a solution for it. It
is not necessarily a good solution, and it is for sure not the only
solution to the problem. Two other possible solutions:

1. Use std::variant and variant visitors. For a non-trivial motivating
example see https://akrzemi1.wordpress.com/2016/02/27/another-polymorphism/
2. Use OO-style class hierarchies with pattern-matching on top: either wait
for the C++ pattern matching, or use a library like Mach7 (
https://github.com/solodon4/Mach7)

The docs in OpenMethod are missing the discussion on these alternatives.
Especially alternative #2 seems to offer a superior solution, simpler, and
with fewer gotchas.

It is not clear to me what is the relation between Boost.OpenMethod and
N2216. Is the goal to implement N2216? Or is it just mentioned for context,
or as a loose inspiration? I hear the replies in the review discussions,
"but this is what N2216 said", also the docs say that the library
implements N2216 with extensions. But note that N2216 calls itself "a
report" and the authors never claimed that they had a solution suitable for
standardization. It was an exploration. But the solution in N2216 has one
important property: the program would never get a "runtime error"
indicating a bad usage of the feature: it would either be a compiler/linker
error or an arbitrary -- but *deterministic* -- selection of the overload.
N2216 allows no "pure virtual" open methods: it requires that you provide a
definition for the "top level" declaration. This way you always have an
implementation to fall back to. Also its solution has a consequence that a
different overrider can be chosen in different loaded DLLs for the same
combination of concrete types. This solution is counterintuitive to me and
looks similar to the ODR-violation. It is also possible that two DLLs
provide a different overrider for the same combination of types. But this
is not surprising. The problem of selecting "the right" overrider (whatever
it means, given the two-dimensional flexibility) is not solvable. N2216
makes this lack of a good solution very clear. It also shows how other
languages fail to solve it (for instance, requiring a "glue code" to be
provided by the person who assembles the final program, which defeats the
purpose of the "two dimensional flexibility" of open methods.

Anyway, the solution for the overrider selection in N2216 is very different
from the one in Boost.OpenMethod, so it is incorrect to say that
Boost.OpenMethod implements N2216. It does not. It chose a different
approach. In this different approach, you can get a runtime error. Worse
still, you can sometimes get a runtime error and sometimes an arbitrary
overrider, and it is difficult to remember when which happens. N2216 says
that a new linker technology would be required to guarantee
no-runtime-error. Boost.OpenMethod seems to lack it. As a consequence, we
are getting a feature with a lot of type-safety holes. They are plugged by
throwing exceptions and calling std::abort(), but they are still holes: you
cannot preemptively prevent them, because of the announced flexibility:
anyone can extend the type hierarchy or introduce new open-methods at
run-time, via linking a DLL. Boost.OpenMethod is inherently type-unsafe and
advertises this as flexibility.

It is a common practice for library functions to have *preconditions* and
to enforce them by calling std::abort() or throwing an exception, but this
works only because the users have the means of controlling the correct
usage of a function:

sqrt(x);

And I could have checked before the call that `x` is negative, or I could
have made sure that `x` is the result of summing squares. Because I am in
control. But in this call:

to_string(matrix);

I am not in control of satisfying a precondition, because I do not control
what other matrix classes have been added to the hierarchy by other users
in other DLLs that I may not even be aware of. This is why exceptions or
std::abort() are not a good solution anymore. It is no longer a viable
precondition.

Another difference between Boost.OpenMethod and N2216 is that in the latter
a programmer never says that they are overriding a virtual method. The
compiler has to deduce the fact from the signatures.

Another difference is that N2216 makes it an error to define a parameter as
virtual when a corresponding class s not a reference to a polymorphic type.
This is a feature (a safety feature). Boost.OpenMethod does not have it: I
can pass any type I want in virtual_ptr only to get an abort at run-time:
https://godbolt.org/z/b1Y5zMcfh

Given this type of a library, where:
* A language feature is implemented as a library
* The feature is about declaring interfaces (rather than used for internal
implementation, such as BOOST_FOREACH)
* The feature itself is controversial and not well explored
* It cannot be implemented as a library
* The feature is complex

I would expect a way more of a discussion and explanations in the
documentation. More concepts and definitions. The lack of those, makes me
further question the soundness of the design.

For one instance, the statement, "Multiple and virtual inheritance are
supported, with the exception of repeated inheritance." What does it mean
"not to support a form of inheritance"? Compiler error? Run-time abort()?
Undefined Behavior?

I can check what the current implementation does, but is this guaranteed,
or is it a happenstance of the today's implementation?

Now, let's compare open methods with pattern matching that can distinguish
different concrete classes in a type hierarchy. Regardless if they are
language features or library emulations. The type-switch (as in
wg21.link/n3449) or "match-expression" (as in wg21.link./p2688) is not
devoid of similar problems: Someone may add a new class to the hierarchy
after I have implemented my match-expression:

double get_area(const Shape& shape) {
  return shape match {
    Circle: let [r] => 3.14 * r * r;
    Rectangle: let [w, h] => w * h;
    _: throw MyLib::Unimplemented();
  };
}

And if some new class Polygon now inherits from Shape, I will get a throw.
But this time, it is plain clear, written down in my code, in what order
the candidates will be considered (up-to-bottom, first match). No surprises
of the form, "I thought that X would be a better match".

Earlier, I made a claim that the authors of N2216 dropped the idea in favor
of pursuing pattern matching. I cannot find any citation that would support
this. I guess I got that from the fact that two of the authors of N2216 --
Bjarne Stroustrup and Yuriy Solodkyy -- later wrote papers only on pattern
matching (wg21.link/n3449,
https://www.stroustrup.com/OpenPatternMatching.pdf). Maybe I got this from
the ISO C++ mailing lists.

If I understood correctly, the idea behind virtual_ptr is that it should be
created early to front-load the expensive hash table look-ups. This tells
me that even though I have to use a lot of macros in my interfaces, they
are still not able to hide the implementation details, and I am exposed to
these details through the pieces of advice, such as "pass around
virtual_ptr and store them as members". Then I am exposed to more things,
like final_virtual_ptr (not to mention unique_virtual_ptr and
shared_virtual_ptr).

Here's an important point. Boost is known to deliver language features in
form of libraries. But the ones I know were to be used inside function
bodies: BOOST_FOREACH, BOOST_AUTO. So my callers do not have to know that I
am even using them. It is my business. But with Boost.OpenMethod, we are
talking about declaring interfaces. When I -- a library author -- make a
choice to use a Boost.OpenMethod-style open method, how do I explain all
these quirks to my users? Now I am spilling all those implementation
details, micro-decisions, limitations to the poor users.

Now, I have a more philosophical question. If we declare open-methods (or
even pattern-matching-like type switches) non-intrusively on class
hierarchies, these hierarchies no longer need to have virtual functions: he
polymorphic behavior can now be implemented as open methods. In that case,
what is the purpose of declaring class hierarchies? Previously the answer
was clear: the types are in the OO-hierarchy because while they have
different implementations, they provide the same interface. Now, with open
methods they do not provide the same interface. So why put them into a
hierarchy?

Every library or a language feature or a tool is a burden: you have to
learn it, your users have to learn it, you need to debug it. In order for
it to be useful, the costs of usage/maintenance must be outweighed by the
benefits of using it. I do not see this being the case for Boost.OpenMethod.

This is my main objection.

I have smaller observations that could be fixed:
* There is a number of mysterious but unexplained terms used, such as
"guide functions" or "with the virtual_ decorators stripped".
* Things are imported into a global namespace (such as virtual_ptr). Don't
do it by default. If users want to see virtual_ptr and final_virtual_ptr as
keywords, let them type `using namespace boost::openmethod::keywords;`.
* The framework behaves differently based on whether my types are copyable
or not. Compare:
** https://godbolt.org/z/zKzYzbj3x
** https://godbolt.org/z/Ya8TrE79K

For some standard review questions:
* I have played with some toy examples using Clang on Compiler Explorer and
using GCC on MinGW on Windows. No problems running the examples.
* I spent a couple of days in total, studying the docs, re-reading related
papers, running examples, filing bugs, and demonstrating the library in my
workplace.
* I have kept an eye on the papers on open-methods, open type-switch and
pattern matching, playing with Mach7 library.

Finally, this review may seem very negative, so let me stress again that I
appreciate the work Jean-Louis has done to explore the subject. A critique
can be seen as a form of appreciation.
Also, thanks to Дмитрий for managing the review.

Regards,
&rzej;