From a76698790753d2ec71f655cdc84d61bcb27780d4 Mon Sep 17 00:00:00 2001 From: Abseil Team Date: Mon, 1 Mar 2021 06:24:39 -0800 Subject: [PATCH] Export of internal Abseil changes -- a9eb3c976c6d8ef4fca3d416847f8fca4bd90dd7 by Derek Mauro : Remove the deprecated container library, which doesn't do anything. This will help prevent user confusion, as seen in #183. PiperOrigin-RevId: 360172262 -- 4f872f651e25a528bdc59ee4e24543fbbd358f00 by Abseil Team : Remove unused nspace alias. PiperOrigin-RevId: 359487559 -- 43e877e464886cf9226012f5bb47910b8995e70f by Abseil Team : Create a StatusToStringMode to control how the ToString behaves. PiperOrigin-RevId: 359339603 -- 0da1291569e167341613359846948c72c8a838e1 by Greg Falcon : Fix a bug in SimpleAtoi/SimpleAtof, which accepted a prefix of "+-" (e.g., "+-5" was parsed as 5.0). This regression was introduced when we migrated these functions to use absl::from_chars. PiperOrigin-RevId: 359135105 GitOrigin-RevId: a9eb3c976c6d8ef4fca3d416847f8fca4bd90dd7 Change-Id: I0e2072cad80651e473ba1d34b1fb3a033dfaba80 --- absl/container/CMakeLists.txt | 9 ----- absl/flags/reflection_test.cc | 2 -- absl/status/status.cc | 30 +++++++++------- absl/status/status.h | 68 ++++++++++++++++++++++++++++++----- absl/status/status_test.cc | 17 +++++++++ absl/strings/numbers.cc | 10 ++++++ absl/strings/numbers_test.cc | 22 ++++++++++++ 7 files changed, 126 insertions(+), 32 deletions(-) diff --git a/absl/container/CMakeLists.txt b/absl/container/CMakeLists.txt index eb202c45..2d7d0e65 100644 --- a/absl/container/CMakeLists.txt +++ b/absl/container/CMakeLists.txt @@ -14,15 +14,6 @@ # limitations under the License. # -# This is deprecated and will be removed in the future. It also doesn't do -# anything anyways. Prefer to use the library associated with the API you are -# using. -absl_cc_library( - NAME - container - PUBLIC -) - absl_cc_library( NAME btree diff --git a/absl/flags/reflection_test.cc b/absl/flags/reflection_test.cc index 4c809009..79cfa90c 100644 --- a/absl/flags/reflection_test.cc +++ b/absl/flags/reflection_test.cc @@ -34,8 +34,6 @@ ABSL_RETIRED_FLAG(bool, bool_retired_flag, false, "bool_retired_flag help"); namespace { -namespace flags = absl::flags_internal; - class ReflectionTest : public testing::Test { protected: void SetUp() override { flag_saver_ = absl::make_unique(); } diff --git a/absl/status/status.cc b/absl/status/status.cc index 7962bb9e..51a0d268 100644 --- a/absl/status/status.cc +++ b/absl/status/status.cc @@ -291,20 +291,26 @@ bool Status::EqualsSlow(const absl::Status& a, const absl::Status& b) { return true; } -std::string Status::ToStringSlow() const { +std::string Status::ToStringSlow(StatusToStringMode mode) const { std::string text; absl::StrAppend(&text, absl::StatusCodeToString(code()), ": ", message()); - status_internal::StatusPayloadPrinter printer = - status_internal::GetStatusPayloadPrinter(); - this->ForEachPayload([&](absl::string_view type_url, - const absl::Cord& payload) { - absl::optional result; - if (printer) result = printer(type_url, payload); - absl::StrAppend( - &text, " [", type_url, "='", - result.has_value() ? *result : absl::CHexEscape(std::string(payload)), - "']"); - }); + + const bool with_payload = (mode & StatusToStringMode::kWithPayload) == + StatusToStringMode::kWithPayload; + + if (with_payload) { + status_internal::StatusPayloadPrinter printer = + status_internal::GetStatusPayloadPrinter(); + this->ForEachPayload([&](absl::string_view type_url, + const absl::Cord& payload) { + absl::optional result; + if (printer) result = printer(type_url, payload); + absl::StrAppend( + &text, " [", type_url, "='", + result.has_value() ? *result : absl::CHexEscape(std::string(payload)), + "']"); + }); + } return text; } diff --git a/absl/status/status.h b/absl/status/status.h index 118f64fb..df9e330c 100644 --- a/absl/status/status.h +++ b/absl/status/status.h @@ -280,6 +280,55 @@ std::string StatusCodeToString(StatusCode code); // Streams StatusCodeToString(code) to `os`. std::ostream& operator<<(std::ostream& os, StatusCode code); +// absl::StatusToStringMode +// +// An `absl::StatusToStringMode` is an enumerated type indicating how +// `absl::Status::ToString()` should construct the output string for an non-ok +// status. +enum class StatusToStringMode : int { + // ToString will not contain any extra data (such as payloads). It will only + // contain the error code and message, if any. + kWithNoExtraData = 0, + // ToString will contain the payloads. + kWithPayload = 1 << 0, +}; + +// absl::StatusToStringMode is specified as a bitmask type, which means the +// following operations must be provided: +inline constexpr StatusToStringMode operator&(StatusToStringMode lhs, + StatusToStringMode rhs) { + return static_cast(static_cast(lhs) & + static_cast(rhs)); +} +inline constexpr StatusToStringMode operator|(StatusToStringMode lhs, + StatusToStringMode rhs) { + return static_cast(static_cast(lhs) | + static_cast(rhs)); +} +inline constexpr StatusToStringMode operator^(StatusToStringMode lhs, + StatusToStringMode rhs) { + return static_cast(static_cast(lhs) ^ + static_cast(rhs)); +} +inline constexpr StatusToStringMode operator~(StatusToStringMode arg) { + return static_cast(~static_cast(arg)); +} +inline StatusToStringMode& operator&=(StatusToStringMode& lhs, + StatusToStringMode rhs) { + lhs = lhs & rhs; + return lhs; +} +inline StatusToStringMode& operator|=(StatusToStringMode& lhs, + StatusToStringMode rhs) { + lhs = lhs | rhs; + return lhs; +} +inline StatusToStringMode& operator^=(StatusToStringMode& lhs, + StatusToStringMode rhs) { + lhs = lhs ^ rhs; + return lhs; +} + // absl::Status // // The `absl::Status` class is generally used to gracefully handle errors @@ -443,15 +492,17 @@ class ABSL_MUST_USE_RESULT Status final { // Status::ToString() // - // Returns a combination of the error code name, the message and any - // associated payload messages. This string is designed simply to be human - // readable and its exact format should not be load bearing. Do not depend on - // the exact format of the result of `ToString()` which is subject to change. + // Returns a string based on the `mode`. By default, it returns combination of + // the error code name, the message and any associated payload messages. This + // string is designed simply to be human readable and its exact format should + // not be load bearing. Do not depend on the exact format of the result of + // `ToString()` which is subject to change. // // The printed code name and the message are generally substrings of the // result, and the payloads to be printed use the status payload printer // mechanism (which is internal). - std::string ToString() const; + std::string ToString( + StatusToStringMode mode = StatusToStringMode::kWithPayload) const; // Status::IgnoreError() // @@ -582,8 +633,7 @@ class ABSL_MUST_USE_RESULT Status final { static uintptr_t PointerToRep(status_internal::StatusRep* r); static status_internal::StatusRep* RepToPointer(uintptr_t r); - // Returns string for non-ok Status. - std::string ToStringSlow() const; + std::string ToStringSlow(StatusToStringMode mode) const; // Status supports two different representations. // - When the low bit is off it is an inlined representation. @@ -747,8 +797,8 @@ inline bool operator!=(const Status& lhs, const Status& rhs) { return !(lhs == rhs); } -inline std::string Status::ToString() const { - return ok() ? "OK" : ToStringSlow(); +inline std::string Status::ToString(StatusToStringMode mode) const { + return ok() ? "OK" : ToStringSlow(mode); } inline void Status::IgnoreError() const { diff --git a/absl/status/status_test.cc b/absl/status/status_test.cc index 24eaed61..7116ba67 100644 --- a/absl/status/status_test.cc +++ b/absl/status/status_test.cc @@ -280,6 +280,23 @@ TEST(Status, ToString) { HasSubstr("[bar='\\xff']"))); } +TEST(Status, ToStringMode) { + absl::Status s(absl::StatusCode::kInternal, "fail"); + s.SetPayload("foo", absl::Cord("bar")); + s.SetPayload("bar", absl::Cord("\377")); + + EXPECT_EQ("INTERNAL: fail", + s.ToString(absl::StatusToStringMode::kWithNoExtraData)); + + EXPECT_THAT(s.ToString(absl::StatusToStringMode::kWithPayload), + AllOf(HasSubstr("INTERNAL: fail"), HasSubstr("[foo='bar']"), + HasSubstr("[bar='\\xff']"))); + + EXPECT_THAT(s.ToString(~absl::StatusToStringMode::kWithPayload), + AllOf(HasSubstr("INTERNAL: fail"), Not(HasSubstr("[foo='bar']")), + Not(HasSubstr("[bar='\\xff']")))); +} + absl::Status EraseAndReturn(const absl::Status& base) { absl::Status copy = base; EXPECT_TRUE(copy.ErasePayload(kUrl1)); diff --git a/absl/strings/numbers.cc b/absl/strings/numbers.cc index e6bf44ce..966d94bd 100644 --- a/absl/strings/numbers.cc +++ b/absl/strings/numbers.cc @@ -46,8 +46,13 @@ ABSL_NAMESPACE_BEGIN bool SimpleAtof(absl::string_view str, float* out) { *out = 0.0; str = StripAsciiWhitespace(str); + // std::from_chars doesn't accept an initial +, but SimpleAtof does, so if one + // is present, skip it, while avoiding accepting "+-0" as valid. if (!str.empty() && str[0] == '+') { str.remove_prefix(1); + if (!str.empty() && str[0] == '-') { + return false; + } } auto result = absl::from_chars(str.data(), str.data() + str.size(), *out); if (result.ec == std::errc::invalid_argument) { @@ -72,8 +77,13 @@ bool SimpleAtof(absl::string_view str, float* out) { bool SimpleAtod(absl::string_view str, double* out) { *out = 0.0; str = StripAsciiWhitespace(str); + // std::from_chars doesn't accept an initial +, but SimpleAtod does, so if one + // is present, skip it, while avoiding accepting "+-0" as valid. if (!str.empty() && str[0] == '+') { str.remove_prefix(1); + if (!str.empty() && str[0] == '-') { + return false; + } } auto result = absl::from_chars(str.data(), str.data() + str.size(), *out); if (result.ec == std::errc::invalid_argument) { diff --git a/absl/strings/numbers_test.cc b/absl/strings/numbers_test.cc index 27616bf8..f3103106 100644 --- a/absl/strings/numbers_test.cc +++ b/absl/strings/numbers_test.cc @@ -392,6 +392,28 @@ TEST(NumbersTest, Atod) { EXPECT_TRUE(std::isnan(d)); } +TEST(NumbersTest, Prefixes) { + double d; + EXPECT_FALSE(absl::SimpleAtod("++1", &d)); + EXPECT_FALSE(absl::SimpleAtod("+-1", &d)); + EXPECT_FALSE(absl::SimpleAtod("-+1", &d)); + EXPECT_FALSE(absl::SimpleAtod("--1", &d)); + EXPECT_TRUE(absl::SimpleAtod("-1", &d)); + EXPECT_EQ(d, -1.); + EXPECT_TRUE(absl::SimpleAtod("+1", &d)); + EXPECT_EQ(d, +1.); + + float f; + EXPECT_FALSE(absl::SimpleAtof("++1", &f)); + EXPECT_FALSE(absl::SimpleAtof("+-1", &f)); + EXPECT_FALSE(absl::SimpleAtof("-+1", &f)); + EXPECT_FALSE(absl::SimpleAtof("--1", &f)); + EXPECT_TRUE(absl::SimpleAtof("-1", &f)); + EXPECT_EQ(f, -1.f); + EXPECT_TRUE(absl::SimpleAtof("+1", &f)); + EXPECT_EQ(f, +1.f); +} + TEST(NumbersTest, Atoenum) { enum E01 { E01_zero = 0,