Skip to content

Commit

Permalink
[mlir][NFC] Apply rule of five to *Pass classes (#80998)
Browse files Browse the repository at this point in the history
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 <anastasiya.pronina@intel.com>
Co-authored-by: Harald Rotuna <harald.razvan.rotuna@intel.com>
  • Loading branch information
3 people authored Mar 5, 2024
1 parent 9b672de commit 90e9e96
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 0 deletions.
19 changes: 19 additions & 0 deletions mlir/include/mlir/Pass/Pass.h
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,9 @@ 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 &) = delete;
Pass(Pass &&) = delete;
Pass &operator=(Pass &&) = delete;

/// Returns the current pass state.
detail::PassExecutionState &getPassState() {
Expand Down Expand Up @@ -349,9 +352,15 @@ class Pass {
/// - A 'std::unique_ptr<Pass> clonePass() const' method.
template <typename OpT = void>
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) {
Expand Down Expand Up @@ -388,9 +397,15 @@ class OperationPass : public Pass {
/// - A 'std::unique_ptr<Pass> clonePass() const' method.
template <>
class OperationPass<void> : 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.
Expand Down Expand Up @@ -444,10 +459,14 @@ class PassWrapper : public BaseT {
static bool classof(const Pass *pass) {
return pass->getTypeID() == TypeID::get<PassT>();
}
~PassWrapper() = default;

protected:
PassWrapper() : BaseT(TypeID::get<PassT>()) {}
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<PassT>(); }
Expand Down
8 changes: 8 additions & 0 deletions mlir/tools/mlir-tblgen/PassGen.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,10 @@ class {0}Base : public {1} {
{0}Base() : {1}(::mlir::TypeID::get<DerivedT>()) {{}
{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() {
Expand Down Expand Up @@ -380,6 +384,10 @@ class {0}Base : public {1} {
{0}Base() : {1}(::mlir::TypeID::get<DerivedT>()) {{}
{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() {
Expand Down

0 comments on commit 90e9e96

Please sign in to comment.