From 2e82a2d7bf380eb3e83a20ab506e7546f96a4e01 Mon Sep 17 00:00:00 2001 From: Adam Cozzette Date: Fri, 11 Oct 2024 09:33:21 -0700 Subject: [PATCH] Reduce our reliance on COPTS variable for compiler flags It's somewhat tedious to explicitly set this option on all of our C++ targets, so I think ideally we should rely primarily on bazelrc files for setting compiler flags. I tried to completely remove `COPTS`, but unfortunately that did not work out--we have so many `-Wsign-compare` warnings that I think we need to keep suppressing them for now or else we will get a lot of complaints. However, I was able to get to the point where `-Wno-sign-compare` is the only flag we need in `COPTS` for non-Windows builds. I explicitly set `-DHAVE_ZLIB` on just the two targets that need it, and removed `-Wno-nonnull` since we are already compliant with that warning. I moved `-Woverloaded-virtual` to our bazelrc files so that CI will enforce that we remain compliant with that. PiperOrigin-RevId: 684863987 --- build_defs/cpp_opts.bzl | 3 --- ci/Linux.bazelrc | 1 + ci/macOS.bazelrc | 3 ++- src/google/protobuf/io/BUILD.bazel | 10 ++++++++-- 4 files changed, 11 insertions(+), 6 deletions(-) diff --git a/build_defs/cpp_opts.bzl b/build_defs/cpp_opts.bzl index d8646d1681..f46b7dcc0a 100644 --- a/build_defs/cpp_opts.bzl +++ b/build_defs/cpp_opts.bzl @@ -17,10 +17,7 @@ COPTS = select({ "/wd4996", # The compiler encountered a deprecated declaration. ], "//conditions:default": [ - "-DHAVE_ZLIB", - "-Woverloaded-virtual", "-Wno-sign-compare", - "-Wno-nonnull", ], }) diff --git a/ci/Linux.bazelrc b/ci/Linux.bazelrc index b4ec98f8c7..3cdf40d218 100644 --- a/ci/Linux.bazelrc +++ b/ci/Linux.bazelrc @@ -1,4 +1,5 @@ import common.bazelrc build --cxxopt=-std=c++14 --host_cxxopt=-std=c++14 +build --cxxopt="-Woverloaded-virtual" 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 index e426b59f57..e91f62639a 100644 --- a/ci/macOS.bazelrc +++ b/ci/macOS.bazelrc @@ -1,6 +1,7 @@ import common.bazelrc build --cxxopt=-std=c++14 --host_cxxopt=-std=c++14 +build --cxxopt="-Woverloaded-virtual" 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 -common --xcode_version_config=@com_google_protobuf//.github:host_xcodes \ No newline at end of file +common --xcode_version_config=@com_google_protobuf//.github:host_xcodes diff --git a/src/google/protobuf/io/BUILD.bazel b/src/google/protobuf/io/BUILD.bazel index 27c4444801..eb865df566 100644 --- a/src/google/protobuf/io/BUILD.bazel +++ b/src/google/protobuf/io/BUILD.bazel @@ -150,7 +150,10 @@ cc_library( name = "gzip_stream", srcs = ["gzip_stream.cc"], hdrs = ["gzip_stream.h"], - copts = COPTS, + copts = COPTS + select({ + "//build_defs:config_msvc": [], + "//conditions:default": ["-DHAVE_ZLIB"], + }), strip_include_prefix = "/src", deps = [ ":io", @@ -192,7 +195,10 @@ cc_test( "tokenizer_unittest.cc", "zero_copy_stream_unittest.cc", ], - copts = COPTS, + copts = COPTS + select({ + "//build_defs:config_msvc": [], + "//conditions:default": ["-DHAVE_ZLIB"], + }), deps = [ ":gzip_stream", ":io",