Skip to content

Commit bb717e2

Browse files
committed
crimson/common/errorator: Always check exception type
We shouldn't bypass this check in the is_same_v<return_t, no_touch_error_marker> case. Signed-off-by: Matan Breizman <[email protected]>
1 parent be9d904 commit bb717e2

File tree

1 file changed

+19
-15
lines changed

1 file changed

+19
-15
lines changed

src/crimson/common/errorator.h

Lines changed: 19 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -381,6 +381,15 @@ class maybe_handle_error_t {
381381
static_assert(!std::is_same_v<return_t, void>,
382382
"error handlers mustn't return void");
383383

384+
// The code below checks for exact match only while
385+
// `catch` would allow to match against a base class as well.
386+
// However, this shouldn't be a big issue for `errorator` as
387+
// ErrorVisitorT are already checked for exhaustiveness at compile-time.
388+
// TODO: why/when is this possible?
389+
if (type_info != ErrorT::error_t::get_exception_ptr_type_info()) {
390+
return;
391+
}
392+
384393
auto ep = take_exception_from_future();
385394

386395
// Any assert_* handler we have:
@@ -404,21 +413,16 @@ class maybe_handle_error_t {
404413
// pointee's type with `__cxa_exception_type()` instead of costly
405414
// re-throwing (via `std::rethrow_exception()`) and matching with
406415
// `catch`. The limitation here is lack of support for hierarchies
407-
// of exceptions. The code below checks for exact match only while
408-
// `catch` would allow to match against a base class as well.
409-
// However, this shouldn't be a big issue for `errorator` as
410-
// ErrorVisitorT are already checked for exhaustiveness at compile-time.
411-
if (type_info == ErrorT::error_t::get_exception_ptr_type_info()) {
412-
413-
// TODO: add missing explanation
414-
if constexpr (std::is_assignable_v<decltype(result), return_t>) {
415-
result = std::invoke(std::forward<ErrorVisitorT>(errfunc),
416-
ErrorT::error_t::from_exception_ptr(std::move(ep)));
417-
} else {
418-
result = FuturatorT::invoke(
419-
std::forward<ErrorVisitorT>(errfunc),
420-
ErrorT::error_t::from_exception_ptr(std::move(ep)));
421-
}
416+
// of exceptions.
417+
418+
// TODO: add missing explanation
419+
if constexpr (std::is_assignable_v<decltype(result), return_t>) {
420+
result = std::invoke(std::forward<ErrorVisitorT>(errfunc),
421+
ErrorT::error_t::from_exception_ptr(std::move(ep)));
422+
} else {
423+
result = FuturatorT::invoke(
424+
std::forward<ErrorVisitorT>(errfunc),
425+
ErrorT::error_t::from_exception_ptr(std::move(ep)));
422426
}
423427
}
424428
}

0 commit comments

Comments
 (0)