<html><head><meta http-equiv="Content-Type" content="text/html; charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; line-break: after-white-space;" class="">Hi! Thank you for review! I addressed your comments and sent v2.<div class="">The only inconsistency is with macro names: I feel like LDECIMAL_BINOP and LDECIMAL_FUNC</div><div class="">are better, since the latter one is only used for functions now (ln, log10, sqrt, abs).<br class=""><div class=""><div class=""><div class=""><br class="webkit-block-placeholder"></div><div class="">
<div dir="auto" style="caret-color: rgb(0, 0, 0); color: rgb(0, 0, 0); letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; word-wrap: break-word; -webkit-nbsp-mode: space; line-break: after-white-space;" class=""><div>--</div><div>Serge Petrenko</div><div><a href="mailto:sergepetrenko@tarantool.org" class="">sergepetrenko@tarantool.org</a></div><div class=""><br class=""></div></div><br class="Apple-interchange-newline"><br class="Apple-interchange-newline">
</div>
<div><br class=""><blockquote type="cite" class=""><div class="">26 июня 2019 г., в 17:02, Vladimir Davydov <<a href="mailto:vdavydov.dev@gmail.com" class="">vdavydov.dev@gmail.com</a>> написал(а):</div><br class="Apple-interchange-newline"><div class=""><div class="">Pasting the whole patch here for review:<br class=""><br class=""><blockquote type="cite" class="">From 565aa457dfb40302797cdd1d9863559fe2c52468 Mon Sep 17 00:00:00 2001<br class="">From: Serge Petrenko <<a href="mailto:sergepetrenko@tarantool.org" class="">sergepetrenko@tarantool.org</a>><br class="">Date: Wed, 5 Jun 2019 15:58:49 +0300<br class="">Subject: [PATCH] decimal: expose decimal type to lua.<br class=""><br class="">Add a decimal library to lua.<br class=""><br class="">Part of #692<br class=""><br class="">@TarantoolBot document<br class="">Title: Document decimal module in lua.<br class=""><br class="">First of all, you have to require the package via<br class="">`decimal = require('decimal')`<br class="">Now you can construct decimals via `new` method.<br class="">Decimals may be constructed from lua numbers, strings, unsigned and<br class="">signed 64 bit integers.<br class="">Decimal is a fixed-point type with maximum 38 digits of precision. All<br class="">the calculations are exact, so, be careful when constructing decimals<br class="">from lua numbers: they may hold only 15 decimal digits of precision.<br class="">You are advised to construct decimals from strings, since strings<br class="">represent decimals exactly, and vice versa.<br class=""><br class="">```<br class="">a = decimal.new(123e-7)<br class="">b = decimal.new('123.456')<br class="">c = decimal.new('123.456e2')<br class="">d = decimal.new(123ULL)<br class="">e = decimal.new(2)<br class="">```<br class="">The allowed operations are addition, subtraction, division,<br class="">multiplication and power. If at least one of the operands is decimal,<br class="">decimal operations are performed. The other operand may be either<br class="">decimal or string, containing a number representation, or a lua number.<br class="">When the operation is called as `decimal.opname`, both operands may be<br class="">strings or lua numbers, e.g. `decimal.add(23, '123.456') == 146.456`:<br class=""></blockquote><br class="">There's no decimal.add anymore.<br class=""><br class=""><blockquote type="cite" class=""><br class="">Operations only fail on an overflow, i.e. when result exceeds 10^38 - 1.<br class="">This includes division by zero. In these cases an error `Operation<br class="">failed` is raised.<br class="">Underflow is also possible, when precision needed to store the exact<br class="">result exceeds 38 digits. Underflow is not an error. When an underflow<br class="">happens, the result is rounded to 38 digits of precision.<br class=""><br class="">```<br class="">tarantool> a + b<br class="">---<br class="">- '123.456012300000000'<br class="">...<br class=""><br class="">tarantool> c - d<br class="">---<br class="">- '12222.6'<br class="">...<br class=""><br class="">tarantool> c / b<br class="">---<br class="">- '100'<br class="">...<br class=""><br class="">tarantool> d * d<br class="">---<br class="">- '15129'<br class="">...<br class=""><br class="">tarantool> d ^ 2<br class="">---<br class="">- '15129'<br class="">...<br class=""><br class="">tarantool> 2 ^ d<br class="">---<br class="">- '10633823966279326983230456482242756608'<br class="">...<br class=""><br class="">tarantool> e ^ d<br class="">---<br class="">- '10633823966279326983230456482242756608'<br class="">...<br class="">```<br class=""></blockquote><br class="">It's unclear what a, b, c, d, e are in this example.<br class=""><br class=""><blockquote type="cite" class="">The following math functions are also supported:<br class="">log10, ln, exp, sqrt, pow. When specified as<br class="">`decimal.opname()`, operations may be performed on<br class="">strings and lua numbers.<br class="">```<br class="">f = decimal.new(100)<br class=""><br class="">tarantool> decimal.log10(f)<br class="">---<br class="">- '2'<br class="">...<br class=""><br class="">tarantool> decimal.sqrt(f)<br class="">---<br class="">- '10'<br class="">...<br class=""><br class="">tarantool> e2 = decimal.exp(2)<br class="">---<br class="">...<br class=""><br class="">tarantool> decimal.ln(e2)<br class="">---<br class="">- '2.0000000000000000000000000000000000000'<br class="">...<br class=""><br class="">There are also `abs` and `tostring` methods, and an unary minus<br class="">operator, which are pretty self-explanatory.<br class=""><br class="">```<br class="">tarantool> a = decimal.new(-5)<br class="">---<br class="">...<br class=""><br class="">tarantool> a<br class="">---<br class="">- '-5.000000000000000'<br class="">...<br class=""></blockquote><br class="">This looks weird. Can't we trim trailing zeros somehow?<br class=""><br class=""><blockquote type="cite" class=""><br class="">tarantool> decimal.abs(a)<br class="">---<br class="">- '5.000000000000000'<br class="">...<br class=""><br class="">tarantool> -a<br class="">---<br class="">- '5.000000000000000'<br class="">...<br class=""><br class="">tostring(a)<br class="">---<br class="">- '-5.000000000000000'<br class="">...<br class=""><br class="">```<br class=""><br class="">`decimal.precision`, `decimal.scale` and `decimal.round` :<br class="">The first two methods return precision, i.e. decimal digits in<br class="">number representation, and scale, i.e. decimal digits after the decimal<br class="">point in the number representation.<br class="">`decimal.round` rounds the number to the given scale.<br class="">```<br class="">tarantool> a = decimal.new('123.456789')<br class="">---<br class="">...<br class=""><br class="">tarantool> decimal.precision(a)<br class="">---<br class="">- 9<br class="">...<br class=""><br class="">tarantool> decimal.scale(a)<br class="">---<br class="">- 6<br class="">...<br class=""><br class="">tarantool> decimal.round(a, 4)<br class="">---<br class="">- '123.4568'<br class="">...<br class=""><br class="">```<br class=""><br class="">Comparsions: `>`, `<`, `>=`, `<=`, `==` are also legal and work as<br class="">expected. You may compare decimals with lua numbers or strings. In that<br class="">case comparsion will happen after the values are converted to decimal<br class="">type.<br class=""><br class="">diff --git a/src/lua/decimal.c b/src/lua/decimal.c<br class="">new file mode 100644<br class="">index 00000000..4dc0dff7<br class="">--- /dev/null<br class="">+++ b/src/lua/decimal.c<br class="">@@ -0,0 +1,335 @@<br class=""></blockquote><br class=""><blockquote type="cite" class="">+#define LDECIMAL_OP2(name, opname)\<br class="">+static int \<br class="">+ldecimal_##name(struct lua_State *L) {\<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span>if (lua_gettop(L) < 2)\<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>return luaL_error(L, "Usage: decimal."#name"(decimal, decimal)");\<br class=""></blockquote><br class="">There's no decimal_mul or decimal_add anymore...<br class=""><br class=""><blockquote type="cite" class="">+<span class="Apple-tab-span" style="white-space:pre"> </span>decimal_t *lhs = lua_todecimal(L, 1);\<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span>decimal_t *rhs = lua_todecimal(L, 2);\<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span>decimal_t *res = lua_pushdecimal(L);\<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span>if (decimal_##opname(res, lhs, rhs) == NULL) {\<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>lua_pop(L, 1);\<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>luaL_error(L, "Operation failed.");\<br class=""></blockquote><br class="">The error message isn't informative. Should be "Decimal overflow"<br class="">I think.<br class=""><br class=""><blockquote type="cite" class="">+<span class="Apple-tab-span" style="white-space:pre"> </span>}\<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span>return 1;\<br class="">+}<br class=""></blockquote><br class="">Please align backslash (\):<br class=""><br class="">#define LDECIMAL_OP2(name, opname)<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>\<br class="">static int<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>\<br class="">ldecimal_##name(struct luaState *L)<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>\<br class=""><br class="">and so on.<br class=""><br class="">Also, please add brief comments to all helper functions.<br class=""><br class=""><blockquote type="cite" class="">+#define LDECIMAL_UNOP(name, opname)\<br class=""></blockquote><br class="">I'd call them LDECIMAL_OP1 and LDECIMAL_OP2 or LDECIMAL_UNOP and<br class="">LDECIMAL_BINOP, for consistency.<br class=""><br class=""><blockquote type="cite" class="">+static int \<br class="">+ldecimal_##name(struct lua_State *L) {\<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span>if (lua_gettop(L) < 1)\<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>return luaL_error(L, "Usage: decimal."#name"(decimal)");\<br class=""></blockquote><br class="">There's no decimal.minus.<br class=""><br class=""><blockquote type="cite" class="">+<span class="Apple-tab-span" style="white-space:pre"> </span>decimal_t *lhs = lua_todecimal(L, 1);\<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span>decimal_t *res = lua_pushdecimal(L);\<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span>if (decimal_##opname(res, lhs) == NULL) {\<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>lua_pop(L, 1);\<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>luaL_error(L, "Operation failed");\<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span>}\<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span>return 1;\<br class="">+}<br class="">+<br class="">+#define LDECIMAL_CMPOP(name, cmp)\<br class="">+static int \<br class="">+ldecimal_##name(struct lua_State *L) {\<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span>if (lua_gettop(L) < 2)\<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>return luaL_error(L, "Usage: decimal.__"#name"(decimal, decimal)");\<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span>decimal_t *lhs = lua_todecimal(L, 1);\<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span>decimal_t *rhs = lua_todecimal(L, 2);\<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span>lua_pushboolean(L, decimal_compare(lhs, rhs) cmp 0);\<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span>return 1;\<br class="">+}<br class="">+<br class="">+uint32_t CTID_DECIMAL;<br class=""></blockquote><br class="">Should be static.<br class=""><br class=""><blockquote type="cite" class="">+<br class="">+static decimal_t *<br class="">+lua_pushdecimal(struct lua_State *L)<br class="">+{<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span>decimal_t *res = luaL_pushcdata(L, CTID_DECIMAL);<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span>return res;<br class="">+}<br class="">+<br class="">+static decimal_t *<br class="">+lua_checkdecimal(struct lua_State *L, int index)<br class="">+{<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span>uint32_t ctypeid;<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span>decimal_t *res = luaL_checkcdata(L, index, &ctypeid);<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span>if (ctypeid != CTID_DECIMAL)<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>luaL_error(L, "Expected decimal as %d argument", index);<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span>return res;<br class="">+}<br class="">+<br class="">+static decimal_t *<br class="">+lua_todecimal(struct lua_State *L, int index)<br class="">+{<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span>/*<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span> * Convert the index, if it is given relative to the top.<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span> * Othervise it will point to a wrong position after<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span> * pushdecimal().<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span> */<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span>if (index < 0)<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>index = lua_gettop(L) + index + 1;<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span>decimal_t *res = lua_pushdecimal(L);<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span>switch(lua_type(L, index))<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span>{<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span>case LUA_TNUMBER:<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span>{<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>double n = lua_tonumber(L, index);<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>if (decimal_from_double(res, n) == NULL)<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>goto err;<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>break;<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span>}<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span>case LUA_TSTRING:<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span>{<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>const char *str = lua_tostring(L, index);<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>if (decimal_from_string(res, str) == NULL)<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>goto err;<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>break;<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span>}<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span>case LUA_TCDATA:<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span>{<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>uint32_t ctypeid;<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>void *cdata = luaL_checkcdata(L, index, &ctypeid);<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>int64_t ival;<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>uint64_t uval;<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>double d;<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>if (ctypeid == CTID_DECIMAL) {<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>/*<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span> * We already have a decimal at the<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span> * desired position.<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span> */<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>lua_pop(L, 1);<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>return (decimal_t *) cdata;<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>}<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>switch (ctypeid)<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>{<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>case CTID_CCHAR:<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>case CTID_INT8:<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>ival = *(int8_t *) cdata;<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>if (decimal_from_int64(res, ival) == NULL)<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>goto err;<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>break;<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>case CTID_INT16:<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>ival = *(int16_t *) cdata;<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>if (decimal_from_int64(res, ival) == NULL)<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>goto err;<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>break;<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>case CTID_INT32:<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>ival = *(int32_t *) cdata;<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>if (decimal_from_int64(res, ival) == NULL)<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>goto err;<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>break;<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>case CTID_INT64:<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>ival = *(int64_t *) cdata;<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>if (decimal_from_int64(res, ival) == NULL)<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>goto err;<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>break;<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>case CTID_UINT8:<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>uval = *(uint8_t *) cdata;<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>if (decimal_from_uint64(res, uval) == NULL)<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>goto err;<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>break;<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>case CTID_UINT16:<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>uval = *(uint16_t *) cdata;<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>if (decimal_from_uint64(res, uval) == NULL)<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>goto err;<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>break;<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>case CTID_UINT32:<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>uval = *(uint32_t *) cdata;<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>if (decimal_from_uint64(res, uval) == NULL)<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>goto err;<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>break;<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>case CTID_UINT64:<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>uval = *(uint64_t *) cdata;<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>if (decimal_from_uint64(res, uval) == NULL)<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>goto err;<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>break;<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>case CTID_FLOAT:<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>d = *(float *) cdata;<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>if (decimal_from_double(res, d) == NULL)<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>goto err;<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>break;<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>case CTID_DOUBLE:<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>d = *(double *) cdata;<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>if (decimal_from_double(res, d) == NULL)<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>goto err;<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>break;<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>default:<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>lua_pop(L, 1);<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>luaL_error(L, "expected decimal, number or string as %d argument", index);<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>}<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>break;<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span>}<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span>default:<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>lua_pop(L, 1);<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>luaL_error(L, "expected decimal, number or string as %d argument", index);<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span>}<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span>lua_replace(L, index);<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span>return res;<br class="">+err:<span class="Apple-tab-span" style="white-space:pre"> </span>/* pop the decimal we prepared on top of the stack. */<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span>lua_pop(L, 1);<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span>luaL_error(L, "Incorrect value to convert to decimal as %d argument", index);<br class=""></blockquote><br class="">Too long lines. Please split.<br class=""><br class="">Also, sometimes you start an error message with a lower-case letter,<br class="">sometimes with an upper-case. Please be consistent.<br class=""><br class=""><blockquote type="cite" class="">+<span class="Apple-tab-span" style="white-space:pre"> </span>/* luaL_error never returns, this is to silence compiler warning. */<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span>return NULL;<br class="">+}<br class="">+<br class="">+LDECIMAL_OP2(add, add);<br class="">+LDECIMAL_OP2(sub, sub);<br class="">+LDECIMAL_OP2(mul, mul);<br class="">+LDECIMAL_OP2(div, div);<br class="">+LDECIMAL_OP2(pow, pow);<br class="">+<br class="">+LDECIMAL_UNOP(log10, log10);<br class="">+LDECIMAL_UNOP(ln, ln);<br class="">+LDECIMAL_UNOP(exp, exp);<br class="">+LDECIMAL_UNOP(sqrt, sqrt);<br class="">+LDECIMAL_UNOP(minus, minus);<br class="">+LDECIMAL_UNOP(abs, abs);<br class="">+<br class="">+LDECIMAL_CMPOP(eq, ==);<br class="">+LDECIMAL_CMPOP(lt, <);<br class="">+LDECIMAL_CMPOP(le, <=);<br class=""></blockquote><br class="">Semicolons (;) are not needed here, as these macros define functions.<br class=""><br class=""><blockquote type="cite" class="">+<br class="">+static int<br class="">+ldecimal_new(struct lua_State *L)<br class="">+{<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span>if (lua_gettop(L) < 1)<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>luaL_error(L, "Usage: decimal.new(value)");<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span>lua_todecimal(L, 1);<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span>decimal_t *lhs = lua_checkdecimal(L, 1);<br class=""></blockquote><br class="">Why not simply<br class=""><br class=""><span class="Apple-tab-span" style="white-space:pre"> </span>decimal_t lhs = lua_todecimal(L, 1);<br class=""><br class="">?<br class=""><br class=""><blockquote type="cite" class="">+<span class="Apple-tab-span" style="white-space:pre"> </span>decimal_t *res = lua_pushdecimal(L);<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span>*res = *lhs;<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span>return 1;<br class="">+}<br class=""></blockquote><br class=""><blockquote type="cite" class="">diff --git a/src/lua/decimal.h b/src/lua/decimal.h<br class="">new file mode 100644<br class="">index 00000000..12b29d39<br class="">--- /dev/null<br class="">+++ b/src/lua/decimal.h<br class="">@@ -0,0 +1,39 @@<br class=""></blockquote><br class=""><blockquote type="cite" class="">+#ifndef TARANTOOL_LUA_DECIMAL_H_INCLUDED<br class="">+#define TARANTOOL_LUA_DECIMAL_H_INCLUDED<br class=""></blockquote><br class="">You forgot to add extern "C".<br class=""><br class=""><blockquote type="cite" class="">+<br class="">+struct lua_State;<br class="">+<br class="">+void<br class="">+tarantool_lua_decimal_init(struct lua_State *L);<br class="">+<br class="">+#endif /* TARANTOOL_LUA_DECIMAL_H_INCLUDED */<br class=""></blockquote><br class=""><blockquote type="cite" class="">diff --git a/test/app/decimal.test.lua b/test/app/decimal.test.lua<br class="">new file mode 100644<br class="">index 00000000..b101f07b<br class="">--- /dev/null<br class="">+++ b/test/app/decimal.test.lua<br class="">@@ -0,0 +1,42 @@<br class="">+decimal = require('decimal')<br class="">+test_run = require('test_run').new()<br class="">+<br class="">+-- check various constructors<br class="">+decimal.new('1234.5678')<br class="">+decimal.new('1e6')<br class="">+decimal.new('-6.234612e2')<br class="">+decimal.new(tonumber64(2^63))<br class="">+decimal.new(12345678ULL)<br class="">+decimal.new(-12345678LL)<br class="">+decimal.new(1)<br class="">+decimal.new(-1)<br class="">+decimal.new(2^64)<br class="">+decimal.new(2^(-20))<br class="">+<br class="">+a = decimal.new('100')<br class="">+decimal.log10(a)<br class="">+decimal.ln(a)<br class="">+-- overflow, e^100 > 10^38<br class="">+decimal.exp(a)<br class="">+-- e^87 < 10^38, no overflow, but rounds<br class="">+-- to 0 digits after the decimal point.<br class="">+decimal.exp(decimal.new(87))<br class="">+decimal.sqrt(a)<br class="">+a<br class="">+a = decimal.new('-123.456')<br class="">+decimal.round(a, 2)<br class="">+decimal.abs(a)<br class="">+a<br class="">+-a<br class="">+a / 10<br class="">+a * 5<br class="">+a + 17<br class="">+a - 0.0001<br class="">+a ^ 2<br class="">+decimal.abs(a) ^ 0.5<br class="">+decimal.abs(a) ^ 0.5 == decimal.sqrt(decimal.abs(a))<br class="">+a - 2 < a - 1<br class="">+a + 1e-10 > a<br class="">+a <= a<br class="">+a >= a<br class="">+a == a<br class=""></blockquote><br class="">Please add more tests:<br class=""><br class=""> - All possible errors thrown by the code checking arguments.<br class=""> - All overflow errors, resulting from both unary and binary operators.<br class=""> - Creating a decimal from all kinds of objects (UINT/INT/DOUBLE...)<br class=""><br class="">In other words, I want to see that every single line of you code is<br class="">covered:<br class=""><br class=""><a href="https://coveralls.io/builds/24193215/source?filename=src/lua/decimal.c" class="">https://coveralls.io/builds/24193215/source?filename=src/lua/decimal.c</a><br class=""><br class="">Also, it might be worth checking that none of the functions occasionally<br class="">modifies the decimal passed to it (i.e. it does create a new object).<br class=""><br class=""></div></div></blockquote></div><br class=""></div></div></div></body></html>