ro marked an inline comment as done. ro added a comment. In D157275#4585184 <https://reviews.llvm.org/D157275#4585184>, @MaskRay wrote:
> Thanks for the update. Regarding testing, it seems that unittests will be > more convenience than creating so many placeholder files in `Inputs/`. > `clang/unittests/Driver/ToolChainTest.cpp` `TEST(ToolChainTest, > VFSGCCInstallation)` has an example for Linux. You can add another for > Solaris. TBH, I'm quite sceptical about the unittest framework here: we already have 2300+ empty files in `clang/test/Driver`, so adding 12 more shouldn't be such a big deal. With a real in-filesystem tree, I can quickly test various scenarious with `clang -v --sysroot`, while this isn't possible with the unittest framework (which I find way harder to read). I'm adding the unittest nonetheless, but would prefer to keep the `lit`-based test instead. ================ Comment at: clang/lib/Driver/ToolChains/Gnu.cpp:2258 - Prefixes.push_back(CandidatePrefix); + SolarisPrefixes.push_back(CandidatePrefix); } ---------------- MaskRay wrote: > It's better to use https://en.wikipedia.org/wiki/Schwartzian_transform here > to avoid calling `llvm::sys::path::filename` and > `Generic_GCC::GCCVersion::Parse` in the comparator. Then the comparator is > just `A < B` and can just be removed. This is way shorter and efficient indeed. Patch updated. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D157275/new/ https://reviews.llvm.org/D157275 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits