dexonsmith added a comment. The patch seems mostly straightforward, but can you clarify whether there was a functionality change for clang-scan-deps? My reading of the code suggests not, because it was using ResourceDirectoryCache before and still will... in which case, can you walk me through how the test covers this code change? (Maybe some comments in the test would help.)
In D108366#2954380 <https://reviews.llvm.org/D108366#2954380>, @jansvoboda11 wrote: > Tagging @kousikk, since this is related to D69122 > <https://reviews.llvm.org/D69122> that introduced `ResourceDirectoryCache` to > `clang-scan-deps`. When the compilation command doesn't have a > `-resource-dir` argument, `ResourceDirectoryCache` invokes the specified > compiler with `-print-resource-dir` and injects the result into the > command-line as `-resource-dir`. > > This happens way before the dependency scanner worker is invoked, meaning the > logic this patch tweaks won't usually kick in. The test passes only because > the invocation of `/our/custom/bin/clang -print-resource-dir` made by > `ResourceDirectoryCache` silently fails (the binary doesn't exist), allowing > the worker to deduce the resource directory using regular driver logic. > > I think both `clang-scan-deps` and the downstream libclang API clearly head > towards only supporting compilers built the same way (same version, > architecture, etc.). The modular dependency scanner already returns > command-lines of cc1 arguments that are not stable across Clang versions. > > I wanted to see if we can reach consensus on removing > `ResourceDirectoryCache` entirely. It makes the resource directory deduction > much more lightweight and is in line with the direction we're already going > regarding compiler compatibility. It also allows `clang-scan-deps` and > libclang API to have the same behavior, which is a desirable property IMO. If > users really want the behavior of `ResourceDirectoryCache`, they can keep > using prior versions `clang-scan-deps`. > > What do you all think? Jan and I already talked offline, but FTR, I agree with the direction of expecting the "dependency scanning" APIs to run from a libclang/clang-scan-deps that's installed alongside clang itself. This is required for the dependency scanning to be correct anyway (since different compilers could have different pre-defined macros). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D108366/new/ https://reviews.llvm.org/D108366 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
