4
\$\begingroup\$

I've implemented a small single-header library over the C++20 coroutines. This library only works on a single thread and it is limited to Linux because it is also based on epoll.

The library does not implement C++ exceptions in any way.

A feature of the library is that you can add "modifiers" to coroutines. If present, those modifiers are called on different entry points inside the coroutine(on entry, on leave, on reentry, on exit, etc.). Those modifiers are inherited by other coroutines that are called, but not those that are scheduled from the current one.

The library can also be viewed here: https://github.com/Pangi790927/utils/blob/main/co_utils.h

The header in question, co_utils.h:

#ifndef CO_UTILS_H
#define CO_UTILS_H

#include <iostream>
#include <unordered_map>
#include <sys/socket.h>
#include <arpa/inet.h>
#include <set>
#include <stack>
#include <map>
#include <queue>
#include <list>
#include <coroutine>
#include <sys/epoll.h>
#include <sys/timerfd.h>

#include "utils.h"
// #include "debug.h"
// #include "misc_utils.h"
// #include "time_utils.h"

/* TODO: define to disable task_t::ever_called */

#define CO_MAX_TIMER_POOL_SIZE  64


/*  - provide CO_NEXT to have all coro traced
    - provide co_REG_INTERN to have internal coroutines named as well
    - add CO_REG when calling a coro to actually register it's name */

#define CO_REG(t)         t
#define CO_REG_INTERN(t)  t
#define CO_NEXT(h)        h

#ifndef CO_REG
# define CO_REG(t)        co::dbg_register(t, sformat("%20s:%5d:%s", __FILE__, __LINE__, #t))
#endif

#ifndef CO_REG_INTERN
# define CO_REG_INTERN(t) co::dbg_register(t, sformat("%26d:__%s", __LINE__, #t))
#endif

#ifndef CO_NEXT
# define CO_NEXT(h)       co::dbg_coro_str_forward(h)
#endif

/* ASSERT COroutine FuNction */
#define ASSERT_COFN(fn_call)\
if (intptr_t(fn_call) < 0) {\
    DBGE("FAILED: " #fn_call);\
    co_return -1;\
}

/* ASSERT End COroutine FuNction (used on the function that you want to use to return the final
result) */
#define ASSERT_ECOFN(fn_call)\
if (intptr_t(fn_call) < 0) {\
    DBGE("FAILED: " #fn_call);\
    co_await co::force_stop(-1);\
    co_return -1;\
}

namespace co
{

struct pool_t;
struct promise_t;
struct co_mod_t;
struct sem_t;

using handle_t = std::coroutine_handle<promise_t>;
using handle_vt = std::coroutine_handle<void>;
using co_mod_ptr_t = std::shared_ptr<co_mod_t>;
using sem_it_t = std::list<handle_t>::iterator;

using trace_ctx_t = void *;
using trace_fn_t = void (*)(int moment, void *coro_addr, trace_ctx_t ctx);

enum {
    CO_MOD_NONE = 0,
    CO_MOD_TIMEOUT,
    CO_MOD_TRACE,

    CO_MOD_TIMEO_STATE_RUNNING,
    CO_MOD_TIMEO_STATE_FD,
    CO_MOD_TIMEO_STATE_SEM,
    CO_MOD_TIMEO_STATE_TIMEO,
    CO_MOD_TIMEO_STATE_STOPPED,

    CO_MOD_TRACE_MOMENT_CALL,       /* When a function called is  */
    CO_MOD_TRACE_MOMENT_LEAVE,
    CO_MOD_TRACE_MOMENT_RETURN,
    CO_MOD_TRACE_MOMENT_REENTRY,
    CO_MOD_TRACE_MOMENT_FD_WAIT,
    CO_MOD_TRACE_MOMENT_FD_UNWAIT,
    CO_MOD_TRACE_MOMENT_SEM_WAIT,
    CO_MOD_TRACE_MOMENT_SEM_UNWAIT,

    CO_MOD_ERR_GENERIC = -1,
    CO_MOD_ERR_TIMEO = -2,
    CO_WAKEUP_ERR = -4,

    CO_WAKEUP_ALL = 0xffff'ffff,
};

struct sleep_handle_t {
    int *fd_ptr = NULL;

    int stop();
};

/* TODO: make internal vars private and mark functions as friend functions */

/* Struct to remember registered actions for child coroutines spawned in the future. Usefull for
registering cancelation functions, for registering traces, etc. New actions are registered by
inserting into the chain and are inherited by copying the chain at it's current root. */
static uint64_t como_num_g = 0;
struct co_mod_t {
    int type;
    co_mod_ptr_t next;

    uint64_t como_num;
    co_mod_t(int type);
    ~co_mod_t();

    co_mod_t(const co_mod_t &) = delete;
    co_mod_t(co_mod_t &&) = delete;
    co_mod_t &operator = (const co_mod_t &) = delete;
    co_mod_t &operator = (co_mod_t &&) = delete;

    union {
        struct { /* used by CO_MOD_TIMEOUT */
            char sem_it_data[sizeof(sem_it_t)];
            char sleep_handle_data[sizeof(sem_it_t)];

            uint64_t timeo_us;
            bool timer_started;
            int state;
            sem_it_t *sem_it;
            sleep_handle_t *sleep_handle;
            int wait_fd;
            sem_t *sem;         /* if the blocking is by an semaphore */
            void *leaf_coro;    /* the lowest coro that is currently waiting */
            void *root_coro;    /* the coro that initiated the sleep, on timeo it will be
                                        rescheduled */
        } timeo;
        struct {
            trace_ctx_t ctx;
            trace_fn_t  fn;
        } trace;
    }m;

    static co_mod_ptr_t attach(co_mod_ptr_t origin, co_mod_ptr_t other);

    /* TODO: figure out if those need to return void or int, some of them should return void */
    static int handle_pmods_call(handle_t handle);
    static int handle_pmods_ret(handle_t handle);
    static int handle_pmods_leave(handle_t handle);
    static int handle_pmods_reentry(handle_t handle);

    static int handle_pmods_fd_wait(handle_t handle, int fd);
    static int handle_pmods_fd_unwait(handle_t handle);
    static int handle_pmods_sem_wait(handle_t handle, sem_t *sem, sem_it_t it);
    static int handle_pmods_sem_unwait(handle_t handle);
};

/* main awaiter and main promise handle. */
struct task_t {
    using promise_type = promise_t;

    task_t(handle_t handle);
    ~task_t();

    task_t(task_t& oth);
    task_t(task_t&& oth);
    task_t &operator = (task_t& oth);
    task_t &operator = (task_t&& oth);

    bool      await_ready() noexcept;
    handle_vt await_suspend(handle_t caller) noexcept;
    int       await_resume() noexcept;

    handle_t handle;
    bool ever_called = false;
};

struct initial_awaiter_t {
    bool await_ready() noexcept;
    void await_suspend(handle_t self) noexcept;
    void await_resume() noexcept;
};

struct final_awaiter_t {
    final_awaiter_t(pool_t *pool);

    bool      await_ready() noexcept;
    handle_vt await_suspend(handle_t oth) noexcept;
    void      await_resume() noexcept;

    static handle_vt final_awaiter_cleanup(pool_t *pool, handle_t ending_task);

    pool_t *pool = nullptr;
};

/* this holds the task metadata */
struct promise_t {
    task_t get_return_object();

    initial_awaiter_t initial_suspend() noexcept;
    final_awaiter_t   final_suspend() noexcept;
    void              return_value(int ret_val);
    void              unhandled_exception();

    int ret_val;
    int call_res;
    pool_t *pool = nullptr;

    handle_t caller;
    co_mod_ptr_t pmods;
};

struct fd_sched_t {
    struct fd_data_t {
        struct waiter_t {
            uint32_t mask = 0;
            void *ptr = nullptr;
        };
        uint32_t mask = 0;
        std::vector<waiter_t> waiters;
    };

    fd_sched_t();
    handle_t get_next();

    int  insert_wait(handle_t to_wait, int fd, uint32_t wait_cond);
    int  remove_wait(int fd, uint32_t wait_cond);
    int  wakeup_wait(int fd, uint32_t mask = CO_WAKEUP_ALL);

    bool pending();
    int  check_events_noblock();
    int  wait_events();
    int  handle_events(int num_evs);

    std::map<int, fd_data_t> waiting_tasks;
    iter_queue<handle_t> ready_tasks;
    std::vector<struct epoll_event> ret_evs;
    int epoll_fd;

};

struct pool_t {
    using task_it_t = std::set<handle_t>::iterator;

    void sched(handle_t handle, co_mod_ptr_t pmods = nullptr);
    void sched(task_t task, co_mod_ptr_t pmods = nullptr);

    int  run();

    handle_vt next_task();
    handle_vt resched_wait_fd(handle_t to_wait, int fd, int wait_cond);

    ~pool_t();

    iter_queue<handle_t> waiting_tasks;
    std::stack<int> timer_fd_pool;
    fd_sched_t fd_sched;
    int ret_val = 0;
};

struct fd_awaiter_t {
    bool      await_ready();
    handle_vt await_suspend(handle_t caller_handle);
    int       await_resume();

    ~fd_awaiter_t();

    int wait_cond;
    int fd;

    handle_t caller_handle;
    pool_t *pool;
};

/* This awaiter does only one thing and that is to schedule a coroutine */
struct sched_awaiter_t {
    handle_t to_sched;

    bool await_ready();
    bool await_suspend(handle_t handle);
    void await_resume();
};

struct yield_awaiter_t {
    bool      await_ready();
    handle_vt await_suspend(handle_t handle);
    void      await_resume();
};

struct sleep_awaiter_t {
    bool      await_ready();
    handle_vt await_suspend(handle_t caller_handle);
    int       await_resume();

    int **fd_ref = NULL;
    uint64_t sleep_us;
    int timer_fd;

    handle_t caller_handle;
    pool_t *pool;
};

/* The semaphore is an object with an internal counter and is usefull in signaling and mutual
exclusion:
    1. co_await - suspends the current coroutine if the counter is zero, else decrements the counter
                  returns an unlocker_t object that has a member function .unlock() which calls
                  rel(). unlocker_t also has a function lock() that does nothing, this is to allow
                  unlocker_t to be used inside std::lock_guard.
    2. rel      - increments the counter.
    3. rel_all  - increments the counter with the amount of waiting coroutines on this semaphore

    At a suspension point coroutines that suspended on a wait are candidates for rescheduling. Upon
    rescheduling the internal counter will be decremented.

    You must manually use (co_await co::yield()) to suspend the current coroutine if you want the
    notified coroutine to have a chance to be rescheduled. You can also call a suspending coro to
    reschedule.
*/

struct sem_t {
    sem_t(int64_t counter = 0);

    /* TODO: make it move safe and delete-copy because in the semaphore it is very important that
    internal pointers don't break and for now on copy you loose the pointer */
    // sem_t(const sem_t &) = delete;
    // sem_t(sem_t &&) = delete;
    // sem_t &operator = (const sem_t &) = delete;
    // sem_t &operator = (sem_t &&) = delete;

    struct unlocker_t {
        unlocker_t(sem_t *sem);

        void lock();
        void unlock();

        sem_t *sem;
    };

    struct sem_awaiter_t {
        sem_awaiter_t(sem_t *sem);

        bool       await_ready();
        handle_vt  await_suspend(handle_t to_suspend);
        unlocker_t await_resume();

        sem_t *sem;
    };

    sem_awaiter_t operator co_await () noexcept;

    void rel();
    void rel_all();
    
    void _dec();                      /* this is not private but don't use it */
    void _erase_waiting(sem_it_t it); /* this is not private but don't use it */

private:
    void _awake_one();

    int64_t counter;
    std::list<handle_t> waiting_on_sem;
};

/* helper functions to convert co:: variables to string */
inline const char *co_str(void *addr);
inline const char *co_str(handle_vt handle);
inline const char *co_str_intern(void *addr, std::string name = "");            // just don't touch
inline const char *co_str_intern(handle_vt handle, std::string co_name = "");   // just don't touch
inline const char *enum_str(int e);
inline std::string epoll_ev2str(uint32_t code);
inline void dbg_trace_fn(int moment, void *addr, void *);
template <typename TASK_T>
inline TASK_T dbg_register(TASK_T &&task, std::string task_name);
template <typename CORO_T>
inline CORO_T dbg_coro_str_forward(CORO_T &&coro);
inline task_t dbg_print_internal_info();

/* This function schedules a coroutine on the current pool, a pointer to a modifier list can be
supplied to attach it to the newly scheduled coroutine.  */
inline sched_awaiter_t sched(task_t to_sched, co_mod_ptr_t pmods = nullptr);

/* This stops the current coroutine from running and places it in the ready queue. */
inline yield_awaiter_t yield();

/* tasks call other tasks, creating call-chains. A modifier can be attached to a task to be
propagated to it and called tasks. The modifiers do not propagate to tasks that are scheduled by
using co::sched or pool->sched, but you can modify the task to contain a modifier. */
inline task_t mod_task(task_t task, co_mod_ptr_t pmods);

/* add a timer for an entire call-chain, on suspension points the whole chain will be destroied if
the timer reached zero and the root task will return CO_MOD_ERR_TIMEO */
inline task_t timed(task_t task, uint64_t timeo_us);

/* add a callback to be called when a call-chain reaches one of the stages CO_MOD_TRACE_MOMENT_* */
inline task_t trace(task_t task, trace_fn_t fn = dbg_trace_fn, trace_ctx_t ctx = NULL);

/* calls an awaitable inside a task_t, this is done to be able to use pmods on awaitables */
template <typename Awaiter>
inline task_t await(Awaiter& awaiter);

/* causes the running pool::run to return */
inline task_t force_stop(int ret);

/* Functions for rw from file descriptors if needed you can use them as an example */
inline task_t accept(int fd, sockaddr *sa, socklen_t *len);
inline task_t read(int fd, void *buff, size_t len);
inline task_t write(int fd, const void *buff, size_t len);
inline task_t read_sz(int fd, void *buff, size_t len);
inline task_t write_sz(int fd, const void *buff, size_t len);

/* event can be EPOLLIN or EPOLLOUT (maybe some others work too?) This can be used to be notified
when those are ready without doing a read or a write. */
inline task_t wait_event(int fd, int event);

/* closing a file descriptor while it is managed by the coroutine pool will break the entire system
so you must use stopfd on the file descriptor before you close it. This makes sure the fd is
awakened and ejected from the system before closing it. For example:

    co_await co::stopfd(fd);
    close(fd);
*/
inline task_t stopfd(int fd);

/* Normal sleeps */
inline task_t sleep_us(uint64_t timeo_us);
inline task_t sleep_ms(uint64_t timeo_ms);
inline task_t sleep_s(uint64_t timeo_s);

/* Those fns are meant to create interuptible sleeps */
inline task_t var_sleep_us(uint64_t timeo_us, sleep_handle_t *sleep_handle);
inline task_t var_sleep_ms(uint64_t timeo_ms, sleep_handle_t *sleep_handle);
inline task_t var_sleep_s(uint64_t timeo_s, sleep_handle_t *sleep_handle);
inline int stop_sleep(sleep_handle_t *sleep_handle);

/* OBS: Broken, breaks gcc if I call it, it worked before */
inline task_t when_all(std::vector<task_t> tasks);

/* IMPLEMENTATION:
================================================================================================= */
/* ============================================================================================== */
/* ============================================================================================== */

inline int sleep_handle_t::stop() {
    ASSERT_FN(stop_sleep(this));
    return 0;
}

inline co_mod_t::co_mod_t(int type) : type(type), como_num(como_num_g++) {
    memset(&m, 0, sizeof(m));
    if (type == CO_MOD_TIMEOUT) {
        m.timeo.sem_it = new (m.timeo.sem_it_data) sem_it_t();
        m.timeo.sleep_handle = new (m.timeo.sleep_handle_data) sleep_handle_t();
    }
}

inline co_mod_t::~co_mod_t() {
    if (type == CO_MOD_TIMEOUT) {
        m.timeo.sem_it->~sem_it_t();
        m.timeo.sleep_handle->~sleep_handle_t();
    }
}

inline co_mod_ptr_t co_mod_t::attach(co_mod_ptr_t origin, co_mod_ptr_t other) {
    if (!origin)
        return other;
    auto curr = origin;
    while (curr) {
        if (!curr->next) {
            curr->next = other;
            break;
        }
        curr = curr->next;
    }
    return origin;
}

/* The handle is the handle to the pmod  */
inline int co_mod_t::handle_pmods_call(handle_t handle) {
    co_mod_ptr_t curr = handle.promise().pmods;
    while (curr) {
        switch (curr->type) {
            case CO_MOD_TIMEOUT: {
                if (!curr->m.timeo.timer_started) {
                    auto timeo_task = [](co_mod_ptr_t pmod) -> task_t
                    {
                        ASSERT_COFN(co_await CO_REG_INTERN(var_sleep_us(pmod->m.timeo.timeo_us,
                                pmod->m.timeo.sleep_handle)));
                        if (pmod->m.timeo.state == CO_MOD_TIMEO_STATE_STOPPED) {
                            /* If we are in this case it means that the action concluded before the
                            timeout so there is nothing more to do */
                            co_return 0;
                        }
                        int state = pmod->m.timeo.state;
                        pmod->m.timeo.state = CO_MOD_TIMEO_STATE_TIMEO;

                        auto root_coro = handle_t::from_address(pmod->m.timeo.root_coro);
                        auto pool = root_coro.promise().pool;
                        /*  At this point we know that the timer elapsed while the coroutine was
                        waiting for some event to complete. We must now stop that event and we must
                        unwrap the call such that the coroutine that made the 'timed' call will be
                        rescheduled and the return value in the respective task will be nagative. */

                        if (state == CO_MOD_TIMEO_STATE_RUNNING) {
                            /* nothing to do here? check in yield maybe? */
                        }
                        if (state == CO_MOD_TIMEO_STATE_FD) {
                            int ret = pool->fd_sched.remove_wait(pmod->m.timeo.wait_fd, EPOLLIN);
                            if (ret < 0) {
                                DBG("Failed to remove fd from wating list, will ignore");
                            }
                        }
                        if (state == CO_MOD_TIMEO_STATE_SEM) {
                            pmod->m.timeo.sem->_erase_waiting(*pmod->m.timeo.sem_it);
                        }

                        /* unwind all the coroutines from leaf to root */
                        auto leaf = handle_t::from_address(pmod->m.timeo.leaf_coro);
                        while (leaf && leaf.address() != root_coro.address()) {
                            leaf.promise().call_res = CO_MOD_ERR_TIMEO;
                            auto caller = leaf.promise().caller;
                            leaf.destroy();
                            leaf = caller;
                        }
                        if (!leaf) {
                            DBG("This makes no sense");
                            co_return CO_MOD_ERR_GENERIC; 
                        }

                        /* schedule the caller of co::timed to be awakened with timeo */
                        root_coro.promise().call_res = CO_MOD_ERR_TIMEO;
                        root_coro.promise().ret_val = CO_MOD_ERR_TIMEO;
                        pool->waiting_tasks.push(handle_t::from_address(
                                final_awaiter_t::final_awaiter_cleanup(pool, root_coro).address()));
                        co_return 0;
                    }(curr);
                    handle.promise().pool->sched(CO_REG_INTERN(timeo_task));
                    curr->m.timeo.timer_started = true;
                    curr->m.timeo.root_coro = handle.address();
                    curr->m.timeo.state = CO_MOD_TIMEO_STATE_RUNNING;
                }
                /* The root_coro is set only once, but the leaf_coro is set every time a new coro
                is spawned. That's because the coro chain must be unwinded on an abort from the
                last called coroutine to the original coroutine. */
                curr->m.timeo.leaf_coro = handle.address();
            } break;
            case CO_MOD_TRACE: {
                curr->m.trace.fn(CO_MOD_TRACE_MOMENT_CALL, handle.address(), curr->m.trace.ctx);
            } break;
        }
        curr = curr->next;
    }
    return 0;
}

inline int co_mod_t::handle_pmods_ret(handle_t handle) {
    co_mod_ptr_t curr = handle.promise().pmods;
    while (curr) {
        switch (curr->type) {
            case CO_MOD_TIMEOUT: {
                curr->m.timeo.leaf_coro = handle.address();
                if (curr->m.timeo.leaf_coro == curr->m.timeo.root_coro) {
                    ASSERT_FN(stop_sleep(curr->m.timeo.sleep_handle));
                    curr->m.timeo.state = CO_MOD_TIMEO_STATE_STOPPED;
                }
            } break;
            case CO_MOD_TRACE: {
                curr->m.trace.fn(CO_MOD_TRACE_MOMENT_RETURN, handle.address(), curr->m.trace.ctx);
            } break;
        }
        curr = curr->next;
    }
    return 0;
}

inline int co_mod_t::handle_pmods_leave(handle_t handle) {
    co_mod_ptr_t curr = handle.promise().pmods;
    while (curr) {
        switch (curr->type) {
            case CO_MOD_TRACE: {
                curr->m.trace.fn(CO_MOD_TRACE_MOMENT_LEAVE, handle.address(), curr->m.trace.ctx);
            } break;
        }
        curr = curr->next;
    }
    return 0;
}

inline int co_mod_t::handle_pmods_reentry(handle_t handle) {
    co_mod_ptr_t curr = handle.promise().pmods;
    while (curr) {
        switch (curr->type) {
            case CO_MOD_TIMEOUT: {
                curr->m.timeo.leaf_coro = handle.address();
            } break;
            case CO_MOD_TRACE: {
                curr->m.trace.fn(CO_MOD_TRACE_MOMENT_REENTRY, handle.address(), curr->m.trace.ctx);
            } break;
        }
        curr = curr->next;
    }
    return 0;
}

inline int co_mod_t::handle_pmods_fd_wait(handle_t handle, int fd) {
    co_mod_ptr_t curr = handle.promise().pmods;
    while (curr) {
        switch (curr->type) {
            case CO_MOD_TIMEOUT: {
                curr->m.timeo.state = CO_MOD_TIMEO_STATE_FD;
                curr->m.timeo.wait_fd = fd;
            } break;
            case CO_MOD_TRACE: {
                curr->m.trace.fn(CO_MOD_TRACE_MOMENT_FD_WAIT, handle.address(), curr->m.trace.ctx);
            } break;
        }
        curr = curr->next;
    }
    return 0;
}

inline int co_mod_t::handle_pmods_fd_unwait(handle_t handle) {
    co_mod_ptr_t curr = handle.promise().pmods;
    while (curr) {
        switch (curr->type) {
            case CO_MOD_TIMEOUT: {
                curr->m.timeo.state = CO_MOD_TIMEO_STATE_RUNNING;
            } break;
            case CO_MOD_TRACE: {
                curr->m.trace.fn(CO_MOD_TRACE_MOMENT_FD_UNWAIT, handle.address(), curr->m.trace.ctx);
            } break;
        }
        curr = curr->next;
    }
    return 0;
}

inline int co_mod_t::handle_pmods_sem_wait(handle_t handle, co::sem_t *sem, sem_it_t it) {
    co_mod_ptr_t curr = handle.promise().pmods;
    while (curr) {
        switch (curr->type) {
            case CO_MOD_TIMEOUT: {
                curr->m.timeo.state = CO_MOD_TIMEO_STATE_SEM;
                curr->m.timeo.sem = sem;
                *curr->m.timeo.sem_it = it;
            } break;
            case CO_MOD_TRACE: {
                curr->m.trace.fn(CO_MOD_TRACE_MOMENT_SEM_WAIT, handle.address(), curr->m.trace.ctx);
            } break;
        }
        curr = curr->next;
    }
    return 0;
}

inline int co_mod_t::handle_pmods_sem_unwait(handle_t handle) {
    co_mod_ptr_t curr = handle.promise().pmods;
    while (curr) {
        switch (curr->type) {
            case CO_MOD_TIMEOUT: {
                curr->m.timeo.state = CO_MOD_TIMEO_STATE_RUNNING;
            } break;
            case CO_MOD_TRACE: {
                curr->m.trace.fn(CO_MOD_TRACE_MOMENT_SEM_UNWAIT, handle.address(),
                        curr->m.trace.ctx);
            } break;
        }
        curr = curr->next;
    }
    return 0;
}


/* task_t: -------------------------------------------------------------------------------------- */

inline task_t::task_t(handle_t handle) : handle(handle) {}

inline task_t::~task_t() {
    if (!ever_called) {
        DBG("This coroutine was never called, that's not ok");
    }
}

inline task_t::task_t(task_t& oth) {
    handle = oth.handle;
    oth.ever_called = true;
}

inline task_t::task_t(task_t&& oth) {
    handle = std::move(oth.handle);
    oth.ever_called = true;
}

inline task_t &task_t::operator = (task_t& oth) {
    handle = oth.handle;
    oth.ever_called = true;
    return *this;
}

inline task_t &task_t::operator = (task_t&& oth) {
    handle = std::move(oth.handle);
    oth.ever_called = true;
    return *this;
}


inline bool task_t::await_ready() noexcept {
    return false;
}

inline handle_vt task_t::await_suspend(handle_t caller) noexcept {
    ever_called = true;
    handle.promise().caller = caller;
    handle.promise().pmods = co_mod_t::attach(handle.promise().pmods, caller.promise().pmods);
    handle.promise().pool = caller.promise().pool;

    co_mod_t::handle_pmods_leave(caller);
    int ret = co_mod_t::handle_pmods_call(handle);
    if (ret < 0) {
        DBG("Failed to handle moded corutine");
        return CO_NEXT(std::noop_coroutine());
    }

    return CO_NEXT(handle);
}

inline int task_t::await_resume() noexcept {
    ever_called = true;
    int ret = handle.promise().ret_val;
    handle.destroy();
    return ret;
}

/* initial_awaiter_t: --------------------------------------------------------------------------- */

inline bool initial_awaiter_t::await_ready() noexcept {
    return false;
}

inline void initial_awaiter_t::await_suspend(handle_t self) noexcept {
}

inline void initial_awaiter_t::await_resume() noexcept {
}

/* final_awaiter_t: ----------------------------------------------------------------------------- */

inline final_awaiter_t::final_awaiter_t(pool_t *pool) : pool(pool) {
}

inline bool final_awaiter_t::await_ready() noexcept {
    return false;
}

inline handle_vt final_awaiter_t::await_suspend(handle_t ending_task) noexcept {
    return CO_NEXT(final_awaiter_cleanup(pool, ending_task));
}

inline void final_awaiter_t::await_resume() noexcept {
    DBG("Exceptions are not my coup of tea, if it goes it goes");
    std::terminate();
}

inline handle_vt final_awaiter_t::final_awaiter_cleanup(pool_t *pool, handle_t ending_task) {
    auto caller = ending_task.promise().caller;
    if (co_mod_t::handle_pmods_ret(ending_task) < 0) {
        DBG("Pmods ret failed for some reason");
        return std::noop_coroutine();
    }
    if (caller) {
        co_mod_t::handle_pmods_reentry(caller);
        return caller;
    }
    else {
        auto ret = pool->next_task();
        ending_task.destroy();
        return ret;
    }
}

/* promise_t: ----------------------------------------------------------------------------------- */

inline task_t promise_t::get_return_object() {
    return task_t{handle_t::from_promise(*this)};
}

inline initial_awaiter_t promise_t::initial_suspend() noexcept {
    return {};
}

inline final_awaiter_t promise_t::final_suspend() noexcept {
    return final_awaiter_t(pool);
}

inline void promise_t::return_value(int ret_val) {
    this->ret_val = ret_val;
}

inline void promise_t::unhandled_exception() {
    DBG("Exceptions are not my coup of tea, if it goes it goes");
    std::terminate();
}

/* fd_sched_t: ---------------------------------------------------------------------------------- */

inline fd_sched_t::fd_sched_t() {
    epoll_fd = epoll_create1(EPOLL_CLOEXEC);
    if (epoll_fd < 0) {
        DBG("Failed to create epoll");
    }
}

inline int fd_sched_t::check_events_noblock() {
    int num_evs;
    ASSERT_FN(num_evs = epoll_wait(epoll_fd, ret_evs.data(), ret_evs.size(), 0));
    int ret = handle_events(num_evs);
    return ret;
}

inline bool fd_sched_t::pending() {
    return ready_tasks.size();
}

inline handle_t fd_sched_t::get_next() {
    auto ret = ready_tasks.front();
    ready_tasks.pop();
    return ret;
}

inline int fd_sched_t::wait_events() {
    int num_evs;
    do {
        num_evs = epoll_wait(epoll_fd, ret_evs.data(), ret_evs.size(), -1);
    } while (num_evs < 0 && errno == EINTR);
    ASSERT_FN(num_evs);
    return handle_events(num_evs);
}

inline int fd_sched_t::insert_wait(handle_t to_wait, int fd, uint32_t wait_cond) {
    if (!wait_cond) {
        DBG("Wait cond can't be zero");
        return -1;
    }
    if (HAS(waiting_tasks, fd)) {
        auto &fd_data = waiting_tasks[fd];
        if (fd_data.mask & wait_cond) {
            DBG("Can't have two coroutines waiting on the same fd and same events %x vs %x",
                    fd_data.mask, wait_cond);
            return -1;
        }
        struct epoll_event ev = {};
        ev.events = wait_cond | fd_data.mask;
        ev.data.fd = fd;
        ASSERT_FN(epoll_ctl(epoll_fd, EPOLL_CTL_MOD, fd, &ev));
        fd_data.mask = ev.events;
        fd_data.waiters.push_back(fd_data_t::waiter_t{ .mask = wait_cond, .ptr = to_wait.address() });
        to_wait.promise().call_res = CO_MOD_ERR_GENERIC;
        return 0;
    }
    else {
        auto &fd_data = waiting_tasks[fd];
        struct epoll_event ev = {};
        ev.events = wait_cond;
        ev.data.fd = fd;
        ASSERT_FN(epoll_ctl(epoll_fd, EPOLL_CTL_ADD, fd, &ev));
        fd_data.mask = ev.events;
        fd_data.waiters.push_back(fd_data_t::waiter_t{ .mask = wait_cond, .ptr = to_wait.address() });
        to_wait.promise().call_res = CO_MOD_ERR_GENERIC;
        ret_evs.push_back(epoll_event{});
        return 0;
    }
    return 0;
}

inline int fd_sched_t::remove_wait(int fd, uint32_t wait_cond) {
    if (!HAS(waiting_tasks, fd)) {
        DBG("Couldn't remove fd: %d", fd);
        return -1;
    }
    auto &fd_data = waiting_tasks[fd];
    if ((fd_data.mask & ~wait_cond) != 0) {
        struct epoll_event ev = {};
        ev.events = fd_data.mask & ~wait_cond;
        ev.data.fd = fd;
        ASSERT_FN(epoll_ctl(epoll_fd, EPOLL_CTL_MOD, fd, &ev));
        fd_data.mask = ev.events;
       
        fd_data.waiters.erase(std::remove_if(
            fd_data.waiters.begin(),
            fd_data.waiters.end(),
            [wait_cond](const fd_data_t::waiter_t& m) { 
                return (wait_cond & m.mask) == m.mask;
            }),
            fd_data.waiters.end()
        );
    }
    else {
        waiting_tasks.erase(fd);
        ASSERT_FN(epoll_ctl(epoll_fd, EPOLL_CTL_DEL, fd, NULL));
        ret_evs.pop_back();
    }

    return 0;
}

inline int fd_sched_t::wakeup_wait(int fd, uint32_t mask) {
    if (!HAS(waiting_tasks, fd)) {
        return 0;
    }
    auto &fd_data = waiting_tasks[fd];
    uint32_t final_mask = 0;
    for (auto &w : fd_data.waiters) {
        if ((w.mask & mask) == w.mask) {
            final_mask |= w.mask;
            auto handle = handle_t::from_address(w.ptr);
            if (handle) {
                handle.promise().call_res = CO_WAKEUP_ERR;
                ready_tasks.push(handle);
            }   
        }
    }
    if (final_mask) {
        ASSERT_FN(remove_wait(fd, final_mask));
    }
    return 0;
}

inline int fd_sched_t::handle_events(int num_evs) {
    for (int i = 0; i < num_evs; i++) {
        if (!HAS(waiting_tasks, ret_evs[i].data.fd)) {
            DBG("Something is vey rowng!");
            return -1;
        }
        auto &fd_data = waiting_tasks[ret_evs[i].data.fd];
        for (auto &w : fd_data.waiters) {
            if (w.mask & ret_evs[i].events) {
                auto handle = handle_t::from_address(w.ptr);
                if (handle) {
                    handle.promise().call_res = 0;
                    ready_tasks.push(handle);
                }
            }
        }
    }
    return 0;
}

/* pool_t: -------------------------------------------------------------------------------------- */

inline void pool_t::sched(handle_t handle, co_mod_ptr_t pmods) {
    // DBG("scheduling: %s", co_str(handle));
    handle.promise().pool = this;
    if (pmods)
        handle.promise().pmods = pmods; /* sched is async, so non blocking in regards to the caller */
    waiting_tasks.push(handle);
}

inline void pool_t::sched(task_t task, co_mod_ptr_t pmods) {
    task.ever_called = true;
    sched(task.handle, pmods);
}

inline int pool_t::run() {
    co_str_intern(std::noop_coroutine().address(), "std::noop_coroutine");

    if (!waiting_tasks.size())
        return 0;
    handle_t first_task = waiting_tasks.front();
    waiting_tasks.pop();
    first_task.resume();
    return ret_val;
}

inline handle_vt pool_t::next_task() {
    if (waiting_tasks.size()) {
        /* We run an already scheduled task */
        handle_t to_resume = waiting_tasks.front();
        waiting_tasks.pop();
        return to_resume;
    }
    else {
        handle_t ret;

        if (fd_sched.pending()) {
            ret = fd_sched.get_next();
        }
        else {
            if (fd_sched.ret_evs.size() == 0) {
                DBG("Scheduler doesn't have any more coroutines to handle, will exit");
                return std::noop_coroutine();
            }
            int err = fd_sched.check_events_noblock();
            if (err < 0) {
                DBG("The epoll loop failed, will stop the scheduler");
                return std::noop_coroutine();
            }
            if (fd_sched.pending()) {
                ret = fd_sched.get_next();
            }
            else {
                err = fd_sched.wait_events();
                if (err < 0) {
                    DBG("The epoll loop failed to wait, stopping the scheduler");
                    return std::noop_coroutine();
                }
                if (!fd_sched.pending()) {
                    DBG("We have no more pending waits, this is an invalid state");
                    return std::noop_coroutine();
                }
                ret = fd_sched.get_next();
            }
        }
        return ret;
    }
}

inline handle_vt pool_t::resched_wait_fd(handle_t to_wait, int fd, int wait_cond) {
    if (fd_sched.insert_wait(to_wait, fd, wait_cond) < 0) {
        DBG("Can't insert a file descriptor for waiting");
        return std::noop_coroutine();
    }
    return next_task();
}

inline pool_t::~pool_t() {
    while (timer_fd_pool.size()) {
        close(timer_fd_pool.top());
        timer_fd_pool.pop();
    }
}

/* fd_awaiter_t --------------------------------------------------------------------------------- */

inline bool fd_awaiter_t::await_ready() {
    return false;
}

inline handle_vt fd_awaiter_t::await_suspend(handle_t caller_handle) {
    /* We get the pool object, we mark in the pool that we wait for events on this fd and we
    let the next coroutine to continue on this thread */
    pool = caller_handle.promise().pool;
    this->caller_handle = caller_handle;
    co_mod_t::handle_pmods_fd_wait(caller_handle, fd);
    return CO_NEXT(pool->resched_wait_fd(caller_handle, fd, wait_cond));
}

inline int fd_awaiter_t::await_resume() {
    /* this coroutine can only suspend on call, so we know that the scheduler was asked to
    take controll of this coroutine and we know that 'sched' and 'handle' are set, as such
    we can request the return value for the fd we just queried */

    co_mod_t::handle_pmods_fd_unwait(caller_handle);
    return caller_handle.promise().call_res;
}

inline fd_awaiter_t::~fd_awaiter_t() {
    int res = caller_handle.promise().call_res;
    if (res != CO_MOD_ERR_TIMEO && res != CO_WAKEUP_ERR)
        pool->fd_sched.remove_wait(fd, wait_cond);
}

/* sched_awaiter_t ------------------------------------------------------------------------------ */

inline bool sched_awaiter_t::await_ready() {
    return false;
}

inline bool sched_awaiter_t::await_suspend(handle_t handle) {
    handle.promise().pool->sched(to_sched);
    return false;
}

inline void sched_awaiter_t::await_resume() {}

/* yield_awaiter_t ------------------------------------------------------------------------------ */

inline bool yield_awaiter_t::await_ready() {
    return false;
}

inline handle_vt yield_awaiter_t::await_suspend(handle_t handle) {
    handle.promise().pool->waiting_tasks.push(handle);
    return CO_NEXT(handle.promise().pool->next_task());
}

inline void yield_awaiter_t::await_resume() {}

/* sleep_awaiter_t ------------------------------------------------------------------------------ */

inline bool sleep_awaiter_t::await_ready() {
    return false;
}

inline handle_vt sleep_awaiter_t::await_suspend(handle_t caller_handle) {
    pool = caller_handle.promise().pool;

    FnScope err_scope;
    if (pool->timer_fd_pool.size() > 0) {
        timer_fd = pool->timer_fd_pool.top();
        pool->timer_fd_pool.pop();
        err_scope([&]{ close(timer_fd); });
    }
    else {
        timer_fd = timerfd_create(CLOCK_REALTIME, 0);
        if (timer_fd < 0) {
            DBGE("Failed to allocate new timer");
            return CO_NEXT(std::noop_coroutine());
        }
    }
    itimerspec its = {};
    its.it_value.tv_nsec = (sleep_us % 1000'000) * 1000ULL;
    its.it_value.tv_sec = sleep_us / 1000'000ULL;
    if (fd_ref) {
        *fd_ref = &timer_fd;
    }
    if (timerfd_settime(timer_fd, 0, &its, NULL) < 0) {
        DBGE("Failed to set expiration date");
        return CO_NEXT(std::noop_coroutine());
    }

    err_scope.disable();
    this->caller_handle = caller_handle;
    co_mod_t::handle_pmods_fd_wait(caller_handle, timer_fd);
    return CO_NEXT(pool->resched_wait_fd(caller_handle, timer_fd, EPOLLIN));
}

inline int sleep_awaiter_t::await_resume() {
    /* this coroutine can only suspend on call, so we know that the scheduler was asked to
    take controll of this coroutine and we know that 'sched' and 'handle' are set, as such
    we can request the return value for the fd we just queried */

    pool->fd_sched.remove_wait(timer_fd, EPOLLIN);
    if (pool->timer_fd_pool.size() >= CO_MAX_TIMER_POOL_SIZE) {
        close(timer_fd);
    }
    else {
        pool->timer_fd_pool.push(timer_fd);
    }

    co_mod_t::handle_pmods_fd_unwait(caller_handle);
    return caller_handle.promise().call_res;
}

/* sem_t ---------------------------------------------------------------------------------------- */

inline sem_t::unlocker_t::unlocker_t(sem_t *sem) : sem(sem) {}

inline void sem_t::unlocker_t::lock() {}

inline void sem_t::unlocker_t::unlock() {
    sem->rel();
}

inline sem_t::sem_t(int64_t counter) : counter(counter) {

}

inline sem_t::sem_awaiter_t::sem_awaiter_t(sem_t *sem) : sem(sem) {

}

inline bool sem_t::sem_awaiter_t::await_ready() {
    if (sem->counter > 0) {
        sem->counter--;
        return true;
    }
    return false;
}

inline handle_vt sem_t::sem_awaiter_t::await_suspend(handle_t to_suspend) {
    /* if we are here it must mean that the counter was zero and as such we must yield */
    sem->waiting_on_sem.push_front(to_suspend);
    sem_it_t it = sem->waiting_on_sem.begin();
    co_mod_t::handle_pmods_sem_wait(to_suspend, sem, it);

    return CO_NEXT(to_suspend.promise().pool->next_task());
}

inline sem_t::unlocker_t sem_t::sem_awaiter_t::await_resume() {
    return sem_t::unlocker_t(sem);
}

inline sem_t::sem_awaiter_t sem_t::operator co_await () noexcept {
    return sem_t::sem_awaiter_t(this);
}


inline void sem_t::rel() {
    if (counter == 0 && waiting_on_sem.size())
        _awake_one();
    else {
        counter++;
        if (counter == 0)
            _awake_one();
    }
}

inline void sem_t::_dec() {
    counter--;
}

inline void sem_t::rel_all() {
    if (counter > 0)
        return ;
    while (waiting_on_sem.size())
        _awake_one();
}

inline void sem_t::_erase_waiting(sem_it_t it) {
    waiting_on_sem.erase(it);
}

inline void sem_t::_awake_one() {
    auto to_awake = waiting_on_sem.back();
    waiting_on_sem.pop_back();
    co_mod_t::handle_pmods_sem_unwait(to_awake);
    to_awake.promise().pool->waiting_tasks.push(to_awake);
}

/* usefull functions ---------------------------------------------------------------------------- */

inline const char *enum_str(int e) {
    switch (e) {
        case CO_MOD_NONE:                    return "CO_MOD_NONE";
        case CO_MOD_TIMEOUT:                 return "CO_MOD_TIMEOUT";
        case CO_MOD_TRACE:                   return "CO_MOD_TRACE";

        case CO_MOD_TIMEO_STATE_RUNNING:     return "CO_MOD_TIMEO_STATE_RUNNING";
        case CO_MOD_TIMEO_STATE_FD:          return "CO_MOD_TIMEO_STATE_FD";
        case CO_MOD_TIMEO_STATE_SEM:         return "CO_MOD_TIMEO_STATE_SEM";
        case CO_MOD_TIMEO_STATE_TIMEO:       return "CO_MOD_TIMEO_STATE_TIMEO";
        case CO_MOD_TIMEO_STATE_STOPPED:     return "CO_MOD_TIMEO_STATE_STOPPED";

        case CO_MOD_TRACE_MOMENT_CALL:       return "CO_MOD_TRACE_MOMENT_CALL";
        case CO_MOD_TRACE_MOMENT_LEAVE:      return "CO_MOD_TRACE_MOMENT_LEAVE";
        case CO_MOD_TRACE_MOMENT_RETURN:     return "CO_MOD_TRACE_MOMENT_RETURN";
        case CO_MOD_TRACE_MOMENT_REENTRY:    return "CO_MOD_TRACE_MOMENT_REENTRY";
        case CO_MOD_TRACE_MOMENT_FD_WAIT:    return "CO_MOD_TRACE_MOMENT_FD_WAIT";
        case CO_MOD_TRACE_MOMENT_FD_UNWAIT:  return "CO_MOD_TRACE_MOMENT_FD_UNWAIT";
        case CO_MOD_TRACE_MOMENT_SEM_WAIT:   return "CO_MOD_TRACE_MOMENT_SEM_WAIT";
        case CO_MOD_TRACE_MOMENT_SEM_UNWAIT: return "CO_MOD_TRACE_MOMENT_SEM_UNWAIT";

        case CO_MOD_ERR_GENERIC:             return "CO_MOD_ERR_GENERIC";
        case CO_MOD_ERR_TIMEO:               return "CO_MOD_ERR_TIMEO";
        default: return "CO_UNKNOWN";
    }
}

/* TODO: make a different co_str for usual usage and for internal usage */
inline const char *co_str(void *addr) {
    return co_str(handle_vt::from_address(addr));
}

inline const char *co_str(handle_vt handle) {
    static std::map<void *, std::string> id_str;
    static uint64_t id = 0;

    if (!HAS(id_str, handle.address()))
        id_str[handle.address()] = sformat("co[%3d]", id++);
    return id_str[handle.address()].c_str();
}

inline const char *co_str_intern(void *addr, std::string name) {
    return co_str_intern(handle_vt::from_address(addr), name);
}

inline const char *co_str_intern(handle_vt handle, std::string co_name) {
    static std::map<void *, std::string> name_str;
    if (!(HAS(name_str, handle.address()) && co_name == ""))
        name_str[handle.address()] = sformat("%s[%s]", co_str(handle), co_name.c_str());
    return name_str[handle.address()].c_str();
}


inline std::string epoll_ev2str(uint32_t code) {
    std::map<int, std::string> ev_str = {
        { EPOLLIN        ,"EPOLLIN"        },
        { EPOLLOUT       ,"EPOLLOUT"       },
        { EPOLLRDHUP     ,"EPOLLRDHUP"     },
        { EPOLLPRI       ,"EPOLLPRI"       },
        { EPOLLERR       ,"EPOLLERR"       },
        { EPOLLHUP       ,"EPOLLHUP"       },
        { EPOLLET        ,"EPOLLET"        },
        { EPOLLONESHOT   ,"EPOLLONESHOT"   },
        { EPOLLWAKEUP    ,"EPOLLWAKEUP"    },
        { EPOLLEXCLUSIVE ,"EPOLLEXCLUSIVE" },
    };

    bool add_or = false;
    std::string ret = "[";
    for (auto &[ev, str] : ev_str) {
        if (code & ev) {
            ret += (add_or ? "|" : "") + str;
            add_or = true;
        }
    }
    ret += "]";
    return ret;
}

inline task_t dbg_print_internal_info() {
    /* TODO: print everything from poll */
    struct dbg_awaiter_t {
        bool await_ready() { return false; }
        bool await_suspend(handle_t curr) {
            auto pool = curr.promise().pool;
            auto sch = &(pool->fd_sched);

            DBG("====== CO::POOL/INTERNALS ======")
            for (auto &handle : pool->waiting_tasks) {
                DBG("pool:waiting: %s", co_str(handle))
            }
            DBG("Timers in pool: %ld", pool->timer_fd_pool.size());
            DBG("Return value: %d", pool->ret_val);

            for (auto &[fd, fdw] : sch->waiting_tasks) {
                DBG("fd: %d mask: %x", fd, fdw.mask);
                for (auto &w : fdw.waiters) {
                    DBG("->\twaiter: %x coro: %s", w.mask, co_str(w.ptr));
                }
            }
            for (auto &handle : sch->ready_tasks) {
                DBG("sch:ready: %s", co_str(handle));
            }
            DBG("------ CO::POOL/INTERNALS ------")

            return false;
        }
        void await_resume() {}
    };
    co_await dbg_awaiter_t{};
    co_return 0;
}

inline sched_awaiter_t sched(task_t to_sched, co_mod_ptr_t pmods) {
    if (pmods)
        to_sched.handle.promise().pmods = pmods;
    to_sched.ever_called = true;
    return sched_awaiter_t{to_sched.handle};
}

inline yield_awaiter_t yield() {
    return yield_awaiter_t{};
}

inline task_t mod_task(task_t task, co_mod_ptr_t pmods) {
    task.handle.promise().pmods = pmods;
    return task;
}

inline task_t timed(task_t task, uint64_t timeo_us) {
    /* here most often then not the promise is allocated but it doesn't yet know what is the pool
    that it is running on. Still, we have access to some pmods. */
    /* What we will do is the following: add the CO_MOD_TIMEOUT to this task and return it to the
    caller. When co_awaited by another task this coroutine will have the timeo flag set with the
    max time it can elapse from that point. The task responsable for the timeout will schedule a
    sleep in paralel to the scheduled coroutine pointing to this co_mod_t.   */

    auto timeo_pmod = std::make_shared<co_mod_t>(CO_MOD_TIMEOUT);
    /* TODO: maybe check if alloc failed? */

    timeo_pmod->m.timeo.timeo_us = timeo_us;

    task.handle.promise().pmods = co_mod_t::attach(timeo_pmod, task.handle.promise().pmods);
    return task;
}

inline task_t trace(task_t task, trace_fn_t fn, trace_ctx_t ctx) {
    auto trace_pmod = std::make_shared<co_mod_t>(CO_MOD_TRACE);

    trace_pmod->m.trace.fn = fn;
    trace_pmod->m.trace.ctx = ctx;

    task.handle.promise().pmods = co_mod_t::attach(trace_pmod, task.handle.promise().pmods);
    return task;
}

template <typename Awaiter>
inline task_t await(Awaiter& awaiter) {
    co_await awaiter;
    co_return 0; 
}

inline task_t force_stop(int ret) {
    struct stop_awaiter_t {
        bool        await_ready() { return false; }
        handle_vt   await_suspend(handle_t curr) {
            curr.promise().pool->ret_val = ret;
            return std::noop_coroutine();
        }
        void        await_resume() {}

        int ret;
    };
    co_await stop_awaiter_t{ret};
    co_return 0;
}

#define CO_INTERNAL_AWAIT(awaiter) \
{ \
    int ret = co_await awaiter; \
    if (ret == CO_WAKEUP_ERR) { \
        co_return CO_WAKEUP_ERR; \
    }\
    if (ret < 0) \
        co_return ret; \
}

inline task_t accept(int fd, sockaddr *sa, socklen_t *len) {
    fd_awaiter_t awaiter {
        .wait_cond = EPOLLIN,
        .fd = fd,
    };
    CO_INTERNAL_AWAIT(awaiter);
    co_return ::accept(fd, sa, len);
}

inline task_t read(int fd, void *buff, size_t len) {
    fd_awaiter_t awaiter {
        .wait_cond = EPOLLIN,
        .fd = fd,
    };
    CO_INTERNAL_AWAIT(awaiter);
    co_return ::read(fd, buff, len);
}

inline task_t write(int fd, const void *buff, size_t len) {
    fd_awaiter_t awaiter {
        .wait_cond = EPOLLOUT,
        .fd = fd,
    };
    CO_INTERNAL_AWAIT(awaiter);
    co_return ::write(fd, buff, len);
}

inline task_t read_sz(int fd, void *buff, size_t len) {
    fd_awaiter_t awaiter {
        .wait_cond = EPOLLIN,
        .fd = fd,
    };
    size_t original_len = len;
    while (true) {
        if (!len)
            break ;
        CO_INTERNAL_AWAIT(awaiter);
        int ret = ::read(fd, buff, len);
        if (ret == 0) {
            DBG("Read failed, closed peer");
            co_return 0;
        }
        else if (ret < 0) {
            DBGE("Failed read");
            co_return -1;
        }
        else {
            len -= ret;
            buff = (char *)buff + ret;
        }
    }
    co_return original_len;
}

inline task_t write_sz(int fd, const void *buff, size_t len) {
    fd_awaiter_t awaiter {
        .wait_cond = EPOLLOUT,
        .fd = fd,
    };
    size_t original_len = len;
    while (true) {
        if (!len)
            break ;
        CO_INTERNAL_AWAIT(awaiter);
        int ret = ::write(fd, buff, len);
        if (ret < 0) {
            DBGE("Failed write");
            co_return -1;
        }
        else {
            len -= ret;
            buff = (char *)buff + ret;
        }
    }
    co_return original_len;
}

inline task_t wait_event(int fd, int event) {
    fd_awaiter_t awaiter {
        .wait_cond = event,
        .fd = fd,
    };
    CO_INTERNAL_AWAIT(awaiter);
    co_return 0;
}

inline task_t stopfd(int fd) {
    struct stopfd_awaiter_t {
        bool await_ready() { return false; }
        bool await_suspend(handle_t curr) {
            res = 0;
            if (curr.promise().pool->fd_sched.wakeup_wait(fd) < 0) {
                DBG("Couldn't awake fd");
                res = -1;
            }
            return false;
        }
        int await_resume() {
            return res;
        }

        int fd;
        int res;
    };
    co_return co_await stopfd_awaiter_t{.fd = fd, .res = 0};
}

inline task_t sleep_us(uint64_t timeo_us) {
    sleep_awaiter_t sleep_awaiter{ .sleep_us = timeo_us };
    ASSERT_COFN(co_await sleep_awaiter);
    co_return 0;
}

inline task_t sleep_ms(uint64_t timeo_ms) {
    ASSERT_COFN(co_await CO_REG_INTERN(sleep_us(timeo_ms * 1000)));
    co_return 0;
}

inline task_t sleep_s(uint64_t timeo_s) {
    ASSERT_COFN(co_await CO_REG_INTERN(sleep_us(timeo_s * 1000'000)));
    co_return 0;
}

inline task_t var_sleep_us(uint64_t timeo_us, sleep_handle_t *sleep_handle) {
    if (timeo_us == 0)
        co_return 0;
    if (sleep_handle->fd_ptr != (int *)(intptr_t)(-1) && sleep_handle->fd_ptr != NULL) {
        DBG("sleep_handle can't be used for multiple var_sleeps at once");
        co_return -1; /* TODO: who receives this return value? */
    }
    if (sleep_handle->fd_ptr == (int *)(intptr_t)(-1)) {
        co_return 0;
    }
    sleep_awaiter_t sleep_awaiter{ .fd_ref = &sleep_handle->fd_ptr, .sleep_us = timeo_us };
    ASSERT_COFN(co_await sleep_awaiter);
    sleep_handle->fd_ptr = (int *)(intptr_t)(-1);
    co_return 0;
}

inline task_t var_sleep_ms(uint64_t timeo_ms, sleep_handle_t *sleep_handle) {
    ASSERT_COFN(co_await CO_REG_INTERN(var_sleep_us(timeo_ms * 1000, sleep_handle)));
    co_return 0;
}

inline task_t var_sleep_s(uint64_t timeo_s, sleep_handle_t *sleep_handle) {
    ASSERT_COFN(co_await CO_REG_INTERN(var_sleep_us(timeo_s * 1000'000, sleep_handle)));
    co_return 0;
}

inline int stop_sleep(sleep_handle_t *sleep_handle) {
    if (sleep_handle->fd_ptr == NULL) {
        /* Timer is stopped apriori */
        sleep_handle->fd_ptr = (int *)(intptr_t)(-1);
        return 0;
    }
    if (sleep_handle->fd_ptr == (int *)(intptr_t)(-1)) {
        /* Timer was stopped, either here or in var_sleep after the timer elapsed */
        return 0;
    }
    itimerspec its = {};
    its.it_value.tv_nsec = 1;
    its.it_value.tv_sec = 0;
    ASSERT_FN(timerfd_settime(*sleep_handle->fd_ptr, 0, &its, NULL));
    sleep_handle->fd_ptr = (int *)(intptr_t)(-1);
    return 0;
}

inline task_t when_all(std::vector<task_t> tasks) {
    sem_t sem(0);
    int ret = 0;
    for (auto &t : tasks) {
        sem._dec();
        auto one_cond_waiter_task = [&sem, &ret](task_t h) -> task_t {
            ret |= co_await h;
            sem.rel();
            co_return ret;
        }(t);
        co_await sched(CO_REG_INTERN(one_cond_waiter_task));
    }
    co_await sem;
    co_return ret;
}

inline void dbg_trace_fn(int e, void *coaddr, void *) {
    DBG("[TRACE] %40s at %s", co::enum_str(e), co::co_str(coaddr));
}


template <typename TASK_T>
inline TASK_T dbg_register(TASK_T &&task, std::string task_name) {
    co_str_intern(task.handle, task_name);
    return std::forward<TASK_T>(task);
}

template <typename CORO_T>
inline CORO_T dbg_coro_str_forward(CORO_T &&coro) {
    DBG("co::> %s", co::co_str_intern(coro));
    return std::forward<CORO_T>(coro);
}

}

#endif

An example to showcase the library, main.cpp:

#include "co_utils.h"

/* EXAMPLE1:
================================================================================================= */

static int pipe_fd[2];

struct msg_t {
    char msg[80];
};

co::task_t write_msg(int fd, const msg_t &msg) {
    ASSERT_COFN(co_await co::write_sz(fd, &msg, sizeof(msg)));
    co_return 0;
}

co::task_t i_send_stuff() {
    msg_t msg;
    for (int i = 0; i < 20; i++) {
        strcpy(msg.msg, sformat("This is a message %d", i).c_str());
        ASSERT_ECOFN(co_await write_msg(pipe_fd[1], msg));
    }
    strcpy(msg.msg, "");
    ASSERT_ECOFN(co_await write_msg(pipe_fd[1], msg));
    co_return 0;
}

co::task_t i_receive_stuff() {
    while (true) {
        msg_t msg;
        ASSERT_ECOFN(co_await co::read_sz(pipe_fd[0], &msg, sizeof(msg)));
        if (std::string(msg.msg) == "")
            break;
        DBG("Received a message: %s", msg.msg);
    }
    co_return 0;
}

/* EXAMPLE2:
================================================================================================= */

co::task_t i_bug_out_at_3() {
    for (int i = 0; i < 20; i++) {
        DBG("%d", i);
        if (i == 3) {
            /* this assert will stop the pool, making run() to return to the caller */
            ASSERT_ECOFN(-1);
        }
    }
    co_return 0;
}

/* EXAMPLE3:
================================================================================================= */

co::task_t yield_example_1() {
    for (int i = 0; i < 10; i++) {
        DBG("%d", i * 2);
        co_await co::yield();
    }
    co_return 0;
}

co::task_t yield_example_2() {
    for (int i = 0; i < 10; i++) {
        DBG("%d", i * 2 + 1);
        co_await co::yield();
    }
    co_return 0;
}

/* EXAMPLE4:
================================================================================================= */

co::task_t timed_example() {
    DBG("Pre-wait");
    int ret = co_await CO_REG(co::timed(co::sleep_s(10), 1000'000));
    DBG("ret: %s", co::enum_str(ret));
    DBG("Post-wait (should see 1s delay instead of 10s)");
    co_return 0;
}

/* EXAMPLE5:
================================================================================================= */

co::task_t traced_example_3() {
    DBG("traced_example_3");
    co_return 0;
}

co::task_t traced_example_2() {
    DBG("traced_example_2");
    ASSERT_COFN(co_await traced_example_3());
    co_return 0;
}

co::task_t traced_example_1() {
    DBG("traced_example_1");
    ASSERT_COFN(co_await traced_example_2());
    co_return 0;
}

/* EXAMPLE6:
================================================================================================= */

co::sem_t sem(-10);

co::task_t sched_print_num(int num) {
    DBG("%d", num);
    sem.rel();
    co_return 0;
}

co::task_t sched_example() {
    DBG("Scheduling ...");
    for (int i = 0; i < 10; i++)
        co_await co::sched(sched_print_num(i));

    DBG("Waiting results ...");
    /* OBS: if there are no more coroutines that can awake the semaphore the pool will just exit,
    this is in contrast to just blocking forever. This is just a result of the way those coroutines
    are implemented in co_utils.h */
    co_await sem;
    DBG("Done");
    co_return 0;
}

/* EXAMPLE7:
================================================================================================= */

co::task_t force_stop_example_1() {
    DBG("Start");
    co_await co::sleep_ms(100);
    DBG("Stop");

    /* co::force_stop will stop all coroutines and will make pool_t::run to return to caller with
    the number passed to co::force_stop as a return value for pool_t::run */
    co_await co::force_stop(0);
    DBG("Not reached");
    co_return 0;
}

co::task_t force_stop_example_2() {
    for (int i = 0; i < 20; i++) {
        DBG("%d", i);
        co_await co::sleep_ms(10);
    }
    co_return 0;
}

/* EXAMPLE8:
================================================================================================= */

co::task_t sender() {
    int num = 0;
    DBG("Started sender");
    while (true) {
        ASSERT_ECOFN(co_await co::write_sz(pipe_fd[1], &num, sizeof(num)));
        num++;
        co_await co::sleep_ms(100);
    }
    co_return 0;
}

co::task_t receiver() {
    int num;
    DBG("Started receiver");
    while (true) {
        int ret = co_await co::read_sz(pipe_fd[0], &num, sizeof(num));
        if (ret == co::CO_WAKEUP_ERR) {
            DBG("Was stopped by quiter");
            co_await co::force_stop(0);
        }
        ASSERT_ECOFN(ret);
        DBG("%d", num);
    }
    co_return 0;
}

co::task_t external_quit() {
    co_await co::sleep_s(1);
    DBG("I'll stop the receiver");
    co_await co::stopfd(pipe_fd[0]);
    co_return 0;
}

/* MAIN:
================================================================================================= */

int main(int argc, char const *argv[])
{
    ASSERT_FN(pipe(pipe_fd));
    co::pool_t pool;

    /* OBS: uncomment only the example you want to run */

    /* Example 1 */
    pool.sched(i_send_stuff());
    pool.sched(i_receive_stuff());

    /* Example 2 */
    // pool.sched(i_bug_out_at_3());

    /* Example 3 */
    // pool.sched(yield_example_1());
    // pool.sched(yield_example_2());

    /* Example 4 */
    // pool.sched(timed_example());

    /* Example 5 */
    // pool.sched(co::trace(traced_example_1()));

    /* Example 6 */
    // pool.sched(sched_example());

    /* Example 7 */
    // pool.sched(force_stop_example_1());
    // pool.sched(force_stop_example_2());

    /* Example 8 */
    // pool.sched(receiver());
    // pool.sched(sender());
    // pool.sched(external_quit());

    ASSERT_FN(pool.run());
    return 0;
}

The header utils.h, needed for compiling a small example:

#ifndef UTILS_H
#define UTILS_H

#include <errno.h>
#include <string.h>
#include <chrono>
#include <string>
#include <vector>
#include <inttypes.h>
#include <unistd.h>
#include <memory>
#include <functional>

inline void logger_log_message(const char *msg) {
    printf("%s", msg);
}

inline uint64_t logger_get_time() {
    using namespace std::chrono;
    return duration_cast<milliseconds>(
        system_clock::now().time_since_epoch()
    ).count();
}

#define DBG_RAW(fmt, dbg_filename_, dbg_line_, dbg_funcname_, ...)                                 \
([&](const char *dbg_filename, int dbg_line, const char *dbg_funcname) {                           \
    std::vector<char> logger_buff;                                                                 \
    uint64_t time_ms = logger_get_time();                                                          \
    logger_buff.resize(snprintf(NULL, 0, "[%" PRIu64 "] %s:%d %s() :> " fmt "\n",                  \
            time_ms, dbg_filename, dbg_line, dbg_funcname, ##__VA_ARGS__) + 1);                    \
    snprintf(logger_buff.data(), logger_buff.size(), "[%" PRIu64 "] %s:%d %s() :> " fmt "\n",      \
            time_ms, dbg_filename, dbg_line, dbg_funcname, ##__VA_ARGS__);                         \
    logger_log_message(logger_buff.data());                                                        \
}(dbg_filename_, dbg_line_, dbg_funcname_));

#define DBG(fmt, ...) DBG_RAW(fmt, __FILE__, __LINE__, __func__, ##__VA_ARGS__)

#define DBGE(fmt, ...)\
    DBG("[SYS] " fmt "[err: %s, code: %d]", ##__VA_ARGS__,\
            strerror(errno), errno)

#define ASSERT_FN(fn_call)\
if (intptr_t(fn_call) < 0) {\
    DBGE("FAILED: " #fn_call);\
    return -1;\
}

#define CHK_MMAP(x) ((x) == MAP_FAILED ? -1 : 0)
#define CHK_BOOL(x) ((x) ? 0 : -1)
#define CHK_PTR(x)  ((x) == NULL ? -1 : 0)

#define HAS(data_struct, key)   (data_struct.find(key) != data_struct.end())

struct FnScope {
    using fn_t = std::function<void(void)>;
    std::vector<fn_t> fns;
    bool done = false;

    FnScope(fn_t fn) {
        add(fn);
    }

    FnScope() {}

    ~FnScope() {
        call();
    }

    void operator() (fn_t fn) {
        add(fn);
    }

    void add(fn_t fn) {
        fns.push_back(fn);
    }

    void disable() {
        fns.clear();
    }

    void call() {
        for (auto &f : fns)
            f();
        fns.clear();
    }
};

template<typename T, typename Container=std::deque<T>>
class iter_queue : public std::queue<T,Container>
{
public:
    typedef typename Container::iterator iterator;
    typedef typename Container::const_iterator const_iterator;

    iterator begin() { return this->c.begin(); }
    iterator end() { return this->c.end(); }
    const_iterator begin() const { return this->c.begin(); }
    const_iterator end() const { return this->c.end(); }
};

template <typename Arg>
inline auto sformat_arg(const Arg& arg) {
    return arg;
}

inline const char *sformat_arg(const std::string& arg) {
    return arg.c_str();
}

template <typename ...Args>
inline std::string sformat(const char *fmt, Args&&...args) {
    auto wformat_err_bypass = &snprintf;
    int cnt = wformat_err_bypass(NULL, 0, fmt, sformat_arg(args)...);
    if (cnt <= 0)
        return "";
    std::vector<char> buff(cnt + 1);
    wformat_err_bypass(buff.data(), buff.size(), fmt, sformat_arg(args)...);
    return buff.data();
}

#endif

You can compile it with g++-11.1.0:

g++ -O3 -std=c++2a -g main.cpp
\$\endgroup\$

1 Answer 1

4
\$\begingroup\$

I was very interested in reviewing this code, because it would be the first time I’ve reviewed any code with coroutines. Unfortunately, I just couldn’t get through it. There is so much code, and it is so poorly organized, with so many issues, that I could barely make it about a quarter of the way through before the review became so huge I had to stop.

As it is, this code is functionally unreadable. There is so much obfuscation with macros and meaningless type aliases, that even reading a simple declaration often required jumping around across quite literally thousands of lines of code to figure what the hell was going on. And of course, all the interfaces and implementations are separated, even when there is no logical reason to do so. I got as far as (what appears to be) the end of the declarations before I had to tap out, which means I didn’t even get to the good stuff: the actual coroutine implementations.

So this review is not even close to complete, but I guess it’s at least something.

#ifndef CO_UTILS_H

It is a good idea to prefix all your macros… but CO_ is hardly enough of a prefix.

#include <iostream>
#include <unordered_map>
#include <sys/socket.h>
#include <arpa/inet.h>
#include <set>
#include <stack>
#include <map>
#include <queue>
#include <list>
#include <coroutine>
#include <sys/epoll.h>
#include <sys/timerfd.h>

It is a good idea to order includes in a logical way. An example would be alphabetical, but also, grouping by library. For example, including all standard library includes first, alphabetically, then grouping by library, alphabetically.

#define CO_MAX_TIMER_POOL_SIZE  64

This should not be a preprocessor macro. It should be a constexpr constant.

#define CO_REG(t)         t
#define CO_REG_INTERN(t)  t
#define CO_NEXT(h)        h

#ifndef CO_REG
# define CO_REG(t)        co::dbg_register(t, sformat("%20s:%5d:%s", __FILE__, __LINE__, #t))
#endif

#ifndef CO_REG_INTERN
# define CO_REG_INTERN(t) co::dbg_register(t, sformat("%26d:__%s", __LINE__, #t))
#endif

#ifndef CO_NEXT
# define CO_NEXT(h)       co::dbg_coro_str_forward(h)
#endif

I’m not sure any of these are a great idea, but for sure just naming them things like CO_REG is a terrible idea. If you saw ??_REG(...) in someone else’s code, what would you think? Does it regulate something? Does it mark something as regular? All a name like that does is piss off a reviewer or maintainer, who now has to go digging through documentation or (god forbid) code to figure out what it does.

Also, keep in mind that macros in general are an increasingly bad idea. As modules become better supported, people are likely going to chose to import header files, rather than include them, because it is both faster, and safer. That will break your entire library if it depends on macros.

/* ASSERT COroutine FuNction */
#define ASSERT_COFN(fn_call)\
if (intptr_t(fn_call) < 0) {\
    DBGE("FAILED: " #fn_call);\
    co_return -1;\
}

Yikes, no.

First, the name. It is not even prefixed with the library prefix, but even if it were… “assert coffin”? Why not ASSERT_COROUTINE_FUNCTION? Because it’s long and stands out too much? Good!

Second, to even begin to figure out what is going on with this macro, I had to dive through almost a dozen layers of indirection spread all across the header file. As near as I can figure, fn_call is supposed to be an int. That would have been nice to know without having to dig through the whole library.

Third… intptr_t is never defined. What’s that? It’s a standard type? No, it is not. std::intptr_t is a standard type, but only once you’ve included <cstdint>, and even then, it is optional. If fn_call is supposed to be an int, then there is no sensible reason to cast it to std::intptr_t. If fn_call is supposed to be a pointer… then there is still no sensible reason to cast it to std::intptr_t, because it doesn’t make any sense to see if it’s negative.

It seems the only point of this macro is to call this DBGE macro (don’t even get me started on that!) if the result of some coroutine call was less than zero, then return -1. This kind of obfuscation is a terrible idea. Can you imagine the frustration of a coder who is trying to figure out why a function—like sleep_ms(), which has a single return statement that returns zero—is somehow returning negative one? Or, even worse, a function like var_sleep_us(), which does visibly return −1, so a coder who sees −1 returned from that function wouldn’t think that there might be another, hidden, statement returning −1, and will likely spend hours in confusion and frustration debugging some problem… only to discover, in the end, the thing that created the problem in the first place was actually one of the macros that is supposed to help with debugging.

At best this macro “helps” the library writer, at the cost of screwing the library user… which is sure to make the library popular among users, hm?

#define ASSERT_ECOFN(fn_call)\

“Assert economical function”? “Assert ecological function”?

namespace co

No, this is a terrible name for a top-level namespace.

If someone wants to use a short name, and it is safe to do so, they can always do namespace co = /* whatever */;.

using handle_t = std::coroutine_handle<promise_t>;
using handle_vt = std::coroutine_handle<void>;
using co_mod_ptr_t = std::shared_ptr<co_mod_t>;
using sem_it_t = std::list<handle_t>::iterator;

These aliases don’t help. In fact, they make the code near unreadable.

An alias like sem_it_t is particularly bad, because it isn’t even a semaphore’s iterator, or an iterator of semaphores. It’s an iterator of coroutine handles. Which, maybe, is something that a semaphore uses? 🤷🏼

using trace_ctx_t = void *;
using trace_fn_t = void (*)(int moment, void *coro_addr, trace_ctx_t ctx);

In C++, we keep the type modifiers with the type name.

In other words, void* coro_addr, not void *coro_addr.

enum {
    CO_MOD_NONE = 0,
    CO_MOD_TIMEOUT,
    CO_MOD_TRACE,

    CO_MOD_TIMEO_STATE_RUNNING,
    CO_MOD_TIMEO_STATE_FD,
    CO_MOD_TIMEO_STATE_SEM,
    CO_MOD_TIMEO_STATE_TIMEO,
    CO_MOD_TIMEO_STATE_STOPPED,

    CO_MOD_TRACE_MOMENT_CALL,       /* When a function called is  */
    CO_MOD_TRACE_MOMENT_LEAVE,
    CO_MOD_TRACE_MOMENT_RETURN,
    CO_MOD_TRACE_MOMENT_REENTRY,
    CO_MOD_TRACE_MOMENT_FD_WAIT,
    CO_MOD_TRACE_MOMENT_FD_UNWAIT,
    CO_MOD_TRACE_MOMENT_SEM_WAIT,
    CO_MOD_TRACE_MOMENT_SEM_UNWAIT,

    CO_MOD_ERR_GENERIC = -1,
    CO_MOD_ERR_TIMEO = -2,
    CO_WAKEUP_ERR = -4,

    CO_WAKEUP_ALL = 0xffff'ffff,
};

This is not an acceptable pattern in modern C++.

First, don’t use old-school C-style enumerations.

Second, don’t use unnamed enumerations.

Third, don’t use ALL_CAPS for enumerators.

This also seems to be at least three or four different groups of enumerators all jammed together. Why? That just hamstrings the compiler’s safety guards. One of the many ways modern compilers try to help make code safer is checking switch statements to make sure that all of an enumerations enumerators are covered, so if you accidentally miss a case, you get a warning. Writing code like the above prevents the compiler from helping you.

struct sleep_handle_t {
    int *fd_ptr = NULL;

nullptr, not NULL.

static uint64_t como_num_g = 0;

First, it should be std::uint64_t, though you need <cstdint>, and it’s only optional. Really, there doesn’t seem to be any logical reason to use std::uint64_t at all. The proper type for this is std::size_t.

More importantly… never create a static global variable in a header. It won’t do what you think it will.

If anything, it should be inline, but there doesn’t seem any reason why this should exist at namespace scope at all. It is an internal detail of co_mod_t.

Speaking of, co_mod_t is up next, but I confess I have no idea what the purpose of this class is, even with the comment. So I can’t comment on the design, only the implementation… which… is not good.

The class appears to be two things: a linked list node/root, and a sum type to hold the data for multiple types of coroutine. It’s the latter part that’s problematic, because it is implemented using a C-style union. That’s bad, but it gets worse.

That’s because, for some reason, you’ve decided that the types all have to be trivial types, so you can initialize them with that old C hack of memset()ing everything to zeroes. Unfortunately, some of the data members of those types are not trivial, so you have to resort to chicanery… and that’s where the first bit of UB gets introduced:

            char sem_it_data[sizeof(sem_it_t)];
            char sleep_handle_data[sizeof(sem_it_t)];

It is not good enough to merely allocate enough space for an object. You also have to align that storage correctly:

alignas(sem_it_t) char sem_it_data[sizeof(sem_it_t)];
// or, better:
alignas(sem_it_t) std::byte sem_it_data[sizeof(sem_it_t)];

And then, of course, if you are accessing that storage as the type you want to put in it, you have to use std::launder()… although you sidestep that problem by using a separate pointer, which works, but means the type now has to be locked in place—both non-copyable and non-movable—which is a pain.

And to hammer home the point that all this nonsense is not only unnecessary, but also a terrible, terrible idea… you didn’t even notice the copy-paste error in the code that causes the second bit of UB, did you? Look more closely at the declaration of sleep_handle_data.

And the third bit of UB comes from the fact that since you just default initialize the union, that means the first type is always automatically the active type… but then that’s not always the type you actually use, and reading a non-active member of a union: UB.

All of this complexity (and the bugs) are completely unnecessary, if you just use a std::variant:

// Note: I reordered the elements of the class into something
// more logical, rather than mixing data members and member
// functions haphazardly.
class co_mod_t
{
public:
    // ... [ all those static functions ] ...

    // Note: single argument constructors should almost always
    // be explicit.
    //
    // Also, this should either be an enum type or a tag type,
    // not a raw int.
    explicit co_mod_t(int type);

    // A better interface:
    //co_mod_t(timeout_tag_t, /* args */);
    //co_mod_t(trace_tag_t, /* args */);

    // ~co_mod_t(); // don't need this anymore

    // Non-copyable:
    co_mod_t(co_mod_t const&) = delete;
    auto operator=(co_mod_t const&) -> co_mod_t& = delete;

    // Non-movable (doesn't need to be anymore):
    //co_mod_t(co_mod_t&&) noexcept;
    //auto operator=(co_mod_t&&) noexcept -> co_mod_t&;

private:
    // "tim-e-o"? If this is supposed to be "timeout", just
    // spell it out!
    struct _timeo_t
    {
        // don't need these anymore:
        //char sem_it_data[sizeof(sem_it_t)];
        //char sleep_handle_data[sizeof(sem_it_t)];
        // just use these instead:
        sem_it_t sem_it = {};
        sleep_handle_t sleep_handle = {};

        std::uint_fast64_t timeo_us = {};   // uint_fast64_t is
                                            // more portable
                                            // than uint64_t
        bool timer_started = false;
        int state = {}; // should probably be an enum type
        //sem_it_t *sem_it;             // don't need these anymore
        //sleep_handle_t *sleep_handle; // don't need these anymore
        int wait_fd = {};
        sem_t* sem = {};    // C-style block comments? in 2023? => /* if the blocking is by an semaphore */
        void* leaf_coro = {};    /* the lowest coro that is currently waiting */
        void* root_coro = {};    /* the coro that initiated the sleep, on timeo it will be
                                    rescheduled */
    };

    struct _trace_t
    {
        trace_ctx_t ctx = {};
        trace_fn_t  fn = {};
    };

    std::variant<
        //std::monostate,   // use this only if you want a "null" state
        _timeo_t,
        _trace_t
    > m;    // needs a better name than "m"

    // int type;    // don't need this anymore
    co_mod_ptr_t next = {};

    std::uint64_t como_num;
};

// Example constructor, done the bad way:
co_mod_t::co_mod_t(int type)
    : como_num{como_num_g++}
{
    switch (type)
    {
    case /* timeout */:
        m.emplace<_timeo>();
        break;
    case /* trace */:
        m.emplace<_trace>();
        break;
    }

    // All data members of the m type still have to be set. :/
}

// Example constructors, done the smart way:
co_mod_t::co_mod_t(timeout_tag_t, /* args */)
    : m{std::in_place_type_t<_timeo_t>, /* args */}
    , como_num{como_num_g++}
{}
co_mod_t::co_mod_t(trace_tag_t, /* args */)
    : m{std::in_place_type_t<_trace_t>, /* args */}
    , como_num{como_num_g++}
{}

// Example usage:
auto co_mod_t::handle_pmods_reentry(handle_t handle) -> int
{
    for (auto curr = handle.promise().pmods; curr; curr = curr->next)
    {
        std::visit(
            [handle](auto&& x)
            {
                using T = std::decay_t<decltype(x)>;
                if constexpr (std::is_same_v<T, _timeo_t>)
                {
                    x.leaf_coro = handle.address();
                }
                else if constexpr (std::is_same_v<T, _trace_t>)
                {
                    x.fn(CO_MOD_TRACE_MOMENT_REENTRY, handle.address(), x.ctx);
                }
            },
            curr->m
        );

        // Or, using the overloaded idiom:
        //std::visit(
        //    overloaded{
        //        [handle](_timeo_t& x) { x.leaf_coro = handle.address(); },
        //        [handle](_trace_t& x) { x.fn(CO_MOD_TRACE_MOMENT_REENTRY, handle.address(), x.ctx); }
        //    },
        //    curr->m
        //);
    }

    return 0;
}

// Another example, for the case where you only care about one
// alternative:
auto co_mod_t::handle_pmods_leave(handle_t handle) -> int
{
    for (auto curr = handle.promise().pmods; curr; curr = curr->next)
    {
        if (auto p = std::get_if<_trace_t>(&(curr->m)); p)
            p->fn(CO_MOD_TRACE_MOMENT_LEAVE, handle.address(), p->ctx);
    }

    return 0;
}

Alternatively, you could simply specify a standardized interface for the variant types, and use that. For example:

class co_mod_t
{
    // [...]

    struct _timeo_t
    {
        // [...]

        auto handle_pmods_reentry(handle_t) -> int;
    };

    struct _trace_t
    {
        // [...]

        auto handle_pmods_reentry(handle_t) -> int;
    };

    std::variant<
        //std::monostate,   // use this only if you want a "null" state
        _timeo_t,
        _trace_t
    > m;

    // [...]
};

auto co_mod_t::handle_pmods_reentry(handle_t handle) -> int
{
    for (auto curr = handle.promise().pmods; curr; curr = curr->next)
    {
        std::visit(
            //[](std::monostate) {},    // only if you have a null state
            [handle](auto& x) { x.handle_pmods_reentry(handle); },
            curr->m
        );
    }

    return 0;
}

It is no exaggeration to say the variant version would be objectively better than the manual union version by literally every metric:

  • It is strongly type-safe.
  • It requires far, far less code, and much simpler code.
  • It is very easy to extend to new types and behaviours.
  • It uses less memory (no need for the extra pointers).
  • It is faster (no need for repeatedly initializing the same memory).
  • It will even work in constexpr contexts.
struct task_t {
    using promise_type = promise_t;

    task_t(handle_t handle);
    ~task_t();

    task_t(task_t& oth);
    task_t(task_t&& oth);
    task_t &operator = (task_t& oth);
    task_t &operator = (task_t&& oth);

Again, single-argument constructors should almost always be explicit.

Now, you have a sorta-kinda copy constructor and copy assignment operator that apparently can both only copy non-const objects. It took a lot more jumping around in the code to figure out why (this is a depressingly common pattern), but it seems to be because you want to set ever_called to true in the object you copy from.

Things like this should be a red flag that your design is flawed. It does not make logical sense that copying a thing changes the thing that was copied.

Indeed, it doesn’t really make a lot of conceptual sense to “copy a task” in any case. Say the task is “buy an apple”. What does it mean to “copy” that task? Buy two apples? Buy the same apple twice? Just logically, a task should be non-copyable. (Moving tasks, however, does make sense, in many ways. You can move a task temporally (buy the apple today, or tomorrow), or in other ways (Alice can buy the apple, or you can move the task to Bob and have him buy it instead… where “Alice” and “Bob” could be threads, execution contexts, or many other things).)

Also, your move ops should probably be noexcept.

struct initial_awaiter_t {
    bool await_ready() noexcept;
    void await_suspend(handle_t self) noexcept;
    void await_resume() noexcept;
};

It took more jumping around in the code to figure out, as seems to be the way with this library, but isn’t this just an inferior remake (no constexpr, not even const-correct) of std::suspend_always?

/* this holds the task metadata */

So why isn’t it defined in the task?

There is something to be said for the style of programming where one declares all the interfaces up front, and then defines all the implementation stuff later. However, that only really works if you:

  1. combine the declarations with some meaningful documentation; and
  2. don’t obfuscate every thing with incomprehensible aliases and macros.

If you’re just separating the declaration and definition by (in some cases literally!!!) a thousand lines of code, all while concealing what the types actually are, all you’re doing is making your code unmaintainable.

struct fd_sched_t {
    // ...

    iter_queue<handle_t> ready_tasks;

Oh, goody. Another incomprehensible type name that gives no real clue to its purpose, that is probably defined hundreds, if not thousands of lines of code elsewhere.

I eventually did find the definition of iter_queue… in the second header of this “single-header library”. Sigh.

So, this is what iter_queue is:

template<typename T, typename Container=std::deque<T>>
class iter_queue : public std::queue<T,Container>
{
public:
    typedef typename Container::iterator iterator;
    typedef typename Container::const_iterator const_iterator;

    iterator begin() { return this->c.begin(); }
    iterator end() { return this->c.end(); }
    const_iterator begin() const { return this->c.begin(); }
    const_iterator end() const { return this->c.end(); }
};

Now, publicly deriving from standard containers like this is not a great idea. std::queue does not have a virtual destructor. The correct thing to do would be to privately inherit from std::queue, and use using declarations to bring in the desired interface.

However, it does raise the question of why you need iteration over a queue. As near as I can figure (again, this took endless jumping around through the code), the one and only use for queue iteration is to print debug info. That’s literally it. You have created a whole new type for no other purpose than that.

But does that make sense? Why even bother to use std::queue in the first place, if you’re just going to crack it open and change its interface? ready_tasks is just an internal type. You could just as well use a std::vector or std::deque; literally the only difference is that instead of .push() and .pop(), you’d write .push_back() and .pop_back() in exactly 4 places, all of which are within member functions of the same class. (One additional place uses .size() where .empty() makes more sense.) So it doesn’t matter at all to users of the class.

Don’t write 10+ lines of code when 0 will do the same job.

Indeed, it rarely makes sense to use the container adapters (std::stack and std::queue) within an implementation. Their only purpose is to restrict the interface of containers. That doesn’t make sense to do for an implementation detail; nobody but the class itself (and its friends) will ever touch the queue or stack, so who cares if is possible to insert into the middle of it? If your class is so unprincipled when using the queue or stack that you actually have to worry about it, then you have much bigger problems.

(Incidentally, the same points apply to the only other use of iter_queue, in pool_t.

    std::vector<struct epoll_event> ret_evs;

Why the struct there? (And what the hell is “ret evs”? Why does everything have to be named so obtusely?)

inline const char *co_str(void *addr);
inline const char *co_str(handle_vt handle);
inline const char *co_str_intern(void *addr, std::string name = "");            // just don't touch
inline const char *co_str_intern(handle_vt handle, std::string co_name = "");   // just don't touch

I wondered why you’d want to return const char* from these functions, rather than a std::string or std::string_view, and after lots of jumping around in the code (😒), I finally concluded that it’s because the return values are intended to be used with this… monstrosity:

#define DBG_RAW(fmt, dbg_filename_, dbg_line_, dbg_funcname_, ...)                                 \
([&](const char *dbg_filename, int dbg_line, const char *dbg_funcname) {                           \
    std::vector<char> logger_buff;                                                                 \
    uint64_t time_ms = logger_get_time();                                                          \
    logger_buff.resize(snprintf(NULL, 0, "[%" PRIu64 "] %s:%d %s() :> " fmt "\n",                  \
            time_ms, dbg_filename, dbg_line, dbg_funcname, ##__VA_ARGS__) + 1);                    \
    snprintf(logger_buff.data(), logger_buff.size(), "[%" PRIu64 "] %s:%d %s() :> " fmt "\n",      \
            time_ms, dbg_filename, dbg_line, dbg_funcname, ##__VA_ARGS__);                         \
    logger_log_message(logger_buff.data());                                                        \
}(dbg_filename_, dbg_line_, dbg_funcname_));

#define DBG(fmt, ...) DBG_RAW(fmt, __FILE__, __LINE__, __func__, ##__VA_ARGS__)

Which, ultimately ends up just being a printf() statement:

inline void logger_log_message(const char *msg) {
    printf("%s", msg);
}

None of the above is acceptable in modern C++.

Let’s start at the bottom. First, it should be std::printf(), not just printf(). But of course, it shouldn’t be printf() at all. This is C++; you should at least be using std::cout, if not std::print().

The formatting could also be done by std::cout, but it would probably be easier to use std::format. All of the macros necessary to get the file and line information are also obsoleted by std::source_location.

So all the DBG and DBG_RAW macro shenanigans are just:

auto dbg_raw(std::string_view msg, std::source_location const& sloc = std::source_location::current())
{
    logger_log_message(
        std::format("[{}] {}:{} {}() :> {}\n",
            logger_get_time(),
            sloc.file_name(),
            sloc.line(),
            sloc.function_name(),
            msg
        )
    );
}

// Just one way to get a source location automagically.
// There are other options.
struct dbg_format_helper_t
{
    std::string_view fmt;
    std::source_location loc;

    dbg_format_helper_t(std::string_view s, std::source_location const& l = std::source_location::current())
        : fmt{s}
        , loc{l}
    {}
};

template <typename... Args>
auto dbg(dbg_format_helper_t dfmt, Args&&... args)
{
    auto msg = std::vformat(dfmt.fmt, std::make_format_args(std::forward<Args>(args)...));

    dbg_raw(msg, dfmt.loc);
}

And the code above is probably 3 or 4 times faster, too.

inline const char *enum_str(int e);

This function is nonsensical. I can pass in any random integer value, and it will happily return a meaningless string.

What you should have is strongly typed enumerations, and then a enum_str() function for each enumeration type. That will not only be simpler and safer (not least because the compiler can automatically check to make sure you didn’t miss any enumerators), it will also be considerably faster.


Summary

This is about where I had to stop. I would have liked to get to the coroutine stuff, but the code is so obfuscated that the moment I stepped away and came back to it after a few hours, I was lost again. There was simply no hope of reviewing anything remotely complex.

There are a number of major issues with this code that make it unusable, and unmaintainable. I’ll try to touch on at least some of them.

There is no real documentation

There are a few comments here and there, but nothing that really explains anything about the code, why it is written the way it is written, how it is intended to be used, what problems it solves, or even why it even exists in the first place.

And far from self-documenting, it’s almost like the code is going out of its way to be incomprehensible. Basic types that would be understandable to any C++ coder are replaced with pointless aliases. Any types or functions that might give a clue to what is going on are given opaque names like co_mod_fun_ptr_foo_t, as if the person writing this code is being charged by the letter, and running out of cash. This isn’t FORTRAN; you don’t have to fit identifiers on a punch card. It’s okay to spell things out (within reason, of course)… if a name is long, it can be given a shorter alias later, by the user, if they desire.

There are examples, but even they do not help, because to run the examples, I would have edit and recompile the code no less than seven freakin’ times!!! Like… seriously? Are you trying to punish anyone who wants to use or understand this library?

When I encounter a new library, the first thing I want to know, before ANYTHING ELSE is: what problem, exactly, does this library solve. I expect an answer to that immediately, an answer that is clear and understandable, or I just write the library off, and move on.

So, what, exactly, is this library trying to accomplish? What problem is it trying to solve? A single simple and clear, non-obfsucated, obtuse, or vague example is all I need. Eight vague, hypothetical examples with meaningless operations (“send stuff” and “receive stuff”) offer less help than a single, clear example of real usage.

The naming is just terrible

It starts right at the top, with the top level namespace given the supremely meaningful and totally-not-likely-to-conflict-with-anything name of… co. And it just gets worse from there.

Most of the type names are needlessly contracted, often incomprehensibly. What the hell is a co_mod_t? A covariant module type? A component modulator?

Part of the problem is that the code is so poorly organized. Take for example, promise_t. What is a reader supposed to conclude when they see promise_t somewhere random in the code? Is it some type of std::promise? No, it is a coroutine promise type. But what kind of coroutine promise type? Doesn’t say. Just says “promise t(ype)”. How helpful. Now I have to scan the whole freakin’ code base to figure it out. First I have to search for the actual promise_t declaration. But finding that doesn’t help, because it’s just declared on its own randomly between final_awaiter_t and fd_sched_t. So I have to use my knowledge of coroutines to surmise that whatever this is the promise type for, it will be aliased as promise_type in that type, and/or it will return that type from get_return_object(). And from that, I can finally find task_t.

Figuring out the purpose of one of a library’s most fundamental types shouldn’t be a puzzle game that requires intimate knowledge of dark corners of the language.

What should be going on here, is either the promise type should be declared within the coroutine type:

class task_t
{
    class promise_type
    {
        // ...
    };
};

or it should be given a meaningful name, like task_promise_t.

And then there are all the aliases. Aliases are supposed to make code more readable, not less, by:

  1. simplifying long and complex types into something simpler (using task_ptr_iterator = std::vector<std::shared_ptr<task_t>>::const_iterator)
  2. giving generic types more purpose-specific names (using task_handle = std::shared_ptr<task_t>); and
  3. creating types that can vary if necessary (might be using int_type = int on one platform and using int_type = long on another).

Most of the aliases fail to serve any of those purposes. For example, what is the point of aliasing std::coroutine_handle<void> to handle_vt? It doesn’t add clarity by simplification. It doesn’t give a more purpose-specific name. And there is no way it could ever be varied, because std::coroutine_handle<void> is a fundamental type required by the language; all coroutines must have handles that are convertible to std::coroutine_handle<void>. All it does is add one more bit of cognitive burden to someone who wants to understand the library; they have to keep in mind that handle_vt is std::coroutine_handle<void>, and mentally replace it everywhere.

And to make matters worse, there are all the macros….

Macros

Other than the include guards, there is not a single macro in this library that is worth the trouble, let alone a good idea.

Most of the macros serve no purpose other than obfuscation. As just one particularly egregious example:

#define HAS(data_struct, key)   (data_struct.find(key) != data_struct.end())

That this exists in a code base in 2023 amazes me. This would not even be acceptable in C++98.

A proper C++20 solution would be:

template <
    std::ranges::input_range R,
    typename K
>
    requires std::indirect_binary_predicate<std::ranges::equal_to, std::ranges::iterator_t<R>, T const*>
constexpr auto has(R&& data_struct, K&& key)
{
    return std::ranges::find(data_struct, std::forward<K>(key)) != std::ranges::end(data_struct);
}

Which is superior to the macro solution in just about every way: faster, more flexible, type safe, etc..

(As of C++23, you could just use std::ranges::contains().)

constexpr advances have made virtually all uses of macros obsolete, but even if we were still stuck in the bad old era before expanded constexpr, most of the macros in this library would be unacceptable. For example:

#define ASSERT_COFN(fn_call)\
if (intptr_t(fn_call) < 0) {\
    DBGE("FAILED: " #fn_call);\
    co_return -1;\
}

// used in:
inline task_t sleep_ms(uint64_t timeo_ms) {
    ASSERT_COFN(co_await CO_REG_INTERN(sleep_us(timeo_ms * 1000)));
    co_return 0;
}

// expands to:
inline task_t sleep_ms(uint64_t timeo_ms) {
    if (intptr_t(co_await CO_REG_INTERN(sleep_us(timeo_ms * 1000))) < 0) {\
        DBGE("FAILED: " "co_await CO_REG_INTERN(sleep_us(timeo_ms * 1000))");\
        co_return -1;\
    };  // <-- NOTE THE STRAY ';'!!!
    co_return 0;
}

Put aside for the moment how unnecessary it is to have having multiple suffixed functions all taking a raw integral type (sleep_ms(uint64_t), sleep_us(uint64_t), sleep_s(uint64_t)) instead of using the type system to get sleep(std::chrono::milliseconds), sleep(std::chrono::microseconds), and sleep(std::chrono::seconds) which you can easily and unambiguously and clearly call with sleep(10ms) or sleep(500us)… or even better, a single sleep(std::chrono::duration<Rep, Period>) function that works with everything, automatically and correctly.

Instead, note how the macro hides control flow. That is bad, very bad, even by ancient C standards. In the function definition, you can’t see where the returns are, or what values may possibly be returned. You could not be blamed for assuming the function will always return zero… an assumption which could completely break other code.

Note also that there is a stray semicolon after the macro… which, with certain warnings set, will break the whole build. That is why it is traditional to write statement-like macros as do { /* actual macro contents */ } while (false).

Indeed, the way the macro is written, it is dangerous, and may quietly break other code. For instance:

auto f()
{
    if (whatever)
        ASSERT_COFN(co_await stuff)
    else
        do_something_if_whatever_is_false();

    co_return 0;
}

// expands to:
auto f()
{
    if (whatever)
        if (intptr_t(co_await stuff) < 0) {\
            DBGE("FAILED: " "co_await stuff");\
            co_return -1;\
        }
    else
        do_something_if_whatever_is_false();

    co_return 0;
}

See the problem?

DON’T. USE. MACROS. They are an ancient technology, which was always dangerous, and which has been almost completely superseded by better, safer, more modern technologies.

ALL uses of macros in this library, except for the include guards (and even then, only so long as this library is not a module), are wrong, and should be removed.

There is poor use of C++’s most important features

The most powerful and important feature of C++ is its strong type system. If you’re not using that, you’re basically just writing C with classes.

For example, there is that massive enum just to create a bunch of integral constants, regardless of whether they are related or not:

enum {
    CO_MOD_NONE = 0,
    CO_MOD_TIMEOUT,
    CO_MOD_TRACE,

    CO_MOD_TIMEO_STATE_RUNNING,
    CO_MOD_TIMEO_STATE_FD,
    CO_MOD_TIMEO_STATE_SEM,
    CO_MOD_TIMEO_STATE_TIMEO,
    CO_MOD_TIMEO_STATE_STOPPED,

    // ... and so on ...

This makes no sense in C++, even going all the way back to C++98. Even in C++98, you would create separate enumerations for separate things. But of course, now we have stronger enumeration typing with enum class:

enum class /* needs a meaningful name, or to be declared in situ */
{
    none,
    timeout,
    trace
};

enum class timeo_state
{
    running,
    fd,
    sem,
    timeo,
    stopped
};

// ... and so on ...

At the very least, this will prevent stupid mistakes like:

// what a coder wanted:
auto timeo_pmod = std::make_shared<co_mod_t>(CO_MOD_TIMEOUT);

// oops, chose wrong enumerator:
auto timeo_pmod = std::make_shared<co_mod_t>(CO_MOD_TIMEO_STATE_TIMEO);

// probably won't happen exactly like this, but could happen
// easily in a more complex function:
auto type = -1; // mean to set type later
// .. but forgot to...
auto timeo_pmod = std::make_shared<co_mod_t>(type);

But even this is a mediocre solution. There are much better ways.

Proper types will help avoid tons of silly mistakes:

co::task_t force_stop_example_2() {
    constexpr auto t = 10; // time to wait in milliseconds
    for (int i = 0; i < 20; i++) {
        DBG("%d", i);
        co_await co::sleep_ms(i);   // oops, meant 't'!!!
        co_await co::sleep_s(t);    // oops, meant 'sleep_ms'!!!
    }
    co_return 0;
}

Just imagine if the sleep function was defined as:

class sleep_awaiter_t
{
public:
    constexpr explicit sleep_awaiter_t(std::chrono::nanoseconds duration)
        : _duration{duration}
    {}

    constexor auto await_ready() const noexcept { return false; }

    constexpr auto await_suspend(std::coroutine_handle<task_t::promise_type> caller_handle)
    {
        // ...

        auto const to_timespec = [](std::chrono::nanoseconds duration)
        {
            constexpr auto ns_per_s = 1'000'000'000L;

            return ::timespec{
                .tv_sec  = ::time_t(duration.count() / ns_per_s),
                .tv_nsec = long(duration.count() % ns_per_s)
            };
        };

        auto its = ::itimerspec{
            .it_value = to_timespec(_duration)
        };

        // ...
    }

    // ... etc. ...

private:
    std::chrono::nanoseconds _duration;

    // ... etc. ...
};

template <typename Rep, typename Period>
auto sleep(std::chrono::duration<Rep, Period> duration)
{
    auto sleep_awaiter = sleep_awaiter_t{duration};

    if (auto const res = co_await sleep_awaiter; res < 0)
    {
        dbge("FAILED: ", "sleep()");
        co_return -1;
    }

    co_return 0;
}

// Usage:
sleep(10s);     // sleep for 10s
sleep(100ms);   // sleep for 100ms
// sleep(10);   // won't compile, need to specify units

There are numerous other places where strong types, including already existing standard library types, would make code much safer, simpler, and more efficient. I mentioned std::variant, for example, and why it is infinitely superior to using naked C unions.

The actual API is impossible to suss out

Everything is public. Everything is at namespace scope. Everything is named incomprehensibly. That means it is literally impossible to determine which parts are internal-only, and which parts are meant to be part of the public interface.

For example, while I can assume that co_str_intern() and dbg_print_internal_info() are meant to be internal-use only (despite the lack of a consistent naming scheme), what about epoll_ev2str()? Are users supposed to use sleep_awaiter_t directly? Or are they only supposed to use the sleep*() functions?

There are debug statements and (what appears to be) debugging-only functionality exposed everywhere. I can’t tell what parts of the library are meant to the public interface, and which parts are just implementation details.

And that means I am bombarded with junk that I don’t need to see to understand what is (supposed to be) going on. That just makes the library even harder to understand. At least if I knew which parts are meant to be the public interface, I could start to figure things out from the top down (I mean… I shouldn’t have to—the library should be better written so that is not necessary). Without even that as a guide to understanding, the library is just an incomprehensible soup of code.

The only stuff that should be publicly visible is the library’s public interface. In rare cases, something might need to be publicly visible that you don’t actually want to be part of the interface… in those cases, you should use uglified names to make it clear that it is not meant to be used by the user.

Everything else should be hidden, either as private data, or in a detail namespace, or something to take it out of the public view.

That will make the amount of code that is publicly visible smaller, and thus, easier to process and understand.

… and so on

The problems mentioned above all share one thing in common: they don’t necessarily introduce bugs, but they do make the code impossible to understand.

(I mean, they do introduce bugs, but they don’t need to. It is possible to write bug-free code with macros, and using C unions rather than std::variant, and with no strong types, and so on.)

And it is because the code is impossible to understand that I couldn’t even get to the meaty stuff. The actual coroutine stuff may be riddled with bugs… or it may be fine. I can’t tell, because it’s just so hard to understand. And if I can’t understand it, I can’t use it, let alone review it.

In addition to everything mentioned above, one thing this library—any library—really needs: tests. Untested code is garbage code, and “garbage code” doesn’t necessarily mean it’s bad or it doesn’t work… it means it is absolutely useless in any real-world project. No one in their right mind would use a library that… probably… “works”… maybe. If even the library’s writers don’t know if it actually works… well, it’s utterly useless, now isn’t it?

That’s why tests are so important. Indeed, it is good practice to write the tests first, before even starting to write a library. That way, you don’t end up in a situation where your library is untestable.

And when I say tests, I mean proper tests. I don’t mean slapping together a simple sample program that you run and then eyeball the output. I mean a battery of unit tests to properly check all specified behaviour of the library, all run automatically. I mean using a testing library. I mean proper testing.

So if you actually want this library to be usable by anyone else, let alone reviewable:

  • You need to reduce the amount of junk that is publicly accessible. Hide anything that is not part of the public API.

  • You need to use proper, strong typing. This will not only protect against errors, it makes code easier to understand because it clearly documents what types and values are valid, limiting the possible.

  • You need to give things proper names. Try to remember that anyone looking at your library’s code will always start out completely clueless about it. Will a name like sem_t guide them toward understanding? Or will it just frustrate them?

  • You need proper documentation, and examples. Don’t be vague or obscure. Give real-world problems that can be solved by your library, and show how. If your library is actually useful, this should be easy.

  • You need tests. This is not optional. Without tests, a library is garbage. Indeed, I’ve heard people say the definition of “legacy code” is “code without tests”.

You could start by rewriting the entire library from the ground up… which is pretty much what is needed… but it might a better idea to just pull out the bits that are actually interesting, and get those reviewed first, and then rebuild around them. It is always going to be easier to get core subsets of a library reviewed than to get an entire library reviewed. Start small, and build up.

\$\endgroup\$
3
  • \$\begingroup\$ Thank you for your time and effort in writing this review! You've made some good points which I'll try to implement and some others with which I simply disagree: the macros and naming. I've seen code with longer names and code with shorter ones and I find the shorter name code easier to understand and the macros simplify too much of my life. Also, once you understand the macros you can easily look for them as potential 'breaking' points. I did expect a review going over the usage of the library and it's internals but I guess people simply didn't understood what I wrote, I'll keep that in mind. \$\endgroup\$
    – Pangi
    Commented Oct 22, 2023 at 20:53
  • \$\begingroup\$ Also another thing: ever_called is the only way I found to make the compiler notify me when a coroutine is not used. When I forget the co_await in front of a coroutine it simply does not do anything and I'm left wondering why something does not work. ever_called is a hack to alleviate this problem a bit, I would be quite happy to listen to a better solution. \$\endgroup\$
    – Pangi
    Commented Oct 22, 2023 at 20:56
  • \$\begingroup\$ And another thing, you said you had problems running the examples, could you tell me what where those problems? \$\endgroup\$
    – Pangi
    Commented Oct 22, 2023 at 21:00

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.