From 47fe09f505219dbfbe8f000bd3841ca543170b35 Mon Sep 17 00:00:00 2001 From: Christoph Purrer Date: Thu, 3 Jul 2025 13:32:28 -0700 Subject: [PATCH] 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 --- .../ReactCommon/callinvoker/ReactCommon/CallInvoker.h | 4 ++-- packages/react-native/ReactCommon/cxxreact/CxxModule.h | 2 +- packages/react-native/ReactCommon/cxxreact/Instance.h | 2 +- packages/react-native/ReactCommon/cxxreact/JSBigString.h | 2 +- packages/react-native/ReactCommon/cxxreact/JSExecutor.h | 4 ++-- .../react-native/ReactCommon/cxxreact/JSModulesUnbundle.h | 2 +- .../react-native/ReactCommon/cxxreact/MessageQueueThread.h | 2 +- packages/react-native/ReactCommon/cxxreact/NativeModule.h | 2 +- .../react-native/ReactCommon/cxxreact/RAMBundleRegistry.h | 2 +- .../reactperflogger/reactperflogger/NativeModulePerfLogger.h | 2 +- 10 files changed, 12 insertions(+), 12 deletions(-) diff --git a/packages/react-native/ReactCommon/callinvoker/ReactCommon/CallInvoker.h b/packages/react-native/ReactCommon/callinvoker/ReactCommon/CallInvoker.h index c3b1469d17d..ee8affb8057 100644 --- a/packages/react-native/ReactCommon/callinvoker/ReactCommon/CallInvoker.h +++ b/packages/react-native/ReactCommon/callinvoker/ReactCommon/CallInvoker.h @@ -45,7 +45,7 @@ class CallInvoker { invokeSync([func](jsi::Runtime&) { func(); }); } - virtual ~CallInvoker() {} + virtual ~CallInvoker() = default; }; using NativeMethodCallFunc = std::function; @@ -58,7 +58,7 @@ class NativeMethodCallInvoker { virtual void invokeSync( const std::string& methodName, NativeMethodCallFunc&& func) = 0; - virtual ~NativeMethodCallInvoker() {} + virtual ~NativeMethodCallInvoker() = default; }; } // namespace facebook::react diff --git a/packages/react-native/ReactCommon/cxxreact/CxxModule.h b/packages/react-native/ReactCommon/cxxreact/CxxModule.h index 7e767f145ef..fed1a7353a3 100644 --- a/packages/react-native/ReactCommon/cxxreact/CxxModule.h +++ b/packages/react-native/ReactCommon/cxxreact/CxxModule.h @@ -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 diff --git a/packages/react-native/ReactCommon/cxxreact/Instance.h b/packages/react-native/ReactCommon/cxxreact/Instance.h index 3c61aa2ab19..9fe147c495e 100644 --- a/packages/react-native/ReactCommon/cxxreact/Instance.h +++ b/packages/react-native/ReactCommon/cxxreact/Instance.h @@ -35,7 +35,7 @@ class ModuleRegistry; class RAMBundleRegistry; struct InstanceCallback { - virtual ~InstanceCallback() {} + virtual ~InstanceCallback() = default; virtual void onBatchComplete() {} virtual void incrementPendingJSCalls() {} virtual void decrementPendingJSCalls() {} diff --git a/packages/react-native/ReactCommon/cxxreact/JSBigString.h b/packages/react-native/ReactCommon/cxxreact/JSBigString.h index 3f8d04f1de3..6d79184de7f 100644 --- a/packages/react-native/ReactCommon/cxxreact/JSBigString.h +++ b/packages/react-native/ReactCommon/cxxreact/JSBigString.h @@ -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; diff --git a/packages/react-native/ReactCommon/cxxreact/JSExecutor.h b/packages/react-native/ReactCommon/cxxreact/JSExecutor.h index ecc1e0fd1c5..c6336457dfc 100644 --- a/packages/react-native/ReactCommon/cxxreact/JSExecutor.h +++ b/packages/react-native/ReactCommon/cxxreact/JSExecutor.h @@ -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 getModuleRegistry() = 0; @@ -53,7 +53,7 @@ class JSExecutorFactory { virtual std::unique_ptr createJSExecutor( std::shared_ptr delegate, std::shared_ptr jsQueue) = 0; - virtual ~JSExecutorFactory() {} + virtual ~JSExecutorFactory() = default; }; class RN_EXPORT JSExecutor { diff --git a/packages/react-native/ReactCommon/cxxreact/JSModulesUnbundle.h b/packages/react-native/ReactCommon/cxxreact/JSModulesUnbundle.h index 2a6c63d3213..cc58077738e 100644 --- a/packages/react-native/ReactCommon/cxxreact/JSModulesUnbundle.h +++ b/packages/react-native/ReactCommon/cxxreact/JSModulesUnbundle.h @@ -36,7 +36,7 @@ class JSModulesUnbundle { std::string code; }; JSModulesUnbundle() {} - virtual ~JSModulesUnbundle() {} + virtual ~JSModulesUnbundle() = default; virtual Module getModule(uint32_t moduleId) const = 0; private: diff --git a/packages/react-native/ReactCommon/cxxreact/MessageQueueThread.h b/packages/react-native/ReactCommon/cxxreact/MessageQueueThread.h index f8946c2f9ee..faff24c116d 100644 --- a/packages/react-native/ReactCommon/cxxreact/MessageQueueThread.h +++ b/packages/react-native/ReactCommon/cxxreact/MessageQueueThread.h @@ -15,7 +15,7 @@ namespace facebook::react { class MessageQueueThread { public: - virtual ~MessageQueueThread() {} + virtual ~MessageQueueThread() = default; virtual void runOnQueue(std::function&&) = 0; // runOnQueueSync and quitSynchronous are dangerous. They should only be // used for initialization and cleanup. diff --git a/packages/react-native/ReactCommon/cxxreact/NativeModule.h b/packages/react-native/ReactCommon/cxxreact/NativeModule.h index 5e4bb138c63..a35408fceff 100644 --- a/packages/react-native/ReactCommon/cxxreact/NativeModule.h +++ b/packages/react-native/ReactCommon/cxxreact/NativeModule.h @@ -31,7 +31,7 @@ using MethodCallResult = std::optional; #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 getMethods() = 0; diff --git a/packages/react-native/ReactCommon/cxxreact/RAMBundleRegistry.h b/packages/react-native/ReactCommon/cxxreact/RAMBundleRegistry.h index 16f6c297b02..c85add57d2a 100644 --- a/packages/react-native/ReactCommon/cxxreact/RAMBundleRegistry.h +++ b/packages/react-native/ReactCommon/cxxreact/RAMBundleRegistry.h @@ -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; diff --git a/packages/react-native/ReactCommon/reactperflogger/reactperflogger/NativeModulePerfLogger.h b/packages/react-native/ReactCommon/reactperflogger/reactperflogger/NativeModulePerfLogger.h index 8348068d934..861778fa24c 100644 --- a/packages/react-native/ReactCommon/reactperflogger/reactperflogger/NativeModulePerfLogger.h +++ b/packages/react-native/ReactCommon/reactperflogger/reactperflogger/NativeModulePerfLogger.h @@ -16,7 +16,7 @@ namespace facebook::react { */ class NativeModulePerfLogger { public: - virtual ~NativeModulePerfLogger() {} + virtual ~NativeModulePerfLogger() = default; /** * NativeModule Initialization.