[PATCH] D157275: [Driver] Select newest GCC installation on Solaris

[Rainer Orth via Phabricator via cfe-commits]

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