Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 6 commits into from
Aug 23, 2024
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .bazelrc
Original file line number Diff line number Diff line change
@@ -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" --copt="-Wno-error=deprecated-non-prototype"


build:dbg --compilation_mode=dbg

build:opt --compilation_mode=opt
Expand Down
3 changes: 2 additions & 1 deletion .github/workflows/test_rust.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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/...
5 changes: 4 additions & 1 deletion .github/workflows/test_upb.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
1 change: 0 additions & 1 deletion build_defs/cpp_opts.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ COPTS = select({
"-Woverloaded-virtual",
"-Wno-sign-compare",
"-Wno-nonnull",
"-Werror",
],
})

Expand Down
1 change: 1 addition & 0 deletions ci/Linux.bazelrc
Original file line number Diff line number Diff line change
@@ -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"
1 change: 1 addition & 0 deletions ci/macOS.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 --copt="-Werror" --copt="-Wno-sign-compare" --copt="-Wno-sign-conversion" --copt="-Wno-error=sign-conversion" --copt="-Wno-deprecated-declarations" --copt="-Wno-error=deprecated-non-prototype"
common --repo_env=BAZEL_NO_APPLE_CPP_TOOLCHAIN=1
7 changes: 3 additions & 4 deletions conformance/binary_json_conformance_suite.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down
10 changes: 0 additions & 10 deletions python/map.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
1 change: 0 additions & 1 deletion python/message.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion python/protobuf.c
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
3 changes: 2 additions & 1 deletion ruby/ext/google/protobuf_c/convert.c
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
1 change: 1 addition & 0 deletions src/google/protobuf/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
8 changes: 4 additions & 4 deletions src/google/protobuf/descriptor_database_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
7 changes: 7 additions & 0 deletions src/google/protobuf/string_view_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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);
Expand All @@ -309,3 +314,5 @@ TEST(StringViewFieldTest, RepeatedSetAndGetByReflection) {
} // namespace
} // namespace protobuf
} // namespace google

#include "google/protobuf/port_undef.inc"
4 changes: 4 additions & 0 deletions third_party/zlib.BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
35 changes: 0 additions & 35 deletions upb/io/zero_copy_stream_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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;
};
Expand Down Expand Up @@ -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) {
Expand Down
2 changes: 1 addition & 1 deletion upb/wire/eps_copy_input_stream_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down
Loading