darion-yaphet commented on code in PR #3685:
URL: https://github.com/apache/brpc/pull/3685#discussion_r4451226941


##########
src/butil/scoped_generic.h:
##########
@@ -84,11 +82,15 @@ class ScopedGeneric {
       : data_(value, traits) {
   }
 
-  // Move constructor for C++03 move emulation.
-  ScopedGeneric(RValue rvalue)
-      : data_(rvalue.object->release(), rvalue.object->get_traits()) {
+  ScopedGeneric(const ScopedGeneric&) = delete;
+  ScopedGeneric& operator=(const ScopedGeneric&) = delete;
+
+  ScopedGeneric(ScopedGeneric&& other)
+      : data_(other.release(), other.get_traits()) {

Review Comment:
   In the current implementation, release() only updates data_.generic, while 
get_traits() returns a reference to the separate Traits base subobject. Neither 
operation depends on the other’s evaluation order, and the traits copy occurs 
inside Data’s constructor after both arguments have been evaluated.
   
   The previous implementation used the same expression, so this refactor 
preserves the existing behavior. I’d prefer to keep it as-is unless there’s a 
concrete case where the order affects correctness.
   



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to