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

f8fe7bd53bfed601f002f521e34ab4bc083fc28b by Matthew Brown <[email protected]>: Ensure a deep copy and proper equality on absl::Status::ErasePayload PiperOrigin-RevId: 298482742
abseil · Mar 4, 2020 · df3ea78 · df3ea78
1 parent b832dce
commit df3ea78
Show file tree

Hide file tree

Showing 2 changed files with 65 additions and 0 deletions.
diff --git a/absl/status/status.cc b/absl/status/status.cc
@@ -147,7 +147,15 @@ void Status::SetPayload(absl::string_view type_url, absl::Cord payload) {
 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;
   }
 

diff --git a/absl/status/status_test.cc b/absl/status/status_test.cc
@@ -204,6 +204,25 @@ TEST(Status, TestComparePayloads) {
   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 @@ TEST(Status, ToString) {
                     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, CopyAssignment) {
   }
 }
 
+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;