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

Feature Request: Exception Support #1412

Open
torrancew opened this issue Dec 17, 2024 · 7 comments · May be fixed by #1426
Open

Feature Request: Exception Support #1412

torrancew opened this issue Dec 17, 2024 · 7 comments · May be fixed by #1426

Comments

@torrancew
Copy link

First off, thank you for the excellent tooling you provide! I really appreciate autocxx, and it made it much easier to release torrancew/xapian-rs.

In the process of writing the bindings, I ran into a few pain points, but the most difficult one was the lack of support for exceptions in autocxx. Upstream Xapian code throws quite a bit, and it would be great to express most of those in the Rust bindings without resorting to writing almost everything by hand.

Would you be open to receiving patches for this? If so, could you provide some high level guidance on how best to start? I'm fairly comfortable in Rust, but my C++ knowledge is... dated. I know enough to know that I don't know much, but I can usually get something that compiles thrown together -- but I'm willing to iterate if you're open to the patches and some potentially naive lines of questioning.

Thanks again!

@adetaylor
Copy link
Collaborator

Thanks for getting in touch.

I can see that this would indeed be pretty painful.

autocxx tries really hard to stick to the basic interop model of cxx, and exception handling in cxx works like this.

So, to make this work in autocxx, we'd need to:

  • Figure out a way to identify which set of C++ functions might throw (please propose a method)
  • Modify the bindings which we pass to cxx such that the return type is wrapped in Result<>

This sounds relatively straightforward so yes, I think I'd be open to patches to do this.

@ttys3
Copy link

ttys3 commented Jan 10, 2025

I have been waiting for this feature for years.

@ttys3
Copy link

ttys3 commented Jan 10, 2025

@torrancew When I needed this feature, I created xapian-rs, which is based on cxx, and later tried autocxx. The code is now in this branch: https://github.com/ttys3/xapian-rs/tree/autocxx. Autocxx is convenient and can automatically generate documentation, but it has a limitation: it doesn't support exceptions, while the cpp code of xapian does.

@torrancew
Copy link
Author

@ttys3 Thanks! Sadly, I'm all too aware -- I have a couple of plain cxx-based xapian bindings laying around, as well. Xapian in particular has a lot of pain points with the cxx limitations (and some with autocxx too), but overall I found that I was able to build a much more complete set of bindings more quickly and with less C++ shim code via autocxx. That let me get to a point where my bindings worked and were usable for less trivial examples, and now I'm starting to evaluate whether it's worth migrating to manual bindings or patching autocxx to close the gaps, and leaning towards patching autocxx. I'd be happy to take contributions on my xapian bindings if you're ever interested!

@adetaylor Thanks so much! I've got some familiarity with how cxx handles exceptions. I've been thinking over the syntax issue for a bit now. So far, the "best" idea I've got is to add a new macro throws! to be used alongside existing macros like block!. It's not really my ideal, but basing it off of noexcept is a non-starter for several reasons, and I haven't come up with anything more elegant thus far.

Some other questions I've found myself asking along the way:

  • How should Exceptions be represented in Rust? This can turn into a can of worms if you let it, but obviously we want to pass some sort of context about what exceptions were encountered back in the Result (I've mostly settled for string stuffing in the past)
  • How should the cxx exception handler be generated (intertwined with the representation question)? How much flexibility should we try to give to the implementor's codebase?

@adetaylor
Copy link
Collaborator

So, autocxx fundamentally works a bit like this:

  1. Run bindgen to generate bindgen-like bindings
  2. Post-process them to generate input to cxx
  3. Feed that into cxx

So, I think the answers to your questions are that the exception handling will be literally that which is already in cxx. It's "just" a matter of giving cxx the right information.

Re throws! yeah I agree, it seems least bad for now. I agree noexcept is no good.

@torrancew
Copy link
Author

@adetaylor Thanks for the guidance.

I've got a branch started, and have had some minor success (working support for wrapped primitives/POD, at least!), hit some snags, and surfaced a couple of questions worth exploring further. I'll send a draft PR over to start working through that stuff. A couple of things I've struggled with in particular:

  • Name canonicalization -- I imagine a user specifying throws!("Klass::method"), but the parts of the engine where function return type conversion happen seem to have a post-bindgen view of the world
  • Handling nuanced return types (UniquePtr<T>/impl moveit::New)
  • "Properly" wiring up Result-wrapping in the current type conversion code -- is it a "cpp_conversion"? a "rust_conversion"? something in between?

@torrancew torrancew linked a pull request Jan 20, 2025 that will close this issue
3 tasks
@adetaylor
Copy link
Collaborator

  • Name canonicalization -- I imagine a user specifying throws!("Klass::method"), but the parts of the engine where function return type conversion happen seem to have a post-bindgen view of the world

Yes. Name handling is one of the messiest parts.

  • Handling nuanced return types (UniquePtr<T>/impl moveit::New)
  • "Properly" wiring up Result-wrapping in the current type conversion code -- is it a "cpp_conversion"? a "rust_conversion"? something in between?

I'd suggest starting as simply as possible, and allowing throws! only for functions where no other conversion occurs. You can expand it later if needs be. Hopefully, for such functions, it's a matter of simply amending the signature that gets generated in codegen_rs for the cxx::bridge mod, and hopefully no other changes are needed.

Obviously, good diagnostics would be needed if throws! was unable to take action because it turned out that the function was in some way more complex than this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants