Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add -Wsuggest-destructor-override check #13213

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -553,6 +553,9 @@ ifdef USE_CLANG
# Used by some teams in Facebook
WARNING_FLAGS += -Wshift-sign-overflow -Wambiguous-reversed-operator \
-Wimplicit-fallthrough -Wreinterpret-base-class -Wundefined-reinterpret-cast
ifeq ($(CC), clang-13)
WARNING_FLAGS += -Wsuggest-destructor-override
endif
endif

ifeq ($(PLATFORM), OS_OPENBSD)
Expand Down
4 changes: 2 additions & 2 deletions cache/sharded_cache.h
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ class CacheShardBase {
class ShardedCacheBase : public Cache {
public:
explicit ShardedCacheBase(const ShardedCacheOptions& opts);
virtual ~ShardedCacheBase() = default;
virtual ~ShardedCacheBase() override = default;

int GetNumShardBits() const;
uint32_t GetNumShards() const;
Expand Down Expand Up @@ -143,7 +143,7 @@ class ShardedCache : public ShardedCacheBase {
sizeof(CacheShard) * GetNumShards()))),
destroy_shards_in_dtor_(false) {}

virtual ~ShardedCache() {
virtual ~ShardedCache() override {
if (destroy_shards_in_dtor_) {
ForEachShard([](CacheShard* cs) { cs->~CacheShard(); });
}
Expand Down
2 changes: 1 addition & 1 deletion db/column_family.h
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ class ColumnFamilyHandleImpl : public ColumnFamilyHandle {
ColumnFamilyHandleImpl(ColumnFamilyData* cfd, DBImpl* db,
InstrumentedMutex* mutex);
// destroy without mutex
virtual ~ColumnFamilyHandleImpl();
virtual ~ColumnFamilyHandleImpl() override;
virtual ColumnFamilyData* cfd() const { return cfd_; }
virtual DBImpl* db() const { return db_; }

Expand Down
2 changes: 1 addition & 1 deletion db/compaction/compaction_picker.h
Original file line number Diff line number Diff line change
Expand Up @@ -255,7 +255,7 @@ class NullCompactionPicker : public CompactionPicker {
NullCompactionPicker(const ImmutableOptions& ioptions,
const InternalKeyComparator* icmp)
: CompactionPicker(ioptions, icmp) {}
virtual ~NullCompactionPicker() {}
virtual ~NullCompactionPicker() override {}

// Always return "nullptr"
Compaction* PickCompaction(
Expand Down
6 changes: 3 additions & 3 deletions db/db_impl/db_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@ class DBImpl : public DB {
DBImpl(const DBImpl&) = delete;
void operator=(const DBImpl&) = delete;

virtual ~DBImpl();
virtual ~DBImpl() override;

// ---- Implementations of the DB interface ----

Expand Down Expand Up @@ -1921,8 +1921,8 @@ class DBImpl : public DB {
const InternalKey* begin = nullptr; // nullptr means beginning of key range
const InternalKey* end = nullptr; // nullptr means end of key range
InternalKey* manual_end = nullptr; // how far we are compacting
InternalKey tmp_storage; // Used to keep track of compaction progress
InternalKey tmp_storage1; // Used to keep track of compaction progress
InternalKey tmp_storage; // Used to keep track of compaction progress
InternalKey tmp_storage1; // Used to keep track of compaction progress

// When the user provides a canceled pointer in CompactRangeOptions, the
// above varaibe is the reference of the user-provided
Expand Down
2 changes: 1 addition & 1 deletion db/db_impl/db_impl_follower.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ class DBImplFollower : public DBImplSecondary {
public:
DBImplFollower(const DBOptions& db_options, std::unique_ptr<Env>&& env,
const std::string& dbname, std::string src_path);
~DBImplFollower();
~DBImplFollower() override;

Status Close() override;

Expand Down
3 changes: 1 addition & 2 deletions db/db_impl/db_impl_readonly.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@

#pragma once


#include <string>
#include <vector>

Expand All @@ -21,7 +20,7 @@ class DBImplReadOnly : public DBImpl {
DBImplReadOnly(const DBImplReadOnly&) = delete;
void operator=(const DBImplReadOnly&) = delete;

virtual ~DBImplReadOnly();
virtual ~DBImplReadOnly() override;

// Implementations of the DB interface
using DBImpl::GetImpl;
Expand Down
8 changes: 5 additions & 3 deletions db/db_test_util.h
Original file line number Diff line number Diff line change
Expand Up @@ -282,7 +282,9 @@ class SpecialEnv : public EnvWrapper {
: env_(env), base_(std::move(b)) {
env_->num_open_wal_file_.fetch_add(1);
}
virtual ~SpecialWalFile() { env_->num_open_wal_file_.fetch_add(-1); }
virtual ~SpecialWalFile() override {
env_->num_open_wal_file_.fetch_add(-1);
}
Status Append(const Slice& data) override {
#if !(defined NDEBUG) || !defined(OS_WIN)
TEST_SYNC_POINT("SpecialEnv::SpecialWalFile::Append:1");
Expand Down Expand Up @@ -594,7 +596,7 @@ class SpecialEnv : public EnvWrapper {
class NoopDirectory : public Directory {
public:
NoopDirectory() {}
~NoopDirectory() {}
~NoopDirectory() override {}

Status Fsync() override { return Status::OK(); }
Status Close() override { return Status::OK(); }
Expand Down Expand Up @@ -1098,7 +1100,7 @@ class DBTestBase : public testing::Test {
// tests, but won't cover the exact fsync logic.
DBTestBase(const std::string path, bool env_do_fsync);

~DBTestBase();
~DBTestBase() override;

static std::string Key(int i) {
char buf[100];
Expand Down
2 changes: 1 addition & 1 deletion db/dbformat.h
Original file line number Diff line number Diff line change
Expand Up @@ -396,7 +396,7 @@ class InternalKeyComparator
// overhead, set `named` to false. In that case, `Name()` will return a
// generic name that is non-specific to the underlying comparator.
explicit InternalKeyComparator(const Comparator* c) : user_comparator_(c) {}
virtual ~InternalKeyComparator() {}
virtual ~InternalKeyComparator() override {}

int Compare(const Slice& a, const Slice& b) const override;

Expand Down
3 changes: 1 addition & 2 deletions db/experimental.cc
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ Status PromoteL0(DB* db, ColumnFamilyHandle* column_family, int target_level) {
return db->PromoteL0(column_family, target_level);
}


Status SuggestCompactRange(DB* db, const Slice* begin, const Slice* end) {
return SuggestCompactRange(db, db->DefaultColumnFamily(), begin, end);
}
Expand Down Expand Up @@ -517,7 +516,7 @@ class SstQueryFilterConfigImpl : public SstQueryFilterConfig {
const KeySegmentsExtractor::KeyCategorySet& categories)
: input_(input), categories_(categories) {}

virtual ~SstQueryFilterConfigImpl() = default;
virtual ~SstQueryFilterConfigImpl() override = default;

virtual std::unique_ptr<SstQueryFilterBuilder> NewBuilder(
bool sanity_checks) const = 0;
Expand Down
5 changes: 2 additions & 3 deletions db/forward_iterator.h
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,12 @@
// (found in the LICENSE.Apache file in the root directory).
#pragma once

#include "rocksdb/comparator.h"

#include <queue>
#include <string>
#include <vector>

#include "memory/arena.h"
#include "rocksdb/comparator.h"
#include "rocksdb/db.h"
#include "rocksdb/iterator.h"
#include "rocksdb/options.h"
Expand Down Expand Up @@ -55,7 +54,7 @@ class ForwardIterator : public InternalIterator {
ForwardIterator(DBImpl* db, const ReadOptions& read_options,
ColumnFamilyData* cfd, SuperVersion* current_sv = nullptr,
bool allow_unprepared_value = false);
virtual ~ForwardIterator();
virtual ~ForwardIterator() override;

void SeekForPrev(const Slice& /*target*/) override {
status_ = Status::NotSupported("ForwardIterator::SeekForPrev()");
Expand Down
4 changes: 2 additions & 2 deletions db/snapshot_checker.h
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ class SnapshotChecker {

class DisableGCSnapshotChecker : public SnapshotChecker {
public:
virtual ~DisableGCSnapshotChecker() {}
virtual ~DisableGCSnapshotChecker() override {}
SnapshotCheckerResult CheckInSnapshot(
SequenceNumber /*sequence*/,
SequenceNumber /*snapshot_sequence*/) const override {
Expand All @@ -46,7 +46,7 @@ class WritePreparedTxnDB;
class WritePreparedSnapshotChecker : public SnapshotChecker {
public:
explicit WritePreparedSnapshotChecker(WritePreparedTxnDB* txn_db);
virtual ~WritePreparedSnapshotChecker() {}
virtual ~WritePreparedSnapshotChecker() override {}

SnapshotCheckerResult CheckInSnapshot(
SequenceNumber sequence, SequenceNumber snapshot_sequence) const override;
Expand Down
2 changes: 1 addition & 1 deletion db/table_properties_collector.h
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ class UserKeyTablePropertiesCollector : public InternalTblPropColl {
explicit UserKeyTablePropertiesCollector(TablePropertiesCollector* collector)
: collector_(collector) {}

virtual ~UserKeyTablePropertiesCollector() {}
virtual ~UserKeyTablePropertiesCollector() override {}

Status InternalAdd(const Slice& key, const Slice& value,
uint64_t file_size) override;
Expand Down
6 changes: 3 additions & 3 deletions db/write_controller.h
Original file line number Diff line number Diff line change
Expand Up @@ -128,21 +128,21 @@ class StopWriteToken : public WriteControllerToken {
public:
explicit StopWriteToken(WriteController* controller)
: WriteControllerToken(controller) {}
virtual ~StopWriteToken();
virtual ~StopWriteToken() override;
};

class DelayWriteToken : public WriteControllerToken {
public:
explicit DelayWriteToken(WriteController* controller)
: WriteControllerToken(controller) {}
virtual ~DelayWriteToken();
virtual ~DelayWriteToken() override;
};

class CompactionPressureToken : public WriteControllerToken {
public:
explicit CompactionPressureToken(WriteController* controller)
: WriteControllerToken(controller) {}
virtual ~CompactionPressureToken();
virtual ~CompactionPressureToken() override;
};

} // namespace ROCKSDB_NAMESPACE
8 changes: 3 additions & 5 deletions env/env_encryption_ctr.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@

#pragma once


#include "rocksdb/env_encryption.h"

namespace ROCKSDB_NAMESPACE {
Expand All @@ -24,8 +23,8 @@ class CTRCipherStream final : public BlockAccessCipherStream {
public:
CTRCipherStream(const std::shared_ptr<BlockCipher>& c, const char* iv,
uint64_t initialCounter)
: cipher_(c), iv_(iv, c->BlockSize()), initialCounter_(initialCounter){}
virtual ~CTRCipherStream(){}
: cipher_(c), iv_(iv, c->BlockSize()), initialCounter_(initialCounter) {}
virtual ~CTRCipherStream() override {}

size_t BlockSize() override { return cipher_->BlockSize(); }

Expand Down Expand Up @@ -55,7 +54,7 @@ class CTREncryptionProvider : public EncryptionProvider {
public:
explicit CTREncryptionProvider(
const std::shared_ptr<BlockCipher>& c = nullptr);
virtual ~CTREncryptionProvider() {}
virtual ~CTREncryptionProvider() override {}

static const char* kClassName() { return "CTR"; }
const char* Name() const override { return kClassName(); }
Expand Down Expand Up @@ -94,4 +93,3 @@ Status NewEncryptedFileSystemImpl(
std::unique_ptr<FileSystem>* fs);

} // namespace ROCKSDB_NAMESPACE

2 changes: 1 addition & 1 deletion env/fs_on_demand.h
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ class OnDemandSequentialFile : public FSSequentialFile {
eof_(false),
offset_(0) {}

virtual ~OnDemandSequentialFile() {}
virtual ~OnDemandSequentialFile() override {}

IOStatus Read(size_t n, const IOOptions& options, Slice* result,
char* scratch, IODebugContext* dbg) override;
Expand Down
16 changes: 8 additions & 8 deletions env/io_posix.h
Original file line number Diff line number Diff line change
Expand Up @@ -273,7 +273,7 @@ class PosixSequentialFile : public FSSequentialFile {
public:
PosixSequentialFile(const std::string& fname, FILE* file, int fd,
size_t logical_block_size, const EnvOptions& options);
virtual ~PosixSequentialFile();
virtual ~PosixSequentialFile() override;

IOStatus Read(size_t n, const IOOptions& opts, Slice* result, char* scratch,
IODebugContext* dbg) override;
Expand Down Expand Up @@ -326,7 +326,7 @@ class PosixRandomAccessFile : public FSRandomAccessFile {
ThreadLocalPtr* thread_local_io_urings
#endif
);
virtual ~PosixRandomAccessFile();
virtual ~PosixRandomAccessFile() override;

IOStatus Read(uint64_t offset, size_t n, const IOOptions& opts, Slice* result,
char* scratch, IODebugContext* dbg) const override;
Expand Down Expand Up @@ -375,7 +375,7 @@ class PosixWritableFile : public FSWritableFile {
explicit PosixWritableFile(const std::string& fname, int fd,
size_t logical_block_size,
const EnvOptions& options);
virtual ~PosixWritableFile();
virtual ~PosixWritableFile() override;

// Need to implement this so the file is truncated correctly
// with direct I/O
Expand Down Expand Up @@ -431,7 +431,7 @@ class PosixMmapReadableFile : public FSRandomAccessFile {
public:
PosixMmapReadableFile(const int fd, const std::string& fname, void* base,
size_t length, const EnvOptions& options);
virtual ~PosixMmapReadableFile();
virtual ~PosixMmapReadableFile() override;
IOStatus Read(uint64_t offset, size_t n, const IOOptions& opts, Slice* result,
char* scratch, IODebugContext* dbg) const override;
void Hint(AccessPattern pattern) override;
Expand Down Expand Up @@ -470,7 +470,7 @@ class PosixMmapFile : public FSWritableFile {
public:
PosixMmapFile(const std::string& fname, int fd, size_t page_size,
const EnvOptions& options);
~PosixMmapFile();
~PosixMmapFile() override;

// Means Close() will properly take care of truncate
// and it does not need any additional information
Expand Down Expand Up @@ -501,7 +501,7 @@ class PosixRandomRWFile : public FSRandomRWFile {
public:
explicit PosixRandomRWFile(const std::string& fname, int fd,
const EnvOptions& options);
virtual ~PosixRandomRWFile();
virtual ~PosixRandomRWFile() override;

IOStatus Write(uint64_t offset, const Slice& data, const IOOptions& opts,
IODebugContext* dbg) override;
Expand All @@ -522,13 +522,13 @@ class PosixRandomRWFile : public FSRandomRWFile {
struct PosixMemoryMappedFileBuffer : public MemoryMappedFileBuffer {
PosixMemoryMappedFileBuffer(void* _base, size_t _length)
: MemoryMappedFileBuffer(_base, _length) {}
virtual ~PosixMemoryMappedFileBuffer();
virtual ~PosixMemoryMappedFileBuffer() override;
};

class PosixDirectory : public FSDirectory {
public:
explicit PosixDirectory(int fd, const std::string& directory_name);
~PosixDirectory();
~PosixDirectory() override;
IOStatus Fsync(const IOOptions& opts, IODebugContext* dbg) override;

IOStatus Close(const IOOptions& opts, IODebugContext* dbg) override;
Expand Down
2 changes: 1 addition & 1 deletion file/sst_file_manager_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ class SstFileManagerImpl : public SstFileManager {
SstFileManagerImpl(const SstFileManagerImpl& sfm) = delete;
SstFileManagerImpl& operator=(const SstFileManagerImpl& sfm) = delete;

~SstFileManagerImpl();
~SstFileManagerImpl() override;

// DB will call OnAddFile whenever a new sst/blob file is added.
Status OnAddFile(const std::string& file_path);
Expand Down
6 changes: 3 additions & 3 deletions include/rocksdb/advanced_cache.h
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@ class Cache : public Customizable {
Cache& operator=(const Cache&) = delete;

// Destroys all remaining entries by calling the associated "deleter"
virtual ~Cache() {}
virtual ~Cache() override {}

static const char* Type() { return "Cache"; }

Expand All @@ -209,7 +209,7 @@ class Cache : public Customizable {

public: // functions
// The type of the Cache
virtual const char* Name() const = 0;
virtual const char* Name() const override = 0;

// The Insert and Lookup APIs below are intended to allow cached objects
// to be demoted/promoted between the primary block cache and a secondary
Expand Down Expand Up @@ -425,7 +425,7 @@ class Cache : public Customizable {
// Prerequisite: no entry is referenced.
virtual void EraseUnRefEntries() = 0;

virtual std::string GetPrintableOptions() const { return ""; }
virtual std::string GetPrintableOptions() const override { return ""; }

// Check for any warnings or errors in the operation of the cache and
// report them to the logger. This is intended only to be called
Expand Down
4 changes: 2 additions & 2 deletions include/rocksdb/compaction_filter.h
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,7 @@ class CompactionFilter : public Customizable {
static const int kUnknownStartLevel = -1;
};

virtual ~CompactionFilter() {}
virtual ~CompactionFilter() override {}
static const char* Type() { return "CompactionFilter"; }
static Status CreateFromString(const ConfigOptions& config_options,
const std::string& name,
Expand Down Expand Up @@ -348,7 +348,7 @@ class CompactionFilter : public Customizable {
// including data loss, unreported corruption, deadlocks, and more.
class CompactionFilterFactory : public Customizable {
public:
virtual ~CompactionFilterFactory() {}
virtual ~CompactionFilterFactory() override {}
static const char* Type() { return "CompactionFilterFactory"; }
static Status CreateFromString(
const ConfigOptions& config_options, const std::string& name,
Expand Down
Loading
Loading