[Mlir-commits] [mlir] [mlir][TableGen] Emit interface traits after all interfaces (PR #147699)

[Andrei Golubev]

andrey-golubev wrote:

> Can you link the before/after output of `mlir/test/lib/Dialect/Test/TestInterfaces.td` in a Github Gist etc.?

Sure. Here's the full gist with "before" and "after" - https://gist.github.com/andrey-golubev/a337305e3ef31c595b8c2f4e9267ab78

In case this goes stale, the snippet below shows the issue (note: my added comments start with `// ag:`):
```cpp
namespace mlir {
class TestCyclicTypeInterfaceA; // ag: forward declaration - only for "A"!
namespace detail {
struct TestCyclicTypeInterfaceAInterfaceTraits {
  struct Concept {
    /// The methods defined by the interface.
    ::mlir::FailureOr<::mlir::TestCyclicTypeInterfaceB> (*returnB)(const Concept *impl, ::mlir::Type );
    // ag: here, TestCyclicTypeInterfaceB must be declared
  };
  template<typename ConcreteType>
  class Model : public Concept {
  public:
    using Interface = ::mlir::TestCyclicTypeInterfaceA;
    Model() : Concept{returnB} {}

    static inline ::mlir::FailureOr<::mlir::TestCyclicTypeInterfaceB> returnB(const Concept *impl, ::mlir::Type tablegen_opaque_val);
  };
  template<typename ConcreteType>
  class FallbackModel : public Concept {
  public:
    using Interface = ::mlir::TestCyclicTypeInterfaceA;
    FallbackModel() : Concept{returnB} {}

    static inline ::mlir::FailureOr<::mlir::TestCyclicTypeInterfaceB> returnB(const Concept *impl, ::mlir::Type tablegen_opaque_val);
  };
  template<typename ConcreteModel, typename ConcreteType>
  class ExternalModel : public FallbackModel<ConcreteModel> {
  public:
    using ConcreteEntity = ConcreteType;
    ::mlir::FailureOr<::mlir::TestCyclicTypeInterfaceB> returnB(::mlir::Type tablegen_opaque_val) const;
  };
};
template <typename ConcreteType>
struct TestCyclicTypeInterfaceATrait;

} // namespace detail

// ag: this is a "declaration" for the interface - works fine as there are no definitions
class TestCyclicTypeInterfaceA : public ::mlir::TypeInterface<TestCyclicTypeInterfaceA, detail::TestCyclicTypeInterfaceAInterfaceTraits> {
public:
  using ::mlir::TypeInterface<TestCyclicTypeInterfaceA, detail::TestCyclicTypeInterfaceAInterfaceTraits>::TypeInterface;
  template <typename ConcreteType>
  struct Trait : public detail::TestCyclicTypeInterfaceATrait<ConcreteType> {};
  ::mlir::FailureOr<::mlir::TestCyclicTypeInterfaceB> returnB() const;
};
// ag: this is a trait generated for the interface above, it has interface method *definitions* inline (due to default implementation)
namespace detail {
  template <typename ConcreteType>
  struct TestCyclicTypeInterfaceATrait : public ::mlir::TypeInterface<TestCyclicTypeInterfaceA, detail::TestCyclicTypeInterfaceAInterfaceTraits>::Trait<ConcreteType> {
    ::mlir::FailureOr<::mlir::TestCyclicTypeInterfaceB> returnB() const {
      // ag: here, TestCyclicTypeInterfaceB must be *complete*
      return mlir::failure();
    }
  };
}// namespace detail
} // namespace mlir
// ag: this is the start of interface "B" definition (but it's too late)
namespace mlir {
class TestCyclicTypeInterfaceB;
namespace detail {
struct TestCyclicTypeInterfaceBInterfaceTraits {
  struct Concept {
    /// The methods defined by the interface.
    ::mlir::FailureOr<::mlir::TestCyclicTypeInterfaceA> (*returnA)(const Concept *impl, ::mlir::Type );
  };
  template<typename ConcreteType>
  class Model : public Concept {
  public:
    using Interface = ::mlir::TestCyclicTypeInterfaceB;
    Model() : Concept{returnA} {}

    static inline ::mlir::FailureOr<::mlir::TestCyclicTypeInterfaceA> returnA(const Concept *impl, ::mlir::Type tablegen_opaque_val);
  };
  template<typename ConcreteType>
  class FallbackModel : public Concept {
  public:
    using Interface = ::mlir::TestCyclicTypeInterfaceB;
    FallbackModel() : Concept{returnA} {}

    static inline ::mlir::FailureOr<::mlir::TestCyclicTypeInterfaceA> returnA(const Concept *impl, ::mlir::Type tablegen_opaque_val);
  };
  template<typename ConcreteModel, typename ConcreteType>
  class ExternalModel : public FallbackModel<ConcreteModel> {
  public:
    using ConcreteEntity = ConcreteType;
    ::mlir::FailureOr<::mlir::TestCyclicTypeInterfaceA> returnA(::mlir::Type tablegen_opaque_val) const;
  };
};
template <typename ConcreteType>
struct TestCyclicTypeInterfaceBTrait;

} // namespace detail
class TestCyclicTypeInterfaceB : public ::mlir::TypeInterface<TestCyclicTypeInterfaceB, detail::TestCyclicTypeInterfaceBInterfaceTraits> {
public:
  using ::mlir::TypeInterface<TestCyclicTypeInterfaceB, detail::TestCyclicTypeInterfaceBInterfaceTraits>::TypeInterface;
  template <typename ConcreteType>
  struct Trait : public detail::TestCyclicTypeInterfaceBTrait<ConcreteType> {};
  ::mlir::FailureOr<::mlir::TestCyclicTypeInterfaceA> returnA() const;
};
namespace detail {
  template <typename ConcreteType>
  struct TestCyclicTypeInterfaceBTrait : public ::mlir::TypeInterface<TestCyclicTypeInterfaceB, detail::TestCyclicTypeInterfaceBInterfaceTraits>::Trait<ConcreteType> {
    ::mlir::FailureOr<::mlir::TestCyclicTypeInterfaceA> returnA() const {
      return mlir::failure();
    }
  };
}// namespace detail
} // namespace mlir
```

what the PR does is:
* moves method definitions (actually, full trait definitions right now) after interfaces, etc. are defined (i.e. complete)
* adds forward declaration section to the very top to avoid needing to forward declare every time in user code (where `#include "TestTypeInterfaces.h.inc"` happens) - this is mostly for convenience

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