katef / kgt

BNF wrangling and railroad diagrams
BSD 2-Clause "Simplified" License
585 stars 30 forks source link

Stop using exit(EXIT_FAILURE) #38

Closed ArthurSonzogni closed 3 years ago

ArthurSonzogni commented 3 years ago

Hi Kate,

What do you think about this change I would like to make: I have some trouble with the uses of:

fprintf(stderr, "unimplemented\n");
exit(EXIT_FAILURE);

This causes the program to exit, which make it unusable when kgt is used as a library. Because it doesn't return to the caller.

Instead, I would like to transform the output_XXX functions to return the error to the caller (via a bool, an enum, or an output params). Instead of calling exit, we will unwind the stack properly. Callers would then be in a good position to decide what to do when KGT can't produce the output.

ArthurSonzogni commented 3 years ago

I gave it a try: https://github.com/ArthurSonzogni/kgt/blob/feature/pass-context/src/context.h

This does two things:

I realize this might sound a bit frightening. There is a lot of plumbing. Hopefully, once it's done, you won't have to do more if you need to extend struct context.

What do you think about the approach? Do you have any suggestion? What do you think should be done to solve this issue in general?

katef commented 3 years ago

Hm. Firstly I agree, that the exits there are lazy, and I only meant them as placeholders.

For a lot of these cases, my intention is to eventually support them where possible, usually by transformations on the grammar.

Meanwhile I'd think introducing struct context would be overkill. What do you think of returning to indicate failure in the usual C style (i.e. returning false from whichever functions)? kgt is missing a bunch of error handling. That really ought to be present anyway.

Then I'd set errno for unsupported things.

ArthurSonzogni commented 3 years ago

Returning false (+maybe setting errno) sounds reasonable. I think I can make some PR doing exactly this, this weekend, if you want.

Le ven. 27 nov. 2020 à 00:23, Katherine notifications@github.com a écrit :

Hm. Firstly I agree, that the exits there are lazy, and I only meant them as placeholders.

For a lot of these cases, my intention is to eventually support them where possible, usually by transformations on the grammar.

Meanwhile I'd think introducing struct context would be overkill. What do you think of returning to indicate failure in the usual C style (i.e. returning false from whichever functions)? kgt is missing a bunch of error handling. That really ought to be present anyway.

Then I'd set errno for unsupported things.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/katef/kgt/issues/38#issuecomment-734505834, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABEJ4QSVTJ6AS4CUFIP65DLSR3PQPANCNFSM4TNTELXQ .

katef commented 3 years ago

Oh that would be wonderful. Thank you so much!

ArthurSonzogni commented 3 years ago

I see this uses C89 standard, but booleans doesn't exist yet. Do you prefer switch to C99 or return enum values instead?

katef commented 3 years ago

Use int for now please, with 1/0 for true and false. I'll convert all my projects to C99 at some point.