commit df3ea785d8c30a9503321a3d35ee7d35808f190d
author Xiaoyi Zhang <zhangxy988@gmail.com> Wed Mar 04 15:57:37 2020 -0500
committer GitHub <noreply@github.com> Wed Mar 04 15:57:37 2020 -0500
tree 28050d1d97bb556d62c355ed08e7301c954fbab8
parent b832dce8489ef7b6231384909fd9b68d5a5ff2b7

Patch LTS with the bug fix for `absl::Status::ErasePayload`. (#632)

f8fe7bd53bfed601f002f521e34ab4bc083fc28b by Matthew Brown <matthewbr@google.com>:

Ensure a deep copy and proper equality on absl::Status::ErasePayload

PiperOrigin-RevId: 298482742

absl/status/status.cc

diff --git a/absl/status/status.cc b/absl/status/status.cc
index bbc1895..df3b740 100644
--- a/absl/status/status.cc
+++ b/absl/status/status.cc
@@ -147,7 +147,15 @@
 bool Status::ErasePayload(absl::string_view type_url) {
   int index = status_internal::FindPayloadIndexByUrl(GetPayloads(), type_url);
   if (index != -1) {
+    PrepareToModify();
     GetPayloads()->erase(GetPayloads()->begin() + index);
+    if (GetPayloads()->empty() && message().empty()) {
+      // Special case: If this can be represented inlined, it MUST be
+      // inlined (EqualsSlow depends on this behavior).
+      StatusCode c = static_cast<StatusCode>(raw_code());
+      Unref(rep_);
+      rep_ = CodeToInlinedRep(c);
+    }
     return true;
   }
 

absl/status/status_test.cc

diff --git a/absl/status/status_test.cc b/absl/status/status_test.cc
index 7cc65e4..ca9488a 100644
--- a/absl/status/status_test.cc
+++ b/absl/status/status_test.cc
@@ -204,6 +204,25 @@
   EXPECT_EQ(bad_status1, bad_status2);
 }
 
+TEST(Status, TestComparePayloadsAfterErase) {
+  absl::Status payload_status(absl::StatusCode::kInternal, "");
+  payload_status.SetPayload(kUrl1, absl::Cord(kPayload1));
+  payload_status.SetPayload(kUrl2, absl::Cord(kPayload2));
+
+  absl::Status empty_status(absl::StatusCode::kInternal, "");
+
+  // Different payloads, not equal
+  EXPECT_NE(payload_status, empty_status);
+  EXPECT_TRUE(payload_status.ErasePayload(kUrl1));
+
+  // Still Different payloads, still not equal.
+  EXPECT_NE(payload_status, empty_status);
+  EXPECT_TRUE(payload_status.ErasePayload(kUrl2));
+
+  // Both empty payloads, should be equal
+  EXPECT_EQ(payload_status, empty_status);
+}
+
 PayloadsVec AllVisitedPayloads(const absl::Status& s) {
   PayloadsVec result;
 
@@ -261,6 +280,36 @@
                     HasSubstr("[bar='\\xff']")));
 }
 
+absl::Status EraseAndReturn(const absl::Status& base) {
+  absl::Status copy = base;
+  EXPECT_TRUE(copy.ErasePayload(kUrl1));
+  return copy;
+}
+
+TEST(Status, CopyOnWriteForErasePayload) {
+  {
+    absl::Status base(absl::StatusCode::kInvalidArgument, "fail");
+    base.SetPayload(kUrl1, absl::Cord(kPayload1));
+    EXPECT_TRUE(base.GetPayload(kUrl1).has_value());
+    absl::Status copy = EraseAndReturn(base);
+    EXPECT_TRUE(base.GetPayload(kUrl1).has_value());
+    EXPECT_FALSE(copy.GetPayload(kUrl1).has_value());
+  }
+  {
+    absl::Status base(absl::StatusCode::kInvalidArgument, "fail");
+    base.SetPayload(kUrl1, absl::Cord(kPayload1));
+    absl::Status copy = base;
+
+    EXPECT_TRUE(base.GetPayload(kUrl1).has_value());
+    EXPECT_TRUE(copy.GetPayload(kUrl1).has_value());
+
+    EXPECT_TRUE(base.ErasePayload(kUrl1));
+
+    EXPECT_FALSE(base.GetPayload(kUrl1).has_value());
+    EXPECT_TRUE(copy.GetPayload(kUrl1).has_value());
+  }
+}
+
 TEST(Status, CopyConstructor) {
   {
     absl::Status status;
@@ -300,6 +349,14 @@
   }
 }
 
+TEST(Status, CopyAssignmentIsNotRef) {
+  const absl::Status status_orig(absl::StatusCode::kInvalidArgument, "message");
+  absl::Status status_copy = status_orig;
+  EXPECT_EQ(status_orig, status_copy);
+  status_copy.SetPayload(kUrl1, absl::Cord(kPayload1));
+  EXPECT_NE(status_orig, status_copy);
+}
+
 TEST(Status, MoveConstructor) {
   {
     absl::Status status;