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