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

Types with non-type template parameters turn into integers of the wrong size #787

Open
bsilver8192 opened this issue Feb 11, 2022 · 8 comments
Labels
bug Something isn't working

Comments

@bsilver8192
Copy link
Contributor

If I have a C++ class with a member function that interacts with a std::chrono::time_point instantiation by value, autocxx generates wrappers that replace time_point with an integer type. My full codebase ends up with uint64_t, my simplified example ends up with uint8_t, not sure why they're different. This results in C++ wrappers that don't compile, so I can't even use other member functions that do have signatures autocxx can handle.

Expected Behavior

Skip generating wrappers for these methods, like other forms of unhandled things.

Actual Behavior

Generated C++ code that doesn't compile.

Steps to Reproduce the Problem

#include <chrono>
struct Clock {
  typedef std::chrono::nanoseconds duration;
};
struct Class {
  int a() { return 42; }
  std::chrono::time_point<Clock> b();
};
  1. In some include_cpp! in some .rs file, generate!("Class").
  2. Try compiling the generated C++ code.

I don't see any integration tests which try compiling the generated C++ code I can add this to and submit a PR, let me know if there's a good way to do that.

Specifications

@adetaylor
Copy link
Collaborator

Thanks for the report.

I don't see any integration tests which try compiling the generated C++ code I can add

Any of the integration tests should try to build the resulting C++ code, and even execute it. They each use the trybuild crate to build and run a complete executable, linking the Rust and C++ together.

adetaylor added a commit that referenced this issue Feb 11, 2022
@adetaylor
Copy link
Collaborator

Here's what bindgen gives us:

        extern "C" {
            #[cpp_semantics(original_name("b"))]
            #[link_name = "\u{1}__ZN5Class1bEv"]
            pub fn Class_b(this: *mut root::Class) -> u8;
        }
        impl Class {
            #[inline]
            pub unsafe fn a(&mut self) -> ::std::os::raw::c_int {
                Class_a(self)
            }
            #[inline]
            pub unsafe fn b(&mut self) -> u8 {
                Class_b(self)
            }
        }

so I'm pretty sure this will turn out to be a limitation of bindgen. It would be nice to detect this limitation and then ignore such APIs (as you say in the issue description). Unfortunately, no obvious clues are currently present in the bindgen output, so I think we'll have to make a change to bindgen here (or to our fork, autocxx_bindgen).

As a first step, I'm reducing this into an even smaller test case.

@adetaylor adetaylor added the bug Something isn't working label Feb 11, 2022
@adetaylor
Copy link
Collaborator

Oh, and for completeness, here's what actually goes wrong with the test case added in #787:

cargo:warning=/var/folders/9l/cjqbc3ws3bxcjh6h2rykj90800m02d/T/.tmpCxKn9X/target/cxx/gen0.cxx:173:29: error: cannot initialize a variable of type '::std::uint8_t (Class::*)()' with an rvalue of type 'std::chrono::time_point<Clock> (Class::*)()': different return type ('::std::uint8_t' (aka 'unsigned char') vs 'std::chrono::time_point<Clock>')
cargo:warning=  ::std::uint8_t (::Class::*b$)() = &::Class::b;
cargo:warning=                            ^       ~~~~~~~~~~~

adetaylor added a commit that referenced this issue Feb 11, 2022
@adetaylor
Copy link
Collaborator

adetaylor commented Feb 12, 2022

Here's the creduced version:

typedef char c;
typedef int d;
typedef c uint8_t;
typedef d e;
namespace std {
using ::uint8_t;
template <e> struct f;
typedef f<0> g;
namespace chrono {
template <typename h, typename = typename h::duration> struct i;
using j = g;
template <typename, typename k> struct i {
  typedef k duration;
  duration l;
};
} // namespace chrono
} // namespace std
struct m {
  typedef std::chrono::j duration;
};
struct Class {
  int a();
  std::chrono::i<m> b();
};

@adetaylor
Copy link
Collaborator

adetaylor commented Feb 12, 2022

Running upstream bindgen ca78ae9c41998713db0c6fa70cd21089cdb2af2c with these arguments:

cargo run -- "--rust-target" "1.47" "--default-enum-style" "rust" "--blocklist-item" "rust::String" "--blocklist-item" "std::unique_ptr" "--blocklist-item" "std::weak_ptr" "--blocklist-item" "std::string" "--blocklist-item" "std::shared_ptr" "--blocklist-item" "std::vector" "--blocklist-item" "rust::Str" "--blocklist-item" "rust::Box" "--allowlist-function" "Class" "--allowlist-function" "Class" "--allowlist-function" "autocxx_make_string_default" "--allowlist-type" "Class" "--allowlist-type" "Class" "--allowlist-type" "autocxx_make_string_default" "--allowlist-var" "Class" "--allowlist-var" "Class" "--allowlist-var" "autocxx_make_string_default" "--no-layout-tests" "--no-derive-copy" "--no-derive-debug" "--no-derive-default" "--enable-cxx-namespaces" "--generate" "functions,types,vars,methods,constructors,destructors" "--generate-inline-functions" "--respect-cxx-access-specs" test.hpp  "--" "-x" "c++" "-std=c++14" "-DBINDGEN"

yields

#[allow(non_snake_case, non_camel_case_types, non_upper_case_globals)]
pub mod root {
    #[allow(unused_imports)]
    use self::super::root;
    pub mod std {
        #[allow(unused_imports)]
        use self::super::super::root;
        pub type g = u8;
        pub mod chrono {
            #[allow(unused_imports)]
            use self::super::super::super::root;
            pub type j = root::std::g;
            #[repr(C)]
            pub struct i<k> {
                pub l: root::std::chrono::i_duration<k>,
                pub _phantom_0:
                    ::std::marker::PhantomData<::std::cell::UnsafeCell<k>>,
            }
            pub type i_duration<k> = k;
        }
    }
    #[repr(C)]
    pub struct m {
        pub _address: u8,
    }
    pub type m_duration = root::std::chrono::j;
    #[repr(C)]
    pub struct Class {
        pub _address: u8,
    }
    extern "C" {
        #[link_name = "\u{1}_ZN5Class1aEv"]
        pub fn Class_a(this: *mut root::Class) -> ::std::os::raw::c_int;
    }
    extern "C" {
        #[link_name = "\u{1}_ZN5Class1bEv"]
        pub fn Class_b(this: *mut root::Class) -> u8;
    }
    impl Class {
        #[inline]
        pub unsafe fn a(&mut self) -> ::std::os::raw::c_int {
            Class_a(self)
        }
        #[inline]
        pub unsafe fn b(&mut self) -> u8 {
            Class_b(self)
        }
    }
}

confirming that this appears to be normal bindgen behavior.

@adetaylor
Copy link
Collaborator

adetaylor commented Feb 12, 2022

Some manual minimization gives this:

namespace chrono {
template <typename h, typename = typename h::duration> struct i;
template <int> struct f;
typedef f<0> j;
template <typename, typename k> struct i {
  typedef k duration;
  duration l;
};
} // namespace chrono
struct m {
  typedef chrono::j duration;
};
struct Class {
  int a();
  chrono::i<m> b();
};

@bsilver8192
Copy link
Contributor Author

After poking into bindgen a bit, I think the bugs there are rust-lang/rust-bindgen#362 and then rust-lang/rust-bindgen#869 in the fallback "opaque blob" codegen. The former looks hard to fix (it mentions libclang changes). Fixing the latter is still going to confuse autocxx into generating C++ code that doesn't compile, but I think that's a duplicate of #687.

@bsilver8192
Copy link
Contributor Author

Looks like the limitation around non-type template parameters means bindgen doesn't have any way to even get the size of the type, which pretty much prevents any chance of making this fully work. Best case right now looks like bindgen getting smarter about omitting types where it can't find the size, instead of guessing their size is 1.

@bsilver8192 bsilver8192 changed the title std::chrono::time_point turns into an integer Types with non-type template parameters turn into u8 Mar 12, 2022
@bsilver8192 bsilver8192 changed the title Types with non-type template parameters turn into u8 Types with non-type template parameters turn into integers of the wrong size Mar 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants