(kudu) branch master updated: [util] optimize Status move assignment operator

[al...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

asf-gitbox-commits pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/kudu.git


The following commit(s) were added to refs/heads/master by this push:
     new 3abfc3e27 [util] optimize Status move assignment operator
3abfc3e27 is described below

commit 3abfc3e270b48c5a2e69ad8b9aebc64e2a8e2644
Author: Alexey Serbin <al...@apache.org>
AuthorDate: Fri Sep 25 16:28:02 2026 -0700

    [util] optimize Status move assignment operator
    
    The C++ standard doesn't require a lot from source objects of
    std::move().  At least, for STL objects the requirements are like
    below [1]:
    
      Unless otherwise specified, all standard library objects that have been
      moved from are placed in a "valid but unspecified state", meaning the
      object's class invariants hold (so functions without preconditions,
      such as the assignment operator, can be safely used on the object
      after it was moved from).
    
    Applying this to the Status class, it's possible to postpone
    deallocating of the internal buffer to the time when the "moved from"
    Status object goes out of scope, not calling 'delete' in the move
    assignment operator itself.
    
    I also snuck in a fix for KUDU_ATTR_NODISCARD macro's comment.
    
    [1] https://cppreference.com/cpp/utility/move
    
    Change-Id: Id98a4e4970e3dfc61c22e46ed8fa69393f56aaae
    Reviewed-on: http://gerrit.cloudera.org:8080/24962
    Reviewed-by: Gabriella Lotz <lo...@gmail.com>
    Reviewed-by: Marton Greber <gr...@gmail.com>
    Tested-by: Marton Greber <gr...@gmail.com>
---
 src/kudu/util/status-test.cc | 4 ++--
 src/kudu/util/status.h       | 8 ++------
 2 files changed, 4 insertions(+), 8 deletions(-)

diff --git a/src/kudu/util/status-test.cc b/src/kudu/util/status-test.cc
index a0aef3d21..c93c45189 100644
--- a/src/kudu/util/status-test.cc
+++ b/src/kudu/util/status-test.cc
@@ -73,7 +73,7 @@ TEST(StatusTest, TestMoveAssignment) {
     Status src = Status::OK();
     Status dst = Status::NotFound("orig dst");
     dst = std::move(src);
-    ASSERT_OK(src); // NOLINT(bugprone-use-after-move)
+    ASSERT_TRUE(src.IsNotFound()); // NOLINT(bugprone-use-after-move)
     ASSERT_OK(dst);
   }
 
@@ -82,7 +82,7 @@ TEST(StatusTest, TestMoveAssignment) {
     Status src = Status::NotFound("orig src");
     Status dst = Status::NotFound("orig dst");
     dst = std::move(src);
-    ASSERT_OK(src); // NOLINT(bugprone-use-after-move)
+    ASSERT_EQ("Not found: orig dst", src.ToString()); // NOLINT(bugprone-use-after-move)
     ASSERT_EQ("Not found: orig src", dst.ToString());
   }
 
diff --git a/src/kudu/util/status.h b/src/kudu/util/status.h
index 107516bc5..b078a355f 100644
--- a/src/kudu/util/status.h
+++ b/src/kudu/util/status.h
@@ -134,7 +134,7 @@
   // types/styles, see [1] for details. So, enable this only if compiling with
   // CLANG or GCC version 13 and newer.
   //
-  // [1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69585 for details).
+  // [1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69585
   #if defined(__clang__) || (defined(__GNUC__) && __GNUC__ >= 13)
     #define KUDU_ATTR_NODISCARD [[nodiscard]] // NOLINT(whitespace/braces)
   #else
@@ -525,11 +525,7 @@ inline Status::Status(Status&& s) noexcept : state_(s.state_) {
 }
 
 inline Status& Status::operator=(Status&& s) noexcept {
-  if (state_ != s.state_) {
-    delete[] state_;
-    state_ = s.state_;
-    s.state_ = nullptr;
-  }
+  std::swap(state_, s.state_);
   return *this;
 }
 #endif