Simplify remove, create_directory and create_directories.

Before this patch they would take an boolean argument to say if the path
already existed. This was redundant with the returned error_code which is able
to represent that. This allowed for callers to incorrectly check only the
existed flag instead of first checking the error code.

Instead, pass in a boolean flag to say if the previous (non-)existence should be
an error or not.

Callers of the of the old simple versions are not affected. They still ignore
the previous (non-)existence as they did before.

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@201979 91177308-0d34-0410-b5e6-96231b3b80d8

include/llvm/Support/FileSystem.h

@@ -273,32 +273,18 @@ error_code make_absolute(SmallVectorImpl<char> &path);
 /// @brief Create all the non-existent directories in path.
 ///
 /// @param path Directories to create.
-/// @param existed Set to true if \a path already existed, false otherwise.
+/// @returns errc::success if is_directory(path), otherwise a platform
-/// @returns errc::success if is_directory(path) and existed have been set,
+///          specific error_code. If IgnoreExisting is false, also returns
-///          otherwise a platform specific error_code.
+///          error if the directory already existed.
-error_code create_directories(const Twine &path, bool &existed);
+error_code create_directories(const Twine &path, bool IgnoreExisting = true);
-
-/// @brief Convenience function for clients that don't need to know if the
-///        directory existed or not.
-inline error_code create_directories(const Twine &Path) {
-  bool Existed;
-  return create_directories(Path, Existed);
-}
 
 /// @brief Create the directory in path.
 ///
 /// @param path Directory to create.
-/// @param existed Set to true if \a path already existed, false otherwise.
+/// @returns errc::success if is_directory(path), otherwise a platform
-/// @returns errc::success if is_directory(path) and existed have been set,
+///          specific error_code. If IgnoreExisting is false, also returns
-///          otherwise a platform specific error_code.
+///          error if the directory already existed.
-error_code create_directory(const Twine &path, bool &existed);
+error_code create_directory(const Twine &path, bool IgnoreExisting = true);
-
-/// @brief Convenience function for clients that don't need to know if the
-///        directory existed or not.
-inline error_code create_directory(const Twine &Path) {
-  bool Existed;
-  return create_directory(Path, Existed);
-}
 
 /// @brief Create a hard link from \a from to \a to.
 ///
@@ -318,18 +304,10 @@ error_code current_path(SmallVectorImpl<char> &result);
 /// @brief Remove path. Equivalent to POSIX remove().
 ///
 /// @param path Input path.
-/// @param existed Set to true if \a path existed, false if it did not.
+/// @returns errc::success if path has been removed or didn't exist, otherwise a
-///                undefined otherwise.
+///          platform specific error code. If IgnoreNonExisting is false, also
-/// @returns errc::success if path has been removed and existed has been
+///          returns error if the file didn't exist.
-///          successfully set, otherwise a platform specific error_code.
+error_code remove(const Twine &path, bool IgnoreNonExisting = true);
-error_code remove(const Twine &path, bool &existed);
-
-/// @brief Convenience function for clients that don't need to know if the file
-///        existed or not.
-inline error_code remove(const Twine &Path) {
-  bool Existed;
-  return remove(Path, Existed);
-}
 
 /// @brief Rename \a from to \a to. Files are renamed as if by POSIX rename().
 ///

lib/Support/Path.cpp

@@ -751,12 +751,12 @@ error_code make_absolute(SmallVectorImpl<char> &path) {
                    "occurred above!");
 }
 
-error_code create_directories(const Twine &Path, bool &Existed) {
+error_code create_directories(const Twine &Path, bool IgnoreExisting) {
   SmallString<128> PathStorage;
   StringRef P = Path.toStringRef(PathStorage);
 
   // Be optimistic and try to create the directory
-  error_code EC = create_directory(P, Existed);
+  error_code EC = create_directory(P, IgnoreExisting);
   // If we succeeded, or had any error other than the parent not existing, just
   // return it.
   if (EC != errc::no_such_file_or_directory)
@@ -771,7 +771,7 @@ error_code create_directories(const Twine &Path, bool &Existed) {
   if ((EC = create_directories(Parent)))
       return EC;
 
-  return create_directory(P, Existed);
+  return create_directory(P, IgnoreExisting);
 }
 
 bool exists(file_status status) {

lib/Support/Unix/Path.inc

@@ -164,12 +164,11 @@ retry_random_path:
   }
 
   case FS_Dir: {
-    bool Existed;
+    if (error_code EC = sys::fs::create_directory(ResultPath.begin(), false)) {
-    error_code EC = sys::fs::create_directory(ResultPath.begin(), Existed);
+      if (EC == errc::file_exists)
-    if (EC)
+        goto retry_random_path;
       return EC;
-    if (Existed)
+    }
-      goto retry_random_path;
     return error_code::success();
   }
   }
@@ -333,16 +332,14 @@ error_code current_path(SmallVectorImpl<char> &result) {
   return error_code::success();
 }
 
-error_code create_directory(const Twine &path, bool &existed) {
+error_code create_directory(const Twine &path, bool IgnoreExisting) {
   SmallString<128> path_storage;
   StringRef p = path.toNullTerminatedStringRef(path_storage);
 
   if (::mkdir(p.begin(), S_IRWXU | S_IRWXG) == -1) {
-    if (errno != errc::file_exists)
+    if (errno != errc::file_exists || !IgnoreExisting)
       return error_code(errno, system_category());
-    existed = true;
+  }
-  } else
-    existed = false;
 
   return error_code::success();
 }
@@ -360,15 +357,14 @@ error_code create_hard_link(const Twine &to, const Twine &from) {
   return error_code::success();
 }
 
-error_code remove(const Twine &path, bool &existed) {
+error_code remove(const Twine &path, bool IgnoreNonExisting) {
   SmallString<128> path_storage;
   StringRef p = path.toNullTerminatedStringRef(path_storage);
 
   struct stat buf;
   if (stat(p.begin(), &buf) != 0) {
-    if (errno != errc::no_such_file_or_directory)
+    if (errno != errc::no_such_file_or_directory || !IgnoreNonExisting)
       return error_code(errno, system_category());
-    existed = false;
     return error_code::success();
   }
 
@@ -381,11 +377,9 @@ error_code remove(const Twine &path, bool &existed) {
     return make_error_code(errc::operation_not_permitted);
 
   if (::remove(p.begin()) == -1) {
-    if (errno != errc::no_such_file_or_directory)
+    if (errno != errc::no_such_file_or_directory || !IgnoreNonExisting)
       return error_code(errno, system_category());
-    existed = false;
+  }
-  } else
-    existed = true;
 
   return error_code::success();
 }

lib/Support/Windows/Path.inc

@@ -264,7 +264,7 @@ error_code current_path(SmallVectorImpl<char> &result) {
   return UTF16ToUTF8(cur_path.begin(), cur_path.size(), result);
 }
 
-error_code create_directory(const Twine &path, bool &existed) {
+error_code create_directory(const Twine &path, bool IgnoreExisting) {
   SmallString<128> path_storage;
   SmallVector<wchar_t, 128> path_utf16;
 
@@ -274,12 +274,9 @@ error_code create_directory(const Twine &path, bool &existed) {
 
   if (!::CreateDirectoryW(path_utf16.begin(), NULL)) {
     error_code ec = windows_error(::GetLastError());
-    if (ec == windows_error::already_exists)
+    if (ec != windows_error::already_exists || !IgnoreExisting)
-      existed = true;
-    else
       return ec;
-  } else
+  }
-    existed = false;
 
   return error_code::success();
 }
@@ -303,43 +300,34 @@ error_code create_hard_link(const Twine &to, const Twine &from) {
   return error_code::success();
 }
 
-error_code remove(const Twine &path, bool &existed) {
+error_code remove(const Twine &path, bool IgnoreNonExisting) {
   SmallString<128> path_storage;
   SmallVector<wchar_t, 128> path_utf16;
 
-  file_status st;
+  file_status ST;
-  error_code EC = status(path, st);
+  if (error_code EC = status(path, ST)) {
-  if (EC) {
+    if (EC != errc::no_such_file_or_directory || !IgnoreNonExisting)
-    if (EC == windows_error::file_not_found ||
+      return EC;
-        EC == windows_error::path_not_found) {
+    return error_code::success();
-      existed = false;
-      return error_code::success();
-    }
-    return EC;
   }
 
   if (error_code ec = UTF8ToUTF16(path.toStringRef(path_storage),
                                   path_utf16))
     return ec;
 
-  if (st.type() == file_type::directory_file) {
+  if (ST.type() == file_type::directory_file) {
     if (!::RemoveDirectoryW(c_str(path_utf16))) {
-      error_code ec = windows_error(::GetLastError());
+      error_code EC = windows_error(::GetLastError());
-      if (ec != windows_error::file_not_found)
+      if (EC != errc::no_such_file_or_directory || !IgnoreNonExisting)
-        return ec;
+        return EC;
-      existed = false;
+    }
-    } else
+    return error_code::success();
-      existed = true;
+  }
-  } else {
+  if (!::DeleteFileW(c_str(path_utf16))) {
-    if (!::DeleteFileW(c_str(path_utf16))) {
+    error_code EC = windows_error(::GetLastError());
-      error_code ec = windows_error(::GetLastError());
+    if (EC != errc::no_such_file_or_directory || !IgnoreNonExisting)
-      if (ec != windows_error::file_not_found)
+      return EC;
-        return ec;
-      existed = false;
-    } else
-      existed = true;
   }
-
   return error_code::success();
 }
 

unittests/Support/Path.cpp

@@ -318,8 +318,10 @@ TEST_F(FileSystemTest, TempFiles) {
   ::close(FD2);
 
   // Remove Temp2.
-  ASSERT_NO_ERROR(fs::remove(Twine(TempPath2), TempFileExists));
+  ASSERT_NO_ERROR(fs::remove(Twine(TempPath2)));
-  EXPECT_TRUE(TempFileExists);
+  ASSERT_NO_ERROR(fs::remove(Twine(TempPath2)));
+  ASSERT_EQ(fs::remove(Twine(TempPath2), false),
+            errc::no_such_file_or_directory);
 
   error_code EC = fs::status(TempPath2.c_str(), B);
   EXPECT_EQ(EC, errc::no_such_file_or_directory);
@@ -344,12 +346,10 @@ TEST_F(FileSystemTest, TempFiles) {
 
   // Remove Temp1.
   ::close(FileDescriptor);
-  ASSERT_NO_ERROR(fs::remove(Twine(TempPath), TempFileExists));
+  ASSERT_NO_ERROR(fs::remove(Twine(TempPath)));
-  EXPECT_TRUE(TempFileExists);
 
   // Remove the hard link.
-  ASSERT_NO_ERROR(fs::remove(Twine(TempPath2), TempFileExists));
+  ASSERT_NO_ERROR(fs::remove(Twine(TempPath2)));
-  EXPECT_TRUE(TempFileExists);
 
   // Make sure Temp1 doesn't exist.
   ASSERT_NO_ERROR(fs::exists(Twine(TempPath), TempFileExists));
@@ -368,23 +368,30 @@ TEST_F(FileSystemTest, TempFiles) {
 #endif
 }
 
+TEST_F(FileSystemTest, CreateDir) {
+  ASSERT_NO_ERROR(fs::create_directory(Twine(TestDirectory) + "foo"));
+  ASSERT_NO_ERROR(fs::create_directory(Twine(TestDirectory) + "foo"));
+  ASSERT_EQ(fs::create_directory(Twine(TestDirectory) + "foo", false),
+            errc::file_exists);
+  ASSERT_NO_ERROR(fs::remove(Twine(TestDirectory) + "foo"));
+}
+
 TEST_F(FileSystemTest, DirectoryIteration) {
   error_code ec;
   for (fs::directory_iterator i(".", ec), e; i != e; i.increment(ec))
     ASSERT_NO_ERROR(ec);
 
   // Create a known hierarchy to recurse over.
-  bool existed;
+  ASSERT_NO_ERROR(
-  ASSERT_NO_ERROR(fs::create_directories(Twine(TestDirectory)
+      fs::create_directories(Twine(TestDirectory) + "/recursive/a0/aa1"));
-                  + "/recursive/a0/aa1", existed));
+  ASSERT_NO_ERROR(
-  ASSERT_NO_ERROR(fs::create_directories(Twine(TestDirectory)
+      fs::create_directories(Twine(TestDirectory) + "/recursive/a0/ab1"));
-                  + "/recursive/a0/ab1", existed));
+  ASSERT_NO_ERROR(fs::create_directories(Twine(TestDirectory) +
-  ASSERT_NO_ERROR(fs::create_directories(Twine(TestDirectory)
+                                         "/recursive/dontlookhere/da1"));
-                  + "/recursive/dontlookhere/da1", existed));
+  ASSERT_NO_ERROR(
-  ASSERT_NO_ERROR(fs::create_directories(Twine(TestDirectory)
+      fs::create_directories(Twine(TestDirectory) + "/recursive/z0/za1"));
-                  + "/recursive/z0/za1", existed));
+  ASSERT_NO_ERROR(
-  ASSERT_NO_ERROR(fs::create_directories(Twine(TestDirectory)
+      fs::create_directories(Twine(TestDirectory) + "/recursive/pop/p1"));
-                  + "/recursive/pop/p1", existed));
   typedef std::vector<std::string> v_t;
   v_t visited;
   for (fs::recursive_directory_iterator i(Twine(TestDirectory)

unittests/Transforms/DebugIR/DebugIR.cpp

@@ -55,9 +55,10 @@ void insertCUDescriptor(Module *M, StringRef File, StringRef Dir,
 /// Attempts to remove file at Path and returns true if it existed, or false if
 /// it did not.
 bool removeIfExists(StringRef Path) {
-  bool existed = false;
+  // This is an approximation, on error we don't know in general if the file
-  sys::fs::remove(Path, existed);
+  // existed or not.
-  return existed;
+  llvm::error_code EC = sys::fs::remove(Path, false);
+  return EC != llvm::errc::no_such_file_or_directory;
 }
 
 char * current_dir() {