Fix warnings in C++ tests and enable strict test warnings Fix remaining conversion and sign-compare warnings in C++ unit tests, add unit tests for ::emboss::support::FitsInSizeT, convert standalone cc_test targets (emboss_cpp_util_google_integration_test and emboss_test_util_test) to use emboss_cc_util_test and emboss_cc_test, and enforce strict compiler warnings across all C++ tests via EMBOSS_TEST_COPTS.
diff --git a/.bazelrc b/.bazelrc index 64c77fe..d68a427 100644 --- a/.bazelrc +++ b/.bazelrc
@@ -4,3 +4,4 @@ # # TODO(bolms): Find and port to a C++11-compatible unit test framework. build --copt=-std=c++17 +build --features=external_include_paths
diff --git a/compiler/back_end/cpp/build_defs.bzl b/compiler/back_end/cpp/build_defs.bzl index 0033b30..f30bc26 100644 --- a/compiler/back_end/cpp/build_defs.bzl +++ b/compiler/back_end/cpp/build_defs.bzl
@@ -18,34 +18,36 @@ load("@rules_cc//cc:cc_test.bzl", "cc_test") load("@rules_python//python:py_test.bzl", "py_test") +load("//runtime/cpp/test:build_defs.bzl", "EMBOSS_TEST_COPTS") -def emboss_cc_test(name, copts = None, no_w_sign_compare = False, **kwargs): +def emboss_cc_test(name, copts = None, **kwargs): """Generates cc_test rules with and without -DEMBOSS_NO_OPTIMIZATIONS. Args: name: The base name of the test target. copts: Additional compiler flags. - no_w_sign_compare: Unused, kept for compatibility. **kwargs: Additional arguments passed to cc_test. """ cc_test( name = name, - copts = copts or [], - features = ["-layering_check"], + copts = EMBOSS_TEST_COPTS + (copts or []), + features = ["external_include_paths", "-layering_check"], **kwargs ) cc_test( name = name + "_no_opts", copts = [ "-DEMBOSS_NO_OPTIMIZATIONS", - ] + ([] if no_w_sign_compare else ["-Wsign-compare"]) + (copts or []), - features = ["-layering_check"], + ] + EMBOSS_TEST_COPTS + (copts or []), + features = ["external_include_paths", "-layering_check"], **kwargs ) cc_test( name = name + "_no_checks", - copts = ["-DEMBOSS_SKIP_CHECKS"] + (copts or []), - features = ["-layering_check"], + copts = [ + "-DEMBOSS_SKIP_CHECKS", + ] + EMBOSS_TEST_COPTS + (copts or []), + features = ["external_include_paths", "-layering_check"], **kwargs ) cc_test( @@ -53,8 +55,8 @@ copts = [ "-DEMBOSS_NO_OPTIMIZATIONS", "-DEMBOSS_SKIP_CHECKS", - ] + ([] if no_w_sign_compare else ["-Wsign-compare"]) + (copts or []), - features = ["-layering_check"], + ] + EMBOSS_TEST_COPTS + (copts or []), + features = ["external_include_paths", "-layering_check"], **kwargs )
diff --git a/compiler/back_end/cpp/testcode/auto_array_size_test.cc b/compiler/back_end/cpp/testcode/auto_array_size_test.cc index e3456e2..a55d345 100644 --- a/compiler/back_end/cpp/testcode/auto_array_size_test.cc +++ b/compiler/back_end/cpp/testcode/auto_array_size_test.cc
@@ -111,7 +111,7 @@ TEST(AutoSizeView, ForEach) { auto view = MakeAutoSizeView(kAutoSize, sizeof kAutoSize).four_struct_array(); - int i = 0; + ::std::size_t i = 0; ::std::for_each(view.begin(), view.end(), [&](ElementView element) { ASSERT_TRUE(element.Equals(view[i++])); }); @@ -125,7 +125,7 @@ TEST(AutoSizeView, ForEachWithTemporaries) { auto view = MakeAutoSizeView(kAutoSize, sizeof kAutoSize); - int i = 0; + ::std::size_t i = 0; ::std::for_each(view.four_struct_array().begin(), view.four_struct_array().end(), [&](ElementView element) { ASSERT_TRUE(element.Equals(view.four_struct_array()[i++])); @@ -178,12 +178,13 @@ TEST(AutoSizeView, Comparison) { auto view = MakeAutoSizeView(kAutoSize, sizeof kAutoSize).four_struct_array(); + const auto count = static_cast</**/ ::std::ptrdiff_t>(view.ElementCount()); - EXPECT_EQ(view.begin() + view.ElementCount(), view.end()); - EXPECT_EQ(view.end() - view.ElementCount(), view.begin()); + EXPECT_EQ(view.begin() + count, view.end()); + EXPECT_EQ(view.end() - count, view.begin()); - EXPECT_EQ(view.rbegin() + view.ElementCount(), view.rend()); - EXPECT_EQ(view.rend() - view.ElementCount(), view.rbegin()); + EXPECT_EQ(view.rbegin() + count, view.rend()); + EXPECT_EQ(view.rend() - count, view.rbegin()); EXPECT_LT(view.begin(), view.end()); EXPECT_LT(view.rbegin(), view.rend()); @@ -205,7 +206,7 @@ TEST(AutoSizeView, RangeBasedFor) { auto view = MakeAutoSizeView(kAutoSize, sizeof kAutoSize).four_struct_array(); - int i = 0; + ::std::size_t i = 0; for (auto element : view) { ASSERT_TRUE(element.Equals(view[i++])); }
diff --git a/compiler/back_end/cpp/testcode/dynamic_size_test.cc b/compiler/back_end/cpp/testcode/dynamic_size_test.cc index a1d5ebf..dff2309 100644 --- a/compiler/back_end/cpp/testcode/dynamic_size_test.cc +++ b/compiler/back_end/cpp/testcode/dynamic_size_test.cc
@@ -107,7 +107,7 @@ writer.header_length().Write(2); writer.message_length().Write(6); EXPECT_EQ(6, writer.message_length().Read()); - for (int i = 0; i < writer.message_length().Read(); ++i) { + for (::std::size_t i = 0; i < writer.message_length().Read(); ++i) { writer.message()[i].Write(i + 1); } EXPECT_EQ(12U, writer.SizeInBytes()); @@ -133,7 +133,7 @@ EXPECT_EQ(5U, writer2.message()[0].Read()); // The padding bytes are already set to the correct values; do not update // them. - for (int i = 0; i < writer2.message_length().Read(); ++i) { + for (::std::size_t i = 0; i < writer2.message_length().Read(); ++i) { writer2.padding()[i].Write(i + 1); } writer2.crc32().Write(0x0c0b0a09); @@ -180,9 +180,9 @@ TEST(ImageView, PixelsAreCorrect) { auto view = ImageView(kThreeByFiveImage, sizeof kThreeByFiveImage); int counter = 1; - for (int x = 0; x < view.size().Read(); ++x) { - for (int y = 0; y < 5; ++y) { - for (int channel = 0; channel < 3; ++channel) { + for (::std::size_t x = 0; x < view.size().Read(); ++x) { + for (::std::size_t y = 0; y < 5; ++y) { + for (::std::size_t channel = 0; channel < 3; ++channel) { EXPECT_EQ(counter, view.pixels()[x][y][channel].Read()) << "x: " << x << "; y: " << y << "; channel: " << channel; ++counter; @@ -196,9 +196,9 @@ auto writer = ImageWriter(buffer, sizeof buffer); writer.size().Write(3); int counter = 1; - for (int x = 0; x < writer.size().Read(); ++x) { - for (int y = 0; y < 5; ++y) { - for (int channel = 0; channel < 3; ++channel) { + for (::std::size_t x = 0; x < writer.size().Read(); ++x) { + for (::std::size_t y = 0; y < 5; ++y) { + for (::std::size_t channel = 0; channel < 3; ++channel) { writer.pixels()[x][y][channel].Write(counter); ++counter; }
diff --git a/compiler/back_end/cpp/testcode/next_keyword_test.cc b/compiler/back_end/cpp/testcode/next_keyword_test.cc index 07b6571..2a584fe 100644 --- a/compiler/back_end/cpp/testcode/next_keyword_test.cc +++ b/compiler/back_end/cpp/testcode/next_keyword_test.cc
@@ -45,7 +45,7 @@ }; const auto view = MakeNextKeywordView(&values); ASSERT_TRUE(view.Ok()); - EXPECT_EQ(1, view.value32().Read()); + EXPECT_EQ(1U, view.value32().Read()); EXPECT_EQ(2, view.value16().Read()); EXPECT_EQ(3, view.value8().Read()); EXPECT_EQ(4, view.value8_offset().Read());
diff --git a/integration/googletest/BUILD b/integration/googletest/BUILD index cddebb9..9fee0d1 100644 --- a/integration/googletest/BUILD +++ b/integration/googletest/BUILD
@@ -13,7 +13,7 @@ # limitations under the License. load("@rules_cc//cc:cc_library.bzl", "cc_library") -load("@rules_cc//cc:cc_test.bzl", "cc_test") +load("//compiler/back_end/cpp:build_defs.bzl", "emboss_cc_test") package( default_applicable_licenses = ["//:license"], @@ -31,13 +31,11 @@ ], ) -cc_test( +emboss_cc_test( name = "emboss_test_util_test", srcs = [ "emboss_test_util_test.cc", ], - copts = ["-Wsign-compare"], - features = ["-layering_check"], deps = [ ":emboss_test_util", "//testdata:complex_structure_emboss",
diff --git a/runtime/cpp/test/BUILD b/runtime/cpp/test/BUILD index 51719b0..af10dfe 100644 --- a/runtime/cpp/test/BUILD +++ b/runtime/cpp/test/BUILD
@@ -14,7 +14,6 @@ # Emboss public definitions. -load("@rules_cc//cc:cc_test.bzl", "cc_test") load( ":build_defs.bzl", "emboss_cc_util_test", @@ -157,16 +156,14 @@ ], ) -cc_test( +emboss_cc_util_test( name = "emboss_cpp_util_google_integration_test", srcs = [ "emboss_cpp_util_google_integration_test.cc", ], copts = [ "-DEMBOSS_FORCE_ALL_CHECKS", - "-Wsign-compare", ], - features = ["-layering_check"], deps = [ "//runtime/cpp:cpp_utils", "@com_google_absl//absl/strings",
diff --git a/runtime/cpp/test/build_defs.bzl b/runtime/cpp/test/build_defs.bzl index 18fb484..65cec8d 100644 --- a/runtime/cpp/test/build_defs.bzl +++ b/runtime/cpp/test/build_defs.bzl
@@ -19,22 +19,39 @@ load("@rules_cc//cc:cc_test.bzl", "cc_test") -def emboss_cc_util_test(name, copts = [], **kwargs): +EMBOSS_TEST_COPTS = [ + "-Wall", + "-Werror=cast-qual", + "-Werror=conversion", + "-Werror=extra", + "-Werror=extra-semi", + "-Werror=implicit-fallthrough", + "-Werror=non-virtual-dtor", + "-Werror=pointer-arith", + "-Werror=redundant-decls", + "-Werror=shadow", + "-Werror=sign-compare", + "-Werror=sign-conversion", + "-Werror=switch-enum", + "-Werror=undef", +] + +def emboss_cc_util_test(name, copts = None, **kwargs): """Constructs two cc_test targets, with and without optimizations.""" cc_test( name = name, - copts = copts + ["-Wsign-compare"], - features = ["-layering_check"], + copts = EMBOSS_TEST_COPTS + (copts or []), + features = ["external_include_paths", "-layering_check"], **kwargs ) cc_test( name = name + "_no_opts", - copts = copts + [ + copts = [ # This is generally a dangerous flag for an individual target, but # these tests do not depend on any other .cc files that might # #include any Emboss headers. "-DEMBOSS_NO_OPTIMIZATIONS", - ], - features = ["-layering_check"], + ] + EMBOSS_TEST_COPTS + (copts or []), + features = ["external_include_paths", "-layering_check"], **kwargs )
diff --git a/runtime/cpp/test/emboss_array_view_test.cc b/runtime/cpp/test/emboss_array_view_test.cc index 1f632e8..9b0184e 100644 --- a/runtime/cpp/test/emboss_array_view_test.cc +++ b/runtime/cpp/test/emboss_array_view_test.cc
@@ -256,9 +256,9 @@ TEST(ArrayView, TextFormatOutput_MultilineComment) { ::std::uint8_t bytes[65]; for (::std::size_t i = 0; i < sizeof bytes; ++i) { - bytes[i] = '0' + (i % 10); + bytes[i] = static_cast</**/ ::std::uint8_t>('0' + (i % 10)); } - for (const ::std::size_t length : {63, 64, 65}) { + for (const ::std::size_t length : {63U, 64U, 65U}) { auto buffer = ReadWriteContiguousBuffer{bytes, length}; ::std::string expected_text = "{\n # "
diff --git a/runtime/cpp/test/emboss_bit_util_test.cc b/runtime/cpp/test/emboss_bit_util_test.cc index 52bff49..5d6b59e 100644 --- a/runtime/cpp/test/emboss_bit_util_test.cc +++ b/runtime/cpp/test/emboss_bit_util_test.cc
@@ -73,6 +73,25 @@ EXPECT_FALSE(IsPowerOfTwo(::std::int8_t{127})); } +TEST(FitsInSizeT, FitsInSizeT) { + EXPECT_TRUE(FitsInSizeT(::std::int32_t{0})); + EXPECT_TRUE(FitsInSizeT(::std::int32_t{1})); + EXPECT_TRUE(FitsInSizeT(::std::numeric_limits</**/ ::std::int32_t>::max())); + EXPECT_FALSE(FitsInSizeT(::std::int32_t{-1})); + EXPECT_FALSE(FitsInSizeT(::std::numeric_limits</**/ ::std::int32_t>::min())); + + EXPECT_TRUE(FitsInSizeT(::std::uint32_t{0})); + EXPECT_TRUE(FitsInSizeT(::std::numeric_limits</**/ ::std::uint32_t>::max())); + + EXPECT_TRUE(FitsInSizeT(::std::int64_t{0})); + EXPECT_FALSE(FitsInSizeT(::std::int64_t{-1})); + EXPECT_FALSE(FitsInSizeT(::std::numeric_limits</**/ ::std::int64_t>::min())); + + EXPECT_TRUE(FitsInSizeT(::std::uint64_t{0})); + EXPECT_TRUE(FitsInSizeT(static_cast</**/ ::std::uint64_t>( + ::std::numeric_limits</**/ ::std::size_t>::max()))); +} + #if defined(EMBOSS_LITTLE_ENDIAN_TO_NATIVE) TEST(EndianConversion, LittleEndianToNative) { ::std::uint16_t data16 = 0;
diff --git a/runtime/cpp/test/emboss_memory_util_test.cc b/runtime/cpp/test/emboss_memory_util_test.cc index 1831085..eec22a8 100644 --- a/runtime/cpp/test/emboss_memory_util_test.cc +++ b/runtime/cpp/test/emboss_memory_util_test.cc
@@ -90,7 +90,7 @@ bytes.data(), 0x7172737475767778UL & (~0x0UL >> (64 - kBits))); auto expected_vector_after_write = init_container<std::vector<CharT>>( 0x78, 0x77, 0x76, 0x75, 0x74, 0x73, 0x72, 0x71); - for (int i = kBits / 8; i < 8; ++i) { + for (::std::size_t i = kBits / 8; i < 8; ++i) { expected_vector_after_write[i] = CharT(i + 1); } EXPECT_EQ(expected_vector_after_write, @@ -102,7 +102,7 @@ bytes.data(), 0x7172737475767778UL >> (64 - kBits)); expected_vector_after_write = init_container<std::vector<CharT>>( 0x71, 0x72, 0x73, 0x74, 0x75, 0x76, 0x77, 0x78); - for (int i = kBits / 8; i < 8; ++i) { + for (::std::size_t i = kBits / 8; i < 8; ++i) { expected_vector_after_write[i] = CharT(i + 1); } EXPECT_EQ(expected_vector_after_write,
diff --git a/runtime/cpp/test/emboss_prelude_test.cc b/runtime/cpp/test/emboss_prelude_test.cc index 889daae..931f14b 100644 --- a/runtime/cpp/test/emboss_prelude_test.cc +++ b/runtime/cpp/test/emboss_prelude_test.cc
@@ -32,7 +32,7 @@ ::emboss::support::LittleEndianByteOrderer<ReadWriteContiguousBuffer>, kBits>; -template </**/ ::std::size_t kBits> +template <int kBits> using ViewParameters = ::emboss::support::FixedSizeViewParameters< kBits, ::emboss::support::AllValuesAreOk>; @@ -776,7 +776,7 @@ BcdViewN<8>{BitBlockN<8>{ReadWriteContiguousBuffer{&byte, 1}}}; for (int i = 0; i < 15; ++i) { for (int j = 0; j < 15; ++j) { - byte = i * 16 + j; + byte = static_cast</**/ ::std::uint8_t>(i * 16 + j); if (i > 9 || j > 9) { EXPECT_FALSE(bcd8_view.Ok()) << i << ", " << j; } else {
diff --git a/runtime/cpp/test/emboss_text_util_test.cc b/runtime/cpp/test/emboss_text_util_test.cc index 7541576..3c30d7e 100644 --- a/runtime/cpp/test/emboss_text_util_test.cc +++ b/runtime/cpp/test/emboss_text_util_test.cc
@@ -430,11 +430,12 @@ // Small helper function for the various WriteIntegerToTextStream tests; just // sets up a stream, forwards its arguments to WriteIntegerToTextStream, and // then returns the text from the stream. -template <typename Arg0, typename... Args> -::std::string WriteIntegerToString(Arg0 &&arg0, Args &&... args) { +template <typename Arg0> +::std::string WriteIntegerToString(Arg0 &&arg0, ::std::uint8_t base, + bool digit_grouping) { TextOutputStream stream; - WriteIntegerToTextStream(::std::forward<Arg0>(arg0), &stream, - ::std::forward<Args>(args)...); + WriteIntegerToTextStream(::std::forward<Arg0>(arg0), &stream, base, + digit_grouping); return stream.Result(); }