[Mlir-commits] [mlir] [mlir][NFC] Apply rule of five to *Pass classes (PR #80998)

Andrei Golubev llvmlistbot at llvm.org
Fri Mar 1 00:51:53 PST 2024


andrey-golubev wrote:

> > Although, technically, @zero9178 is right that explicitly deleting member functions could actually break compilation
> 
> I thought these were implicitly deleted right now?

Well, move construction semantics would dispatch to copy-ctor afaict. Other than that,
* Defining one of {destructor, copy-ctor, copy-assign-operator} prevents implicit move semantics (so e.g. `Pass`  has implicitly deleted move ctor / assignment operator)
* `PassOptions`, being non-copyable, prevents copy assignment operator from being generated (`PassOptions` is also non-movable so kind of reinforces the previous bullet I guess)

(this is the state without modifications)

So, precisely speaking, `Pass x(std::move(y))` would work now (via user-defined copy-ctor) and wouldn't work with this patch. But: given that copy-ctors in these classes are under `protected` access, only the inheritance chains (+ friends?) benefit from this, so I wouldn't bother.
Bottomline: judging by the CI, nothing is using the `Pass x(std::move(y))` within MLIR/LLVM and changing this to `Pass x(y)` seems trivial enough for me (if any user code relies on this).

> > * which makes me question the "NFC" label
> 
> NFC is about the behavior of the compiler, changing APIs is just "business as usual" (any NFC refactoring we do is breaking C++ APIs).

Aha, I see. Well, technically there are several compilers but I get the point :)

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


More information about the Mlir-commits mailing list