From 3d2772a24a302cb1f18c1d66e107d7ac3a5f53c6 Mon Sep 17 00:00:00 2001 From: "Golubev, Andrey" Date: Wed, 7 Feb 2024 15:14:50 +0000 Subject: [PATCH] [mlir][NFC] Apply rule of five to *Pass classes Define all special member functions for mlir::Pass, mlir::OperationPass, mlir::PassWrapper and PassGen types since these classes explicitly specify copy-ctor. This, subsequently, should silence static analysis checkers that report rule-of-3 / rule-of-5 violations. Given the nature of the types, however, mark other special member functions deleted: the semantics of a Pass type object seems to be that it is only ever created by being wrapped in a smart pointer, so the special member functions are never to be used externally (except for the copy-ctor - it is "special" since it is a "delegating" ctor for derived pass types to use during cloning - see https://reviews.llvm.org/D104302 for details). Deleting other member functions means that `Pass x(std::move(y))` - that used to silently work (via copy-ctor) - would fail to compile now. Yet, as the copy ctors through the hierarchy are under 'protected' access, the issue is unlikely to appear in practice. Co-authored-by: Asya Pronina Co-authored-by: Harald Rotuna --- mlir/include/mlir/Pass/Pass.h | 19 +++++++++++++++++++ mlir/tools/mlir-tblgen/PassGen.cpp | 8 ++++++++ 2 files changed, 27 insertions(+) diff --git a/mlir/include/mlir/Pass/Pass.h b/mlir/include/mlir/Pass/Pass.h index 121b253eb83fe..070e0cad38787 100644 --- a/mlir/include/mlir/Pass/Pass.h +++ b/mlir/include/mlir/Pass/Pass.h @@ -161,6 +161,9 @@ class Pass { explicit Pass(TypeID passID, std::optional opName = std::nullopt) : passID(passID), opName(opName) {} Pass(const Pass &other) : Pass(other.passID, other.opName) {} + Pass &operator=(const Pass &) = delete; + Pass(Pass &&) = delete; + Pass &operator=(Pass &&) = delete; /// Returns the current pass state. detail::PassExecutionState &getPassState() { @@ -349,9 +352,15 @@ class Pass { /// - A 'std::unique_ptr clonePass() const' method. template class OperationPass : public Pass { +public: + ~OperationPass() = default; + protected: OperationPass(TypeID passID) : Pass(passID, OpT::getOperationName()) {} OperationPass(const OperationPass &) = default; + OperationPass &operator=(const OperationPass &) = delete; + OperationPass(OperationPass &&) = delete; + OperationPass &operator=(OperationPass &&) = delete; /// Support isa/dyn_cast functionality. static bool classof(const Pass *pass) { @@ -388,9 +397,15 @@ class OperationPass : public Pass { /// - A 'std::unique_ptr clonePass() const' method. template <> class OperationPass : public Pass { +public: + ~OperationPass() = default; + protected: OperationPass(TypeID passID) : Pass(passID) {} OperationPass(const OperationPass &) = default; + OperationPass &operator=(const OperationPass &) = delete; + OperationPass(OperationPass &&) = delete; + OperationPass &operator=(OperationPass &&) = delete; /// Indicate if the current pass can be scheduled on the given operation type. /// By default, generic operation passes can be scheduled on any operation. @@ -444,10 +459,14 @@ class PassWrapper : public BaseT { static bool classof(const Pass *pass) { return pass->getTypeID() == TypeID::get(); } + ~PassWrapper() = default; protected: PassWrapper() : BaseT(TypeID::get()) {} PassWrapper(const PassWrapper &) = default; + PassWrapper &operator=(const PassWrapper &) = delete; + PassWrapper(PassWrapper &&) = delete; + PassWrapper &operator=(PassWrapper &&) = delete; /// Returns the derived pass name. StringRef getName() const override { return llvm::getTypeName(); } diff --git a/mlir/tools/mlir-tblgen/PassGen.cpp b/mlir/tools/mlir-tblgen/PassGen.cpp index 11af6497cecf5..90aa67115a400 100644 --- a/mlir/tools/mlir-tblgen/PassGen.cpp +++ b/mlir/tools/mlir-tblgen/PassGen.cpp @@ -183,6 +183,10 @@ class {0}Base : public {1} { {0}Base() : {1}(::mlir::TypeID::get()) {{} {0}Base(const {0}Base &other) : {1}(other) {{} + {0}Base& operator=(const {0}Base &) = delete; + {0}Base({0}Base &&) = delete; + {0}Base& operator=({0}Base &&) = delete; + ~{0}Base() = default; /// Returns the command-line argument attached to this pass. static constexpr ::llvm::StringLiteral getArgumentName() { @@ -380,6 +384,10 @@ class {0}Base : public {1} { {0}Base() : {1}(::mlir::TypeID::get()) {{} {0}Base(const {0}Base &other) : {1}(other) {{} + {0}Base& operator=(const {0}Base &) = delete; + {0}Base({0}Base &&) = delete; + {0}Base& operator=({0}Base &&) = delete; + ~{0}Base() = default; /// Returns the command-line argument attached to this pass. static constexpr ::llvm::StringLiteral getArgumentName() {