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

*_enum() takes item name after parse_callbacks processing instead of before #1891

Closed
johnmave126 opened this issue Sep 8, 2020 · 3 comments

Comments

@johnmave126
Copy link
Contributor

Input C/C++ Header

enum Test {
    OptionA,
    OptionB,
    OptionC
};

Bindgen Invocation

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

#[derive(Debug)]
struct RenameCallbacks {}

impl bindgen::callbacks::ParseCallbacks for RenameCallbacks {
    fn item_name(&self, original_variant_name: &str) -> Option<String> {
        if original_variant_name == "Test" {
            Some("AnotherName".into())
        } else {
            None
        }
    }
}

fn main() {
    let out_path = PathBuf::from(env::var("OUT_DIR").unwrap());
    let bindings = bindgen::Builder::default()
        .header("test.hpp")
        .parse_callbacks(Box::new(RenameCallbacks {}))
        .whitelist_type("Test")
        .bitfield_enum("Test")
        .generate()
        .expect("Couldn't generate bindings!");
    bindings
        .write_to_file(out_path.join("bindings.rs"))
        .expect("Couldn't write bindings!");
}

Actual Results

pub const AnotherName_OptionA: AnotherName = 0;
pub const AnotherName_OptionB: AnotherName = 1;
pub const AnotherName_OptionC: AnotherName = 2;
pub type AnotherName = ::std::os::raw::c_int;

Expected Results

Should generate bitfield enum. I figured out that changing the invocation to

    let bindings = bindgen::Builder::default()
        .header("test.hpp")
        .parse_callbacks(Box::new(RenameCallbacks {}))
        .whitelist_type("Test")
        .bitfield_enum("AnotherName")
        .generate()
        .expect("Couldn't generate bindings!");

will generate expected bindings.

This may not be a bug but it is definitely not documented. And it seems inconsistent with other APIs, since other APIs like whitelist_type() asks for name before parse_callbacks processing.

@emilio
Copy link
Contributor

emilio commented Sep 8, 2020

Yeah, I agree the inconsitency is wrong. This seems like a long-standing bug. The issue is that this line:

let path = item.canonical_path(ctx);

Is using canonical_path instead of path_for_whitelisting. It probably predates the parse callbacks machinery.

Should be a pretty easy bug to fix.

@johnmave126
Copy link
Contributor Author

I searched through the code base and issues a little bit and found #1125 and #1162 , and it seems that whitelist/blacklist was using canonical_path at that time. Now that whitelist/blacklist has moved away from that, should canonical_path be completely removed from the code base? The only other occurrence of canonical_path is

let mut path = item.canonical_path(ctx);
let args: Vec<_> = self
.template_arguments()
.iter()
.map(|arg| {
let arg_path = arg.canonical_path(ctx);
arg_path[1..].join("::")
})
.collect();

which I believe may have the same issue when using opaque_type()

@emilio
Copy link
Contributor

emilio commented Sep 9, 2020

There are multiple occurrences of both canonical_path and namespace_aware_canonical_path in src/codegen/mod.rs (as expected, as that's the "final" path we generate code for).

However yes, I think that code should probably also be using path_for_whitelisting and so on.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants