Redundant Twine->StringRef->Twine conversions in llvm::sys::fs::make_absolute?

llvm::sys::fs::make_absolute converts its first parameter (const Twine &current_directory) to StringRef p(path.data(), path.size()), and then passes that StringRef to several functions (path::has_root_directory, path::has_root_name, and path::append) that accept Twines as parameters. Since llvm::StringRef can implicitly convert to an llvm::Twine, p converts to a bunch of Twine temporaries.

In a few places, namely path::has_root_directory & path::has_root_name, that temporary Twine is again converted to a StringRef:

bool has_root_directory(const Twine &path) {
SmallString<128> path_storage;
StringRef p = path.toStringRef(path_storage);

return !root_directory(p).empty();
}

Is there some reason for this? If not, I’ll write a patch to delay the StringRef p(path.data(), path.size()) construction until it’s actually needed (calls to path::root_name(p) & path::relative_path(p)).

Are you talking about the local (static) function make_absolute, not the llvm::sys::fs::make_absolute ?

Anyhow, StringRef p() is not constructed from the Twine current_directory but from the SmallVector &path which is the relative path to being made absolute, .This StringRef(SmallVector) construction should be zero-cost.

The Twine current_directory is constructed only later, if use_current_directory. It may still be possible to optimize this code path but first check how much the use_current_directory=true is code path used vs the use_current_directory=false code path.

Are you talking about the local (static) function make_absolute, not the llvm::sys::fs::make_absolute ?

I’m referring to the static function… the funny thing is that the static function is actually defined in the llvm::sys::fs namespace.

Anyhow, StringRef p() is not constructed from the Twine current_directory but from the SmallVector &path which is the relative path to being made absolute,

Oh, duh. I’m not sure how I missed that while typing my email. That kinda moots the whole point of writing a patch. Nevermind.