aaron.ballman added inline comments.
================
Comment at: lib/Driver/ToolChains/Gnu.cpp:1511
+ StringRef Suff64 = "/64";
+ // Solaris uses platform-specific suffixes instead of /64
+ if (TargetTriple.getOS() == llvm::Triple::Solaris) {
----------------
Add a period at the end of the comment.
================
Comment at: lib/Driver/ToolChains/Gnu.cpp:1732
- // Then look for distribution supplied gcc installations.
+ // Now prefix(es) that correspond to distribution-supplied gcc
installations
if (D.SysRoot.empty()) {
----------------
s/Now/Next, look for
Add a period at the end of the comment.
================
Comment at: lib/Driver/ToolChains/Gnu.cpp:1734
if (D.SysRoot.empty()) {
- // Look for RHEL devtoolsets.
- Prefixes.push_back("/opt/rh/devtoolset-6/root/usr");
- Prefixes.push_back("/opt/rh/devtoolset-4/root/usr");
- Prefixes.push_back("/opt/rh/devtoolset-3/root/usr");
- Prefixes.push_back("/opt/rh/devtoolset-2/root/usr");
- // And finally in /usr.
- Prefixes.push_back("/usr");
+ // typically /usr
+ AddDefaultGCCPrefixes(TargetTriple, Prefixes, D.SysRoot);
----------------
s/typically/Typically
Add a period at the end of the comment.
================
Comment at: lib/Driver/ToolChains/Gnu.cpp:1807
+void Generic_GCC::GCCInstallationDetector::AddDefaultGCCPrefixes(
+ const llvm::Triple &TargetTriple, SmallVectorImpl<std::string> &Prefixes,
+ StringRef SysRoot) {
----------------
I'd drop the `llvm::` here.
================
Comment at: lib/Driver/ToolChains/Gnu.cpp:1813-1814
+ // /usr/gcc/<major>.<minor>/lib/gcc/<triple>/<major>.<minor>.<patch>/
+ // so we need to find those /usr/gcc/*/lib/gcc libdirs
+ // and go with /usr/gcc/<version> as a prefix
+
----------------
This comment can probably be re-flowed, and needs a period at the end of it.
================
Comment at: lib/Driver/ToolChains/Gnu.cpp:1837
+
+ // non-Solaris is much simpler - most systems just go with "/usr"
+ if (SysRoot.empty()) {
----------------
s/non/Non
Needs a period at the end of the comment.
================
Comment at: lib/Driver/ToolChains/Gnu.cpp:1840
+ // Yet, still look for RHEL devtoolsets
+ // (should it be done Linux-only??)
+ Prefixes.push_back("/opt/rh/devtoolset-6/root/usr");
----------------
This is a good question to get an answer to before committing. :-) I don't know
the answer myself, unfortunately.
================
Comment at: lib/Driver/ToolChains/Gnu.h:253
+ void AddDefaultGCCPrefixes(const llvm::Triple &TargetTriple,
+ SmallVectorImpl<std::string> &Prefixes,
----------------
Might as well drop the `llvm::` since the namespace isn't used for
`SmallVectorImpl`.
================
Comment at: lib/Driver/ToolChains/Gnu.h:309
+ // FIXME: This should be final, but CrossWindows toolchain does weird
+ // things that cant be easily generalized
void AddClangCXXStdlibIncludeArgs(
----------------
s/but CrossWindows/but the CrossWindows
s/cant/can't
Add a period at the end of the comment.
================
Comment at: lib/Driver/ToolChains/Solaris.cpp:151
+ if (GCCInstallation.isValid()) {
+ // On Solaris gcc uses both architecture-specific path with triple in it
+ // as well as more generic lib path (+arch suffix)
----------------
s/both architecture/both an architecture
================
Comment at: lib/Driver/ToolChains/Solaris.cpp:152
+ // On Solaris gcc uses both architecture-specific path with triple in it
+ // as well as more generic lib path (+arch suffix)
+ addPathIfExists(D,
----------------
s/as more/as a more
Add a period at the end of the comment.
================
Comment at: lib/Driver/ToolChains/Solaris.cpp:160
- addPathIfExists(D, getDriver().SysRoot + LibPath, Paths);
+ // if we are currently running Clang inside of the requested system root,
+ // add its parent library path to those searched.
----------------
s/if/If
================
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) {
----------------
Shouldn't use `auto` here because the type is not spelled out in the
initialization.
https://reviews.llvm.org/D35755
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
