Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update exception handling to use std::exception_ptr #1180

Open
wants to merge 11 commits into
base: master
Choose a base branch
from
Open
2 changes: 1 addition & 1 deletion Cargo.toml
Expand Up @@ -35,7 +35,7 @@ cxx-build = { version = "=1.0.91", path = "gen/build" }
cxx-gen = { version = "0.7", path = "gen/lib" }
cxx-test-suite = { version = "0", path = "tests/ffi" }
rustversion = "1.0"
trybuild = { version = "1.0.66", features = ["diff"] }
trybuild = { version = "1.0.78", features = ["diff"] }

[lib]
doc-scrape-examples = false
Expand Down
29 changes: 25 additions & 4 deletions book/src/binding/result.md
Expand Up @@ -25,7 +25,19 @@ the Rust side produces an error.
Note that the return type written inside of cxx::bridge must be written without
a second type parameter. Only the Ok type is specified for the purpose of the
FFI. The Rust *implementation* (outside of the bridge module) may pick any error
type as long as it has a std::fmt::Display impl.
type as long as it has a `std::fmt::Display` or `cxx:ToCxxException`
implementation.

Exception is built from the actual error type via `cxx::ToCxxException` trait
which converts the error type into a custom exception by the user code, if such
an implementation exists, else using `cxx::ToCxxExceptionDefault`, which only
requires the type to implement `std::fmt::Display` trait. The sole trait method
of both traits returns a `cxx::CxxException`, which wraps a `std::exception_ptr`
on the C++ side. An implementation of `cxx::ToCxxException` will call the
appropriate C++ function (again, via the bridge) to construct the
`std::exception_ptr`, likely using standard C++ function
`std::make_exception_ptr()` to wrap an exception. The signature on the C++ side
expects `std::exception_ptr` for `cxx::CxxException` on the Rust side.

```rust,noplayground
# use std::io;
Expand All @@ -51,9 +63,10 @@ fn fallible2() -> Result<(), io::Error> {
}
```

The exception that gets thrown by CXX on the C++ side is always of type
`rust::Error` and has the following C++ public API. The `what()` member function
gives the error message according to the Rust error's std::fmt::Display impl.
The exception that gets thrown by CXX on the C++ side is of type `rust::Error`
(unless otherwise specified by `cxx::ToCxxException` trait for a custom error
type) and has the following C++ public API. The `what()` member function gives
the error message according to the Rust error's `std::fmt::Display` implementation.

```cpp,hidelines
// rust/cxx.h
Expand Down Expand Up @@ -85,6 +98,12 @@ a second type parameter. Only the Ok type is specified for the purpose of the
FFI. The resulting error type created by CXX when an `extern "C++"` function
throws will always be of type **[`cxx::Exception`]**.

Note that this exception can be converted to [`cxx::CxxException`] using its
`Into` trait implementation and returned back to C++ later, as a `Result` with
error type `CxxException`, providing a transparent bridge from the original C++
exception thrown in a C++ callback through Rust API back to the C++ code calling
the Rust API without loss of information.

[`cxx::Exception`]: https://docs.rs/cxx/*/cxx/struct.Exception.html

```rust,noplayground
Expand Down Expand Up @@ -141,6 +160,8 @@ static void trycatch(Try &&func, Fail &&fail) noexcept try {
func();
} catch (const std::exception &e) {
fail(e.what());
} catch (...) {
fail("<no message>");
}
#
# } // namespace behavior
Expand Down
40 changes: 21 additions & 19 deletions gen/src/builtin.rs
Expand Up @@ -20,7 +20,7 @@ pub struct Builtins<'a> {
pub manually_drop: bool,
pub maybe_uninit: bool,
pub trycatch: bool,
pub ptr_len: bool,
pub repr_cxxresult: bool,
pub repr_fat: bool,
pub rust_str_new_unchecked: bool,
pub rust_str_repr: bool,
Expand Down Expand Up @@ -138,7 +138,7 @@ pub(super) fn write(out: &mut OutFile) {
}

if builtin.trycatch {
builtin.ptr_len = true;
builtin.repr_cxxresult = true;
}

out.begin_block(Block::Namespace("rust"));
Expand Down Expand Up @@ -217,13 +217,27 @@ pub(super) fn write(out: &mut OutFile) {
writeln!(out, "using Fat = ::std::array<::std::uintptr_t, 2>;");
}

if builtin.ptr_len {
if builtin.repr_cxxresult {
include.exception = true;
include.cstddef = true;
out.next_section();
writeln!(out, "struct PtrLen final {{");
writeln!(out, " void *ptr;");
writeln!(out, " ::std::size_t len;");
writeln!(out, "}};");
writeln!(out, "struct CxxException final {{");
writeln!(out, " void *repr_ptr;");
writeln!(out, "#if _MSC_VER >= 1700");
writeln!(out, " void *repr_ptr_2;");
writeln!(out, "#endif");
writeln!(out, "}};");
writeln!(out, "struct CxxResult final {{");
writeln!(out, " CxxException exc;");
writeln!(out, "}};");
writeln!(out, "struct Exception final {{");
writeln!(out, " CxxResult res;");
writeln!(out, " PtrLen msg;");
writeln!(out, "}};");
}

out.end_block(Block::Namespace("repr"));
Expand Down Expand Up @@ -258,11 +272,11 @@ pub(super) fn write(out: &mut OutFile) {
include.string = true;
out.next_section();
writeln!(out, "class Fail final {{");
writeln!(out, " ::rust::repr::PtrLen &throw$;");
writeln!(out, " ::rust::repr::Exception &throw$;");
writeln!(out, "public:");
writeln!(
out,
" Fail(::rust::repr::PtrLen &throw$) noexcept : throw$(throw$) {{}}",
" Fail(::rust::repr::Exception &throw$) noexcept : throw$(throw$) {{}}",
);
writeln!(out, " void operator()(char const *) noexcept;");
writeln!(out, " void operator()(std::string const &) noexcept;");
Expand Down Expand Up @@ -345,20 +359,6 @@ pub(super) fn write(out: &mut OutFile) {
writeln!(out, "}};");
}

if builtin.rust_error {
out.next_section();
writeln!(out, "template <>");
writeln!(out, "class impl<Error> final {{");
writeln!(out, "public:");
writeln!(out, " static Error error(repr::PtrLen repr) noexcept {{");
writeln!(out, " Error error;");
writeln!(out, " error.msg = static_cast<char const *>(repr.ptr);");
writeln!(out, " error.len = repr.len;");
writeln!(out, " return error;");
writeln!(out, " }}");
writeln!(out, "}};");
}

if builtin.destroy {
out.next_section();
writeln!(out, "template <typename T>");
Expand Down Expand Up @@ -414,6 +414,8 @@ pub(super) fn write(out: &mut OutFile) {
writeln!(out, " func();");
writeln!(out, "}} catch (::std::exception const &e) {{");
writeln!(out, " fail(e.what());");
writeln!(out, "}} catch (...) {{");
writeln!(out, " fail(\"<no message>\");");
writeln!(out, "}}");
out.end_block(Block::Namespace("behavior"));
}
Expand Down
32 changes: 20 additions & 12 deletions gen/src/write.rs
Expand Up @@ -211,7 +211,7 @@ fn pick_includes_and_builtins(out: &mut OutFile, apis: &[Api]) {
Some(Isize) => out.builtin.rust_isize = true,
Some(CxxString) => out.include.string = true,
Some(RustString) => out.builtin.rust_string = true,
Some(Bool) | Some(Char) | Some(F32) | Some(F64) | None => {}
Some(Bool) | Some(Char) | Some(F32) | Some(F64) | Some(CxxExceptionPtr) | None => {}
},
Type::RustBox(_) => out.builtin.rust_box = true,
Type::RustVec(_) => out.builtin.rust_vec = true,
Expand Down Expand Up @@ -706,8 +706,8 @@ fn write_cxx_function_shim<'a>(out: &mut OutFile<'a>, efn: &'a ExternFn) {
out.begin_block(Block::ExternC);
begin_function_definition(out);
if efn.throws {
out.builtin.ptr_len = true;
write!(out, "::rust::repr::PtrLen ");
out.builtin.repr_cxxresult = true;
write!(out, "::rust::repr::Exception ");
} else {
write_extern_return_type_space(out, &efn.ret);
}
Expand Down Expand Up @@ -783,9 +783,9 @@ fn write_cxx_function_shim<'a>(out: &mut OutFile<'a>, efn: &'a ExternFn) {
writeln!(out, ";");
write!(out, " ");
if efn.throws {
out.builtin.ptr_len = true;
out.builtin.repr_cxxresult = true;
out.builtin.trycatch = true;
writeln!(out, "::rust::repr::PtrLen throw$;");
writeln!(out, "::rust::repr::Exception throw$;");
writeln!(out, " ::rust::behavior::trycatch(");
writeln!(out, " [&] {{");
write!(out, " ");
Expand Down Expand Up @@ -856,7 +856,10 @@ fn write_cxx_function_shim<'a>(out: &mut OutFile<'a>, efn: &'a ExternFn) {
}
writeln!(out, ";");
if efn.throws {
writeln!(out, " throw$.ptr = nullptr;");
writeln!(out, " throw$.res.exc.repr_ptr = nullptr;");
#[cfg(target_env = "msvc")]
writeln!(out, " throw$.res.exc.repr_ptr_2 = nullptr;");
writeln!(out, " throw$.msg.ptr = nullptr;");
writeln!(out, " }},");
writeln!(out, " ::rust::detail::Fail(throw$));");
writeln!(out, " return throw$;");
Expand Down Expand Up @@ -899,8 +902,8 @@ fn write_rust_function_decl_impl(
) {
out.next_section();
if sig.throws {
out.builtin.ptr_len = true;
write!(out, "::rust::repr::PtrLen ");
out.builtin.repr_cxxresult = true;
write!(out, "::rust::repr::CxxResult ");
} else {
write_extern_return_type_space(out, &sig.ret);
}
Expand Down Expand Up @@ -1074,8 +1077,8 @@ fn write_rust_function_shim_impl(
}
}
if sig.throws {
out.builtin.ptr_len = true;
write!(out, "::rust::repr::PtrLen error$ = ");
out.builtin.repr_cxxresult = true;
write!(out, "::rust::repr::CxxResult error$ = ");
}
write!(out, "{}(", invoke);
let mut needs_comma = false;
Expand Down Expand Up @@ -1123,8 +1126,12 @@ fn write_rust_function_shim_impl(
writeln!(out, ";");
if sig.throws {
out.builtin.rust_error = true;
writeln!(out, " if (error$.ptr) {{");
writeln!(out, " throw ::rust::impl<::rust::Error>::error(error$);");
writeln!(out, " if (error$.exc.repr_ptr) {{");
writeln!(out, " void *ppexc = &error$.exc;");
writeln!(
out,
" std::rethrow_exception(std::move(*static_cast<std::exception_ptr*>(ppexc)));"
);
writeln!(out, " }}");
}
if indirect_return {
Expand Down Expand Up @@ -1323,6 +1330,7 @@ fn write_atom(out: &mut OutFile, atom: Atom) {
F64 => write!(out, "double"),
CxxString => write!(out, "::std::string"),
RustString => write!(out, "::rust::String"),
CxxExceptionPtr => write!(out, "::std::exception_ptr"),
}
}

Expand Down
43 changes: 33 additions & 10 deletions macro/src/expand.rs
Expand Up @@ -146,6 +146,7 @@ fn expand(ffi: Module, doc: Doc, attrs: OtherAttrs, apis: &[Api], types: &Types)
clippy::ptr_as_ptr,
clippy::upper_case_acronyms,
clippy::use_self,
clippy::items_after_statements,
)]
#vis #mod_token #ident #expanded
}
Expand Down Expand Up @@ -471,7 +472,7 @@ fn expand_cxx_function_decl(efn: &ExternFn, types: &Types) -> TokenStream {
});
let all_args = receiver.chain(args);
let ret = if efn.throws {
quote!(-> ::cxx::private::Result)
quote!(-> ::cxx::private::CxxResultWithMessage)
} else {
expand_extern_return_type(&efn.ret, types, true)
};
Expand Down Expand Up @@ -1108,7 +1109,15 @@ fn expand_rust_function_shim_impl(
None => quote_spanned!(span=> &mut ()),
};
requires_closure = true;
expr = quote_spanned!(span=> ::cxx::private::r#try(#out, #expr));
expr = quote_spanned!(span=>
match #expr {
Ok(ok) => {
::core::ptr::write(#out, ok);
::cxx::private::CxxResult::new()
}
Err(err) => ::cxx::private::CxxResult::from(err),
}
);
} else if indirect_return {
requires_closure = true;
expr = quote_spanned!(span=> ::cxx::core::ptr::write(__return, #expr));
Expand All @@ -1123,7 +1132,7 @@ fn expand_rust_function_shim_impl(
expr = quote_spanned!(span=> ::cxx::private::prevent_unwind(__fn, #closure));

let ret = if sig.throws {
quote!(-> ::cxx::private::Result)
quote!(-> ::cxx::private::CxxResult)
} else {
expand_extern_return_type(&sig.ret, types, false)
};
Expand Down Expand Up @@ -1166,16 +1175,12 @@ fn expand_rust_function_shim_super(
let args = sig.args.iter().map(|arg| quote!(#arg));
let all_args = receiver.chain(args);

let ret = if let Some((result, _langle, rangle)) = sig.throws_tokens {
let ret = if let Some((result, _langle, _rangle)) = &sig.throws_tokens {
let ok = match &sig.ret {
Some(ret) => quote!(#ret),
None => quote!(()),
};
// Set spans that result in the `Result<...>` written by the user being
// highlighted as the cause if their error type has no Display impl.
let result_begin = quote_spanned!(result.span=> ::cxx::core::result::Result<#ok, impl);
let result_end = quote_spanned!(rangle.span=> ::cxx::core::fmt::Display>);
quote!(-> #result_begin #result_end)
quote_spanned!(result.span=> -> ::cxx::core::result::Result<#ok, ::cxx::CxxException>)
} else {
expand_return_type(&sig.ret)
};
Expand All @@ -1192,9 +1197,23 @@ fn expand_rust_function_shim_super(
}
};

let call = if let Some((result, _langle, rangle)) = &sig.throws_tokens {
// Set spans that result in the `Result<...>` written by the user being
// highlighted as the cause if their error type is not convertible to
// CxxException (i.e., no `Display` trait by default).
let result_begin = quote_spanned! { result.span=>
|e| ::cxx::map_rust_error_to_cxx_exception!
};
let result_end = quote_spanned! { rangle.span=> (e) };
quote_spanned! {span=>
#call(#(#vars,)*).map_err( #result_begin #result_end )
}
} else {
quote_spanned! {span=> #call(#(#vars,)*) }
};
quote_spanned! {span=>
#unsafety fn #local_name #generics(#(#all_args,)*) #ret {
#call(#(#vars,)*)
#call
}
}
}
Expand Down Expand Up @@ -1751,6 +1770,10 @@ fn expand_extern_type(ty: &Type, types: &Types, proper: bool) -> TokenStream {
let span = ident.rust.span();
quote_spanned!(span=> ::cxx::private::RustString)
}
Type::Ident(ident) if ident.rust == CxxExceptionPtr => {
let span = ident.rust.span();
quote_spanned!(span=> ::cxx::CxxException)
}
Type::RustBox(ty) | Type::UniquePtr(ty) => {
let span = ty.name.span();
if proper && types.is_considered_improper_ctype(&ty.inner) {
Expand Down