[PATCH] D69122: Add support to find out resource dir and add it as compilation args

[Michael Spencer via Phabricator via cfe-commits]

Bigcheese requested changes to this revision.
Bigcheese added a comment.
This revision now requires changes to proceed.

So I agree solving this problem makes sense, but I have some issues with the current patch.

- This defaults to doing the lookup. The test suite, and some real outputs, just uses `"clang ..."` as the command, not an absolute path. If we always do the lookup then this would be whatever `clang` is in `PATH`, not the `clang` we're testing.
- While looking into the above, I think I discovered that the way `FindResourceDir ` constructs `ClangBinaryPath` is broken. It uses the `"directory"` entry from the compilation database, which is the working directory of where the compile occurred. It should be extremely rare for this ever to contain a `clang`, and even rarer for it to be the one that was intended.
- This touches a lot of `clang/lib/Tooling` that I don't think it needs to.

For the first issue we may just want to fix the tests to point to a specific `clang`, it's annoying, but doable. An alternative would be to //only// do `FindResourceDir` when given an absolute path to a compiler, instead of trying to guess. I think I prefer this approach, as if you have an installed `clang` in your path, there should be a `clang-scan-deps` next to it, which would use the same resource dir.

For the second and third issues, I think you can just remove the `StringRef Directory` arg and use `FindInEnvPath` to get the correct `clang`.

You shouldn't need it anymore, but you can replace `MakeAbsolutePath` with `llvm::sys::path::make_absolute`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69122