Add option for inkcpp to compile without exceptions

[harryr0se]

Hello!

Certain devices like the Playdate console don't have support for exceptions, currently in the code the path to disable exceptions is mixed into other defines.

This PR separates that out into its own INKCPP_NO_EXCEPTIONS define with a path that prints to stderr and then calls abort()

[JBenda]

Thanks for the input. The idea with the printf and abort, I do quit, like.

In this branch
If it is ok for you, I would merge this branch on top of your PR, since they edit a similar topic.

One question: Do you think INKCPP_NO_EH is different from INKCPP_NO_EXCEPTION?
Maybe we can make an option with multiple values?
Like
INKCPP_EH = (NO_EXCEPTION, NO_CATCH, ON)?
@willvale, do you also agree, or was your intent with INKCPP_NO_EH different?

[harryr0se]

Heya :)

One question: Do you think INKCPP_NO_EH is different from INKCPP_NO_EXCEPTION?

Ah yes, I didn't actually notice that define before, I'm guessing "EH" stands for "exception handling?
Those should definitely be combined IMO, I've just pushed that to the branch let me know if you want them combined in a different way (I think we can get by on this just being an on/off setting)

One thing that could also be nice would be INKCPP_NO_EXCEPTION also setting the relevant compiler flags

If it is ok for you, I would merge this branch on top of your PR, since they edit a similar topic.

I'm happy for you to combine the branches into one PR, they seem like a good fit together

[harryr0se]

Was also wondering if we could get some of these set up in CI to make sure that any future code that uses exceptions or RTTI is wrapped correctly?

[JBenda]

So rebase the other branch on yours, so it should be easy to merge (ff).

Correct, EH = error handling

The branch of mine also disables EH and RTTI for the clib per default (since it is not a C feature ...)

A) One question from my side:
abort() and fprintf are CSTD/STL functionality.
We should also wrap this in the corresponding defines ...

B) The compile flags auto-setting sounds good; everything you can not screw up is a good thing.

C) Because of the CI: my first instinct would be to build a no EH/RTTI flavor and run all tests against it
For this we would need to:

• C.1) Disable all tests that explicitly test EH
• C.2) Ensure that the resulting binary really does not use EH/RTTI ...
  Or did you think of a different kind of test?

[harryr0se]

So rebase the other branch on yours, so it should be easy to merge (ff).

If you don't mind, would you be able to do the merging/rebase? I had a look and there's quite a few changes there that I feel you'll have the most context on. Alternatively we could land this and you then rebase

A) One question from my side: abort() and fprintf are CSTD/STL functionality. We should also wrap this in the corresponding defines ...

Ah yes, I've wrapped them with INK_ENABLE_CSTD which I believe is correct. One thing I really want is to make sure we always have a path that logs in some way otherwise we'd get silent fatal errors

B) The compile flags auto-setting sounds good; everything you can not screw up is a good thing.

So I gave this a go, unfortunately it seems like the compiler options then get pulled into the inkcpp_cl compiler which has quite a few uses of exceptions. For my use case I'm only compiling the runtime, inkcpp and inkcpp_o without exceptions

In general I think it makes sense for the runtime to have this option but not the compiler, do you feel comfortable with that?

C) Because of the CI: my first instinct would be to build a no EH/RTTI flavor and run all tests against it For this we would need to:

I think that would be a valid test, as a starting point, personally I'd just be happy with a CI job that simply checks that the inkcpp and inkcpp_o targets compile successfully without exceptions and RTTI

[willvale]

EH in my original #128 was for EXCEPTION_HANDLING - INKCPP_NO_EXCEPTION is a better name.

I agree some kind of error output is important, I'd prefer if it was made available to the caller though, via e.g. getLastError(), vs. calling abort(). Although that would be more relevant for testing/tooling than production code.

NB: It would also be good to be able to build a release version of inkcpp, without the inkAsserts - suggest adding INKCPP_NO_DEBUG for that?

[JBenda]

I think an INKCPP_NO_DEBUG is misleading (I know that is how C assert handles it), since whether you want your application to stop on error or continue as far as possible depends on the use case.

If I see it correctly, we have the following error-handling strategies:

1. C++ exception, requires C++
2. C fprintf(stderr) + abort(), requires C STD libs
3. Store the error and make it available with, e.g., getLastError() requires nothing, could lead to crashes (like nullptr)
4. Skip InkAsserts, requires nothing

So maybe INKCPP_EXCEPTION_HANDLING=(CPP_EXCEPTION|C_ABORT|CHECK_ONLY|NONE)?

[harryr0se]

Thanks for the discussion!

My personal preference is generally to propagate errors up to the caller when something goes wrong, either via the particular functions return type or via a getLastError() api rather than exceptions. As mentioned one of the benefits is portability and avoiding these CSTD / STL issues.
But that's obviously a bigger change and I respect that the code isn't currently set up that way

I feel like a mode which avoids the abort is a bit risky given that currently inkcpp treats asserts both as a way of error logging and bailing out of the code path

So maybe INKCPP_EXCEPTION_HANDLING=(CPP_EXCEPTION|C_ABORT|CHECK_ONLY|NONE)?

That seems to cover the options discussed in this thread, is that something you'd like to handle in this PR or would you be happy for it to be a follow up?

I'm not sure I'm going to be able to dedicate much time to this for a bit
If you'd like to take some time to plan a bigger bit of work on error handling then we could close this and create an issue?
It would be nice to land this basic NO_EXCEPTION support for my playdate project but let me know how you want to proceed

[JBenda]

Thanks for the feedback.
A new issue+PR would be the best. Especially since getLastError will require an extension of the API. And we can start implementing the required test cases.

Is the current state sufficient for you? If yes I would merge this PR soon.

[harryr0se]

Is the current state sufficient for you? If yes I would merge this PR soon.

Yup, this fits my needs for the moment. Thank you!