dropbox / djinni

A tool for generating cross-language type declarations and interface bindings.
Apache License 2.0
2.88k stars 487 forks source link

support-lib: don't let a noexcept function throw exceptions #367

Closed libcg closed 6 years ago

libcg commented 6 years ago

Fixes the following compilation error after updating to NDK r17:

../deps/djinni/support-lib/jni/djinni_support.cpp:456:9: error: 'jniDefaultSetPendingFromCurrent' has a non-throwing exception specification but can still throw [-Werror,-Wexceptions]
        throw;
        ^
../deps/djinni/support-lib/jni/djinni_support.cpp:453:6: note: function declared non-throwing here
void jniDefaultSetPendingFromCurrent(JNIEnv * env, const char * /*ctx*/) noexcept {
     ^                                                                   ~~~~~~~~
artwyman commented 6 years ago

I've learned to be wary of catch (...) because it catches things you wouldn't think of as (C++) exceptions on some platforms. My experience was on Windows, where it catches "Windows Structured Exception Handling" exceptions (an old C thing built on LONGJMP), as well as debugger breakpoints.

Per the comment after the catch, the intent of the code here is that any exception which isn't a std::exception should just call terminate() when it hits the noexcept spec, which the language guarantees. That can probably be accomplished without the warning with another layer of indirection. I.e. put the code inside a function with no exception spec, then call it from the main function marked noexcept.

xianwen commented 6 years ago

Hi, @libcg: I noticed you haven't signed the CLA yet, could you please sign it here: https://opensource.dropbox.com/cla/ Thanks a lot!

4brunu commented 6 years ago

Now that https://github.com/dropbox/djinni/pull/378 is merged, I think that this PR may not be needed anymore, because I think booth PR address the same issue.

artwyman commented 6 years ago

Replaced by #378