C++17 bindings (uses "modern" data types for option,result,resource)

[cpetig]

Implements #826

• Implement tests
  • Fix variant
  • Fix results test
• Improve resource handling
• WASI 0.3: async, stream, future
• fixed size lists

[cpetig]

Part of #1098

[cpetig]

The functionality isn't 100% complete, but might still exceed the completeness of existing languages. So I would like to start the discussion on when it would be considered ready for merge.

[cpetig]

(flavorful probably requires introducing borrow variants of all records - as an extension)

[cpetig]

@alexcrichton No urgency, but I would like to get this PR merged at some time. So feed-back about what to change is most welcome. The patch had been in draft for a long time but with enough tests added I have more confidence now.

How to solve the verify-publish failure? Would you like to register wit-bindgen-cpp as a crate on crates.io owned by you and the bytecode alliance?

[alexcrichton]

Sounds good! Everything here looks good to me, although I'm not doing a super-careful review of all the guest bits. If it passes tests though I think that's a fantastic starting place. For verify-publish you'll want to update this list here and I think it should be good to go.

One other question before merging infrastructure-wise: if you agree I think it'd be reasonable to jettison the old C++ support in testing. It's a bit awkward to have both that and this and I don't think there's much use for retaining the old *.cpp tests that exclusively use the C API. We can figure out perhaps how to test that differently in the future but with a first-class generator that supports C++ I think it's best to defer to that mostly in testing. (e.g. dropping cpp17 in the test in favor of just calling it cpp, etc)

[TartanLlama]

Great to see this! I had a play around but couldn't see a way to get it to copy the wit-{guest,common}.h headers into the bindings directory. Is there some flag I'm missing, or is there another way that users are expected to install these headers?

Also, I noticed that record types in the guest bindings are passed by value. Is the intention that the guest should std::move records into imported calls? For example:

interface guy-fighter-host {
    record type-of-guy {
        name: string,
        strength: u8,
        charisma: u8,
        agility: u8,
        battle-cries: list<string>,
    }
    invent-entirely-new-type-of-guy: func(guy-type: type-of-guy);
}

Generates:

namespace tl {namespace guy_fighter {namespace guy_fighter_host {struct TypeOfGuy {
  wit::string name;
  uint8_t strength;
  uint8_t charisma;
  uint8_t agility;
  wit::vector<wit::string> battle_cries;
};
void InventEntirelyNewTypeOfGuy(TypeOfGuy guy_type);
}}}

Then since wit::vector is not copyable, this usage wouldn't compile:

 void exports::guy_fighter_plugin::Init() {
    using namespace tl::guy_fighter::guy_fighter_host;
    wit::string battle_cries[] = {
        wit::string::from_view("I love templates!"),
    };
    TypeOfGuy guy_type {
        .name = wit::string::from_view("C++ Guy"),
        .strength = 10,
        .charisma = 5,
        .agility = 7,
        .battle_cries = wit::vector<wit::string>::from_view(wit::span<wit::string>{battle_cries, 3})
    };
    InventEntirelyNewTypeOfGuy(guy_type); // Can't copy guy_type, must move
}

[cpetig]

For verify-publish you'll want to update this list here and I think it should be good to go.

Done.

One other question before merging infrastructure-wise: if you agree I think it'd be reasonable to jettison the old C++ support in testing. It's a bit awkward to have both that and this and I don't think there's much use for retaining the old *.cpp tests that exclusively use the C API. We can figure out perhaps how to test that differently in the future but with a first-class generator that supports C++ I think it's best to defer to that mostly in testing. (e.g. dropping cpp17 in the test in favor of just calling it cpp, etc)

I guess it makes sense to rename the "compile C as C++" test from cpp to cpp85 (first edition of the book describing the language before templates) as it uses minimal C++.

But, I have a hard time naming cpp17 the official cpp, as I watch the fragmentation of the C++ ecosystem. I chose 17 because this is the highest number blessed by MISRA for safety critical applications. This also matches with AUTOSAR's adaptive platform flavor of C++ (which is my main target). But I added a cpp17 compatible std::expected by @TartanLlama for Result as this is the closest you get to ara::core::Result within standard C++. For std::future::then() (continuation as defined by the concurrency TS addendum to C++11 which is still in evolution) and streams I didn't find any solution, yet.

Sorry, personally I was an absolute C++ fan 25 years ago and defended it for safety critical applications, but today I only write this code to achieve maintainable compatibility of large (legacy) C++ code bases with Rust.

[cpetig]

Great to see this! I had a play around but couldn't see a way to get it to copy the wit-{guest,common}.h headers into the bindings directory. Is there some flag I'm missing, or is there another way that users are expected to install these headers?

I haven't decided a good way to provide these headers to the applications, wit-bindgen could include them and paste them into the generated directory, but I have modified versions of them for the C++ flavors used on a per project base. Basically this became more and more of a support library, see also these more things to come for WASI 0.3 stream support.

So install and pkg-config seem inevitable in the future. It just isn't high priority to me.

Also, I noticed that record types in the guest bindings are passed by value. Is the intention that the guest should std::move records into imported calls? For example:

If you take a look into why the "flavorful" test fails (I didn't include the broken test code), you can see that two variants of each data type are needed (owned and borrowed) to handle it well.

The fact that move semantics in C++ is difficult to learn made me create the new api setting which isn't a perfect fit for the component model, unless you go for the symmetric ABI variant. But nobody wants to teach the intricacies of the more efficient old API where incoming and outgoing functions have a different signature, because of ownership semantics.

I have no good answer (besides CPB) on how passing resources in records as arguments should feel less confusing as the resources get consumed but passed via a const& argument.

Short: The header file was intended to remain minimal, for practical use some copy and move ctors and assignment operators are missing. Any feed-back and patch to make it less painful is most welcome.

[cpetig]

With respect to wit-vector: wit-vector is an owning handle (much like std::vector, but compatible with Rust as you can turn it into a malloced pointer and a length and then turn it to a Rust Vec via from_raw)

(Deeply) copying this is probably always a bad idea, this is why there is no copy ctor.

[cpetig]

PS: Please keep in mind that I am mostly targeting "code a minimal adapter in C++ and quickly get back to the sunny Rust side" scenarios, so usability on the C++ side isn't my main goal.

[alexcrichton]

Sounds reasonable to me 👍

Would you prefer to land this and iterate in-tree? Or would you prefer to follow-up with anything here before doing so?

[cpetig]

Would you prefer to land this and iterate in-tree? Or would you prefer to follow-up with anything here before doing so?

After working on this for two years out of tree, and having plenty of code building on this in other branches, I really would like to merge to reduce the difference between the trees. Feed-back on the complex design choices of ownership is more likely to come in tree.

And the new testing environment gave it a significant quality boost, although I know of long existing unsolved questions around option<string> as an argument to guest imported functions. I think optional<string_view> would be the best fit, but C++ types are less orthogonal than Rust types (see void in templates vs () in generics).

Teaser: I have a working example exchanging future<string> between C++ and Rust and hope to finish stream<string> this weekend. (This is compiling to native, running on wasmtime requires a C++ implementation of async_support for the canonical ABI which I plan for later)

[alexcrichton]

That's what I figured just wanted to be sure! It goes without saying but I want to say it anyway: thank you so much for your work on this! I very much look forward to the upcoming features and iteration in-tree 🎊