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. #17938

Merged
merged 5 commits into from
Aug 23, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
2 changes: 2 additions & 0 deletions .bazelrc
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
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"

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/... //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/... //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"
1 change: 1 addition & 0 deletions ci/macOS.bazelrc
Original file line number Diff line number Diff line change
@@ -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="-Wno-sign-conversion" --copt="-Wno-error=sign-conversion"
common --repo_env=BAZEL_NO_APPLE_CPP_TOOLCHAIN=1
common --xcode_version_config=@com_google_protobuf//.github:host_xcodes
7 changes: 3 additions & 4 deletions conformance/binary_json_conformance_suite.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3470,12 +3470,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
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 @@ -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:
Expand Down
4 changes: 2 additions & 2 deletions rust/cpp_kernel/map.cc
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,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"
2 changes: 1 addition & 1 deletion rust/cpp_kernel/map.h
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@
__PB_RUST_EXPOSE_SCALAR_MAP_METHODS( \
std::string, ProtoString, google::protobuf::rust::PtrAndLen, \
std::string(key.ptr, key.len), \
google::protobuf::rust::PtrAndLen(cpp_key.data(), cpp_key.size()), value_ty, \
(google::protobuf::rust::PtrAndLen{cpp_key.data(), cpp_key.size()}), value_ty, \
rust_value_ty, ffi_view_ty, ffi_value_ty, to_cpp_value, to_ffi_value);

#endif // GOOGLE_PROTOBUF_RUST_CPP_KERNEL_MAP_H__
2 changes: 1 addition & 1 deletion rust/cpp_kernel/repeated.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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, \
Expand Down
2 changes: 1 addition & 1 deletion rust/cpp_kernel/strings.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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()};
}
}
2 changes: 0 additions & 2 deletions rust/cpp_kernel/strings.h
Original file line number Diff line number Diff line change
Expand Up @@ -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.
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 @@ -1047,6 +1047,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 @@ -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.
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