ThePhD / sol2

Sol3 (sol2 v3.0) - a C++ <-> Lua API wrapper with advanced features and top notch performance - is here, and it's great! Documentation:
http://sol2.rtfd.io/
MIT License
4.06k stars 492 forks source link

Critical: std::shared_ptr wrong type dispatch on MSVC Release #1556

Open deadlocklogic opened 7 months ago

deadlocklogic commented 7 months ago
  1. Repro
    
    #define SOL_ALL_SAFETIES_ON 1
    #include <sol/sol.hpp>

struct ClassA { virtual ~ClassA() = default;

static std::shared_ptr<ClassA> makeShared()
{
    return std::make_shared<ClassA>();
}

};

class ClassB { };

int main() { sol::state lua;

lua.open_libraries(sol::lib::base);

lua["call"] = [](sol::object ptr)
{
    if ((ptr.is<std::shared_ptr<ClassB>>()))
    {
        return "ClassB"; // Actual behavior: should not get in here in our test!!!
    }
    if ((ptr.is<std::shared_ptr<ClassA>>()))
    {
        return "ClassA"; // Expected behavior
    }
    return "nullptr";
};
lua.new_usertype<ClassA>("ClassA", "makeShared", &ClassA::makeShared);
lua.new_usertype<ClassB>("ClassB");

lua.script(R"(

do local instance = ClassA.makeShared() assert(call(instance) == "ClassA") end )", [](lua_State*, sol::protected_function_result pfr) { sol::error err = pfr; std::cout << err.what() << std::endl; return pfr; });

return 0;

}


2. Tests fails unexpectedly (Expected behavior: `ClassA`, Actual behavior: `ClassB`).
    **Notice 1**: this behavior is on `MSVC` `Release` (`Debug`/`RelWithDebInfo` work fine)
    **Notice 2**: `std::unique_ptr` behaves correctly.

3. Compiler/IDE: Visual Studio
    Language: C++
    sol2 version: latest
@ThePhD  👀.

Thanks.
totalgee commented 3 months ago

I can repro that. In Release (where your assertion fails), note that it passes if you change the order of your checks (so it also passes the case for shared_ptr<ClassA>. Hmm, the reason is that in fact, ptr.is returns true for any shared_ptr type, such as shared_ptr<int>!

    // This seems to return true in Release, for sol::object of any shared_ptr<> type
    bool result = ptr.is<std::shared_ptr<int>>();
deadlocklogic commented 1 month ago

@totalgee Exactly, weirdly enough. It is sad that these bugs exist in a nice library.