Make virtual destructors default implemented - instead of empty one (#52382)

Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/52382

Changelog: [Internal]

In C++, both `virtual ~CallInvoker() {}` and `virtual ~CallInvoker() = default` can be used to define a virtual destructor. However, they have slightly different implications:

1. `virtual ~CallInvoker() {}`:
    * This is the traditional way of defining a virtual destructor.
    * It provides an empty implementation for the destructor, which does nothing.
    * The compiler will not generate a default implementation, as you've provided one explicitly.
2. `virtual ~CallInvoker() = default`:
    * This is a more modern way of defining a virtual destructor (introduced in C++11).
    * It tells the compiler to generate a default implementation for the destructor.
    * The default implementation will perform the necessary cleanup operations, such as calling the destructors of base classes and member variables.

In general, `= default` is considered better because it:
* Avoids unnecessary code duplication: By letting the compiler generate the default implementation, you avoid duplicating code that's already generated by the compiler.
* Improves maintainability: If the class has member variables or base classes with non-trivial destructors, using `= default` ensures that the correct cleanup operations are performed without requiring manual updates.
* Conveys intent: Using `= default` clearly indicates that the destructor should perform its default behavior, making the code easier to understand.

So, unless you have a specific reason to provide a custom implementation, `virtual ~CallInvoker() = default` is generally the better choice.

Reviewed By: rshest

Differential Revision: D77685932

fbshipit-source-id: 78c81f8e400069ad38d8d7405dafeb0b6db8e67b
This commit is contained in:
Christoph Purrer
2025-07-03 13:32:28 -07:00
committed by Facebook GitHub Bot
parent c9f1778faf
commit 47fe09f505
10 changed files with 12 additions and 12 deletions
@@ -45,7 +45,7 @@ class CallInvoker {
invokeSync([func](jsi::Runtime&) { func(); });
}
virtual ~CallInvoker() {}
virtual ~CallInvoker() = default;
};
using NativeMethodCallFunc = std::function<void()>;
@@ -58,7 +58,7 @@ class NativeMethodCallInvoker {
virtual void invokeSync(
const std::string& methodName,
NativeMethodCallFunc&& func) = 0;
virtual ~NativeMethodCallInvoker() {}
virtual ~NativeMethodCallInvoker() = default;
};
} // namespace facebook::react
@@ -205,7 +205,7 @@ class CxxModule {
* This may block, if necessary to complete cleanup before the
* object is destroyed.
*/
virtual ~CxxModule() {}
virtual ~CxxModule() = default;
/**
* @return the name of this module. This will be the name used to {@code
@@ -35,7 +35,7 @@ class ModuleRegistry;
class RAMBundleRegistry;
struct InstanceCallback {
virtual ~InstanceCallback() {}
virtual ~InstanceCallback() = default;
virtual void onBatchComplete() {}
virtual void incrementPendingJSCalls() {}
virtual void decrementPendingJSCalls() {}
@@ -35,7 +35,7 @@ class JSBigString {
JSBigString(const JSBigString&) = delete;
JSBigString& operator=(const JSBigString&) = delete;
virtual ~JSBigString() {}
virtual ~JSBigString() = default;
virtual bool isAscii() const = 0;
@@ -33,7 +33,7 @@ class RAMBundleRegistry;
// Executor implementations to call from JS into native code.
class ExecutorDelegate {
public:
virtual ~ExecutorDelegate() {}
virtual ~ExecutorDelegate() = default;
virtual std::shared_ptr<ModuleRegistry> getModuleRegistry() = 0;
@@ -53,7 +53,7 @@ class JSExecutorFactory {
virtual std::unique_ptr<JSExecutor> createJSExecutor(
std::shared_ptr<ExecutorDelegate> delegate,
std::shared_ptr<MessageQueueThread> jsQueue) = 0;
virtual ~JSExecutorFactory() {}
virtual ~JSExecutorFactory() = default;
};
class RN_EXPORT JSExecutor {
@@ -36,7 +36,7 @@ class JSModulesUnbundle {
std::string code;
};
JSModulesUnbundle() {}
virtual ~JSModulesUnbundle() {}
virtual ~JSModulesUnbundle() = default;
virtual Module getModule(uint32_t moduleId) const = 0;
private:
@@ -15,7 +15,7 @@ namespace facebook::react {
class MessageQueueThread {
public:
virtual ~MessageQueueThread() {}
virtual ~MessageQueueThread() = default;
virtual void runOnQueue(std::function<void()>&&) = 0;
// runOnQueueSync and quitSynchronous are dangerous. They should only be
// used for initialization and cleanup.
@@ -31,7 +31,7 @@ using MethodCallResult = std::optional<folly::dynamic>;
#ifndef RCT_FIT_RM_OLD_RUNTIME
class NativeModule {
public:
virtual ~NativeModule() {}
virtual ~NativeModule() = default;
virtual std::string getName() = 0;
virtual std::string getSyncMethodName(unsigned int methodId) = 0;
virtual std::vector<MethodDescriptor> getMethods() = 0;
@@ -43,7 +43,7 @@ class RN_EXPORT RAMBundleRegistry {
void registerBundle(uint32_t bundleId, std::string bundlePath);
JSModulesUnbundle::Module getModule(uint32_t bundleId, uint32_t moduleId);
virtual ~RAMBundleRegistry(){};
virtual ~RAMBundleRegistry() = default;
private:
JSModulesUnbundle* getBundle(uint32_t bundleId) const;
@@ -16,7 +16,7 @@ namespace facebook::react {
*/
class NativeModulePerfLogger {
public:
virtual ~NativeModulePerfLogger() {}
virtual ~NativeModulePerfLogger() = default;
/**
* NativeModule Initialization.