Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use of std::shared_ptr on classes without std::enable_shared_from_this #170

Closed
rpatters1 opened this issue Mar 2, 2024 · 1 comment · Fixed by #172 or #174
Closed

Use of std::shared_ptr on classes without std::enable_shared_from_this #170

rpatters1 opened this issue Mar 2, 2024 · 1 comment · Fixed by #172 or #174
Labels
bug Something isn't working

Comments

@rpatters1
Copy link
Contributor

I work with a large legacy C++ framework that returns allocated class instances to Lua using std::shared_ptr. Unfortunately, adding std::enable_shared_from_this to these classes is not feasible in a systematic fashion, due to it causing horrible memory crashes unrelated to LuaBridge.

Fortunately, LB3 only requires std::enable_shared_from_this when trying to pass a std::shared_ptr back to C++ from Lua, and my legacy framework never needs to do this. So I am indeed grateful that the requirement for it is limited. I am furthermore grateful that it fails to compile if I attempt it.

With that said, I am wondering if the following is a bug:

class foo {};
class bar
{
public:
   std::shared_ptr<foo> getter() const { return std::shared_ptr<foo>(new foo); }
   void setter(std::shared_ptr<foo>) {};
};

   luabridge::getGlobalNamespace(L)
      .beginClass<foo>("foo")
      .endClass()
      .beginClass<bar>("bar")
         .addFunction("getter", &bar::getter) // does not fail (thank you!)
//         .addFunction("setter", &bar::setter) // fails due to no std::enable_shared_from_this (as it should)
         .addProperty("get", &bar::getter)  // fails due to no std::enable_shared_from_this, BUT SHOULD IT?
      .endClass();

See the addProperty call. It is only adding a getter, but it is still triggering the static_assert that requires std::enable_shared_from_this. I am wondering if this is a bug or intentional behavior.

@kunitoki
Copy link
Owner

kunitoki commented Mar 2, 2024

It's probably an oversight

@kunitoki kunitoki reopened this Mar 3, 2024
@kunitoki kunitoki mentioned this issue Mar 3, 2024
kunitoki added a commit that referenced this issue Mar 3, 2024
@kunitoki kunitoki added bug Something isn't working ready and removed ready labels Mar 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
2 participants