mgorny requested changes to this revision.
mgorny added inline comments.
This revision now requires changes to proceed.
================
Comment at: lib/Driver/ToolChains/Gnu.cpp:2284
+ // Test the path based on the version in /etc/env.d/gcc/config-{tuple}.
+ GentooScanPaths.push_back(SysRootPrefix + "/usr/lib/gcc/" +
+ ActiveVersion.first.str() + "/" +
----------------
I'm not sure if there's a point in keeping this if you actually parse the
config file. I can't think of a really valid case when GCC would be installed
without LDPATH in the config file. Not saying it's wrong. You may want at least
to push it after the configfile paths though.
================
Comment at: lib/Driver/ToolChains/Gnu.cpp:2287
+ ActiveVersion.second.str());
+ // Scan the Config file to find installed GCC libaries path.
+ // A typical content of gcc config file:
----------------
Typo: 'libaries'
================
Comment at: lib/Driver/ToolChains/Gnu.cpp:2296
+ if (ConfigFile) {
+ SmallVector<StringRef, 2> ConfigLines;
+ ConfigFile.get()->getBuffer().split(ConfigLines, "\n");
----------------
Misindent.
================
Comment at: lib/Driver/ToolChains/Gnu.cpp:2300
+ ConfLine = ConfLine.trim();
+ if (ConfLine.consume_front("LDPATH=\"") &&
+ ConfLine.consume_back("\"")) {
----------------
This probably doesn't harm but I'd prefer if quotes weren't considered an
obligatory part of the string. I'd rather grab it by `LDPATH=` and strip quotes
afterwards if present on both sides. But I won't insist on this.
================
Comment at: lib/Driver/ToolChains/Gnu.cpp:2303
+ std::pair<StringRef, StringRef> GentooLibs = ConfLine.split(':');
+ GentooScanPaths.push_back(GentooLibs.first.str());
+ if (!GentooLibs.second.empty()) {
----------------
Here you seem to assume that there would be at most 2 paths. That's a wrong
assumption — there are triple-ABI targets (e.g. amd64 with x32 variant), and
there is no reason prohibiting more LDPATHs. So please use the 'full' split,
and iterate over all paths.
Repository:
rC Clang
https://reviews.llvm.org/D45233
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
