[Mlir-commits] [mlir] [mlir] Apply rule of five for Pass / OperationPass (PR #80998)

[Markus Böck]

================
@@ -161,6 +161,13 @@ class Pass {
   explicit Pass(TypeID passID, std::optional<StringRef> opName = std::nullopt)
       : passID(passID), opName(opName) {}
   Pass(const Pass &other) : Pass(other.passID, other.opName) {}
+  Pass &operator=(const Pass &other) {
+    this->passID = other.passID;
+    this->opName = other.opName;
+    return *this;
+  }
+  Pass(Pass &&) = delete;
----------------
zero9178 wrote:

> yes, but I'm not sure whether the compiler would (should) see it: I guess by design the moved-from object can only be assigned-to or deleted. so even though we see that move == copy, it doesn't mean that the compiler has to adhere to this? (all it knows about is "there's a move construction / move assignment operation happening") - maybe inlined code is special though.

C++ compilers do not assign any specific semantics to move constructors. The "valid but undefined state" post-condition is implemented by user-code, not by the compiler. From the POV of the compiler move constructors and assignment operators don't inherently modify the argument. They are just different overloads selected based on the type of the argument. `std::move` casts an lvalue-ref to an rvalue-ref to make this overload selection choose the move constructor. What the move constructor does is 100% up-to-user code (or the auto-generated one).
> (also, i'm pretty sure some static analyzer would come around and say what we do is bad, lol.)

Given it still adheres to the common post-condition of  "valid but undefined state" that static-analyzers implement it should be fine. It's probably also not productive to assume the worst-case without evidence.

> I guess this would still work through the copy-ctor? Pass pseudoMove(std::move(other)); // calls copy-ctor when move-ctor is deleted?

That is the goal. See e.g. https://godbolt.org/z/6MKMjEYhj In the current state the move constructor is deleted. A deleted function still participates in overload-resolution but causes an error when selected. Explicitly delegating the move constructor to the copy constructor solves that. 

https://github.com/llvm/llvm-project/pull/80998