[PATCH] D63092: [Frontend] Use executable path when creating invocation from cmdline

Petr Hosek via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Jun 10 14:37:15 PDT 2019


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





More information about the cfe-commits mailing list