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

12 files changed, 42 insertions, 42 deletions

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..ba0a46d 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;
 }
@@ -220,7 +220,7 @@ void DBImpl::MaybeIgnoreError(Status* s) const {
   }
 }
 
-void DBImpl::DeleteObsoleteFiles() {
+void DBImpl::RemoveObsoleteFiles() {
   mutex_.AssertHeld();
 
   if (!bg_error_.ok()) {
@@ -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();
 }
@@ -569,7 +569,7 @@ void DBImpl::CompactMemTable() {
     imm_->Unref();
     imm_ = nullptr;
     has_imm_.store(false, std::memory_order_release);
-    DeleteObsoleteFiles();
+    RemoveObsoleteFiles();
   } else {
     RecordBackgroundError(s);
   }
@@ -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_);
@@ -749,7 +749,7 @@ void DBImpl::BackgroundCompaction() {
     }
     CleanupCompaction(compact);
     c->ReleaseInputs();
-    DeleteObsoleteFiles();
+    RemoveObsoleteFiles();
   }
   delete c;
 
@@ -1506,7 +1506,7 @@ Status DB::Open(const Options& options, const std::string& dbname, DB** dbptr) {
     s = impl->versions_->LogAndApply(&edit, &impl->mutex_);
   }
   if (s.ok()) {
-    impl->DeleteObsoleteFiles();
+    impl->RemoveObsoleteFiles();
     impl->MaybeScheduleCompaction();
   }
   impl->mutex_.Unlock();
@@ -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_impl.h b/db/db_impl.h
index 685735c..c7b0172 100644
--- a/db/db_impl.h
+++ b/db/db_impl.h
@@ -116,7 +116,7 @@ class DBImpl : public DB {
   void MaybeIgnoreError(Status* s) const;
 
   // Delete any unneeded files and stale in-memory entries.
-  void DeleteObsoleteFiles() EXCLUSIVE_LOCKS_REQUIRED(mutex_);
+  void RemoveObsoleteFiles() EXCLUSIVE_LOCKS_REQUIRED(mutex_);
 
   // Compact the in-memory write buffer to disk.  Switches to a new
   // log-file/memtable and writes a new descriptor iff successful.
diff --git a/db/db_test.cc b/db/db_test.cc
index 3f41c36..2ee6761 100644
--- a/db/db_test.cc
+++ b/db/db_test.cc
@@ -509,7 +509,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;
       }
     }
@@ -1666,7 +1666,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;
@@ -1693,7 +1693,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;
@@ -2279,7 +2279,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 b2d2adb..60e4631 100644
--- a/db/fault_injection_test.cc
+++ b/db/fault_injection_test.cc
@@ -77,7 +77,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);
       }
     }
   }
@@ -138,12 +138,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();
@@ -303,8 +303,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);
@@ -340,17 +340,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;
@@ -482,7 +482,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 ea137e6..04b39ae 100644
--- a/db/recovery_test.cc
+++ b/db/recovery_test.cc
@@ -100,19 +100,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]; }
@@ -212,7 +212,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();
@@ -327,7 +327,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);
     }
   }
 }