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

Inconsistent behaviour of derive_default for opaque types #3096

Open
wmmc88 opened this issue Jan 24, 2025 · 1 comment
Open

Inconsistent behaviour of derive_default for opaque types #3096

wmmc88 opened this issue Jan 24, 2025 · 1 comment

Comments

@wmmc88
Copy link

wmmc88 commented Jan 24, 2025

There seems to be inconsistent behavior for derive_default based on whether the type is detected to be opaque by bindgen or explicitly marked as opaque by bindgen.

For a C definition of struct Foo; for an opaque struct, bindgen will generate the following with derive_default(true):

#[repr(C)]
#[derive(Debug, Copy, Clone)]
pub struct Foo {
    _unused: [u8; 0],
}

When Foo is also explicitly marked opaque with .opaque_type("Foo"), it generates the following:

#[repr(C)]
#[derive(Debug, Default, Copy, Clone)]
pub struct Foo {
    _unused: [u8; 0],
}

I don't see why explicitly marking the type as opaque should change whether or not Default is derived. I also think that it makes even less sense that Default gets derived for a type that is supposed to be Opaque, since opaque types should never be directly accessed and should be handled only via pointers.

Other Information
rust-bindgen 0.71.1
clang version 19.1.7
Target: x86_64-pc-windows-msvc

Full Code Sample
build.rs:

use std::env;
use std::path::PathBuf;

fn main() {
    tracing_subscriber::fmt::init();

    let builder = bindgen::Builder::default()
        .header_contents(
            "input.h",
            r#"
struct Foo;

struct BAR__{int unused;};
typedef struct BAR__ *BAR;
"#,
        )
        .parse_callbacks(Box::new(bindgen::CargoCallbacks::new()))
        .derive_default(true);

    let out_path = PathBuf::from(env::var("OUT_DIR").unwrap());

    builder
        .clone()
        .generate()
        .expect("Unable to generate bindings")
        .write_to_file((out_path).join("not-explicit-opaque.rs"))
        .expect("Couldn't write bindings!");

    builder
        .opaque_type("Foo")
        .opaque_type("BAR__") // Only BAR__ should be marked as opaque. The convenience type BAR should remain as a pointer to the opaque BAR__. 
        .generate()
        .expect("Unable to generate bindings")
        .write_to_file((out_path).join("explicit-opaque.rs"))
        .expect("Couldn't write bindings!");
}

Output:
not-explicit-opaque.rs:

/* automatically generated by rust-bindgen 0.71.1 */

#[repr(C)]
#[derive(Debug, Copy, Clone)]
pub struct Foo {
    _unused: [u8; 0],
}
#[repr(C)]
#[derive(Debug, Default, Copy, Clone)]
pub struct BAR__ {
    pub unused: ::std::os::raw::c_int,
}
#[allow(clippy::unnecessary_operation, clippy::identity_op)]
const _: () = {
    ["Size of BAR__"][::std::mem::size_of::<BAR__>() - 4usize];
    ["Alignment of BAR__"][::std::mem::align_of::<BAR__>() - 4usize];
    ["Offset of field: BAR__::unused"][::std::mem::offset_of!(BAR__, unused) - 0usize];
};
pub type BAR = *mut BAR__;

explicit-opaque.rs:

/* automatically generated by rust-bindgen 0.71.1 */

#[repr(C)]
#[derive(Debug, Default, Copy, Clone)]
pub struct Foo {
    _unused: [u8; 0],
}
#[repr(C)]
#[repr(align(4))]
#[derive(Debug, Default, Copy, Clone)]
pub struct BAR__ {
    pub _bindgen_opaque_blob: u32,
}
#[allow(clippy::unnecessary_operation, clippy::identity_op)]
const _: () = {
    ["Size of BAR__"][::std::mem::size_of::<BAR__>() - 4usize];
    ["Alignment of BAR__"][::std::mem::align_of::<BAR__>() - 4usize];
};
pub type BAR = *mut BAR__;

Output Diff (left is no explicit opaque. right is explicit opaque):
Image

@jschwe
Copy link
Contributor

jschwe commented Jan 30, 2025

Opaque types shouldn't derive any of Copy, Clone or default. See also: #1656.

( I have some patches related to opaque types on my fork of bindgen, but didn't have the time yet to clean the ones related to opaque types up enough to open a PR)

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