1830486 - Add some Rust for DOM

Status: NEW

(Core :: DOM: Core & HTML, task)

Description

[Kagami Rosylight [:saschanaz] (they/them)]

For now this is nothing really, I just wanted to put my PoC patch somewhere.

Comment 1

[Kagami Rosylight [:saschanaz] (they/them)]

Attachment: WIP: Bug 1830486 - Part 1: Add `cxx` crate dependency

Comment 2

[Kagami Rosylight [:saschanaz] (they/them)]

Attachment: WIP: Bug 1830486 - Part 2: Add TestInterfaceRust

Depends on D176757

Comment 3

[Kagami Rosylight [:saschanaz] (they/them)]

Attachment: WIP: Bug 1830486 - Part 3: Add string type test

Depends on D176776

Comment 4

[Emilio Cobos Álvarez (:emilio)]

Curious, why using cxx instead of the other rust binding generators? This doesn't seem too different to what l10n code does with cbindgen.

Comment 5

[Kagami Rosylight [:saschanaz] (they/them)]

My understanding with cbindgen is that it always requires separate extern "C" functions and the C++ counterpart needs to wrap it with a separate class for better usage. cxx does the wrapper part for us, so it should ideally be possible to write a whole class in Rust without any handwritten C++ wrapper class.

That said, for now I don't see a way to actually skip the wrapper because of nsWrapperCache and cycle collection.

Comment 6

[Kagami Rosylight [:saschanaz] (they/them)]

the other rust binding generators?

BTW, this is interestingly plural. Any better suggestion instead of cxx and cbindgen?

Comment 7

[Kagami Rosylight [:saschanaz] (they/them)]

It could be interesting to migrate other modules to cxx and remove the wrappers, btw.

Comment 8

[Emilio Cobos Álvarez (:emilio)]

(In reply to Kagami [:saschanaz] from comment #5)

My understanding with cbindgen is that it always requires separate extern "C" functions and the C++ counterpart needs to wrap it with a separate class for better usage.

FWIW you don't need to make it a separate class, cbindgen supports methods like:

impl Foo {
    #[no_mangle]
    #[export_name = "foo_bar"]
    pub extern "C" fn bar(&self) -> u32 { ... }

You could even use a #[webidl] proc macro for all the boilerplate I guess.

cxx does the wrapper part for us, so it should ideally be possible to write a whole class in Rust without any handwritten C++ wrapper class.

In the past when I looked at cxx it was great at ergonomics, but not so great at performance. e.g., string passing always required copying and allocating, while with cbindgen you can use UTF8String on WebIDL, and &nsAString on the rust side to read the utf-8 back without any extra copy...

cbindgen could be taught about some macros btw (it already knows about bitflags for example), so that you could have a #[webidl] macro that generated all the boilerplate. I guess that'd be somewhat similar to what you're doing.

(In reply to Kagami [:saschanaz] from comment #6)

BTW, this is interestingly plural. Any better suggestion instead of cxx and cbindgen?

You could potentially do it the other way around with bindgen (so, generate bindings for some C++ bits that you call from some rust proc macro or so). Not saying it's necessarily better than what you're doing, but I guess xpcom-rs is doing something along those lines (though with manual bindings rather than bindgen).

We also have the uniffi thing that has some amount of support for WebIDL if I understand it right? See bug 1766045.

In general, I just want to avoid adding yet another Rust-to-C++ bindings mechanism if we can...

Comment 9

[Kagami Rosylight [:saschanaz] (they/them)]

(In reply to Emilio Cobos Álvarez (:emilio) from comment #8)

FWIW you don't need to make it a separate class, cbindgen supports methods like:

I'm not sure I follow this one, because our IDL bindings layer still expects a class and foo_bar should be wrapped as Foo::Bar for that, am I misunderstanding?

You could even use a #[webidl] proc macro for all the boilerplate I guess.

Indeed, but maintaining it is a burden and here I wanted to use existing things as much as possible. That doesn't mean it's not an option, sure.

In the past when I looked at cxx it was great at ergonomics, but not so great at performance. e.g., string passing always required copying and allocating, while with cbindgen you can use UTF8String on WebIDL, and &nsAString on the rust side to read the utf-8 back without any extra copy...

https://phabricator.services.mozilla.com/D176786 already uses &nsAString with cxx, so I guess that's not a blocker?

cbindgen could be taught about some macros btw (it already knows about bitflags for example), so that you could have a #[webidl] macro that generated all the boilerplate. I guess that'd be somewhat similar to what you're doing.

Could you provide a relevant link? 👀

You could potentially do it the other way around with bindgen (so, generate bindings for some C++ bits that you call from some rust proc macro or so). Not saying it's necessarily better than what you're doing, but I guess xpcom-rs is doing something along those lines (though with manual bindings rather than bindgen).

Looks like that's what Google is doing in https://github.com/google/autocxx, although I haven't tried this one. Anyway, the purpose here is to implement Web IDL interface in Rust so that the generated C++ bindings code can call it, and I don't see how can that be done in the-other-way-around way. Could you elaborate?

We also have the uniffi thing that has some amount of support for WebIDL if I understand it right? See bug 1766045.

My impression was that they were migrating away from Web IDL, but it would be interesting if we could reuse some of that here!

In general, I just want to avoid adding yet another Rust-to-C++ bindings mechanism if we can...

Yeah, I'd like to reduce the complexity as much as possible.

Comment 10

[Emilio Cobos Álvarez (:emilio)]

(In reply to Kagami [:saschanaz] from comment #9)

I'm not sure I follow this one, because our IDL bindings layer still expects a class and foo_bar should be wrapped as Foo::Bar for that, am I misunderstanding?

Oh I see. I misunderstood your point. FWIW cbindgen can totally generate methods, it just doesn't by default. See this for some convenience methods we have in a variety of classes. Though I guess it'd need some plumbing so that they don't need to be written by hand.

You could even use a #[webidl] proc macro for all the boilerplate I guess.

Indeed, but maintaining it is a burden and here I wanted to use existing things as much as possible. That doesn't mean it's not an option, sure.

If we eventually have some CC integration we probably need that anyways to derive cycle collection tho, right? We ideally don't want to make people write their own cycle collection.

In the past when I looked at cxx it was great at ergonomics, but not so great at performance. e.g., string passing always required copying and allocating, while with cbindgen you can use UTF8String on WebIDL, and &nsAString on the rust side to read the utf-8 back without any extra copy...

https://phabricator.services.mozilla.com/D176786 already uses &nsAString with cxx, so I guess that's not a blocker?

That's on the other direction tho, right? (As a return value, not an argument)

Could you provide a relevant link? 👀

See https://github.com/eqrion/cbindgen/pull/293 for example. Though if we wanted the same for the webidl crate (without using cargo-expand or so) we'd probably want some sort of hook in cbindgen (since this would be custom stuff, not something generally applicable like bitflags).

Looks like that's what Google is doing in https://github.com/google/autocxx, although I haven't tried this one. Anyway, the purpose here is to implement Web IDL interface in Rust so that the generated C++ bindings code can call it, and I don't see how can that be done in the-other-way-around way. Could you elaborate?

You'd call BindingUtils functions from rust for example, maybe. It'd be implementing the glue at a different layer.

My impression was that they were migrating away from Web IDL, but it would be interesting if we could reuse some of that here!

I don't know, wasn't aware of that :)

Comment 11

[Kagami Rosylight [:saschanaz] (they/them)]

(In reply to Emilio Cobos Álvarez (:emilio) from comment #10)

Oh I see. I misunderstood your point. FWIW cbindgen can totally generate methods, it just doesn't by default. See this for some convenience methods we have in a variety of classes. Though I guess it'd need some plumbing so that they don't need to be written by hand.

Oh thanks, TIL export.body. But indeed it still requires writing things by hand, could be great if that becomes automatic.

If we eventually have some CC integration we probably need that anyways to derive cycle collection tho, right? We ideally don't want to make people write their own cycle collection.

That's for sure, yup.

That's on the other direction tho, right? (As a return value, not an argument)

But an argument also uses reference and shouldn't be different, doesn't it? This should be easy to confirm, let me try.

See https://github.com/eqrion/cbindgen/pull/293 for example. Though if we wanted the same for the webidl crate (without using cargo-expand or so) we'd probably want some sort of hook in cbindgen (since this would be custom stuff, not something generally applicable like bitflags).

Sounds like it, although I need to read more. My patches are currently way too immature to do anything that far though, let's see how it goes...

You'd call BindingUtils functions from rust for example, maybe. It'd be implementing the glue at a different layer.

Like calling CreateInterfaceObjects from Rust? Hmm, sounds possible, although it also sounds like the code would duplicate some part of dom/bindings/Codegen.py, which I'd like to defer 😛

I don't know, wasn't aware of that :)

Anyway, UniFFI doesn't support C++/Rust per the docs https://mozilla.github.io/uniffi-rs/#supported-languages 😬

Comment 12

[Kagami Rosylight [:saschanaz] (they/them)]

Attachment: WIP: Bug 1830486 - Part 4: Add IDL argument test

Depends on D176786