[libcxx-commits] [PATCH] D117268: [libc++] Install clang-tidy in docker containers and create compile_commands.json

Mon Jan 17 08:34:52 PST 2022

ldionne added a comment.

In D117268#3246850 <https://reviews.llvm.org/D117268#3246850>, @sammccall wrote:

> In D117268#3244971 <https://reviews.llvm.org/D117268#3244971>, @ldionne wrote:
>
>> @sammccall Do you know whether `clang-tidy` can use `compile_flags.txt`? If it did, that could perhaps solve both this issue and also https://github.com/llvm/llvm-project/issues/45348.
>
> Yes and no:
>
> - if you run clang-tidy on a file, it will correctly discover `compile_flags.txt` (or you can force it with `-p`) and use its flags
> - but if you want to run clang-tidy on a whole project, you need to somehow choose what files are in scope. `run-clang-tidy.py` is the usual tool for this and only supports compile_commands.json.

Awesome, thanks for the information. In our case, we are not even trying to lint the `.cpp` files that `compile_commands.json` contains -- we're trying to lint our headers, and those are not mentioned in `compile_commands.json`.

@philnik I am now fairly convinced that the right path forward is to provide `compile_flags.txt` and to point `clang-tidy` to it in D117174 <https://reviews.llvm.org/D117174> using `-p`.

I'd suggest dropping the `compile_commands.json` part of this change and adding `compile_flags.txt` in D117174 <https://reviews.llvm.org/D117174> (in other words, this patch could only touch the Dockerfile). To begin, I would place `compile_flags.txt` in a place like `libcxx/test/libcxx/`, and as a separate commit, we can move it to the libc++ installation proper so as to fix https://github.com/llvm/llvm-project/issues/45348. The reason for doing that in a separate commit is that I strongly suspect it's going to be harder to land than it seems due to people including `compile_flags.txt`, so it would be best to keep it separate in order to unblock you.

Sorry for the churn, but at last I think this is a much cleaner approach.

P.S.: Based on the GitHub bug report, I think we'll want our `compile_flags.txt` to contain this:

  -stdlib=libc++
  -stdlib++-isystemv1
  -xc++-header

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117268