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

Reply via email to