jll63 / yomm2

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

specializing on `shared_ptr<Animal> const&` doesn't work for me. #31

Closed lfnoise closed 2 years ago

lfnoise commented 2 years ago

Thanks for this library. It feels like I have a new superpower!

According to the reference doc, it should be possible to dispatch on shared_ptr<Animal> const& but if I add the const ref qualifier, I get lots of template error messages, the first of which says Static_assert failed due to requirement 'shared_ptr_traits<const std::__1::shared_ptr<ScalarT<double> > &>::is_shared_ptr'. I assume I am doing something wrong.

Here is an example. It works as is. But if I change the #if 1 to #if 0 to enable const ref for all arguments, then it doesn't compile.

I am compiling with Clang in Xcode 12.3.

#include <yorel/yomm2/cute.hpp>
#include <iostream>
#include <memory>
#include <string>

#if 1
#define CONSTREF
#else
#define CONSTREF const&
#endif

using yorel::yomm2::virtual_;

class Value { // base class for values
public:
    virtual ~Value() = default;
};

class Scalar : public Value {};  // base class for scalars

template <class T>
class ScalarT : public Scalar {
    T x_;
public:
    ScalarT(T x) : x_(x) {}
    T x() const noexcept { return x_; }
};

// convenience function to create a shared ScalarT.
template <class T>
std::shared_ptr<Value> num(T x) { return std::make_shared<ScalarT<T>>(x); }

register_class(Value);
register_class(Scalar, Value);
register_class(ScalarT<int>, Scalar);
register_class(ScalarT<double>, Scalar);

declare_method(std::shared_ptr<Value>, plus,
    (virtual_<std::shared_ptr<Value> CONSTREF>, virtual_<std::shared_ptr<Value> CONSTREF>));

declare_method(std::string, str,
    (virtual_<std::shared_ptr<Value> CONSTREF>));

define_method(std::shared_ptr<Value>, plus,
    (std::shared_ptr<Value> CONSTREF a, std::shared_ptr<Value> CONSTREF b))
{
    throw std::runtime_error("unsupported values in plus");
}

define_method(std::shared_ptr<Value>, plus,
    (std::shared_ptr<ScalarT<int>> CONSTREF a, std::shared_ptr<ScalarT<int>> CONSTREF b))
{
    return num(a->x() + b->x());
}

define_method(std::shared_ptr<Value>, plus,
    (std::shared_ptr<ScalarT<double>> CONSTREF a, std::shared_ptr<ScalarT<double>> CONSTREF b))
{
    return num(a->x() + b->x());
}

define_method(std::string, str, (std::shared_ptr<Value> CONSTREF a))
{
    throw std::runtime_error("unsupported value in str");
}

define_method(std::string, str, (std::shared_ptr<ScalarT<int>> CONSTREF a))
{
    return std::to_string(a->x());
}

define_method(std::string, str, (std::shared_ptr<ScalarT<double>> CONSTREF a))
{
    return std::to_string(a->x());
}

int main(int argc, const char * argv[])
{
    yorel::yomm2::update_methods();

    auto a = num(1);
    auto b = num(2);
    auto c = num(3.01);
    auto d = num(4.001);
    std::cout << str(plus(a, b)) << "\n";
    std::cout << str(plus(c, d)) << "\n";
    return 0;
}
jll63 commented 2 years ago

Thanks for this library. It feels like I have a new superpower!

Thank you! May I ask what you (plan to) use it for, and, if yes, if the project is available as open source? I am trying to build a portfolio of use cases.

According to the reference doc, it should be possible to dispatch on shared_ptr<Animal> const&

Ah, I am going to disappoint you ;-) This is a doc bug, not a code bug. What you are trying to do is illegal, for the same reason that the following examples are:


struct Base {
    virtual void test(std::shared_ptr<Scalar> const &) {}
};

struct Wrong : Base {
    void test(std::shared_ptr<ScalarT<int>> const &) override {}
};

[build] /home/jll/dev/yomm2/tests/lab.cpp:47:54: error: non-virtual member function marked 'override' hides virtual member function
[build]     void test(std::shared_ptr<ScalarT<int>> const &) override {}
[build]                                                      ^
[build] /home/jll/dev/yomm2/tests/lab.cpp:43:18: note: hidden overloaded virtual function 'Base::test' declared here: type mismatch at 1st parameter ('const std::shared_ptr<Scalar> &' vs 'const std::shared_ptr<ScalarT<int>> &')
[build]     virtual void test(std::shared_ptr<Scalar> const &) {}

A shared_ptr to a derived class is-a shared_ptr to a base class, but the corresponding references (const or not) are not related. Just like a vector of pointers to apples is not a vector to pointers to fruits.

Note that the same holds if we use dumb pointers:


struct Base {
    virtual void test(Scalar* const &) {}
};

struct Wrong : Base {
    void test(ScalarT<int>* const &) override {}
};

[build] /home/jll/dev/yomm2/tests/lab.cpp:37:38: error: non-virtual member function marked 'override' hides virtual member function
[build]     void test(ScalarT<int>* const &) override {}
[build]                                      ^
[build] /home/jll/dev/yomm2/tests/lab.cpp:33:18: note: hidden overloaded virtual function 'Base::test' declared here: type mismatch at 1st parameter ('Scalar *const &' vs 'ScalarT<int> *const &')
[build]     virtual void test(Scalar* const &) {}
[build]                  ^

I will amend the doc...

lfnoise commented 2 years ago

It will be for an audio signal dataflow graph compiler. Not released yet. I was the original author of SuperCollider which is a language for computer music synthesis and algorithmic composition.

Regarding const references, I've not looked enough into your implementation to understand how it works, so naively, I guess I don't understand why it isn't possible. I had thought that what you might be doing was the following: Since I've already registered my classes you know what the hierarchy is, I thought you would strip off the shared_ptr and const& from the argument type, then get the type info and determine the inheritance by referencing the registered type info. I saw this using base_type = typename std::remove_cv_t<std::remove_reference_t<T>>; and calls to typeid in virtual_traits and assumed you were doing something like that.

The reason I would like to use const references is to not unnecessarily inc/dec the shared_ptr reference count.

cppguru commented 2 years ago

Normally when you do not want to change the reference count you either pass around a "naked" pointer (no, it is not more dangerous, the const ref won't keep it alive either), or if you may need to share ownership, or it may go away, then a weak_ptr. Passing around smart pointers by const reference just gives you an indirection to reach the actual pointer, it does not buy you anything. If a function does not deal with ownership, it should not deal with a smart pointer. With that simple rule the problem is gone. :)

lfnoise commented 2 years ago

It's not that simple. This is an expression language for creating data flow graphs. Function calls are quite nested and every function call is a constructor of a node in a data flow graph. If these functions were to return a weak_ptr, then objects would be immediately destroyed before they could be accessed by the consuming expression. If these functions were to return raw pointers, then an arena allocator or garbage collector would be needed. So shared_ptrs are returned and consumed by const reference. If the object needs to be kept, then the shared_ptr can be copied. If it is only a temporary intermediate value, then no unnecessary increments to the reference are made. The shared_ptr is kept alive by being in a temporary on the stack until the expression ends.

cppguru commented 2 years ago

Perhaps I misunderstood then. I thought we were talking about function parameters for multiple dispatch, not return values.

jll63 commented 2 years ago

Perhaps I misunderstood then. I thought we were talking about function parameters for multiple dispatch, not return values.

I think I see what the OP means. Sometimes you want to return one of the arguments, and that forces you to pass shared_ptrs. Consider a multi-method that multiplies matrices. It could have overrides for the cases where one of the arguments has the type identity_matrix. In that case, just return the other argument. See this discussion.

cppguru commented 2 years ago

OK, so we are talking about a temporary shared_ptr and not wanting to write .ptr() with the call to the function? I can understand nobody wants to write matrix.ptr() * aVector.ptr(). :)

jll63 commented 2 years ago

Actually, I am beginning to think that your code should indeed work. Give me a bit of time to think about it again...

lfnoise commented 2 years ago

Yes, I don't want to have to write .get() everywhere. Digression: I have written another version of this system before, using multiple dispatch by hand. An example of code is below. Every function is creating an object which may or may not use the argument objects. It may hash-cons an argument to an equivalent existing graph (common subexpression elimination), or rewrite it using algebraic transformations. V is a value wrapper, which could be a shared_ptr<Value>. Ideally, and this would be a different feature request, I could dispatch on a type contained by my own value wrapper class, not just shared_ptr. That way I could support having custom constructors, for scalar values for example, which the code below exploits. All the numbers get wrapped in a V via a conversion constructor.

...
V lfsaw (V fm, V pm) { return bi(phasor(fm * T(), pm)); } 

V bubbles() {
    V freq = nnhz(lfsaw(.4) * 24 + lfsaw(vec(8, 7.23)) * 3 + 81);
    V out = .04 * fsinosc(freq);
    out = combn(fadein(out, .1), .2, 4);
    return jackout(out);
}
jll63 commented 2 years ago

Can you try this branch? https://github.com/jll63/yomm2/pull/32

jll63 commented 2 years ago

Digression

Digression too ;-) From afar it looks like your system might actually benefit from using an arena.

jll63 commented 2 years ago

Ideally, and this would be a different feature request, I could dispatch on a type contained by my own value wrapper class, not just shared_ptr.

You are not alone!

As you have already found out, shared_ptr is supported via a mechanism meant to be extensible. I did not document it because I want the freedom to improve it. Also, I have an old branch with a POC of dispatching on a Boost variant.

As mentioned in the other issue, my desire to support templatized methods may force me to clean up and document parts of YOMM2.

jll63 commented 2 years ago

Oh yes, one more thing: it looks like what you want is more like open pattern matching; here is a great paper by, Solodkyy.

lfnoise commented 2 years ago

Can you try this branch? #32

Yes, this works, thanks!