rust-lang / rust-bindgen

Automatically generates Rust FFI bindings to C (and some C++) libraries.
https://rust-lang.github.io/rust-bindgen/
BSD 3-Clause "New" or "Revised" License
4.22k stars 678 forks source link

Multiple definitions when a pointer gets typedef’d to the same name as a struct #2227

Closed linkmauve closed 1 year ago

linkmauve commented 2 years ago

Input C/C++ Header

typedef const struct foo {
    void *bar;
} *foo;

Bindgen Invocation

$ bindgen --no-layout-tests log.h > log.rs

Actual Results

/* automatically generated by rust-bindgen 0.59.2 */

#[repr(C)]
#[derive(Debug, Copy, Clone)]
pub struct foo {
    pub bar: *mut ::std::os::raw::c_void,
}
pub type foo = *const foo;

and/or

error[E0428]: the name `foo` is defined multiple times
 --> log.rs:8:1
  |
5 | pub struct foo {
  | -------------- previous definition of the type `foo` here
...
8 | pub type foo = *const foo;
  | ^^^^^^^^^^^^^^^^^^^^^^^^^^ `foo` redefined here
  |
  = note: `foo` must be defined only once in the type namespace of this module

warning: type `foo` should have an upper camel case name
 --> log.rs:8:10
  |
8 | pub type foo = *const foo;
  |          ^^^ help: convert the identifier to upper camel case (notice the capitalization): `Foo`
  |
  = note: `#[warn(non_camel_case_types)]` on by default

error[E0601]: `main` function not found in crate `log`
 --> log.rs:8:27
  |
8 | pub type foo = *const foo;
  |                           ^ consider adding a `main` function to `log.rs`

error[E0391]: cycle detected when expanding type alias `foo`
 --> log.rs:8:23
  |
8 | pub type foo = *const foo;
  |                       ^^^
  |
  = note: ...which immediately requires expanding type alias `foo` again
  = note: type aliases cannot be recursive
  = help: consider using a struct, enum, or union instead to break the cycle
  = help: see <https://doc.rust-lang.org/reference/types.html#recursive-types> for more information
note: cycle used when computing type of `<impl at log.rs:4:10: 4:15>`
 --> log.rs:4:10
  |
4 | #[derive(Debug, Copy, Clone)]
  |          ^
  = note: this error originates in the derive macro `Debug` (in Nightly builds, run with -Z macro-backtrace for more info)

error: aborting due to 3 previous errors; 1 warning emitted

Some errors have detailed explanations: E0391, E0428, E0601.
For more information about an error, try `rustc --explain E0391`.

Expected Results

I’m not quite sure, as the issue is that struct * and * are two “namespaces” in C, and not in C++ or Rust. One solution would be to rename either the pointer typedef or the struct, another would be to not include the pointer typedef at all (but then people might get confused why it isn’t present, so perhaps emit a warning as well?), or at least reject the input file altogether as the result won’t be well-formed anyway.

The project in question uses this hack when built under C++, so for my usecase I could #define __cplusplus in order to run bindgen in C++ mode:

#ifdef __cplusplus
# define PL_STRUCT(name) struct name##_t
#else
# define PL_STRUCT(name) struct name
#endif
ojeda commented 2 years ago

We hit this with Linux kernel code like https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/include/linux/maple_tree.h#n75.

I think providing a customizable prefix/suffix or moving the typedef inside a module with a customizable name would be the best.

pvdrz commented 1 year ago

I have a potential fix for this but I have a tiny confusion about C semantics here. Let's say we have this

typedef const struct foo {
    void *bar;
} *foo_ptr;

struct foo returns_struct();
foo_ptr returns_ptr();

If now we remove the _ptr prefix we have:

typedef const struct foo {
    void *bar;
} *foo;

struct foo returns_struct();
foo returns_ptr();

so, neither gcc nor clang complain about this so my question is: does returns_struct still returns a struct? or a pointer to a struct?

ojeda commented 1 year ago

The struct (struct foo) and the alias (foo) are in different C name spaces (tag vs. ordinary). returns_struct() indeed returns the struct in both cases, because using struct before foo will make it look for the identifier in the tag name space.

By the way, the Maple tree removed the problematic typedefs in the kernel, so at the moment we should not be triggering this issue.