[PATCH] D154903: clangd: Provide the resource dir via environment variable
madscientist added a comment. 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: 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. - If the resource-dir were to become built in then I would imagine that this variable would be changed to either be empty or (preferable) contain a special token such as "builtin" so that the compiler driver could know this. How this would or could work would depend greatly on exactly how the built-in facilities were implemented, but if for example all that's needed is to avoid adding GCC's intrinsics headers to the list, then the compiler driver that saw a `CLANGD_RESOURCE_DIR` value of `builtin` (for example) would remove the GCC intrinsics directory completely, rather than replacing it with the clangd resource directory. 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? 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. 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. 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. 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
[PATCH] D156044: [clangd] Exclude builtin headers from system include extraction
madscientist added a comment. I built latest main with this patch applied and it does fix the issues in my environment. My headers that include xmmintrin.h etc. don't throw errors in clangd. Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156044/new/ https://reviews.llvm.org/D156044 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D156044: [clangd] Exclude builtin headers from system include extraction
madscientist added a comment. (just a note that another way to do this check is to look for some common intrinsic file in each directory generated by the compiler driver, and remove any directory containing those files. But, if the `-print-file-name=include` trick works reliably that's good too). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156044/new/ https://reviews.llvm.org/D156044 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D154903: clangd: Provide the resource dir via environment variable
madscientist created this revision.
Herald added subscribers: kadircet, arphaman.
Herald added a project: All.
madscientist published this revision for review.
madscientist added a comment.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang-tools-extra.
Ready for review please see https://github.com/clangd/clangd/issues/1695
Set an environment variable CLANGD_RESOURCE_DIR to the path of the Clang
resource directory, so that the compiler driver can (a) know it's being
invoked from clangd and (b) massage its system include paths to use the
Clang resources if it wishes to.
This addresses clangd/clangd#1695
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D154903
Files:
clang-tools-extra/clangd/tool/ClangdMain.cpp
Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -982,6 +982,22 @@
Opts.Encoding = ForceOffsetEncoding;
setIncludeCleanerAnalyzesStdlib(IncludeCleanerStdlib);
+ constexpr const char *ResourceEnvVar = "CLANGD_RESOURCE_DIR";
+ std::string Resources;
+ if (!::getenv(ResourceEnvVar)) {
+if (Opts.ResourceDir)
+ Resources = *Opts.ResourceDir;
+if (Resources.empty()) {
+ static int StaticForMainAddr;
+ Resources = CompilerInvocation::GetResourcesPath(
+ "clangd", (void *)&StaticForMainAddr);
+}
+if (!Resources.empty()) {
+ ::setenv(ResourceEnvVar, Resources.c_str(), 1);
+ log("Setting {0} to \"{1}\"", ResourceEnvVar, Resources);
+}
+ }
+
if (CheckFile.getNumOccurrences()) {
llvm::SmallString<256> Path;
if (auto Error =
Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -982,6 +982,22 @@
Opts.Encoding = ForceOffsetEncoding;
setIncludeCleanerAnalyzesStdlib(IncludeCleanerStdlib);
+ constexpr const char *ResourceEnvVar = "CLANGD_RESOURCE_DIR";
+ std::string Resources;
+ if (!::getenv(ResourceEnvVar)) {
+if (Opts.ResourceDir)
+ Resources = *Opts.ResourceDir;
+if (Resources.empty()) {
+ static int StaticForMainAddr;
+ Resources = CompilerInvocation::GetResourcesPath(
+ "clangd", (void *)&StaticForMainAddr);
+}
+if (!Resources.empty()) {
+ ::setenv(ResourceEnvVar, Resources.c_str(), 1);
+ log("Setting {0} to \"{1}\"", ResourceEnvVar, Resources);
+}
+ }
+
if (CheckFile.getNumOccurrences()) {
llvm::SmallString<256> Path;
if (auto Error =
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D154903: clangd: Provide the resource dir via environment variable
madscientist added a comment. Thanks for adding Sam. I tried to do this but failed: his Phabricator handle isn't available in CODE_OWNERS.txt and my attempts to add him via his email address failed. I have no Phabricator fu! 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
