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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
