Hirrolot / datatype99

Algebraic data types for C99
MIT License
1.36k stars 23 forks source link

Some suggestions for improvements #10

Closed tylov closed 3 years ago

tylov commented 3 years ago

This project has great potentials. I have a few practical suggestion for use as an external library:

  1. Use the full '99' names as default, e.g. datatype99 match99, of99 e.g. to avoid name clashes and makes it clear its from the lib.
  2. The compilation is quite slow. Noticeable when using TinyC (https://repo.or.cz/tinycc.git) Use the mob branch. It could be because of all headers are included many times, and causes a lot of IO and file parsing. If this is the case, I suggest the lib only should be used by including metalang99.h, and make sure only one #include per file throughout.
  3. Error: You are using backticks ` somewhere which is not an allowed character. (Error reported by TinyC).
  4. The macro #define expr99(Expr, expr) ((Expr *)(Expr[]){expr}) could maybe be part of the lib.
  5. Consider renaming the project / keyword to sumtype99.

The ast.c example can be easier to read by using infix instead of prefix syntax:

// clang-format off
datatype99 ( Expr,
    (Const, double),
    (Add, Expr *, Expr *),
    (Sub, Expr *, Expr *),
    (Mul, Expr *, Expr *),
    (Div, Expr *, Expr *)
);
// clang-format on

double eval(const Expr *expr) {
    match99 (*expr) {
        of99 (Const, number) return *number;
        of99 (Add, lhs, rhs) return eval(*lhs) + eval(*rhs);
        of99 (Sub, lhs, rhs) return eval(*lhs) - eval(*rhs);
        of99 (Mul, lhs, rhs) return eval(*lhs) * eval(*rhs);
        of99 (Div, lhs, rhs) return eval(*lhs) / eval(*rhs);
    }
    return 0;
}

#define EX(lhs, op, rhs) op(expr99(Expr, lhs), expr99(Expr, rhs))

int main(void) {
    // 53 + (155 / 5) - 113
    Expr expr = EX(Const(53), Add, EX(EX(Const(155), Div, Const(5)), Sub, Const(113)));

    /*
     * Output:
     * -29.000000
     */
    printf("%f\n", eval(&expr));
}
Hirrolot commented 3 years ago

Thank you for your feedback!

1) You mean using the *99 macros in README.md or turn on DATATYPE99_NO_ALIASES by default? 4) I think expr99 is more a subject of such libraries as https://github.com/ramdeoshubham/macros than of Datatype99. 5) Indeed, I named this project Datatype99 because of the name of its main macro, datatype99 (which is, in turn, originated from Haskell), and what's more, I'm planning to add record types so this library would rather be described as "Algebraic data types for C99". 6) Nice catch, fixed.

I'll try Datatype99 on TinyC and answer the rest of questions bit later.

tylov commented 3 years ago

You're welcome. 1. Yes I meant e.g. users must define DATATYPE99_ALIASES if they want the short names.

Hirrolot commented 3 years ago

TinyCC seems not to work at all with Metalang99. I think this is the cause:

#define CALL(f, ...) f(__VA_ARGS__)
#define CONST()      123

// error: macro 'CONST' used with too many args
CALL(CONST, )

But CONST() works as expected. I've sent them a bug report.

tylov commented 3 years ago

Yes, that's a bug. It does not work with CALL(CONST) either. #define CONST(...) 123 is a workaround for now. Thanks for reporting.

Hirrolot commented 3 years ago

Datatype99 now must work on TCC. Compilation times on GCC/TCC are almost the same:

$ time gcc examples/binary_tree.c -Imetalang99/include -I. -ftrack-macro-expansion=0
real    0m0,120s
user    0m0,103s
sys 0m0,015s

$ time tcc examples/binary_tree.c -Imetalang99/include -I.
real    0m0,131s
user    0m0,122s
sys 0m0,000s

Regarding the 99 prefix: currently, it's not possible because Datatype99 is already 1.x.y according to semver, and the change is breaking.

Thanks again for reporting.