From 19e6557d2e4c09908b11d20f13d04e8bcdcc3195 Mon Sep 17 00:00:00 2001 From: Pieter De Baets Date: Thu, 11 May 2017 04:46:44 -0700 Subject: [PATCH] Cleanup cxxreact BUCK file Reviewed By: mzlee Differential Revision: D5020563 fbshipit-source-id: 286a2d1c44623b5b9fd923ef8684d263500791b3 --- ReactCommon/DEFS | 4 + ReactCommon/cxxreact/BUCK | 146 ++++++++--------------- ReactCommon/cxxreact/JSCExecutor.cpp | 4 +- ReactCommon/cxxreact/ModuleRegistry.cpp | 4 +- ReactCommon/cxxreact/Platform.cpp | 14 ++- ReactCommon/cxxreact/SampleCxxModule.cpp | 5 +- ReactCommon/cxxreact/SampleCxxModule.h | 10 +- 7 files changed, 72 insertions(+), 115 deletions(-) diff --git a/ReactCommon/DEFS b/ReactCommon/DEFS index 83eab26aa4..30dcaf619c 100644 --- a/ReactCommon/DEFS +++ b/ReactCommon/DEFS @@ -1,3 +1,7 @@ # Set up common deps GLOG_DEP = "//ReactAndroid/build/third-party-ndk/glog:glog" + +INSPECTOR_FLAGS = [] + +JSC_DEPS = [] diff --git a/ReactCommon/cxxreact/BUCK b/ReactCommon/cxxreact/BUCK index 032fee2f0c..06842f073b 100644 --- a/ReactCommon/cxxreact/BUCK +++ b/ReactCommon/cxxreact/BUCK @@ -1,96 +1,15 @@ -CXX_LIBRARY_COMPILER_FLAGS = [] +include_defs("//ReactCommon/DEFS") -REACT_LIBRARY_EXTRA_COMPILER_FLAGS = [] +CXX_LIBRARY_COMPILER_FLAGS = [ + "-std=c++14", + "-Wall", +] if THIS_IS_FBOBJC: - include_defs("xplat//configurations/buck/apple/dependency_aggregator_defs") inherited_buck_flags = STATIC_LIBRARY_IOS_FLAGS - CXX_LIBRARY_COMPILER_FLAGS = inherited_buck_flags.get_flag_value('compiler_flags') - REACT_LIBRARY_EXTRA_COMPILER_FLAGS = ['-Wno-shadow', '-Wno-missing-prototypes', '-Wno-global-constructors'] + CXX_LIBRARY_COMPILER_FLAGS += inherited_buck_flags.get_flag_value('compiler_flags') -def kwargs_add(base_kwargs, **new_kwargs): - ret_kwargs = dict(base_kwargs) - for name, add_value in new_kwargs.iteritems(): - if name in ret_kwargs: - # Don't use +=, it will modify base_kwargs - ret_kwargs[name] = ret_kwargs[name] + add_value - else: - ret_kwargs[name] = add_value - return ret_kwargs - -if THIS_IS_FBANDROID: - include_defs('//ReactAndroid/DEFS') - - def react_library(**kwargs): - kwargs = kwargs_add( - kwargs, - compiler_flags = [ - '-Wno-pessimizing-move', - ], - deps = [ - '//xplat/folly:molly' - ]) - - cxx_library( - name = 'bridge', - **kwargs_add( - kwargs, - preprocessor_flags = [ - '-DWITH_JSC_EXTRA_TRACING=1', - '-DWITH_JSC_MEMORY_PRESSURE=1', - '-DWITH_REACT_INTERNAL_SETTINGS=1', - '-DWITH_FB_MEMORY_PROFILING=1', - '-DWITH_INSPECTOR=1', - ], - deps = JSC_DEPS, - visibility = [ - # TL;DR: If you depend on this target directly, you're gonna have a - # Bad Time(TM). - # - # `facebook::react::JSCExecutor::initOnJSVMThread` (in `:bridge`) does - # some platform-dependant setup. Exactly what setup to do is - # determined by some static functors, defined in `Platform.h`, which - # are initially `nullptr`. On Android, these functors are properly - # assigned as part of `xreact`'s `JNI_OnLoad`. By depending directly - # on the bridge, we can mess up the SO initialisation order, causing - # `initOnJSVMThread` to be called before the platform-specific hooks - # have been properly initialised. Bad Times(TM). - # -- @ashokmenon (2017/01/03) - react_native_target('jni/xreact/jni:jni'), - react_native_xplat_target('cxxreact/...'), - ], - ) - ) - -if THIS_IS_FBOBJC: - - if should_use_dependency_aggregation(): - INSPECTOR_FLAGS = [] - else: - INSPECTOR_FLAGS = [ '-DWITH_INSPECTOR=1', ] - - def react_library(**kwargs): - fb_apple_library( - name = 'bridge', - header_path_prefix = "cxxreact", - inherited_buck_flags = STATIC_LIBRARY_IOS_FLAGS, - frameworks = [ - '$SDKROOT/System/Library/Frameworks/JavaScriptCore.framework', - ], - tests = [ - react_native_xplat_target('cxxreact/tests:tests') - ], - **kwargs_add( - kwargs, - preprocessor_flags = DEBUG_PREPROCESSOR_FLAGS + INSPECTOR_FLAGS, - deps = [ - '//xplat/folly:molly', - ], - visibility = [ 'PUBLIC' ], - ) - ) - -cxx_library( +fb_xplat_cxx_library( name = "module", compiler_flags = CXX_LIBRARY_COMPILER_FLAGS, exported_headers = [ @@ -98,9 +17,9 @@ cxx_library( "JsArgumentHelpers.h", "JsArgumentHelpers-inl.h", ], + fbobjc_header_path_prefix = "cxxreact", force_static = True, header_namespace = "cxxreact", - labels = ["accounts_for_platform_and_build_mode_flags"], visibility = [ "PUBLIC", ], @@ -110,14 +29,11 @@ cxx_library( ], ) -cxx_library( +fb_xplat_cxx_library( name = "samplemodule", srcs = ["SampleCxxModule.cpp"], compiler_flags = CXX_LIBRARY_COMPILER_FLAGS + [ "-fno-omit-frame-pointer", - "-Wall", - "-Werror", - "-std=c++1y", "-fexceptions", ], exported_headers = ["SampleCxxModule.h"], @@ -154,18 +70,46 @@ CXXREACT_PUBLIC_HEADERS = [ "SystraceSection.h", ] -react_library( +fb_xplat_cxx_library( + name = "bridge", srcs = glob( ["*.cpp"], excludes = ["SampleCxxModule.cpp"], ), - compiler_flags = [ - "-Wall", + compiler_flags = CXX_LIBRARY_COMPILER_FLAGS + [ "-fexceptions", "-frtti", - "-std=c++1y", - ] + REACT_LIBRARY_EXTRA_COMPILER_FLAGS, + ], exported_headers = CXXREACT_PUBLIC_HEADERS, + fbandroid_preprocessor_flags = [ + "-DWITH_JSC_EXTRA_TRACING=1", + "-DWITH_JSC_MEMORY_PRESSURE=1", + "-DWITH_REACT_INTERNAL_SETTINGS=1", + "-DWITH_FB_MEMORY_PROFILING=1", + ], + fbandroid_visibility = [ + # TL;DR: If you depend on this target directly, you're gonna have a + # Bad Time(TM). + # + # `facebook::react::JSCExecutor::initOnJSVMThread` (in `:bridge`) does + # some platform-dependant setup. Exactly what setup to do is + # determined by some static functors, defined in `Platform.h`, which + # are initially `nullptr`. On Android, these functors are properly + # assigned as part of `xreact`'s `JNI_OnLoad`. By depending directly + # on the bridge, we can mess up the SO initialisation order, causing + # `initOnJSVMThread` to be called before the platform-specific hooks + # have been properly initialised. Bad Times(TM). + # -- @ashokmenon (2017/01/03) + react_native_target("jni/xreact/jni:jni"), + react_native_xplat_target("cxxreact/..."), + ], + fbobjc_frameworks = [ + "$SDKROOT/System/Library/Frameworks/JavaScriptCore.framework", + ], + fbobjc_header_path_prefix = "cxxreact", + fbobjc_inherited_buck_flags = STATIC_LIBRARY_IOS_FLAGS, + fbobjc_preprocessor_flags = DEBUG_PREPROCESSOR_FLAGS, + fbobjc_visibility = ["PUBLIC"], force_static = True, header_namespace = "cxxreact", headers = glob( @@ -175,12 +119,16 @@ react_library( preprocessor_flags = [ "-DLOG_TAG=\"ReactNative\"", "-DWITH_FBSYSTRACE=1", + ] + INSPECTOR_FLAGS, + tests = [ + react_native_xplat_target("cxxreact/tests:tests"), ], xcode_public_headers_symlinks = True, deps = [ ":module", "//xplat/fbsystrace:fbsystrace", + "//xplat/folly:molly", react_native_xplat_target("jschelpers:jschelpers"), react_native_xplat_target("microprofiler:microprofiler"), - ], + ] + JSC_DEPS, ) diff --git a/ReactCommon/cxxreact/JSCExecutor.cpp b/ReactCommon/cxxreact/JSCExecutor.cpp index 0a43f1eb8d..42dc75e02c 100644 --- a/ReactCommon/cxxreact/JSCExecutor.cpp +++ b/ReactCommon/cxxreact/JSCExecutor.cpp @@ -200,13 +200,13 @@ void JSCExecutor::initOnJSVMThread() throw(JSException) { // Create a custom global class, so we can store data in it later using JSObjectSetPrivate JSClassRef globalClass = nullptr; { - SystraceSection s("JSClassCreate"); + SystraceSection s_("JSClassCreate"); JSClassDefinition definition = kJSClassDefinitionEmpty; definition.attributes |= kJSClassAttributeNoAutomaticPrototype; globalClass = JSC_JSClassCreate(useCustomJSC, &definition); } { - SystraceSection s("JSGlobalContextCreateInGroup"); + SystraceSection s_("JSGlobalContextCreateInGroup"); m_context = JSC_JSGlobalContextCreateInGroup(useCustomJSC, nullptr, globalClass); } JSC_JSClassRelease(useCustomJSC, globalClass); diff --git a/ReactCommon/cxxreact/ModuleRegistry.cpp b/ReactCommon/cxxreact/ModuleRegistry.cpp index 649c1438ec..72c9e5ecbb 100644 --- a/ReactCommon/cxxreact/ModuleRegistry.cpp +++ b/ReactCommon/cxxreact/ModuleRegistry.cpp @@ -72,12 +72,12 @@ folly::Optional ModuleRegistry::getConfig(const std::string& name) folly::dynamic config = folly::dynamic::array(name); { - SystraceSection s("getConstants"); + SystraceSection s_("getConstants"); config.push_back(module->getConstants()); } { - SystraceSection s("getMethods"); + SystraceSection s_("getMethods"); std::vector methods = module->getMethods(); folly::dynamic methodNames = folly::dynamic::array; diff --git a/ReactCommon/cxxreact/Platform.cpp b/ReactCommon/cxxreact/Platform.cpp index 60d894c469..d95ea08c34 100644 --- a/ReactCommon/cxxreact/Platform.cpp +++ b/ReactCommon/cxxreact/Platform.cpp @@ -5,17 +5,21 @@ namespace facebook { namespace react { +#if __clang__ +#pragma clang diagnostic push +#pragma clang diagnostic ignored "-Wglobal-constructors" +#endif + namespace ReactMarker { -LogTaggedMarker logTaggedMarker; +LogTaggedMarker logTaggedMarker = nullptr; void logMarker(const ReactMarkerId markerId) { logTaggedMarker(markerId, nullptr); } - }; namespace PerfLogging { -InstallNativeHooks installNativeHooks; +InstallNativeHooks installNativeHooks = nullptr; }; namespace JSNativeHooks { @@ -23,4 +27,8 @@ Hook loggingHook = nullptr; Hook nowHook = nullptr; } +#if __clang__ +#pragma clang diagnostic pop +#endif + } } diff --git a/ReactCommon/cxxreact/SampleCxxModule.cpp b/ReactCommon/cxxreact/SampleCxxModule.cpp index c0a0d9b36e..89b5c5279d 100644 --- a/ReactCommon/cxxreact/SampleCxxModule.cpp +++ b/ReactCommon/cxxreact/SampleCxxModule.cpp @@ -133,9 +133,8 @@ void SampleCxxModule::load(folly::dynamic args, Callback cb) { }}} -// By convention, the function name should be the same as the class -// name. -extern "C" facebook::xplat::module::CxxModule *SampleCxxModule() { +// By convention, the function name should be the same as the class name. +facebook::xplat::module::CxxModule *SampleCxxModule() { return new facebook::xplat::samples::SampleCxxModule( folly::make_unique()); } diff --git a/ReactCommon/cxxreact/SampleCxxModule.h b/ReactCommon/cxxreact/SampleCxxModule.h index e90f9f7c2e..ded0b43613 100644 --- a/ReactCommon/cxxreact/SampleCxxModule.h +++ b/ReactCommon/cxxreact/SampleCxxModule.h @@ -1,13 +1,12 @@ // Copyright 2004-present Facebook. All Rights Reserved. -#ifndef FBSAMPLEXPLATMODULE -#define FBSAMPLEXPLATMODULE - -#include +#pragma once #include #include +#include + namespace facebook { namespace xplat { namespace samples { // In a less contrived example, Sample would be part of a traditional @@ -48,5 +47,4 @@ private: }}} -#endif - +extern "C" facebook::xplat::module::CxxModule *SampleCxxModule();