Update exception handling to use std::exception_ptr
#1180
Conversation
The exception object can be passed as a pointer via `CxxException` error through Rust frames. A method to create the default message-based `rust::Error` and to drop and clone the exception are provided.
This represents C++ `std::exception_ptr` on the Rust side via `CxxException` object, which can be mentioned as a return type for functions creating custom exceptions or evaluating exceptions caught from C++ (added in further commits).
This makes the interface between C++ exception handling and Rust `Result` cleaner and allows passing C++ exception from the inner C++ call to the outer C++ call via Rust's `CxxException` unmodified, i.e., without losing information. To allow custom exception types, trait `ToCxxException` was introduced. With this trait, it's possible to convert a Rust error to the wrapper `CxxExeception` via a C++ function, thus allowing throwing other exceptions as well. This required changing `r#try` function into macros, so we can properly default to `rust::Error` for errors not having `ToCxxException` defined. Background: The `throw` statement in C++ (__cxa_throw) effectively first allocates space on the heap and creates the exception within, then starts unwinding. This can be also done via standard C++11 API in two steps. First, `std::make_exception_ptr()` creates a new `std::exception_ptr`, which points into this allocated space and internally is just a pointer (it's a smart pointer much like `std::shared_ptr`). Then, `std::rethrow_exception()` can be used to actually throw and/or rethrow this exception. Basically, the new implementation now uses `std::make_exception_ptr()` called from Rust to construct an exception for the `Result<_, E>` and then after returning it back to C++ via `CxxResult` (which is now BTW smaller, just 8B) the C++ part throws it using `std::rethrow_exception()`.
schreter
force-pushed
the
std_exception_ptr
branch
2 times, most recently
from
a4c2566
to
bb68c92
Compare
|
@dtolnay I seem unable to get the "ui" checks correct. With "my" nightly compiler (freshly updated), the output differs from what CI is expecting (mine also prints out the trait). Now, optically, there is no difference (except for $RUST replacing the path and such). I'll try to analyze further. Any tips getting this to work? E.g., do you use an explicit nightly version? Further, it seems like msvc compiler ABI is not yet updated to use exception_ptr via a simple pointer. I'll address that in a follow-up commit (I need to change the repr appropriately). BTW, how to add a reviewer? The "Reviewers" section to the right doesn't allow me to add any. Thanks. |
Since invalid `Result<T>` output changed, the expected file had to be updated. The output actually got better, since now the location of the actual declaration of the offending struct is also shown, not only the usage.
schreter
force-pushed
the
std_exception_ptr
branch
2 times, most recently
from
dfd6648
to
dc203b7
Compare
Contrary to the most platforms, which just store a single pointer in `std::exception_ptr`, MSVC stores two. Add necessary code to handle this.
schreter
force-pushed
the
std_exception_ptr
branch
from
dc203b7
to
b7bb463
Compare
Previously, modifying tests.h/cc would not trigger rebuild of the bridge, effectively preventing test development.
C++ error message returned by `what` must be NUL-terminated. However, the current copy function only copied the characters, but didn't add the NUL. Allocate one more byte and set it to NUL.
|
@dtolnay OK, I found the root cause of the The crate I'll disable the test for now, since it's not a problem in the implementation, but rather in the |
…ate. The normalization in `trybuild` crate doesn't work in the CI (but works fine locally).
|
I addressed the normalization issue in dtolnay/trybuild#223 and created a PR with the fix in dtolnay/trybuild#224. When that is through, the disabled test can be re-enabled. |
Instead, expand the match directly in `expand.rs` to minimize the public API.
schreter
force-pushed
the
std_exception_ptr
branch
from
24490ed
to
380b9ef
Compare
|
@dtolnay the change is now complete (modulo the one disabled Can you please review & comment and ideally merge? The PR is composed of three commits, which are touching individual aspects needed to implement the feature:
This is to make the reviewing easier. Other exception changes (like panic handling, transparent forwarding of C++ exceptions through Rust frames, etc.) depend on this. I'll publish them when this one is merged. Thanks. |
|
@dtolnay This PR sits here already since one month without any review (as are also the RFCs for further direction). Since most of the other "goodies" we developed internally and want to publish for everyone depend on it (such as panic and bad_alloc handling), it needs to go in first. It seems like you lost interest in development of Again, I don't want to publish a forked crate, I'd like to bring the changes upstream. Thanks. |
|
This is a really useful feature! A lot of C++ uses exceptions, and letting I noticed your patch adds a bunch of exception code not checking for the I really appreciate the work here, but I think it's important that we take one step forward without another back and continue to support the no-except case. I understand a lot of code still uses exceptions, so this approach is a useful feature for many users. But looking forward, I feel like any discussion about this particular feature is woefully incomplete without addressing the elephant in the room-- Best |
|
@riidefi Thanks for the feedback.
It may be lacking some, I'm not sure. I changed primarily the code which was already protected.
Right... It would be nice to support it as well, but it's not that simple. The error type must be then transferrable to C++ per value as well, which would quite limit us here. Aside from that, huge amount of existing code in C++ (including STL) unfortunately uses exceptions at many places. Even with Exception handling has one more advantage - the backtrace of the exception can be recorded in the exception and it's very helpful for diagnosis purposes. Doing the same with Rust's The Unfortunately, my patches to |
This makes the interface between C++ exception handling and Rust
Resultcleaner and allows passing C++ exception from the inner C++ call to the outer C++ call via Rust'sCxxExceptionunmodified, i.e., without losing information.To allow custom exception types, trait
ToCxxExceptionwas introduced. With this trait, it's possible to convert a Rust error to the wrapperCxxExeceptionvia a C++ function, thus allowing throwing other exceptions as well. This required changingr#tryfunction into macros, so we can properly default torust::Errorfor errors not havingToCxxExceptiondefined.Background: The
throwstatement in C++ (__cxa_throw) effectively first allocates space on the heap and creates the exception within, then starts unwinding. This can be also done via standard C++11 API in two steps. First,std::make_exception_ptr()creates a newstd::exception_ptr, which points into this allocated space and internally is just a pointer (it's a smart pointer much likestd::shared_ptr). Then,std::rethrow_exception()can be used to actually throw and/or rethrow this exception.Basically, the new implementation now uses
std::make_exception_ptr()called from Rust to construct an exception for theResult<_, E>and then after returning it back to C++ viaCxxResult(which is now BTW smaller, just 8B) the C++ part throws it usingstd::rethrow_exception().