Skip to content

libbindgen: error: expected type, found keyword type #342

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
bbigras opened this issue Dec 15, 2016 · 19 comments
Closed

libbindgen: error: expected type, found keyword type #342

bbigras opened this issue Dec 15, 2016 · 19 comments

Comments

@bbigras
Copy link

bbigras commented Dec 15, 2016

rustc 1.15.0-nightly (daf8c1dfc 2016-12-05)
libbindgen = "0.1.2"
clang 3.9.0-3
opencv 3.1.0-6
Arch Linux 64-bit

wrapper.hpp

#include <opencv2/core/core.hpp>

build.rs

extern crate libbindgen;

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

fn main() {
    println!("cargo:rustc-link-lib=opencv_core");

    let bindings = libbindgen::Builder::default()
        .header("wrapper.hpp")
        .generate()
        .expect("Unable to generate bindings");

    let out_path = PathBuf::from(env::var("OUT_DIR").unwrap());
    bindings.write_to_file(out_path.join("bindings.rs"))
        .expect("Couldn't write bindings!");
}
$ cargo build
    Blocking waiting for file lock on build directory
   Compiling opencv v0.1.0 (file:///home/bruno/dev/rust/opencv)
error: expected type, found keyword `type`
    --> /home/bruno/dev/rust/opencv/target/debug/build/opencv-b0a1388c401941c1/out/bindings.rs:3367:46
     |
3367 |                                           -> type-parameter-0-1>,
     |                                              ^^^^

error: Could not compile `opencv`.
@emilio
Copy link
Contributor

emilio commented Dec 15, 2016

Can you paste or upload the bindings.rs file?

@emilio
Copy link
Contributor

emilio commented Dec 15, 2016

That usually means that there's some template magic we can't support in rust. We can try to fix this case if it's not too painful, but otherwise the alternatives are to use the whitelisting/blacklisting APIs to deal with that.

@emilio
Copy link
Contributor

emilio commented Dec 15, 2016

Oh, and thanks for the reports, they're really helpful :)

@bbigras
Copy link
Author

bbigras commented Dec 15, 2016

bindings.zip

@emilio
Copy link
Contributor

emilio commented Dec 15, 2016

Yeah, so it's trying to generate the whole STL, that is something that
we're not extremely bad at, but still not good enough, you may see we
don't generate correct rust code for std::pointer_to_unary_function, for
example:

#[repr(C)]
#[derive(Debug, Copy, Clone)]
pub struct std_pointer_to_unary_function<_Arg, _Result> {
    pub _M_ptr: ::std::option::Option<unsafe extern "C" fn(arg1: _Arg)
                                          -> type-parameter-0-1>,
    pub _phantom_1: ::std::marker::PhantomData<_Result>,
}

We can try to fix those particular issues, but for C++ libraries it's
usually saner to just whitelist what you want. In this case, doing
something like:

.whitelist_type("Cv.*")
.whitelist_function("cv.*)

should do the trick :)

@bbigras
Copy link
Author

bbigras commented Dec 15, 2016

It worked for type-parameter-0-1. 👍 Thanks.

Now I have two other cases:

$ cargo build
    Blocking waiting for file lock on build directory
   Compiling opencv v0.1.0 (file:///home/bbigras/dev/rust/rs-opencv)
error: expected identifier, found keyword `type`
    --> /home/bbigras/dev/rust/rs-opencv/target/debug/build/opencv-2abe0d5d07f86d1d/out/bindings.rs:1226:19
     |
1226 |     pub unsafe fn type(&self) -> ::std::os::raw::c_int {
     |                   ^^^^

error: expected one of `(` or `<`, found `~`
    --> /home/bbigras/dev/rust/rs-opencv/target/debug/build/opencv-2abe0d5d07f86d1d/out/bindings.rs:1288:15
     |
1288 |     pub fn cv_~String();
     |               ^

error: Could not compile `opencv`.

bindings.zip

@emilio
Copy link
Contributor

emilio commented Dec 15, 2016

Nice, the rest are fallout from recent changes. Should be fixed with #344.

@emilio
Copy link
Contributor

emilio commented Dec 15, 2016

We didn't have a test for inline destructors, and it bited us :P

@emilio
Copy link
Contributor

emilio commented Dec 15, 2016

Also, I didn't notice that the opencv library used namespaces, bindgen has support for those, in case of name conflicts.

We plan to enable them by default sometime soon, and since both SpiderMonkey and Gecko are using bindgen with that feature enabled, I'm pretty confident they'd be solid enough for OpenCv.

bors-servo pushed a commit that referenced this issue Dec 15, 2016
Properly mangle method names, don't generate destructors.

r? @fitzgen

Fixes a few issues seen in #342
@bbigras
Copy link
Author

bbigras commented Dec 15, 2016

Also, I didn't notice that the opencv library used namespaces, bindgen has support for those, in case of name conflicts.

Do you mean enable_cxx_namespaces()? Can I use it instead of the whitelist options?

With your 'fix-dtors' branch (and still using .whitelisted_type("Cv.*").whitelisted_function("cv.*")) I get:

error[E0428]: a type named `std_char_traits` has already been defined in this module
   --> /home/bbigras/dev/rust/rs-opencv/target/debug/build/opencv-2abe0d5d07f86d1d/out/bindings.rs:185:1
    |
174 |   pub struct std_char_traits<_CharT> {
    |  _- starting here...
175 | |     pub _address: u8,
176 | |     pub _phantom_0: ::std::marker::PhantomData<_CharT>,
177 | | }
    | |_- ...ending here: previous definition of `std_char_traits` here
...
185 |   pub struct std_char_traits<_CharT> {
    |  _^ starting here...
186 | |     pub _address: u8,
187 | |     pub _phantom_0: ::std::marker::PhantomData<_CharT>,
188 | | }
    | |_^ ...ending here: already defined

error: unions are unstable and possibly buggy (see issue #32836)
  --> /home/bbigras/dev/rust/rs-opencv/target/debug/build/opencv-2abe0d5d07f86d1d/out/bindings.rs:67:1
   |
67 |   pub union std_namespace_basic_string__bindgen_ty_2<_CharT, _Traits, _Alloc> {
   |  _^ starting here...
68 | |     pub _M_local_buf: *mut _CharT,
69 | |     pub _M_allocated_capacity: std_namespace_basic_string_size_type<_Alloc>,
70 | |     pub _phantom_1: ::std::marker::PhantomData<_Traits>,
71 | | }
   | |_^ ...ending here
   |
   = help: add #![feature(untagged_unions)] to the crate attributes to enable

error: unions are unstable and possibly buggy (see issue #32836)
   --> /home/bbigras/dev/rust/rs-opencv/target/debug/build/opencv-2abe0d5d07f86d1d/out/bindings.rs:482:1
    |
482 |   pub union Cv32suf {
    |  _^ starting here...
483 | |     pub i: ::std::os::raw::c_int,
484 | |     pub u: ::std::os::raw::c_uint,
485 | |     pub f: f32,
486 | | }
    | |_^ ...ending here
    |
    = help: add #![feature(untagged_unions)] to the crate attributes to enable

error: unions are unstable and possibly buggy (see issue #32836)
   --> /home/bbigras/dev/rust/rs-opencv/target/debug/build/opencv-2abe0d5d07f86d1d/out/bindings.rs:497:1
    |
497 |   pub union Cv64suf {
    |  _^ starting here...
498 | |     pub i: int64,
499 | |     pub u: uint64,
500 | |     pub f: f64,
501 | | }
    | |_^ ...ending here
    |
    = help: add #![feature(untagged_unions)] to the crate attributes to enable

error: unions are unstable and possibly buggy (see issue #32836)
   --> /home/bbigras/dev/rust/rs-opencv/target/debug/build/opencv-2abe0d5d07f86d1d/out/bindings.rs:560:1
    |
560 |   pub union _bindgen_ty_16__bindgen_ty_1 {
    |  _^ starting here...
561 | |     pub __wch: ::std::os::raw::c_uint,
562 | |     pub __wchb: [::std::os::raw::c_char; 4usize],
563 | | }
    | |_^ ...ending here
    |
    = help: add #![feature(untagged_unions)] to the crate attributes to enable

error: unions are unstable and possibly buggy (see issue #32836)
    --> /home/bbigras/dev/rust/rs-opencv/target/debug/build/opencv-2abe0d5d07f86d1d/out/bindings.rs:9037:1
     |
9037 |   pub union CvFileNode__bindgen_ty_1 {
     |  _^ starting here...
9038 | |     pub f: f64,
9039 | |     pub i: ::std::os::raw::c_int,
9040 | |     pub str: CvString,
9041 | |     pub seq: *mut CvSeq,
9042 | |     pub map: *mut CvFileNodeHash,
9043 | | }
     | |_^ ...ending here
     |
     = help: add #![feature(untagged_unions)] to the crate attributes to enable

error: unions are unstable and possibly buggy (see issue #32836)
    --> /home/bbigras/dev/rust/rs-opencv/target/debug/build/opencv-2abe0d5d07f86d1d/out/bindings.rs:9177:1
     |
9177 |   pub union CvMat__bindgen_ty_1 {
     |  _^ starting here...
9178 | |     pub ptr: *mut uchar,
9179 | |     pub s: *mut ::std::os::raw::c_short,
9180 | |     pub i: *mut ::std::os::raw::c_int,
9181 | |     pub fl: *mut f32,
9182 | |     pub db: *mut f64,
9183 | | }
     | |_^ ...ending here
     |
     = help: add #![feature(untagged_unions)] to the crate attributes to enable

error: unions are unstable and possibly buggy (see issue #32836)
    --> /home/bbigras/dev/rust/rs-opencv/target/debug/build/opencv-2abe0d5d07f86d1d/out/bindings.rs:9194:1
     |
9194 |   pub union CvMat__bindgen_ty_2 {
     |  _^ starting here...
9195 | |     pub rows: ::std::os::raw::c_int,
9196 | |     pub height: ::std::os::raw::c_int,
9197 | | }
     | |_^ ...ending here
     |
     = help: add #![feature(untagged_unions)] to the crate attributes to enable

error: unions are unstable and possibly buggy (see issue #32836)
    --> /home/bbigras/dev/rust/rs-opencv/target/debug/build/opencv-2abe0d5d07f86d1d/out/bindings.rs:9208:1
     |
9208 |   pub union CvMat__bindgen_ty_3 {
     |  _^ starting here...
9209 | |     pub cols: ::std::os::raw::c_int,
9210 | |     pub width: ::std::os::raw::c_int,
9211 | | }
     | |_^ ...ending here
     |
     = help: add #![feature(untagged_unions)] to the crate attributes to enable

error: unions are unstable and possibly buggy (see issue #32836)
    --> /home/bbigras/dev/rust/rs-opencv/target/debug/build/opencv-2abe0d5d07f86d1d/out/bindings.rs:9272:1
     |
9272 |   pub union CvMatND__bindgen_ty_1 {
     |  _^ starting here...
9273 | |     pub ptr: *mut uchar,
9274 | |     pub fl: *mut f32,
9275 | |     pub db: *mut f64,
9276 | |     pub i: *mut ::std::os::raw::c_int,
9277 | |     pub s: *mut ::std::os::raw::c_short,
9278 | | }
     | |_^ ...ending here
     |
     = help: add #![feature(untagged_unions)] to the crate attributes to enable

error: aborting due to 9 previous errors

error: Could not compile `opencv`.

bindings.zip

@emilio
Copy link
Contributor

emilio commented Dec 15, 2016

So the untagged union thing is that we prefer to generate those if we can given we can support more nasty stuff like template parameters in unions properly. But since it's only nightly rust by default, you can override that with no_unstable_rust.

I'm really surprised about the double std::char_traits thing (I've been able to reproduce, and I'll dig into it).

That being said, I've been able to generate bindings for OpenCv with the following bits:

    let bindings = Builder::default()
        .no_unstable_rust()
        .header("wrapper.h")
        .clang_arg("-x")
        .clang_arg("c++")
        .opaque_type("std::char_traits")
        .enable_cxx_namespaces()
        .no_unstable_rust()
        .whitelisted_function("cv.*")
        .whitelisted_type("Cv.*")
        .whitelisted_type("cv::.*")
        .generate()
        .expect("Unable to generate bindings");

The whitelisting can probably get smarter, and you can cut-off parts of the API you don't want with the opaque/blacklisting APIs.

@emilio
Copy link
Contributor

emilio commented Dec 15, 2016

Oh, forgot to mention that the patches in #346 are needed.

@bbigras
Copy link
Author

bbigras commented Dec 15, 2016

That being said, I've been able to generate bindings for OpenCv with the following bits

Nice, thanks!!

By the way some tests are failing when I run cargo test. Do you want me to paste them here or I guess you have installed opencv if you were able to generate bindings successfully.

@bbigras
Copy link
Author

bbigras commented Dec 15, 2016

Also, are the warnings normal?

warning: found non-foreign-function-safe member in struct marked #[repr(C)]: found struct without foreign-function-safe representation annotation in foreign module, consider adding a #[repr(C)] attribute to the type, #[warn(improper_ctypes)] on by default

@emilio
Copy link
Contributor

emilio commented Dec 15, 2016

Yes, unfortunately stuff that uses PhantomData and similar triggers that kind of warning.

@emilio
Copy link
Contributor

emilio commented Dec 15, 2016

That's rust-lang/rust#34798. Regarding the type size and alignment failures, I hadn't taken a look at them. I'd need to rewrite the crate since I deleted it, but was easy enough last time, so I'll re-do it :)

@emilio
Copy link
Contributor

emilio commented Dec 15, 2016

So the failures are mostly expected.

On one hand, there's a failure with the Scalar type. We can't do much about it since it's defined in terms of number generics:

template<typename _Tp> class Scalar_ : public Vec<_Tp, 4>

The other ones are the std::vector type and std::string on my system. That's because we don't do associated types properly, and we end up getting a byte-sized type instead of a pointer-sized type. We can fix that with better info from libclang.

All the rest are derived from those. There's additionally the std::vector<bool> specialization, that is something that we could indeed special-case as a one-off, but otherwise it's template specialization madness we can't support on rust.

If you find one of these problems, there are multiple ways to work around the problem, depending on needs.

If you don't need to access the members, the easiest thing is marking stuff as opaque. Marking a type as opaque tries to replace that type with a proper replacement in each situation. For example:

.opaque_type("std::vector")

Would fix a bunch of tests, and replace the fields with properly-aligned arrays in rust.

Other possible workaround/fix, for when you need access to the type's field, would be using replacements. For example, I know that std::vector is represented by three words in the STL, one pointer to the initial element, other to the last element, and other to the end of the reserved storage.

I could make the wrapper.h file look as follows:

/**
 * <div rustbindgen replaces="std::vector"></div>
 */
template<typename T, typename Alloc>
class vector_replacement {
  T* start;
  T* end;
  T* storage_end;
};

#include <opencv2/core.hpp>

That will replace the definition of std::vector by the type I just wrote, that bindgen can detect correctly. This wouldn't fix the vector<bool> problem, but since we don't use it, it seems to be a nice solution if we needed access to it.

Combining those configuration options, you can ensure the layout of your types is correct, even when bindgen can't quite do the correct thing.

I'm not sure if I missed something (this should probably be documented in a more visible place). @fitzgen may correct me if I'm wrong.

@fitzgen
Copy link
Member

fitzgen commented Dec 16, 2016

I'm not sure if I missed something (this should probably be documented in a more visible place). @fitzgen may correct me if I'm wrong.

Yeah, I think we need a users' guide. I've been kind of meaning to write one, but (a) I have higher priorities (sorta lame excuse) and (b) we've been overhauling a lot of this stuff frequently (eg "_" => "::" in replacements and whitelisting etc).

I think maybe if we (I? you? :P) make a skeleton of a guide, people can start helping to contribute. But I do think we need the skeleton first to act as a framework for others who want to jump in and help.

@fitzgen
Copy link
Member

fitzgen commented Jul 21, 2017

I think we can close this issue, as it seems we have a users' guide now and there doesn't seem to be much left here that is actionable.

@fitzgen fitzgen closed this as completed Jul 21, 2017
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

3 participants