[clang-tidy] No. 53,54 enable cppcoreguidelines-c-copy-assignment-signature and bugprone-use-after-move (#56601)

xiaoyewww · web-flow · commit cc9e869960a6 · 2023-08-29T10:41:00.000+08:00
diff --git a/.clang-tidy b/.clang-tidy
@@ -43,7 +43,7 @@ bugprone-misplaced-widening-cast,
 bugprone-unhandled-self-assignment,
 bugprone-unused-raii,
 -bugprone-unused-return-value,
--bugprone-use-after-move,
+bugprone-use-after-move,
 -bugprone-virtual-near-miss,
 -clang-analyzer-apiModeling.StdCLibraryFunctions,
 -clang-analyzer-apiModeling.TrustNonnull,
@@ -153,7 +153,7 @@ clang-analyzer-unix.Vfork,
 -clang-analyzer-valist.ValistBase,
 cppcoreguidelines-avoid-c-arrays,
 -cppcoreguidelines-avoid-goto,
--cppcoreguidelines-c-copy-assignment-signature,
+cppcoreguidelines-c-copy-assignment-signature,
 -cppcoreguidelines-explicit-virtual-functions,
 -cppcoreguidelines-init-variables,
 -cppcoreguidelines-narrowing-conversions,
diff --git a/paddle/fluid/distributed/rpc/rpc_agent.cc b/paddle/fluid/distributed/rpc/rpc_agent.cc
@@ -43,7 +43,7 @@ RpcAgent::RpcAgent(std::string name, std::vector<WorkerInfo> infos) {
   PADDLE_ENFORCE_EQ(
       server_.AddService(rpc_service_.get(), brpc::SERVER_DOESNT_OWN_SERVICE),
       0,
-      platform::errors::Fatal("Fail to add service: %s", name));
+      platform::errors::Fatal("Fail to add service: %s", name_));
 }
 
 int RpcAgent::StartWorker() {
diff --git a/paddle/fluid/framework/new_executor/program_interpreter.cc b/paddle/fluid/framework/new_executor/program_interpreter.cc
@@ -614,7 +614,6 @@ void ProgramInterpreter::Convert(
   for (size_t op_idx = 0; op_idx < op_nums; ++op_idx) {
     auto& op_func_node = nodes[op_idx];
     auto* dev_ctx_ = stream_analyzer_.ParseDeviceContext(op_func_node);
-    vec_instruction_.emplace_back(op_idx, std::move(op_func_node), *dev_ctx_);
 #ifdef PADDLE_WITH_CUDA
     if (FLAGS_new_executor_use_cuda_graph) {
       auto& op = op_func_node.operator_base_;
@@ -637,6 +636,7 @@ void ProgramInterpreter::Convert(
           .RecordCapturingDeviceContext(dev_ctx_);
     }
 #endif
+    vec_instruction_.emplace_back(op_idx, std::move(op_func_node), *dev_ctx_);
   }
 
   BuildOperatorDependences();
diff --git a/paddle/fluid/memory/allocation/stream_safe_custom_device_allocator.cc b/paddle/fluid/memory/allocation/stream_safe_custom_device_allocator.cc
@@ -121,7 +121,7 @@ StreamSafeCustomDeviceAllocator::StreamSafeCustomDeviceAllocator(
       place_(std::move(place)),
       default_stream_(std::move(default_stream)) {
   std::lock_guard<SpinLock> lock_guard(allocator_map_lock_);
-  allocator_map_[place].emplace_back(this);
+  allocator_map_[place_].emplace_back(this);
 }
 
 StreamSafeCustomDeviceAllocator::~StreamSafeCustomDeviceAllocator() {
diff --git a/paddle/fluid/operators/math/beam_search.cc b/paddle/fluid/operators/math/beam_search.cc
@@ -130,10 +130,14 @@ class BeamSearchFunctor<phi::CPUContext, T> {
              ((score == in.score) && (offset < in.offset));
     }
 
-    inline void operator=(const Item &in) {
-      offset = in.offset;
-      id = in.id;
-      score = in.score;
+    inline Item &operator=(const Item &in) {
+      if (this != &in) {
+        this->offset = in.offset;
+        this->id = in.id;
+        this->score = in.score;
+        return *this;
+      }
+      return *this;
     }
 
     std::string ToString() {
diff --git a/paddle/phi/core/sparse_coo_tensor.cc b/paddle/phi/core/sparse_coo_tensor.cc
@@ -57,11 +57,15 @@ SparseCooTensor::SparseCooTensor(const SparseCooTensor& other) {
   set_meta(other.meta());
 }
 
-SparseCooTensor SparseCooTensor::operator=(const SparseCooTensor& other) {
-  this->non_zero_elements_ = other.non_zero_elements_;
-  this->non_zero_indices_ = other.non_zero_indices_;
-  this->coalesced_ = other.coalesced_;
-  set_meta(other.meta());
+SparseCooTensor& SparseCooTensor::operator=(const SparseCooTensor& other) {
+  if (this != &other) {
+    this->non_zero_elements_ = other.non_zero_elements_;
+    this->non_zero_indices_ = other.non_zero_indices_;
+    this->coalesced_ = other.coalesced_;
+    set_meta(other.meta());
+    return *this;
+  }
+
   return *this;
 }
 
diff --git a/paddle/phi/core/sparse_coo_tensor.h b/paddle/phi/core/sparse_coo_tensor.h
@@ -58,7 +58,7 @@ class SparseCooTensor : public TensorBase,
   SparseCooTensor(SparseCooTensor&& other);
 
   /// \brief SparseCooTensor shallow copy assignment.
-  SparseCooTensor operator=(const SparseCooTensor& other);
+  SparseCooTensor& operator=(const SparseCooTensor& other);
 
   /// \brief Destroy the tensor object and release exclusive resources.
   virtual ~SparseCooTensor() = default;
diff --git a/paddle/phi/kernels/cpu/weighted_sample_neighbors_kernel.cc b/paddle/phi/kernels/cpu/weighted_sample_neighbors_kernel.cc
@@ -35,10 +35,16 @@ struct GraphWeightedNode {
   }
   GraphWeightedNode(T node_id, float weight_key, T eid = 0)
       : node_id(node_id), weight_key(weight_key), eid(eid) {}
-  void operator=(const GraphWeightedNode<T>& other) {
-    node_id = other.node_id;
-    weight_key = other.weight_key;
-    eid = other.eid;
+
+  GraphWeightedNode& operator=(const GraphWeightedNode<T>& other) {
+    if (this != &other) {
+      this->node_id = other.node_id;
+      this->weight_key = other.weight_key;
+      this->eid = other.eid;
+      return *this;
+    }
+
+    return *this;
   }
   friend bool operator>(const GraphWeightedNode<T>& n1,
                         const GraphWeightedNode<T>& n2) {
diff --git a/test/cpp/fluid/reader/reader_blocking_queue_test.cc b/test/cpp/fluid/reader/reader_blocking_queue_test.cc
@@ -204,15 +204,21 @@ struct MyClass {
   explicit MyClass(int val) : val_(val) {}
   MyClass(const MyClass& b) { val_ = b.val_; }
   MyClass(MyClass&& b) { val_ = b.val_; }
-  void operator=(const MyClass& b) { val_ = b.val_; }
+  MyClass& operator=(const MyClass& b) {
+    if (this != &b) {
+      val_ = b.val_;
+      return *this;
+    }
+    return *this;
+  }
 
   int val_;
 };
 
 TEST(BlockingQueue, MyClassTest) {
   BlockingQueue<MyClass> q(2);
   MyClass a(200);
-  q.Send(std::move(a));
+  q.Send(a);
   MyClass b;
   q.Receive(&b);
   EXPECT_EQ(a.val_, b.val_);
diff --git a/test/cpp/phi/core/test_string_tensor.cc b/test/cpp/phi/core/test_string_tensor.cc
@@ -94,7 +94,7 @@ TEST(pstring, func) {
   CHECK_EQ(copy_nchar_str, "AAAAA");
 
   // Test Move Ctor
-  pstring move_nchar_str(std::move(nchar_str));
+  pstring move_nchar_str(nchar_str);
   CHECK_EQ(move_nchar_str, "AAAAA");
   pstring std_str(std::string("BBBB"));
   CHECK_EQ(std_str, "BBBB");

Original file line number	Diff line number	Diff line change
`@@ -43,7 +43,7 @@ RpcAgent::RpcAgent(std::string name, std::vector<WorkerInfo> infos) {`
`43`	`43`	`PADDLE_ENFORCE_EQ(`
`44`	`44`	`server_.AddService(rpc_service_.get(), brpc::SERVER_DOESNT_OWN_SERVICE),`
`45`	`45`	`0,`
`46`		`- platform::errors::Fatal("Fail to add service: %s", name));`
	`46`	`+ platform::errors::Fatal("Fail to add service: %s", name_));`
`47`	`47`	`}`
`48`	`48`
`49`	`49`	`int RpcAgent::StartWorker() {`
Original file line number	Diff line number	Diff line change
`@@ -614,7 +614,6 @@ void ProgramInterpreter::Convert(`
`614`	`614`	`for (size_t op_idx = 0; op_idx < op_nums; ++op_idx) {`
`615`	`615`	`auto& op_func_node = nodes[op_idx];`
`616`	`616`	`auto* dev_ctx_ = stream_analyzer_.ParseDeviceContext(op_func_node);`
`617`		`- vec_instruction_.emplace_back(op_idx, std::move(op_func_node), *dev_ctx_);`
`618`	`617`	`#ifdef PADDLE_WITH_CUDA`
`619`	`618`	`if (FLAGS_new_executor_use_cuda_graph) {`
`620`	`619`	`auto& op = op_func_node.operator_base_;`
`@@ -637,6 +636,7 @@ void ProgramInterpreter::Convert(`
`637`	`636`	`.RecordCapturingDeviceContext(dev_ctx_);`
`638`	`637`	`}`
`639`	`638`	`#endif`
	`639`	`+ vec_instruction_.emplace_back(op_idx, std::move(op_func_node), *dev_ctx_);`
`640`	`640`	`}`
`641`	`641`
`642`	`642`	`BuildOperatorDependences();`
Original file line number	Diff line number	Diff line change
`@@ -121,7 +121,7 @@ StreamSafeCustomDeviceAllocator::StreamSafeCustomDeviceAllocator(`
`121`	`121`	`place_(std::move(place)),`
`122`	`122`	`default_stream_(std::move(default_stream)) {`
`123`	`123`	`std::lock_guard<SpinLock> lock_guard(allocator_map_lock_);`
`124`		`- allocator_map_[place].emplace_back(this);`
	`124`	`+ allocator_map_[place_].emplace_back(this);`
`125`	`125`	`}`
`126`	`126`
`127`	`127`	`StreamSafeCustomDeviceAllocator::~StreamSafeCustomDeviceAllocator() {`