[PATCH] D138546: Clangd: Preserve target flags in system includes extractor

Tue Dec 27 23:07:21 PST 2022

nridge added a comment.

In D138546#4017356 <https://reviews.llvm.org/D138546#4017356>, @ArcsinX wrote:

> In other words, with this patch we preserve `--target=...`, but this option can appear from https://github.com/llvm/llvm-project/blob/main/clang-tools-extra/clangd/GlobalCompilationDatabase.cpp#L253

Huh, I had no idea we had logic to add a `--target` flag there. (Nor that gcc does not recognize this option.)

@cpsauer just to double-check, in your case the `--target` flag is explicitly written in the `compile_commands.json`, and the idea is that in such case we want to pass it to the system include extractor (since it's safe to assume the compiler supports the flag in that case), but not in the case where we add it?

In D138546#4018119 <https://reviews.llvm.org/D138546#4018119>, @cpsauer wrote:

> probably we should be relayering things to operate on raw commands for query driver?

Not quite **raw** commands: the fix for https://github.com/clangd/clangd/issues/1173 was to make sure the inferred language (e.g. `-x c++`) is added before query-driver, and the fix for https://github.com/clangd/clangd/issues/1089 was to make sure that edits from the config file are applied before query-driver (because `Compiler` can be overridden there).

But I think we could achieve the intended effect by removing the call to `inferTargetAndDriverMode()` from `GlobalCompilationDatabase.cpp`, and instead performing the equivalent logic in `CommandMangler`, //after// invoking the `SystemIncludeExtractor`?

Any thoughts on whether that would be reasonable / whether it would break anything else, are welcome.

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

https://reviews.llvm.org/D138546