C API exception handling

[pca006132]

Users can build manifold with exception enabled, and I expect distros to package manifold with exception enabled. Typical C bindings try to catch the exception before the exception crossing FFI boundary. Should we do the same here? And what should be the return value of various functions when we catch an exception?

@geoffder @NickUfer @cartesian-theatrics

[geoffder]

Non threadsafe and least breaking way: add a global unexposed integer pointer that is accessed and reset by an exposed check error function.

Safer way: user provides error int themselves at every call as the last argument, which isn't the end of the world since they can leave it off if they built without exceptions.

As far as returned values it depends on the function. Would we want to use null, then leave it up to the user to check the error value each time? Or empty / negative nonsense values (user still has to check anyway, but then no null deref when they screw up).

[pca006132]

I was thinking about using a thread local global integer pointer for error number, but for languages with green thread, things may go wrong when they migrate to another OS thread.

I prefer adding an error int pointer argument to the end of every call (that can error). If they don't want it, they can just supply a nullptr.

For return value, maybe empty/negative nonsense? It matches our API behavior when exception is disabled, e.g. boolean failure will result in an empty manifold object with status message.

[pca006132]

@harmanpa I remember you are also maintaining some java binding?

[elalish]

This sounds like a big change - is this a breaking change we need to deal with for v3.0?

[pca006132]

Yeah, I think if C binding users are fine with adding a error num pointer argument, we should probably add it to our 3.0 release

[harmanpa]

@pca006132 Yes we have Java bindings, I'd be happy with an error int, we'd then use that to determine a type of Java exception to throw.

[elalish]

@pca006132 when we were discussing this I thought we no longer had a MANIFOLD_EXCEPTIONS flag and that it was all rolled up into MANIFOLD_DEBUG, but I think I must have just been looking at our Readme, which appears to be wrong. Since we do have these two flags separately, what do you think? Are folks using the C bindings really going to want to build with exceptions? Or should we just disallow that and skip this?

[pca006132]

I think we should just ask the users if they will be using the system provided package. If they are using that, I don't think we can ask distros to build with exception disabled (considering it is not the default, and not something like parallelization that is incredibly useful).

[pca006132]

@geoffder @harmanpa @NickUfer @cartesian-theatrics @james-bern

[NickUfer]

I'm fine with any change. I currently paused further development of the rust bindings until we are sure what the API changes will be, so I can avoid some extra work 😄 There is also nothing to break as the first fully implemented version wasn't released yet.

[elalish]

Ah, fair enough, I wasn't thinking about distros. SG.

[cartesian-theatrics]

Don't have too much of an opinion here, but java bindings handle c++ exceptions seamlessly (they turn into java exceptions). I haven't had any issues compiling with exceptions. I don't favor explicit parameterization of errors. If I was going to not use exceptions, I'd rather just use the existing status enum on Manifolds for custom exception handling--i.e. we just have some empty Manifold that always transforms to itself.

[james-bern]

@geoffder @harmanpa @NickUfer @cartesian-theatrics @james-bern

NOTE: I'm definitely a bit out of my depth here.

I am using the C bindings and have no plans to use exceptions.

Something like this

Non threadsafe and least breaking way: add a global unexposed integer pointer that is accessed and reset by an exposed check error function.

sounds good to me (similar to glGetError if I understand correctly). In my project, I currently only call Manifold functions and use Manifold types inside of one or two functions. The other 99% of my code doesn't know Manifold exists.

[pca006132]

Can't say for the other users, but I think rust binding will be fine with having manifold as submodule and use build.rs to build it as a static library (with exception disabled), so it doesn't have to care about the system package. I think C API users don't care much about exceptions, or are fine with exception (Java converts c++ exception into Java exceptions). I think maybe we can just leave it as is for now?

I think the C binding is not as important as the other APIs because users can always resort to building their own C binding in the worst case, and it is not clear to me which design is the best (e.g. implicit vs explicit error num). And I'm thinking about removing the user provided buffer, I feel like it is making our binding more complicated without much benefit (we still have a C/C++ managed heap anyway). So I think designs regarding the C API should not be a release blocker.

[geoffder]

For safety reasons I do think the only pointer way is best (on calls that could lead to exceptions coming from the stdlib).

I've given my thoughts on the buffer before hand but I personally think it should stay since the idea with the bindings was making them accessible to FFI which might work better in some languages when the buffers begin life as managed, rather than getting registered afterward.

[pca006132]

Should probably open a new thread if we want to discuss this further. I think storing just a pointer in the buffer should also work, and this is effectively what we have now for things like vector: it is a fixed-size struct to some C++ managed heap.

[pca006132]

If we do #1041, we can close this because there should be no exceptions that C API users have to handle.