Skip to content

Commit

Permalink
Fix error reporting
Browse files Browse the repository at this point in the history
  • Loading branch information
@kunitoki
kunitoki committed Apr 17, 2023
1 parent d469f83 commit e3b672e
Show file tree
Hide file tree
Showing 4 changed files with 92 additions and 52 deletions.
56 changes: 9 additions & 47 deletions Source/LuaBridge/detail/CFunctions.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,19 +29,19 @@ namespace detail {
* @tparam Start Start index where stack variables are located in the lua stack.
*/
template <class T>
auto unwrap_argument_or_error(lua_State* L, std::size_t index)
auto unwrap_argument_or_error(lua_State* L, std::size_t index, std::size_t start)
{
auto result = Stack<T>::get(L, static_cast<int>(index));
auto result = Stack<T>::get(L, static_cast<int>(index + start));
if (! result)
luaL_error(L, "Error decoding argument #%d: %s", static_cast<int>(index), result.message().c_str());
raise_lua_error(L, "Error decoding argument #%d: %s", static_cast<int>(index + 1), result.message().c_str());

return std::move(*result);
}

template <class ArgsPack, std::size_t Start, std::size_t... Indices>
auto make_arguments_list_impl(lua_State* L, std::index_sequence<Indices...>)
{
return tupleize(unwrap_argument_or_error<std::tuple_element_t<Indices, ArgsPack>>(L, Start + Indices)...);
return tupleize(unwrap_argument_or_error<std::tuple_element_t<Indices, ArgsPack>>(L, Indices, Start)...);
}

template <class ArgsPack, std::size_t Start>
Expand Down Expand Up @@ -351,7 +351,7 @@ inline int read_only_error(lua_State* L)

s = s + "'" + lua_tostring(L, lua_upvalueindex(1)) + "' is read-only";

luaL_error(L, "%s", s.c_str());
raise_lua_error(L, "%s", s.c_str());

return 0;
}
Expand Down Expand Up @@ -399,26 +399,6 @@ struct property_getter<T, void>
}
};

#if 0
template <class T>
struct property_getter<std::reference_wrapper<T>, void>
{
static int call(lua_State* L)
{
LUABRIDGE_ASSERT(lua_islightuserdata(L, lua_upvalueindex(1)));

std::reference_wrapper<T>* ptr = static_cast<std::reference_wrapper<T>*>(lua_touserdata(L, lua_upvalueindex(1)));
LUABRIDGE_ASSERT(ptr != nullptr);

auto result = Stack<T&>::push(L, ptr->get());
if (! result)
luaL_error(L, "%s", result.message().c_str());

return 1;
}
};
#endif

/**
* @brief lua_CFunction to get a class data member.
*
Expand Down Expand Up @@ -505,24 +485,6 @@ struct property_setter<T, void>
}
};

#if 0
template <class T>
struct property_setter<std::reference_wrapper<T>, void>
{
static int call(lua_State* L)
{
LUABRIDGE_ASSERT(lua_islightuserdata(L, lua_upvalueindex(1)));

std::reference_wrapper<T>* ptr = static_cast<std::reference_wrapper<T>*>(lua_touserdata(L, lua_upvalueindex(1)));
LUABRIDGE_ASSERT(ptr != nullptr);

ptr->get() = Stack<T>::get(L, 1);

return 0;
}
};
#endif

/**
* @brief lua_CFunction to set a class data member.
*
Expand Down Expand Up @@ -1137,7 +1099,7 @@ int constructor_container_proxy(lua_State* L)

auto result = UserdataSharedHelper<C, false>::push(L, object);
if (! result)
luaL_error(L, "%s", result.message().c_str());
raise_lua_error(L, "%s", result.message().c_str());

return 1;
}
Expand All @@ -1153,7 +1115,7 @@ int constructor_placement_proxy(lua_State* L)
std::error_code ec;
auto* value = UserdataValue<T>::place(L, ec);
if (! value)
luaL_error(L, "%s", ec.message().c_str());
raise_lua_error(L, "%s", ec.message().c_str());

constructor<T, Args>::call(value->getObject(), std::move(args));

Expand Down Expand Up @@ -1184,7 +1146,7 @@ struct constructor_forwarder
std::error_code ec;
auto* value = UserdataValue<T>::place(L, ec);
if (! value)
luaL_error(L, "%s", ec.message().c_str());
raise_lua_error(L, "%s", ec.message().c_str());

T* obj = placement_constructor<T>::construct(
value->getObject(), m_func, std::move(args));
Expand Down Expand Up @@ -1221,7 +1183,7 @@ struct factory_forwarder
std::error_code ec;
auto* value = UserdataValueExternal<T>::place(L, obj, m_dealloc, ec);
if (! value)
luaL_error(L, "%s", ec.message().c_str());
raise_lua_error(L, "%s", ec.message().c_str());

return obj;
}
Expand Down
14 changes: 10 additions & 4 deletions Source/LuaBridge/detail/LuaHelpers.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -499,13 +499,19 @@ void* lua_newuserdata_aligned(lua_State* L, Args&&... args)
/**
* @brief Safe error able to walk backwards for error reporting correctly.
*/
inline int raise_lua_error(lua_State *L, const char *fmt, ...)
inline int raise_lua_error(lua_State* L, const char* fmt, ...)
{
va_list argp;
va_start(argp, fmt);
lua_pushvfstring(L, fmt, argp);
va_end(argp);

const char* message = lua_tostring(L, -1);
if (message != nullptr && std::string_view(message)[0] == '[')
return lua_error_x(L);

bool pushed_error = false;
for (int level = 2; level > 0; --level)
for (int level = 1; level <= 2; ++level)
{
lua_Debug ar;

Expand All @@ -529,8 +535,8 @@ inline int raise_lua_error(lua_State *L, const char *fmt, ...)
if (! pushed_error)
lua_pushliteral(L, "");

lua_pushvfstring(L, fmt, argp);
va_end(argp);
lua_pushvalue(L, -2);
lua_remove(L, -3);
lua_concat(L, 2);

return lua_error_x(L);
Expand Down
72 changes: 72 additions & 0 deletions Tests/Source/ClassTests.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2113,6 +2113,78 @@ TEST_F(ClassMetaMethods, metamethodsShouldNotBeWritable)
#endif
}

namespace {
struct StringGetter
{
std::string str;

const char* getString() const { return str.c_str(); }
void setString(const char* val) { str = val; }
};
} // namespace

TEST_F(ClassMetaMethods, ErrorLineWithProperties)
{
luabridge::getGlobalNamespace(L)
.beginClass<StringGetter>("StringGetter")
.addConstructor<void(*)()>()
.addFunction("setString", &StringGetter::setString)
.addProperty("str", &StringGetter::getString, &StringGetter::setString)
.endClass();

#if LUABRIDGE_HAS_EXCEPTIONS
try
{
runLua(R"(
local myStringGetter = StringGetter()
myStringGetter.str = 12
)");

EXPECT_TRUE(false);
}
catch (const std::exception& ex)
{
EXPECT_EQ(0, std::string_view(ex.what()).find("[string \"...\"]:3"));
EXPECT_EQ(std::string_view::npos, std::string_view(ex.what()).find("[string \"...\"]:3", 1));
}

try
{
runLua(R"(
local myStringGetter = StringGetter()
myStringGetter:setString(12)
)");

EXPECT_TRUE(false);
}
catch (const std::exception& ex)
{
EXPECT_EQ(0, std::string_view(ex.what()).find("[string \"...\"]:3"));
EXPECT_EQ(std::string_view::npos, std::string_view(ex.what()).find("[string \"...\"]:3", 1));
}
#endif

{
auto [result, errorString] = runLuaCaptureError(R"(
local myStringGetter = StringGetter()
myStringGetter.str = 12
)");

EXPECT_EQ(0, std::string_view(errorString).find("[string \"...\"]:3"));
EXPECT_EQ(std::string_view::npos, std::string_view(errorString).find("[string \"...\"]:3", 1));
}

{
auto [result, errorString] = runLuaCaptureError(R"(
local myStringGetter = StringGetter()
myStringGetter:setString(12)
)");

EXPECT_EQ(0, std::string_view(errorString).find("[string \"...\"]:3"));
EXPECT_EQ(std::string_view::npos, std::string_view(errorString).find("[string \"...\"]:3", 1));
}
}

TEST_F(ClassMetaMethods, SimulateArray)
{
using ContainerType = std::vector<std::string>;
Expand Down
2 changes: 1 addition & 1 deletion Tests/Source/TestBase.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ inline int luaL_loadstring(lua_State *L, const char *s)
[](char* x) { std::free(x); }
);

return luau_load(L, "code", bytecode.get(), bytecodeSize, 0);
return luau_load(L, "...", bytecode.get(), bytecodeSize, 0);
}
#endif

Expand Down

0 comments on commit e3b672e

Please sign in to comment.