From c4ede3854236244fbcefd66b56579c9608ff0664 Mon Sep 17 00:00:00 2001 From: Nicholas-Baron Date: Thu, 15 Apr 2021 10:43:29 -0700 Subject: [PATCH] Everything: Add `-Wnon-virtual-dtor` flag This flag warns on classes which have `virtual` functions but do not have a `virtual` destructor. This patch adds both the flag and missing destructors. The access level of the destructors was determined by a two rules of thumb: 1. A destructor should have a similar or lower access level to that of a constructor. 2. Having a `private` destructor implicitly deletes the default constructor, which is probably undesirable for "interface" types (classes with only virtual functions and no data). In short, most of the added destructors are `protected`, unless the compiler complained about access. --- CMakeLists.txt | 1 + Kernel/ACPI/Parser.h | 1 + Kernel/Interrupts/InterruptManagement.h | 3 +++ Kernel/PCI/Access.h | 2 ++ Userland/Applications/PixelPaint/Image.h | 3 +++ Userland/Applications/SoundPlayer/Player.h | 2 ++ Userland/Applications/SoundPlayer/VisualizationBase.h | 3 +++ Userland/Applications/SpaceAnalyzer/TreeMapWidget.h | 3 +++ Userland/Libraries/LibCrypto/Checksum/ChecksumFunction.h | 3 +++ Userland/Libraries/LibCrypto/Cipher/Cipher.h | 6 ++++++ Userland/Libraries/LibCrypto/Hash/HashFunction.h | 3 +++ Userland/Libraries/LibCrypto/PK/Code/Code.h | 2 ++ Userland/Libraries/LibCrypto/PK/PK.h | 2 ++ Userland/Libraries/LibJS/Console.h | 2 ++ Userland/Libraries/LibJS/Runtime/Cell.h | 1 + Userland/Libraries/LibTextCodec/Decoder.h | 3 +++ Userland/Libraries/LibWeb/Page/Page.h | 3 +++ Userland/Libraries/LibX86/Instruction.h | 6 ++++++ Userland/Libraries/LibX86/Interpreter.h | 3 +++ Userland/Shell/NodeVisitor.h | 3 +++ Userland/Utilities/test-js.cpp | 2 ++ 21 files changed, 57 insertions(+) diff --git a/CMakeLists.txt b/CMakeLists.txt index ba26dda423b..f8ebf56909c 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -174,6 +174,7 @@ add_compile_options(-Wlogical-op) add_compile_options(-Wmisleading-indentation) add_compile_options(-Wmissing-declarations) add_compile_options(-Wno-nonnull-compare) +add_compile_options(-Wnon-virtual-dtor) add_compile_options(-Wno-unknown-warning-option) add_compile_options(-Wundef) add_compile_options(-Wunused) diff --git a/Kernel/ACPI/Parser.h b/Kernel/ACPI/Parser.h index e9fecebcf1e..52aecd82847 100644 --- a/Kernel/ACPI/Parser.h +++ b/Kernel/ACPI/Parser.h @@ -69,6 +69,7 @@ public: protected: explicit Parser(PhysicalAddress rsdp); + virtual ~Parser() = default; private: static void set_the(Parser&); diff --git a/Kernel/Interrupts/InterruptManagement.h b/Kernel/Interrupts/InterruptManagement.h index 89f24780ec2..15db4869641 100644 --- a/Kernel/Interrupts/InterruptManagement.h +++ b/Kernel/Interrupts/InterruptManagement.h @@ -83,6 +83,9 @@ public: void enumerate_interrupt_handlers(Function); IRQController& get_interrupt_controller(int index); +protected: + virtual ~InterruptManagement() = default; + private: InterruptManagement(); PhysicalAddress search_for_madt(); diff --git a/Kernel/PCI/Access.h b/Kernel/PCI/Access.h index 7e050df0beb..2de53a1db1a 100644 --- a/Kernel/PCI/Access.h +++ b/Kernel/PCI/Access.h @@ -66,6 +66,8 @@ protected: u16 early_read_type(Address address); Access(); + virtual ~Access() = default; + Vector m_physical_ids; }; diff --git a/Userland/Applications/PixelPaint/Image.h b/Userland/Applications/PixelPaint/Image.h index 858c2381e11..6c8ff24b5e7 100644 --- a/Userland/Applications/PixelPaint/Image.h +++ b/Userland/Applications/PixelPaint/Image.h @@ -49,6 +49,9 @@ public: virtual void image_did_modify_layer_stack() { } virtual void image_did_change() { } virtual void image_select_layer(Layer*) { } + +protected: + virtual ~ImageClient() = default; }; class Image : public RefCounted { diff --git a/Userland/Applications/SoundPlayer/Player.h b/Userland/Applications/SoundPlayer/Player.h index 00d5b65a3f0..1b042ff7aab 100644 --- a/Userland/Applications/SoundPlayer/Player.h +++ b/Userland/Applications/SoundPlayer/Player.h @@ -84,6 +84,8 @@ public: PlaybackManager& manager() { return m_player_state.manager; } protected: + virtual ~Player() = default; + PlayerState m_player_state; RefPtr m_playlist_model; }; diff --git a/Userland/Applications/SoundPlayer/VisualizationBase.h b/Userland/Applications/SoundPlayer/VisualizationBase.h index 839a7d8b102..6e89871eea5 100644 --- a/Userland/Applications/SoundPlayer/VisualizationBase.h +++ b/Userland/Applications/SoundPlayer/VisualizationBase.h @@ -32,4 +32,7 @@ class Visualization { public: virtual void set_buffer(RefPtr buffer) = 0; virtual void set_samplerate(int) { } + +protected: + virtual ~Visualization() = default; }; diff --git a/Userland/Applications/SpaceAnalyzer/TreeMapWidget.h b/Userland/Applications/SpaceAnalyzer/TreeMapWidget.h index 644b4b6478f..8145e4814b0 100644 --- a/Userland/Applications/SpaceAnalyzer/TreeMapWidget.h +++ b/Userland/Applications/SpaceAnalyzer/TreeMapWidget.h @@ -37,6 +37,9 @@ struct TreeMapNode { virtual size_t num_children() const = 0; virtual const TreeMapNode& child_at(size_t i) const = 0; virtual void sort_children_by_area() const = 0; + +protected: + virtual ~TreeMapNode() = default; }; struct TreeMap : public RefCounted { diff --git a/Userland/Libraries/LibCrypto/Checksum/ChecksumFunction.h b/Userland/Libraries/LibCrypto/Checksum/ChecksumFunction.h index c62ff1d4d91..e5e4e23ba32 100644 --- a/Userland/Libraries/LibCrypto/Checksum/ChecksumFunction.h +++ b/Userland/Libraries/LibCrypto/Checksum/ChecksumFunction.h @@ -35,6 +35,9 @@ class ChecksumFunction { public: virtual void update(ReadonlyBytes data) = 0; virtual ChecksumType digest() = 0; + +protected: + virtual ~ChecksumFunction() = default; }; } diff --git a/Userland/Libraries/LibCrypto/Cipher/Cipher.h b/Userland/Libraries/LibCrypto/Cipher/Cipher.h index a17db5a8c06..4aa9f1c6041 100644 --- a/Userland/Libraries/LibCrypto/Cipher/Cipher.h +++ b/Userland/Libraries/LibCrypto/Cipher/Cipher.h @@ -92,6 +92,9 @@ public: ptr[index] = (u8)value; } +protected: + virtual ~CipherBlock() = default; + private: virtual Bytes bytes() = 0; PaddingMode m_padding_mode; @@ -132,6 +135,9 @@ public: virtual String class_name() const = 0; +protected: + virtual ~Cipher() = default; + private: PaddingMode m_padding_mode; }; diff --git a/Userland/Libraries/LibCrypto/Hash/HashFunction.h b/Userland/Libraries/LibCrypto/Hash/HashFunction.h index 3529615f500..c9e7d70c00c 100644 --- a/Userland/Libraries/LibCrypto/Hash/HashFunction.h +++ b/Userland/Libraries/LibCrypto/Hash/HashFunction.h @@ -57,6 +57,9 @@ public: virtual void reset() = 0; virtual String class_name() const = 0; + +protected: + virtual ~HashFunction() = default; }; } } diff --git a/Userland/Libraries/LibCrypto/PK/Code/Code.h b/Userland/Libraries/LibCrypto/PK/Code/Code.h index b5a70ddb149..590d097c124 100644 --- a/Userland/Libraries/LibCrypto/PK/Code/Code.h +++ b/Userland/Libraries/LibCrypto/PK/Code/Code.h @@ -48,6 +48,8 @@ public: HashFunction& hasher() { return m_hasher; } protected: + virtual ~Code() = default; + HashFunction m_hasher; }; diff --git a/Userland/Libraries/LibCrypto/PK/PK.h b/Userland/Libraries/LibCrypto/PK/PK.h index 4186eb0c91b..9761c586a33 100644 --- a/Userland/Libraries/LibCrypto/PK/PK.h +++ b/Userland/Libraries/LibCrypto/PK/PK.h @@ -60,6 +60,8 @@ public: virtual size_t output_size() const = 0; protected: + virtual ~PKSystem() = default; + PublicKeyType m_public_key; PrivateKeyType m_private_key; }; diff --git a/Userland/Libraries/LibJS/Console.h b/Userland/Libraries/LibJS/Console.h index 0782ddf14d6..600e14e9a1a 100644 --- a/Userland/Libraries/LibJS/Console.h +++ b/Userland/Libraries/LibJS/Console.h @@ -93,6 +93,8 @@ public: virtual Value count_reset() = 0; protected: + virtual ~ConsoleClient() = default; + VM& vm(); GlobalObject& global_object() { return m_console.global_object(); } diff --git a/Userland/Libraries/LibJS/Runtime/Cell.h b/Userland/Libraries/LibJS/Runtime/Cell.h index df4a19f19a1..9927ef2cc62 100644 --- a/Userland/Libraries/LibJS/Runtime/Cell.h +++ b/Userland/Libraries/LibJS/Runtime/Cell.h @@ -58,6 +58,7 @@ public: protected: virtual void visit_impl(Cell*) = 0; + virtual ~Visitor() = default; }; virtual void visit_edges(Visitor&) { } diff --git a/Userland/Libraries/LibTextCodec/Decoder.h b/Userland/Libraries/LibTextCodec/Decoder.h index 3acb23f533e..1740d6975e4 100644 --- a/Userland/Libraries/LibTextCodec/Decoder.h +++ b/Userland/Libraries/LibTextCodec/Decoder.h @@ -33,6 +33,9 @@ namespace TextCodec { class Decoder { public: virtual String to_utf8(const StringView&) = 0; + +protected: + virtual ~Decoder() = default; }; class UTF8Decoder final : public Decoder { diff --git a/Userland/Libraries/LibWeb/Page/Page.h b/Userland/Libraries/LibWeb/Page/Page.h index 2df1f2a1fdc..dab43a39a49 100644 --- a/Userland/Libraries/LibWeb/Page/Page.h +++ b/Userland/Libraries/LibWeb/Page/Page.h @@ -113,6 +113,9 @@ public: virtual String page_did_request_prompt(const String&, const String&) { return {}; } virtual String page_did_request_cookie(const URL&, Cookie::Source) { return {}; } virtual void page_did_set_cookie(const URL&, const String&, Cookie::Source) { } + +protected: + virtual ~PageClient() = default; }; } diff --git a/Userland/Libraries/LibX86/Instruction.h b/Userland/Libraries/LibX86/Instruction.h index 1c29ce65dd6..24473a1754d 100644 --- a/Userland/Libraries/LibX86/Instruction.h +++ b/Userland/Libraries/LibX86/Instruction.h @@ -41,6 +41,9 @@ typedef void (Interpreter::*InstructionHandler)(const Instruction&); class SymbolProvider { public: virtual String symbolicate(FlatPtr, u32* offset = nullptr) const = 0; + +protected: + virtual ~SymbolProvider() = default; }; template @@ -316,6 +319,9 @@ public: virtual u16 read16() = 0; virtual u32 read32() = 0; virtual u64 read64() = 0; + +protected: + virtual ~InstructionStream() = default; }; class SimpleInstructionStream final : public InstructionStream { diff --git a/Userland/Libraries/LibX86/Interpreter.h b/Userland/Libraries/LibX86/Interpreter.h index efbf4164d88..49f71b50cef 100644 --- a/Userland/Libraries/LibX86/Interpreter.h +++ b/Userland/Libraries/LibX86/Interpreter.h @@ -622,6 +622,9 @@ public: virtual void wrap_0xD2(const Instruction&) = 0; virtual void wrap_0xD3_16(const Instruction&) = 0; virtual void wrap_0xD3_32(const Instruction&) = 0; + +protected: + virtual ~Interpreter() = default; }; typedef void (Interpreter::*InstructionHandler)(const Instruction&); diff --git a/Userland/Shell/NodeVisitor.h b/Userland/Shell/NodeVisitor.h index 8452d6d968a..581443a27cb 100644 --- a/Userland/Shell/NodeVisitor.h +++ b/Userland/Shell/NodeVisitor.h @@ -75,6 +75,9 @@ public: virtual void visit(const AST::VariableDeclarations*); virtual void visit(const AST::WriteAppendRedirection*); virtual void visit(const AST::WriteRedirection*); + +protected: + virtual ~NodeVisitor() = default; }; } diff --git a/Userland/Utilities/test-js.cpp b/Userland/Utilities/test-js.cpp index 665f9be4542..e9fb4e51934 100644 --- a/Userland/Utilities/test-js.cpp +++ b/Userland/Utilities/test-js.cpp @@ -99,6 +99,8 @@ public: s_the = this; } + virtual ~TestRunner() = default; + void run(); const Test::Counts& counts() const { return m_counts; }