From 5e5bcf37450d07f7f2812255bbd1df35d8e6ce75 Mon Sep 17 00:00:00 2001 From: Benoit Germain Date: Mon, 27 Oct 2025 08:43:22 +0100 Subject: verbose_errors improvement * Use std::format instead of sprintf for verbose errors when decoding table keys * Add a unit test for the different table key types --- src/_pch.hpp | 1 + src/intercopycontext.cpp | 23 ++++++++++---------- tests/lanes_as_upvalue.lua | 2 +- unit_tests/UnitTests.vcxproj | 1 + unit_tests/UnitTests.vcxproj.filters | 3 +++ unit_tests/legacy_tests.cpp | 3 ++- unit_tests/misc_tests.cpp | 9 ++++++++ unit_tests/scripts/misc/verbose_errors.lua | 34 ++++++++++++++++++++++++++++++ 8 files changed, 62 insertions(+), 14 deletions(-) create mode 100644 unit_tests/scripts/misc/verbose_errors.lua diff --git a/src/_pch.hpp b/src/_pch.hpp index a77b7f5..38489b6 100644 --- a/src/_pch.hpp +++ b/src/_pch.hpp @@ -8,6 +8,7 @@ #include #include #include +#include #include #include #ifndef __PROSPERO__ diff --git a/src/intercopycontext.cpp b/src/intercopycontext.cpp index 61ce08f..0ccd619 100644 --- a/src/intercopycontext.cpp +++ b/src/intercopycontext.cpp @@ -474,29 +474,28 @@ void InterCopyContext::interCopyKeyValuePair() const // for debug purposes, let's try to build a useful name if (luaW_type(L1, _key_i) == LuaType::STRING) { std::string_view const _key{ luaW_tostring(L1, _key_i) }; - size_t const _bufLen{ name.size() + _key.size() + 2 }; // +2 for separator dot and terminating 0 - _valPath = static_cast(alloca(_bufLen)); - sprintf(_valPath, "%s." STRINGVIEW_FMT, name.data(), (int) _key.size(), _key.data()); + _valPath = static_cast(alloca(name.size() + _key.size() + 2)); // +2 for separator dot and terminating 0 + *std::format_to(_valPath, "{}.{}", name, _key) = 0; } #if defined LUA_LNUM || LUA_VERSION_NUM >= 503 else if (lua_isinteger(L1, _key_i)) { lua_Integer const key{ lua_tointeger(L1, _key_i) }; - _valPath = (char*) alloca(name.size() + 32 + 3); // +3 for [] and terminating 0 - sprintf(_valPath, "%s[" LUA_INTEGER_FMT "]", name.data(), key); + _valPath = static_cast(alloca(name.size() + 32 + 3)); // +3 for [] and terminating 0 + *std::format_to(_valPath, "{}[{}]", name, key) = 0; } #endif // defined LUA_LNUM || LUA_VERSION_NUM >= 503 else if (luaW_type(L1, _key_i) == LuaType::NUMBER) { lua_Number const key{ lua_tonumber(L1, _key_i) }; - _valPath = (char*) alloca(name.size() + 32 + 3); // +3 for [] and terminating 0 - sprintf(_valPath, "%s[" LUA_NUMBER_FMT "]", name.data(), key); + _valPath = static_cast(alloca(name.size() + 32 + 3)); // +3 for [] and terminating 0 + *std::format_to(_valPath, "{}[{}]", name, key) = 0; } else if (luaW_type(L1, _key_i) == LuaType::LIGHTUSERDATA) { void* const _key{ lua_touserdata(L1, _key_i) }; - _valPath = (char*) alloca(name.size() + 16 + 5); // +5 for [U:] and terminating 0 - sprintf(_valPath, "%s[U:%p]", name.data(), _key); + _valPath = static_cast(alloca(name.size() + 16 + 5)); // +5 for [U:] and terminating 0 + *std::format_to(_valPath, "{}[U:{}]", name, _key) = 0; } else if (luaW_type(L1, _key_i) == LuaType::BOOLEAN) { int const _key{ lua_toboolean(L1, _key_i) }; - _valPath = (char*) alloca(name.size() + 8); // +8 for [], 'false' and terminating 0 - sprintf(_valPath, "%s[%s]", name.data(), _key ? "true" : "false"); + _valPath = static_cast(alloca(name.size() + 8)); // +8 for [], 'false' and terminating 0 + *std::format_to(_valPath, "{}[{}]", name, _key ? "true" : "false") = 0; } } @@ -1430,7 +1429,7 @@ InterCopyResult InterCopyContext::interCopy(int const n_) const for (StackIndex _i{ (L1_i != 0) ? L1_i.value() : (_top_L1 - n_ + 1) }, _j{ 1 }; _j <= n_; ++_i, ++_j) { char _tmpBuf[16]; if (U->verboseErrors) { - sprintf(_tmpBuf, "arg_%d", _j.value()); + *std::format_to(_tmpBuf, "arg#{}", _j.value()) = 0; _c.name = _tmpBuf; } _c.L1_i = SourceIndex{ _i.value() }; diff --git a/tests/lanes_as_upvalue.lua b/tests/lanes_as_upvalue.lua index beef22e..1c316e4 100644 --- a/tests/lanes_as_upvalue.lua +++ b/tests/lanes_as_upvalue.lua @@ -8,4 +8,4 @@ local g = lanes.gen( "*", { name = 'auto', error_trace_level = "extended"}, foo) -- this should raise an error as lanes.timer_lane is a Lane (a non-deep full userdata) local res, err = pcall( g) -print( "Generating lane yielded: ", tostring( res), tostring( err)) +assert(res == false and type(err) == "string", "got " .. tostring(res) .. " " .. tostring(err)) diff --git a/unit_tests/UnitTests.vcxproj b/unit_tests/UnitTests.vcxproj index 300eb9b..bb0a2af 100644 --- a/unit_tests/UnitTests.vcxproj +++ b/unit_tests/UnitTests.vcxproj @@ -1163,6 +1163,7 @@ + diff --git a/unit_tests/UnitTests.vcxproj.filters b/unit_tests/UnitTests.vcxproj.filters index 57d8918..9b3d023 100644 --- a/unit_tests/UnitTests.vcxproj.filters +++ b/unit_tests/UnitTests.vcxproj.filters @@ -159,5 +159,8 @@ Scripts\linda + + Scripts\misc + \ No newline at end of file diff --git a/unit_tests/legacy_tests.cpp b/unit_tests/legacy_tests.cpp index 84581d2..d66937c 100644 --- a/unit_tests/legacy_tests.cpp +++ b/unit_tests/legacy_tests.cpp @@ -33,7 +33,8 @@ MAKE_TEST_CASE(func_is_string) MAKE_TEST_CASE(irayo_closure) MAKE_TEST_CASE(irayo_recursive) MAKE_TEST_CASE(keeper) -//MAKE_TEST_CASE(linda_perf) +MAKE_TEST_CASE(lanes_as_upvalue) +// MAKE_TEST_CASE(linda_perf) MAKE_TEST_CASE(manual_register) MAKE_TEST_CASE(nameof) MAKE_TEST_CASE(objects) diff --git a/unit_tests/misc_tests.cpp b/unit_tests/misc_tests.cpp index a2199aa..3848a75 100644 --- a/unit_tests/misc_tests.cpp +++ b/unit_tests/misc_tests.cpp @@ -191,3 +191,12 @@ TEST_CASE("misc.convert_max_attempts.is_respected") " l:send('k', t)" // send the table, it should raise an error because the converter retries too many times ); } + +#define MAKE_TEST_CASE(DIR, FILE, CONDITION) \ + TEST_CASE("scripted_tests." #DIR "." #FILE) \ + { \ + FileRunner _runner(R"(.\unit_tests\scripts)"); \ + _runner.performTest(FileRunnerParam{ #DIR "/" #FILE, TestType::CONDITION }); \ + } + +MAKE_TEST_CASE(misc, verbose_errors, AssertNoLuaError) diff --git a/unit_tests/scripts/misc/verbose_errors.lua b/unit_tests/scripts/misc/verbose_errors.lua new file mode 100644 index 0000000..40948ca --- /dev/null +++ b/unit_tests/scripts/misc/verbose_errors.lua @@ -0,0 +1,34 @@ +local fixture = require "fixture" -- require fixture before lanes so that it is not registered and will cause transfer errors +local lanes = require("lanes").configure{verbose_errors = true} +local l = lanes.linda{name = "my linda"} + + +local do_test = function(key_) + local t = {[key_] = fixture.newuserdata()} + local b, e = pcall(l.send, l, "k", t) + assert(b == false and type(e) == "string") + local t_key = type(key_) + if t_key == "string" then + local x, y = string.find(e, "arg#2." .. key_, 1, true) + assert(x and y, "got " .. e) + elseif t_key == "boolean" then + local x, y = string.find(e, "arg#2[" .. tostring(key_) .. "]", 1, true) + assert(x and y, "got " .. e) + elseif t_key == "number" then + local x, y = string.find(e, "arg#2[" .. key_ .. "]", 1, true) + assert(x and y, "got " .. e) + elseif t_key == "userdata" then + local t_name + -- light userdata is formatted by std::format, where the pointer is written as a lowercase hex literal + local expected = "arg#2[U:0x" .. string.lower(string.match(tostring(key_), "userdata: (%x+)")) .. "]" + local x, y = string.find(e, expected, 1, true) + assert(x and y, "expecting " .. expected .. " got " .. e) + end +end + +do_test("bob") +do_test(true) +do_test(false) +do_test(42) +do_test(42.44) +do_test(lanes.null) -- cgit v1.2.3-55-g6feb