From 0c1ca8743e052232afc653d3fadcc66916074b97 Mon Sep 17 00:00:00 2001 From: Christoph Purrer Date: Fri, 25 Nov 2022 14:42:41 -0800 Subject: [PATCH] Replace folly::Optional with std::optional (#35436) Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/35436 Using std::optional as react-native has been using C++17 for quite some time changelog: [Internal] Reviewed By: cortinico Differential Revision: D41415031 fbshipit-source-id: 8165a4f1b24c49c58fcb5af59a50f31e96816cc4 --- React/CoreModules/RCTPlatform.mm | 4 ++-- React/CxxModule/RCTNativeModule.mm | 4 ++-- .../src/main/jni/react/jni/JavaModuleWrapper.cpp | 4 ++-- .../src/main/jni/react/jni/JavaModuleWrapper.h | 4 ++-- .../src/main/jni/react/jni/MethodInvoker.cpp | 4 ++-- .../src/main/jni/react/jni/ReadableNativeMap.h | 4 ++-- ReactCommon/cxxreact/ModuleRegistry.cpp | 11 +++++------ ReactCommon/cxxreact/ModuleRegistry.h | 4 ++-- ReactCommon/cxxreact/NativeModule.h | 4 ++-- ReactCommon/jsiexecutor/jsireact/JSIExecutor.cpp | 15 ++++++++------- ReactCommon/jsiexecutor/jsireact/JSIExecutor.h | 7 ++++--- .../jsiexecutor/jsireact/JSINativeModules.cpp | 12 ++++++------ .../jsiexecutor/jsireact/JSINativeModules.h | 6 +++--- .../components/text/ParagraphShadowNode.h | 1 - .../__tests__/HeaderWriterTest.js | 2 +- .../__tests__/PropertyTest.js | 6 +++--- .../hermes-inspector-msggen/src/HeaderWriter.js | 7 +++---- packages/hermes-inspector-msggen/src/Property.js | 4 ++-- 18 files changed, 51 insertions(+), 52 deletions(-) diff --git a/React/CoreModules/RCTPlatform.mm b/React/CoreModules/RCTPlatform.mm index a9acfeeb1adf31..7c76ffb1f8a32c 100644 --- a/React/CoreModules/RCTPlatform.mm +++ b/React/CoreModules/RCTPlatform.mm @@ -15,7 +15,7 @@ #import "CoreModulesPlugins.h" -#import +#import using namespace facebook::react; @@ -75,7 +75,7 @@ - (dispatch_queue_t)methodQueue .major = [versions[@"major"] doubleValue], .patch = [versions[@"patch"] doubleValue], .prerelease = [versions[@"prerelease"] isKindOfClass:[NSNull class]] - ? folly::Optional{} + ? std::optional{} : [versions[@"prerelease"] doubleValue]}), }); }); diff --git a/React/CxxModule/RCTNativeModule.mm b/React/CxxModule/RCTNativeModule.mm index 3e400a0c88706e..e394e327c95b7f 100644 --- a/React/CxxModule/RCTNativeModule.mm +++ b/React/CxxModule/RCTNativeModule.mm @@ -160,7 +160,7 @@ static MethodCallResult invokeInner( */ BridgeNativeModulePerfLogger::syncMethodCallFail("N/A", "N/A"); } - return folly::none; + return std::nullopt; } id method = moduleData.methods[methodId]; @@ -231,7 +231,7 @@ static MethodCallResult invokeInner( RCT_PROFILE_END_EVENT(RCTProfileTagAlways, @""); } - return folly::none; + return std::nullopt; } } diff --git a/ReactAndroid/src/main/jni/react/jni/JavaModuleWrapper.cpp b/ReactAndroid/src/main/jni/react/jni/JavaModuleWrapper.cpp index b737cd0d23e03b..5cfdf7f5943c8c 100644 --- a/ReactAndroid/src/main/jni/react/jni/JavaModuleWrapper.cpp +++ b/ReactAndroid/src/main/jni/react/jni/JavaModuleWrapper.cpp @@ -62,7 +62,7 @@ std::string JavaNativeModule::getSyncMethodName(unsigned int reactMethodId) { auto &methodInvoker = syncMethods_[reactMethodId]; - if (!methodInvoker.hasValue()) { + if (!methodInvoker.has_value()) { throw std::invalid_argument(folly::to( "methodId ", reactMethodId, " is not a recognized sync method")); } @@ -147,7 +147,7 @@ MethodCallResult JavaNativeModule::callSerializableNativeHook( } auto &method = syncMethods_[reactMethodId]; - CHECK(method.hasValue() && method->isSyncHook()) + CHECK(method.has_value() && method->isSyncHook()) << "Trying to invoke a asynchronous method as synchronous hook"; return method->invoke(instance_, wrapper_->getModule(), params); } diff --git a/ReactAndroid/src/main/jni/react/jni/JavaModuleWrapper.h b/ReactAndroid/src/main/jni/react/jni/JavaModuleWrapper.h index e4a56e453c4b18..f5bf7dcdf84590 100644 --- a/ReactAndroid/src/main/jni/react/jni/JavaModuleWrapper.h +++ b/ReactAndroid/src/main/jni/react/jni/JavaModuleWrapper.h @@ -9,7 +9,7 @@ #include #include -#include +#include #include "MethodInvoker.h" @@ -82,7 +82,7 @@ class JavaNativeModule : public NativeModule { std::weak_ptr instance_; jni::global_ref wrapper_; std::shared_ptr messageQueueThread_; - std::vector> syncMethods_; + std::vector> syncMethods_; }; } // namespace react diff --git a/ReactAndroid/src/main/jni/react/jni/MethodInvoker.cpp b/ReactAndroid/src/main/jni/react/jni/MethodInvoker.cpp index b9009c1a5d07f0..4223a449980999 100644 --- a/ReactAndroid/src/main/jni/react/jni/MethodInvoker.cpp +++ b/ReactAndroid/src/main/jni/react/jni/MethodInvoker.cpp @@ -279,7 +279,7 @@ MethodCallResult MethodInvoker::invoke( case 'v': env->CallVoidMethodA(module.get(), method_, args); throwPendingJniExceptionAsCppException(); - return folly::none; + return std::nullopt; case 'z': PRIMITIVE_CASE_CASTING(Boolean, bool) @@ -307,7 +307,7 @@ MethodCallResult MethodInvoker::invoke( default: LOG(FATAL) << "Unknown return type: " << returnType; - return folly::none; + return std::nullopt; } } diff --git a/ReactAndroid/src/main/jni/react/jni/ReadableNativeMap.h b/ReactAndroid/src/main/jni/react/jni/ReadableNativeMap.h index 0b79bcd35592ba..bf6a44245abfb8 100644 --- a/ReactAndroid/src/main/jni/react/jni/ReadableNativeMap.h +++ b/ReactAndroid/src/main/jni/react/jni/ReadableNativeMap.h @@ -8,9 +8,9 @@ #pragma once #include -#include #include #include +#include #include "NativeCommon.h" #include "NativeMap.h" @@ -38,7 +38,7 @@ struct ReadableNativeMap : jni::HybridClass { jni::local_ref> importKeys(); jni::local_ref> importValues(); jni::local_ref> importTypes(); - folly::Optional keys_; + std::optional keys_; static jni::local_ref createWithContents(folly::dynamic &&map); static void mapException(const std::exception &ex); diff --git a/ReactCommon/cxxreact/ModuleRegistry.cpp b/ReactCommon/cxxreact/ModuleRegistry.cpp index 5a0221b02f9f87..962a84e0bacebe 100644 --- a/ReactCommon/cxxreact/ModuleRegistry.cpp +++ b/ReactCommon/cxxreact/ModuleRegistry.cpp @@ -92,8 +92,7 @@ std::vector ModuleRegistry::moduleNames() { return names; } -folly::Optional ModuleRegistry::getConfig( - const std::string &name) { +std::optional ModuleRegistry::getConfig(const std::string &name) { SystraceSection s("ModuleRegistry::getConfig", "module", name); // Initialize modulesByName_ @@ -107,14 +106,14 @@ folly::Optional ModuleRegistry::getConfig( if (unknownModules_.find(name) != unknownModules_.end()) { BridgeNativeModulePerfLogger::moduleJSRequireBeginningFail(name.c_str()); BridgeNativeModulePerfLogger::moduleJSRequireEndingStart(name.c_str()); - return folly::none; + return std::nullopt; } if (!moduleNotFoundCallback_) { unknownModules_.insert(name); BridgeNativeModulePerfLogger::moduleJSRequireBeginningFail(name.c_str()); BridgeNativeModulePerfLogger::moduleJSRequireEndingStart(name.c_str()); - return folly::none; + return std::nullopt; } BridgeNativeModulePerfLogger::moduleJSRequireBeginningEnd(name.c_str()); @@ -128,7 +127,7 @@ folly::Optional ModuleRegistry::getConfig( if (!wasModuleRegisteredWithRegistry) { BridgeNativeModulePerfLogger::moduleJSRequireEndingStart(name.c_str()); unknownModules_.insert(name); - return folly::none; + return std::nullopt; } } else { BridgeNativeModulePerfLogger::moduleJSRequireBeginningEnd(name.c_str()); @@ -187,7 +186,7 @@ folly::Optional ModuleRegistry::getConfig( if (config.size() == 2 && config[1].empty()) { // no constants or methods - return folly::none; + return std::nullopt; } else { return ModuleConfig{index, std::move(config)}; } diff --git a/ReactCommon/cxxreact/ModuleRegistry.h b/ReactCommon/cxxreact/ModuleRegistry.h index 20e30677d40b81..2b68a3c572ef3a 100644 --- a/ReactCommon/cxxreact/ModuleRegistry.h +++ b/ReactCommon/cxxreact/ModuleRegistry.h @@ -12,8 +12,8 @@ #include #include -#include #include +#include #ifndef RN_EXPORT #define RN_EXPORT __attribute__((visibility("default"))) @@ -47,7 +47,7 @@ class RN_EXPORT ModuleRegistry { std::vector moduleNames(); - folly::Optional getConfig(const std::string &name); + std::optional getConfig(const std::string &name); void callNativeMethod( unsigned int moduleId, diff --git a/ReactCommon/cxxreact/NativeModule.h b/ReactCommon/cxxreact/NativeModule.h index a088f5b7d3a797..5fa439703e1282 100644 --- a/ReactCommon/cxxreact/NativeModule.h +++ b/ReactCommon/cxxreact/NativeModule.h @@ -10,8 +10,8 @@ #include #include -#include #include +#include namespace facebook { namespace react { @@ -25,7 +25,7 @@ struct MethodDescriptor { : name(std::move(n)), type(std::move(t)) {} }; -using MethodCallResult = folly::Optional; +using MethodCallResult = std::optional; class NativeModule { public: diff --git a/ReactCommon/jsiexecutor/jsireact/JSIExecutor.cpp b/ReactCommon/jsiexecutor/jsireact/JSIExecutor.cpp index 10552e0adae077..c4a804d95b24ea 100644 --- a/ReactCommon/jsiexecutor/jsireact/JSIExecutor.cpp +++ b/ReactCommon/jsiexecutor/jsireact/JSIExecutor.cpp @@ -495,20 +495,21 @@ Value JSIExecutor::nativeCallSyncHook(const Value *args, size_t count) { /** * Note: - * In RCTNativeModule, folly::none is returned from callSerializableNativeHook - * when executing a NativeModule method fails. Therefore, it's safe to not - * terminate the syncMethodCall when folly::none is returned. + * In RCTNativeModule, std::nullopt is returned from + * callSerializableNativeHook when executing a NativeModule method fails. + * Therefore, it's safe to not terminate the syncMethodCall when std::nullopt + * is returned. * - * TODO: In JavaNativeModule, folly::none is returned when the synchronous + * TODO: In JavaNativeModule, std::nullopt is returned when the synchronous * NativeModule method has the void return type. Change this to return - * folly::dynamic(nullptr) instead, so that folly::none is reserved for + * folly::dynamic(nullptr) instead, so that std::nullopt is reserved for * exceptional scenarios. * - * TODO: Investigate CxxModule infra to see if folly::none is used for + * TODO: Investigate CxxModule infra to see if std::nullopt is used for * returns in exceptional scenarios. **/ - if (!result.hasValue()) { + if (!result.has_value()) { return Value::undefined(); } diff --git a/ReactCommon/jsiexecutor/jsireact/JSIExecutor.h b/ReactCommon/jsiexecutor/jsireact/JSIExecutor.h index 1a8d01f631f14b..90119ec4093200 100644 --- a/ReactCommon/jsiexecutor/jsireact/JSIExecutor.h +++ b/ReactCommon/jsiexecutor/jsireact/JSIExecutor.h @@ -15,6 +15,7 @@ #include #include #include +#include namespace facebook { namespace react { @@ -127,9 +128,9 @@ class JSIExecutor : public JSExecutor { JSIScopedTimeoutInvoker scopedTimeoutInvoker_; RuntimeInstaller runtimeInstaller_; - folly::Optional callFunctionReturnFlushedQueue_; - folly::Optional invokeCallbackAndReturnFlushedQueue_; - folly::Optional flushedQueue_; + std::optional callFunctionReturnFlushedQueue_; + std::optional invokeCallbackAndReturnFlushedQueue_; + std::optional flushedQueue_; }; using Logger = diff --git a/ReactCommon/jsiexecutor/jsireact/JSINativeModules.cpp b/ReactCommon/jsiexecutor/jsireact/JSINativeModules.cpp index 2dadd43e287638..4ccf849cffe488 100644 --- a/ReactCommon/jsiexecutor/jsireact/JSINativeModules.cpp +++ b/ReactCommon/jsiexecutor/jsireact/JSINativeModules.cpp @@ -45,7 +45,7 @@ Value JSINativeModules::getModule(Runtime &rt, const PropNameID &name) { } auto module = createModule(rt, moduleName); - if (!module.hasValue()) { + if (!module.has_value()) { BridgeNativeModulePerfLogger::moduleJSRequireEndingFail(moduleName.c_str()); // Allow lookup to continue in the objects own properties, which allows for // overrides of NativeModules @@ -61,11 +61,11 @@ Value JSINativeModules::getModule(Runtime &rt, const PropNameID &name) { } void JSINativeModules::reset() { - m_genNativeModuleJS = folly::none; + m_genNativeModuleJS = std::nullopt; m_objects.clear(); } -folly::Optional JSINativeModules::createModule( +std::optional JSINativeModules::createModule( Runtime &rt, const std::string &name) { bool hasLogger(ReactMarker::logTaggedMarker); @@ -80,8 +80,8 @@ folly::Optional JSINativeModules::createModule( } auto result = m_moduleRegistry->getConfig(name); - if (!result.hasValue()) { - return folly::none; + if (!result.has_value()) { + return std::nullopt; } Value moduleInfo = m_genNativeModuleJS->call( @@ -92,7 +92,7 @@ folly::Optional JSINativeModules::createModule( CHECK(moduleInfo.isObject()) << "Module returned from genNativeModule isn't an Object"; - folly::Optional module( + std::optional module( moduleInfo.asObject(rt).getPropertyAsObject(rt, "module")); if (hasLogger) { diff --git a/ReactCommon/jsiexecutor/jsireact/JSINativeModules.h b/ReactCommon/jsiexecutor/jsireact/JSINativeModules.h index c6aaacfa41cc50..fd769a703b40c0 100644 --- a/ReactCommon/jsiexecutor/jsireact/JSINativeModules.h +++ b/ReactCommon/jsiexecutor/jsireact/JSINativeModules.h @@ -11,8 +11,8 @@ #include #include -#include #include +#include namespace facebook { namespace react { @@ -27,11 +27,11 @@ class JSINativeModules { void reset(); private: - folly::Optional m_genNativeModuleJS; + std::optional m_genNativeModuleJS; std::shared_ptr m_moduleRegistry; std::unordered_map m_objects; - folly::Optional createModule( + std::optional createModule( jsi::Runtime &rt, const std::string &name); }; diff --git a/ReactCommon/react/renderer/components/text/ParagraphShadowNode.h b/ReactCommon/react/renderer/components/text/ParagraphShadowNode.h index 42ef650a3cf842..7a9e3adfa5b0ab 100644 --- a/ReactCommon/react/renderer/components/text/ParagraphShadowNode.h +++ b/ReactCommon/react/renderer/components/text/ParagraphShadowNode.h @@ -7,7 +7,6 @@ #pragma once -#include #include #include #include diff --git a/packages/hermes-inspector-msggen/__tests__/HeaderWriterTest.js b/packages/hermes-inspector-msggen/__tests__/HeaderWriterTest.js index 1c9da38f7b99ab..da8bf0c03a6b50 100644 --- a/packages/hermes-inspector-msggen/__tests__/HeaderWriterTest.js +++ b/packages/hermes-inspector-msggen/__tests__/HeaderWriterTest.js @@ -45,7 +45,7 @@ test('emits type decl', () => { runtime::ScriptId scriptId{}; int lineNumber{}; - folly::Optional columnNumber; + std::optional columnNumber; }; `); }); diff --git a/packages/hermes-inspector-msggen/__tests__/PropertyTest.js b/packages/hermes-inspector-msggen/__tests__/PropertyTest.js index f1f1c8a5be7e3b..96dcf6b4ab6fac 100644 --- a/packages/hermes-inspector-msggen/__tests__/PropertyTest.js +++ b/packages/hermes-inspector-msggen/__tests__/PropertyTest.js @@ -41,7 +41,7 @@ test('parses optional primitive prop', () => { expect(prop.optional).toBe(true); expect(prop.description).toBe('Average sample interval in bytes.'); - expect(prop.getFullCppType()).toBe('folly::Optional'); + expect(prop.getFullCppType()).toBe('std::optional'); expect(prop.getCppIdentifier()).toBe('samplingInterval'); expect(prop.getInitializer()).toBe(''); }); @@ -61,7 +61,7 @@ test('parses optional ref prop', () => { expect(prop.$ref).toBe('Runtime.ExceptionDetails'); expect(prop.description).toBe('Exception details if any.'); - expect(prop.getFullCppType()).toBe('folly::Optional'); + expect(prop.getFullCppType()).toBe('std::optional'); expect(prop.getCppIdentifier()).toBe('exceptionDetails'); expect(prop.getInitializer()).toBe(''); }); @@ -105,7 +105,7 @@ test('parses optional array items prop', () => { expect(prop.items).toEqual({ 'type': 'string' }); expect(prop.description).toBe('Hit breakpoints IDs'); - expect(prop.getFullCppType()).toBe('folly::Optional>'); + expect(prop.getFullCppType()).toBe('std::optional>'); expect(prop.getCppIdentifier()).toBe('hitBreakpoints'); expect(prop.getInitializer()).toBe(''); }); diff --git a/packages/hermes-inspector-msggen/src/HeaderWriter.js b/packages/hermes-inspector-msggen/src/HeaderWriter.js index 5784f9f2f261bd..560c9580789830 100644 --- a/packages/hermes-inspector-msggen/src/HeaderWriter.js +++ b/packages/hermes-inspector-msggen/src/HeaderWriter.js @@ -53,10 +53,9 @@ export class HeaderWriter { #include + #include #include - #include - namespace facebook { namespace hermes { namespace inspector { @@ -242,7 +241,7 @@ function emitUnknownRequestDecl(stream: Writable) { folly::dynamic toDynamic() const override; void accept(RequestHandler &handler) const override; - folly::Optional params; + std::optional params; }; `); @@ -273,7 +272,7 @@ function emitErrorResponseDecl(stream: Writable) { int code; std::string message; - folly::Optional data; + std::optional data; }; `); diff --git a/packages/hermes-inspector-msggen/src/Property.js b/packages/hermes-inspector-msggen/src/Property.js index 2ee60145ff6790..56dcff6a72cdcd 100644 --- a/packages/hermes-inspector-msggen/src/Property.js +++ b/packages/hermes-inspector-msggen/src/Property.js @@ -80,7 +80,7 @@ function maybeWrapOptional( recursive: ?boolean, ) { if (optional) { - return recursive ? `std::unique_ptr<${type}>` : `folly::Optional<${type}>`; + return recursive ? `std::unique_ptr<${type}>` : `std::optional<${type}>`; } return type; } @@ -130,7 +130,7 @@ class PrimitiveProperty extends Property { } getInitializer(): string { - // folly::Optional doesn't need to be explicitly zero-init + // std::optional doesn't need to be explicitly zero-init if (this.optional) { return ''; }