Remove Parsing of C++ Exception String #10637
Conversation
codereport
added
2 - In Progress
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## branch-22.08 #10637 +/- ##
===============================================
Coverage ? 86.31%
===============================================
Files ? 144
Lines ? 22751
Branches ? 0
===============================================
Hits ? 19637
Misses ? 3114
Partials ? 0 Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
| * Example usage: | ||
| * | ||
| * @code | ||
| * CUDF_EXPECTS(lhs->dtype == rhs->dtype, "Column type mismatch"); | ||
| * // throws cudf::logic_error | ||
| * CUDF_EXPECTS(lhs.type() != rhs.type(), "Column type mismatch"); |
There was a problem hiding this comment.
== ? (or else this is expecting lhs and rhs to having mismatching types so the error string is backwards?)
| * @param[in] _expection_type The exception type to throw; must inherit | ||
| * `std::exception`. If not specified (i.e. if only two macro |
There was a problem hiding this comment.
Let's add a static_assert for this while we're at it.
| #define GET_CUDF_FAIL_MACRO(_1, _2, NAME, ...) NAME | ||
| #define CUDF_FAIL_2(_what, _exception_type) \ | ||
| /*NOLINTNEXTLINE(bugprone-macro-parentheses)*/ \ | ||
| throw _exception_type{"cuDF failure at:" __FILE__ ":" CUDF_STRINGIFY(__LINE__) ": " _what}; |
There was a problem hiding this comment.
Maybe for DRY this should just call CUDF_EXPECTS(false, _exception_type, _what)?
| @@ -114,8 +114,14 @@ struct TypeList<Types<TYPES...>> { | |||
| exception); \ | |||
| } while (0) | |||
|
|
|||
| #define CUDF_EXPECT_THROW_MESSAGE(x, msg) \ | |||
| EXPECT_THROW_MESSAGE(x, cudf::logic_error, "cuDF failure at:", msg) | |||
| #define CUDF_EXPECT_THROW_MESSAGE(...) \ | |||
There was a problem hiding this comment.
I don't know when these test macros snuck in, but I recall a long time ago advocating strongly against them. I think it's fragile and unimportant for us to be testing for the exact contents of an exceptions error message.
Don't block this PR on removing these, but file it away in your memory banks for future work.
github-actions
bot
added
CMake
|
This PR has been labeled |
github-actions
bot
removed
CMake
|
This PR has been labeled |
This job resolves half of #10632 and a part of #10200.
Currently, in certain places in cuDF Python we parse the string messages returned from C++ std::exception::what. This should be removed and replaced by returning a more appropriate exception that Cython can automatically handle. If there is no "appropriate" exception, then a custom exception should be created and the Cython machinery should be written to handle it.