Move -Werror to our test/dev bazelrc files. (#17939)

* Move -Werror to our test/dev bazelrc files.

Putting it into BUILD files unintentionally forces it on all our downstream users.  Instead, we just want to enable this during testing and let them choose for themselves in their builds.

Note, that this expands the scope of -Werror to our entire repo for CI, so a bunch of fixes and opt-outs had to be applied to get this change passing.

Closed #14714

PiperOrigin-RevId: 666903224

* Fix extra warnings on 27.x

* Fix zlib issues on macos

* Fix spurious upb warnings in 27.x

* Second try at zlib/macos issues

* Only disable deprecated-non-prototype on macos-14

.bazelrc

@@ -1,5 +1,8 @@
 build --cxxopt=-std=c++17 --host_cxxopt=-std=c++17
 
+build --copt="-Werror" --copt="-Wno-sign-compare" --copt="-Wno-sign-conversion" --copt="-Wno-error=sign-conversion"  --copt="-Wno-deprecated-declarations"
+
+
 build:dbg --compilation_mode=dbg
 
 build:opt --compilation_mode=opt

.github/workflows/test_objectivec.yml

@@ -76,6 +76,7 @@ jobs:
           - OS: macos-14
             PLATFORM: "visionos"
             XCODE: "15.2"
+            EXTRA_FLAGS: --copt="-Wno-deprecated-non-prototype"
     name: CocoaPods ${{ matrix.PLATFORM }} ${{ matrix.CONFIGURATION }}
     runs-on: ${{ matrix.OS }}
     steps:
@@ -91,7 +92,7 @@ jobs:
           credentials: ${{ secrets.GAR_SERVICE_ACCOUNT }}
           bazel-cache: cocoapods/${{ matrix.XCODE }}
           bash: |
-            ./regenerate_stale_files.sh $BAZEL_FLAGS --xcode_version="${{ matrix.XCODE }}"
+            ./regenerate_stale_files.sh $BAZEL_FLAGS ${{ matrix.EXTRA_FLAGS }} --xcode_version="${{ matrix.XCODE }}"
             pod lib lint --verbose \
               --configuration=${{ matrix.CONFIGURATION }} \
               --platforms=${{ matrix.PLATFORM }} \

.github/workflows/test_rust.yml

@@ -27,6 +27,7 @@ jobs:
           credentials: ${{ secrets.GAR_SERVICE_ACCOUNT }}
           bazel-cache: rust_linux
           bazel: >-
-            test //rust:protobuf_upb_test //rust:protobuf_cpp_test
+            test --cxxopt=-std=c++17 --host_cxxopt=-std=c++17 --copt="-Wno-return-type-c-linkage"
+            //rust:protobuf_upb_test //rust:protobuf_cpp_test
             //rust/test/rust_proto_library_unit_test:rust_upb_aspect_test
             //src/google/protobuf/compiler/rust/...

.github/workflows/test_upb.yml

@@ -59,7 +59,10 @@ jobs:
           image: "us-docker.pkg.dev/protobuf-build/containers/test/linux/gcc:12.2-6.3.0-63dd26c0c7a808d92673a3e52e848189d4ab0f17"
           credentials: ${{ secrets.GAR_SERVICE_ACCOUNT }}
           bazel-cache: "upb-bazel-gcc"
-          bazel: test --cxxopt=-std=c++17 --host_cxxopt=-std=c++17 -c opt //bazel/... //benchmarks/... //lua/... //protos/... //protos_generator/... //python/... //upb/... //upb_generator/...
+          bazel: >-
+            test --cxxopt=-std=c++17 --host_cxxopt=-std=c++17 -c opt
+            --copt="-Wno-error=maybe-uninitialized" --copt="-Wno-error=attributes"
+            //bazel/... //benchmarks/... //lua/... //protos/... //protos_generator/... //python/... //upb/... //upb_generator/...
 
   windows:
     strategy:

build_defs/cpp_opts.bzl

@@ -21,7 +21,6 @@ COPTS = select({
         "-Woverloaded-virtual",
         "-Wno-sign-compare",
         "-Wno-nonnull",
-        "-Werror",
     ],
 })
 

ci/Linux.bazelrc

@@ -1,3 +1,4 @@
 import common.bazelrc
 
 build --cxxopt=-std=c++14 --host_cxxopt=-std=c++14
+build --copt="-Werror" --copt="-Wno-sign-compare" --copt="-Wno-sign-conversion" --copt="-Wno-error=sign-conversion" --copt="-Wno-deprecated-declarations"

ci/macOS.bazelrc

@@ -1,4 +1,5 @@
 import common.bazelrc
 
 build --cxxopt=-std=c++14 --host_cxxopt=-std=c++14
+build --copt="-Werror" --copt="-Wno-sign-compare" --copt="-Wno-sign-conversion" --copt="-Wno-error=sign-conversion" --copt="-Wno-deprecated-declarations"
 common --repo_env=BAZEL_NO_APPLE_CPP_TOOLCHAIN=1

conformance/binary_json_conformance_suite.cc

@@ -3449,12 +3449,11 @@ BinaryAndJsonConformanceSuiteImpl<MessageType>::GetFieldForOneofType(
 template <typename MessageType>
 std::string BinaryAndJsonConformanceSuiteImpl<MessageType>::SyntaxIdentifier()
     const {
-  if constexpr (std::is_same<MessageType, TestAllTypesProto2>::value) {
+  if (std::is_same<MessageType, TestAllTypesProto2>::value) {
     return "Proto2";
-  } else if constexpr (std::is_same<MessageType, TestAllTypesProto3>::value) {
+  } else if (std::is_same<MessageType, TestAllTypesProto3>::value) {
     return "Proto3";
-  } else if constexpr (std::is_same<MessageType,
-                                    TestAllTypesProto2Editions>::value) {
+  } else if (std::is_same<MessageType, TestAllTypesProto2Editions>::value) {
     return "Editions_Proto2";
   } else {
     return "Editions_Proto3";

python/map.c

@@ -259,16 +259,6 @@ static Py_ssize_t PyUpb_MapContainer_Length(PyObject* _self) {
   return map ? upb_Map_Size(map) : 0;
 }
 
-static PyUpb_MapContainer* PyUpb_MapContainer_Check(PyObject* _self) {
-  PyUpb_ModuleState* state = PyUpb_ModuleState_Get();
-  if (!PyObject_TypeCheck(_self, state->message_map_container_type) &&
-      !PyObject_TypeCheck(_self, state->scalar_map_container_type)) {
-    PyErr_Format(PyExc_TypeError, "Expected protobuf map, but got %R", _self);
-    return NULL;
-  }
-  return (PyUpb_MapContainer*)_self;
-}
-
 int PyUpb_Message_InitMapAttributes(PyObject* map, PyObject* value,
                                     const upb_FieldDef* f);
 

python/message.c

@@ -1593,7 +1593,6 @@ static PyObject* PyUpb_Message_WhichOneof(PyObject* _self, PyObject* name) {
 }
 
 PyObject* DeepCopy(PyObject* _self, PyObject* arg) {
-  PyUpb_Message* self = (void*)_self;
   const upb_MessageDef* def = PyUpb_Message_GetMsgdef(_self);
   const upb_MiniTable* mini_table = upb_MessageDef_MiniTable(def);
   upb_Message* msg = PyUpb_Message_GetIfReified(_self);

python/protobuf.c

@@ -240,7 +240,7 @@ static void* upb_trim_allocfunc(upb_alloc* alloc, void* ptr, size_t oldsize,
   }
 }
 static upb_alloc trim_alloc = {&upb_trim_allocfunc};
-static const upb_alloc* global_alloc = &trim_alloc;
+static upb_alloc* global_alloc = &trim_alloc;
 // end:github_only
 
 static upb_Arena* PyUpb_NewArena(void) {

ruby/ext/google/protobuf_c/convert.c

@@ -204,7 +204,8 @@ upb_MessageValue Convert_RubyToUpb(VALUE value, const char* name,
           ret.uint64_val = NUM2ULL(value);
           break;
         default:
-          break;
+          rb_raise(cTypeError, "Convert_RubyToUpb(): Unexpected type %d",
+                   (int)type_info.type);
       }
       break;
     default:

src/google/protobuf/BUILD.bazel

@@ -1038,6 +1038,7 @@ cc_test(
     name = "string_view_test",
     srcs = ["string_view_test.cc"],
     deps = [
+        ":port",
         ":protobuf",
         ":unittest_string_view_cc_proto",
         "@com_google_absl//absl/strings:string_view",

src/google/protobuf/descriptor_database_unittest.cc

@@ -447,14 +447,14 @@ TEST_P(DescriptorDatabaseTest, ConflictingExtensionError) {
       "            extendee: \".Foo\" }");
 }
 
-INSTANTIATE_TEST_CASE_P(
+INSTANTIATE_TEST_SUITE_P(
     Simple, DescriptorDatabaseTest,
     testing::Values(&SimpleDescriptorDatabaseTestCase::New));
-INSTANTIATE_TEST_CASE_P(
+INSTANTIATE_TEST_SUITE_P(
     MemoryConserving, DescriptorDatabaseTest,
     testing::Values(&EncodedDescriptorDatabaseTestCase::New));
-INSTANTIATE_TEST_CASE_P(Pool, DescriptorDatabaseTest,
-                        testing::Values(&DescriptorPoolDatabaseTestCase::New));
+INSTANTIATE_TEST_SUITE_P(Pool, DescriptorDatabaseTest,
+                         testing::Values(&DescriptorPoolDatabaseTestCase::New));
 
 #endif  // GTEST_HAS_PARAM_TEST
 

src/google/protobuf/string_view_test.cc

@@ -12,6 +12,9 @@
 #include "google/protobuf/text_format.h"
 #include "google/protobuf/unittest_string_view.pb.h"
 
+// Must be included last.
+#include "google/protobuf/port_def.inc"
+
 namespace google {
 namespace protobuf {
 namespace {
@@ -284,10 +287,12 @@ TEST(StringViewFieldTest, RepeatedSetAndGetByReflection) {
   }
 
   // MutableRepeatedPtrField().
+  PROTOBUF_IGNORE_DEPRECATION_START;
   for (auto& it :
        *reflection->MutableRepeatedPtrField<std::string>(&message, field)) {
     it.append(it);
   }
+  PROTOBUF_IGNORE_DEPRECATION_STOP;
   {
     const auto& rep_str =
         reflection->GetRepeatedFieldRef<std::string>(message, field);
@@ -309,3 +314,5 @@ TEST(StringViewFieldTest, RepeatedSetAndGetByReflection) {
 }  // namespace
 }  // namespace protobuf
 }  // namespace google
+
+#include "google/protobuf/port_undef.inc"

third_party/zlib.BUILD

@@ -59,6 +59,10 @@ cc_library(
     hdrs = _ZLIB_PREFIXED_HEADERS,
     copts = select({
         "@platforms//os:windows": [],
+        "@platforms//os:macos": [
+            "-Wno-unused-variable",
+            "-Wno-implicit-function-declaration",
+        ],
         "//conditions:default": [
             "-Wno-deprecated-non-prototype",
             "-Wno-unused-variable",

upb/io/zero_copy_stream_test.cc

@@ -45,12 +45,6 @@ class IoTest : public testing::Test {
   // WriteStuff() writes.
   void ReadStuff(upb_ZeroCopyInputStream* input, bool read_eof = true);
 
-  // Similar to WriteStuff, but performs more sophisticated testing.
-  int WriteStuffLarge(upb_ZeroCopyOutputStream* output);
-  // Reads and tests a stream that should have been written to
-  // via WriteStuffLarge().
-  void ReadStuffLarge(upb_ZeroCopyInputStream* input);
-
   static const int kBlockSizes[];
   static const int kBlockSizeCount;
 };
@@ -157,35 +151,6 @@ void IoTest::ReadStuff(upb_ZeroCopyInputStream* input, bool read_eof) {
   }
 }
 
-int IoTest::WriteStuffLarge(upb_ZeroCopyOutputStream* output) {
-  WriteString(output, "Hello world!\n");
-  WriteString(output, "Some te");
-  WriteString(output, "xt.  Blah blah.");
-  WriteString(output, std::string(100000, 'x'));  // A very long string
-  WriteString(output, std::string(100000, 'y'));  // A very long string
-  WriteString(output, "01234567890123456789");
-
-  const int result = upb_ZeroCopyOutputStream_ByteCount(output);
-  EXPECT_EQ(result, 200055);
-  return result;
-}
-
-// Reads text from an input stream and expects it to match what WriteStuff()
-// writes.
-void IoTest::ReadStuffLarge(upb_ZeroCopyInputStream* input) {
-  ReadString(input, "Hello world!\nSome text.  ");
-  EXPECT_TRUE(upb_ZeroCopyInputStream_Skip(input, 5));
-  ReadString(input, "blah.");
-  EXPECT_TRUE(upb_ZeroCopyInputStream_Skip(input, 100000 - 10));
-  ReadString(input, std::string(10, 'x') + std::string(100000 - 20000, 'y'));
-  EXPECT_TRUE(upb_ZeroCopyInputStream_Skip(input, 20000 - 10));
-  ReadString(input, "yyyyyyyyyy01234567890123456789");
-  EXPECT_EQ(upb_ZeroCopyInputStream_ByteCount(input), 200055);
-
-  uint8_t byte;
-  EXPECT_EQ(ReadFromInput(input, &byte, 1), 0);
-}
-
 // ===================================================================
 
 TEST_F(IoTest, ArrayIo) {

upb/wire/eps_copy_input_stream_test.cc

@@ -281,7 +281,7 @@ TEST(EpsCopyInputStreamTest, ZeroSize) {
 // }
 //
 // // Test with:
-// //  $ blaze run --config=fuzztest third_party/upb:eps_copy_input_stream_test \
+// //  $ bazel run --config=fuzztest third_party/upb:eps_copy_input_stream_test
 // //   -- --gunit_fuzz=
 // FUZZ_TEST(EpsCopyFuzzTest, TestAgainstFakeStream)
 //     .WithDomains(ArbitraryEpsCopyTestScript());