Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Some suggestions for improvements #10

Closed
tylov opened this issue Apr 24, 2021 · 5 comments
Closed

Some suggestions for improvements #10

tylov opened this issue Apr 24, 2021 · 5 comments

Comments

@tylov
Copy link

tylov commented Apr 24, 2021

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 added a commit that referenced this issue Apr 25, 2021
@hirrolot
Copy link
Owner

Thank you for your feedback!

  1. You mean using the *99 macros in README.md or turn on DATATYPE99_NO_ALIASES by default?
  2. I think expr99 is more a subject of such libraries as https://github.com/ramdeoshubham/macros than of Datatype99.
  3. 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".
  4. Nice catch, fixed.

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

@tylov
Copy link
Author

tylov commented Apr 25, 2021

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

@hirrolot
Copy link
Owner

hirrolot commented May 2, 2021

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
Copy link
Author

tylov commented May 2, 2021

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
Copy link
Owner

hirrolot commented May 3, 2021

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.

@hirrolot hirrolot closed this as completed May 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants