sol2 icon indicating copy to clipboard operation
sol2 copied to clipboard

Optional argument doesn't check type and yields nullopt for type mismatch

Open gotschmarcel-ni opened this issue 3 years ago • 2 comments
trafficstars

I assumed that a function accepting an optional X should only be allowed to be called with X or nil, but instead it's called with nullopt for invalid types.

Example:

sol::state lua;

lua["func"] = [](std::optional<int> value) {
    std::cout << value.value_or(-1) << std::endl;
};

lua.script("func('hello')"); -- Prints -1, but expected failure

Repro: https://godbolt.org/z/h7WaKWvc42

Sol Version: 3.2.1 Clang 13.0.1

gotschmarcel-ni avatar May 25 '22 10:05 gotschmarcel-ni

Because of previous use with sol, I expect std::optional to basically never produce error (A value is either an int or it is converted into a nullopt). This is quite consistent in the documentation from what I have read. For example here https://sol2.readthedocs.io/en/latest/tutorial/variables.html?highlight=optional#reading

Here is an alternative way to do what you try: https://godbolt.org/z/cM7ba96nP

#define SOL_ALL_SAFETIES_ON 1
#include <limits>
#include <sol/sol.hpp>
#include <iostream>
#include <variant>

template<typename T, typename VARIANT>
T VariantGet(VARIANT& value, T default_value) {
    auto* ptr = std::get_if<T>(&value);
    if (ptr)
        return *ptr;
    return default_value;
}

int main()
{
    sol::state lua;

    lua["func"] = [](std::variant<int, sol::nil_t> value) {
        std::cout << VariantGet(value, -1) << std::endl;
    };

    // lua.script("func('hello')"); // error
    lua.script("func(100)"); // prints 100
    lua.script("func()"); // prints -1
}

Rochet2 avatar May 30 '22 22:05 Rochet2

It's true that the optional is used a lot for ignoring type errors, when trying to convert a Lua object to a concrete type. Here it kind of makes sense, because you're asking to convert and since that might fail you get an optional result. Kind of like the result type in Rust. For function arguments it still feels weird to me, because I'm not asking to convert any object to my argument if it can, it's more like this argument is optional. I guess in this case I'd be better off with using sol::overload with two different overloads. I can understand why it's consistent in behaviour though, because the args are implicitly converted to concrete types.

gotschmarcel-ni avatar May 31 '22 18:05 gotschmarcel-ni