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

This reverts commit 99fd54a. The addition of `-Werror` in `.bazelrc` appears to cause errors in our release process, so let's roll this back for now.
protocolbuffers · Aug 27, 2024 · 1efed95 · 1efed95
1 parent 99fd54a
commit 1efed95
Show file tree

Hide file tree

Showing 18 changed files with 61 additions and 36 deletions.
diff --git a/.bazelrc b/.bazelrc
@@ -1,8 +1,5 @@
 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

diff --git a/.github/workflows/test_objectivec.yml b/.github/workflows/test_objectivec.yml
@@ -76,7 +76,6 @@ 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:
@@ -92,7 +91,7 @@ jobs:
           credentials: ${{ secrets.GAR_SERVICE_ACCOUNT }}
           bazel-cache: cocoapods/${{ matrix.XCODE }}
           bash: |
-            ./regenerate_stale_files.sh $BAZEL_FLAGS ${{ matrix.EXTRA_FLAGS }} --xcode_version="${{ matrix.XCODE }}"
+            ./regenerate_stale_files.sh $BAZEL_FLAGS --xcode_version="${{ matrix.XCODE }}"
             pod lib lint --verbose \
               --configuration=${{ matrix.CONFIGURATION }} \
               --platforms=${{ matrix.PLATFORM }} \

diff --git a/.github/workflows/test_rust.yml b/.github/workflows/test_rust.yml
@@ -27,7 +27,6 @@ jobs:
           credentials: ${{ secrets.GAR_SERVICE_ACCOUNT }}
           bazel-cache: rust_linux
           bazel: >-
-            test --cxxopt=-std=c++17 --host_cxxopt=-std=c++17 --copt="-Wno-return-type-c-linkage"
-            //rust:protobuf_upb_test //rust:protobuf_cpp_test
+            test //rust:protobuf_upb_test //rust:protobuf_cpp_test
             //rust/test/rust_proto_library_unit_test:rust_upb_aspect_test
             //src/google/protobuf/compiler/rust/...
diff --git a/.github/workflows/test_upb.yml b/.github/workflows/test_upb.yml
@@ -59,10 +59,7 @@ 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
-            --copt="-Wno-error=maybe-uninitialized" --copt="-Wno-error=attributes"
-            //bazel/... //benchmarks/... //lua/... //protos/... //protos_generator/... //python/... //upb/... //upb_generator/...
+          bazel: test --cxxopt=-std=c++17 --host_cxxopt=-std=c++17 -c opt //bazel/... //benchmarks/... //lua/... //protos/... //protos_generator/... //python/... //upb/... //upb_generator/...
 
   windows:
     strategy:

diff --git a/build_defs/cpp_opts.bzl b/build_defs/cpp_opts.bzl
@@ -21,6 +21,7 @@ COPTS = select({
         "-Woverloaded-virtual",
         "-Wno-sign-compare",
         "-Wno-nonnull",
+        "-Werror",
     ],
 })
 

diff --git a/ci/Linux.bazelrc b/ci/Linux.bazelrc
@@ -1,4 +1,3 @@
 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"
diff --git a/ci/macOS.bazelrc b/ci/macOS.bazelrc
@@ -1,5 +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"
 common --repo_env=BAZEL_NO_APPLE_CPP_TOOLCHAIN=1
diff --git a/conformance/binary_json_conformance_suite.cc b/conformance/binary_json_conformance_suite.cc
@@ -3449,11 +3449,12 @@ BinaryAndJsonConformanceSuiteImpl<MessageType>::GetFieldForOneofType(
 template <typename MessageType>
 std::string BinaryAndJsonConformanceSuiteImpl<MessageType>::SyntaxIdentifier()
     const {
-  if (std::is_same<MessageType, TestAllTypesProto2>::value) {
+  if constexpr (std::is_same<MessageType, TestAllTypesProto2>::value) {
     return "Proto2";
-  } else if (std::is_same<MessageType, TestAllTypesProto3>::value) {
+  } else if constexpr (std::is_same<MessageType, TestAllTypesProto3>::value) {
     return "Proto3";
-  } else if (std::is_same<MessageType, TestAllTypesProto2Editions>::value) {
+  } else if constexpr (std::is_same<MessageType,
+                                    TestAllTypesProto2Editions>::value) {
     return "Editions_Proto2";
   } else {
     return "Editions_Proto3";

diff --git a/python/map.c b/python/map.c
@@ -259,6 +259,16 @@ 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);
 

diff --git a/python/message.c b/python/message.c
@@ -1593,6 +1593,7 @@ 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);

diff --git a/python/protobuf.c b/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 upb_alloc* global_alloc = &trim_alloc;
+static const upb_alloc* global_alloc = &trim_alloc;
 // end:github_only
 
 static upb_Arena* PyUpb_NewArena(void) {

diff --git a/ruby/ext/google/protobuf_c/convert.c b/ruby/ext/google/protobuf_c/convert.c
@@ -204,8 +204,7 @@ upb_MessageValue Convert_RubyToUpb(VALUE value, const char* name,
           ret.uint64_val = NUM2ULL(value);
           break;
         default:
-          rb_raise(cTypeError, "Convert_RubyToUpb(): Unexpected type %d",
-                   (int)type_info.type);
+          break;
       }
       break;
     default:

diff --git a/src/google/protobuf/BUILD.bazel b/src/google/protobuf/BUILD.bazel
@@ -1038,7 +1038,6 @@ 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",

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

diff --git a/src/google/protobuf/string_view_test.cc b/src/google/protobuf/string_view_test.cc
@@ -12,9 +12,6 @@
 #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 {
@@ -287,12 +284,10 @@ 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);
@@ -314,5 +309,3 @@ TEST(StringViewFieldTest, RepeatedSetAndGetByReflection) {
 }  // namespace
 }  // namespace protobuf
 }  // namespace google
-
-#include "google/protobuf/port_undef.inc"
diff --git a/third_party/zlib.BUILD b/third_party/zlib.BUILD
@@ -59,10 +59,6 @@ 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",

diff --git a/upb/io/zero_copy_stream_test.cc b/upb/io/zero_copy_stream_test.cc
@@ -45,6 +45,12 @@ 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;
 };
@@ -151,6 +157,35 @@ 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) {

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