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

From: Jean-Louis Leroy (jl_at_[hidden])
Date: 2025-05-02 04:50:10

Hi Christian,

Thanks for the review!

> 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 many years policies in YOMM2 existed in hiding, just for the benefit of unit
tests. But it changed due to user feedback.

> 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?

I very much agreed with you, until someone on reddit convinced me in a
snap. He was
a game developer, told me that my library was my library was unusable in his
field. Why? Because we disable standard RTTI. But why on Earth do that? Because
some people look at the strings embedded in the binaries and use the info to
reverse-engineer the game, cheat, or whatnot.

Games are an important segment of the industry, also I have been eyeing embedded
programming.

> 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.

Yeah, that's a mouthful.

Since reading your reply, I've had ideas for improvements. I'll experiment with
them during the week-end, and come back to this.

> 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.

Yes, I think I can provide shortcuts for that.

> 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".

No, it *literally* calls abort.

I was guided by what happens when you call a pure virtual function. The program
aborts, *maybe* with a short diagnostic.

> 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".

To clear any misunderstanding, I think that exceptions are great, far superior
to any alternative I know of, except in very specific and rare contexts. So why
this choice?

A big chunk of the community is allergic to exceptions. Promoting a form of
polymorphism that relies on inheritance and tables of function pointers is an
uphill battle already. Adding exceptions to the backpack makes it worse.

Also, when you call a pure virtual function, it doesn't throw. UB, why not?
Sadly it may be better than throwing.

But you got me thinking about a "more sane path"...

N2216 and the papers that precede it sort of sidestep the question...They
require every open-method to have a definition. The motivation is, what should
happen if no overrider is applicable. Should we throw an exception? But that is
unacceptable in the context of embedded programming. So they require every
base-method to have an implementation, problem solved.

Then they add "solutions" for dealing with ambiguities. In the first paper, it's
using covariant return types as tie-breakers. Eventually, in N2216, they pick
"an" overrider after every attempt at finding an only best one have failed.

In the end, the open-function mechanism *itself* doesn't need to throw or abort,
because it has eliminated every reason to do so.

You have to look hard, though, to find an *example* of a base-method
implementation.

In N2216:

    bool intersect(virtual Shape&, virtual Shape&) { }

This doesn't look good, it silently returns something in a situation that is
likely a bug.

In Solodkyy's paper "Simplifying the Analysis of c++ Programs":

    int eval (virtual const Expr&) { throw std::invalid argument ("eval"); }

This is better. I think that it is what most base-method implementations will
look like. A counter-example is my "animals meet" example: they ignore each
other. But it's a made-up example.

So why not synthesize an erroring base-method overrider if none has been
provided?

I can enforce the paper's requirement of a base-method overrider at runtime (in
initialize), maybe even at link time (but it would break some regularities
elsewhere). It would be a way of copping out of the issue altogether. It would
probably make noexcept open-methods feasible without complexifying the policy
even more.

> 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.

Since the macros follow that order, I changed all the code to follow it as well,
hoping to achieve some sort of subliminal teaching ;-)

> 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.

It's a very static-polymorphic thing. The interaction between objects,
virtual_ptrs to objects, and virtual smart ptr to objects. I have to improve
that part. And yes, provide short examples.

> 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 think that `next` is the likely correct way in most cases. But you are not
forced to use BOOST_OPENMETHOD_OVERRIDER, it's just there in case you feel you
need it.

> 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.

So this is a late addition after a comment from Joaquin. I leaned
towards "agree"
because this is legal:

    namespace foo {}

    void foo::bar() {}

But yeah I can easily flip the other way.

> Otherwise, I think the library is very enjoyable.

This warms my heart :)