Skip to content

Commit

Permalink
Reduce our reliance on COPTS variable for compiler flags
Browse files Browse the repository at this point in the history
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
  • Loading branch information
acozzette authored and copybara-github committed Oct 11, 2024
1 parent f64d63c commit 2e82a2d
Show file tree
Hide file tree
Showing 4 changed files with 11 additions and 6 deletions.
3 changes: 0 additions & 3 deletions build_defs/cpp_opts.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,7 @@ COPTS = select({
"/wd4996", # The compiler encountered a deprecated declaration.
],
"//conditions:default": [
"-DHAVE_ZLIB",
"-Woverloaded-virtual",
"-Wno-sign-compare",
"-Wno-nonnull",
],
})

Expand Down
1 change: 1 addition & 0 deletions ci/Linux.bazelrc
Original file line number Diff line number Diff line change
@@ -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"
3 changes: 2 additions & 1 deletion ci/macOS.bazelrc
Original file line number Diff line number Diff line change
@@ -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
common --xcode_version_config=@com_google_protobuf//.github:host_xcodes
10 changes: 8 additions & 2 deletions src/google/protobuf/io/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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",
Expand Down

0 comments on commit 2e82a2d

Please sign in to comment.