rustls / rustls-ffi

Use Rustls from any language
Other
125 stars 31 forks source link

dtolnay/cxx? #69

Open sayrer opened 3 years ago

sayrer commented 3 years ago

I searched for an evaluation of https://github.com/dtolnay/cxx, but didn't find one. I've had really good luck with it, and it might work well in this project alongside a C header.

sayrer commented 3 years ago

Thinking more about this... I believe the goals of this project would be better served by a reproducible Bazel build and dtolnay/cxx as the developer workflow. A Makefile and pkg-config stuff is fine to maintain for consumers.

jsha commented 3 years ago

I'll take a look! It looks like this would replace cbindgen; is that accurate? Is the goal to offer richer support for C++, or to provide a better developer experience for people working on crustls?

sayrer commented 3 years ago

The goal would be a better experience for crustls developers and safer code. A nice C++ API would just be a side-effect.

The cxx crate will let you avoid implementing stuff like this: https://github.com/abetterinternet/crustls/blob/main/src/rslice.rs#L128

There is a comparison to bindgen/cbindgen here: https://github.com/dtolnay/cxx#comparison-vs-bindgen-and-cbindgen

jsha commented 3 years ago

You mention that we could avoid implementing rustls_str and rustls_slice_bytes. I see that cxx offers a C++ rust::Str https://cxx.rs/binding/str.html, but does it also offer a C equivalent? Do you know any examples of C APIs produced with cxx that I could look at for comparison?

sayrer commented 3 years ago

It does not. I think you would have to implement a C API for rust::Str with these methods it provides:

  // Note: no null terminator.
  const char *data() const noexcept;
  size_t size() const noexcept;
  size_t length() const noexcept;

But you would still get cxx's safety guarantees across the FFI boundary, which I have found to be very valuable.

jsha commented 3 years ago

Currently rustls_str is:

typedef struct rustls_str {
  const char *data;
  size_t len;
} rustls_str;

I think asking C code to go through accessor methods like rustls_str_data(rstr) and rustls_str_len(rstr) would be harder to use without providing a corresponding increase in safety.

But you would still get cxx's safety guarantees across the FFI boundary, which I have found to be very valuable.

This sounds great! I'm afraid I can't figure out from the documentation what exactly those safety guarantees are. Am I right in understanding that they apply only in C++, like offering no mutators on rustls::Str?

If I implement the C interface for crustls by way of a C++ interface, does that mean that C environments need a C++ toolchain as well as a C one? Is there overhead in the built library from the C++ library?

The first two projects integrating crustls (curl and Apache) are both C, so I've been prioritizing a good C API. I'm interested in C++ too if there's a project that would benefit. But if adding cxx into the mix means more complexity and doesn't offer better safety for the C API, I'm inclined to wait until there's a C++ project on the horizon that wants C++ APIs.

sayrer commented 3 years ago

This sounds great! I'm afraid I can't figure out from the documentation what exactly those safety guarantees are.

Roughly: the project generates both sides of the FFI boundary in the same Rust macro. This approach statically prevents many kinds of bugs.

If I implement the C interface for crustls by way of a C++ interface, does that mean that C environments need a C++ toolchain as well as a C one?

Yes, but I'm not aware of any common toolchain that doesn't implement C++. I'd suggest dropping anything that doesn't when building crustls itself--the project can still be used with C compilers after the library is created, and Rust requires LLVM anyway.

Unfortunately, I think the best way to evaluate this choice might be to compare a cxx version with the current approach. That could mean wasted effort, but it's really difficult to evaluate this trade-off in prose.

sayrer commented 3 years ago

Is there overhead in the built library from the C++ library?

Missed this question. The overhead should be zero or negligible, so the choice is more about ergonomics and safety.

icing commented 3 years ago

[Currently rustls_str is:] Currently rustls_str is:

typedef struct rustls_str {
  const char *data;
  size_t len;
} rustls_str;

I think asking C code to go through accessor methods like rustls_str_data(rstr) and rustls_str_len(rstr) would be harder to use without providing a corresponding increase in safety.

Apart from my preference for exposing a struct for such data, I think bringing the C++ dependencies into a C bridge like crustls must be carefully weighted against possible complications regarding mangling, runtime, memory footprints, release complexity and other such things. The examples shown on the project drag in the C++ std library functions for all kind of things.

In case of problems (and this all is about problems), one would need to debug a bunch of generated C++ sources and that alone would make me personally want to avoid that at all costs.

On the Rust side, there would also be a price to pay. One of the services of a C API like crustls is to define a stable interface. Lifetimes in the C world are much longer than Rust crates maybe used to. If you auto-generate the API, any change in rustls needs to be done very carefully or ripple though the API in incompatible ways.

Sooner or later, one has to write code to provide backward compatible classes and methods in C++ that somehow sit between and may also need adjustment on any future release of the bridge toolchain as that does some internal reshuffling/renaming which "does not matter to the outside, really".

All past experience from the last 30 years of auto-generated interfaces for other languages make me want to run from this. ;-)

sayrer commented 3 years ago

Well, https://cxx.rs has a pretty good description of the bridge. The resulting C++ code really isn't very complex imho. Here's one I did for twitter-text.rs: https://github.com/sayrer/twitter-text/blob/cxx/rust_bindings/cpp/twitter.h Pretty boring C++ that could easily be wrapped for C.

Here's what the generated header looks like: https://gist.github.com/sayrer/d084cd5d5c989fc9f0e93c15cdc32de5. This is the only generated file, and the macro is pretty good at cutting out features that aren't used in cxx.h/cxx.cc.

See also https://cxx.rs/context.html, which discusses the trade-off here. (The book is new, I haven't read it yet)

sayrer commented 3 years ago

If you auto-generate the API, any change in rustls needs to be done very carefully or ripple though the API in incompatible ways.

fwiw, this is a really nice part of cxx. You can use a rust::Box or unique_ptr, depending on what you want to do, and it will call drop() on the Rust side when appropriate.

lu-zero commented 3 years ago

A plain C API does not carry the whole C++ runtime with it. Please leave the option to have plain C api with no additional dependencies.