Guard against self-assignment; NFC

[AaronBallman]

This was caught by a static analysis tool and seemed like a reasonable code quality improvement.

[llvmbot]

@llvm/pr-subscribers-clang

Author: Aaron Ballman (AaronBallman)

Changes

This was caught by a static analysis tool and seemed like a reasonable code quality improvement.

---

Full diff: https://github.com/llvm/llvm-project/pull/145743.diff

1 Files Affected:

• (modified) clang/include/clang/AST/Type.h (+5-3)

diff --git a/clang/include/clang/AST/Type.h b/clang/include/clang/AST/Type.h
index 24f3ae78e857b..f4d39afded322 100644
--- a/clang/include/clang/AST/Type.h
+++ b/clang/include/clang/AST/Type.h
@@ -6410,9 +6410,11 @@ class SpirvOperand {
   ~SpirvOperand() {}
 
   SpirvOperand &operator=(const SpirvOperand &Other) {
-    this->Kind = Other.Kind;
-    this->ResultType = Other.ResultType;
-    this->Value = Other.Value;
+    if (this != &Other) {
+      this->Kind = Other.Kind;
+      this->ResultType = Other.ResultType;
+      this->Value = Other.Value;
+    }
     return *this;
   }
 

[erichkeane]

I mean... I guess? The copy below is unbelievably cheap except for maybe ap-int. I DO find myself wondering why this (and operator==) isnt just = default though. Could we try that instead?

[AaronBallman]

That would require C++20, wouldn't it?

[erichkeane]

For comparison yes. Sorry, my suggestion was for THIS, not for operator== (though TBH, I expected you to try both and tell my why it wouldn't work, and I hadn't recalled that we only added it for compare then :) ).

But operator= should work with just = default.

My problem with the guard is how inexpensive the copy is (though I see llvm::APInt doesn't actually support self-move-assign, but DOES have the this = &RHS for copy) compared to a branch.

So =default would 'get it right'.

[AaronBallman]

okay, that's worth a shot.

[cor3ntin]

approving this because it's a nice code cleaning, however defaulting operator= does not protect against self assignment afaik