Skip to content

Commit

Permalink
Implement ReadableNative*::mapException without dynamic_cast
Browse files Browse the repository at this point in the history
Summary:
Changing fbjni's mapException interface to use `std::exception_ptr`, this enables us to use `std::rethrow_exception` and `catch` statements to check for specific exception types, rather than using dynamic_casts and having an explicit dependency on RTTI.

While generally, we should always be enabling RTTI and exception support simultaneously, we have a number of targets where we don't, or where avoiding RTTI provides significant binary size gains.

Changelog: [Internal]

Reviewed By: mhorowitz

Differential Revision: D34379950

fbshipit-source-id: 446d105f9fb50ada3526f242f98e163215abd0ab
  • Loading branch information
javache authored and facebook-github-bot committed Feb 25, 2022
1 parent 0464ece commit 2554f9b
Show file tree
Hide file tree
Showing 3 changed files with 13 additions and 12 deletions.
14 changes: 6 additions & 8 deletions cxx/fbjni/detail/Hybrid.h
Original file line number Diff line number Diff line change
Expand Up @@ -258,14 +258,12 @@ class HybridClass : public detail::HybridTraits<Base>::CxxBase {
return JavaPart::newInstance(std::move(args)...);
}

// If a hybrid class throws an exception which derives from
// std::exception, it will be passed to mapException on the hybrid
// class, or nearest ancestor. This allows boilerplate exception
// translation code (for example, calling throwNewJavaException on a
// particular java class) to be hoisted to a common function. If
// mapException returns, then the std::exception will be translated
// to Java.
static void mapException(const std::exception& ex) {
// If a hybrid class throws an exception, it will be passed to mapException
// on the hybrid class, or nearest ancestor. This allows boilerplate exception
// translation code (for example, calling throwNewJavaException on a particular
// java class) to be hoisted to a common function. If mapException returns,
// then the std::exception will be translated to Java.
static void mapException(std::exception_ptr ex) {
(void)ex;
}
};
Expand Down
4 changes: 2 additions & 2 deletions cxx/fbjni/detail/Registration-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -119,8 +119,8 @@ struct MethodWrapper {
// this will get the right type for the registered method.
auto cobj = static_cast<C*>(ref->cthis());
return (cobj->*method)(std::forward<Args>(args)...);
} catch (const std::exception& ex) {
C::mapException(ex);
} catch (...) {
C::mapException(std::current_exception());
throw;
}
}
Expand Down
7 changes: 5 additions & 2 deletions test/jni/hybrid_tests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -123,8 +123,11 @@ class TestHybridClass : public facebook::jni::HybridClass<TestHybridClass> {
return newObjectCxxArgs(3, "three", true);
}

static void mapException(const std::exception& ex) {
if (dynamic_cast<const TestException*>(&ex) != 0) {
static void mapException(std::exception_ptr ex) {
try {
std::rethrow_exception(ex);
}
catch (const TestException &ex) {
throwNewJavaException("java/lang/ArrayStoreException", "");
}
}
Expand Down

0 comments on commit 2554f9b

Please sign in to comment.