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. Closed #14714 PiperOrigin-RevId: 582400675
protocolbuffers · Aug 22, 2024 · 8135707 · 8135707
1 parent a33a81b
commit 8135707
Show file tree

Hide file tree

Showing 17 changed files with 38 additions and 22 deletions.
diff --git a/.bazelrc b/.bazelrc
@@ -1,5 +1,7 @@
 build --cxxopt=-std=c++17 --host_cxxopt=-std=c++17
 
+build --copt="-Werror" --copt="-Wno-sign-compare" --copt="-Wsign-conversion" --copt="-Wreturn-type-c-linkage"
+
 build:dbg --compilation_mode=dbg
 
 build:opt --compilation_mode=opt

diff --git a/.github/workflows/test_rust.yml b/.github/workflows/test_rust.yml
@@ -28,6 +28,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
+            //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/build_defs/cpp_opts.bzl b/build_defs/cpp_opts.bzl
@@ -21,7 +21,6 @@ COPTS = select({
         "-Woverloaded-virtual",
         "-Wno-sign-compare",
         "-Wno-nonnull",
-        "-Werror",
     ],
 })
 

diff --git a/ci/Linux.bazelrc b/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="-Wsign-conversion" --copt="-Wreturn-type-c-linkage"
diff --git a/ci/macOS.bazelrc b/ci/macOS.bazelrc
@@ -1,5 +1,6 @@
 import common.bazelrc
 
 build --cxxopt=-std=c++14 --host_cxxopt=-std=c++14
+build --copt="-Werror" --copt="-Wno-sign-compare" --copt="-Wsign-conversion" --copt="-Wreturn-type-c-linkage"
 common --repo_env=BAZEL_NO_APPLE_CPP_TOOLCHAIN=1
 common --xcode_version_config=@com_google_protobuf//.github:host_xcodes
diff --git a/conformance/binary_json_conformance_suite.cc b/conformance/binary_json_conformance_suite.cc
@@ -3501,12 +3501,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";

diff --git a/conformance/conformance_test.cc b/conformance/conformance_test.cc
@@ -531,8 +531,8 @@ bool ConformanceTestSuite::RunTest(const std::string& test_name,
 
   // In essence, find what wildcarded test names expand to or direct matches
   // (without wildcards).
-  if (auto result = failure_list_root_.WalkDownMatch(test_name);
-      result.has_value()) {
+  auto result = failure_list_root_.WalkDownMatch(test_name);
+  if (result.has_value()) {
     string matched_equivalent = result.value();
     unmatched_.erase(matched_equivalent);
     TestStatus expansion;

diff --git a/conformance/failure_list_trie_node.cc b/conformance/failure_list_trie_node.cc
@@ -15,7 +15,8 @@ namespace google {
 namespace protobuf {
 
 absl::Status FailureListTrieNode::Insert(absl::string_view test_name) {
-  if (auto result = WalkDownMatch(test_name); result.has_value()) {
+  auto result = WalkDownMatch(test_name);
+  if (result.has_value()) {
     return absl::AlreadyExistsError(
         absl::StrFormat("Test name  %s  already exists in the trie  FROM  %s",
                         test_name, result.value()));
@@ -74,7 +75,8 @@ absl::optional<std::string> FailureListTrieNode::WalkDownMatch(
           return std::string(appended);
         }
       } else {
-        if (auto result = child->WalkDownMatch(to_match); result.has_value()) {
+        auto result = child->WalkDownMatch(to_match);
+        if (result.has_value()) {
           return absl::StrCat(appended, ".", result.value());
         }
       }

diff --git a/ruby/ext/google/protobuf_c/convert.c b/ruby/ext/google/protobuf_c/convert.c
@@ -228,7 +228,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:

diff --git a/rust/cpp_kernel/map.cc b/rust/cpp_kernel/map.cc
@@ -108,7 +108,7 @@ void IterGet(const internal::UntypedMapIterator* iter,
   internal::NodeBase* node = iter->node_;
   if constexpr (std::is_same<Key, PtrAndLen>::value) {
     const std::string* s = static_cast<const std::string*>(node->GetVoidKey());
-    *key = PtrAndLen(s->data(), s->size());
+    *key = PtrAndLen{s->data(), s->size()};
   } else {
     *key = *static_cast<const Key*>(node->GetVoidKey());
   }
@@ -219,10 +219,10 @@ __PB_RUST_EXPOSE_SCALAR_MAP_METHODS_FOR_VALUE_TYPE(int64_t, i64, int64_t,
 __PB_RUST_EXPOSE_SCALAR_MAP_METHODS_FOR_VALUE_TYPE(
     std::string, ProtoBytes, google::protobuf::rust::PtrAndLen, std::string*,
     std::move(*value),
-    google::protobuf::rust::PtrAndLen(cpp_value.data(), cpp_value.size()));
+    (google::protobuf::rust::PtrAndLen{cpp_value.data(), cpp_value.size()}));
 __PB_RUST_EXPOSE_SCALAR_MAP_METHODS_FOR_VALUE_TYPE(
     std::string, ProtoString, google::protobuf::rust::PtrAndLen, std::string*,
     std::move(*value),
-    google::protobuf::rust::PtrAndLen(cpp_value.data(), cpp_value.size()));
+    (google::protobuf::rust::PtrAndLen{cpp_value.data(), cpp_value.size()}));
 
 }  // extern "C"
diff --git a/rust/cpp_kernel/repeated.cc b/rust/cpp_kernel/repeated.cc
@@ -77,7 +77,7 @@ expose_repeated_field_methods(int64_t, i64);
   google::protobuf::rust::PtrAndLen proto2_rust_RepeatedField_##ty##_get(      \
       google::protobuf::RepeatedPtrField<std::string>* r, size_t index) {      \
     const std::string& s = r->Get(index);                            \
-    return google::protobuf::rust::PtrAndLen(s.data(), s.size());              \
+    return google::protobuf::rust::PtrAndLen{s.data(), s.size()};              \
   }                                                                  \
   void proto2_rust_RepeatedField_##ty##_set(                         \
       google::protobuf::RepeatedPtrField<std::string>* r, size_t index,        \

diff --git a/rust/cpp_kernel/strings.cc b/rust/cpp_kernel/strings.cc
@@ -33,6 +33,6 @@ std::string* proto2_rust_cpp_new_string(google::protobuf::rust::PtrAndLen src) {
 void proto2_rust_cpp_delete_string(std::string* str) { delete str; }
 
 google::protobuf::rust::PtrAndLen proto2_rust_cpp_string_to_view(std::string* str) {
-  return google::protobuf::rust::PtrAndLen(str->data(), str->length());
+  return google::protobuf::rust::PtrAndLen{str->data(), str->length()};
 }
 }
diff --git a/rust/cpp_kernel/strings.h b/rust/cpp_kernel/strings.h
@@ -22,8 +22,6 @@ struct PtrAndLen {
   /// Borrows the memory.
   const char* ptr;
   size_t len;
-
-  PtrAndLen(const char* ptr, size_t len) : ptr(ptr), len(len) {}
 };
 
 // Represents an owned string for FFI purposes.

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

diff --git a/src/google/protobuf/descriptor_database_unittest.cc b/src/google/protobuf/descriptor_database_unittest.cc
@@ -446,14 +446,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));
 
 TEST(EncodedDescriptorDatabaseExtraTest, FindNameOfFileContainingSymbol) {
   // Create two files, one of which is in two parts.

diff --git a/src/google/protobuf/string_view_test.cc b/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"
diff --git a/third_party/zlib.BUILD b/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",