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

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.

23 files changed, 140 insertions, 90 deletions

diff --git a/benchmarks/db_bench.cc b/benchmarks/db_bench.cc
index 3696023..82ed892 100644
--- a/benchmarks/db_bench.cc
+++ b/benchmarks/db_bench.cc
@@ -409,7 +409,7 @@ class Benchmark {
     g_env->GetChildren(FLAGS_db, &files);
     for (size_t i = 0; i < files.size(); i++) {
       if (Slice(files[i]).starts_with("heap-")) {
-        g_env->DeleteFile(std::string(FLAGS_db) + "/" + files[i]);
+        g_env->RemoveFile(std::string(FLAGS_db) + "/" + files[i]);
       }
     }
     if (!FLAGS_use_existing_db) {
@@ -907,7 +907,7 @@ class Benchmark {
     delete file;
     if (!ok) {
       fprintf(stderr, "heap profiling not supported\n");
-      g_env->DeleteFile(fname);
+      g_env->RemoveFile(fname);
     }
   }
 };
diff --git a/benchmarks/db_bench_sqlite3.cc b/benchmarks/db_bench_sqlite3.cc
index d3fe339..9c32a2d 100644
--- a/benchmarks/db_bench_sqlite3.cc
+++ b/benchmarks/db_bench_sqlite3.cc
@@ -328,7 +328,7 @@ class Benchmark {
           std::string file_name(test_dir);
           file_name += "/";
           file_name += files[i];
-          Env::Default()->DeleteFile(file_name.c_str());
+          Env::Default()->RemoveFile(file_name.c_str());
         }
       }
     }
diff --git a/benchmarks/db_bench_tree_db.cc b/benchmarks/db_bench_tree_db.cc
index b2f6646..43f0f65 100644
--- a/benchmarks/db_bench_tree_db.cc
+++ b/benchmarks/db_bench_tree_db.cc
@@ -301,7 +301,7 @@ class Benchmark {
           std::string file_name(test_dir);
           file_name += "/";
           file_name += files[i];
-          Env::Default()->DeleteFile(file_name.c_str());
+          Env::Default()->RemoveFile(file_name.c_str());
         }
       }
     }
diff --git a/db/builder.cc b/db/builder.cc
index 9520ee4..943e857 100644
--- a/db/builder.cc
+++ b/db/builder.cc
@@ -71,7 +71,7 @@ Status BuildTable(const std::string& dbname, Env* env, const Options& options,
   if (s.ok() && meta->file_size > 0) {
     // Keep it
   } else {
-    env->DeleteFile(fname);
+    env->RemoveFile(fname);
   }
   return s;
 }
diff --git a/db/db_impl.cc b/db/db_impl.cc
index 95e2bb4..727047f 100644
--- a/db/db_impl.cc
+++ b/db/db_impl.cc
@@ -206,7 +206,7 @@ Status DBImpl::NewDB() {
     // Make "CURRENT" file that points to the new manifest file.
     s = SetCurrentFile(env_, dbname_, 1);
   } else {
-    env_->DeleteFile(manifest);
+    env_->RemoveFile(manifest);
   }
   return s;
 }
@@ -282,7 +282,7 @@ void DBImpl::DeleteObsoleteFiles() {
   // are therefore safe to delete while allowing other threads to proceed.
   mutex_.Unlock();
   for (const std::string& filename : files_to_delete) {
-    env_->DeleteFile(dbname_ + "/" + filename);
+    env_->RemoveFile(dbname_ + "/" + filename);
   }
   mutex_.Lock();
 }
@@ -729,7 +729,7 @@ void DBImpl::BackgroundCompaction() {
     // Move file to next level
     assert(c->num_input_files(0) == 1);
     FileMetaData* f = c->input(0, 0);
-    c->edit()->DeleteFile(c->level(), f->number);
+    c->edit()->RemoveFile(c->level(), f->number);
     c->edit()->AddFile(c->level() + 1, f->number, f->file_size, f->smallest,
                        f->largest);
     status = versions_->LogAndApply(c->edit(), &mutex_);
@@ -1539,15 +1539,15 @@ Status DestroyDB(const std::string& dbname, const Options& options) {
     for (size_t i = 0; i < filenames.size(); i++) {
       if (ParseFileName(filenames[i], &number, &type) &&
           type != kDBLockFile) {  // Lock file will be deleted at end
-        Status del = env->DeleteFile(dbname + "/" + filenames[i]);
+        Status del = env->RemoveFile(dbname + "/" + filenames[i]);
         if (result.ok() && !del.ok()) {
           result = del;
         }
       }
     }
     env->UnlockFile(lock);  // Ignore error since state is already gone
-    env->DeleteFile(lockname);
-    env->DeleteDir(dbname);  // Ignore error in case dir contains other files
+    env->RemoveFile(lockname);
+    env->RemoveDir(dbname);  // Ignore error in case dir contains other files
   }
   return result;
 }
diff --git a/db/db_test.cc b/db/db_test.cc
index 1bd5afc..8cd90f3 100644
--- a/db/db_test.cc
+++ b/db/db_test.cc
@@ -504,7 +504,7 @@ class DBTest : public testing::Test {
     FileType type;
     for (size_t i = 0; i < filenames.size(); i++) {
       if (ParseFileName(filenames[i], &number, &type) && type == kTableFile) {
-        EXPECT_LEVELDB_OK(env_->DeleteFile(TableFileName(dbname_, number)));
+        EXPECT_LEVELDB_OK(env_->RemoveFile(TableFileName(dbname_, number)));
         return true;
       }
     }
@@ -1661,7 +1661,7 @@ TEST_F(DBTest, DBOpen_Options) {
 TEST_F(DBTest, DestroyEmptyDir) {
   std::string dbname = testing::TempDir() + "db_empty_dir";
   TestEnv env(Env::Default());
-  env.DeleteDir(dbname);
+  env.RemoveDir(dbname);
   ASSERT_TRUE(!env.FileExists(dbname));
 
   Options opts;
@@ -1688,7 +1688,7 @@ TEST_F(DBTest, DestroyEmptyDir) {
 
 TEST_F(DBTest, DestroyOpenDB) {
   std::string dbname = testing::TempDir() + "open_db_dir";
-  env_->DeleteDir(dbname);
+  env_->RemoveDir(dbname);
   ASSERT_TRUE(!env_->FileExists(dbname));
 
   Options opts;
@@ -2274,7 +2274,7 @@ void BM_LogAndApply(int iters, int num_base_files) {
 
   for (int i = 0; i < iters; i++) {
     VersionEdit vedit;
-    vedit.DeleteFile(2, fnum);
+    vedit.RemoveFile(2, fnum);
     InternalKey start(MakeKey(2 * fnum), 1, kTypeValue);
     InternalKey limit(MakeKey(2 * fnum + 1), 1, kTypeDeletion);
     vedit.AddFile(2, fnum++, 1 /* file size */, start, limit);
diff --git a/db/fault_injection_test.cc b/db/fault_injection_test.cc
index db8580c..8f2b647 100644
--- a/db/fault_injection_test.cc
+++ b/db/fault_injection_test.cc
@@ -72,7 +72,7 @@ Status Truncate(const std::string& filename, uint64_t length) {
       if (s.ok()) {
         s = env->RenameFile(tmp_name, filename);
       } else {
-        env->DeleteFile(tmp_name);
+        env->RemoveFile(tmp_name);
       }
     }
   }
@@ -133,12 +133,12 @@ class FaultInjectionTestEnv : public EnvWrapper {
                          WritableFile** result) override;
   Status NewAppendableFile(const std::string& fname,
                            WritableFile** result) override;
-  Status DeleteFile(const std::string& f) override;
+  Status RemoveFile(const std::string& f) override;
   Status RenameFile(const std::string& s, const std::string& t) override;
 
   void WritableFileClosed(const FileState& state);
   Status DropUnsyncedFileData();
-  Status DeleteFilesCreatedAfterLastDirSync();
+  Status RemoveFilesCreatedAfterLastDirSync();
   void DirWasSynced();
   bool IsFileCreatedSinceLastDirSync(const std::string& filename);
   void ResetState();
@@ -298,8 +298,8 @@ void FaultInjectionTestEnv::UntrackFile(const std::string& f) {
   new_files_since_last_dir_sync_.erase(f);
 }
 
-Status FaultInjectionTestEnv::DeleteFile(const std::string& f) {
-  Status s = EnvWrapper::DeleteFile(f);
+Status FaultInjectionTestEnv::RemoveFile(const std::string& f) {
+  Status s = EnvWrapper::RemoveFile(f);
   EXPECT_LEVELDB_OK(s);
   if (s.ok()) {
     UntrackFile(f);
@@ -335,17 +335,17 @@ void FaultInjectionTestEnv::ResetState() {
   SetFilesystemActive(true);
 }
 
-Status FaultInjectionTestEnv::DeleteFilesCreatedAfterLastDirSync() {
-  // Because DeleteFile access this container make a copy to avoid deadlock
+Status FaultInjectionTestEnv::RemoveFilesCreatedAfterLastDirSync() {
+  // Because RemoveFile access this container make a copy to avoid deadlock
   mutex_.Lock();
   std::set<std::string> new_files(new_files_since_last_dir_sync_.begin(),
                                   new_files_since_last_dir_sync_.end());
   mutex_.Unlock();
   Status status;
   for (const auto& new_file : new_files) {
-    Status delete_status = DeleteFile(new_file);
-    if (!delete_status.ok() && status.ok()) {
-      status = std::move(delete_status);
+    Status remove_status = RemoveFile(new_file);
+    if (!remove_status.ok() && status.ok()) {
+      status = std::move(remove_status);
     }
   }
   return status;
@@ -477,7 +477,7 @@ class FaultInjectionTest : public testing::Test {
         ASSERT_LEVELDB_OK(env_->DropUnsyncedFileData());
         break;
       case RESET_DELETE_UNSYNCED_FILES:
-        ASSERT_LEVELDB_OK(env_->DeleteFilesCreatedAfterLastDirSync());
+        ASSERT_LEVELDB_OK(env_->RemoveFilesCreatedAfterLastDirSync());
         break;
       default:
         assert(false);
diff --git a/db/filename.cc b/db/filename.cc
index 85de45c..9b451fc 100644
--- a/db/filename.cc
+++ b/db/filename.cc
@@ -133,7 +133,7 @@ Status SetCurrentFile(Env* env, const std::string& dbname,
     s = env->RenameFile(tmp, CurrentFileName(dbname));
   }
   if (!s.ok()) {
-    env->DeleteFile(tmp);
+    env->RemoveFile(tmp);
   }
   return s;
 }
diff --git a/db/recovery_test.cc b/db/recovery_test.cc
index cf6574e..e5cc916 100644
--- a/db/recovery_test.cc
+++ b/db/recovery_test.cc
@@ -95,19 +95,19 @@ class RecoveryTest : public testing::Test {
 
   std::string LogName(uint64_t number) { return LogFileName(dbname_, number); }
 
-  size_t DeleteLogFiles() {
+  size_t RemoveLogFiles() {
     // Linux allows unlinking open files, but Windows does not.
     // Closing the db allows for file deletion.
     Close();
     std::vector<uint64_t> logs = GetFiles(kLogFile);
     for (size_t i = 0; i < logs.size(); i++) {
-      EXPECT_LEVELDB_OK(env_->DeleteFile(LogName(logs[i]))) << LogName(logs[i]);
+      EXPECT_LEVELDB_OK(env_->RemoveFile(LogName(logs[i]))) << LogName(logs[i]);
     }
     return logs.size();
   }
 
-  void DeleteManifestFile() {
-    ASSERT_LEVELDB_OK(env_->DeleteFile(ManifestFileName()));
+  void RemoveManifestFile() {
+    ASSERT_LEVELDB_OK(env_->RemoveFile(ManifestFileName()));
   }
 
   uint64_t FirstLogFile() { return GetFiles(kLogFile)[0]; }
@@ -207,7 +207,7 @@ TEST_F(RecoveryTest, LargeManifestCompacted) {
 
 TEST_F(RecoveryTest, NoLogFiles) {
   ASSERT_LEVELDB_OK(Put("foo", "bar"));
-  ASSERT_EQ(1, DeleteLogFiles());
+  ASSERT_EQ(1, RemoveLogFiles());
   Open();
   ASSERT_EQ("NOT_FOUND", Get("foo"));
   Open();
@@ -322,7 +322,7 @@ TEST_F(RecoveryTest, MultipleLogFiles) {
 TEST_F(RecoveryTest, ManifestMissing) {
   ASSERT_LEVELDB_OK(Put("foo", "bar"));
   Close();
-  DeleteManifestFile();
+  RemoveManifestFile();
 
   Status status = OpenWithStatus();
   ASSERT_TRUE(status.IsCorruption());
diff --git a/db/repair.cc b/db/repair.cc
index d9d12ba..d2a495e 100644
--- a/db/repair.cc
+++ b/db/repair.cc
@@ -341,7 +341,7 @@ class Repairer {
       }
     }
     if (!s.ok()) {
-      env_->DeleteFile(copy);
+      env_->RemoveFile(copy);
     }
   }
 
@@ -386,7 +386,7 @@ class Repairer {
     file = nullptr;
 
     if (!status.ok()) {
-      env_->DeleteFile(tmp);
+      env_->RemoveFile(tmp);
     } else {
       // Discard older manifests
       for (size_t i = 0; i < manifests_.size(); i++) {
@@ -398,7 +398,7 @@ class Repairer {
       if (status.ok()) {
         status = SetCurrentFile(env_, dbname_, 1);
       } else {
-        env_->DeleteFile(tmp);
+        env_->RemoveFile(tmp);
       }
     }
     return status;
diff --git a/db/version_edit.cc b/db/version_edit.cc
index cd770ef..3e9012f 100644
--- a/db/version_edit.cc
+++ b/db/version_edit.cc
@@ -232,7 +232,7 @@ std::string VersionEdit::DebugString() const {
     r.append(compact_pointers_[i].second.DebugString());
   }
   for (const auto& deleted_files_kvp : deleted_files_) {
-    r.append("\n  DeleteFile: ");
+    r.append("\n  RemoveFile: ");
     AppendNumberTo(&r, deleted_files_kvp.first);
     r.append(" ");
     AppendNumberTo(&r, deleted_files_kvp.second);
diff --git a/db/version_edit.h b/db/version_edit.h
index 0de4531..137b4b1 100644
--- a/db/version_edit.h
+++ b/db/version_edit.h
@@ -71,7 +71,7 @@ class VersionEdit {
   }
 
   // Delete the specified "file" from the specified "level".
-  void DeleteFile(int level, uint64_t file) {
+  void RemoveFile(int level, uint64_t file) {
     deleted_files_.insert(std::make_pair(level, file));
   }
 
diff --git a/db/version_edit_test.cc b/db/version_edit_test.cc
index 39ea8b7..acafab0 100644
--- a/db/version_edit_test.cc
+++ b/db/version_edit_test.cc
@@ -27,7 +27,7 @@ TEST(VersionEditTest, EncodeDecode) {
     edit.AddFile(3, kBig + 300 + i, kBig + 400 + i,
                  InternalKey("foo", kBig + 500 + i, kTypeValue),
                  InternalKey("zoo", kBig + 600 + i, kTypeDeletion));
-    edit.DeleteFile(4, kBig + 700 + i);
+    edit.RemoveFile(4, kBig + 700 + i);
     edit.SetCompactPointer(i, InternalKey("x", kBig + 900 + i, kTypeValue));
   }
 
diff --git a/db/version_set.cc b/db/version_set.cc
index cd07346..2d5e51a 100644
--- a/db/version_set.cc
+++ b/db/version_set.cc
@@ -853,7 +853,7 @@ Status VersionSet::LogAndApply(VersionEdit* edit, port::Mutex* mu) {
       delete descriptor_file_;
       descriptor_log_ = nullptr;
       descriptor_file_ = nullptr;
-      env_->DeleteFile(new_manifest_file);
+      env_->RemoveFile(new_manifest_file);
     }
   }
 
@@ -1502,7 +1502,7 @@ bool Compaction::IsTrivialMove() const {
 void Compaction::AddInputDeletions(VersionEdit* edit) {
   for (int which = 0; which < 2; which++) {
     for (size_t i = 0; i < inputs_[which].size(); i++) {
-      edit->DeleteFile(level_ + which, inputs_[which][i]->number);
+      edit->RemoveFile(level_ + which, inputs_[which][i]->number);
     }
   }
 }
diff --git a/helpers/memenv/memenv.cc b/helpers/memenv/memenv.cc
index 31d2bc0..383c78b 100644
--- a/helpers/memenv/memenv.cc
+++ b/helpers/memenv/memenv.cc
@@ -309,7 +309,7 @@ class InMemoryEnv : public EnvWrapper {
     return Status::OK();
   }
 
-  void DeleteFileInternal(const std::string& fname)
+  void RemoveFileInternal(const std::string& fname)
       EXCLUSIVE_LOCKS_REQUIRED(mutex_) {
     if (file_map_.find(fname) == file_map_.end()) {
       return;
@@ -319,19 +319,19 @@ class InMemoryEnv : public EnvWrapper {
     file_map_.erase(fname);
   }
 
-  Status DeleteFile(const std::string& fname) override {
+  Status RemoveFile(const std::string& fname) override {
     MutexLock lock(&mutex_);
     if (file_map_.find(fname) == file_map_.end()) {
       return Status::IOError(fname, "File not found");
     }
 
-    DeleteFileInternal(fname);
+    RemoveFileInternal(fname);
     return Status::OK();
   }
 
   Status CreateDir(const std::string& dirname) override { return Status::OK(); }
 
-  Status DeleteDir(const std::string& dirname) override { return Status::OK(); }
+  Status RemoveDir(const std::string& dirname) override { return Status::OK(); }
 
   Status GetFileSize(const std::string& fname, uint64_t* file_size) override {
     MutexLock lock(&mutex_);
@@ -350,7 +350,7 @@ class InMemoryEnv : public EnvWrapper {
       return Status::IOError(src, "File not found");
     }
 
-    DeleteFileInternal(target);
+    RemoveFileInternal(target);
     file_map_[target] = file_map_[src];
     file_map_.erase(src);
     return Status::OK();
diff --git a/helpers/memenv/memenv_test.cc b/helpers/memenv/memenv_test.cc
index 2001101..3f03cb6 100644
--- a/helpers/memenv/memenv_test.cc
+++ b/helpers/memenv/memenv_test.cc
@@ -83,12 +83,12 @@ TEST_F(MemEnvTest, Basics) {
   ASSERT_TRUE(!rand_file);
 
   // Check that deleting works.
-  ASSERT_TRUE(!env_->DeleteFile("/dir/non_existent").ok());
-  ASSERT_LEVELDB_OK(env_->DeleteFile("/dir/g"));
+  ASSERT_TRUE(!env_->RemoveFile("/dir/non_existent").ok());
+  ASSERT_LEVELDB_OK(env_->RemoveFile("/dir/g"));
   ASSERT_TRUE(!env_->FileExists("/dir/g"));
   ASSERT_LEVELDB_OK(env_->GetChildren("/dir", &children));
   ASSERT_EQ(0, children.size());
-  ASSERT_LEVELDB_OK(env_->DeleteDir("/dir"));
+  ASSERT_LEVELDB_OK(env_->RemoveDir("/dir"));
 }
 
 TEST_F(MemEnvTest, ReadWrite) {
diff --git a/include/leveldb/env.h b/include/leveldb/env.h
index 112fe96..31fb4c4 100644
--- a/include/leveldb/env.h
+++ b/include/leveldb/env.h
@@ -22,21 +22,18 @@
 #include "leveldb/export.h"
 #include "leveldb/status.h"
 
+// This workaround can be removed when leveldb::Env::DeleteFile is removed.
 #if defined(_WIN32)
-// The leveldb::Env class below contains a DeleteFile method.
-// At the same time, <windows.h>, a fairly popular header
-// file for Windows applications, defines a DeleteFile macro.
+// On Windows, the method name DeleteFile (below) introduces the risk of
+// triggering undefined behavior by exposing the compiler to different
+// declarations of the Env class in different translation units.
 //
-// Without any intervention on our part, the result of this
-// unfortunate coincidence is that the name of the
-// leveldb::Env::DeleteFile method seen by the compiler depends on
-// whether <windows.h> was included before or after the LevelDB
-// headers.
+// This is because <windows.h>, a fairly popular header file for Windows
+// applications, defines a DeleteFile macro. So, files that include the Windows
+// header before this header will contain an altered Env declaration.
 //
-// To avoid headaches, we undefined DeleteFile (if defined) and
-// redefine it at the bottom of this file. This way <windows.h>
-// can be included before this file (or not at all) and the
-// exported method will always be leveldb::Env::DeleteFile.
+// This workaround ensures that the compiler sees the same Env declaration,
+// independently of whether <windows.h> was included.
 #if defined(DeleteFile)
 #undef DeleteFile
 #define LEVELDB_DELETEFILE_UNDEFINED
@@ -54,7 +51,7 @@ class WritableFile;
 
 class LEVELDB_EXPORT Env {
  public:
-  Env() = default;
+  Env();
 
   Env(const Env&) = delete;
   Env& operator=(const Env&) = delete;
@@ -124,13 +121,47 @@ class LEVELDB_EXPORT Env {
                              std::vector<std::string>* result) = 0;
 
   // Delete the named file.
-  virtual Status DeleteFile(const std::string& fname) = 0;
+  //
+  // The default implementation calls DeleteFile, to support legacy Env
+  // implementations. Updated Env implementations must override RemoveFile and
+  // ignore the existence of DeleteFile. Updated code calling into the Env API
+  // must call RemoveFile instead of DeleteFile.
+  //
+  // A future release will remove DeleteDir and the default implementation of
+  // RemoveDir.
+  virtual Status RemoveFile(const std::string& fname);
+
+  // DEPRECATED: Modern Env implementations should override RemoveFile instead.
+  //
+  // The default implementation calls RemoveFile, to support legacy Env user
+  // code that calls this method on modern Env implementations. Modern Env user
+  // code should call RemoveFile.
+  //
+  // A future release will remove this method.
+  virtual Status DeleteFile(const std::string& fname);
 
   // Create the specified directory.
   virtual Status CreateDir(const std::string& dirname) = 0;
 
   // Delete the specified directory.
-  virtual Status DeleteDir(const std::string& dirname) = 0;
+  //
+  // The default implementation calls DeleteDir, to support legacy Env
+  // implementations. Updated Env implementations must override RemoveDir and
+  // ignore the existence of DeleteDir. Modern code calling into the Env API
+  // must call RemoveDir instead of DeleteDir.
+  //
+  // A future release will remove DeleteDir and the default implementation of
+  // RemoveDir.
+  virtual Status RemoveDir(const std::string& dirname);
+
+  // DEPRECATED: Modern Env implementations should override RemoveDir instead.
+  //
+  // The default implementation calls RemoveDir, to support legacy Env user
+  // code that calls this method on modern Env implementations. Modern Env user
+  // code should call RemoveDir.
+  //
+  // A future release will remove this method.
+  virtual Status DeleteDir(const std::string& dirname);
 
   // Store the size of fname in *file_size.
   virtual Status GetFileSize(const std::string& fname, uint64_t* file_size) = 0;
@@ -333,14 +364,14 @@ class LEVELDB_EXPORT EnvWrapper : public Env {
                      std::vector<std::string>* r) override {
     return target_->GetChildren(dir, r);
   }
-  Status DeleteFile(const std::string& f) override {
-    return target_->DeleteFile(f);
+  Status RemoveFile(const std::string& f) override {
+    return target_->RemoveFile(f);
   }
   Status CreateDir(const std::string& d) override {
     return target_->CreateDir(d);
   }
-  Status DeleteDir(const std::string& d) override {
-    return target_->DeleteDir(d);
+  Status RemoveDir(const std::string& d) override {
+    return target_->RemoveDir(d);
   }
   Status GetFileSize(const std::string& f, uint64_t* s) override {
     return target_->GetFileSize(f, s);
@@ -375,7 +406,8 @@ class LEVELDB_EXPORT EnvWrapper : public Env {
 
 }  // namespace leveldb
 
-// Redefine DeleteFile if necessary.
+// This workaround can be removed when leveldb::Env::DeleteFile is removed.
+// Redefine DeleteFile if it was undefined earlier.
 #if defined(_WIN32) && defined(LEVELDB_DELETEFILE_UNDEFINED)
 #if defined(UNICODE)
 #define DeleteFile DeleteFileW
diff --git a/util/env.cc b/util/env.cc
index d2f0aef..1ea01c8 100644
--- a/util/env.cc
+++ b/util/env.cc
@@ -4,14 +4,36 @@
 
 #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 +69,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 b35ba05..491ef43 100644
--- a/util/env_test.cc
+++ b/util/env_test.cc
@@ -188,7 +188,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));
@@ -205,14 +205,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));
@@ -229,7 +229,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