From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp63.i.mail.ru (smtp63.i.mail.ru [217.69.128.43]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 3102946970E for ; Tue, 14 Jan 2020 17:21:38 +0300 (MSK) Date: Tue, 14 Jan 2020 17:21:36 +0300 From: Sergey Ostanevich Message-ID: <20200114142136.GB547@tarantool.org> References: <20200110154805.94649-1-olegrok@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20200110154805.94649-1-olegrok@tarantool.org> Subject: Re: [Tarantool-patches] [PATCH v3] error: add __concat method to error object List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: olegrok@tarantool.org Cc: Oleg Babin , tarantool-patches@dev.tarantool.org Hi! Thanks for the patch! I wonder if you're interested in coverage of all Lua types in tests? On my side I found that decimals are not so fancy: tarantool> b=box.error.new(box.error.UNKNOWN) --- ... tarantool> a=require("decimal").new("1.5") --- ... tarantool> b..a --- - error: 'builtin/error.lua:165: attempt to concatenate ''string'' and ''struct 106''' ... unlike the uuid: tarantool> z=require("uuid").new() --- ... tarantool> b..z --- - error: 'builtin/error.lua:165: attempt to concatenate ''string'' and ''struct tt_uuid''' ... But this is not relevant to the patch, which is LGTM. Regards, Sergos On 10 Jan 18:48, olegrok@tarantool.org wrote: > From: Oleg Babin > > Usually functions return pair {nil, err} and expected that err is string. > Let's make the behaviour of error object closer to string > and define __concat metamethod. > > Closes #4489 > > The case of error "error_mt.__concat(): neither of args is an error" > is not covered by tests because of #4723 > --- > Changes in v3: > - Improve concatenation logic > - Add more test cases > Changes in v2: > - Added tests > > Issue: https://github.com/tarantool/tarantool/issues/4489 > Branch: olegrok/4489-concat-for-errors > > src/lua/error.lua | 11 +++++++ > test/box/misc.result | 75 +++++++++++++++++++++++++++++++++++++++++- > test/box/misc.test.lua | 24 +++++++++++++- > 3 files changed, 108 insertions(+), 2 deletions(-) > > diff --git a/src/lua/error.lua b/src/lua/error.lua > index d091ee48b..7f249864a 100644 > --- a/src/lua/error.lua > +++ b/src/lua/error.lua > @@ -160,9 +160,20 @@ local function error_index(err, key) > return error_methods[key] > end > > +local function error_concat(lhs, rhs) > + if ffi.istype('struct error', lhs) then > + return tostring(lhs) .. rhs > + elseif ffi.istype('struct error', rhs) then > + return lhs .. tostring(rhs) > + else > + error('error_mt.__concat(): neither of args is an error') > + end > +end > + > local error_mt = { > __index = error_index; > __tostring = error_message; > + __concat = error_concat; > }; > > ffi.metatype('struct error', error_mt); > diff --git a/test/box/misc.result b/test/box/misc.result > index 3ca4e31ac..5ac5e0f26 100644 > --- a/test/box/misc.result > +++ b/test/box/misc.result > @@ -199,6 +199,79 @@ box.error.new() > - error: 'Usage: box.error.new(code, args)' > ... > -- > +-- gh-4489: box.error has __concat metamethod > +-- > +test_run:cmd("push filter '(.builtin/.*.lua):[0-9]+' to '\\1'") > +--- > +- true > +... > +e = box.error.new(box.error.UNKNOWN) > +--- > +... > +'left side: ' .. e > +--- > +- 'left side: Unknown error' > +... > +e .. ': right side' > +--- > +- 'Unknown error: right side' > +... > +e .. nil > +--- > +- error: 'builtin/error.lua: attempt to concatenate local ''rhs'' (a nil value)' > +... > +nil .. e > +--- > +- error: 'builtin/error.lua: attempt to concatenate local ''lhs'' (a nil value)' > +... > +e .. box.NULL > +--- > +- error: 'builtin/error.lua: attempt to concatenate ''string'' and ''void *''' > +... > +box.NULL .. e > +--- > +- error: 'builtin/error.lua: attempt to concatenate ''void *'' and ''string''' > +... > +123 .. e > +--- > +- 123Unknown error > +... > +e .. 123 > +--- > +- Unknown error123 > +... > +e .. e > +--- > +- Unknown errorUnknown error > +... > +e .. {} > +--- > +- error: 'builtin/error.lua: attempt to concatenate local ''rhs'' (a table value)' > +... > +{} .. e > +--- > +- error: 'builtin/error.lua: attempt to concatenate local ''lhs'' (a table value)' > +... > +-1ULL .. e > +--- > +- error: 'builtin/error.lua: attempt to concatenate ''uint64_t'' and ''string''' > +... > +e .. -1ULL > +--- > +- error: 'builtin/error.lua: attempt to concatenate ''string'' and ''uint64_t''' > +... > +1LL .. e > +--- > +- error: 'builtin/error.lua: attempt to concatenate ''int64_t'' and ''string''' > +... > +e .. 1LL > +--- > +- error: 'builtin/error.lua: attempt to concatenate ''string'' and ''int64_t''' > +... > +e = nil > +--- > +... > +-- > -- System errors expose errno as a field. > -- > _, err = require('fio').open('not_existing_file') > @@ -1065,7 +1138,7 @@ error() > --- > - error: null > ... > --- A test case for bitwise operations > +-- A test case for bitwise operations > bit.lshift(1, 32) > --- > - 1 > diff --git a/test/box/misc.test.lua b/test/box/misc.test.lua > index c3e992a48..e1c2f990f 100644 > --- a/test/box/misc.test.lua > +++ b/test/box/misc.test.lua > @@ -59,6 +59,28 @@ e = box.error.new(box.error.CREATE_SPACE, "space", "error") > e > box.error.new() > > +-- > +-- gh-4489: box.error has __concat metamethod > +-- > +test_run:cmd("push filter '(.builtin/.*.lua):[0-9]+' to '\\1'") > +e = box.error.new(box.error.UNKNOWN) > +'left side: ' .. e > +e .. ': right side' > +e .. nil > +nil .. e > +e .. box.NULL > +box.NULL .. e > +123 .. e > +e .. 123 > +e .. e > +e .. {} > +{} .. e > +-1ULL .. e > +e .. -1ULL > +1LL .. e > +e .. 1LL > +e = nil > + > -- > -- System errors expose errno as a field. > -- > @@ -281,7 +303,7 @@ dostring('return abc') > dostring('return ...', 1, 2, 3) > -- A test case for Bug#1043804 lua error() -> server crash > error() > --- A test case for bitwise operations > +-- A test case for bitwise operations > bit.lshift(1, 32) > bit.band(1, 3) > bit.bor(1, 2) > -- > 2.23.0 >