[Support] Remove error return value from one overload of fs::make_absolute

[labath]

Summary

The version of make_absolute which accepted a specific directory to use
as the "base" for the computation could never fail, even though it
returned a std::error_code. The reason for that seems to be historical

• the CWD flavour (which can fail due to failure to retrieve CWD) was

there first, and the new version was implemented by extending that.

This removes the error return value from the non-CWD overload and
reimplements the CWD version on top of that. This enables us to remove
some dead code where people were pessimistically trying to handle the
errors returned from this function.

[sammccall]

Much nicer :-)
Thanks!

clang/lib/Lex/HeaderSearch.cpp

std::string HeaderSearch::suggestPathToFileForDiagnostics(
   for (unsigned I = 0; I != SearchDirs.size(); ++I) {
     // FIXME: Support this search within frameworks and header maps.
     if (!SearchDirs[I].isNormalDir())
       continue;
 
     StringRef Dir = SearchDirs[I].getDir()->getName();
     llvm::SmallString<32> DirPath(Dir.begin(), Dir.end());
     if (!WorkingDir.empty() && !path::is_absolute(Dir)) {
-      auto err = fs::make_absolute(WorkingDir, DirPath);
-      if (!err)
-        path::remove_dots(DirPath, /*remove_dot_dot=*/true);
+      fs::make_absolute(WorkingDir, DirPath);
       Dir = DirPath;
     }
     for (auto NI = path::begin(File), NE = path::end(File),
               DI = path::begin(Dir), DE = path::end(Dir);
          /*termination condition in loop*/; ++NI, ++DI) {
       // '.' components in File are ignored.
       while (NI != NE && *NI == ".")
         ++NI;

llvm/include/llvm/Support/FileSystem.h

 ///
 /// Makes \a path absolute using the \a current_directory if it is not already.
 /// An empty \a path will result in the \a current_directory.
 ///
 /// /absolute/path   => /absolute/path
 /// relative/../path => <current-directory>/relative/../path
 ///
 /// @param path A path that is modified to be an absolute path.
-/// @returns errc::success if \a path has been made absolute, otherwise a
-///          platform-specific error_code.
-std::error_code make_absolute(const Twine &current_directory,
-                              SmallVectorImpl<char> &path);
+void make_absolute(const Twine &current_directory, SmallVectorImpl<char> &path);
 
 /// Make \a path an absolute path.
 ///
 /// Makes \a path absolute using the current directory if it is not already. An
 /// empty \a path will result in the current directory.
 ///
 /// /absolute/path   => /absolute/path
 /// relative/../path => <current-directory>/relative/../path

llvm/lib/Support/Path.cpp

 
 std::error_code
 getPotentiallyUniqueTempFileName(const Twine &Prefix, StringRef Suffix,
                                  SmallVectorImpl<char> &ResultPath) {
   int Dummy;
   return createTemporaryFile(Prefix, Suffix, Dummy, ResultPath, FS_Name);
 }
 
-static std::error_code make_absolute(const Twine &current_directory,
-                                     SmallVectorImpl<char> &path,
-                                     bool use_current_directory) {
+void make_absolute(const Twine &current_directory,
+                   SmallVectorImpl<char> &path) {
   StringRef p(path.data(), path.size());
 
   bool rootDirectory = path::has_root_directory(p);
   bool rootName =
       (real_style(Style::native) != Style::windows) || path::has_root_name(p);
 
   // Already absolute.
   if (rootName && rootDirectory)
-    return std::error_code();
+    return;
 
   // All of the following conditions will need the current directory.
   SmallString<128> current_dir;
-  if (use_current_directory)
-    current_directory.toVector(current_dir);
-  else if (std::error_code ec = current_path(current_dir))
-    return ec;
 
   // Relative path. Prepend the current directory.
   if (!rootName && !rootDirectory) {
     // Append path to the current directory.
     path::append(current_dir, p);
     // Set path to the result.
     path.swap(current_dir);
-    return std::error_code();
+    return;
   }
 
   if (!rootName && rootDirectory) {
     StringRef cdrn = path::root_name(current_dir);
     SmallString<128> curDirRootName(cdrn.begin(), cdrn.end());
     path::append(curDirRootName, p);
     // Set path to the result.
     path.swap(curDirRootName);
-    return std::error_code();
+    return;
   }
 
   if (rootName && !rootDirectory) {
     StringRef pRootName      = path::root_name(p);
     StringRef bRootDirectory = path::root_directory(current_dir);
     StringRef bRelativePath  = path::relative_path(current_dir);
     StringRef pRelativePath  = path::relative_path(p);
 
     SmallString<128> res;
     path::append(res, pRootName, bRootDirectory, bRelativePath, pRelativePath);
     path.swap(res);
-    return std::error_code();
+    return;
   }
 
   llvm_unreachable("All rootName and rootDirectory combinations should have "
                    "occurred above!");
 }
 
-std::error_code make_absolute(const Twine &current_directory,
-                              SmallVectorImpl<char> &path) {
-  return make_absolute(current_directory, path, true);
-}
-
 std::error_code make_absolute(SmallVectorImpl<char> &path) {
-  return make_absolute(Twine(), path, false);
+  if (path::is_absolute(path))
+    return {};
+
+  SmallString<128> current_dir;
+  if (std::error_code ec = current_path(current_dir))
+    return ec;
+
+  make_absolute(current_dir, path);
+  return {};
 }
 
 std::error_code create_directories(const Twine &Path, bool IgnoreExisting,
                                    perms Perms) {
   SmallString<128> PathStorage;
   StringRef P = Path.toStringRef(PathStorage);
 
   // Be optimistic and try to create the directory

llvm/lib/Support/VirtualFileSystem.cpp

 std::error_code FileSystem::makeAbsolute(SmallVectorImpl<char> &Path) const {
   if (llvm::sys::path::is_absolute(Path))
     return {};
 
   auto WorkingDir = getCurrentWorkingDirectory();
   if (!WorkingDir)
     return WorkingDir.getError();
 
-  return llvm::sys::fs::make_absolute(WorkingDir.get(), Path);
+  llvm::sys::fs::make_absolute(WorkingDir.get(), Path);
+  return {};
 }
 
 std::error_code FileSystem::getRealPath(const Twine &Path,
                                         SmallVectorImpl<char> &Output) const {
   return errc::operation_not_permitted;
 }
 
 std::error_code FileSystem::isLocal(const Twine &Path, bool &Result) {

llvm/tools/llvm-opt-report/OptReport.cpp

if (EC) {
     WithColor::error() << "Can't open file " << OutputFileName << ": "
                        << EC.message() << "\n";
     return false;
   }
 
   bool FirstFile = true;
   for (auto &FI : LocationInfo) {
     SmallString<128> FileName(FI.first);
-    if (!InputRelDir.empty()) {
-      if (std::error_code EC = sys::fs::make_absolute(InputRelDir, FileName)) {
-        WithColor::error() << "Can't resolve file path to " << FileName << ": "
-                           << EC.message() << "\n";
-        return false;
-      }
-    }
+    if (!InputRelDir.empty())
+      sys::fs::make_absolute(InputRelDir, FileName);
 
     const auto &FileInfo = FI.second;
 
     ErrorOr<std::unique_ptr<MemoryBuffer>> Buf =
         MemoryBuffer::getFile(FileName);
     if (std::error_code EC = Buf.getError()) {
       WithColor::error() << "Can't open file " << FileName << ": "
                          << EC.message() << "\n";

llvm/unittests/Support/Path.cpp

for (SmallVector<StringRef, 40>::const_iterator i = paths.begin(),
     stem = path::stem(filename);
     ext  = path::extension(filename);
     EXPECT_EQ(*sys::path::rbegin(filename), (stem + ext).str());
 
     path::native(*i, temp_store);
   }
 
   SmallString<32> Relative("foo.cpp");
-  ASSERT_NO_ERROR(sys::fs::make_absolute("/root", Relative));
+  sys::fs::make_absolute("/root", Relative);
   Relative[5] = '/'; // Fix up windows paths.
   ASSERT_EQ("/root/foo.cpp", Relative);
 }
 
 TEST(Support, FilenameParent) {
   EXPECT_EQ("/", path::filename("/"));
   EXPECT_EQ("", path::parent_path("/"));