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

Handle enough template specialization for std::unique_ptr? #1887

Closed
adetaylor opened this issue Aug 28, 2020 · 2 comments
Closed

Handle enough template specialization for std::unique_ptr? #1887

adetaylor opened this issue Aug 28, 2020 · 2 comments

Comments

@adetaylor
Copy link
Contributor

Input C/C++ Header

Full test case here: https://gist.github.com/adetaylor/f878dbf25367c2553971bf72b1ed269f (164 lines)

Important bits here:

template <class _Tp> struct default_delete {};

template <class _Tp, class = void> struct __has_pointer_type : false_type {};

namespace __pointer_type_imp {

template <class _Tp, class _Dp, bool = __has_pointer_type<_Dp>::value>
struct __pointer_type {
  typedef typename _Dp::pointer type;
};

template <class _Tp, class _Dp> struct __pointer_type<_Tp, _Dp, false> {
  typedef _Tp *type;
};

}; // namespace __pointer_type_imp

template <class _Tp, class _Dp> struct __pointer_type {
  typedef typename __pointer_type_imp::__pointer_type<
      _Tp, typename remove_reference<_Dp>::type>::type type;
};

template <class _Tp, class _Dp = default_delete<_Tp>> class unique_ptr {
public:
  typedef _Dp deleter_type;

private:
  //_Tp* __ptr;
  __pointer_type<_Tp, deleter_type> __ptr;
};

}; // namespace std

std::unique_ptr<uint32_t> give_up1();
std::unique_ptr<uint8_t> give_up2();

Bindgen Invocation

$ ./bindgen/target/debug/bindgen --no-layout-tests --whitelist-function give_up1 --whitelist-function give_up2 test-minimal.hpp -- -x c++ -std=c++14

Actual Results

The same std_unique_ptr type is issued for both std::unique_ptrs even though one points to a uint8_t and the other to a uint32_t.

extern "C" {
    #[link_name = "\u{1}__Z8give_up1v"]
    pub fn give_up() -> std_unique_ptr;
}
extern "C" {
    #[link_name = "\u{1}__Z8give_up2v"]
    pub fn give_up2() -> std_unique_ptr;
}

(where std_unique_ptr ends up being a struct containing a single u8).

Expected Results

extern "C" {
    #[link_name = "\u{1}__Z8give_up1v"]
    pub fn give_up() -> std_unique_ptr<u32>;
}
extern "C" {
    #[link_name = "\u{1}__Z8give_up2v"]
    pub fn give_up2() -> std_unique_ptr<u8>;
}

(where the struct std_unique_ptr<u8> does indeed contain a pointer to a u8).

These 'expected results' are what we get if we alter the C++ to uncomment the line A and comment the line B.

Diagnosis

The test case above is a minimized version of std::unique_ptr and all its dependencies. It sadly depends on template specialization, so it's unsurprising that bindgen doesn't do a perfect job.

Here's what it's trying to do:

  1. unique_ptr contains a __pointer_type<main type, deleter type>. There are two specializations of __pointer_type:
  2. For deleter_types which declare a typename called pointer, that's what gets used.
  3. For anything else, main type* gets used.

Solution

I get that this is very, very hard and presumably the solutions are those explored in #297 and involve (at least) bindgen being able to calculate any constexpr to work out the correct template specialization! (or to use clang in a completely different way to ask it the same questions).

Mostly I'm writing this up so that other people who search the bindgen issues list for unique_ptr get a write up of why it doesn't work yet.

Context

I'm generating snippets of code for use with https://github.com/dtolnay/cxx (my very early experiments are here: https://github.com/google/autocxx). I'd like to be able to use bindgen to replace std_unique_ptr<T> with UniquePtr<T>. But, at the moment, bindgen simply discards the <T> bit because bindgen's analysis on whether the parameter is used decides that it's not.

As I delve deeper into this I might see if I can add a builder method to request bindgen to keep template arguments on certain types which it would otherwise discard. I'm not sure if that'll be possible, but that's the way I'm thinking about this.

What to do with this issue

It should probably be closed as a duplicate of #297!

@emilio
Copy link
Contributor

emilio commented Aug 29, 2020

So, for reference, returning std::unique_ptr from a function just won't work (generally, at least), because of ABI issues. MSVC treats types with destructors differently from Rust in extern functions, and expects them to be returned differently. That being said, it makes sense to do this anyway IMO.

What we've done for this kind of stuff in Firefox, is using the replaces functionality to replace it with a simpler type, so having something like this fed to bindgen:

/**
 * <div rustbindgen="true" replaces="std::unique_ptr">
 */
template<typename T> class simple_unique_ptr {
  T* ptr;
};

You don't need to use this type ever again in your C++ and bindgen should automatically generate the right thing.

@adetaylor
Copy link
Contributor Author

Thanks, that's more or less worked!

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

No branches or pull requests

2 participants