From 94433ef0615616b1ccb8b2193bde777c10406ac0 Mon Sep 17 00:00:00 2001 From: Abseil Team Date: Mon, 17 Oct 2022 15:40:12 -0700 Subject: [PATCH] Fix various warnings for _WIN32. Bug: chromium:1292951 PiperOrigin-RevId: 481757795 Change-Id: I03c808222c6c4d3d7052576ab4b36141e5f1ebbc --- absl/log/internal/log_format.cc | 33 +++++++++++++++++++++------ absl/log/internal/test_helpers.cc | 2 +- absl/log/log_entry_test.cc | 10 ++++++-- absl/log/log_modifier_methods_test.cc | 6 +++-- absl/log/scoped_mock_log_test.cc | 2 +- 5 files changed, 40 insertions(+), 13 deletions(-) diff --git a/absl/log/internal/log_format.cc b/absl/log/internal/log_format.cc index 5d40d253..5b280a2d 100644 --- a/absl/log/internal/log_format.cc +++ b/absl/log/internal/log_format.cc @@ -46,6 +46,31 @@ ABSL_NAMESPACE_BEGIN namespace log_internal { namespace { +// This templated function avoids compiler warnings about tautological +// comparisons when log_internal::Tid is unsigned. It can be replaced with a +// constexpr if once the minimum C++ version Abseil suppports is C++17. +template +inline std::enable_if_t::value> +PutLeadingWhitespace(T tid, char*& p) { + if (tid < 10) *p++ = ' '; + if (tid < 100) *p++ = ' '; + if (tid < 1000) *p++ = ' '; + if (tid < 10000) *p++ = ' '; + if (tid < 100000) *p++ = ' '; + if (tid < 1000000) *p++ = ' '; +} + +template +inline std::enable_if_t::value> +PutLeadingWhitespace(T tid, char*& p) { + if (tid >= 0 && tid < 10) *p++ = ' '; + if (tid > -10 && tid < 100) *p++ = ' '; + if (tid > -100 && tid < 1000) *p++ = ' '; + if (tid > -1000 && tid < 10000) *p++ = ' '; + if (tid > -10000 && tid < 100000) *p++ = ' '; + if (tid > -100000 && tid < 1000000) *p++ = ' '; +} + // The fields before the filename are all fixed-width except for the thread ID, // which is of bounded width. size_t FormatBoundedFields(absl::LogSeverity severity, absl::Time timestamp, @@ -110,13 +135,7 @@ size_t FormatBoundedFields(absl::LogSeverity severity, absl::Time timestamp, absl::numbers_internal::PutTwoDigits(static_cast(usecs % 100), p); p += 2; *p++ = ' '; - constexpr bool unsigned_tid_t = !std::is_signed::value; - if ((unsigned_tid_t || tid >= 0) && tid < 10) *p++ = ' '; - if ((unsigned_tid_t || tid > -10) && tid < 100) *p++ = ' '; - if ((unsigned_tid_t || tid > -100) && tid < 1000) *p++ = ' '; - if ((unsigned_tid_t || tid > -1000) && tid < 10000) *p++ = ' '; - if ((unsigned_tid_t || tid > -10000) && tid < 100000) *p++ = ' '; - if ((unsigned_tid_t || tid > -100000) && tid < 1000000) *p++ = ' '; + PutLeadingWhitespace(tid, p); p = absl::numbers_internal::FastIntToBuffer(tid, p); *p++ = ' '; const size_t bytes_formatted = static_cast(p - buf.data()); diff --git a/absl/log/internal/test_helpers.cc b/absl/log/internal/test_helpers.cc index bff5cc17..0de5b96b 100644 --- a/absl/log/internal/test_helpers.cc +++ b/absl/log/internal/test_helpers.cc @@ -46,7 +46,7 @@ bool DiedOfFatal(int exit_status) { // Depending on NDEBUG and (configuration?) MSVC's abort either results // in error code 3 (SIGABRT) or error code 0x80000003 (breakpoint // triggered). - return ::testing::ExitedWithCode(3)(exit_status & ~0x80000000); + return ::testing::ExitedWithCode(3)(exit_status & 0x7fffffff); #elif defined(__Fuchsia__) // The Fuchsia death test implementation kill()'s the process when it detects // an exception, so it should exit with the corresponding code. See diff --git a/absl/log/log_entry_test.cc b/absl/log/log_entry_test.cc index 8d0afb3c..7238356e 100644 --- a/absl/log/log_entry_test.cc +++ b/absl/log/log_entry_test.cc @@ -208,10 +208,13 @@ TEST(LogEntryTest, EmptyFields) { } TEST(LogEntryTest, NegativeFields) { + // When Abseil's minimum C++ version is C++17, this conditional can be + // converted to a constexpr if and the static_cast below removed. if (std::is_signed::value) { LogEntryTestPeer entry("foo.cc", -1234, kUsePrefix, absl::LogSeverity::kInfo, "2020-01-02T03:04:05.6789", - -451, "hello world"); + static_cast(-451), + "hello world"); EXPECT_THAT(entry.FormatLogMessage(), Eq("I0102 03:04:05.678900 -451 foo.cc:-1234] hello world")); EXPECT_THAT(entry.FormatPrefixIntoSizedBuffer(1000), @@ -313,12 +316,15 @@ TEST(LogEntryTest, LongFields) { } TEST(LogEntryTest, LongNegativeFields) { + // When Abseil's minimum C++ version is C++17, this conditional can be + // converted to a constexpr if and the static_cast below removed. if (std::is_signed::value) { LogEntryTestPeer entry( "I am the very model of a modern Major-General / " "I've information vegetable, animal, and mineral.", -2147483647, kUsePrefix, absl::LogSeverity::kInfo, - "2020-01-02T03:04:05.678967896789", -2147483647, + "2020-01-02T03:04:05.678967896789", + static_cast(-2147483647), "I know the kings of England, and I quote the fights historical / " "From Marathon to Waterloo, in order categorical."); EXPECT_THAT( diff --git a/absl/log/log_modifier_methods_test.cc b/absl/log/log_modifier_methods_test.cc index a9bf38b7..42e13b1b 100644 --- a/absl/log/log_modifier_methods_test.cc +++ b/absl/log/log_modifier_methods_test.cc @@ -130,7 +130,8 @@ TEST(TailCallsModifiesTest, WithTimestamp) { TEST(TailCallsModifiesTest, WithThreadID) { absl::ScopedMockLog test_sink(absl::MockLogDefault::kDisallowUnexpected); - EXPECT_CALL(test_sink, Send(AllOf(ThreadID(Eq(1234))))); + EXPECT_CALL(test_sink, + Send(AllOf(ThreadID(Eq(absl::LogEntry::tid_t{1234}))))); test_sink.StartCapturingLogs(); LOG(INFO).WithThreadID(1234) << "hello world"; @@ -152,7 +153,8 @@ TEST(TailCallsModifiesTest, WithMetadataFrom) { Send(AllOf(SourceFilename(Eq("fake/file")), SourceBasename(Eq("file")), SourceLine(Eq(123)), Prefix(IsFalse()), LogSeverity(Eq(absl::LogSeverity::kWarning)), - Timestamp(Eq(absl::UnixEpoch())), ThreadID(Eq(456)), + Timestamp(Eq(absl::UnixEpoch())), + ThreadID(Eq(absl::LogEntry::tid_t{456})), TextMessage(Eq("forwarded: hello world")), Verbosity(Eq(7)), ENCODED_MESSAGE( EqualsProto(R"pb(value { literal: "forwarded: " } diff --git a/absl/log/scoped_mock_log_test.cc b/absl/log/scoped_mock_log_test.cc index 50689a0e..44b8d737 100644 --- a/absl/log/scoped_mock_log_test.cc +++ b/absl/log/scoped_mock_log_test.cc @@ -98,7 +98,7 @@ TEST(ScopedMockLogTest, LogMockCatchAndMatchSendExpectations) { log, Send(AllOf(SourceFilename(Eq("/my/very/very/very_long_source_file.cc")), SourceBasename(Eq("very_long_source_file.cc")), - SourceLine(Eq(777)), ThreadID(Eq(1234)), + SourceLine(Eq(777)), ThreadID(Eq(absl::LogEntry::tid_t{1234})), TextMessageWithPrefix(Truly([](absl::string_view msg) { return absl::EndsWith( msg, " very_long_source_file.cc:777] Info message");