perbone / luascript

Lua language support for Godot Engine
Apache License 2.0
629 stars 44 forks source link

Reference to local #45

Closed alexzk1 closed 2 years ago

alexzk1 commented 2 years ago

Just tried to build things... And got scaring messages, which are hidden behind macros FORCE_INLINE in your code, but, I would check that:

In file included from modules/luascript/lua_script.cpp:26: modules/luascript/lua_script_language.h: In static member function 'static MutexLock&& LuaScriptLanguage::acquire()': modules/luascript/lua_script_language.h:43:33: warning: reference to local variable 'lock' returned [-Wreturn-local-addr] 43 | return std::move(lock); | ~~~^~~~ modules/luascript/lua_script_language.h:42:27: note: declared here 42 | MutexLock lock(LuaScriptLanguage::get_singleton()->mutex); | ^~~~ [ 27%] Compiling ==> modules/luascript/lua_script_instance.cpp In file included from modules/luascript/lua_script_instance.cpp:23: modules/luascript/lua_script_language.h: In static member function 'static MutexLock&& LuaScriptLanguage::acquire()': modules/luascript/lua_script_language.h:43:33: warning: reference to local variable 'lock' returned [-Wreturn-local-addr] 43 | return std::move(lock); | ~~~^~~~ modules/luascript/lua_script_language.h:42:27: note: declared here 42 | MutexLock lock(LuaScriptLanguage::get_singleton()->mutex); | ^~~~ [ 27%] Compiling ==> modules/luascript/lua_script_language.cpp In file included from modules/luascript/lua_script_language.cpp:28: modules/luascript/lua_script_language.h: In static member function 'static MutexLock&& LuaScriptLanguage::acquire()': modules/luascript/lua_script_language.h:43:33: warning: reference to local variable 'lock' returned [-Wreturn-local-addr] 43 | return std::move(lock); | ~~~^~~~ modules/luascript/lua_script_language.h:42:27: note: declared here 42 | MutexLock lock(LuaScriptLanguage::get_singleton()->mutex);

I think proper way is just:

static MutexLock LuaScriptLanguage::acquire()

and then "return lock;"

If lock has proper move constructor/operator compiler will handle that properly. Also if you want be sure MOVE made, delete copiers explicit like MutexLock(const MutexLock&) = delete;

perbone commented 2 years ago

What is the Godot's branch you are compiling with? I can assume you read that it's for 3.x series right?

alexzk1 commented 2 years ago

What is the Godot's branch you are compiling with? I can assume you read that it's for 3.x series right?

Yes, but expanded macro in warning does not look right. I mean ... returning && is wrong, returning & is wrong unless object is static, like:

inline T& func() { static T a; return a; } that is possible correct use of reference returning. In your case that could be

class Lock { Lock(Lock&&) = default; Lock(const Lock&) = delete; Lock& operator=(const Lock&) = delete; Lock& operator=(Lock&&) = default; }

Lock get() { Lock lock; return lock; // <<-----that will do 100% move here }