-
Notifications
You must be signed in to change notification settings - Fork 12
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
Clang include args still needed on Arch #262
Comments
Thanks for the report. I've come to believe I tried to debug this on an libclang C header search path (via hawkmoth)
clang C header search path
libclang C++ header search path (via hawkmoth)
clang C++ header search path
ErrorsBoth C and C++ preprocess fail to find the C headers through
It's just really odd that EDIT: Since the files are there under |
Thanks. To reproduce without hawkmoth:
So the python version uses a pwd-relative path when it should be anchored at @foutrelis Any idea? |
@heftig Thanks for creating the reproducer without hawkmoth. I looked at what the other distros have. It's actually not as rosy as I believed it to be: Debian/Ubuntu
Alpine
Fedora
|
Debian applies this patch, which looks relevant: https://salsa.debian.org/pkg-llvm-team/llvm-toolchain/-/blob/18/debian/patches/fix-clang-path-and-build.diff |
I wonder if there's a libclang upstream bug about this. Or if this is about some downstream build configuration? |
I blame the arch using reviewer 😛 Anyway, if Fedora also has this issue... Maybe it is worthwhile reverting this ourselves for now and avoid the pain for unwitting packagers? What do you think @jnikula? It may also help users understanding how to fix their projects if they have our example to go by.
I think you found it already about a month ago :) Linking it here for convenience: llvm/llvm-project#18150. -->8-- No real surprises here, but I did do a quick check to see that this is indeed libclang's fault and not the bindings'. I also did this before looking for a bug upstream and, following the code around, I found the same difference between clang and libclang in deducing the header path as the ticket author. #include <clang-c/Index.h>
#include <iostream>
int main(){
CXIndex index = clang_createIndex(0, 0);
const char *args[] = {"-x", "c", "-E", "-Wp,-v"};
CXTranslationUnit unit =
clang_parseTranslationUnit(index, "/dev/null",
args, 4, nullptr, 0,
CXTranslationUnit_None);
if (unit == nullptr) {
std::cerr << "Unable to parse translation unit. Quitting.\n";
return 1;
}
for (unsigned i = 0; i < clang_getNumDiagnostics(unit); ++i) {
auto diag = clang_getDiagnostic(unit, i);
auto string = clang_getDiagnosticSpelling(diag);
std::cout << clang_getCString(string) << std::endl;
}
return 0;
}
|
I think the approach with |
Maybe I misunderstood something then. Seems like Kind of related, I've wondered before if a better default would actually be |
My point is, we don't really specify which libclang to use. We don't have any discovery of our own for that. The user may specify it through a few different ways, and the Python bindings do the rest. How are we supposed to know which Maybe we should indeed use something like |
Oh, I also don't think there's any requirement that the |
This reverts the effects of commit 1243852, while fixing the inconsistent handling of C/C++ domains since libclang can't always figure out the right search paths for its own version of Clang despite prior claims. It is still unclear whether the upstream issue is an actual issue or if this is the intended behaviour (llvm/llvm-project#18150), though it _is_ an issue and simply using the fixed helper function and configuration parameter comes at a suitably low cost to maintenance. Fixes jnikula#262.
This reverts the effects of commit 1243852, while fixing the inconsistent handling of C/C++ domains since libclang can't always figure out the right search paths for its own version of Clang despite prior claims. It is still unclear whether the upstream issue is an actual issue or if this is the intended behaviour (llvm/llvm-project#18150), though it _is_ an issue and simply using the fixed helper function and configuration parameter comes at a suitably low cost to maintenance. Fixes jnikula#262.
This reverts the effects of commit 1243852, while fixing the inconsistent handling of C/C++ domains since libclang can't always figure out the right search paths for its own version of Clang despite prior claims. It is still unclear whether the upstream issue is an actual issue or if this is the intended behaviour (llvm/llvm-project#18150), though it _is_ an issue and simply using the fixed helper function and configuration parameter comes at a suitably low cost to maintenance. Fixes jnikula#262.
This reverts the effects of commit 1243852, while fixing the inconsistent handling of C/C++ domains since libclang can't always figure out the right search paths for its own version of Clang despite prior claims. It is still unclear whether the upstream issue is an actual issue or if this is the intended behaviour (llvm/llvm-project#18150), though it _is_ an issue and simply using the fixed helper function and configuration parameter comes at a suitably low cost to maintenance. Fixes jnikula#262.
This reverts the effects of commit 1243852, while fixing the inconsistent handling of C/C++ domains since libclang can't always figure out the right search paths for its own version of Clang despite prior claims. It is still unclear whether the upstream issue is an actual issue or if this is the intended behaviour (llvm/llvm-project#18150), though it _is_ an issue and simply using the fixed helper function and configuration parameter comes at a suitably low cost to maintenance. Fixes jnikula#262.
I had to revert 1243852 in order to get the test suite to pass again for the Arch Linux package build. Otherwise the tests would fail to find include files like
stddef.h
andstdbool.h
.Is this an issue with our Clang?
The text was updated successfully, but these errors were encountered: