Remove dead code (#15407)

Summary:
Pull Request resolved: https://github.com/meta/rocksdb/pull/15407

X-link: https://github.com/facebook/rocksdb/pull/15407

## Summary

A full clang-tidy scan of RocksDB reports 65 dead-code findings in
existing code:
- `clang-analyzer-deadcode.DeadStores` (20)
- `-Wunused-member-function` (24)
- `-Wunreachable-code-return` / `-Wunreachable-code-loop-increment` (6)
- `-Wmissing-noreturn` (9)
- `-Wextra-semi` (6)

Every PR that touches nearby lines gets flagged for them, and some hide
real mistakes (an unchecked result, a loop that can only run once). This
diff fixes 57 of them.

What changed:
- Unused private and file-local member functions are removed, along with
  members that only fed them and local classes that are never
  instantiated. Examples: `EmptyIterator` in the hash memtable reps,
  `PersistentCacheFromCache` in `db_block_cache_test.cc`, and
  `LegacyFileSystemWrapper::target()`. Each one was confirmed unused in
  every build configuration and is file-local or private.
- Unreachable code after `return`, after a `throw`, or after an infinite
  loop is removed, but only where every compiler still sees every path
  returning. A loop that always returned on its first pass is now an
  `if`. A trailing return after `#ifdef` branches is now an
  `#else`/`#endif`.
- Dead stores are removed. Google Benchmark loop variables are marked
  `[[maybe_unused]]` (`for ([[maybe_unused]] auto _ : state)`).
- `[[noreturn]]` is added to functions that never return, such as
  `ErrorExit` in `db_bench`.
- Stray `;` after member function bodies are removed.

Not changed: `[[noreturn]]` was not added to `NoopDelete` helpers whose
body is `assert(false)`, because they return in release builds. It was
also not added to `MemTableRep::InsertConcurrently` (public virtual) or to
`LDBTool::Run` (public, deprecated), and lambdas have no portable C++20
syntax for it.

Reviewed By: xingbowang

Differential Revision: D123186311

fbshipit-source-id: 378348c02a494aef6786827e3d18b44cbd280a04

db/column_family_test.cc

@@ -468,7 +468,6 @@ class ColumnFamilyTestBase : public testing::Test {
       }
     }
     return ret;
-    return 0;
   }
 
   void AssertCountLiveLogFiles(int value) {

db/compaction/compaction_service_test.cc

@@ -3413,9 +3413,9 @@ TEST_F(CompactionServiceTest, EmptyResult) {
           ColumnFamilyMetaData cf_meta;
           db_->GetColumnFamilyMetaData(&cf_meta);
           for (const auto& level : cf_meta.levels) {
-            for (const auto& file : level.files) {
+            if (!level.files.empty()) {
               // Use one of the existing file name
-              *n = test::GetFileNumber(file.name);
+              *n = test::GetFileNumber(level.files.front().name);
               need_to_override_file_number = false;
               return;
             }

db/db_basic_test.cc

@@ -6293,7 +6293,7 @@ class DBBasicTestTrackWal : public DBTestBase,
     VectorLogPtr log_files;
     EXPECT_OK(dbfull()->GetSortedWalFiles(log_files));
     return static_cast<int>(log_files.size());
-  };
+  }
 };
 
 TEST_P(DBBasicTestTrackWal, DoNotTrackObsoleteWal) {
@@ -6663,7 +6663,6 @@ TEST_P(DBBasicTestWithParallelIO, MultiGet) {
 
 TEST_P(DBBasicTestWithParallelIO, MultiGetDirectIO) {
   class FakeDirectIOEnv : public EnvWrapper {
-    class FakeDirectIOSequentialFile;
     class FakeDirectIORandomAccessFile;
 
    public:
@@ -6687,19 +6686,6 @@ TEST_P(DBBasicTestWithParallelIO, MultiGetDirectIO) {
     }
 
    private:
-    class FakeDirectIOSequentialFile : public SequentialFileWrapper {
-     public:
-      FakeDirectIOSequentialFile(std::unique_ptr<SequentialFile>&& file)
-          : SequentialFileWrapper(file.get()), file_(std::move(file)) {}
-      ~FakeDirectIOSequentialFile() override {}
-
-      bool use_direct_io() const override { return true; }
-      size_t GetRequiredBufferAlignment() const override { return 1; }
-
-     private:
-      std::unique_ptr<SequentialFile> file_;
-    };
-
     class FakeDirectIORandomAccessFile : public RandomAccessFileWrapper {
      public:
       FakeDirectIORandomAccessFile(std::unique_ptr<RandomAccessFile>&& file)

db/db_block_cache_test.cc

@@ -20,7 +20,6 @@
 #include "db/db_test_util.h"
 #include "env/unique_id_gen.h"
 #include "port/stack_trace.h"
-#include "rocksdb/persistent_cache.h"
 #include "rocksdb/statistics.h"
 #include "rocksdb/table.h"
 #include "rocksdb/table_properties.h"
@@ -240,53 +239,6 @@ TEST_F(DBBlockCacheTest, TestWithoutCompressedBlockCache) {
 #ifdef SNAPPY
 
 namespace {
-class PersistentCacheFromCache : public PersistentCache {
- public:
-  PersistentCacheFromCache(std::shared_ptr<Cache> cache, bool read_only)
-      : cache_(cache), read_only_(read_only) {}
-
-  Status Insert(const Slice& key, const char* data,
-                const size_t size) override {
-    if (read_only_) {
-      return Status::NotSupported();
-    }
-    std::unique_ptr<char[]> copy{new char[size]};
-    std::copy_n(data, size, copy.get());
-    Status s = cache_.Insert(key, copy.get(), size);
-    if (s.ok()) {
-      copy.release();
-    }
-    return s;
-  }
-
-  Status Lookup(const Slice& key, std::unique_ptr<char[]>* data,
-                size_t* size) override {
-    auto handle = cache_.Lookup(key);
-    if (handle) {
-      char* ptr = cache_.Value(handle);
-      *size = cache_.get()->GetCharge(handle);
-      data->reset(new char[*size]);
-      std::copy_n(ptr, *size, data->get());
-      cache_.Release(handle);
-      return Status::OK();
-    } else {
-      return Status::NotFound();
-    }
-  }
-
-  bool IsCompressed() override { return false; }
-
-  StatsType Stats() override { return StatsType(); }
-
-  std::string GetPrintableOptions() const override { return ""; }
-
-  uint64_t NewId() override { return cache_.get()->NewId(); }
-
- private:
-  BasicTypedSharedCacheInterface<char[], CacheEntryRole::kMisc> cache_;
-  bool read_only_;
-};
-
 class ReadOnlyCacheWrapper : public CacheWrapper {
  public:
   using CacheWrapper::CacheWrapper;

db/db_compaction_test.cc

@@ -594,8 +594,6 @@ class FlushedFileCollector : public EventListener {
     return result;
   }
 
-  void ClearFlushedFiles() { flushed_files_.clear(); }
-
  private:
   std::vector<std::string> flushed_files_;
   std::mutex mutex_;

db/db_io_failure_test.cc

@@ -225,8 +225,6 @@ class CorruptionFS : public FileSystemWrapper {
 
   bool VerifyRetry() { return corrupt_len_ > 0 && corrupt_fname_.empty(); }
 
-  int read_count() { return read_count_; }
-
   int corruption_trigger() { return corruption_trigger_; }
 
  private:

db/db_properties_test.cc

@@ -1971,13 +1971,10 @@ TEST_F(DBPropertiesTest, MinObsoleteSstNumberToKeep) {
 
     void SetDB(DB* db) { db_ = db; }
 
-    int GetNumCompactions() { return num_compactions_; }
-
     // True if we've verified the property for at least one output file
     bool Validated() { return validated_; }
 
    private:
-    int num_compactions_ = 0;
     bool validated_ = false;
     DB* db_ = nullptr;
   };

db/log_writer.h

@@ -130,7 +130,7 @@ class Writer {
 
   size_t TEST_block_offset() const { return block_offset_; }
 
-  SequenceNumber GetLastSeqnoRecorded() const { return last_seqno_recorded_; };
+  SequenceNumber GetLastSeqnoRecorded() const { return last_seqno_recorded_; }
 
  private:
   std::unique_ptr<WritableFileWriter> dest_;

db_stress_tool/db_stress_compaction_service.cc

@@ -59,7 +59,6 @@ CompactionServiceJobStatus DbStressCompactionService::Wait(
       Env::Default()->SleepForMicroseconds(kWaitIntervalInMicros);
     }
   }
-  return CompactionServiceJobStatus::kFailure;
 }
 
 }  // namespace ROCKSDB_NAMESPACE

env/composite_env.cc

@@ -219,8 +219,6 @@ class CompositeWritableFileWrapper : public WritableFile {
     return target_->Allocate(offset, len, io_opts, &dbg);
   }
 
-  std::unique_ptr<FSWritableFile>* target() { return &target_; }
-
  private:
   std::unique_ptr<FSWritableFile> target_;
 };

env/env.cc

@@ -378,10 +378,7 @@ class LegacyFileSystemWrapper : public FileSystem {
   static const char* kClassName() { return "LegacyFileSystem"; }
   const char* Name() const override { return kClassName(); }
 
-  // Return the target to which this Env forwards all calls
-  Env* target() const { return target_; }
-
-  // The following text is boilerplate that forwards all methods to target()
+  // The following text is boilerplate that forwards all methods to target_
   IOStatus NewSequentialFile(const std::string& f, const FileOptions& file_opts,
                              std::unique_ptr<FSSequentialFile>* r,
                              IODebugContext* /*dbg*/) override {

env/env_posix.cc

@@ -238,12 +238,6 @@ class PosixEnv : public CompositeEnv {
     PosixEnv& deflt;
   };
 
-  void SetFD_CLOEXEC(int fd, const EnvOptions* options) {
-    if ((options == nullptr || options->set_fd_cloexec) && fd > 0) {
-      fcntl(fd, F_SETFD, fcntl(fd, F_GETFD) | FD_CLOEXEC);
-    }
-  }
-
 #ifndef ROCKSDB_NO_DYNAMIC_EXTENSION
   // Loads the named library into the result.
   // If the input name is empty, the current executable is loaded

memtable/hash_linklist_rep.cc

@@ -75,8 +75,6 @@ struct Node {
     next_.store(x, std::memory_order_release);
   }
   // No-barrier variants that can be safely used in a few locations.
-  Node* NoBarrier_Next() { return next_.load(std::memory_order_relaxed); }
-
   void NoBarrier_SetNext(Node* x) { next_.store(x, std::memory_order_relaxed); }
 
   // Needed for placement new below which is fine
@@ -249,18 +247,7 @@ class HashLinkListRep : public MemTableRep {
     return (n != nullptr) && (compare_(n->key, key) < 0);
   }
 
-  bool KeyIsAfterOrAtNode(const Slice& internal_key, const Node* n) const {
-    // nullptr n is considered infinite
-    return (n != nullptr) && (compare_(n->key, internal_key) <= 0);
-  }
-
-  bool KeyIsAfterOrAtNode(const Key& key, const Node* n) const {
-    // nullptr n is considered infinite
-    return (n != nullptr) && (compare_(n->key, key) <= 0);
-  }
-
   Node* FindGreaterOrEqualInBucket(Node* head, const Slice& key) const;
-  Node* FindLessOrEqualInBucket(Node* head, const Slice& key) const;
 
   class FullListIterator : public MemTableRep::Iterator {
    public:
@@ -479,28 +466,6 @@ class HashLinkListRep : public MemTableRep {
     const HashLinkListRep& memtable_rep_;
     std::unique_ptr<MemtableSkipList::Iterator> skip_list_iter_;
   };
-
-  class EmptyIterator : public MemTableRep::Iterator {
-    // This is used when there wasn't a bucket. It is cheaper than
-    // instantiating an empty bucket over which to iterate.
-   public:
-    EmptyIterator() = default;
-    bool Valid() const override { return false; }
-    const char* key() const override {
-      assert(false);
-      return nullptr;
-    }
-    void Next() override {}
-    void Prev() override {}
-    void Seek(const Slice& /*user_key*/,
-              const char* /*memtable_key*/) override {}
-    void SeekForPrev(const Slice& /*user_key*/,
-                     const char* /*memtable_key*/) override {}
-    void SeekToFirst() override {}
-    void SeekToLast() override {}
-
-   private:
-  };
 };
 
 HashLinkListRep::HashLinkListRep(

memtable/hash_skiplist_rep.cc

@@ -208,28 +208,6 @@ class HashSkipListRep : public MemTableRep {
     // the underlying memtable
     const HashSkipListRep& memtable_rep_;
   };
-
-  class EmptyIterator : public MemTableRep::Iterator {
-    // This is used when there wasn't a bucket. It is cheaper than
-    // instantiating an empty bucket over which to iterate.
-   public:
-    EmptyIterator() = default;
-    bool Valid() const override { return false; }
-    const char* key() const override {
-      assert(false);
-      return nullptr;
-    }
-    void Next() override {}
-    void Prev() override {}
-    void Seek(const Slice& /*internal_key*/,
-              const char* /*memtable_key*/) override {}
-    void SeekForPrev(const Slice& /*internal_key*/,
-                     const char* /*memtable_key*/) override {}
-    void SeekToFirst() override {}
-    void SeekToLast() override {}
-
-   private:
-  };
 };
 
 HashSkipListRep::HashSkipListRep(const MemTableRep::KeyComparator& compare,

microbench/db_basic_bench.cc

@@ -178,7 +178,7 @@ static void DBOpen(benchmark::State& state) {
 
   auto rnd = Random(123);
 
-  for (auto _ : state) {
+  for ([[maybe_unused]] auto _ : state) {
     {
       Status s = DB::Open(options, db_name, &db);
       if (!s.ok()) {
@@ -225,7 +225,7 @@ static void DBClose(benchmark::State& state) {
 
   auto rnd = Random(12345);
 
-  for (auto _ : state) {
+  for ([[maybe_unused]] auto _ : state) {
     state.PauseTiming();
     {
       Status s = DB::Open(options, db_name, &db);
@@ -285,7 +285,7 @@ static void DBPut(benchmark::State& state) {
   auto wo = WriteOptions();
   wo.disableWAL = !enable_wal;
 
-  for (auto _ : state) {
+  for ([[maybe_unused]] auto _ : state) {
     state.PauseTiming();
     Slice key = kg.Next();
     std::string val = rnd.RandomString(static_cast<int>(per_key_size));
@@ -396,7 +396,7 @@ static void ManualCompaction(benchmark::State& state) {
   get_perf_context()->EnablePerLevelPerfContext();
   get_perf_context()->Reset();
   CompactionOptions co;
-  for (auto _ : state) {
+  for ([[maybe_unused]] auto _ : state) {
     s = db->CompactFiles(co, files_before_compact, 1);
     if (!s.ok()) {
       state.SkipWithError(s.ToString().c_str());
@@ -489,7 +489,7 @@ static void ManualFlush(benchmark::State& state) {
   }
 
   auto wo = WriteOptions();
-  for (auto _ : state) {
+  for ([[maybe_unused]] auto _ : state) {
     state.PauseTiming();
     for (uint64_t i = 0; i < key_num; i++) {
       Status s = db->Put(wo, kg.Next(),
@@ -630,7 +630,7 @@ static void DBGet(benchmark::State& state) {
   }
   size_t not_found = 0;
   if (negative_query) {
-    for (auto _ : state) {
+    for ([[maybe_unused]] auto _ : state) {
       std::string val;
       Status s = db->Get(ro, kg_rnd.NextNonExist(), &val);
       if (s.IsNotFound()) {
@@ -638,7 +638,7 @@ static void DBGet(benchmark::State& state) {
       }
     }
   } else {
-    for (auto _ : state) {
+    for ([[maybe_unused]] auto _ : state) {
       std::string val;
       Status s = db->Get(ro, kg_rnd.Next(), &val);
       if (s.IsNotFound()) {
@@ -765,7 +765,7 @@ static void SimpleGetWithPerfContext(benchmark::State& state) {
   uint64_t get_from_table_nanos = 0;
   SetPerfLevel(kEnableTime);
   get_perf_context()->EnablePerLevelPerfContext();
-  for (auto _ : state) {
+  for ([[maybe_unused]] auto _ : state) {
     std::string val;
     get_perf_context()->Reset();
     Status s = db->Get(ro, kg.NextNonExist(), &val);
@@ -868,7 +868,7 @@ static void DBGetMergeOperandsInMemtable(benchmark::State& state) {
   GetMergeOperandsOptions get_merge_ops_opts;
   get_merge_ops_opts.expected_max_number_of_operands =
       static_cast<int>(kNumEntriesPerKey);
-  for (auto _ : state) {
+  for ([[maybe_unused]] auto _ : state) {
     int num_value_operands = 0;
     Status s = db->GetMergeOperands(
         ReadOptions(), db->DefaultColumnFamily(), random_key_gen.Next(),
@@ -966,7 +966,7 @@ static void DBGetMergeOperandsInSstFile(benchmark::State& state) {
   GetMergeOperandsOptions get_merge_ops_opts;
   get_merge_ops_opts.expected_max_number_of_operands =
       static_cast<int>(kNumEntriesPerKey);
-  for (auto _ : state) {
+  for ([[maybe_unused]] auto _ : state) {
     int num_value_operands = 0;
     ReadOptions read_opts;
     read_opts.verify_checksums = false;
@@ -1083,7 +1083,7 @@ static void DataBlockSeek(benchmark::State& state) {
 
   SetPerfLevel(kEnableTime);
   uint64_t total = 0;
-  for (auto _ : state) {
+  for ([[maybe_unused]] auto _ : state) {
     DataBlockIter* iter = reader.NewDataIterator(options.comparator,
                                                  kDisableGlobalSequenceNumber);
     uint32_t index = rnd.Uniform(static_cast<int>(num_records));
@@ -1157,7 +1157,7 @@ static void IteratorSeek(benchmark::State& state) {
     }
   }
 
-  for (auto _ : state) {
+  for ([[maybe_unused]] auto _ : state) {
     std::unique_ptr<Iterator> iter{nullptr};
     state.PauseTiming();
     if (!iter) {
@@ -1248,7 +1248,7 @@ static void IteratorNext(benchmark::State& state) {
     }
   }
 
-  for (auto _ : state) {
+  for ([[maybe_unused]] auto _ : state) {
     std::unique_ptr<Iterator> iter{nullptr};
     state.PauseTiming();
     if (!iter) {
@@ -1325,7 +1325,7 @@ static void IteratorNextWithPerfContext(benchmark::State& state) {
   SetPerfLevel(kEnableTime);
   get_perf_context()->EnablePerLevelPerfContext();
 
-  for (auto _ : state) {
+  for ([[maybe_unused]] auto _ : state) {
     std::unique_ptr<Iterator> iter{nullptr};
     state.PauseTiming();
     if (!iter) {
@@ -1410,7 +1410,7 @@ static void IteratorPrev(benchmark::State& state) {
     }
   }
 
-  for (auto _ : state) {
+  for ([[maybe_unused]] auto _ : state) {
     std::unique_ptr<Iterator> iter{nullptr};
     state.PauseTiming();
     if (!iter) {
@@ -1502,7 +1502,7 @@ static void PrefixSeek(benchmark::State& state) {
     }
   }
 
-  for (auto _ : state) {
+  for ([[maybe_unused]] auto _ : state) {
     std::unique_ptr<Iterator> iter{nullptr};
     state.PauseTiming();
     if (!iter) {
@@ -1591,7 +1591,7 @@ static void RandomAccessFileReaderRead(benchmark::State& state) {
   std::unique_ptr<char[]> scratch(new char[2048]);
   Slice result;
   uint64_t idx = 0;
-  for (auto _ : state) {
+  for ([[maybe_unused]] auto _ : state) {
     s = readers[idx++ % kFileNum]->Read(0, kDefaultPageSize / 3, io_options,
                                         &result, scratch.get(), nullptr);
     if (!s.ok()) {

microbench/ribbon_bench.cc

@@ -78,7 +78,7 @@ static void FilterBuild(benchmark::State& state) {
   auto rnd = Random32(12345);
   uint32_t filter_num = rnd.Next();
   // run the test
-  for (auto _ : state) {
+  for ([[maybe_unused]] auto _ : state) {
     std::unique_ptr<FilterBitsBuilder> builder(tester->GetBuilder());
     for (uint32_t i = 0; i < kEntryNum; i++) {
       builder->AddKey(km.Get(filter_num, i));
@@ -109,7 +109,7 @@ static void FilterQueryPositive(benchmark::State& state) {
 
   // run test
   uint32_t i = 0;
-  for (auto _ : state) {
+  for ([[maybe_unused]] auto _ : state) {
     i++;
     i = i % kEntryNum;
     reader->MayMatch(km.Get(filter_num, i));
@@ -138,7 +138,7 @@ static void FilterQueryNegative(benchmark::State& state) {
   // run test
   uint32_t i = 0;
   double fp_cnt = 0;
-  for (auto _ : state) {
+  for ([[maybe_unused]] auto _ : state) {
     i++;
     auto result = reader->MayMatch(km.Get(filter_num + 1, i));
     if (result) {

options/customizable_test.cc

@@ -59,21 +59,6 @@ DEFINE_bool(enable_print, false, "Print options generated to console.");
 
 namespace ROCKSDB_NAMESPACE {
 namespace {
-class StringLogger : public Logger {
- public:
-  using Logger::Logv;
-  void Logv(const char* format, va_list ap) override {
-    char buffer[1000];
-    vsnprintf(buffer, sizeof(buffer), format, ap);
-    string_.append(buffer);
-  }
-  const std::string& str() const { return string_; }
-  void clear() { string_.clear(); }
-
- private:
-  std::string string_;
-};
-
 class TestCustomizable : public Customizable {
  public:
   TestCustomizable(const std::string& name) : name_(name) {}
@@ -216,11 +201,6 @@ class SimpleConfigurable : public Configurable {
 
  public:
   SimpleConfigurable() { RegisterOptions(&simple_, &simple_option_info); }
-
-  explicit SimpleConfigurable(
-      const std::unordered_map<std::string, OptionTypeInfo>* map) {
-    RegisterOptions(&simple_, map);
-  }
 };
 
 static void GetMapFromProperties(

table/block_based/block.h

@@ -114,8 +114,9 @@ class BlockReadAmpBitmap {
   size_t ApproximateMemoryUsage() const {
 #ifdef ROCKSDB_MALLOC_USABLE_SIZE
     return malloc_usable_size((void*)this);
-#endif  // ROCKSDB_MALLOC_USABLE_SIZE
+#else
     return sizeof(*this);
+#endif  // ROCKSDB_MALLOC_USABLE_SIZE
   }
 
  private:

table/block_based/mock_block_based_table.h

@@ -32,7 +32,7 @@ class MockBlockBasedTableTester {
 
   explicit MockBlockBasedTableTester(const FilterPolicy* filter_policy)
       : MockBlockBasedTableTester(
-            std::shared_ptr<const FilterPolicy>(filter_policy)) {};
+            std::shared_ptr<const FilterPolicy>(filter_policy)) {}
 
   explicit MockBlockBasedTableTester(
       std::shared_ptr<const FilterPolicy> filter_policy)

table/block_fetcher_test.cc

@@ -281,13 +281,6 @@ class BlockFetcherTest : public testing::Test {
 
   std::string Path(const std::string& fname) { return test_dir_ + "/" + fname; }
 
-  void WriteToFile(const std::string& content, const std::string& filename) {
-    std::unique_ptr<FSWritableFile> f;
-    ASSERT_OK(fs_->NewWritableFile(Path(filename), FileOptions(), &f, nullptr));
-    ASSERT_OK(f->Append(content, IOOptions(), nullptr));
-    ASSERT_OK(f->Close(IOOptions(), nullptr));
-  }
-
   void NewFileWriter(const std::string& filename,
                      std::unique_ptr<WritableFileWriter>* writer) {
     std::string path = Path(filename);

table/table_test.cc

@@ -6368,7 +6368,6 @@ namespace {
 class TestPrefixExtractor : public ROCKSDB_NAMESPACE::SliceTransform {
  public:
   ~TestPrefixExtractor() override = default;
-  ;
   const char* Name() const override { return "TestPrefixExtractor"; }
 
   ROCKSDB_NAMESPACE::Slice Transform(

table/two_level_iterator.cc

@@ -75,11 +75,6 @@ class TwoLevelIndexIterator : public InternalIteratorBase<IndexValue> {
   bool IsValuePinned() const override { return false; }
 
  private:
-  void SaveError(const Status& s) {
-    if (status_.ok() && !s.ok()) {
-      status_ = s;
-    }
-  }
   void SkipEmptyDataBlocksForward();
   void SkipEmptyDataBlocksBackward();
   void SetSecondLevelIterator(InternalIteratorBase<IndexValue>* iter);

tools/db_bench_tool.cc

@@ -4574,7 +4574,7 @@ class Benchmark {
     fprintf(stderr, "...Verified\n");
   }
 
-  void ErrorExit() {
+  [[noreturn]] void ErrorExit() {
     if (holds_db_use_guard_ || is_secondary_update_thread_) {
       // Neither kind of DB-user context can safely run the mutation cleanup
       // path:

util/duplicate_detector.h

@@ -60,7 +60,6 @@ class DuplicateDetector {
           "Recovering an entry from a dropped column family. "
           "WAL must must have been flushed before dropping the column "
           "family");
-      return;
     }
     auto cmp = h->GetComparator();
     keys_[cf] = CFKeys(SetComparator(cmp));

util/filelock_test.cc

@@ -106,7 +106,6 @@ class LockTest : public testing::Test {
       fprintf(stderr, "Fork failed\n");
       return false;
     }
-    return false;
 
 #endif
   }

utilities/backup/backup_engine.cc

@@ -401,7 +401,6 @@ class BackupEngineImpl {
     void SetSequenceNumber(uint64_t sequence_number) {
       sequence_number_ = sequence_number;
     }
-    uint64_t GetSequenceNumber() const { return sequence_number_; }
 
     const std::string& GetAppMetadata() const { return app_metadata_; }
 
@@ -419,14 +418,6 @@ class BackupEngineImpl {
 
     bool Empty() const { return files_.empty(); }
 
-    std::shared_ptr<FileInfo> GetFile(const std::string& filename) const {
-      auto it = file_infos_->find(filename);
-      if (it == file_infos_->end()) {
-        return nullptr;
-      }
-      return it->second;
-    }
-
     const std::vector<std::shared_ptr<FileInfo>>& GetFiles() const {
       return files_;
     }
@@ -606,8 +597,6 @@ class BackupEngineImpl {
     std::string dst_path_tmp;
     std::string dst_path;
     std::string dst_relative;
-    BackupAfterCopyOrCreateWorkItem()
-        : shared(false), needed_to_copy(false), backup_env(nullptr) {}
 
     BackupAfterCopyOrCreateWorkItem(
         BackupAfterCopyOrCreateWorkItem&& o) noexcept {
@@ -679,7 +668,6 @@ class BackupEngineImpl {
     std::string from_file;
     std::string to_file;
     std::string checksum_hex;
-    RestoreAfterCopyOrCreateWorkItem() {}
     RestoreAfterCopyOrCreateWorkItem(std::future<WorkItemResult>&& _result,
                                      const std::string& _from_file,
                                      const std::string& _to_file,

utilities/backup/backup_engine_test.cc

@@ -256,9 +256,6 @@ class TestFs : public FileSystemWrapper {
     limit_written_files_--;
     IOStatus s = FileSystemWrapper::NewWritableFile(f, file_opts, r, dbg);
     if (s.ok()) {
-      if ((*r)->use_direct_io()) {
-        ++num_direct_writers_;
-      }
       ++num_writers_;
     }
     return s;
@@ -421,7 +418,6 @@ class TestFs : public FileSystemWrapper {
     num_seq_readers_ = 0;
     num_direct_seq_readers_ = 0;
     num_writers_ = 0;
-    num_direct_writers_ = 0;
   }
 
   int num_rand_readers() { return num_rand_readers_; }
@@ -429,8 +425,6 @@ class TestFs : public FileSystemWrapper {
   int num_seq_readers() { return num_seq_readers_; }
   int num_direct_seq_readers() { return num_direct_seq_readers_; }
   int num_writers() { return num_writers_; }
-  // FIXME(?): unused
-  int num_direct_writers() { return num_direct_writers_; }
 
  private:
   port::Mutex mutex_;
@@ -454,7 +448,6 @@ class TestFs : public FileSystemWrapper {
   std::atomic<int> num_seq_readers_{};
   std::atomic<int> num_direct_seq_readers_{};
   std::atomic<int> num_writers_{};
-  std::atomic<int> num_direct_writers_{};
 };  // TestFs
 
 class FileManager : public EnvWrapper {

utilities/transactions/transaction_test.h

@@ -567,7 +567,7 @@ class MySQLStyleTransactionTest
       options.write_buffer_size = 1024;
       EXPECT_OK(ReOpen());
     }
-  };
+  }
 
  protected:
   // Also emulate slow threads by addin artiftial delays

utilities/transactions/write_prepared_transaction_test.cc

@@ -688,7 +688,7 @@ class SeqAdvanceConcurrentTest
         split_id_(std::get<4>(GetParam())),
         split_cnt_(std::get<5>(GetParam())) {
     special_env.skip_fsync_ = true;
-  };
+  }
 
  protected:
   // A test is split into split_cnt_ tests, each identified with split_id_ where