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

Support calling virtual functions #195

Closed
adetaylor opened this issue Jan 12, 2021 · 1 comment · Fixed by #198
Closed

Support calling virtual functions #195

adetaylor opened this issue Jan 12, 2021 · 1 comment · Fixed by #198

Comments

@adetaylor
Copy link
Collaborator

Currently there's no way to call virtual functions.

integration_tests::test_virtual_fns does not actually call virtual functions; it just checks they don't break other things.

For this test code:

        #include <cstdint>
        class A {
        public:
            A(uint32_t num) : b(num) {}
            virtual uint32_t foo(uint32_t a) { return a+1; };
            virtual ~A() {}
            uint32_t b;
        };
        class B: public A {
        public:
            B() : A(3), c(4) {}
            virtual uint32_t foo(uint32_t a) { return a+2; };
            uint32_t c;
        };

bindgen generates:

mod bindgen { /* automatically generated by rust-bindgen 0.56.1 */
    
    #[allow(non_snake_case, non_camel_case_types, non_upper_case_globals)]
    pub mod root {
        #[allow(unused_imports)]
        use self::super::root;
        #[repr(C)]
        pub struct A__bindgen_vtable(::std::os::raw::c_void);
        #[repr(C)]
        pub struct A {
            pub vtable_: *const A__bindgen_vtable,
            pub b: u32,
        }
        extern "C" {
            #[link_name = "\u{1}__ZN1AC1Ej"]
            pub fn A(this: *mut root::A, num: u32);
        }
        impl A {
            #[inline]
            pub unsafe fn new(num: u32) -> Self {
                let mut __bindgen_tmp = ::std::mem::MaybeUninit::uninit();
                A(__bindgen_tmp.as_mut_ptr(), num);
                __bindgen_tmp.assume_init()
            }
        }
        extern "C" {
            #[link_name = "\u{1}__ZN1A3fooEj"]
            pub fn foo(this: *mut ::std::os::raw::c_void, a: u32) -> u32;
        }
        extern "C" {
            #[link_name = "\u{1}__ZN1AD1Ev"]
            pub fn A_destructor(this: *mut root::A);
        }
        #[repr(C)]
        pub struct B {
            pub _base: root::A,
            pub c: u32,
        }
        extern "C" {
            #[link_name = "\u{1}__ZN1BC1Ev"]
            pub fn B(this: *mut root::B);
        }
        impl B {
            #[inline]
            pub unsafe fn new() -> Self {
                let mut __bindgen_tmp = ::std::mem::MaybeUninit::uninit();
                B(__bindgen_tmp.as_mut_ptr());
                __bindgen_tmp.assume_init()
            }
        }
        extern "C" {
            #[bindgen_original_name("foo")]
            #[link_name = "\u{1}__ZN1B3fooEj"]
            pub fn foo1(this: *mut ::std::os::raw::c_void, a: u32) -> u32;
        }
    }
     }

There's not much we can use there to deduce the existence of a virtual function on the A and B types. The parameter to the foo functions is not helpful.

Ideally we'd modify autocxx-bindgen to indicate that certain functions are virtual functions (adding to the list in #124). As a gross hack, we could assume that the output ordering is such that a this: *mut ::std::os::raw::c_void parameter represents the previously encountered virtual struct.

However we identify these virtual functions, we should not attempt to call them in Rust. Instead, we should always generate a FunctionWrapper for such calls, which will use -> on the std::unique_ptr and thus end up calling the right overridden version of the function, without any daft vtable interpretation on the Rust side. We'd need to make suitable entries in the impl blocks. And we'd want to do the same for any derived types, even if they didn't themselves implement the function.

@adetaylor
Copy link
Collaborator Author

Pure virtual functions aren't covered here - that's #305.

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

Successfully merging a pull request may close this issue.

1 participant