Cleanup cxxreact BUCK file

Reviewed By: mzlee

Differential Revision: D5020563

fbshipit-source-id: 286a2d1c44623b5b9fd923ef8684d263500791b3

ReactCommon/DEFS

@@ -1,3 +1,7 @@
 # Set up common deps
 
 GLOG_DEP = "//ReactAndroid/build/third-party-ndk/glog:glog"
+
+INSPECTOR_FLAGS = []
+
+JSC_DEPS = []

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,
 )

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);

ReactCommon/cxxreact/ModuleRegistry.cpp

@@ -72,12 +72,12 @@ folly::Optional<ModuleConfig> 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<MethodDescriptor> methods = module->getMethods();
 
     folly::dynamic methodNames = folly::dynamic::array;

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
+
 } }

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<facebook::xplat::samples::Sample>());
 }

ReactCommon/cxxreact/SampleCxxModule.h

@@ -1,13 +1,12 @@
 // Copyright 2004-present Facebook. All Rights Reserved.
 
-#ifndef FBSAMPLEXPLATMODULE
-#define FBSAMPLEXPLATMODULE
-
-#include <cxxreact/CxxModule.h>
+#pragma once
 
 #include <memory>
 #include <vector>
 
+#include <cxxreact/CxxModule.h>
+
 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();