Skip to content

Commit

Permalink
tidy: check for non-virtual destructors
Browse files Browse the repository at this point in the history
The check implements [this guideline][1]. A base class' destructor
should either be public and virtual, meaning that the object can be
destroyed through a pointer to the base class (e.g. a unique_ptr<Base>),
or protected and non-virtual, meaning that the destructor cannot be
invoked on a pointer to the base class. Private destructors would mean
that children can't invoke the parent destructor, which is undesirable.

  [1]: http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Rc-dtor-virtual
  • Loading branch information
Riolku committed Nov 24, 2023
1 parent c29c28d commit 170fdf2
Show file tree
Hide file tree
Showing 9 changed files with 10 additions and 4 deletions.
2 changes: 2 additions & 0 deletions .clang-tidy
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ Checks:

concurrency-*,

cppcoreguidelines-virtual-class-destructor,

misc-unused-parameters,

modernize-use-override,
Expand Down
2 changes: 1 addition & 1 deletion src/include/processor/operator/aggregate/base_aggregate.h
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ class BaseAggregateSharedState {

virtual std::pair<uint64_t, uint64_t> getNextRangeToRead() = 0;

virtual ~BaseAggregateSharedState() {}
~BaseAggregateSharedState() {}

protected:
std::mutex mtx;
Expand Down
2 changes: 1 addition & 1 deletion src/include/processor/operator/aggregate/hash_aggregate.h
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
namespace kuzu {
namespace processor {

class HashAggregateSharedState : public BaseAggregateSharedState {
class HashAggregateSharedState final : public BaseAggregateSharedState {

public:
explicit HashAggregateSharedState(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
namespace kuzu {
namespace processor {

class SimpleAggregateSharedState : public BaseAggregateSharedState {
class SimpleAggregateSharedState final : public BaseAggregateSharedState {
public:
explicit SimpleAggregateSharedState(
const std::vector<std::unique_ptr<function::AggregateFunction>>& aggregateFunctions);
Expand Down
1 change: 1 addition & 0 deletions src/include/processor/operator/filtering_operator.h
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ class SelVectorOverWriter {
currentSelVector =
std::make_shared<common::SelectionVector>(common::DEFAULT_VECTOR_CAPACITY);
}
virtual ~SelVectorOverWriter() = default;

protected:
void restoreSelVector(std::shared_ptr<common::SelectionVector>& selVector);
Expand Down
1 change: 1 addition & 0 deletions src/include/processor/operator/mask.h
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ class MaskCollection {
class NodeSemiMask {
public:
explicit NodeSemiMask(storage::NodeTable* nodeTable) : nodeTable{nodeTable} {}
virtual ~NodeSemiMask() = default;

virtual void init(transaction::Transaction* trx) = 0;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ class ParsingDriver {

public:
ParsingDriver(common::DataChunk& chunk);
virtual ~ParsingDriver() = default;

bool done(uint64_t rowNum);
void addValue(uint64_t rowNum, common::column_id_t columnIdx, std::string_view value);
Expand Down
2 changes: 1 addition & 1 deletion src/include/storage/buffer_manager/bm_file_handle.h
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ class BMFileHandle : public FileHandle {
BMFileHandle(const std::string& path, uint8_t flags, BufferManager* bm,
common::PageSizeClass pageSizeClass, FileVersionedType fileVersionedType);

~BMFileHandle();
~BMFileHandle() override;

// This function assumes the page is already LOCKED.
inline void setLockedPageDirty(common::page_idx_t pageIdx) {
Expand Down
1 change: 1 addition & 0 deletions src/include/storage/file_handle.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ class FileHandle {
constexpr static uint8_t O_IN_MEM_TEMP_FILE{0b0000'0011};

FileHandle(const std::string& path, uint8_t flags);
virtual ~FileHandle() = default;

common::page_idx_t addNewPage();
common::page_idx_t addNewPages(common::page_idx_t numPages);
Expand Down

0 comments on commit 170fdf2

Please sign in to comment.