[clang-tools-extra] [clangd] Forward --target to system include extraction (PR #65824)

Nathan Ridge via cfe-commits cfe-commits at lists.llvm.org
Sun Sep 10 01:00:40 PDT 2023


https://github.com/HighCommander4 requested changes to this pull request.

Thanks for the patch!

There is some prior work on this which I've looked over and will summarize:

 * This change was previously proposed in [D138546](https://reviews.llvm.org/D138546)
 * During its review, we discovered that clangd would run a processing step on the compile command that sometimes _adds_ a `-target` flag, inferred from the compiler name, to the command, _before_ running `SystemIncludeExtractor`, and propagating `-target` to the extraction command then caused an error if the compiler was a gcc (since gcc does not support `-target`).
   * The patch was updated to fix this (by moving that processing step to happen _after_ `SystemIncludeExtractor`). That change was since independently made and merged in [D143436](https://reviews.llvm.org/D143436).
 * We also discovered that `SystemIncludeExtractor` was failing to include arguments like `--sysroot` in the key under which it caches its results.
   * That has since been fixed in [D146941](https://reviews.llvm.org/D146941).

Activity on [D138546](https://reviews.llvm.org/D138546) has since stalled, but the next step would have been to rebase it on top of [D143436](https://reviews.llvm.org/D143436) and [D146941](https://reviews.llvm.org/D146941)... which is basically what this patch does!

So, I think this patch should be good to go (modulo a small bug I commented on inline).

cc @cpsauer (author of [D138546](https://reviews.llvm.org/D138546)) as an FYI

https://github.com/llvm/llvm-project/pull/65824


More information about the cfe-commits mailing list