Skip to content

loops in the name graph #839

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

Closed
db48x opened this issue Jul 21, 2017 · 7 comments
Closed

loops in the name graph #839

db48x opened this issue Jul 21, 2017 · 7 comments
Labels

Comments

@db48x
Copy link
Contributor

db48x commented Jul 21, 2017

Input C/C++ Header

I don't have a minimal test case, and the whole thing is multiple megabytes. Perhaps I can revisit this soon and minimize one.

Bindgen Invocation

    let bindings = bindgen::Builder::default()
        .generate_comments(true)
        .clang_arg("-I../../src")
        .clang_arg("-I../../lib")
        .header("wrapper.h")
        .hide_type("BOOL_VECTOR_BITS_PER_CHAR")
        .generate()
        .expect("Unable to generate bindings");

Actual Results

#[repr(u32)]
#[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)]
pub enum gnutls_cipher_algorithm {
    GNUTLS_CIPHER_UNKNOWN = 0,
    GNUTLS_CIPHER_NULL = 1,
    // etc…
}
pub use self::gnutls_cipher_algorithm as gnutls_cipher_algorithm_t;
// … 2000 lines later …
pub use self::gnutls_cipher_algorithm_t as gnutls_cipher_algorithm;

Expected Results

The first pub use is fine, because many of the nearby function declarations use gnutls_cipher_algorithm_t. The second one is just silly, so it should be pruned.

@fitzgen
Copy link
Member

fitzgen commented Jul 21, 2017

Thanks again! As I said in the other bug, the best way to help us fix this issue is by providing a reduced test case: https://github.com/servo/rust-bindgen/blob/master/CONTRIBUTING.md#using-creduce-to-minimize-test-cases

@db48x
Copy link
Contributor Author

db48x commented Jul 21, 2017

Agreed, but I would rather file it without a testcase and forget to revisit the issue than not file it at all.

@emilio
Copy link
Contributor

emilio commented Jul 22, 2017

Agreed, but I would rather file it without a testcase and forget to revisit the issue than not file it at all.

Yeah, that's fair :)

Also sometimes it just rings a bell and we immediately know what's wrong, like in #840.

Without a reduced test-case we usually can't afford the time to investigate it in depth, but it's good that we can track it somewhere anyway, so thanks for it!

@emilio
Copy link
Contributor

emilio commented Jul 22, 2017

AFAICT this is gnutls, which does:

// gnutls.h
typedef enum gnutls_cipher_algorithm {
	GNUTLS_CIPHER_UNKNOWN = 0,
	// ...
} gnutls_cipher_algorithm_t;

And then:

// compat.h
/* Stuff deprected in 2.x */
typedef gnutls_cipher_algorithm_t gnutls_cipher_algorithm
    _GNUTLS_GCC_ATTR_DEPRECATED;

@emilio
Copy link
Contributor

emilio commented Jul 22, 2017

And thus a reduced test-case could be:

typedef enum foo {
  BAR,
  BAZ,
} foo_t;

typedef foo_t foo;

@emilio
Copy link
Contributor

emilio commented Jul 22, 2017

Which is quite annoying if you ask me.

@pvdrz
Copy link
Contributor

pvdrz commented Sep 19, 2022

If you run this test case with the current version of bindgen, you get:

pub const foo_BAR: foo = 0;
pub const foo_BAZ: foo = 1;
pub type foo = ::std::os::raw::c_uint;
pub use self::foo as foo_t;

if you mar foo as a rustified_enum:

#[repr(u32)]
#[derive(Debug, Copy, Clone, Hash, PartialEq, Eq)]
pub enum foo {
    BAR = 0,
    BAZ = 1,
}
pub use self::foo as foo_t;

So I think we can close this.

@pvdrz pvdrz closed this as completed Sep 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants