Revisit error processing.

- Centralize exception handling from bound C++ code, leave only one
  try/catch wrapper and use it everywhere.
- Wrap custom lua_CFunction(s) with exception safety net.
- Revisit error messages.

CMakeLists.txt

@@ -12,6 +12,8 @@ set(LUA_LIB_NAME luabind_lua CACHE STRING "lua library name")
 # variable to include tests to build phase
 set(LUABIND_TEST ON CACHE BOOL "variable to determine whether tests are included in build phase or not")
 
+option(CODE_COVERAGE "Enable coverage reporting" OFF)
+
 add_subdirectory(third_party)
 
 add_library(luabind INTERFACE)
@@ -24,18 +26,19 @@ if(INCLUDE_LUA_LIB_WITH_EXTERN_C)
     target_compile_definitions(luabind INTERFACE INCLUDE_LUA_LIB_WITH_EXTERN_C)
 endif(INCLUDE_LUA_LIB_WITH_EXTERN_C)
 
-option(CODE_COVERAGE "Enable coverage reporting" OFF)
-if(CODE_COVERAGE)
-    # Add required flags (GCC & LLVM/Clang)
-    target_compile_options(luabind INTERFACE
-        -O0        # no optimization
-        -g         # generate debug info
-        --coverage # sets all required flags
-    )
-    target_link_options(luabind INTERFACE --coverage)
-endif(CODE_COVERAGE)
-
 if(LUABIND_TEST)
+    target_compile_options(luabind INTERFACE -Wall -Wextra -Wnewline-eof -Wformat -Werror)
+
+    if(CODE_COVERAGE)
+        # Add required flags (GCC & LLVM/Clang)
+        target_compile_options(luabind INTERFACE
+            -O0        # no optimization
+            -g         # generate debug info
+            --coverage # sets all required flags
+        )
+        target_link_options(luabind INTERFACE --coverage)
+    endif(CODE_COVERAGE)
+
     enable_testing()
     add_subdirectory(tests)
 endif()

include/luabind/bind.hpp

@@ -36,118 +36,128 @@ class class_ {
 
         // one __index to rule them all and in lua bind them
         lua_pushliteral(L, "__index");
-        lua_pushcfunction(L, index_);
+        lua_pushcfunction(L, lua_function<index_>::safe_invoke);
         lua_rawset(L, mt_idx);
 
         lua_pushliteral(L, "__newindex");
-        lua_pushcfunction(L, new_index);
+        lua_pushcfunction(L, lua_function<new_index>::safe_invoke);
         lua_rawset(L, mt_idx);
 
         user_data::add_destructing_functions(L, mt_idx);
-        function("delete", user_data::destruct);
+        function<user_data::destruct>("delete");
         lua_pop(L, 1); // pop metatable
     }
 
     template <typename... Args>
     class_& constructor(const std::string_view name) {
         static_assert(std::is_constructible_v<Type, Args...>, "class should be constructible with given arguments");
-        return constructor(name, ctor_wrapper<Type, Args...>::invoke);
+        return constructor<ctor_wrapper<Type, Args...>::invoke>(name);
     }
 
     template <typename... Args>
     class_& construct_shared(const std::string_view name) {
         static_assert(std::is_constructible_v<Type, Args...>, "class should be constructible with given arguments");
-        return constructor(name, shared_ctor_wrapper<Type, Args...>::invoke);
+        return constructor<shared_ctor_wrapper<Type, Args...>::invoke>(name);
     }
 
-    class_& constructor(const std::string_view name, lua_CFunction functor) {
+    template <lua_CFunction func>
+    class_& constructor(const std::string_view name) {
         _info->get_metatable(_L);
         value_mirror<std::string_view>::to_lua(_L, name);
-        lua_pushcfunction(_L, functor);
+        lua_pushcfunction(_L, lua_function<func>::safe_invoke);
         lua_rawset(_L, -3);
         lua_pop(_L, 1); // pop metatable
         return *this;
     }
 
     template <auto func>
+        requires(!std::is_same_v<decltype(func), lua_CFunction>)
     class_& function(const std::string_view name) {
-        return function(name, function_wrapper<decltype(func), func>::invoke);
+        return function<function_wrapper<decltype(func), func>::invoke>(name);
     }
 
-    class_& function(const std::string_view name, lua_CFunction luaFunction) {
-        _info->functions[std::string {name}] = luaFunction;
+    template <lua_CFunction func>
+    class_& function(const std::string_view name) {
+        _info->functions[std::string {name}] = lua_function<func>::safe_invoke;
         return *this;
     }
 
     template <auto func>
+        requires(!std::is_same_v<decltype(func), lua_CFunction>)
     class_& class_function(const std::string_view name) {
-        return class_function(name, class_function_wrapper<decltype(func), func>::invoke);
+        return class_function<class_function_wrapper<decltype(func), func>::invoke>(name);
     }
 
-    class_& class_function(const std::string_view name, lua_CFunction luaFunction) {
+    template <lua_CFunction func>
+    class_& class_function(const std::string_view name) {
         _info->get_metatable(_L);
         value_mirror<std::string_view>::to_lua(_L, name);
-        lua_pushcfunction(_L, luaFunction);
+        lua_pushcfunction(_L, lua_function<func>::safe_invoke);
         lua_rawset(_L, -3);
         lua_pop(_L, 1); // pop metatable
         return *this;
     }
 
     template <auto prop>
+        requires(std::is_member_pointer_v<decltype(prop)>)
     class_& property_readonly(const std::string_view name) {
-        return property_readonly(name, property_wrapper<get, decltype(prop), prop>::invoke);
+        return property_readonly<property_wrapper<get, decltype(prop), prop>::invoke>(name);
     }
 
-    class_& property_readonly(const std::string_view name, lua_CFunction getter_function) {
-        _info->properties.emplace(name, property_data(getter_function, nullptr));
+    template <lua_CFunction func>
+    class_& property_readonly(const std::string_view name) {
+        _info->properties.emplace(name, property_data(lua_function<func>::safe_invoke, nullptr));
         return *this;
     }
 
     template <auto prop>
+        requires(std::is_member_pointer_v<decltype(prop)>)
     class_& property(const std::string_view name) {
-        static_assert(std::is_member_pointer_v<decltype(prop)>);
         if constexpr (std::is_member_object_pointer_v<decltype(prop)>) {
-            return property(name,
-                            property_wrapper<get, decltype(prop), prop>::invoke,
-                            property_wrapper<set, decltype(prop), prop>::invoke);
+            return property<property_wrapper<get, decltype(prop), prop>::invoke,
+                            property_wrapper<set, decltype(prop), prop>::invoke>(name);
         } else {
-            return property(name, property_wrapper<get, decltype(prop), prop>::invoke, nullptr);
+            return property_readonly<property_wrapper<get, decltype(prop), prop>::invoke>(name);
         }
     }
 
     template <auto getter, auto setter>
+        requires(std::is_member_pointer_v<decltype(getter)> && std::is_member_pointer_v<decltype(setter)>)
     class_& property(const std::string_view name) {
-        static_assert(std::is_member_pointer_v<decltype(getter)>);
-        static_assert(std::is_member_pointer_v<decltype(setter)>);
-        return property(name,
-                        property_wrapper<get, decltype(getter), getter>::invoke,
-                        property_wrapper<set, decltype(setter), setter>::invoke);
+        return property<property_wrapper<get, decltype(getter), getter>::invoke,
+                        property_wrapper<set, decltype(setter), setter>::invoke>(name);
     }
 
-    class_& property(const std::string_view name, lua_CFunction getter_function, lua_CFunction setter_function) {
-        _info->properties.emplace(name, property_data(getter_function, setter_function));
+    template <lua_CFunction getter, lua_CFunction setter>
+    class_& property(const std::string_view name) {
+        _info->properties.emplace(name,
+                                  property_data(lua_function<getter>::safe_invoke, lua_function<setter>::safe_invoke));
         return *this;
     }
 
     template <auto getter>
+        requires(!std::is_same_v<decltype(getter), lua_CFunction>)
     class_& array_access() {
         return array_access(function_wrapper<decltype(getter), getter>::invoke);
     }
 
-    class_& array_access(lua_CFunction getter_function) {
-        _info->array_access_getter = getter_function;
+    template <lua_CFunction getter>
+    class_& array_access() {
+        _info->array_access_getter = lua_function<getter>::safe_invoke;
         return *this;
     }
 
     template <auto getter, auto setter>
+        requires(!std::is_same_v<decltype(getter), lua_CFunction> && !std::is_same_v<decltype(setter), lua_CFunction>)
     class_& array_access() {
-        return array_access(function_wrapper<decltype(getter), getter>::invoke,
-                            function_wrapper<decltype(setter), setter>::invoke);
+        return array_access<function_wrapper<decltype(getter), getter>::invoke,
+                            function_wrapper<decltype(setter), setter>::invoke>();
     }
 
-    class_& array_access(lua_CFunction getter_function, lua_CFunction setter_function) {
-        _info->array_access_getter = getter_function;
-        _info->array_access_setter = setter_function;
+    template <lua_CFunction getter, lua_CFunction setter>
+    class_& array_access() {
+        _info->array_access_getter = lua_function<getter>::safe_invoke;
+        _info->array_access_setter = lua_function<setter>::safe_invoke;
         return *this;
     }
 
@@ -168,12 +178,12 @@ class class_ {
         const bool is_integer = lua_isinteger(L, 2);
         const int key_type = lua_type(L, 2);
         if (!is_integer && key_type != LUA_TSTRING) {
-            luaL_error(L, "Key type should be integer or string, '%s' is provided.", lua_typename(L, key_type));
+            reportError("Key type should be integer or string, '%s' is provided.", lua_typename(L, key_type));
         }
 
         if (is_integer) {
             if (info->array_access_getter == nullptr) {
-                luaL_error(L, "Type '%s' does not provide array get access.", info->name.c_str());
+                reportError("Type '%s' does not provide array get access.", info->name.c_str());
             }
             return info->array_access_getter(L);
         }
@@ -183,7 +193,7 @@ class class_ {
         auto p_it = info->properties.find(key);
         if (p_it != info->properties.end()) {
             if (p_it->second.getter == nullptr) {
-                luaL_error(L, "Property named '%s' does not have a getter.", key.data());
+                reportError("Property named '%s' does not have a getter.", key.data());
             }
             return p_it->second.getter(L);
         }
@@ -221,12 +231,12 @@ class class_ {
         const bool is_integer = lua_isinteger(L, 2);
         const int key_type = lua_type(L, 2);
         if (!is_integer && key_type != LUA_TSTRING) {
-            luaL_error(L, "Key type should be integer or string, '%s' is provided.", lua_typename(L, key_type));
+            reportError("Key type should be integer or string, '%s' is provided.", lua_typename(L, key_type));
         }
 
         if (is_integer) {
             if (info->array_access_setter == nullptr) {
-                luaL_error(L, "Type '%s' does not provide array set access.", info->name.c_str());
+                reportError("Type '%s' does not provide array set access.", info->name.c_str());
             }
             return info->array_access_setter(L);
         }
@@ -236,7 +246,7 @@ class class_ {
         auto p_it = info->properties.find(key);
         if (p_it != info->properties.end()) {
             if (p_it->second.setter == nullptr) {
-                luaL_error(L, "Property '%s' is read only.", key.data());
+                reportError("Property '%s' is read only.", key.data());
             }
             return p_it->second.setter(L);
         }
@@ -255,14 +265,16 @@ class class_ {
     type_info* _info;
 };
 
-inline void function(lua_State* L, const std::string_view name, lua_CFunction luaFunction) {
-    lua_pushcfunction(L, luaFunction);
+template <lua_CFunction func>
+inline void function(lua_State* L, const std::string_view name) {
+    lua_pushcfunction(L, lua_function<func>::safe_invoke);
     lua_setglobal(L, name.data());
 }
 
 template <auto func>
+    requires(!std::is_same_v<decltype(func), lua_CFunction>)
 void function(lua_State* L, const std::string_view name) {
-    function(L, name, function_wrapper<decltype(func), func>::invoke);
+    function<function_wrapper<decltype(func), func>::invoke>(L, name);
 }
 
 } // namespace luabind

include/luabind/exception.hpp

@@ -1,6 +1,7 @@
 #ifndef LUABIND_EXCEPTION_HPP
 #define LUABIND_EXCEPTION_HPP
 
+#include <cstdarg>
 #include <exception>
 #include <string>
 #include <string_view>
@@ -21,6 +22,17 @@ class error : public std::exception {
     std::string _message;
 };
 
+// TODO replace with std::format when supported by compilers
+[[gnu::format(printf, 1, 2)]] inline void reportError(const char* fmt, ...) {
+    std::va_list args;
+    va_start(args, fmt);
+    constexpr size_t bufferSize = 1024;
+    char buffer[bufferSize];
+    std::vsnprintf(buffer, bufferSize, fmt, args);
+    va_end(args);
+    throw error {buffer};
+}
+
 } // namespace luabind
 
 #endif // LUABIND_EXCEPTION_HPP

include/luabind/mirror.hpp

@@ -53,9 +53,15 @@ struct value_mirror<T*> {
 
     static T* from_lua(lua_State* L, int idx) {
         auto ud = user_data::from_lua(L, idx);
+        if (ud == nullptr || ud->object == nullptr) {
+            return nullptr;
+        }
         auto p = dynamic_cast<T*>(ud->object);
-        if (ud->object != nullptr && p == nullptr) {
-            throw error("Invalid type");
+        if (p == nullptr) {
+            reportError("Argument at %i has invalid type. Need '%s' but got '%s'.",
+                        idx,
+                        type_storage::type_name<T>(L).data(),
+                        ud->info->name.c_str());
         }
         return p;
     }
@@ -84,14 +90,16 @@ struct value_mirror<std::shared_ptr<T>> {
         auto ud = user_data::from_lua(L, idx);
         auto sud = dynamic_cast<shared_user_data*>(ud);
         if (sud == nullptr) {
-            throw error("Can't get shared_ptr from user data");
+            reportError("Argument %i does not represent shared_ptr.", idx);
         }
         auto r = std::dynamic_pointer_cast<T>(sud->data);
         if (!r) {
-            throw error("Invalid type");
+            reportError("Argument at %i has invalid type. Need '%s' but got '%s'.",
+                        idx,
+                        type_storage::type_name<T>(L).data(),
+                        ud->info->name.c_str());
         }
         return r;
-        // return sud ? std::dynamic_pointer_cast<T>(sud->data) : cpp_type {};
     }
 };
 
@@ -116,7 +124,7 @@ struct value_mirror<bool> {
     static bool from_lua(lua_State* L, int idx) {
         int isb = lua_isboolean(L, idx);
         if (isb != 1) {
-            throw luabind::error("Provided argument is not a boolean");
+            reportError("Provided argument at %i is not a boolean.", idx);
         }
         int r = lua_toboolean(L, idx);
         return static_cast<bool>(r);
@@ -141,13 +149,13 @@ struct number_mirror {
     static raw_type from_lua(lua_State* L, int idx) {
         if constexpr (std::is_integral_v<raw_type>) {
             if (0 == lua_isinteger(L, idx)) {
-                throw luabind::error("Provided argument is not an integer.");
+                reportError("Provided argument at %i is not an integer.", idx);
             }
             return static_cast<raw_type>(lua_tointeger(L, idx));
         } else {
-            if (0 == lua_isnumber(L, idx)) {
+            if (lua_type(L, idx) != LUA_TNUMBER) {
                 // lua_error does longjmp, think about memory leak.
-                throw luabind::error("Provided argument is not a number.");
+                reportError("Provided argument at %i is not a number.", idx);
             }
             return static_cast<raw_type>(lua_tonumber(L, idx));
         }
@@ -186,7 +194,7 @@ struct value_mirror<std::string_view> {
 
     static std::string_view from_lua(lua_State* L, int idx) {
         if (lua_isstring(L, idx) == 0) {
-            throw luabind::error("Provided argument is not a string.");
+            reportError("Provided argument at %i is not a string.", idx);
         }
         size_t len;
         const char* lv = lua_tolstring(L, idx, &len);
@@ -227,10 +235,10 @@ struct value_mirror<std::pair<T, Y>> {
 
     static type from_lua(lua_State* L, int idx) {
         if (lua_istable(L, idx) == 0) {
-            throw luabind::error("Provided argument is not a table.");
+            reportError("Provided argument at %i for the pair is not a table.", idx);
         }
         if (lua_rawlen(L, idx) != 2) {
-            throw luabind::error("Table is of invalid length.");
+            reportError("Provided table at %i for the pair value has invalid length.", idx);
         }
         lua_rawgeti(L, idx, 1);
         T f = value_mirror<T>::from_lua(L, -1);