[llvm] r198955 - Remove remove_all. A compiler has no need for recursively deleting a directory.
Sean Silva
silvas at purdue.edu
Fri Jan 10 12:57:50 PST 2014
On Fri, Jan 10, 2014 at 1:36 PM, Rafael Espindola <
rafael.espindola at gmail.com> wrote:
> Author: rafael
> Date: Fri Jan 10 14:36:42 2014
> New Revision: 198955
>
> URL: http://llvm.org/viewvc/llvm-project?rev=198955&view=rev
> Log:
> Remove remove_all. A compiler has no need for recursively deleting a
> directory.
>
Do we have a temporary directory RAII class or something? What is the
recommended way to handle the case where you have multiple temporary files
but for hygiene it is preferred to keep them together inside a temporary
directory? (isn't this what Clang does with its temporaries?)
-- Sean Silva
>
> Modified:
> llvm/trunk/include/llvm/Support/FileSystem.h
> llvm/trunk/lib/Support/Path.cpp
> llvm/trunk/unittests/Support/FileOutputBufferTest.cpp
> llvm/trunk/unittests/Support/LockFileManagerTest.cpp
> llvm/trunk/unittests/Support/Path.cpp
>
> Modified: llvm/trunk/include/llvm/Support/FileSystem.h
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Support/FileSystem.h?rev=198955&r1=198954&r2=198955&view=diff
>
> ==============================================================================
> --- llvm/trunk/include/llvm/Support/FileSystem.h (original)
> +++ llvm/trunk/include/llvm/Support/FileSystem.h Fri Jan 10 14:36:42 2014
> @@ -341,22 +341,6 @@ inline error_code remove(const Twine &Pa
> return remove(Path, Existed);
> }
>
> -/// @brief Recursively remove all files below \a path, then \a path.
> Files are
> -/// removed as if by POSIX remove().
> -///
> -/// @param path Input path.
> -/// @param num_removed Number of files removed.
> -/// @returns errc::success if path has been removed and num_removed has
> been
> -/// successfully set, otherwise a platform specific error_code.
> -error_code remove_all(const Twine &path, uint32_t &num_removed);
> -
> -/// @brief Convenience function for clients that don't need to know how
> many
> -/// files were removed.
> -inline error_code remove_all(const Twine &Path) {
> - uint32_t Removed;
> - return remove_all(Path, Removed);
> -}
> -
> /// @brief Rename \a from to \a to. Files are renamed as if by POSIX
> rename().
> ///
> /// @param from The path to rename from.
>
> Modified: llvm/trunk/lib/Support/Path.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/Path.cpp?rev=198955&r1=198954&r2=198955&view=diff
>
> ==============================================================================
> --- llvm/trunk/lib/Support/Path.cpp (original)
> +++ llvm/trunk/lib/Support/Path.cpp Fri Jan 10 14:36:42 2014
> @@ -983,45 +983,6 @@ error_code identify_magic(const Twine &p
> return error_code::success();
> }
>
> -namespace {
> -error_code remove_all_r(StringRef path, file_type ft, uint32_t &count) {
> - if (ft == file_type::directory_file) {
> - // This code would be a lot better with exceptions ;/.
> - error_code ec;
> - directory_iterator i(path, ec);
> - if (ec) return ec;
> - for (directory_iterator e; i != e; i.increment(ec)) {
> - if (ec) return ec;
> - file_status st;
> - if (error_code ec = i->status(st)) return ec;
> - if (error_code ec = remove_all_r(i->path(), st.type(), count))
> return ec;
> - }
> - bool obviously_this_exists;
> - if (error_code ec = remove(path, obviously_this_exists)) return ec;
> - assert(obviously_this_exists);
> - ++count; // Include the directory itself in the items removed.
> - } else {
> - bool obviously_this_exists;
> - if (error_code ec = remove(path, obviously_this_exists)) return ec;
> - assert(obviously_this_exists);
> - ++count;
> - }
> -
> - return error_code::success();
> -}
> -} // end unnamed namespace
> -
> -error_code remove_all(const Twine &path, uint32_t &num_removed) {
> - SmallString<128> path_storage;
> - StringRef p = path.toStringRef(path_storage);
> -
> - file_status fs;
> - if (error_code ec = status(path, fs))
> - return ec;
> - num_removed = 0;
> - return remove_all_r(p, fs.type(), num_removed);
> -}
> -
> error_code directory_entry::status(file_status &result) const {
> return fs::status(Path, result);
> }
>
> Modified: llvm/trunk/unittests/Support/FileOutputBufferTest.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/Support/FileOutputBufferTest.cpp?rev=198955&r1=198954&r2=198955&view=diff
>
> ==============================================================================
> --- llvm/trunk/unittests/Support/FileOutputBufferTest.cpp (original)
> +++ llvm/trunk/unittests/Support/FileOutputBufferTest.cpp Fri Jan 10
> 14:36:42 2014
> @@ -56,6 +56,7 @@ TEST(FileOutputBuffer, Test) {
> uint64_t File1Size;
> ASSERT_NO_ERROR(fs::file_size(Twine(File1), File1Size));
> ASSERT_EQ(File1Size, 8192ULL);
> + ASSERT_NO_ERROR(fs::remove(File1.str()));
>
> // TEST 2: Verify abort case.
> SmallString<128> File2(TestDirectory);
> @@ -71,6 +72,7 @@ TEST(FileOutputBuffer, Test) {
> bool Exists = false;
> ASSERT_NO_ERROR(fs::exists(Twine(File2), Exists));
> EXPECT_FALSE(Exists);
> + ASSERT_NO_ERROR(fs::remove(File2.str()));
>
> // TEST 3: Verify sizing down case.
> SmallString<128> File3(TestDirectory);
> @@ -94,6 +96,7 @@ TEST(FileOutputBuffer, Test) {
> uint64_t File3Size;
> ASSERT_NO_ERROR(fs::file_size(Twine(File3), File3Size));
> ASSERT_EQ(File3Size, 5000ULL);
> + ASSERT_NO_ERROR(fs::remove(File3.str()));
>
> // TEST 4: Verify file can be made executable.
> SmallString<128> File4(TestDirectory);
> @@ -112,9 +115,9 @@ TEST(FileOutputBuffer, Test) {
> ASSERT_NO_ERROR(fs::status(Twine(File4), Status));
> bool IsExecutable = (Status.permissions() & fs::owner_exe);
> EXPECT_TRUE(IsExecutable);
> + ASSERT_NO_ERROR(fs::remove(File4.str()));
>
> // Clean up.
> - uint32_t RemovedCount;
> - ASSERT_NO_ERROR(fs::remove_all(TestDirectory.str(), RemovedCount));
> + ASSERT_NO_ERROR(fs::remove(TestDirectory.str()));
> }
> } // anonymous namespace
>
> Modified: llvm/trunk/unittests/Support/LockFileManagerTest.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/Support/LockFileManagerTest.cpp?rev=198955&r1=198954&r2=198955&view=diff
>
> ==============================================================================
> --- llvm/trunk/unittests/Support/LockFileManagerTest.cpp (original)
> +++ llvm/trunk/unittests/Support/LockFileManagerTest.cpp Fri Jan 10
> 14:36:42 2014
> @@ -40,7 +40,8 @@ TEST(LockFileManagerTest, Basic) {
> // Now that the lock is out of scope, the file should be gone.
> EXPECT_FALSE(sys::fs::exists(StringRef(LockedFile)));
>
> - sys::fs::remove_all(StringRef(TmpDir));
> + EC = sys::fs::remove(StringRef(TmpDir));
> + ASSERT_FALSE(EC);
> }
>
> } // end anonymous namespace
>
> Modified: llvm/trunk/unittests/Support/Path.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/Support/Path.cpp?rev=198955&r1=198954&r2=198955&view=diff
>
> ==============================================================================
> --- llvm/trunk/unittests/Support/Path.cpp (original)
> +++ llvm/trunk/unittests/Support/Path.cpp Fri Jan 10 14:36:42 2014
> @@ -225,8 +225,7 @@ protected:
> }
>
> virtual void TearDown() {
> - uint32_t removed;
> - ASSERT_NO_ERROR(fs::remove_all(TestDirectory.str(), removed));
> + ASSERT_NO_ERROR(fs::remove(TestDirectory.str()));
> }
> };
>
> @@ -414,6 +413,18 @@ TEST_F(FileSystemTest, DirectoryIteratio
> ASSERT_LT(a0, aa1);
> ASSERT_LT(a0, ab1);
> ASSERT_LT(z0, za1);
> +
> + ASSERT_NO_ERROR(fs::remove(Twine(TestDirectory) + "/recursive/a0/aa1"));
> + ASSERT_NO_ERROR(fs::remove(Twine(TestDirectory) + "/recursive/a0/ab1"));
> + ASSERT_NO_ERROR(fs::remove(Twine(TestDirectory) + "/recursive/a0"));
> + ASSERT_NO_ERROR(
> + fs::remove(Twine(TestDirectory) + "/recursive/dontlookhere/da1"));
> + ASSERT_NO_ERROR(fs::remove(Twine(TestDirectory) +
> "/recursive/dontlookhere"));
> + ASSERT_NO_ERROR(fs::remove(Twine(TestDirectory) + "/recursive/pop/p1"));
> + ASSERT_NO_ERROR(fs::remove(Twine(TestDirectory) + "/recursive/pop"));
> + ASSERT_NO_ERROR(fs::remove(Twine(TestDirectory) + "/recursive/z0/za1"));
> + ASSERT_NO_ERROR(fs::remove(Twine(TestDirectory) + "/recursive/z0"));
> + ASSERT_NO_ERROR(fs::remove(Twine(TestDirectory) + "/recursive"));
> }
>
> const char archive[] = "!<arch>\x0A";
> @@ -479,6 +490,7 @@ TEST_F(FileSystemTest, Magic) {
> ASSERT_NO_ERROR(fs::has_magic(file_pathname.c_str(), magic, res));
> EXPECT_TRUE(res);
> EXPECT_EQ(i->magic, fs::identify_magic(magic));
> + ASSERT_NO_ERROR(fs::remove(Twine(file_pathname)));
> }
> }
>
> @@ -509,6 +521,7 @@ TEST_F(FileSystemTest, CarriageReturn) {
> MemoryBuffer::getFile(FilePathname.c_str(), Buf);
> EXPECT_EQ(Buf->getBuffer(), "\n");
> }
> + ASSERT_NO_ERROR(fs::remove(Twine(FilePathname)));
> }
> #endif
>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140110/0ee9312a/attachment.html>
More information about the llvm-commits
mailing list