phosek added a comment.

In D63092#1536774 <https://reviews.llvm.org/D63092#1536774>, @sammccall wrote:

> `argv[0]` does carry important information though, I think this will break a 
> lot of things. It's... concerning that no tests broke.
>
> For example, if it's `clang` or `g++` or `clang-cl` then that affects how 
> command lines are parsed (regardless of whether the path actually exists).


You're right, I can see how this change can break the driver mode detection.

> Additionally I don't think all tools that call 
> createInvocationFromCommandLine carry around all the files you want.
> 
> The wrapper tool case is a problem though, I see some options:
> 
> - change the process that generates the compilation database to emit a path 
> to the underlying compiler instead, as tooling currently expects. This may be 
> painful if e.g. goma pretends to be a regular toolchain. An argument can be 
> made it should pretend harder (e.g. symlinking directories around it so 
> discovery works as usual)

I think this is the right solution for compiler wrappers like goma, the problem 
is that this requires changing any tool that generates compilation database to 
have explicit notion of compiler launcher and exclude that from the command 
(today we just prepend the tool to the command 
<https://chromium.googlesource.com/chromium/src/+/refs/heads/master/build/toolchain/gcc_toolchain.gni#171>).
 This includes tool like GN and potentially even Ninja which is a non-trivial 
task. Until that happens, `clangd` is unusable for projects like Fuchsia 
(`clangd` cannot find standard headers and we're getting too many warnings for 
those).

> - change the process that generates the CDB to inject the relevant directory 
> paths explicitly as flags

This would mean either duplicating the Clang driver logic inside the build 
system, which is something I'd like to avoid (why use the driver at all in that 
case?), or have some mechanism through which the driver would have to export 
all the necessary information (e.g. where to find standard library headers) for 
the build system to use.

> - make driver aware of the distinction between "invoked-as" and "current 
> binary" and use the right path in the right places

I'll look into this.

> (Incidentally if Driver is going to use the current binary, we shouldn't need 
> to pass it in as a parameter, right?)
> 
> There are some potentially-unsolvable cases here: e.g. if your wrapper is 
> `magic-remote-build` then maybe there's no standard library locally, or no 
> way to find it.

Do you think it's worth updating the compilation database documentation 
<https://clang.llvm.org/docs/JSONCompilationDatabase.html> to explicitly say 
that the command has to start with the compiler invocation? Currently it says 
that "The compile command executed." which is IMHO not sufficiently precise.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63092/new/

https://reviews.llvm.org/D63092



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to