Add Env::Remove{File,Dir} which obsolete Env::Delete{File,Dir}.

The "DeleteFile" method name causes pain for Windows developers, because
<windows.h> #defines a DeleteFile macro to DeleteFileW or DeleteFileA.
Current code uses workarounds, like #undefining DeleteFile everywhere an
Env is declared, implemented, or used.

This CL removes the need for workarounds by renaming Env::DeleteFile to
Env::RemoveFile. For consistency, Env::DeleteDir is also renamed to
Env::RemoveDir. A few internal methods are also renamed for consistency.
Software that supports Windows is expected to migrate any Env
implementations and usage to Remove{File,Dir}, and never use the name
Env::Delete{File,Dir} in its code.

The renaming is done in a backwards-compatible way, at the risk of
making it slightly more difficult to build a new correct Env
implementation. The backwards compatibility is achieved using the
following hacks:

1) Env::Remove{File,Dir} methods are added, with a default
    implementation that calls into Env::Delete{File,Dir}. This makes old
    Env implementations compatible with code that calls into the updated
    API.
2) The Env::Delete{File,Dir} methods are no longer pure virtuals.
    Instead, they gain a default implementation that calls into
    Env::Remove{File,Dir}. This makes updated Env implementations
    compatible with code that calls into the old API.

The cost of this approach is that it's possible to write an Env without
overriding either Rename{File,Dir} or Delete{File,Dir}, without getting
a compiler warning. However, attempting to run the test suite will
immediately fail with an infinite call stack ending in
{Remove,Delete}{File,Dir}, making developers aware of the problem.

PiperOrigin-RevId: 288710907

6 files changed, 31 insertions, 21 deletions

diff --git a/util/env.cc b/util/env.cc
index d2f0aef..40e6071 100644
--- a/util/env.cc
+++ b/util/env.cc
@@ -4,14 +4,28 @@
 
 #include "leveldb/env.h"
 
+// This workaround can be removed when leveldb::Env::DeleteFile is removed.
+// See env.h for justification.
+#if defined(_WIN32) && defined(LEVELDB_DELETEFILE_UNDEFINED)
+#undef DeleteFile
+#endif
+
 namespace leveldb {
 
+Env::Env() = default;
+
 Env::~Env() = default;
 
 Status Env::NewAppendableFile(const std::string& fname, WritableFile** result) {
   return Status::NotSupported("NewAppendableFile", fname);
 }
 
+Status Env::RemoveDir(const std::string& dirname) { return DeleteDir(dirname); }
+Status Env::DeleteDir(const std::string& dirname) { return RemoveDir(dirname); }
+
+Status Env::RemoveFile(const std::string& fname) { return DeleteFile(fname); }
+Status Env::DeleteFile(const std::string& fname) { return RemoveFile(fname); }
+
 SequentialFile::~SequentialFile() = default;
 
 RandomAccessFile::~RandomAccessFile() = default;
@@ -47,7 +61,7 @@ static Status DoWriteStringToFile(Env* env, const Slice& data,
   }
   delete file;  // Will auto-close if we did not close above
   if (!s.ok()) {
-    env->DeleteFile(fname);
+    env->RemoveFile(fname);
   }
   return s;
 }
diff --git a/util/env_posix.cc b/util/env_posix.cc
index 00ca9ae..d84cd1e 100644
--- a/util/env_posix.cc
+++ b/util/env_posix.cc
@@ -587,7 +587,7 @@ class PosixEnv : public Env {
     return Status::OK();
   }
 
-  Status DeleteFile(const std::string& filename) override {
+  Status RemoveFile(const std::string& filename) override {
     if (::unlink(filename.c_str()) != 0) {
       return PosixError(filename, errno);
     }
@@ -601,7 +601,7 @@ class PosixEnv : public Env {
     return Status::OK();
   }
 
-  Status DeleteDir(const std::string& dirname) override {
+  Status RemoveDir(const std::string& dirname) override {
     if (::rmdir(dirname.c_str()) != 0) {
       return PosixError(dirname, errno);
     }
diff --git a/util/env_posix_test.cc b/util/env_posix_test.cc
index ed4ac96..36f226f 100644
--- a/util/env_posix_test.cc
+++ b/util/env_posix_test.cc
@@ -209,7 +209,7 @@ TEST_F(EnvPosixTest, TestOpenOnRead) {
   for (int i = 0; i < kNumFiles; i++) {
     delete files[i];
   }
-  ASSERT_LEVELDB_OK(env_->DeleteFile(test_file));
+  ASSERT_LEVELDB_OK(env_->RemoveFile(test_file));
 }
 
 #if HAVE_O_CLOEXEC
@@ -228,7 +228,7 @@ TEST_F(EnvPosixTest, TestCloseOnExecSequentialFile) {
   CheckCloseOnExecDoesNotLeakFDs(open_fds);
   delete file;
 
-  ASSERT_LEVELDB_OK(env_->DeleteFile(file_path));
+  ASSERT_LEVELDB_OK(env_->RemoveFile(file_path));
 }
 
 TEST_F(EnvPosixTest, TestCloseOnExecRandomAccessFile) {
@@ -256,7 +256,7 @@ TEST_F(EnvPosixTest, TestCloseOnExecRandomAccessFile) {
   for (int i = 0; i < kReadOnlyFileLimit; i++) {
     delete mmapped_files[i];
   }
-  ASSERT_LEVELDB_OK(env_->DeleteFile(file_path));
+  ASSERT_LEVELDB_OK(env_->RemoveFile(file_path));
 }
 
 TEST_F(EnvPosixTest, TestCloseOnExecWritableFile) {
@@ -273,7 +273,7 @@ TEST_F(EnvPosixTest, TestCloseOnExecWritableFile) {
   CheckCloseOnExecDoesNotLeakFDs(open_fds);
   delete file;
 
-  ASSERT_LEVELDB_OK(env_->DeleteFile(file_path));
+  ASSERT_LEVELDB_OK(env_->RemoveFile(file_path));
 }
 
 TEST_F(EnvPosixTest, TestCloseOnExecAppendableFile) {
@@ -290,7 +290,7 @@ TEST_F(EnvPosixTest, TestCloseOnExecAppendableFile) {
   CheckCloseOnExecDoesNotLeakFDs(open_fds);
   delete file;
 
-  ASSERT_LEVELDB_OK(env_->DeleteFile(file_path));
+  ASSERT_LEVELDB_OK(env_->RemoveFile(file_path));
 }
 
 TEST_F(EnvPosixTest, TestCloseOnExecLockFile) {
@@ -307,7 +307,7 @@ TEST_F(EnvPosixTest, TestCloseOnExecLockFile) {
   CheckCloseOnExecDoesNotLeakFDs(open_fds);
   ASSERT_LEVELDB_OK(env_->UnlockFile(lock));
 
-  ASSERT_LEVELDB_OK(env_->DeleteFile(file_path));
+  ASSERT_LEVELDB_OK(env_->RemoveFile(file_path));
 }
 
 TEST_F(EnvPosixTest, TestCloseOnExecLogger) {
@@ -324,7 +324,7 @@ TEST_F(EnvPosixTest, TestCloseOnExecLogger) {
   CheckCloseOnExecDoesNotLeakFDs(open_fds);
   delete file;
 
-  ASSERT_LEVELDB_OK(env_->DeleteFile(file_path));
+  ASSERT_LEVELDB_OK(env_->RemoveFile(file_path));
 }
 
 #endif  // HAVE_O_CLOEXEC
diff --git a/util/env_test.cc b/util/env_test.cc
index 09e9d39..223090e 100644
--- a/util/env_test.cc
+++ b/util/env_test.cc
@@ -193,7 +193,7 @@ TEST_F(EnvTest, ReopenWritableFile) {
   std::string test_dir;
   ASSERT_LEVELDB_OK(env_->GetTestDirectory(&test_dir));
   std::string test_file_name = test_dir + "/reopen_writable_file.txt";
-  env_->DeleteFile(test_file_name);
+  env_->RemoveFile(test_file_name);
 
   WritableFile* writable_file;
   ASSERT_LEVELDB_OK(env_->NewWritableFile(test_file_name, &writable_file));
@@ -210,14 +210,14 @@ TEST_F(EnvTest, ReopenWritableFile) {
 
   ASSERT_LEVELDB_OK(ReadFileToString(env_, test_file_name, &data));
   ASSERT_EQ(std::string("42"), data);
-  env_->DeleteFile(test_file_name);
+  env_->RemoveFile(test_file_name);
 }
 
 TEST_F(EnvTest, ReopenAppendableFile) {
   std::string test_dir;
   ASSERT_LEVELDB_OK(env_->GetTestDirectory(&test_dir));
   std::string test_file_name = test_dir + "/reopen_appendable_file.txt";
-  env_->DeleteFile(test_file_name);
+  env_->RemoveFile(test_file_name);
 
   WritableFile* appendable_file;
   ASSERT_LEVELDB_OK(env_->NewAppendableFile(test_file_name, &appendable_file));
@@ -234,7 +234,7 @@ TEST_F(EnvTest, ReopenAppendableFile) {
 
   ASSERT_LEVELDB_OK(ReadFileToString(env_, test_file_name, &data));
   ASSERT_EQ(std::string("hello world!42"), data);
-  env_->DeleteFile(test_file_name);
+  env_->RemoveFile(test_file_name);
 }
 
 }  // namespace leveldb
diff --git a/util/env_windows.cc b/util/env_windows.cc
index 2dd7794..449f564 100644
--- a/util/env_windows.cc
+++ b/util/env_windows.cc
@@ -33,10 +33,6 @@
 #include "util/mutexlock.h"
 #include "util/windows_logger.h"
 
-#if defined(DeleteFile)
-#undef DeleteFile
-#endif  // defined(DeleteFile)
-
 namespace leveldb {
 
 namespace {
@@ -505,7 +501,7 @@ class WindowsEnv : public Env {
     return Status::OK();
   }
 
-  Status DeleteFile(const std::string& filename) override {
+  Status RemoveFile(const std::string& filename) override {
     if (!::DeleteFileA(filename.c_str())) {
       return WindowsError(filename, ::GetLastError());
     }
@@ -519,7 +515,7 @@ class WindowsEnv : public Env {
     return Status::OK();
   }
 
-  Status DeleteDir(const std::string& dirname) override {
+  Status RemoveDir(const std::string& dirname) override {
     if (!::RemoveDirectoryA(dirname.c_str())) {
       return WindowsError(dirname, ::GetLastError());
     }
diff --git a/util/env_windows_test.cc b/util/env_windows_test.cc
index c75ca7b..15c0274 100644
--- a/util/env_windows_test.cc
+++ b/util/env_windows_test.cc
@@ -52,7 +52,7 @@ TEST_F(EnvWindowsTest, TestOpenOnRead) {
   for (int i = 0; i < kNumFiles; i++) {
     delete files[i];
   }
-  ASSERT_LEVELDB_OK(env_->DeleteFile(test_file));
+  ASSERT_LEVELDB_OK(env_->RemoveFile(test_file));
 }
 
 }  // namespace leveldb