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

select cpp or c search paths based on clang args #1351

Merged
merged 1 commit into from
Jul 30, 2018

Conversation

Imirsen
Copy link

@Imirsen Imirsen commented Jul 26, 2018

No description provided.

@Imirsen
Copy link
Author

Imirsen commented Jul 26, 2018

I was having troubling using bindgen on a simple C project that included stddef.h. Initially worked around by manually specify the path for Clang system headers files to bindgen in the builder args. Dug a little deeper and realized that for C projects, bindgen still targets the CPP search paths when finding paths to pass to the driver. I propose looking at the clang args as was already done in a debugging function to determine whether the project is C, or C++, and then selecting the search paths based on that.

Copy link
Contributor

@emilio emilio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great, thanks very much!

Is it hard to construct a test-case for this? I guess it depends on the environment...

@@ -1633,8 +1637,17 @@ impl Bindings {
if !has_target_arg {
// TODO: distinguish C and C++ paths? C++'s should be enough, I
// guess.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove the TODO?

@emilio
Copy link
Contributor

emilio commented Jul 30, 2018

I'll merge and remove the todo in a followup. Thanks again!

@emilio emilio merged commit 0f9b272 into rust-lang:master Jul 30, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants