koka-lang / libmprompt Goto Github PK

First of all, thank you for all your work on algebraic effects, especially libhandler and Koka -- for me, they have always been a strong source of inspiration and examples to follow.

In this issue, I propose using Metalang99 to clean up the API of libmpeff, as well as to improve its implementation.

Problem

Currently, the header file mpeff.h exposes the set of macros MPE_DECLARE_EFFECT(0-2) and MPE_DEFINE_EFFECT(0-7). Owing to the lack of iteration facilities of the C/C++ preprocessor, these are implemented by copy-pasting the same pattern with respect to a number of macro parameters. Needless to say that this approach leads to error-prone and hard-to-maintain code, and moreover, complicates the API: a user needs to specify the number of parameters like this: MPE_DEFINE_EFFECT5(...) and change the name as the number of parameters changes.

A similar problem is manual macro overloading on a number of parameters:

#define MPE_DECLARE_OP(effect, op) extern const struct mpe_optag_s MPE_OPTAG_DEF(effect, op);

#define MPE_DECLARE_OP0(effect, op, restype)                                                       \
    MPE_DECLARE_OP(effect, op)                                                                     \
    restype effect##_##op();

#define MPE_DECLARE_OP1(effect, op, restype, argtype)                                              \
    MPE_DECLARE_OP(effect, op)                                                                     \
    restype effect##_##op(argtype arg);

Besides MPE_DECLARE_OP, there are MPE_DECLARE_VOIDOP, MPE_DEFINE_OP, MPE_DEFINE_VOIDOP, MPE_WRAP_FUN, and MPE_WRAP_VOIDFUN.

Solution

We could rewrite these macros using Metalang99, a standard-conforming header-only library that augments the C99/C++11 preprocessor with extra metaprogramming facilities, including handy iteration patterns.

MPE_DECLARE_EFFECT

With the aid of Metalang99, MPE_DECLARE_EFFECT takes the following form:

#define MPE_DECLARE_EFFECT(...)                                                                    \
    extern const char *MPE_EFFECT(                                                                 \
        ML99_VARIADICS_GET(0)(__VA_ARGS__))[ML99_VARIADICS_COUNT(__VA_ARGS__) + 1];

That is it -- now you can specify an arbitrary amount of parameters (to be precise, up to 63) and it will work fine.

MPE_DEFINE_EFFECT

MPE_DEFINE_EFFECT would be slightly more complex:

#define MPE_DEFINE_EFFECT(...)                                                                     \
    ML99_IF(                                                                                       \
        ML99_VARIADICS_IS_SINGLE(__VA_ARGS__),                                                     \
        MPE_PRIV_DEFINE_EFFECT_0,                                                                  \
        MPE_PRIV_DEFINE_EFFECT_N)                                                                  \
    (__VA_ARGS__)

#define MPE_PRIV_DEFINE_EFFECT_0(effect) const char *MPE_EFFECT(effect)[2] = {#effect, NULL};

#define MPE_PRIV_DEFINE_EFFECT_N(effect, ...)                                                      \
    const char *MPE_EFFECT(effect)[ML99_VARIADICS_COUNT(__VA_ARGS__) + 2] = {                      \
        #effect,                                                                                   \
        ML99_EVAL(MPE_PRIV_opNameForEach(effect, __VA_ARGS__)) NULL};                              \
    ML99_EVAL(MPE_PRIV_defineEffectForEach(effect, __VA_ARGS__))

/*
 * #effect "/" #op1, ..., #effect "/" #opN,
 */
#define MPE_PRIV_opNameForEach(effect, ...)                                                        \
    ML99_variadicsForEach(ML99_appl(v(MPE_PRIV_opName), v(effect)), v(__VA_ARGS__))

#define MPE_PRIV_opName_IMPL(effect, op) v(#effect "/" #op, )

/*
 * const struct mpe_optag_s MPE_OPTAG_DEF(effect, op1) = { MPE_EFFECT(effect), 1 };
 * ...
 * const struct mpe_optag_s MPE_OPTAG_DEF(effect, opN) = { MPE_EFFECT(effect), N };
 */
#define MPE_PRIV_defineEffectForEach(effect, ...)                                                  \
    ML99_variadicsForEachI(ML99_appl(v(MPE_PRIV_defineEffect), v(effect)), v(__VA_ARGS__))

#define MPE_PRIV_defineEffect_IMPL(effect, op, i)                                                  \
    v(const struct mpe_optag_s MPE_OPTAG_DEF(effect, op) = {MPE_EFFECT(effect), i};)

#define MPE_PRIV_opName_ARITY       2
#define MPE_PRIV_defineEffect_ARITY 3

Macro overloading

MPE_DECLARE_OP and its friends could be defined as follows:

#define MPE_DECLARE_OP(effect, ...) ML99_OVERLOAD(MPE_PRIV_DECLARE_OP_, effect, __VA_ARGS__)

#define MPE_PRIV_DECLARE_OP_2(effect, op) extern const struct mpe_optag_s MPE_OPTAG_DEF(effect, op);

#define MPE_PRIV_DECLARE_OP_3(effect, op, restype)                                                 \
    MPE_PRIV_DECLARE_OP_2(effect, op)                                                              \
    restype effect##_##op();

#define MPE_PRIV_DECLARE_OP_4(effect, op, restype, argtype)                                        \
    MPE_PRIV_DECLARE_OP_2(effect, op)                                                              \
    restype effect##_##op(argtype arg);

In user code, just type MPE_DECLARE_OP(reader, ask, long) and it will overload automatically.

To see how it performs in practice, take a look at my branch. Just git clone https://github.com/Hirrolot/metalang99 to the root project directory and execute the usual build procedure.

Advantages

• More convenient API.
• Less error-prone, more maintainable implementation.

Disadvantages

• A third-party dependency (though it is header-only).
• -ftrack-macro-expansion=0 (GCC) or -fmacro-backtrace-limit=1 (Clang) should be specified because otherwise, compilation errors will go insane.

Final words

The aforementioned syntax is not a final result but just an example of how can we refine the API. In theory, the syntax could be even more concise. I will be willing to refactor the source code more if needed and can help with anything. Hopefully, this should not be tough, and the semantics should remain completely the same.

Metalang99 is already stable v1.x.y and it is very unlikely that it will significantly change in near future. It has been tested on Datatype99 and Interface99 with successful results.

I believe libmprompt has a bright future, please do not abandon it.

I have been working on schrodingerzhu/libmprompt-sys. The async_workers example is already working there.

Any idea on higher-level wrappers?

It seems that the prompt will violate same-thread-lock, same-thread-release rule in some case. (Consider a thread pool to execute multiple prompts). I dont know whether it is my fault so I open this to ask whether the library support that usage currently.

When buiding a debug release on FreeBSD, the backtrace call needs to add -lexecinfo as per backtrace(3), but the current cmake configuration does not seem to include it.

$ cmake -DCMAKE_BUILD_TYPE=Debug ../../
-- The C compiler identification is Clang 11.0.1
-- The CXX compiler identification is Clang 11.0.1
-- The ASM compiler identification is Clang
-- Found assembler: /usr/bin/cc
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Check for working C compiler: /usr/bin/cc - skipped
-- Detecting C compile features
-- Detecting C compile features - done
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Check for working CXX compiler: /usr/bin/c++ - skipped
-- Detecting CXX compile features
-- Detecting CXX compile features - done
-- Use the C++ compiler to compile (Clang) (MP_USE_C=OFF)
-- 
-- Libraries : libmpromptx, libmpeffx
-- Build type: Debug
-- Compiler  : /usr/bin/c++
--             -Wno-deprecated
-- 
-- Configuring done
-- Generating done
-- Build files have been written to: /usr/home/gdiazlo/src/libmprompt/out/debug
$ make
Scanning dependencies of target mpeff
[  3%] Building CXX object CMakeFiles/mpeff.dir/src/mpeff/main.c.o
[  7%] Building ASM object CMakeFiles/mpeff.dir/src/mprompt/asm/longjmp_amd64.S.o
[ 10%] Linking CXX static library libmpeffx.a
[ 10%] Built target mpeff
Scanning dependencies of target test_mp_example_async
[ 14%] Building CXX object CMakeFiles/test_mp_example_async.dir/test/test_mp_example_async.c.o
[ 17%] Linking CXX executable test_mp_example_async
ld: error: undefined symbol: backtrace
>>> referenced by mprompt.c:756 (/usr/home/gdiazlo/src/libmprompt/src/mpeff/../mprompt/mprompt.c:756)
>>>               main.c.o:(mp_backtrace(void**, int)) in archive libmpeffx.a
c++: error: linker command failed with exit code 1 (use -v to see invocation)
*** Error code 1

Stop.
make[2]: stopped in /usr/home/gdiazlo/src/libmprompt/out/debug
*** Error code 1

Stop.
make[1]: stopped in /usr/home/gdiazlo/src/libmprompt/out/debug
*** Error code 1

Stop.
make: stopped in /usr/home/gdiazlo/src/libmprompt/out/debug
$ 

Hello

The test are failing on FreeBSD13 (seems they introduced PROT_MAX on this version), with a EINVAL error in mmap call when compiling a release version.

% ./test_mp_async 
run requests...
libmprompt: error: failed to allocate mmap memory of size 274877906944
            code : 22: Invalid argument
libmprompt: error: unable to allocate a stack
4000: signal: sys: abort (core dumped)
% 

Removing PROT_MAX but leaving prot |= PROT_READ | PROT_WRITE the test does not crash, but never ends, eating a single core for more than an hour, with no increase of memory usage. The other tests behave simmilarly.

The semantics of MPE_OP_TAIL_NOOP and MPE_OP_TAIL_NOOP don't follow their description,
nor the one of the similar tags on libhandler.

MPE_OP_TAIL_NOOP,   ///< resume at most once without performing operations; and if resumed, it is the last action performed by the operation function.
MPE_OP_TAIL,        ///< resume at most once; and if resumed, it is the last action performed by the operation function.

The problem is that the implementation seems to assume that the resumption is always resumed,
even when the handler discards it and just returns. libhandler has a marker to indicate whether the
resumption was resumed or not. A similar approach could be taken.

I may be missing a correct way to discard a resumption instead of just discarding it.

A small test:

MPE_DEFINE_EFFECT1(sphinx, answer)
MPE_DEFINE_VOIDOP1(sphinx, answer, mpe_string_t)

void* brian(void* arg){
    UNUSED(arg);
    // Arrive at Thebes
    sphinx_answer("Scooters");
    // Die
    mpt_assert(false, "Brian should have been eaten by the sphinx");
    return mpe_voidp_int(1);
}

/*-----------------------------------------------------------------
  Sphinx handler
-----------------------------------------------------------------*/

static void* _sphinx_answer(mpe_resume_t* r, void* local, void* arg){
    if (strcmp(mpe_mpe_string_t_voidp(arg), "Person") == 0) {
        return mpe_resume_tail(r, local, NULL);
    } else {
        // I couldn't find a way to release it, this one is not intended for MPE_RESUMPTION_INPLACE
        // mpe_resume_release(r);
        return mpe_voidp_int(0);
    }
}

static const mpe_handlerdef_t sphinx_hdef = { MPE_EFFECT(sphinx), NULL, {
    { MPE_OP_TAIL, MPE_OPTAG(sphinx, answer), &_sphinx_answer },
    { MPE_OP_NULL, mpe_op_null, NULL}
}};

static void* sphinx_handle(mpe_actionfun_t action) {
    return mpe_handle(&sphinx_hdef, NULL, action, NULL);
}