Merge pull request #6175 from sfc-gh-anoyes/anoyes/delete-non-virtual-destructor

Enable -Wdelete-non-virtual-dtor for clang build

bindings/c/test/workloads/SimpleWorkload.cpp

@@ -30,7 +30,7 @@
 
 namespace {
 
-struct SimpleWorkload : FDBWorkload {
+struct SimpleWorkload final : FDBWorkload {
 	static const std::string name;
 	static const std::string KEY_PREFIX;
 	std::mt19937 random;

cmake/ConfigureCompiler.cmake

@@ -283,7 +283,6 @@ else()
       -Woverloaded-virtual
       -Wshift-sign-overflow
       # Here's the current set of warnings we need to explicitly disable to compile warning-free with clang 11
-      -Wno-delete-non-virtual-dtor
       -Wno-sign-compare
       -Wno-undefined-var-template
       -Wno-unknown-warning-option

fdbserver/CoroFlowCoro.actor.cpp

@@ -119,7 +119,7 @@ class WorkPool final : public IThreadPool, public ReferenceCounted<WorkPool<Thre
 		}
 	};
 
-	struct Worker : Threadlike {
+	struct Worker final : Threadlike {
 		Pool* pool;
 		IThreadPoolReceiver* userData;
 		bool stop;

fdbserver/DiskQueue.actor.cpp

@@ -869,7 +869,7 @@ public:
 	}
 };
 
-class DiskQueue : public IDiskQueue, public Tracked<DiskQueue> {
+class DiskQueue final : public IDiskQueue, public Tracked<DiskQueue> {
 public:
 	// FIXME: Is setting lastCommittedSeq to -1 instead of 0 necessary?
 	DiskQueue(std::string basename,
@@ -1539,7 +1539,7 @@ private:
 // This works by performing two commits when uncommitted data is popped:
 //	Commit 1 - pop only previously committed data and push new data (i.e., commit uncommitted data)
 //  Commit 2 - finish pop into uncommitted data
-class DiskQueue_PopUncommitted : public IDiskQueue {
+class DiskQueue_PopUncommitted final : public IDiskQueue {
 
 public:
 	DiskQueue_PopUncommitted(std::string basename,

fdbserver/LogSystemDiskQueueAdapter.h

@@ -41,7 +41,7 @@ struct PeekTxsInfo {
 	    knownCommittedVersion(knownCommittedVersion) {}
 };
 
-class LogSystemDiskQueueAdapter : public IDiskQueue {
+class LogSystemDiskQueueAdapter final : public IDiskQueue {
 public:
 	// This adapter is designed to let KeyValueStoreMemory use ILogSystem
 	// as a backing store, so that the transaction subsystem can in

flow/DeterministicRandom.h

@@ -30,7 +30,7 @@
 
 #include <random>
 
-class DeterministicRandom : public IRandom, public ReferenceCounted<DeterministicRandom> {
+class DeterministicRandom final : public IRandom, public ReferenceCounted<DeterministicRandom> {
 private:
 	std::mt19937 random;
 	uint64_t next;

flow/actorcompiler/ActorCompiler.cs

@@ -431,10 +431,15 @@ namespace actorcompiler
             writer.WriteLine("public:");
             writer.WriteLine("\tusing FastAllocated<{0}>::operator new;", fullClassName);
             writer.WriteLine("\tusing FastAllocated<{0}>::operator delete;", fullClassName);
+
+            writer.WriteLine("#pragma clang diagnostic push");
+            writer.WriteLine("#pragma clang diagnostic ignored \"-Wdelete-non-virtual-dtor\"");
             if (actor.returnType != null)
                 writer.WriteLine("\tvoid destroy() override {{ ((Actor<{0}>*)this)->~Actor(); operator delete(this); }}", actor.returnType);
             else
                 writer.WriteLine("\tvoid destroy() {{ ((Actor<void>*)this)->~Actor(); operator delete(this); }}");
+            writer.WriteLine("#pragma clang diagnostic pop");
+
             foreach (var cb in callbacks)
                 writer.WriteLine("friend struct {0};", cb.type);
 

flow/flow.h

@@ -747,7 +747,14 @@ public:
 	int getFutureReferenceCount() const { return futures; }
 	int getPromiseReferenceCount() const { return promises; }
 
-	virtual void destroy() { delete this; }
+	// Derived classes should override destroy.
+	virtual void destroy() {
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wdelete-non-virtual-dtor"
+		delete this;
+#pragma clang diagnostic pop
+	}
+
 	virtual void cancel() {}
 
 	void addCallbackAndDelFutureRef(Callback<T>* cb) {
@@ -967,6 +974,8 @@ struct NotifiedQueue : private SingleCallback<T>, FastAllocated<NotifiedQueue<T>
 		SingleCallback<T>::next = this;
 	}
 
+	virtual ~NotifiedQueue() = default;
+
 	bool isReady() const { return !queue.empty() || error.isValid(); }
 	bool isError() const { return queue.empty() && error.isValid(); } // the *next* thing queued is an error
 	uint32_t size() const { return queue.size(); }

flow/genericactors.actor.h

@@ -870,7 +870,7 @@ template <class T>
 class QuorumCallback;
 
 template <class T>
-struct Quorum : SAV<Void> {
+struct Quorum final : SAV<Void> {
 	int antiQuorum;
 	int count;
 
@@ -1558,7 +1558,9 @@ Future<Void> yieldPromiseStream(FutureStream<T> input,
 	}
 }
 
-struct YieldedFutureActor : SAV<Void>, ActorCallback<YieldedFutureActor, 1, Void>, FastAllocated<YieldedFutureActor> {
+struct YieldedFutureActor final : SAV<Void>,
+                                  ActorCallback<YieldedFutureActor, 1, Void>,
+                                  FastAllocated<YieldedFutureActor> {
 	Error in_error_state;
 
 	typedef ActorCallback<YieldedFutureActor, 1, Void> CB1;