-
Notifications
You must be signed in to change notification settings - Fork 738
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
Add --cxx
flag
#2269
Add --cxx
flag
#2269
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
☔ The latest upstream changes (presumably 4b006da) made this pull request unmergeable. Please resolve the merge conflicts. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not particularly opposed to this, but how does this address #1456?
In that issue, the user was using the Builder
API to use c++
directly. I think to address that we should probably either adjust the docs to provide an example that passes the two arguments separately, or maybe just add an add_cxx_flags
API there that effectively does clang_args(["-x", "c++"])
. But honestly I think the example would be more than enough.
I'd say that if someone asks for c++ support and the answer is
There is less chance for error than with
regardless of what documentation says. I don't have super strong opinions about this feature so I'm OK with not merging this. |
Not really, |
Yeah we agree on that. My point is that using clang args is more error prone while regular args are not as there is no way to set arbitrary arguments for bindgen but we can add the invalid |
Right, but the original issue was about the library usage, so this patch doesn't change anything there afaict. |
this also includes a |
☔ The latest upstream changes (presumably #2284) made this pull request unmergeable. Please resolve the merge conflicts. |
☔ The latest upstream changes (presumably b3ac3ef) made this pull request unmergeable. Please resolve the merge conflicts. |
Addresses #1456