Tarantool development patches archive
 help / color / mirror / Atom feed
* [PATCH 0/2] decimal: expose decimal module to lua.
@ 2019-06-19 15:58 Serge Petrenko
  2019-06-19 15:58 ` [PATCH 1/2] lua/utils: add a function to register FFI metatypes Serge Petrenko
  2019-06-19 15:58 ` [PATCH 2/2] decimal: expose decimal type to lua Serge Petrenko
  0 siblings, 2 replies; 8+ messages in thread
From: Serge Petrenko @ 2019-06-19 15:58 UTC (permalink / raw)
  To: vdavydov.dev; +Cc: tarantool-patches, Serge Petrenko

https://github.com/tarantool/tarantool/issues/692
https://github.com/tarantool/tarantool/tree/sp/gh-692-decimal-lua

This patchset adds decimal module to lua.

Since FFI CDATA is chosen to store decimals on lua stack, a new helper to
register FFI metatypes is added in the first patch.

The second patch does the job itself of exposing the decimal module to lua and
adds a minor tes. The patch also contains a documentation request in the commit
message.

Serge Petrenko (2):
  lua/utils: add a function to register FFI metatypes.
  decimal: expose decimal type to lua.

 src/CMakeLists.txt        |   1 +
 src/lua/decimal.c         | 327 ++++++++++++++++++++++++++++++++++++++
 src/lua/decimal.h         |  39 +++++
 src/lua/init.c            |   2 +
 src/lua/utils.c           |  28 ++++
 src/lua/utils.h           |  13 ++
 test/app/decimal.result   | 172 ++++++++++++++++++++
 test/app/decimal.test.lua |  50 ++++++
 8 files changed, 632 insertions(+)
 create mode 100644 src/lua/decimal.c
 create mode 100644 src/lua/decimal.h
 create mode 100644 test/app/decimal.result
 create mode 100644 test/app/decimal.test.lua

-- 
2.20.1 (Apple Git-117)

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 1/2] lua/utils: add a function to register FFI metatypes.
  2019-06-19 15:58 [PATCH 0/2] decimal: expose decimal module to lua Serge Petrenko
@ 2019-06-19 15:58 ` Serge Petrenko
  2019-06-19 15:58 ` [PATCH 2/2] decimal: expose decimal type to lua Serge Petrenko
  1 sibling, 0 replies; 8+ messages in thread
From: Serge Petrenko @ 2019-06-19 15:58 UTC (permalink / raw)
  To: vdavydov.dev; +Cc: tarantool-patches, Serge Petrenko

A ffi metatype has a CTypeID, which can be used to push cdata of the
type on the lua stack, and has an associated metatable, automatically
applied to every created member of the type.
This allows the behavior similar to pushing userdata and assigning a
metatable to it.

Needed for #692
---
 src/lua/utils.c | 28 ++++++++++++++++++++++++++++
 src/lua/utils.h | 13 +++++++++++++
 2 files changed, 41 insertions(+)

diff --git a/src/lua/utils.c b/src/lua/utils.c
index 01a0cd894..0a4bcf517 100644
--- a/src/lua/utils.c
+++ b/src/lua/utils.c
@@ -141,6 +141,34 @@ luaL_ctypeid(struct lua_State *L, const char *ctypename)
 	return ctypeid;
 }
 
+uint32_t
+luaL_metatype(struct lua_State *L, const char *ctypename,
+	      const struct luaL_Reg *methods)
+{
+	/* Create a metatable for our ffi metatype. */
+	luaL_register_type(L, ctypename, methods);
+	int idx = lua_gettop(L);
+	/*
+	 * Get ffi.metatype function. It is like typeof with
+	 * an additional effect of registering a metatable for
+	 * all the cdata objects of the type.
+	 */
+	luaL_loadstring(L, "return require('ffi').metatype");
+	lua_call(L, 0, 1);
+	assert(lua_gettop(L) == idx + 1 && lua_isfunction(L, idx + 1));
+	lua_pushstring(L, ctypename);
+	/* Push the freshly created metatable as the second parameter. */
+	luaL_getmetatable(L, ctypename);
+	assert(lua_gettop(L) == idx + 3 && lua_istable(L, idx + 3));
+	lua_call(L, 2, 1);
+	uint32_t ctypetypeid;
+	CTypeID ctypeid = *(CTypeID *)luaL_checkcdata(L, idx + 1, &ctypetypeid);
+	assert(ctypetypeid == CTID_CTYPEID);
+
+	lua_settop(L, idx);
+	return ctypeid;
+}
+
 int
 luaL_cdef(struct lua_State *L, const char *what)
 {
diff --git a/src/lua/utils.h b/src/lua/utils.h
index 943840ec0..7e7cdc0c6 100644
--- a/src/lua/utils.h
+++ b/src/lua/utils.h
@@ -129,6 +129,19 @@ luaL_cdef(struct lua_State *L, const char *ctypename);
 
 /** \endcond public */
 
+/**
+ * @brief Return CTypeID (FFI) of given CDATA type,
+ * register a metatable with \a methods to be
+ * associated with every value of the given
+ * type on its creation iva FFI.
+ * @sa luaL_register_type
+ * @sa luaL_ctypeid
+ * @return CTypeID
+ */
+uint32_t
+luaL_metatype(struct lua_State *L, const char *ctypename,
+	      const struct luaL_Reg *methods);
+
 static inline lua_Integer
 luaL_arrlen(struct lua_State *L, int idx)
 {
-- 
2.20.1 (Apple Git-117)

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 2/2] decimal: expose decimal type to lua.
  2019-06-19 15:58 [PATCH 0/2] decimal: expose decimal module to lua Serge Petrenko
  2019-06-19 15:58 ` [PATCH 1/2] lua/utils: add a function to register FFI metatypes Serge Petrenko
@ 2019-06-19 15:58 ` Serge Petrenko
  2019-06-20 11:02   ` Serge Petrenko
  2019-06-21 15:53   ` Vladimir Davydov
  1 sibling, 2 replies; 8+ messages in thread
From: Serge Petrenko @ 2019-06-19 15:58 UTC (permalink / raw)
  To: vdavydov.dev; +Cc: tarantool-patches, Serge Petrenko

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 `tonumber` 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.tonumber(123e-7)
b = decimal.tonumber('123.456')
c = decimal.tonumber('123.456e2')
d = decimal.tonumber(123ULL)
e = decimal.tonumber(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`:
```
tarantool> a + b
---
- '123.456012300000000'
...

tarantool> decimal.add(a,b)
---
- '123.456012300000000'
...

tarantool> c - d
---
- '12222.6'
...

tarantool> decimal.sub(c,d)
---
- '12222.6'
...

tarantool> c / b
---
- '100'
...

tarantool> decimal.div(c, b)
---
- '100'
...

tarantool> d * d
---
- '15129'
...

tarantool> decimal.mul(d, d)
---
- '15129'
...

tarantool> d ^ 2
---
- '15129'
...

tarantool> 2 ^ d
---
- '10633823966279326983230456482242756608'
...

tarantool> e ^ d
---
- '10633823966279326983230456482242756608'
...
```
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.tonumber(100)
tarantool> f:log10()
---
- '2'
...

tarantool> decimal.log10(f)
---
- '2'
...

tarantool> f:sqrt()
---
- '10'
...

tarantool> decimal.sqrt(f)
---
- '10'
...

tarantool> e2 = decimal.exp(2)
---
...

tarantool> e2:ln()
---
- '2.0000000000000000000000000000000000000'
...

tarantool> decimal.ln(e2)
---
- '2.0000000000000000000000000000000000000'
...

tarantool> e:exp() == e2
---
- true
...

tarantool> e:exp():ln() == e
---
- true
...

```

There are also `abs`, `minus` and `tostring` methods, which are pretty
self-explanatory.

```
tarantool> a = decimal.tonumber(-5)
---
...

tarantool> a
---
- '-5.000000000000000'
...

tarantool> a:abs()
---
- '5.000000000000000'
...

tarantool> decimal.abs(a)
---
- '5.000000000000000'
...

tarantool> a:minus()
---
- '5.000000000000000'
...

tarantool> -a
---
- '5.000000000000000'
...

tarantool> decimal.minus(a)
---
- '5.000000000000000'
...

tarantool> a:tostring()
---
- '-5.000000000000000'
...

tarantool> decimal.tostring(a)
---
- '-5.000000000000000'
...

```

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.
---
 src/CMakeLists.txt        |   1 +
 src/lua/decimal.c         | 327 ++++++++++++++++++++++++++++++++++++++
 src/lua/decimal.h         |  39 +++++
 src/lua/init.c            |   2 +
 test/app/decimal.result   | 172 ++++++++++++++++++++
 test/app/decimal.test.lua |  50 ++++++
 6 files changed, 591 insertions(+)
 create mode 100644 src/lua/decimal.c
 create mode 100644 src/lua/decimal.h
 create mode 100644 test/app/decimal.result
 create mode 100644 test/app/decimal.test.lua

diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
index 33b64f6a6..acd719e9b 100644
--- a/src/CMakeLists.txt
+++ b/src/CMakeLists.txt
@@ -120,6 +120,7 @@ set (server_sources
      lua/string.c
      lua/buffer.c
      lua/swim.c
+     lua/decimal.c
      ${lua_sources}
      ${PROJECT_SOURCE_DIR}/third_party/lua-yaml/lyaml.cc
      ${PROJECT_SOURCE_DIR}/third_party/lua-yaml/b64.c
diff --git a/src/lua/decimal.c b/src/lua/decimal.c
new file mode 100644
index 000000000..72b458519
--- /dev/null
+++ b/src/lua/decimal.c
@@ -0,0 +1,327 @@
+/*
+ * Copyright 2010-2019, Tarantool AUTHORS, please see AUTHORS file.
+ *
+ * Redistribution and use in source and binary forms, with or
+ * without modification, are permitted provided that the following
+ * conditions are met:
+ *
+ * 1. Redistributions of source code must retain the above
+ *    copyright notice, this list of conditions and the
+ *    following disclaimer.
+ *
+ * 2. Redistributions in binary form must reproduce the above
+ *    copyright notice, this list of conditions and the following
+ *    disclaimer in the documentation and/or other materials
+ *    provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY <COPYRIGHT HOLDER> ``AS IS'' AND
+ * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED
+ * TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL
+ * <COPYRIGHT HOLDER> OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT,
+ * INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
+ * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+ * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR
+ * BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF
+ * LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF
+ * THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+ * SUCH DAMAGE.
+ */
+
+#include "lua/decimal.h"
+#include "lib/core/decimal.h"
+#include "lua/utils.h"
+
+#include <lua.h>
+#include <lauxlib.h>
+#include <lualib.h>
+
+#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)");\
+	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.");\
+	}\
+	return 1;\
+}
+#define LDECIMAL_UNOP(name, opname)\
+static int \
+ldecimal_##name(struct lua_State *L) {\
+	if (lua_gettop(L) < 1)\
+		return luaL_error(L, "Usage: decimal."#name"(decimal)");\
+	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;
+
+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);
+	/* 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, <=);
+
+static int
+ldecimal_tonumber(struct lua_State *L)
+{
+	if (lua_gettop(L) < 1)
+		luaL_error(L, "Usage: decimal.tonumber(value)");
+	lua_todecimal(L, 1);
+	decimal_t *lhs = lua_checkdecimal(L, 1);
+	decimal_t *res = lua_pushdecimal(L);
+	*res = *lhs;
+	return 1;
+}
+
+static int
+ldecimal_round(struct lua_State *L)
+{
+	if (lua_gettop(L) < 2)
+		return luaL_error(L, "Usage: decimal.round(decimal, scale)");
+	decimal_t *lhs = lua_checkdecimal(L, 1);
+	int n = lua_tointeger(L, 2);
+	decimal_t *res = lua_pushdecimal(L);
+	*res = *lhs;
+	decimal_round(res, n);
+	return 1;
+}
+
+static int
+ldecimal_tostring(struct lua_State *L)
+{
+	if (lua_gettop(L) < 1)
+		return luaL_error(L, "Usage: decimal.tostring(decimal)");
+	decimal_t *lhs = lua_checkdecimal(L, 1);
+	lua_pushstring(L, decimal_to_string(lhs));
+	return 1;
+}
+
+static const luaL_Reg ldecimal_mt[] = {
+	{"log10", ldecimal_log10},
+	{"ln", ldecimal_ln},
+	{"exp", ldecimal_exp},
+	{"sqrt", ldecimal_sqrt},
+	{"round", ldecimal_round},
+	{"minus", ldecimal_minus},
+	{"abs", ldecimal_abs},
+	{"tostring", ldecimal_tostring},
+	{"__unm", ldecimal_minus},
+	{"__add", ldecimal_add},
+	{"__sub", ldecimal_sub},
+	{"__mul", ldecimal_mul},
+	{"__div", ldecimal_div},
+	{"__pow", ldecimal_pow},
+	{"__eq", ldecimal_eq},
+	{"__lt", ldecimal_lt},
+	{"__le", ldecimal_le},
+	{"__tostring", ldecimal_tostring},
+	{NULL, NULL}
+};
+
+static const luaL_Reg ldecimal_lib[] = {
+	{"eq", ldecimal_eq},
+	{"lt", ldecimal_lt},
+	{"le", ldecimal_le},
+	{"add", ldecimal_add},
+	{"sub", ldecimal_sub},
+	{"mul", ldecimal_mul},
+	{"div", ldecimal_div},
+	{"log10", ldecimal_log10},
+	{"ln", ldecimal_ln},
+	{"pow", ldecimal_pow},
+	{"exp", ldecimal_exp},
+	{"sqrt", ldecimal_sqrt},
+	{"round", ldecimal_round},
+	{"minus", ldecimal_minus},
+	{"abs", ldecimal_abs},
+	{"tostring", ldecimal_tostring},
+	{"tonumber", ldecimal_tonumber},
+	{NULL, NULL}
+};
+
+void
+tarantool_lua_decimal_init(struct lua_State *L)
+{
+	int rc = luaL_cdef(L, "typedef struct {"
+				       "int32_t digits;"
+				       "int32_t exponent;"
+				       "uint8_t bits;"
+				       "uint16_t lsu[13];"
+			      "} decimal_t;");
+	assert(rc == 0);
+	luaL_register_module(L, "decimal", ldecimal_lib);
+	lua_pop(L, 1);
+	/*
+	 * luaL_metatype is similar to luaL_ctypeid +
+	 * luaL_register_type.
+	 * The metatable is set automatically to every
+	 * cdata of the new ctypeid ever created via ffi.
+	 */
+	CTID_DECIMAL = luaL_metatype(L, "decimal_t", ldecimal_mt);
+	assert(CTID_DECIMAL != 0);
+}
diff --git a/src/lua/decimal.h b/src/lua/decimal.h
new file mode 100644
index 000000000..12b29d392
--- /dev/null
+++ b/src/lua/decimal.h
@@ -0,0 +1,39 @@
+/*
+ * Copyright 2010-2019, Tarantool AUTHORS, please see AUTHORS file.
+ *
+ * Redistribution and use in source and binary forms, with or
+ * without modification, are permitted provided that the following
+ * conditions are met:
+ *
+ * 1. Redistributions of source code must retain the above
+ *    copyright notice, this list of conditions and the
+ *    following disclaimer.
+ *
+ * 2. Redistributions in binary form must reproduce the above
+ *    copyright notice, this list of conditions and the following
+ *    disclaimer in the documentation and/or other materials
+ *    provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY <COPYRIGHT HOLDER> ``AS IS'' AND
+ * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED
+ * TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL
+ * <COPYRIGHT HOLDER> OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT,
+ * INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
+ * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+ * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR
+ * BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF
+ * LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF
+ * THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+ * SUCH DAMAGE.
+ */
+#ifndef TARANTOOL_LUA_DECIMAL_H_INCLUDED
+#define TARANTOOL_LUA_DECIMAL_H_INCLUDED
+
+struct lua_State;
+
+void
+tarantool_lua_decimal_init(struct lua_State *L);
+
+#endif /* TARANTOOL_LUA_DECIMAL_H_INCLUDED */
diff --git a/src/lua/init.c b/src/lua/init.c
index 9982828d9..fbaedd4cd 100644
--- a/src/lua/init.c
+++ b/src/lua/init.c
@@ -59,6 +59,7 @@
 #include "lua/httpc.h"
 #include "lua/utf8.h"
 #include "lua/swim.h"
+#include "lua/decimal.h"
 #include "digest.h"
 #include <small/ibuf.h>
 
@@ -454,6 +455,7 @@ tarantool_lua_init(const char *tarantool_bin, int argc, char **argv)
 	tarantool_lua_pickle_init(L);
 	tarantool_lua_digest_init(L);
 	tarantool_lua_swim_init(L);
+	tarantool_lua_decimal_init(L);
 	luaopen_http_client_driver(L);
 	lua_pop(L, 1);
 	luaopen_msgpack(L);
diff --git a/test/app/decimal.result b/test/app/decimal.result
new file mode 100644
index 000000000..93c1c7557
--- /dev/null
+++ b/test/app/decimal.result
@@ -0,0 +1,172 @@
+decimal = require('decimal')
+---
+...
+test_run = require('test_run').new()
+---
+...
+-- check various constructors
+decimal.tonumber('1234.5678')
+---
+- '1234.5678'
+...
+decimal.tonumber('1e6')
+---
+- '1000000'
+...
+decimal.tonumber('-6.234612e2')
+---
+- '-623.4612'
+...
+decimal.tonumber(tonumber64(2^63))
+---
+- '9223372036854775808.000000000000000'
+...
+decimal.tonumber(12345678ULL)
+---
+- '12345678'
+...
+decimal.tonumber(-12345678LL)
+---
+- '-12345678'
+...
+decimal.tonumber(1)
+---
+- '1.000000000000000'
+...
+decimal.tonumber(-1)
+---
+- '-1.000000000000000'
+...
+decimal.tonumber(2^64)
+---
+- '18446744073709551616.000000000000000'
+...
+decimal.tonumber(2^(-20))
+---
+- '0.000000953674316'
+...
+a = decimal.tonumber('100')
+---
+...
+a:log10()
+---
+- '2'
+...
+a:ln()
+---
+- '4.6051701859880913680359829093687284152'
+...
+-- overflow, e^100 > 10^38
+a:exp()
+---
+- error: Operation failed
+...
+-- e^87 < 10^38, no overflow, but rounds
+-- to 0 digits after the decimal point.
+decimal.tonumber(87):exp()
+---
+- '60760302250568721495223289381302760753'
+...
+a:sqrt()
+---
+- '10'
+...
+a
+---
+- '100'
+...
+a = decimal.tonumber('-123.456')
+---
+...
+a:round(2)
+---
+- '-123.46'
+...
+a:round(1)
+---
+- '-123.5'
+...
+a:round(0)
+---
+- '-123'
+...
+a:abs()
+---
+- '123.456'
+...
+a:tostring()
+---
+- '-123.456'
+...
+-a
+---
+- '123.456'
+...
+a / 10
+---
+- '-12.3456'
+...
+a * 5
+---
+- '-617.280000000000000000'
+...
+a + 17
+---
+- '-106.456000000000000'
+...
+a - 0.0001
+---
+- '-123.456100000000000'
+...
+a ^ 2
+---
+- '15241.383936'
+...
+a:abs() ^ 0.5
+---
+- '11.111075555498666484621494041182192341'
+...
+a:abs() ^ 0.5 == a:abs():sqrt()
+---
+- true
+...
+a - 2 < a - 1
+---
+- true
+...
+a + 1e-10 > a
+---
+- true
+...
+a <= a
+---
+- true
+...
+a >= a
+---
+- true
+...
+a == a
+---
+- true
+...
+decimal.tostring(a)
+---
+- '-123.456'
+...
+a:tostring()
+---
+- '-123.456'
+...
+decimal.abs(a)
+---
+- '123.456'
+...
+decimal.minus(a)
+---
+- '123.456'
+...
+decimal.round(a, 2)
+---
+- '-123.46'
+...
diff --git a/test/app/decimal.test.lua b/test/app/decimal.test.lua
new file mode 100644
index 000000000..4751c1500
--- /dev/null
+++ b/test/app/decimal.test.lua
@@ -0,0 +1,50 @@
+decimal = require('decimal')
+test_run = require('test_run').new()
+
+-- check various constructors
+decimal.tonumber('1234.5678')
+decimal.tonumber('1e6')
+decimal.tonumber('-6.234612e2')
+decimal.tonumber(tonumber64(2^63))
+decimal.tonumber(12345678ULL)
+decimal.tonumber(-12345678LL)
+decimal.tonumber(1)
+decimal.tonumber(-1)
+decimal.tonumber(2^64)
+decimal.tonumber(2^(-20))
+
+a = decimal.tonumber('100')
+a:log10()
+a:ln()
+-- overflow, e^100 > 10^38
+a:exp()
+-- e^87 < 10^38, no overflow, but rounds
+-- to 0 digits after the decimal point.
+decimal.tonumber(87):exp()
+a:sqrt()
+a
+a = decimal.tonumber('-123.456')
+a:round(2)
+a:round(1)
+a:round(0)
+a:abs()
+a:tostring()
+-a
+a / 10
+a * 5
+a + 17
+a - 0.0001
+a ^ 2
+a:abs() ^ 0.5
+a:abs() ^ 0.5 == a:abs():sqrt()
+a - 2 < a - 1
+a + 1e-10 > a
+a <= a
+a >= a
+a == a
+
+decimal.tostring(a)
+a:tostring()
+decimal.abs(a)
+decimal.minus(a)
+decimal.round(a, 2)
-- 
2.20.1 (Apple Git-117)

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/2] decimal: expose decimal type to lua.
  2019-06-19 15:58 ` [PATCH 2/2] decimal: expose decimal type to lua Serge Petrenko
@ 2019-06-20 11:02   ` Serge Petrenko
  2019-06-21 15:53   ` Vladimir Davydov
  1 sibling, 0 replies; 8+ messages in thread
From: Serge Petrenko @ 2019-06-20 11:02 UTC (permalink / raw)
  To: Vladimir Davydov; +Cc: tarantool-patches

A minor fix for the release build. Pushed on the branch

diff --git a/src/lua/decimal.c b/src/lua/decimal.c
index 72b458519..9e2fd41b5 100644
--- a/src/lua/decimal.c
+++ b/src/lua/decimal.c
@@ -314,6 +314,7 @@ tarantool_lua_decimal_init(struct lua_State *L)
                                       "uint16_t lsu[13];"
                              "} decimal_t;");
        assert(rc == 0);
+       (void)rc;
        luaL_register_module(L, "decimal", ldecimal_lib);
        lua_pop(L, 1);
        /*

--
Serge Petrenko
sergepetrenko@tarantool.org




> 19 июня 2019 г., в 18:58, Serge Petrenko <sergepetrenko@tarantool.org> написал(а):
> 
> 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 `tonumber` 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.tonumber(123e-7)
> b = decimal.tonumber('123.456')
> c = decimal.tonumber('123.456e2')
> d = decimal.tonumber(123ULL)
> e = decimal.tonumber(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`:
> ```
> tarantool> a + b
> ---
> - '123.456012300000000'
> ...
> 
> tarantool> decimal.add(a,b)
> ---
> - '123.456012300000000'
> ...
> 
> tarantool> c - d
> ---
> - '12222.6'
> ...
> 
> tarantool> decimal.sub(c,d)
> ---
> - '12222.6'
> ...
> 
> tarantool> c / b
> ---
> - '100'
> ...
> 
> tarantool> decimal.div(c, b)
> ---
> - '100'
> ...
> 
> tarantool> d * d
> ---
> - '15129'
> ...
> 
> tarantool> decimal.mul(d, d)
> ---
> - '15129'
> ...
> 
> tarantool> d ^ 2
> ---
> - '15129'
> ...
> 
> tarantool> 2 ^ d
> ---
> - '10633823966279326983230456482242756608'
> ...
> 
> tarantool> e ^ d
> ---
> - '10633823966279326983230456482242756608'
> ...
> ```
> 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.tonumber(100)
> tarantool> f:log10()
> ---
> - '2'
> ...
> 
> tarantool> decimal.log10(f)
> ---
> - '2'
> ...
> 
> tarantool> f:sqrt()
> ---
> - '10'
> ...
> 
> tarantool> decimal.sqrt(f)
> ---
> - '10'
> ...
> 
> tarantool> e2 = decimal.exp(2)
> ---
> ...
> 
> tarantool> e2:ln()
> ---
> - '2.0000000000000000000000000000000000000'
> ...
> 
> tarantool> decimal.ln(e2)
> ---
> - '2.0000000000000000000000000000000000000'
> ...
> 
> tarantool> e:exp() == e2
> ---
> - true
> ...
> 
> tarantool> e:exp():ln() == e
> ---
> - true
> ...
> 
> ```
> 
> There are also `abs`, `minus` and `tostring` methods, which are pretty
> self-explanatory.
> 
> ```
> tarantool> a = decimal.tonumber(-5)
> ---
> ...
> 
> tarantool> a
> ---
> - '-5.000000000000000'
> ...
> 
> tarantool> a:abs()
> ---
> - '5.000000000000000'
> ...
> 
> tarantool> decimal.abs(a)
> ---
> - '5.000000000000000'
> ...
> 
> tarantool> a:minus()
> ---
> - '5.000000000000000'
> ...
> 
> tarantool> -a
> ---
> - '5.000000000000000'
> ...
> 
> tarantool> decimal.minus(a)
> ---
> - '5.000000000000000'
> ...
> 
> tarantool> a:tostring()
> ---
> - '-5.000000000000000'
> ...
> 
> tarantool> decimal.tostring(a)
> ---
> - '-5.000000000000000'
> ...
> 
> ```
> 
> 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.
> ---
> src/CMakeLists.txt        |   1 +
> src/lua/decimal.c         | 327 ++++++++++++++++++++++++++++++++++++++
> src/lua/decimal.h         |  39 +++++
> src/lua/init.c            |   2 +
> test/app/decimal.result   | 172 ++++++++++++++++++++
> test/app/decimal.test.lua |  50 ++++++
> 6 files changed, 591 insertions(+)
> create mode 100644 src/lua/decimal.c
> create mode 100644 src/lua/decimal.h
> create mode 100644 test/app/decimal.result
> create mode 100644 test/app/decimal.test.lua
> 
> diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
> index 33b64f6a6..acd719e9b 100644
> --- a/src/CMakeLists.txt
> +++ b/src/CMakeLists.txt
> @@ -120,6 +120,7 @@ set (server_sources
>      lua/string.c
>      lua/buffer.c
>      lua/swim.c
> +     lua/decimal.c
>      ${lua_sources}
>      ${PROJECT_SOURCE_DIR}/third_party/lua-yaml/lyaml.cc
>      ${PROJECT_SOURCE_DIR}/third_party/lua-yaml/b64.c
> diff --git a/src/lua/decimal.c b/src/lua/decimal.c
> new file mode 100644
> index 000000000..72b458519
> --- /dev/null
> +++ b/src/lua/decimal.c
> @@ -0,0 +1,327 @@
> +/*
> + * Copyright 2010-2019, Tarantool AUTHORS, please see AUTHORS file.
> + *
> + * Redistribution and use in source and binary forms, with or
> + * without modification, are permitted provided that the following
> + * conditions are met:
> + *
> + * 1. Redistributions of source code must retain the above
> + *    copyright notice, this list of conditions and the
> + *    following disclaimer.
> + *
> + * 2. Redistributions in binary form must reproduce the above
> + *    copyright notice, this list of conditions and the following
> + *    disclaimer in the documentation and/or other materials
> + *    provided with the distribution.
> + *
> + * THIS SOFTWARE IS PROVIDED BY <COPYRIGHT HOLDER> ``AS IS'' AND
> + * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED
> + * TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
> + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL
> + * <COPYRIGHT HOLDER> OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT,
> + * INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
> + * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
> + * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR
> + * BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF
> + * LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF
> + * THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
> + * SUCH DAMAGE.
> + */
> +
> +#include "lua/decimal.h"
> +#include "lib/core/decimal.h"
> +#include "lua/utils.h"
> +
> +#include <lua.h>
> +#include <lauxlib.h>
> +#include <lualib.h>
> +
> +#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)");\
> +	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.");\
> +	}\
> +	return 1;\
> +}
> +#define LDECIMAL_UNOP(name, opname)\
> +static int \
> +ldecimal_##name(struct lua_State *L) {\
> +	if (lua_gettop(L) < 1)\
> +		return luaL_error(L, "Usage: decimal."#name"(decimal)");\
> +	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;
> +
> +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);
> +	/* 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, <=);
> +
> +static int
> +ldecimal_tonumber(struct lua_State *L)
> +{
> +	if (lua_gettop(L) < 1)
> +		luaL_error(L, "Usage: decimal.tonumber(value)");
> +	lua_todecimal(L, 1);
> +	decimal_t *lhs = lua_checkdecimal(L, 1);
> +	decimal_t *res = lua_pushdecimal(L);
> +	*res = *lhs;
> +	return 1;
> +}
> +
> +static int
> +ldecimal_round(struct lua_State *L)
> +{
> +	if (lua_gettop(L) < 2)
> +		return luaL_error(L, "Usage: decimal.round(decimal, scale)");
> +	decimal_t *lhs = lua_checkdecimal(L, 1);
> +	int n = lua_tointeger(L, 2);
> +	decimal_t *res = lua_pushdecimal(L);
> +	*res = *lhs;
> +	decimal_round(res, n);
> +	return 1;
> +}
> +
> +static int
> +ldecimal_tostring(struct lua_State *L)
> +{
> +	if (lua_gettop(L) < 1)
> +		return luaL_error(L, "Usage: decimal.tostring(decimal)");
> +	decimal_t *lhs = lua_checkdecimal(L, 1);
> +	lua_pushstring(L, decimal_to_string(lhs));
> +	return 1;
> +}
> +
> +static const luaL_Reg ldecimal_mt[] = {
> +	{"log10", ldecimal_log10},
> +	{"ln", ldecimal_ln},
> +	{"exp", ldecimal_exp},
> +	{"sqrt", ldecimal_sqrt},
> +	{"round", ldecimal_round},
> +	{"minus", ldecimal_minus},
> +	{"abs", ldecimal_abs},
> +	{"tostring", ldecimal_tostring},
> +	{"__unm", ldecimal_minus},
> +	{"__add", ldecimal_add},
> +	{"__sub", ldecimal_sub},
> +	{"__mul", ldecimal_mul},
> +	{"__div", ldecimal_div},
> +	{"__pow", ldecimal_pow},
> +	{"__eq", ldecimal_eq},
> +	{"__lt", ldecimal_lt},
> +	{"__le", ldecimal_le},
> +	{"__tostring", ldecimal_tostring},
> +	{NULL, NULL}
> +};
> +
> +static const luaL_Reg ldecimal_lib[] = {
> +	{"eq", ldecimal_eq},
> +	{"lt", ldecimal_lt},
> +	{"le", ldecimal_le},
> +	{"add", ldecimal_add},
> +	{"sub", ldecimal_sub},
> +	{"mul", ldecimal_mul},
> +	{"div", ldecimal_div},
> +	{"log10", ldecimal_log10},
> +	{"ln", ldecimal_ln},
> +	{"pow", ldecimal_pow},
> +	{"exp", ldecimal_exp},
> +	{"sqrt", ldecimal_sqrt},
> +	{"round", ldecimal_round},
> +	{"minus", ldecimal_minus},
> +	{"abs", ldecimal_abs},
> +	{"tostring", ldecimal_tostring},
> +	{"tonumber", ldecimal_tonumber},
> +	{NULL, NULL}
> +};
> +
> +void
> +tarantool_lua_decimal_init(struct lua_State *L)
> +{
> +	int rc = luaL_cdef(L, "typedef struct {"
> +				       "int32_t digits;"
> +				       "int32_t exponent;"
> +				       "uint8_t bits;"
> +				       "uint16_t lsu[13];"
> +			      "} decimal_t;");
> +	assert(rc == 0);
> +	luaL_register_module(L, "decimal", ldecimal_lib);
> +	lua_pop(L, 1);
> +	/*
> +	 * luaL_metatype is similar to luaL_ctypeid +
> +	 * luaL_register_type.
> +	 * The metatable is set automatically to every
> +	 * cdata of the new ctypeid ever created via ffi.
> +	 */
> +	CTID_DECIMAL = luaL_metatype(L, "decimal_t", ldecimal_mt);
> +	assert(CTID_DECIMAL != 0);
> +}
> diff --git a/src/lua/decimal.h b/src/lua/decimal.h
> new file mode 100644
> index 000000000..12b29d392
> --- /dev/null
> +++ b/src/lua/decimal.h
> @@ -0,0 +1,39 @@
> +/*
> + * Copyright 2010-2019, Tarantool AUTHORS, please see AUTHORS file.
> + *
> + * Redistribution and use in source and binary forms, with or
> + * without modification, are permitted provided that the following
> + * conditions are met:
> + *
> + * 1. Redistributions of source code must retain the above
> + *    copyright notice, this list of conditions and the
> + *    following disclaimer.
> + *
> + * 2. Redistributions in binary form must reproduce the above
> + *    copyright notice, this list of conditions and the following
> + *    disclaimer in the documentation and/or other materials
> + *    provided with the distribution.
> + *
> + * THIS SOFTWARE IS PROVIDED BY <COPYRIGHT HOLDER> ``AS IS'' AND
> + * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED
> + * TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
> + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL
> + * <COPYRIGHT HOLDER> OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT,
> + * INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
> + * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
> + * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR
> + * BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF
> + * LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF
> + * THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
> + * SUCH DAMAGE.
> + */
> +#ifndef TARANTOOL_LUA_DECIMAL_H_INCLUDED
> +#define TARANTOOL_LUA_DECIMAL_H_INCLUDED
> +
> +struct lua_State;
> +
> +void
> +tarantool_lua_decimal_init(struct lua_State *L);
> +
> +#endif /* TARANTOOL_LUA_DECIMAL_H_INCLUDED */
> diff --git a/src/lua/init.c b/src/lua/init.c
> index 9982828d9..fbaedd4cd 100644
> --- a/src/lua/init.c
> +++ b/src/lua/init.c
> @@ -59,6 +59,7 @@
> #include "lua/httpc.h"
> #include "lua/utf8.h"
> #include "lua/swim.h"
> +#include "lua/decimal.h"
> #include "digest.h"
> #include <small/ibuf.h>
> 
> @@ -454,6 +455,7 @@ tarantool_lua_init(const char *tarantool_bin, int argc, char **argv)
> 	tarantool_lua_pickle_init(L);
> 	tarantool_lua_digest_init(L);
> 	tarantool_lua_swim_init(L);
> +	tarantool_lua_decimal_init(L);
> 	luaopen_http_client_driver(L);
> 	lua_pop(L, 1);
> 	luaopen_msgpack(L);
> diff --git a/test/app/decimal.result b/test/app/decimal.result
> new file mode 100644
> index 000000000..93c1c7557
> --- /dev/null
> +++ b/test/app/decimal.result
> @@ -0,0 +1,172 @@
> +decimal = require('decimal')
> +---
> +...
> +test_run = require('test_run').new()
> +---
> +...
> +-- check various constructors
> +decimal.tonumber('1234.5678')
> +---
> +- '1234.5678'
> +...
> +decimal.tonumber('1e6')
> +---
> +- '1000000'
> +...
> +decimal.tonumber('-6.234612e2')
> +---
> +- '-623.4612'
> +...
> +decimal.tonumber(tonumber64(2^63))
> +---
> +- '9223372036854775808.000000000000000'
> +...
> +decimal.tonumber(12345678ULL)
> +---
> +- '12345678'
> +...
> +decimal.tonumber(-12345678LL)
> +---
> +- '-12345678'
> +...
> +decimal.tonumber(1)
> +---
> +- '1.000000000000000'
> +...
> +decimal.tonumber(-1)
> +---
> +- '-1.000000000000000'
> +...
> +decimal.tonumber(2^64)
> +---
> +- '18446744073709551616.000000000000000'
> +...
> +decimal.tonumber(2^(-20))
> +---
> +- '0.000000953674316'
> +...
> +a = decimal.tonumber('100')
> +---
> +...
> +a:log10()
> +---
> +- '2'
> +...
> +a:ln()
> +---
> +- '4.6051701859880913680359829093687284152'
> +...
> +-- overflow, e^100 > 10^38
> +a:exp()
> +---
> +- error: Operation failed
> +...
> +-- e^87 < 10^38, no overflow, but rounds
> +-- to 0 digits after the decimal point.
> +decimal.tonumber(87):exp()
> +---
> +- '60760302250568721495223289381302760753'
> +...
> +a:sqrt()
> +---
> +- '10'
> +...
> +a
> +---
> +- '100'
> +...
> +a = decimal.tonumber('-123.456')
> +---
> +...
> +a:round(2)
> +---
> +- '-123.46'
> +...
> +a:round(1)
> +---
> +- '-123.5'
> +...
> +a:round(0)
> +---
> +- '-123'
> +...
> +a:abs()
> +---
> +- '123.456'
> +...
> +a:tostring()
> +---
> +- '-123.456'
> +...
> +-a
> +---
> +- '123.456'
> +...
> +a / 10
> +---
> +- '-12.3456'
> +...
> +a * 5
> +---
> +- '-617.280000000000000000'
> +...
> +a + 17
> +---
> +- '-106.456000000000000'
> +...
> +a - 0.0001
> +---
> +- '-123.456100000000000'
> +...
> +a ^ 2
> +---
> +- '15241.383936'
> +...
> +a:abs() ^ 0.5
> +---
> +- '11.111075555498666484621494041182192341'
> +...
> +a:abs() ^ 0.5 == a:abs():sqrt()
> +---
> +- true
> +...
> +a - 2 < a - 1
> +---
> +- true
> +...
> +a + 1e-10 > a
> +---
> +- true
> +...
> +a <= a
> +---
> +- true
> +...
> +a >= a
> +---
> +- true
> +...
> +a == a
> +---
> +- true
> +...
> +decimal.tostring(a)
> +---
> +- '-123.456'
> +...
> +a:tostring()
> +---
> +- '-123.456'
> +...
> +decimal.abs(a)
> +---
> +- '123.456'
> +...
> +decimal.minus(a)
> +---
> +- '123.456'
> +...
> +decimal.round(a, 2)
> +---
> +- '-123.46'
> +...
> diff --git a/test/app/decimal.test.lua b/test/app/decimal.test.lua
> new file mode 100644
> index 000000000..4751c1500
> --- /dev/null
> +++ b/test/app/decimal.test.lua
> @@ -0,0 +1,50 @@
> +decimal = require('decimal')
> +test_run = require('test_run').new()
> +
> +-- check various constructors
> +decimal.tonumber('1234.5678')
> +decimal.tonumber('1e6')
> +decimal.tonumber('-6.234612e2')
> +decimal.tonumber(tonumber64(2^63))
> +decimal.tonumber(12345678ULL)
> +decimal.tonumber(-12345678LL)
> +decimal.tonumber(1)
> +decimal.tonumber(-1)
> +decimal.tonumber(2^64)
> +decimal.tonumber(2^(-20))
> +
> +a = decimal.tonumber('100')
> +a:log10()
> +a:ln()
> +-- overflow, e^100 > 10^38
> +a:exp()
> +-- e^87 < 10^38, no overflow, but rounds
> +-- to 0 digits after the decimal point.
> +decimal.tonumber(87):exp()
> +a:sqrt()
> +a
> +a = decimal.tonumber('-123.456')
> +a:round(2)
> +a:round(1)
> +a:round(0)
> +a:abs()
> +a:tostring()
> +-a
> +a / 10
> +a * 5
> +a + 17
> +a - 0.0001
> +a ^ 2
> +a:abs() ^ 0.5
> +a:abs() ^ 0.5 == a:abs():sqrt()
> +a - 2 < a - 1
> +a + 1e-10 > a
> +a <= a
> +a >= a
> +a == a
> +
> +decimal.tostring(a)
> +a:tostring()
> +decimal.abs(a)
> +decimal.minus(a)
> +decimal.round(a, 2)
> -- 
> 2.20.1 (Apple Git-117)
> 

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/2] decimal: expose decimal type to lua.
  2019-06-19 15:58 ` [PATCH 2/2] decimal: expose decimal type to lua Serge Petrenko
  2019-06-20 11:02   ` Serge Petrenko
@ 2019-06-21 15:53   ` Vladimir Davydov
  2019-06-24 14:53     ` [tarantool-patches] " Serge Petrenko
  1 sibling, 1 reply; 8+ messages in thread
From: Vladimir Davydov @ 2019-06-21 15:53 UTC (permalink / raw)
  To: Serge Petrenko; +Cc: tarantool-patches

On Wed, Jun 19, 2019 at 06:58:05PM +0300, Serge Petrenko wrote:
> 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 `tonumber` 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.tonumber(123e-7)
> b = decimal.tonumber('123.456')
> c = decimal.tonumber('123.456e2')
> d = decimal.tonumber(123ULL)
> e = decimal.tonumber(2)
> ```

tonumber is a confusing name IMO. Let's rename it to decimal.new()

> 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`:
> ```

Can these functions fail (overflow, underflow)? What happens on error?
Please mention in this document.

> tarantool> a + b
> ---
> - '123.456012300000000'
> ...
> 
> tarantool> decimal.add(a,b)
> ---
> - '123.456012300000000'
> ...
> 
> tarantool> c - d
> ---
> - '12222.6'
> ...
> 
> tarantool> decimal.sub(c,d)

I don't think we need decimal.add/sub/div/mul methods as long as we have
corresponding operators.

> 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.tonumber(100)
> tarantool> f:log10()

This looks weird. I think we should only allow decimal.log10(x),
not x:log10().

> ---
>  src/CMakeLists.txt        |   1 +
>  src/lua/decimal.c         | 327 ++++++++++++++++++++++++++++++++++++++
>  src/lua/decimal.h         |  39 +++++
>  src/lua/init.c            |   2 +
>  test/app/decimal.result   | 172 ++++++++++++++++++++
>  test/app/decimal.test.lua |  50 ++++++
>  6 files changed, 591 insertions(+)
>  create mode 100644 src/lua/decimal.c
>  create mode 100644 src/lua/decimal.h
>  create mode 100644 test/app/decimal.result
>  create mode 100644 test/app/decimal.test.lua

Please consider implementing this using Lua ffi. Take a look at
src/lua/uuid.lua for example. They say that ffi implementation is
generally faster than Lua C, because it doesn't break JIT traces.
It might be worth benchmarking the two approaches.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [tarantool-patches] [PATCH 2/2] decimal: expose decimal type to lua.
  2019-06-21 15:53   ` Vladimir Davydov
@ 2019-06-24 14:53     ` Serge Petrenko
  2019-06-26 14:02       ` Vladimir Davydov
  0 siblings, 1 reply; 8+ messages in thread
From: Serge Petrenko @ 2019-06-24 14:53 UTC (permalink / raw)
  To: Vladimir Davydov; +Cc: tarantool-patches

[-- Attachment #1: Type: text/plain, Size: 10636 bytes --]

Hi! Thank you for review!
I addressed your comments, and pushed the new version on the branch.
The diff is below. It is rather big, but I feel like it belongs here, rather than in
a `v2` letter, since the changes are very minor.
--
Serge Petrenko
sergepetrenko@tarantool.org




> 21 июня 2019 г., в 18:53, Vladimir Davydov <vdavydov.dev@gmail.com> написал(а):
> 
> On Wed, Jun 19, 2019 at 06:58:05PM +0300, Serge Petrenko wrote:
>> 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 `tonumber` 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.tonumber(123e-7)
>> b = decimal.tonumber('123.456')
>> c = decimal.tonumber('123.456e2')
>> d = decimal.tonumber(123ULL)
>> e = decimal.tonumber(2)
>> ```
> 
> tonumber is a confusing name IMO. Let's rename it to decimal.new()

Ok

> 
>> 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`:
>> ```
> 
> Can these functions fail (overflow, underflow)? What happens on error?
> Please mention in this document.

I made the message more informative.

> 
>> tarantool> a + b
>> ---
>> - '123.456012300000000'
>> ...
>> 
>> tarantool> decimal.add(a,b)
>> ---
>> - '123.456012300000000'
>> ...
>> 
>> tarantool> c - d
>> ---
>> - '12222.6'
>> ...
>> 
>> tarantool> decimal.sub(c,d)
> 
> I don't think we need decimal.add/sub/div/mul methods as long as we have
> corresponding operators.

No problem.

> 
>> 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.tonumber(100)
>> tarantool> f:log10()
> 
> This looks weird. I think we should only allow decimal.log10(x),
> not x:log10().

All the math functions are now accessed as decimal.func(), decimal.scale() and
decimal.precision() are added.

> 
>> ---
>> src/CMakeLists.txt        |   1 +
>> src/lua/decimal.c         | 327 ++++++++++++++++++++++++++++++++++++++
>> src/lua/decimal.h         |  39 +++++
>> src/lua/init.c            |   2 +
>> test/app/decimal.result   | 172 ++++++++++++++++++++
>> test/app/decimal.test.lua |  50 ++++++
>> 6 files changed, 591 insertions(+)
>> create mode 100644 src/lua/decimal.c
>> create mode 100644 src/lua/decimal.h
>> create mode 100644 test/app/decimal.result
>> create mode 100644 test/app/decimal.test.lua
> 
> Please consider implementing this using Lua ffi. Take a look at
> src/lua/uuid.lua for example. They say that ffi implementation is
> generally faster than Lua C, because it doesn't break JIT traces.
> It might be worth benchmarking the two approaches.

I have written a proof-of-concept ffi implementation and tested its performance.
It has shown a ~25% performance decrease over the current representation.
The details are in the issue comments (https://github.com/tarantool/tarantool/issues/692#issuecomment-505043929)
I propose to leave existing implementation as is.



diff --git a/src/lua/decimal.c b/src/lua/decimal.c
index 9e2fd41b5..4dc0dff7d 100644
--- a/src/lua/decimal.c
+++ b/src/lua/decimal.c
@@ -227,10 +227,10 @@ LDECIMAL_CMPOP(lt, <);
 LDECIMAL_CMPOP(le, <=);
 
 static int
-ldecimal_tonumber(struct lua_State *L)
+ldecimal_new(struct lua_State *L)
 {
 	if (lua_gettop(L) < 1)
-		luaL_error(L, "Usage: decimal.tonumber(value)");
+		luaL_error(L, "Usage: decimal.new(value)");
 	lua_todecimal(L, 1);
 	decimal_t *lhs = lua_checkdecimal(L, 1);
 	decimal_t *res = lua_pushdecimal(L);
@@ -251,6 +251,28 @@ ldecimal_round(struct lua_State *L)
 	return 1;
 }
 
+static int
+ldecimal_scale(struct lua_State *L)
+{
+	if (lua_gettop(L) < 1)
+		return luaL_error(L, "Usage: decimal.scale(decimal)");
+	decimal_t *lhs = lua_checkdecimal(L, 1);
+	int scale = decimal_scale(lhs);
+	lua_pushnumber(L, scale);
+	return 1;
+}
+
+static int
+ldecimal_precision(struct lua_State *L)
+{
+	if (lua_gettop(L) < 1)
+		return luaL_error(L, "Usage: decimal.precisiion(decimal)");
+	decimal_t *lhs = lua_checkdecimal(L, 1);
+	int precision = decimal_precision(lhs);
+	lua_pushnumber(L, precision);
+	return 1;
+}
+
 static int
 ldecimal_tostring(struct lua_State *L)
 {
@@ -262,14 +284,6 @@ ldecimal_tostring(struct lua_State *L)
 }
 
 static const luaL_Reg ldecimal_mt[] = {
-	{"log10", ldecimal_log10},
-	{"ln", ldecimal_ln},
-	{"exp", ldecimal_exp},
-	{"sqrt", ldecimal_sqrt},
-	{"round", ldecimal_round},
-	{"minus", ldecimal_minus},
-	{"abs", ldecimal_abs},
-	{"tostring", ldecimal_tostring},
 	{"__unm", ldecimal_minus},
 	{"__add", ldecimal_add},
 	{"__sub", ldecimal_sub},
@@ -284,23 +298,16 @@ static const luaL_Reg ldecimal_mt[] = {
 };
 
 static const luaL_Reg ldecimal_lib[] = {
-	{"eq", ldecimal_eq},
-	{"lt", ldecimal_lt},
-	{"le", ldecimal_le},
-	{"add", ldecimal_add},
-	{"sub", ldecimal_sub},
-	{"mul", ldecimal_mul},
-	{"div", ldecimal_div},
 	{"log10", ldecimal_log10},
 	{"ln", ldecimal_ln},
 	{"pow", ldecimal_pow},
 	{"exp", ldecimal_exp},
 	{"sqrt", ldecimal_sqrt},
 	{"round", ldecimal_round},
-	{"minus", ldecimal_minus},
+	{"scale", ldecimal_scale},
+	{"precision", ldecimal_precision},
 	{"abs", ldecimal_abs},
-	{"tostring", ldecimal_tostring},
-	{"tonumber", ldecimal_tonumber},
+	{"new", ldecimal_new},
 	{NULL, NULL}
 };
 
diff --git a/test/app/decimal.result b/test/app/decimal.result
index 93c1c7557..22590a082 100644
--- a/test/app/decimal.result
+++ b/test/app/decimal.result
@@ -5,69 +5,69 @@ test_run = require('test_run').new()
 ---
 ...
 -- check various constructors
-decimal.tonumber('1234.5678')
+decimal.new('1234.5678')
 ---
 - '1234.5678'
 ...
-decimal.tonumber('1e6')
+decimal.new('1e6')
 ---
 - '1000000'
 ...
-decimal.tonumber('-6.234612e2')
+decimal.new('-6.234612e2')
 ---
 - '-623.4612'
 ...
-decimal.tonumber(tonumber64(2^63))
+decimal.new(tonumber64(2^63))
 ---
 - '9223372036854775808.000000000000000'
 ...
-decimal.tonumber(12345678ULL)
+decimal.new(12345678ULL)
 ---
 - '12345678'
 ...
-decimal.tonumber(-12345678LL)
+decimal.new(-12345678LL)
 ---
 - '-12345678'
 ...
-decimal.tonumber(1)
+decimal.new(1)
 ---
 - '1.000000000000000'
 ...
-decimal.tonumber(-1)
+decimal.new(-1)
 ---
 - '-1.000000000000000'
 ...
-decimal.tonumber(2^64)
+decimal.new(2^64)
 ---
 - '18446744073709551616.000000000000000'
 ...
-decimal.tonumber(2^(-20))
+decimal.new(2^(-20))
 ---
 - '0.000000953674316'
 ...
-a = decimal.tonumber('100')
+a = decimal.new('100')
 ---
 ...
-a:log10()
+decimal.log10(a)
 ---
 - '2'
 ...
-a:ln()
+decimal.ln(a)
 ---
 - '4.6051701859880913680359829093687284152'
 ...
 -- overflow, e^100 > 10^38
-a:exp()
+decimal.exp(a)
 ---
 - error: Operation failed
 ...
 -- e^87 < 10^38, no overflow, but rounds
 -- to 0 digits after the decimal point.
-decimal.tonumber(87):exp()
+decimal.exp(decimal.new(87))
 ---
 - '60760302250568721495223289381302760753'
 ...
-a:sqrt()
+decimal.sqrt(a)
 ---
 - '10'
 ...
@@ -75,26 +75,18 @@ a
 ---
 - '100'
 ...
-a = decimal.tonumber('-123.456')
+a = decimal.new('-123.456')
 ---
 ...
-a:round(2)
+decimal.round(a, 2)
 ---
 - '-123.46'
 ...
-a:round(1)
----
-- '-123.5'
-...
-a:round(0)
----
-- '-123'
-...
-a:abs()
+decimal.abs(a)
 ---
 - '123.456'
 ...
-a:tostring()
+a
 ---
 - '-123.456'
 ...
@@ -122,11 +114,11 @@ a ^ 2
 ---
 - '15241.383936'
 ...
-a:abs() ^ 0.5
+decimal.abs(a) ^ 0.5
 ---
 - '11.111075555498666484621494041182192341'
 ...
-a:abs() ^ 0.5 == a:abs():sqrt()
+decimal.abs(a) ^ 0.5 == decimal.sqrt(decimal.abs(a))
 ---
 - true
 ...
@@ -150,23 +142,3 @@ a == a
 ---
 - true
 ...
-decimal.tostring(a)
----
-- '-123.456'
-...
-a:tostring()
----
-- '-123.456'
-...
-decimal.abs(a)
----
-- '123.456'
-...
-decimal.minus(a)
----
-- '123.456'
-...
-decimal.round(a, 2)
----
-- '-123.46'
-...
diff --git a/test/app/decimal.test.lua b/test/app/decimal.test.lua
index 4751c1500..b101f07b6 100644
--- a/test/app/decimal.test.lua
+++ b/test/app/decimal.test.lua
@@ -2,49 +2,41 @@ decimal = require('decimal')
 test_run = require('test_run').new()
 
 -- check various constructors
-decimal.tonumber('1234.5678')
-decimal.tonumber('1e6')
-decimal.tonumber('-6.234612e2')
-decimal.tonumber(tonumber64(2^63))
-decimal.tonumber(12345678ULL)
-decimal.tonumber(-12345678LL)
-decimal.tonumber(1)
-decimal.tonumber(-1)
-decimal.tonumber(2^64)
-decimal.tonumber(2^(-20))
+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.tonumber('100')
-a:log10()
-a:ln()
+a = decimal.new('100')
+decimal.log10(a)
+decimal.ln(a)
 -- overflow, e^100 > 10^38
-a:exp()
+decimal.exp(a)
 -- e^87 < 10^38, no overflow, but rounds
 -- to 0 digits after the decimal point.
-decimal.tonumber(87):exp()
-a:sqrt()
+decimal.exp(decimal.new(87))
+decimal.sqrt(a)
+a
+a = decimal.new('-123.456')
+decimal.round(a, 2)
+decimal.abs(a)
 a
-a = decimal.tonumber('-123.456')
-a:round(2)
-a:round(1)
-a:round(0)
-a:abs()
-a:tostring()
 -a
 a / 10
 a * 5
 a + 17
 a - 0.0001
 a ^ 2
-a:abs() ^ 0.5
-a:abs() ^ 0.5 == a:abs():sqrt()
+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
-
-decimal.tostring(a)
-a:tostring()
-decimal.abs(a)
-decimal.minus(a)
-decimal.round(a, 2)


[-- Attachment #2: Type: text/html, Size: 22451 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [tarantool-patches] [PATCH 2/2] decimal: expose decimal type to lua.
  2019-06-24 14:53     ` [tarantool-patches] " Serge Petrenko
@ 2019-06-26 14:02       ` Vladimir Davydov
  2019-06-28 14:39         ` Serge Petrenko
  0 siblings, 1 reply; 8+ messages in thread
From: Vladimir Davydov @ 2019-06-26 14:02 UTC (permalink / raw)
  To: Serge Petrenko, g; +Cc: tarantool-patches

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.c

Also, 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).

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [tarantool-patches] [PATCH 2/2] decimal: expose decimal type to lua.
  2019-06-26 14:02       ` Vladimir Davydov
@ 2019-06-28 14:39         ` Serge Petrenko
  0 siblings, 0 replies; 8+ messages in thread
From: Serge Petrenko @ 2019-06-28 14:39 UTC (permalink / raw)
  To: Vladimir Davydov; +Cc: tarantool-patches

[-- Attachment #1: Type: text/plain, Size: 13911 bytes --]

Hi! Thank you for review! I addressed your comments and sent v2.
The only inconsistency is with macro names: I feel like LDECIMAL_BINOP and LDECIMAL_FUNC
are better, since the latter one is only used for functions now (ln, log10, sqrt, abs).

--
Serge Petrenko
sergepetrenko@tarantool.org




> 26 июня 2019 г., в 17:02, Vladimir Davydov <vdavydov.dev@gmail.com> написал(а):
> 
> 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.c
> 
> Also, 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).
> 


[-- Attachment #2: Type: text/html, Size: 39241 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2019-06-28 14:39 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-06-19 15:58 [PATCH 0/2] decimal: expose decimal module to lua Serge Petrenko
2019-06-19 15:58 ` [PATCH 1/2] lua/utils: add a function to register FFI metatypes Serge Petrenko
2019-06-19 15:58 ` [PATCH 2/2] decimal: expose decimal type to lua Serge Petrenko
2019-06-20 11:02   ` Serge Petrenko
2019-06-21 15:53   ` Vladimir Davydov
2019-06-24 14:53     ` [tarantool-patches] " Serge Petrenko
2019-06-26 14:02       ` Vladimir Davydov
2019-06-28 14:39         ` Serge Petrenko

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox