From 9f120efcf4b294606a24ea3b4828dc2662e9393a Mon Sep 17 00:00:00 2001 From: Joshua Gross Date: Fri, 26 Feb 2021 23:26:05 -0800 Subject: [PATCH] ReactCommon/utils: Migrate uses of NDEBUG to REACT_NATIVE_DEBUG + react_native_assert Summary: For better cross-platform consistency, migrate usages of NDEBUG to REACT_NATIVE_DEBUG. See flags.h for explanation. Changelog: [Internal] Reviewed By: PeteTheHeat Differential Revision: D26695275 fbshipit-source-id: 85aae94105a2817d345d25f736386e545dff0a9a --- ReactCommon/react/utils/Android.mk | 6 ++++-- ReactCommon/react/utils/BUCK | 1 + .../utils/CalledOnceMovableOnlyFunction.h | 10 +++++---- ReactCommon/react/utils/ContextContainer.h | 21 ++++++++++++------- .../react/utils/ManagedObjectWrapper.h | 4 +++- ReactCommon/react/utils/RunLoopObserver.cpp | 9 ++++---- 6 files changed, 33 insertions(+), 18 deletions(-) diff --git a/ReactCommon/react/utils/Android.mk b/ReactCommon/react/utils/Android.mk index 8a8cfca179..4e15562a50 100644 --- a/ReactCommon/react/utils/Android.mk +++ b/ReactCommon/react/utils/Android.mk @@ -11,7 +11,7 @@ LOCAL_MODULE := react_utils LOCAL_SRC_FILES := $(wildcard $(LOCAL_PATH)/*.cpp) -LOCAL_C_INCLUDES := $(LOCAL_PATH) +LOCAL_C_INCLUDES := $(LOCAL_PATH) $(LOCAL_PATH)/../../ LOCAL_EXPORT_C_INCLUDES := $(LOCAL_PATH)/../../ LOCAL_CFLAGS := \ @@ -20,6 +20,8 @@ LOCAL_CFLAGS := \ LOCAL_CFLAGS += -fexceptions -frtti -std=c++14 -Wall LOCAL_STATIC_LIBRARIES := -LOCAL_SHARED_LIBRARIES := +LOCAL_SHARED_LIBRARIES := libreact_debug include $(BUILD_SHARED_LIBRARY) + +$(call import-module,react/debug) diff --git a/ReactCommon/react/utils/BUCK b/ReactCommon/react/utils/BUCK index 2eddc79ce7..6e2f7ab2fe 100644 --- a/ReactCommon/react/utils/BUCK +++ b/ReactCommon/react/utils/BUCK @@ -59,5 +59,6 @@ rn_xplat_cxx_library( "//xplat/folly:molly", "//xplat/jsi:jsi", react_native_xplat_target("better:better"), + react_native_xplat_target("react/debug:debug"), ], ) diff --git a/ReactCommon/react/utils/CalledOnceMovableOnlyFunction.h b/ReactCommon/react/utils/CalledOnceMovableOnlyFunction.h index b6d31707d1..4d4d2c0d05 100644 --- a/ReactCommon/react/utils/CalledOnceMovableOnlyFunction.h +++ b/ReactCommon/react/utils/CalledOnceMovableOnlyFunction.h @@ -7,6 +7,8 @@ #include +#include + namespace facebook { namespace react { @@ -31,7 +33,7 @@ class CalledOnceMovableOnlyFunction { } ~CalledOnceMovableOnlyFunction() { - assert( + react_native_assert( (wasCalled_ || wasMovedFrom_) && "`CalledOnceMovableOnlyFunction` is destroyed before being called."); } @@ -57,7 +59,7 @@ class CalledOnceMovableOnlyFunction { CalledOnceMovableOnlyFunction &operator=( CalledOnceMovableOnlyFunction &&other) noexcept { - assert( + react_native_assert( (wasCalled_ || wasMovedFrom_) && "`CalledOnceMovableOnlyFunction` is re-assigned before being called."); wasCalled_ = false; @@ -71,10 +73,10 @@ class CalledOnceMovableOnlyFunction { * Callable. */ ReturnT operator()(ArgumentT... args) { - assert( + react_native_assert( !wasMovedFrom_ && "`CalledOnceMovableOnlyFunction` is called after being moved from."); - assert( + react_native_assert( !wasCalled_ && "`CalledOnceMovableOnlyFunction` is called more than once."); diff --git a/ReactCommon/react/utils/ContextContainer.h b/ReactCommon/react/utils/ContextContainer.h index a8d0965566..f0e2613fdb 100644 --- a/ReactCommon/react/utils/ContextContainer.h +++ b/ReactCommon/react/utils/ContextContainer.h @@ -15,6 +15,9 @@ #include #include +#include +#include + namespace facebook { namespace react { @@ -43,7 +46,7 @@ class ContextContainer final { instances_.insert({key, std::make_shared(instance)}); -#ifndef NDEBUG +#ifdef REACT_NATIVE_DEBUG typeNames_.insert({key, typeid(T).name()}); #endif } @@ -57,7 +60,7 @@ class ContextContainer final { instances_.erase(key); -#ifndef NDEBUG +#ifdef REACT_NATIVE_DEBUG typeNames_.erase(key); #endif } @@ -73,7 +76,7 @@ class ContextContainer final { for (auto const &pair : contextContainer.instances_) { instances_.erase(pair.first); instances_.insert(pair); -#ifndef NDEBUG +#ifdef REACT_NATIVE_DEBUG typeNames_.erase(pair.first); typeNames_.insert( {pair.first, contextContainer.typeNames_.at(pair.first)}); @@ -90,12 +93,14 @@ class ContextContainer final { T at(std::string const &key) const { std::shared_lock lock(mutex_); - assert( + react_native_assert( instances_.find(key) != instances_.end() && "ContextContainer doesn't have an instance for given key."); - assert( +#ifdef REACT_NATIVE_DEBUG + react_native_assert( typeNames_.at(key) == typeid(T).name() && "ContextContainer stores an instance of different type for given key."); +#endif return *std::static_pointer_cast(instances_.at(key)); } @@ -113,9 +118,11 @@ class ContextContainer final { return {}; } - assert( +#ifdef REACT_NATIVE_DEBUG + react_native_assert( typeNames_.at(key) == typeid(T).name() && "ContextContainer stores an instance of different type for given key."); +#endif return *std::static_pointer_cast(iterator->second); } @@ -124,7 +131,7 @@ class ContextContainer final { mutable better::shared_mutex mutex_; // Protected by mutex_`. mutable better::map> instances_; -#ifndef NDEBUG +#ifdef REACT_NATIVE_DEBUG mutable better::map typeNames_; #endif }; diff --git a/ReactCommon/react/utils/ManagedObjectWrapper.h b/ReactCommon/react/utils/ManagedObjectWrapper.h index 6231580157..f91cd3e99d 100644 --- a/ReactCommon/react/utils/ManagedObjectWrapper.h +++ b/ReactCommon/react/utils/ManagedObjectWrapper.h @@ -7,6 +7,8 @@ #pragma once +#include + #if defined(__OBJC__) && defined(__cplusplus) #if TARGET_OS_MAC && TARGET_OS_IPHONE @@ -65,7 +67,7 @@ inline std::shared_ptr wrapManagedObjectWeakly(id object) noexcept inline id unwrapManagedObjectWeakly(std::shared_ptr const &object) noexcept { RCTInternalGenericWeakWrapper *weakWrapper = (RCTInternalGenericWeakWrapper *)unwrapManagedObject(object); - assert(weakWrapper && "`RCTInternalGenericWeakWrapper` instance must not be `nil`."); + react_native_assert(weakWrapper && "`RCTInternalGenericWeakWrapper` instance must not be `nil`."); return weakWrapper.object; } diff --git a/ReactCommon/react/utils/RunLoopObserver.cpp b/ReactCommon/react/utils/RunLoopObserver.cpp index bca81b6664..d24c3ede72 100644 --- a/ReactCommon/react/utils/RunLoopObserver.cpp +++ b/ReactCommon/react/utils/RunLoopObserver.cpp @@ -7,7 +7,7 @@ #include "RunLoopObserver.h" -#include +#include namespace facebook { namespace react { @@ -19,8 +19,9 @@ RunLoopObserver::RunLoopObserver( void RunLoopObserver::setDelegate(Delegate const *delegate) const noexcept { // We need these constraints to ensure basic thread-safety. - assert(delegate && "A delegate must not be `nullptr`."); - assert(!delegate_ && "`RunLoopObserver::setDelegate` must be called once."); + react_native_assert(delegate && "A delegate must not be `nullptr`."); + react_native_assert( + !delegate_ && "`RunLoopObserver::setDelegate` must be called once."); delegate_ = delegate; } @@ -47,7 +48,7 @@ void RunLoopObserver::activityDidChange(Activity activity) const noexcept { return; } - assert( + react_native_assert( !owner_.expired() && "`owner_` is null. The caller must `lock` the owner and check it for being not null.");