sammccall added a comment. TL;DR: can you try just excluding GCC's resource dir, rather than replacing it with clang's? I think that might solve your problem, and is probably something we should be doing automatically.
In D154903#4525329 <https://reviews.llvm.org/D154903#4525329>, @madscientist wrote: > Sorry for the delay in replying. I'll try to respond to the various points > you made: > > - I don't quite understand the first bullet, "more ways to initialize the > resource dir". As far as I can see there's only one place it's set. If my > change is using a "non-official" alternative to obtaining the resource dir > then certainly I would be happy to change my code to fix that The resource dir for a parse can be set in the following ways AFAIK, some of which are hopefully unreachable: - it can be set by passing -resource-dir to clangd, which gets propagated into the compile flags by CommandMangler - CommandMangler::detect() can choose it - the compile flags from the CDB can contain -resource-dir, overriding CommandMangler - the clang driver that we invoke as a library can detect it (hopefully never happens as a production CommandMangler always ensures -resource-dir is set) - it could be left unset if the clang driver fails to detect it (ditto) I'm not saying you're injecting it into the wrong place, I'm just saying adding a sixth path has a cost. > really the goal of my patch is to ensure that there is one single way to find > this directory and this is //preserved// by telling the compiler driver > explicitly where it is, rather than having multiple places attempting to > figure it out for themselves. As far as the driver is concerned, that's already the case before this patch: we endeavour to always pass `-resource-dir` to the driver. This patch attempts to sync that setting to/from another place, and doesn't entirely succeed (in the third case the resource dir used for parsing does not match the environment variable). > If using the built-in headers from one compiler with a different compiler is > fated to break in mysterious ways, does that mean you feel it's not useful to > try to use clangd in any project which doesn't use clang as its compiler? Not at all, but clangd should use clangd's built-in headers even if you're parsing a project that builds with another compiler. The built-in headers expose a (fairly) standard interface that code is expected to depend on, and their implementation is tightly coupled to the parser they ship with. For example, Clang's `<tgmath.h>` uses `__attribute__((__overloadable__))`, which GCC doesn't support, while gcc's `<tgmath.h>` uses `__builtin_tgmath`, which Clang doesn't support. These are used to implement e.g. generic `acosh()`, which is a public interface code can rely on. This is the difference between the built-in headers and the (rest of the) standard library: stdlibs generally try to be mostly portable between compilers and versions, the built-in headers are coupled. (In principle it's just as incorrect to try to use e.g. clang-12's builtin headers with clangd-13 - these really don't try to be portable). > I hope that this is not the case since (although the idea has been raised on > their mailing lists) there is not much work going on in GCC to create a > GCC-based LSP server. Although I do agree that there are no guarantees, in > reality clangd works very well in my GCC-based project if I replace the GCC > intrinsics directory with the clangd resource directory when the compiler > driver generates its list of built-in locations. Ahh, I finally think I understand what you're trying to do here... is this right? - you have a project that builds with GCC, using GCC's include path - you want to reuse that config with clangd, so you're using --query-driver to extract the include path list - the extracted list includes GCC's built-in headers, and these get inserted onto clangd's regular include path - now GCC's built-in headers shadow clangd's, so we have the "doesn't work" scenario described above - so you want to rewrite the list GCC's driver reports, replacing GCC's built-in headers with clangd's - and for that you need clangd's resource dir That would work but it's more than you need to do: clangd's resource dir is still on the search path (at the end), so if you simply **remove** GCC's built-in headers from the list then they'll stop being shadowed. Can you test if this works? This seems like a design oversight in query-driver. It should probably be doing this filtering by default, if there's a reasonable way to detect which entries are builtin includes. clang supports `-nobuiltininc` but GCC does not. On the other hand `clang --print-file-name=include` gives the built-in headers path, and the same works with GCC. Maybe we can invoke that separately and exclude it. > Regarding alternative solutions: > > - My environment is cross-platform between GNU/Linux (with various > distributions and on various platforms such as x86_64, arm64, and power9 and > power10), MacOS on both x86_64 and m1/m2, and Windows (10+) on x86_64. So > relying on things like /proc/$PPID/exe (or even bash!) is not ideal. > - Your solution requires me to know implicitly where (compared to the clangd > binary) the resource directory lives; that seems less reliable than having > clangd tell me. I don't quite understand the reassurance that these > implementation details are stable enough to assume to be "fact", compared > with the concern in the first bullet above about how they might change. > > Your solution also assumes there is only one version of clang installed on > the system; in fact my system has multiple versions so > `.../lib/clang/*/include` expands to multiple paths which parses incorrectly > (clangd assumes one directory per line). Of course I could have my compiler > driver run `clangd -version` and parse the output to find the right version; > my resource directory is `/usr/lib/clang/16` but the version is `16.0.0`... > it is do-able but it just seems increasingly hairy. > > At the moment I don't have a different compiler driver for clangd than the > standard compiler wrapper script that is provided via `compile_commands.json` > although of course I could create one and change my `.clangd` to point to it. > I was going to simply modify my compiler wrapper to DTRT if it discovered it > was being invoked with `-v` and `CLANGD_RESOURCE_DIR` was set. This all makes sense, working around this outside clangd is indeed going to get messy, especially when cross-platform. For general unsupportable messing-with-resource-dir stuff this might still be where we'd end up, but it sounds like we're actually talking about a problem that should be fixed for all users. > Also, on one of my systems clangd is apparently configured to use > posix_spawn() to invoke the compiler driver, which does not work with shell > scripts. But that just seems like a bug somewhere. I get: > > E[17:23:24.164] System include extraction: driver execution failed with > return code: -1 - 'posix_spawn failed: Exec format error'. Args: > [/home/pds/src/clangd/bin/qdriver.sh -E -x c++ - -v] > > although running that command from the shell prompt works fine. Yikes, from a cursory look it seems glibc posix_spawn is a bit of a mess... Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154903/new/ https://reviews.llvm.org/D154903 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
