Dependent qualified types don't work

[adetaylor]

Given this code:

/**
* <div rustbindgen=\"true\" replaces=\"std::string\">
*/
class FakeString {
    char* ptr;
};

#include <string>
#include <cstdint>

template <typename STRING_TYPE>
class BasicStringPiece;

typedef BasicStringPiece<std::string> StringPiece;

template <typename STRING_TYPE> class BasicStringPiece {
public:
    typedef size_t size_type;
    typedef typename STRING_TYPE::value_type value_type;
    const value_type* ptr_;
    size_type length_;
};

struct Origin {
    // void SetHost(StringPiece host);
    StringPiece host;
};

bindgen emits this:

pub type StringPiece = BasicStringPiece;
#[repr(C)]
#[derive(Debug, Copy, Clone)]
pub struct BasicStringPiece {
    pub ptr_: *const BasicStringPiece_value_type,
    pub length_: BasicStringPiece_size_type,
}
pub type BasicStringPiece_size_type = size_t;
pub type BasicStringPiece_value_type = [u8; 0usize];
#[repr(C)]
pub struct Origin {
    pub host: StringPiece,
}

which is fine from a Rust point of view, but no good for us because the SetHost function would get C++ bindings generated as std::unique_ptr<BasicStringPiece> instead of std::unique_ptr<StringPiece> or std::unique_ptr<BasicStringPiece<std::string>>.

[adetaylor]

There seem to be three problems here:

• Sometimes bindgen correctly emits the templated type as a Rust generic. We weren't handling that correctly; we were passing such types onto cxx::bridge as type aliases and that resulted in errors. Fixed in Allow simple templated types in bindgen generation #107 . Note that this doesn't allow us to use such types in client code, but it means they no longer break usage of downstream types (e.g. Origin in the above example).
• bindgen does not handle types with associated types. This may be a known limitation. Issue reported here in case it's do-able. Associated types not resolved to concrete type rust-lang/rust-bindgen#1924
• Our suppression of std::string is causing trouble when it's used in templated types. Maybe there's some way we can make it more opaque but still allow it.

[adetaylor]

To be specific on the last point: we are expunging all mention of std::string from bindgen output by means of a prelude and a blocklist. So, when we encounter a type (base::StringPiece) which fundamentally depends on std::string (or specifically std::string::value_type) - shock - it doesn't work!

I am planning to see if I can persuade bindgen to output enough of std::string for this to work, without so much that everything breaks.

[adetaylor]

It turns out bindgen does a creditable job of producing bindings for std::string. I doubt they truly represent the real layout in memory, and of course they don't represent the pinned nature of it, but they are internally consistent and build. However, the arrangement of types generated by bindgen is predictably fragile, and our various alterations break everything. That being the case, here are some options:

1. Allow bindgen bindings to pass through to the resulting code untouched. Use them as a guide to generate extra bindings, both in the bindgen output mod, and elsewhere (in the cxx::bridge mod and the top level mod), but don't change anything.
2. As above, but do alter types to be non-POD unless the user has specifically asked for them to be POD. This is fiddly.
3. As above, but switch from a POD allowlist to a POD blocklist, and pass that blocklist to bindgen such that it can make types opaque.
4. Continue to meddle with bindgen bindings as we do now, but allow the user to mark some types as opaque.

[adetaylor]

Current state of play:

• bindgen doesn't correctly identify that some types are templated, per Associated types not resolved to concrete type rust-lang/rust-bindgen#1924. I'm working on this in the background. For such types, we don't really stand a chance - we inevitably tell C++ that this is a plain old type not a templated type, and that results in compilation failures. That accounts for the need to block! some types in my current real-world experiments e.g. https://twitter.com/adehohum/status/1334732132522967041. In general though bindgen does a really remarkable job of passing on the generic-ness of types to us.
• cxx does not understand generic types (as far as I understand it) with the exception of those it knows about - std::vector, etc. So for other generic types we have to squash them down to a flat type on their way between bindgen and cxx.
• So, since Slightly better templated type support #161, we now recognize the impending doom, and generate a typedef in C++ to turn each concrete instance into a type of its own.
• We do not attach methods to such types. This could be achieved relatively easily. The same applies to constructors.
• Such types are always opaque.
• So at the moment, such types are only useful if they're returned by one function and then you pass them to another. You can't interact with them in any other way.
• Improve testing for concrete templated types #162 says we don't even test that properly 👍

Next steps here:

• Consider whether we can allow cxx to understand generic types. This would be a big lift but would make things much better.
• Otherwise, consider attaching methods and constructors to our synthesized concrete types.
• In parallel, continue to work on that bindgen stuff.

[dtolnay]

cxx does not understand generic types (as far as I understand it) with the exception of those it knows about - std::vector, etc. So for other generic types we have to squash them down to a flat type on their way between bindgen and cxx.

It would be good to get this implemented upstream as extern "C++" { type Container<T>; }.

[adetaylor]

Current state of play here with respect to Chromium:

• https://chromium-review.googlesource.com/c/chromium/src/+/2784885 made that we no longer suffer this problem with base::StringPiece (thanks Jan!)
• But unfortunately base::Optional still seems to run into the same problem (or similar)... I will work on diagnosing

[adetaylor]

I posted a status update to rust-lang/rust-bindgen#1924

[adetaylor]

I'm removing the "bug" label here since we now gracefully avoid mis-generations in such cases. As such this now becomes a C++ feature for which we need to add support.

[adetaylor]

#1459 has enabled this test because we now rely on bindgen to do the right thing here, thanks to #1456.