[cfe-commits] r143869 - in /cfe/trunk/lib/Driver: ToolChains.cpp ToolChains.h

David Blaikie dblaikie at gmail.com
Sun Nov 6 08:07:08 PST 2011


> -static void addPathIfExists(const std::string &Path,
> -                            ToolChain::path_list &Paths) {
> -  if (llvm::sys::fs::exists(Path)) Paths.push_back(Path);
> +static void addPathIfExists(Twine Path, ToolChain::path_list &Paths) {
> +  if (llvm::sys::fs::exists(Path)) Paths.push_back(Path.str());
>  }

pedantic efficiency: This would probably be slightly better (skip some
copying of temporaries) if you could write the Twine out into a
std::string (eg: resize the vector by one then Path.str(Paths.back())
- though that API doesn't exist in Twine. Could be added but no big
deal.

>  /// \brief Get our best guess at the multiarch triple for a target.
> @@ -1823,8 +1822,8 @@
>   // OpenSuse stores the linker with the compiler, add that to the search
>   // path.
>   ToolChain::path_list &PPaths = getProgramPaths();
> -  PPaths.push_back(GCCInstallation.getParentLibPath() + "/../" +
> -                   GCCInstallation.getTriple() + "/bin");
> +  PPaths.push_back(Twine(GCCInstallation.getParentLibPath() + "/../" +
> +                         GCCInstallation.getTriple() + "/bin").str());

Just having () around this expression should be enough:

PPaths.push_back((GCCInstallation.getParentLibPath() + "/../" +
                       GCCInstallation.getTriple() + "/bin").str())

Since StringRef string concatenation is defined for Twines. If it
wasn't, then Twining the result wouldn't really improving efficiency
anyway, since it's going to create all the temporaries first. (same
suggestion as the first could apply too, though it'd look a bit
awkward:

(foo + "/../" + bar + "/bin").str(PPaths.back());

)

- David




More information about the cfe-commits mailing list