Enable -Wpedantic for targets inside ReactCommon

Summary:
React Native is compiled downstream with MSVC, meaning the introduction of code depending on language extensions specific to gcc/clang may cause breakage.

We can enable `-Wpedantic` to flag any behavior not officially supported by the specified C++ standard. This will includes rules beyond what MSVC has trouble with, but seems to not have too many "noisy warnings".

This change enables -Wpedantic in BUCK targets within ReactCommon.

This makes the OSS C++ build for Android/iOS slightly more permissive than the internal build, A followup is to add the changes to OSS build logic as well, to avoid contributors seeing more errors upon internal submission. (checking with cortinico on how to do this for Android).

react-native-windows uses a higher warning level than `-Wall`, which is an additional cause of compilation failures, but is not addressed as part of this change.

Changelog:
[Internal][Changed] - Enable -Wpedantic for targets inside ReactCommon

Reviewed By: rshest

Differential Revision: D38457812

fbshipit-source-id: 014da1ac0b7ad8f78154e6e447ed58def6bd0d47

React/Base/RCTAssert.h

@@ -159,18 +159,18 @@ RCT_EXTERN NSString *RCTFormatStackTrace(NSArray<NSDictionary<NSString *, id> *>
  */
 #if DEBUG
 
-#define RCTAssertThread(thread, format...)                                                                            \
+#define RCTAssertThread(thread, ...)                                                                                  \
   _Pragma("clang diagnostic push") _Pragma("clang diagnostic ignored \"-Wdeprecated-declarations\"") RCTAssert(       \
       [(id)thread isKindOfClass:[NSString class]]       ? [RCTCurrentThreadName() isEqualToString:(NSString *)thread] \
           : [(id)thread isKindOfClass:[NSThread class]] ? [NSThread currentThread] == (NSThread *)thread              \
                                                         : dispatch_get_current_queue() == (dispatch_queue_t)thread,   \
-      format);                                                                                                        \
+      __VA_ARGS__);                                                                                                   \
   _Pragma("clang diagnostic pop")
 
 #else
 
-#define RCTAssertThread(thread, format...) \
-  do {                                     \
+#define RCTAssertThread(thread, ...) \
+  do {                               \
   } while (0)
 
 #endif

ReactAndroid/src/main/jni/react/jni/JCallback.h

@@ -30,9 +30,16 @@ class JCxxCallbackImpl : public jni::HybridClass<JCxxCallbackImpl, JCallback> {
       "Lcom/facebook/react/bridge/CxxCallbackImpl;";
 
   static void registerNatives() {
+#if __clang__
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wgnu-zero-variadic-macro-arguments"
+#endif
     javaClassStatic()->registerNatives({
         makeNativeMethod("nativeInvoke", JCxxCallbackImpl::invoke),
     });
+#if __clang__
+#pragma clang diagnostic pop
+#endif
   }
 
  private:

ReactCommon/butter/BUCK

@@ -29,6 +29,7 @@ rn_xplat_cxx_library(
         ],
         prefix = "butter",
     ),
+    compiler_flags_pedantic = True,
     fbobjc_compiler_flags = APPLE_COMPILER_FLAGS,
     fbobjc_preprocessor_flags = get_preprocessor_flags_for_build_mode() + get_apple_inspector_flags(),
     force_static = True,

ReactCommon/callinvoker/BUCK

@@ -10,6 +10,7 @@ rn_xplat_cxx_library(
         ],
         prefix = "ReactCommon",
     ),
+    compiler_flags_pedantic = True,
     labels = [
         "pfh:ReactNative_CommonInfrastructurePlaceholder",
         "supermodule:xplat/default/public.react_native.infra",

ReactCommon/cxxreact/BUCK

@@ -12,6 +12,7 @@ rn_xplat_cxx_library(
         ],
         prefix = "cxxreact",
     ),
+    compiler_flags_pedantic = True,
     fbobjc_compiler_flags = get_apple_compiler_flags(),
     force_static = True,
     labels = [
@@ -36,6 +37,7 @@ rn_xplat_cxx_library(
         [("", "JSBigString.h")],
         prefix = "cxxreact",
     ),
+    compiler_flags_pedantic = True,
     fbobjc_compiler_flags = get_apple_compiler_flags(),
     force_static = True,
     labels = [
@@ -60,6 +62,7 @@ rn_xplat_cxx_library(
     compiler_flags = [
         "-fno-omit-frame-pointer",
     ],
+    compiler_flags_pedantic = True,
     fbobjc_compiler_flags = get_apple_compiler_flags(),
     labels = [
         "pfh:ReactNative_CommonInfrastructurePlaceholder",
@@ -118,6 +121,7 @@ rn_xplat_cxx_library(
         )
         for header in CXXREACT_PUBLIC_HEADERS
     ]),
+    compiler_flags_pedantic = True,
     fbandroid_preprocessor_flags = get_android_inspector_flags(),
     fbobjc_compiler_flags = get_apple_compiler_flags(),
     fbobjc_force_static = True,

ReactCommon/cxxreact/JSBundleType.h

@@ -36,16 +36,19 @@ enum struct ScriptTag {
  * bytes from a bundle in a way that gives access to that information.
  */
 FOLLY_PACK_PUSH
+
+struct FOLLY_PACK_ATTR Magic32 {
+  uint32_t value;
+  uint32_t reserved_;
+};
+
 struct FOLLY_PACK_ATTR BundleHeader {
   BundleHeader() {
     std::memset(this, 0, sizeof(BundleHeader));
   }
 
   union {
-    struct {
-      uint32_t value;
-      uint32_t reserved_;
-    } magic32;
+    Magic32 magic32;
     uint64_t magic64;
   };
   uint32_t version;

ReactCommon/jsi/BUCK

@@ -19,6 +19,7 @@ rn_xplat_cxx_library(
         "-Wdelete-non-virtual-dtor",
         "-Wwrite-strings",
     ],
+    compiler_flags_pedantic = True,
     cxx_compiler_flags = [
         "-Wglobal-constructors",
         "-Wmissing-prototypes",
@@ -43,6 +44,7 @@ rn_xplat_cxx_library(
     exported_headers = [
         "jsi/JSIDynamic.h",
     ],
+    compiler_flags_pedantic = True,
     fbobjc_force_static = True,
     labels = [
         "pfh:ReactNative_CommonInfrastructurePlaceholder",
@@ -68,6 +70,7 @@ rn_xplat_cxx_library(
         "JSCRuntime.h",
     ],
     apple_sdks = (IOS, MACOSX),
+    compiler_flags_pedantic = True,
     fbobjc_compiler_flags = [
         "-Os",
     ],

ReactCommon/jsinspector/BUCK

@@ -26,6 +26,7 @@ rn_xplat_cxx_library(
         ],
         prefix = "jsinspector",
     ),
+    compiler_flags_pedantic = True,
     fbandroid_preferred_linkage = "shared",
     labels = [
         "pfh:ReactNative_CommonInfrastructurePlaceholder",

ReactCommon/logger/BUCK

@@ -26,6 +26,7 @@ rn_xplat_cxx_library(
         ],
         prefix = "logger",
     ),
+    compiler_flags_pedantic = True,
     fbandroid_preferred_linkage = "shared",
     labels = [
         "pfh:ReactNative_CommonInfrastructurePlaceholder",

ReactCommon/react/bridging/BUCK

@@ -7,6 +7,7 @@ rn_xplat_cxx_library(
     exported_headers = glob(["*.h"]),
     compiler_flags_enable_exceptions = True,
     compiler_flags_enable_rtti = True,
+    compiler_flags_pedantic = True,
     labels = [
         "pfh:ReactNative_CommonInfrastructurePlaceholder",
         "supermodule:xplat/default/public.react_native.infra",
@@ -28,6 +29,7 @@ rn_xplat_cxx_library(
     name = "testlib",
     header_namespace = "react/bridging",
     exported_headers = glob(["tests/*.h"]),
+    compiler_flags_pedantic = True,
     platforms = (ANDROID, APPLE, CXX),
     visibility = ["PUBLIC"],
     exported_deps = [

ReactCommon/react/config/BUCK

@@ -20,6 +20,7 @@ rn_xplat_cxx_library(
     ),
     compiler_flags_enable_exceptions = True,
     compiler_flags_enable_rtti = True,  # for ReactNativeConfig
+    compiler_flags_pedantic = True,
     fbobjc_compiler_flags = APPLE_COMPILER_FLAGS,
     fbobjc_preprocessor_flags = get_preprocessor_flags_for_build_mode() + get_apple_inspector_flags(),
     force_static = True,

ReactCommon/react/debug/BUCK

@@ -29,6 +29,7 @@ rn_xplat_cxx_library(
         ],
         prefix = "react/debug",
     ),
+    compiler_flags_pedantic = True,
     exported_platform_linker_flags = [
         (
             "^android.*",

ReactCommon/react/nativemodule/core/BUCK

@@ -15,6 +15,7 @@ rn_xplat_cxx_library(
     compiler_flags = [
         "-Wno-global-constructors",
     ],
+    compiler_flags_pedantic = True,
     fbandroid_deps = [
         react_native_target("jni/react/jni:jni"),
         FBJNI_TARGET,

ReactCommon/react/nativemodule/samples/BUCK

@@ -12,6 +12,7 @@ rn_xplat_cxx_library(
         ],
         prefix = "ReactCommon",
     ),
+    compiler_flags_pedantic = True,
     fbandroid_deps = [
         react_native_target("jni/react/jni:jni"),
         FBJNI_TARGET,

ReactCommon/react/renderer/animations/BUCK

@@ -34,6 +34,7 @@ rn_xplat_cxx_library(
     compiler_flags = [
         "-lm",
     ],
+    compiler_flags_pedantic = True,
     fbobjc_compiler_flags = APPLE_COMPILER_FLAGS,
     fbobjc_preprocessor_flags = get_preprocessor_flags_for_build_mode() + get_apple_inspector_flags(),
     force_static = True,

ReactCommon/react/renderer/attributedstring/BUCK

@@ -31,6 +31,7 @@ rn_xplat_cxx_library(
         ],
         prefix = "react/renderer/attributedstring",
     ),
+    compiler_flags_pedantic = True,
     fbandroid_deps = [
         react_native_xplat_target("react/renderer/mapbuffer:mapbuffer"),
     ],

ReactCommon/react/renderer/componentregistry/BUCK

@@ -30,6 +30,7 @@ rn_xplat_cxx_library(
         ],
         prefix = "react/renderer/componentregistry",
     ),
+    compiler_flags_pedantic = True,
     fbobjc_compiler_flags = APPLE_COMPILER_FLAGS,
     fbobjc_preprocessor_flags = get_preprocessor_flags_for_build_mode() + get_apple_inspector_flags(),
     force_static = True,

ReactCommon/react/renderer/componentregistry/native/BUCK

@@ -29,6 +29,7 @@ rn_xplat_cxx_library(
         ],
         prefix = "react/renderer/componentregistry/native",
     ),
+    compiler_flags_pedantic = True,
     fbobjc_compiler_flags = APPLE_COMPILER_FLAGS,
     fbobjc_preprocessor_flags = get_preprocessor_flags_for_build_mode() + get_apple_inspector_flags(),
     force_static = True,

ReactCommon/react/renderer/components/image/BUCK

@@ -29,6 +29,7 @@ rn_xplat_cxx_library(
         ],
         prefix = "react/renderer/components/image",
     ),
+    compiler_flags_pedantic = True,
     fbandroid_deps = [
         react_native_xplat_target("react/renderer/mapbuffer:mapbuffer"),
     ],

ReactCommon/react/renderer/components/inputaccessory/BUCK

@@ -24,6 +24,7 @@ rn_xplat_cxx_library(
         ],
         prefix = "react/renderer/components/inputaccessory",
     ),
+    compiler_flags_pedantic = True,
     fbobjc_compiler_flags = APPLE_COMPILER_FLAGS,
     fbobjc_preprocessor_flags = get_preprocessor_flags_for_build_mode() + get_apple_inspector_flags(),
     force_static = True,

ReactCommon/react/renderer/components/legacyviewmanagerinterop/BUCK

@@ -27,6 +27,7 @@ rn_xplat_cxx_library(
         ],
         prefix = "react/renderer/components/legacyviewmanagerinterop",
     ),
+    compiler_flags_pedantic = True,
     fbobjc_compiler_flags = APPLE_COMPILER_FLAGS,
     fbobjc_preprocessor_flags = get_preprocessor_flags_for_build_mode() + get_apple_inspector_flags(),
     force_static = True,

ReactCommon/react/renderer/components/modal/BUCK

@@ -31,6 +31,7 @@ rn_xplat_cxx_library(
         ],
         prefix = "react/renderer/components/modal",
     ),
+    compiler_flags_pedantic = True,
     fbandroid_deps = [
         react_native_xplat_target("react/renderer/mapbuffer:mapbuffer"),
     ],

ReactCommon/react/renderer/components/progressbar/BUCK

@@ -32,6 +32,7 @@ rn_xplat_cxx_library(
         ],
         prefix = "react/renderer/components/progressbar",
     ),
+    compiler_flags_pedantic = True,
     cxx_tests = [":tests"],
     fbandroid_deps = [
         react_native_target("jni/react/jni:jni"),

ReactCommon/react/renderer/components/root/BUCK

@@ -29,6 +29,7 @@ rn_xplat_cxx_library(
         ],
         prefix = "react/renderer/components/root",
     ),
+    compiler_flags_pedantic = True,
     fbobjc_compiler_flags = APPLE_COMPILER_FLAGS,
     fbobjc_preprocessor_flags = get_preprocessor_flags_for_build_mode() + get_apple_inspector_flags(),
     labels = [

ReactCommon/react/renderer/components/safeareaview/BUCK

@@ -24,6 +24,7 @@ rn_xplat_cxx_library(
         ],
         prefix = "react/renderer/components/safeareaview",
     ),
+    compiler_flags_pedantic = True,
     fbobjc_compiler_flags = APPLE_COMPILER_FLAGS,
     fbobjc_preprocessor_flags = get_preprocessor_flags_for_build_mode() + get_apple_inspector_flags(),
     force_static = True,

ReactCommon/react/renderer/components/scrollview/BUCK

@@ -32,6 +32,7 @@ rn_xplat_cxx_library(
         ],
         prefix = "react/renderer/components/scrollview",
     ),
+    compiler_flags_pedantic = True,
     fbandroid_deps = [
         react_native_xplat_target("react/renderer/mapbuffer:mapbuffer"),
     ],

ReactCommon/react/renderer/components/slider/BUCK

@@ -33,6 +33,7 @@ rn_xplat_cxx_library(
         ],
         prefix = "react/renderer/components/slider",
     ),
+    compiler_flags_pedantic = True,
     cxx_tests = [":tests"],
     fbandroid_deps = [
         react_native_target("jni/react/jni:jni"),

ReactCommon/react/renderer/components/switch/BUCK

@@ -34,6 +34,7 @@ rn_xplat_cxx_library(
         ],
         prefix = "react/renderer/components/androidswitch",
     ),
+    compiler_flags_pedantic = True,
     cxx_tests = [":tests"],
     fbandroid_deps = [
         react_native_target("jni/react/jni:jni"),

ReactCommon/react/renderer/components/text/BUCK

@@ -36,6 +36,7 @@ rn_xplat_cxx_library(
         ],
         prefix = "react/renderer/components/text",
     ),
+    compiler_flags_pedantic = True,
     cxx_tests = [":tests"],
     fbandroid_deps = [
         react_native_xplat_target("react/renderer/mapbuffer:mapbuffer"),

ReactCommon/react/renderer/components/textinput/BUCK

@@ -33,6 +33,7 @@ rn_xplat_cxx_library(
         ],
         prefix = "react/renderer/components/androidtextinput",
     ),
+    compiler_flags_pedantic = True,
     cxx_tests = [":tests"],
     fbandroid_deps = [
         react_native_xplat_target("react/renderer/mapbuffer:mapbuffer"),

ReactCommon/react/renderer/components/textinput/iostextinput/BUCK

@@ -33,6 +33,7 @@ rn_xplat_cxx_library(
         # TODO(shergin) T26519801 Figure out better directories structure
         prefix = "react/renderer/components/iostextinput",
     ),
+    compiler_flags_pedantic = True,
     cxx_tests = [":tests"],
     fbobjc_compiler_flags = APPLE_COMPILER_FLAGS,
     fbobjc_preprocessor_flags = get_preprocessor_flags_for_build_mode() + get_apple_inspector_flags(),

ReactCommon/react/renderer/components/unimplementedview/BUCK

@@ -29,6 +29,7 @@ rn_xplat_cxx_library(
         ],
         prefix = "react/renderer/components/unimplementedview",
     ),
+    compiler_flags_pedantic = True,
     fbobjc_compiler_flags = APPLE_COMPILER_FLAGS,
     fbobjc_preprocessor_flags = get_preprocessor_flags_for_build_mode() + get_apple_inspector_flags(),
     labels = [

ReactCommon/react/renderer/components/view/BUCK

@@ -35,6 +35,7 @@ rn_xplat_cxx_library(
         ],
         prefix = "react/renderer/components/view",
     ),
+    compiler_flags_pedantic = True,
     fbobjc_compiler_flags = APPLE_COMPILER_FLAGS,
     fbobjc_preprocessor_flags = get_preprocessor_flags_for_build_mode() + get_apple_inspector_flags(),
     force_static = True,

ReactCommon/react/renderer/components/view/propsConversions.h

@@ -657,30 +657,28 @@ static inline void fromRawValue(
         attrIterator->second.hasType<std::string>());
 
     result = NativeDrawable{
-        .kind = NativeDrawable::Kind::ThemeAttr,
-        .themeAttr = (std::string)attrIterator->second,
+        NativeDrawable::Kind::ThemeAttr,
+        (std::string)attrIterator->second,
     };
   } else if (type == "RippleAndroid") {
     auto color = map.find("color");
     auto borderless = map.find("borderless");
     auto rippleRadius = map.find("rippleRadius");
 
     result = NativeDrawable{
-        .kind = NativeDrawable::Kind::Ripple,
-        .ripple =
-            NativeDrawable::Ripple{
-                .color = color != map.end() && color->second.hasType<int32_t>()
-                    ? (int32_t)color->second
-                    : std::optional<int32_t>{},
-                .borderless = borderless != map.end() &&
-                        borderless->second.hasType<bool>()
-                    ? (bool)borderless->second
-                    : false,
-                .rippleRadius = rippleRadius != map.end() &&
-                        rippleRadius->second.hasType<Float>()
-                    ? (Float)rippleRadius->second
-                    : std::optional<Float>{},
-            },
+        NativeDrawable::Kind::Ripple,
+        std::string{},
+        NativeDrawable::Ripple{
+            color != map.end() && color->second.hasType<int32_t>()
+                ? (int32_t)color->second
+                : std::optional<int32_t>{},
+            borderless != map.end() && borderless->second.hasType<bool>()
+                ? (bool)borderless->second
+                : false,
+            rippleRadius != map.end() && rippleRadius->second.hasType<Float>()
+                ? (Float)rippleRadius->second
+                : std::optional<Float>{},
+        },
     };
   } else {
     LOG(ERROR) << "Unknown native drawable type: " << type;