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

Old clang version leads to errors which a hard to track down #528

Closed
themasch opened this issue Feb 18, 2017 · 8 comments
Closed

Old clang version leads to errors which a hard to track down #528

themasch opened this issue Feb 18, 2017 · 8 comments

Comments

@themasch
Copy link

I tried to build pq-sys but only had clang-3.7 installed. The result was some weird issues with name mangling resulting in #[link_ name(...)] lines that contain a wrong(?) name.

After updating to llvm/clang 3.9 it works fine.

Would it make sense to show a warning when running with an old clang?

The current README.md states that 3.9 is "recommended" but older version should work "with some features disabled". And someone who uses for example pq-sys or diesel doesn't always read this.

Input C/C++ Header

Not exactly minimum, sorry:
https://gist.github.com/anonymous/d4a7e1bfd5e4e7b908d37e1f0f5dbca4

Bindgen Invokation

    let out_dir = env::var("OUT_DIR").unwrap();
    let mut builder = Builder::default()
        .no_unstable_rust()
        .header("wrapper.h");

    if let Some(path) = pg_config_output("--includedir") {
        builder = builder.clang_arg(format!("-I{}", path));
    }

    builder.generate()
        .expect("Unable to generate bindings for libpq")
        .write_to_file(PathBuf::from(out_dir).join("bindings.rs"))
        .expect("Unable to write bindings to file");```

Actual Results

created binding.rs:
https://gist.github.com/anonymous/665c7a87777af7168553a3d59e10bc3a#file-bindings-rs-L2082

error message:
https://gist.github.com/themasch/41aab23df9d57a578c26a1a501f7d9dc

Expected Results

binding.rs created with:
https://gist.github.com/anonymous/259f4d566c2703d7a4bcd827f2497340

@sgrif
Copy link

sgrif commented Feb 18, 2017

Just to tack onto this, we've been getting a ton of bug reports since we switched to use the recommended method of using build.rs to generate the bindings. Here's another example: sgrif/pq-sys#9 (comment)

It seems like the main reason for requiring bleeding edge clang is for C++ features. However, most people will not have something that up to date out of the box, and it's unfortunate that it's imposing restrictions/causing bugs on the more common case of a simple C header.

@emilio
Copy link
Contributor

emilio commented Feb 18, 2017

Hmm, this is extremely weird, so the wrong link names are C++-mangled names.

Those are just the names that clang gives us, which seems to be wrong in older versions of libclang?

I don't have a libclang 3.7 installation right now, though I can try to find something to test along the week. If you could test some reduced version of it and find a more reduced testcase, that'd help I guess.

Meanwhile, we can probably try to add an option to distrust Clang's mangling, which would work for C libraries. Would that help?

@emilio
Copy link
Contributor

emilio commented Feb 19, 2017

I'm landing an option to avoid clang mangling in #532

bors-servo pushed a commit that referenced this issue Feb 23, 2017
@valpackett
Copy link

Just encountered the same issue because 3.7 is the default clang on FreeBSD 11.0. I have all versions up to and including 4.0 installed though (as clang39, clang40 etc.) Is there any way bindgen could pick up the additionally installed clangs?

@emilio
Copy link
Contributor

emilio commented Feb 28, 2017

You should be able to use the LIBCLANG_PATH in order to be able to select the one that you want.

@cholcombe973
Copy link

I hit the same issue with 3.6 on Ubuntu 16.04.2

@fitzgen
Copy link
Member

fitzgen commented Jul 21, 2017

Note that libclang 3.8 is really our minimum supported libclang. Older versions may sort of work, but we aren't testing them in CI (and don't have plans to).


@emilio Is there anything left to do here or can we close this issue?

@emilio
Copy link
Contributor

emilio commented Aug 11, 2017

Yeah, I think we can close this.

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

No branches or pull requests

6 participants