decimal: add the library into the module API

[Totktonada]

The existing API was designed to allow a user to allocate decimal values
in his/her own memory (say, on the stack). This commit mostly just
exports the functions to the module API, so it allows a user of the
module API to do the same. As result the decimal number structure is
exposed too (it is NOT opacue). A module API user should know a size and
an alignment of the structure.

The downside is that we can't freely modify the data layout of decimals
in tarantool anymore (without ABI breakage). It is deliberate decision:
I doubt that we'll need to change decimals representation in a future.

Added STRUCT_DECIMAL macro to allow a module API user to, say, include
decNumber library and alias tarantool's decimal type to decNumber's
structure.

Kept strtodec() private: it does not follow the common decimal_*()
naming pattern. We can add it into the module API later with another
name.

Kept decimal_str() private: it looks dangerous without at least good
explanation when data in the static buffer are invalidated. There is
decimal_to_string() that writes to an explicitly provided buffer.

Replaced enum with #define for better portability: enum has
implementation defined size and we shoudn't use it, when ABI
compatibility matters.

Changed headers order in test files to alias decimal_t to decNumber and
don't step into the 'conflicting types' error.

Part of #7228

[Totktonada]

(My bad, I didn't run checkpatch before sending the patchset. Will fix today.)

[Totktonada]

Fixed checkpatch suggestions except the following one:

ERROR: do not add new typedefs
#179: FILE: src/lib/core/decimal.h:97:
+typedef STRUCT_DECIMAL decimal_t;

Filed tarantool/checkpatch#27.

[sergepetrenko]

Hi! Thanks for the patch!

2 minor comments below.

Also I'm a little bit worried by this:

The downside is that we can't freely modify the data layout of decimals
in tarantool anymore (without ABI breakage). It is deliberate decision:
I doubt that we'll need to change decimals representation in a future.

There was a theoretical possibility we would increase DECIMAL_DIGITS if someone finds that 38 digits are not enough. This isn't possible anymore as I understand?

[Totktonada]

There was a theoretical possibility we would increase DECIMAL_DIGITS if someone finds that 38 digits are not enough. This isn't possible anymore as I understand?

Strictly speaking, it'll break ABI for a module, which leans on particular decimal size, alignment or data layout. Say, if it allocates decimals on a stack. I guess that modules that just marshall values from/to Lua and converts them to strings (in large buffers) will be okay.

TBH, I still doubt on this point, but I can't propose anything better. Hide the structure and let a user to call malloc-backed decimal_new()? Doesn't look nice for arithmetic operations. Expose decimal_size() and let user to use non-standard alloca() (and take care to aligment pitfalls around arrays)? Introduce an allocator argument for decimal_new()? Don't know. It would make the user side code more complex, but I'm not sure that the theoretical future change worth it.

My personal opinion is that it worth to go ahead rather than spin for a long time and don't offer any API.

I'm open for a discussion if there will be a proposal what exactly to do.

[sergepetrenko]

My personal opinion is that it worth to go ahead rather than spin for a long time and don't offer any API.
I'm open for a discussion if there will be a proposal what exactly to do.

I'm fine with the current approach

[locker]

I don't like exposing the decimal number layout to the user. If we switch to a different decimal library or even just reconfigure libdecnumber, as @sergepetrenko pointed out, we'll break the ABI. We can avoid potential ABI breakages by reserving a substantial amount of bytes for the decimal object:

struct decimal {
    char data[64];
};

[Totktonada]

This way the following code (which uses an API from PR #7429) will write behind the buffer:

int
luaT_push_twice(struct lua_State *L, const decimal_t *dec)
{
        decimal_t *res_1 = luaT_newdecimal(L);
        decimal_t *res_2 = luaT_newdecimal(L);
        *res_1 = *dec;
        *res_2 = *dec;
        return 2;
}

(I would discuss both C and Lua/C API verbally, that's tricky.)

[locker]

I guess we could add decimal_copy to avoid that.

[locker]

I don't think it's a good idea to expose our internal functions as the module API, because it limits our refactoring capabilities. For example, I don't like decimal_t, because we can't forward-declare it and have to include the huge libdecnumber header to declare functions that use it. I'd like to rework it as follows:

struct decimal {
    decNumber impl;
};

But with this becoming a part of the public API I wouldn't be able to do that.

Also, shouldn't all module API functions have a box_ or tt_ prefix to avoid possible name collisions?

[Totktonada]

After a voice discussion we agreed that we should provide appropriate padding for the public decimal structure to allow future extending. And we shouldn't expose anything about internal data layout. I implemented the new API variants in PR #7497 and PR #7498 (those are two alternatives, we need to choose one of them).