[PATCH] D35755: [Solaris] gcc toolchain handling revamp

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Aside from a coding style nit and the unanswered question that hopefully @tstellar can help answer, this LGTM. I'll wait to accept until we figure out the answer for Linux, however.



================
Comment at: lib/Driver/ToolChains/Solaris.cpp:208
   if (GCCInstallation.isValid()) {
-    GCCVersion Version = GCCInstallation.getVersion();
-    addSystemInclude(DriverArgs, CC1Args,
-                     getDriver().SysRoot + "/usr/gcc/" +
-                     Version.MajorStr + "." +
-                     Version.MinorStr +
-                     "/include/c++/" + Version.Text);
-    addSystemInclude(DriverArgs, CC1Args,
-                     getDriver().SysRoot + "/usr/gcc/" + Version.MajorStr +
-                     "." + Version.MinorStr + "/include/c++/" +
-                     Version.Text + "/" +
-                     GCCInstallation.getTriple().str());
+    const auto &Callback = Multilibs.includeDirsCallback();
+    if (Callback) {
----------------
fedor.sergeev wrote:
> aaron.ballman wrote:
> > Shouldn't use `auto` here because the type is not spelled out in the initialization.
> I believe this is the case described in LLVM coding standard as "when the type would have been abstracted away anyways". It would be MultilibSet::IncludeDirsFunc, which IMO does not add anything to readability.
I've always taken that to refer to things like range-based for loop variables and functions that return iterators; not functions that return concrete types. I prefer to see the explicit type in this case so that I am able to better understand the arguments to be passed to the function. When it's abstracted behind `auto`, I have to work harder to find the information I need compared to when the type is explicitly spelled out.


https://reviews.llvm.org/D35755