SOL_SAFE_FUNCTION_CALLS does not recognize derived classes

[diath]

With the SOL_SAFE_FUNCTION_CALLS, I'd expect the type checker to actually allow inherited classes to be passed to a base pointer argument.

But instead, the following code:

#include <cstdio>
#include <memory>

#define SOL_SAFE_FUNCTION_CALLS 1
#include <sol/sol.hpp>

struct Animal {
    void say() {
        std::printf("* quiet *");
    }
};

struct Dog: public Animal {
    void say() {
        std::printf("* woof *");
    }
};

static void accept(std::shared_ptr<Animal> a)
{
    a->say();
};

int main(int, char **)
{
    sol::state state;

    state.new_usertype<Animal>("Animal",
        sol::factories([]() {
            return std::make_shared<Animal>();
        })
    );

    state.new_usertype<Dog>("Dog",
        sol::factories([] {
            return std::make_shared<Dog>();
        }),
        sol::base_classes, sol::bases<Animal>()
    );

    state["accept"] = &accept;

    state.script("accept(Dog.new())");
    return 0;
};

Produces the following error: sol: runtime error: stack index 1, expected userdata, received sol.sol::d::u<Dog>: value is a userdata but is not the correct unique usertype (bad argument into 'void(std::shared_ptr<Animal>)')

[Rochet2]

You should make accept function take in an Animal* according to this

When you bind a function to Lua, please take any pointer arguments as T*, unless you specifically know you are going to match the exact type of the unique/shared pointer and the class it wraps. sol3 cannot support “implicit wrapped pointer casting”, such as taking a std::shared_ptr<MySecondBaseClass> when the function is passed a std::shared_ptr<MyDerivedClass>. Sometimes it may work because the compiler might be able to line up your classes in such a way that raw casts work, but this is undefined behavior through and through and sol3 has no mechanisms by which it can make this safe and not blow up in the user’s face.

Also seems like in the example the say function was expected to be virtual, but its not. So calling say on the Animal* will result in * quiet * regardless of it being Dog. I would add virtual and override in the Animal and Dog respectively if that was the case.

[diath]

You should make accept function take in an Animal* according to this

That's rather unfortunate as I'm wrapping an API that uses smart pointers and not raw pointers and I have no control over it, however, based on the docs you quoted, does it mean I should not use smart pointers at all, even when there's no inheritance involved? Ah, nevermind, it seems like your quote swallowed up the template types from the doc, so it should be fine for the same type.

Also seems like in the example the say function was expected to be virtual, but its not. So calling say on the Animal will result in quiet * regardless of it being Dog. I would add virtual and override in the Animal and Dog respectively if that was the case.

Right, it was just a quick example I whipped up and accidentally forgot the virtual keyword.

I don't get it though, does it mean that shared_ptr support in sol is completely useless? If I have a factory function such as the example in my original post, and say I wanted to store the Lua-created instances in a vector on the C++ side, there would be no way to take a raw pointer and store it as shared_ptr to keep the object alive.

Given the following modified example, what would be the idiomatic way to achieve this without triggering UB?

#include <cstdio>
#include <memory>
#include <vector>

#define SOL_SAFE_FUNCTION_CALLS 0
#include <sol/sol.hpp>

struct Animal {
    virtual void say() {
        std::printf("* quiet *\n");
    }
};

struct Dog: public Animal {
    void say() final {
        std::printf("* woof *\n");
    }
};

static void accept(std::shared_ptr<Animal> a)
{
    static std::vector<std::shared_ptr<Animal>> animals;
    animals.push_back(a);

    a->say();
};

int main(int, char **)
{
    sol::state state;

    state.new_usertype<Animal>("Animal",
        sol::factories([]() {
            return std::make_shared<Animal>();
        })
    );

    state.new_usertype<Dog>("Dog",
        sol::factories([] {
            return std::make_shared<Dog>();
        }),
        sol::base_classes, sol::bases<Animal>()
    );

    state["accept"] = &accept;

    state.script("accept(Dog.new())");
    return 0;
};

[diath]

I was thinking about adding a wrapper that takes a raw pointer, then use a shared_ptr to take ownership of the object, however each of these solutions has its own issues.

Allocating with new in the factory function will result in a memory leak:

#include <cstdio>
#include <memory>
#include <vector>

#define SOL_SAFE_FUNCTION_CALLS 0
#include <sol/sol.hpp>

struct Animal {
    virtual ~Animal() {
        std::printf("~Animal();\n");
    }
    virtual void say() {
        std::printf("* quiet *\n");
    }
};

struct Dog: public Animal {
    ~Dog() {
        std::printf("~Dog();\n");
    }
    void say() final {
        std::printf("* woof *\n");
    }
};

static void accept(std::shared_ptr<Animal> a)
{
    static std::vector<std::shared_ptr<Animal>> animals;
    animals.push_back(a);

    a->say();
};

int main(int, char **)
{
    sol::state state;
    state.open_libraries(sol::lib::base);

    state.new_usertype<Animal>("Animal",
        sol::factories([]() {
            return new Animal;
        })
    );

    state.new_usertype<Dog>("Dog",
        sol::factories([]() {
            return new Dog;
        }),
        sol::base_classes, sol::bases<Animal>()
    );

    state["accept"] = [] (Animal *animal) {
        std::shared_ptr<Animal> owned_animal { animal };
        accept(owned_animal);
    };

    state.script("accept(Dog.new())");
    state.script("Dog.new()");
    state.script("collectgarbage('collect');");

    return 0;
};

Allocating using the default Sol-owned memory will result in a double free:

#include <cstdio>
#include <memory>
#include <vector>

#define SOL_SAFE_FUNCTION_CALLS 0
#include <sol/sol.hpp>

struct Animal {
    virtual ~Animal() {
        std::printf("~Animal();\n");
    }
    virtual void say() {
        std::printf("* quiet *\n");
    }
};

struct Dog: public Animal {
    ~Dog() {
        std::printf("~Dog();\n");
    }
    void say() final {
        std::printf("* woof *\n");
    }
};

static void accept(std::shared_ptr<Animal> a)
{
    static std::vector<std::shared_ptr<Animal>> animals;
    animals.push_back(a);

    a->say();
};

int main(int, char **)
{
    sol::state state;
    state.open_libraries(sol::lib::base);

    state.new_usertype<Animal>("Animal"/*,
        sol::factories([]() {
            return new Animal;
        })*/
    );

    state.new_usertype<Dog>("Dog",
        /*sol::factories([]() {
            return new Dog;
        }),*/
        sol::base_classes, sol::bases<Animal>()
    );

    state["accept"] = [] (Animal *animal) {
        std::shared_ptr<Animal> owned_animal { animal };
        accept(owned_animal);
    };

    state.script("accept(Dog.new())");
    state.script("Dog.new()");
    state.script("collectgarbage('collect');");

    return 0;
};

[Rochet2]

One possibility is to use std::variant to provide all subclasses as arguments to the function:

static void accept(std::variant<std::shared_ptr<Animal>, std::shared_ptr<Dog>> v)
{
    static std::vector<std::shared_ptr<Animal>> animals;
    std::visit([&](auto&& a)
        {
            animals.push_back(a);
            a->say();
        }, v);
};
state["accept"] = &accept;

Or sol::override:

static void accept(std::shared_ptr<Animal> a)
{
    static std::vector<std::shared_ptr<Animal>> animals;
    animals.push_back(a);
    a->say();
};
state["accept"] = sol::overload(
        [](std::shared_ptr<Dog> a){ return accept(a); },
        [](std::shared_ptr<Cat> a){ return accept(a); }
);

In similar way you could also get in a sol::object and then use the obj.is<Dog*> type checks for example to see what the type of the object is and then take it from lua with exact type.

The downside is obviously that you are required to list all of the types you need the function to be able to process in all of these solutions. Could be there are other ways to get around the issue. 🤔

[diath]

I guess I will have to go with either of these, thanks, I'm closing this since I guess it's intended behavior.