jll63 / yomm2

Fast, orthogonal, open multi-methods. Solve the Expression Problem in C++17.
Boost Software License 1.0
348 stars 18 forks source link

Friendship with multi-methods #7

Closed matpen closed 4 years ago

matpen commented 4 years ago

I have been a user of yomm11 for quite some time. At the time where I started using it I faced a problem with the use of multi-methods from classes, which I solved by using a suggestion by @jll63.

The example elaborated at the time is attached for reference: much like in the matrix example, multi-methods are used to handle a hierarchy of classes, however in this case the "entrypoint" is a public method of the Painter class.

The requirement was to access some private member of the Painter class from the multi-methods. Since it is not possible to declare a multi-method as a member of the class, the solution was to friend the class generated by the preprocessor, and pass the instance to the various multi-methods.

However, in yomm2 the same solution seems more difficult to achieve: the library generates a numeric ID for the namespace where it declares the methods, which I presume depends on the order of declaration and thus cannot be reliably predicted. Attached is my attempt to solve this, but I have the feeling that it is a brittle solution.

Any suggestions for the above? Would a variant of declare_method and define_method, to use within a class eligible for implementation? For example:

class Painter
{
public:
    void paint(const Geometry& geometry);
private:
    int counter = 0;
    declare_class_method(void, paintObject, (virtual_<const Geometry&>));
};

define_class_method(void, Painter, paintObject, (const Geometry& /*geometry*/)) {
    // ...
}
jll63 commented 4 years ago

Glad to hear that someone is using my libraries :) I did not have much feedback, but while googling I did find mentions of people using it without me ever having heard from them, and in once instance commenting that, unlike Mach7, it worked without a hiccup.

Anyway...

Indeed trying to predict the __COUNTER__ value based hidden namespace is not a reliable method. You could always put the code to be befriended in an explicitly named function and have the override forward to it. But, not cool...

I see two possibilities for extending yomm2 to better support friendship.

  1. Make define_method take an optional argument (or create a new macro altogether) that allows explicitly naming the override. Basically this is the "manual" approach, with a bit of syntactic sugar.
  2. Or, I could change the way the internals are hidden. For example, I think that it may be possible to declare a template inside an anonymous namespace, then use specializations of that template as containers for the methods internal stuff, in lieu of the hidden namespace. Something along these lines: (pseudocode)
    
    // create by yomm2.hpp
    namespace {
    template<class method_key> struct  yOMM2_method>;
    template<class method_key, typename override_key> struct yOMM2_override;
    }

// expanded from declare_method struct yOMM2_method<yOMM2_DECLARE_KEY(paintObject)> { // declarations }; // definitions xxx yOMM2_method<yOMM2_DECLARE_KEY(paintObject)>::yyy;

// expanded from define_method struct yOMM2_method< yOMM2_DECLAREKEY(paintObject), void(Painter&, virtual<const Geometry&>)

{ // declarations }; // definitions xxx yOMM2_method< yOMM2_DECLAREKEY(paintObject), void(Painter&, virtual<const Geometry&>) ::yyy;


Then we would have something stable and predictable for a `declare_friend_method` macro to target.

Thinking about it, perhaps there would be an upside to removing the anonymous namespace: it would detect multiple overrides with the same signature at link time.

I am on vacation so I cannot work on this approach for a couple of weeks (and keep my family happy). Would you like to try it and perhaps send me a PR? It looks like you have some understanding of the internals of yomm2.

matpen commented 4 years ago

Glad to hear that someone is using my libraries :)

Oh yes, indeed I find it very useful! Thanks btw for the great work!

You could always put the code to be befriended in an explicitly named function and have the override forward to it. But, not cool...

My current solution is indeed something like this, basically forwarding the private stuff to the methods, but it quickly gets messy.

I see two possibilities for extending yomm2 to better support friendship.

The second solution seems even better. I agree that removing the namespace might enable the compiler to perform extra checks, and the user would probably want to add the namespace himself anyway.

I am on vacation so I cannot work on this approach for a couple of weeks (and keep my family happy).

Haha family first! Enjoy your vacation then, no rush!

Would you like to try it and perhaps send me a PR? It looks like you have some understanding of the internals of yomm2.

I already tried... but unfortunately I got lost in code. What I did to setup my example was just to look at the preprocessor output. But I would gladly help with testing, if that is of any use!

jll63 commented 4 years ago

I toyed with option (1), and came up with this: https://github.com/jll63/yomm2/blob/users/matpen/tests/lab.cpp

I am still thinking about the best way to support friendship for a method as a whole (i.e. granting friendship to all the overriders of a method).

As for option (2), I think I remember the reason for switching to hidden namespaces. IIRC yomm11 used templates as containers for internal data structures, with the consequence that methods had to be overridden in the same namespace as the declaration. But I'll keep searching for a solution to this.

matpen commented 4 years ago

Hey, thanks for working on that already! Hope you will not upset your family this way! ;)

I am inspecting the prototype you linked and I have the following remarks:

jll63 commented 4 years ago

connected to that, one issue might be that the paint_geo name must be unique across the program

If it identifies a specific method, it has to be unique ;-)

it probably cannot be namespaced as of now.

Yes it can. That's the purpose of the the named_method/YOMM2_NAMED_METHOD macros (which I may rename to declare_named_method/YOMM2_DECLARE_NAMED_METHOD). It simply expands to struct <method-name> and can be used to name a method defined in any namespace (of course the override and the name will need to be in the same namespace). Once that is done, friendship can be declared using a qualified name. E.g.:


class Painter;

namespace methods {
named_method(paint_geo);

// Implements paint
declare_method(void, paintObject, (Painter&, virtual_<const Geometry&>));
}

class Painter
{
public:
    void paint(const Geometry& geometry);
private:
    int counter = 0;
    friend named_method(methods::paint_geo);
};

void Painter::paint(const Geometry& geometry)
{
    methods::paintObject(*this, geometry);
}

namespace methods {
// method override definitions
}

what about using a struct as container for all methods? This would allow us to have friend struct method_container(paint_geo), thereby automatically extending friendship to all overrides.

I assume that you mean a struct template? Otherwise the override set would be closed.

If yes, that is how yomm11 works. The problem with this is that a template can be specialized only in the same namespace. Thus method definitions have to be in the same namespace as the corresponding declaration. I didn't like that, that's why I switched to using a (plain) struct inside a hidden namespaces as a container for the specialization (its body but also its next pointer).

By the way, your example is interesting. Usually, the first thing I reply when asked about friendship is "it defeats the purpose of open methods". But this is true only in the case where the specializations need access to the private parts of the classes of their virtual arguments. In your case, the hierarchy rooted in the virtual parameter is left untouched. Which is the #1 merit of open methods.

the call_named_method() idea might be really useful!

Yeah, once an override is directly addressable, you can do this. yomm11 has an API (GET_SPECIALIZATION) for fetching the specialization that would be called given a set of arguments. It has two drawbacks though. Firstly, it performs a method resolution (and incurs the cost). Secondly, it merely returns the bast match (if any). In other words, if you ask, say, the specialization for (Bulldog), and there is only a specialization for (Dog), you will silently get that one.

matpen commented 4 years ago

It simply expands to struct and can be used to name a method defined in any namespace

I have the feeling that this is heading in the right direction. By looking at the last example, it seems to provide exactly what we need!

friend named_method(methods::paint_geo);

Especially this, it is exactly what I attempted to hack into yomm2 before opening this issue... but I quickly realized it is not simple to do.

By the way, your example is interesting.

Glad I could provide a useful use-case. For the project I am working on I made tons of research which basically always lead me to "there is no clean way to do that"... "that" being solving the expression problem, for which I finally found the definition in your introduction and articles!

jll63 commented 4 years ago

I modified the API slightly:

I merged to master and tagged as version 1.0.1. I am keeping the feature experimental for a couple of weeks, then if we are still happy with it, I will document it and bump the minor version number.

matpen commented 4 years ago

Thank you! This is perfect timing, as my project is just entering the phase where I could really make use of the new feature. This will allow me to test is extensively.

Today I already started by installing the new version and testing the example, which works perfectly. The combination of friend_named_method() and define_named_method() is really comfortable.

One thing I realized during the test is that there seem to be no way to extend the friendship to all overloads, as you proposed. I think in my case I can workaround with next() or the new call_named_method(), but before I attempt that I wanted to ask if I am missing something after inspecting the commits, or you are still planning the feature (or abandoned the idea).

jll63 commented 4 years ago

One thing I realized during the test is that there seem to be no way to extend the friendship to all overloads,

I don't see any solution that would not use templates. I am going to play with a variant of the new macros that would use a template<...> struct as a container for the specialization bodies, but that solution will run into the same problem as yomm11, namely that the template will need to be specialized inside the same namespace. At least the problem will be limited to friend specializations.

I think in my case I can workaround with next() or the new call_named_method()

It's one of the two things a like with your example. Friendship is granted to one specialization at a time, and is not "inherited", just like friending a class does not friend its subclasses. I try to follow the C++ philosophy as much as possible.

matpen commented 4 years ago

Great to hear that the idea is still actual. I think it would be nice to have, but I do grasp the problems behind the implementation. So I will start implementing with what we have so far, and report back periodically.

I try to follow the C++ philosophy as much as possible.

That is always a good thing, btw ;)

jll63 commented 4 years ago

Here's a new take at the problem: https://github.com/jll63/yomm2/blob/friendship-again/examples/friendship.cpp

Still a WIP, I am pondering whether to wrap these in macros:

    template<typename...> friend struct segments::painters;
    friend struct shapes::painters<void(Painter&, const shapes::Shape&)>;
    painters<void(Painter&, const Shape&)>::call(painter, square);
    painters<void(Painter&, const Shape&)>::call(painter, circle);
jll63 commented 4 years ago

OK this is starting to look good...Maybe we'll need an overload for method_container to forward declare a specialization in a container.

matpen commented 4 years ago

Sorry for the long delay in my response: I got involved in a project with a tight schedule, and only now get to review these improvements.

Which, I must say, they do look amazing! It is now possible to extend friendship to all overloads, and the usage example looks exactly like I hoped for! Moreover, I noticed that you went the extra mile, and added one trick I did not consider: we can now also limit friendship to methods which handle a sub-hierarchy of classes, very useful!

The only difficulty I see is with this declaration, which is somewhat non-obvious, at least by looking at the example. I tried to inspect the diff but it is still too advanced for me to provide suggestions.

Can't wait to try out the new features. As soon as I get the chance, I will upgrade my library version and report back.

jll63 commented 4 years ago

Glad you like it!

method_definition yields the body of the override/specialization/definition with the given signature in the given container. You can then use it to call the specialization or take a pointer to it. IOW it replaces the previous call macro.

I am not sure about the name though. If you have a better idea...I considered method_from, container_method, get_method and others I forgot.

The problem is that there is no "standard" naming convention for open methods. In CLOS, defgeneric declares the entry point, and defmethod adds a specialization. defmethod inspired my naming and concept scheme, which is: declare_method declares the name and the signature of a family of functions, and define_method adds a definition to the family.

At this point, the return type is not necessary in those macros. I put it anyway because I wanted a uniform convention, i.e. avoid confusion between macros that need the return type and macros that just need the signature.

I am also pondering a "inline" variant of define_method(container, ...), hesitating between define_inline_method, inline_define_method and define_method_inline :-/

matpen commented 4 years ago

method_definition yields the body of the override/specialization/definition with the given signature in the given container. You can then use it to call the specialization or take a pointer to it. IOW it replaces the previous call macro.

This is now clear: so it can also replace usages of next(), to ensure the overload is the expected one. Once the concept is understood, the name is quite fitting, I believe. Other ideas could be:

I would avoid container_method though, which would be really confusing due to the presence of method_container... Unless you really like one of my suggestions (which I am not 100% convinced myself about), I would just stick with the current choice of method_definition.

At this point, the return type is not necessary in those macros. I put it anyway because I wanted a uniform convention

I totally agree with this choice. The input might be unnecessary, but from the user point of view it is something that is at one's disposal anyway, and it stays consistent with the other macros, avoiding one more thing to remember.

I am also pondering a "inline" variant of define_method(container, ...), hesitating between define_inline_method, inline_define_method and define_method_inline

This might be useful too... I see you have plenty of inspiration! define_method_inline is the name I would prefer, because in a theoretical sorted list of available methods would appear next to the define_method` counterpart, making it immediately clear that it provides the same functionality "but inline".

jll63 commented 4 years ago

This is now clear: so it can also replace usages of next(), to ensure the overload is the expected one.

You can, but you should avoid doing as much as possible. It defeats the goals of openness and composability.

This might be useful too... I see you have plenty of inspiration!

Aaawww, thanks.

If you have time, please review the reference in https://github.com/jll63/yomm2/pull/8

jll63 commented 4 years ago

Addressed in https://github.com/jll63/yomm2/releases/tag/v1.1.0

Can you share the name of your company and give an overview of the project(s) you use yomm2 (and before yomm11) in? I'd like to make a list of users. If it's not possible, no worry.

matpen commented 4 years ago

Addressed in https://github.com/jll63/yomm2/releases/tag/v1.1.0

Thank you for taking the time and effort to work on this. I believe that it makes the library very much complete and attractive. And it has immediate practical application in my use case!

Can you share the name of your company and give an overview of the project(s) you use yomm2 (and before yomm11) in? I'd like to make a list of users. If it's not possible, no worry.

I am freelancing, so you are welcome to use my github username in the list. As for the project description, I would say as follows (feel free to adapt the text to the context):

The project I am working on is related to structural engineering. 
Because of the special kind of data involved, it requires a custom dedicated visualization engine.
May different shapes need to be handled, each of which is implemented as a separate class, along with algorithms for geometry manipulation. 
In this architecture, it makes sense to separate the visualizer from the classes, and the open multi-method approach perfectly suits this use case.

Again thanks for the friendly support: I will make sure to share more ideas or issues, should they arise while using the library!

jll63 commented 4 years ago

Again thanks for the friendly support: I will make sure to share more ideas or issues, should they arise while using the library!

And thank you for the interesting use case :)