[mlir] ODS: emit interface traits outside of the interface class

[ftynse]

Summary

ODS currently emits the interface trait class as a nested class inside the
interface class. As an unintended consequence, the default implementations of
interface methods have implicit access to static fields of the interface class,
e.g. those declared in extraClassDeclaration, including private methods (!),
or in the parent class. This may break the use of default implementations for
external models, which are not defined in the interface class, and generally
complexifies the abstraction.

Emit intraface traits outside of the interface class itself to avoid accidental
implicit visibility. Public static fields can still be accessed via explicit
qualification with a class name, e.g., MyOpInterface::staticMethod() instead
of staticMethod.

Update the documentation to clarify the role of extraClassDeclaration in
interfaces.

mlir/docs/Interfaces.md

*   C++ Namespace (`cppNamespace`)
     -   The C++ namespace that the interface class should be generated in.
 *   Methods (`methods`)
     -   The list of interface hook methods that are defined by the IR object.
     -   The structure of these methods is defined below.
 *   Extra Class Declarations (Optional: `extraClassDeclaration`)
     -   Additional C++ code that is generated in the declaration of the
         interface class. This allows for defining methods and more on the user
         facing interface class, that do not need to hook into the IR entity.
+        These declarations are _not_ implicitly visible in default
+        implementations of interface methods, but static declarations may be
+        accessed with full name qualification.
 
 `OpInterface` classes may additionally contain the following:
 
 *   Verifier (`verify`)
     -   A C++ code block containing additional verification applied to the
         operation that the interface is attached to.
     -   The structure of this code block corresponds 1-1 with the structure of a
         [`Trait::verifyTrait`](Traits.md) method.
*   DefaultImplementation (Optional)
     -   An optional explicit default implementation of the interface method.
     -   This implementation is placed within the `Trait` class that is attached
         to the IR entity, and does not directly affect any of the interface
         classes. As such, this method has the same characteristics as any other
         [`Trait`](Traits.md) method.
     -   `ConcreteAttr`/`ConcreteOp`/`ConcreteType` is an implicitly defined
         `typename` that can be used to refer to the type of the derived IR
         entity currently being operated on.
+    -   This may refer to static fields of the interface class using the
+        qualified name, e.g., `TestOpInterface::staticMethod()`.
 
 ODS also allows for generating declarations for the `InterfaceMethod`s of an
 operation if the operation specifies the interface with
 `DeclareOpInterfaceMethods` (see an example below).
 
 Examples:
 
 ~~~tablegen

mlir/include/mlir/Interfaces/ControlFlowInterfaces.td

InterfaceMethod<[{
       }],
       "Block *", "getSuccessorForOperands",
       (ins "ArrayRef<Attribute>":$operands), [{}],
       /*defaultImplementation=*/[{ return nullptr; }]
     >
   ];
 
   let verify = [{
-    auto concreteOp = cast<ConcreteOpType>($_op);
+    auto concreteOp = cast<ConcreteOp>($_op);
     for (unsigned i = 0, e = $_op->getNumSuccessors(); i != e; ++i) {
       Optional<OperandRange> operands = concreteOp.getSuccessorOperands(i);
       if (failed(detail::verifyBranchSuccessorOperands($_op, i, operands)))
         return failure();
     }
     return success();
   }];
 }
InterfaceMethod<[{
         unsigned numRegions = this->getOperation()->getNumRegions();
         assert(countPerRegion.empty());
         countPerRegion.resize(numRegions, kUnknownNumRegionInvocations);
       }]
     >
   ];
 
   let verify = [{
-    static_assert(!ConcreteOpType::template hasTrait<OpTrait::ZeroRegion>(),
+    static_assert(!ConcreteOp::template hasTrait<OpTrait::ZeroRegion>(),
                   "expected operation to have non-zero regions");
     return success();
   }];
 
   let extraClassDeclaration = [{
     /// Convenience helper in case none of the operands is known.
     void getSuccessorRegions(Optional<unsigned> index,
                              SmallVectorImpl<RegionSuccessor> &regions) {

mlir/tools/mlir-tblgen/OpInterfacesGen.cpp

for (auto &method : interface.getMethods()) {
       emitMethodNameAndArgs(method, os, valueType,
                             /*addThisArg=*/!method.isStatic(),
                             /*addConst=*/false);
       os << ";\n";
     }
     os << "  };\n";
   }
 
+  // Emit the template for the external model.
   os << "  template<typename ConcreteModel, typename " << valueTemplate
      << ">\n";
   os << "  class ExternalModel : public FallbackModel<ConcreteModel> {\n";
-
-  // Emit the template for the external model if there are no extra class
-  // declarations.
-  if (interface.getExtraClassDeclaration()) {
-    os << "  };\n";
-    return;
-  }
-
   os << "  public:\n";
 
   // Emit declarations for methods that have default implementations. Other
   // methods are expected to be implemented by the concrete derived model.
   for (auto &method : interface.getMethods()) {
     if (!method.getDefaultImplementation())
       continue;
     os << "    ";
if (!method.isStatic())
       os << "tablegen_opaque_val" << (method.arg_empty() ? "" : ", ");
     llvm::interleaveComma(
         method.getArguments(), os,
         [&](const InterfaceMethod::Argument &arg) { os << arg.name; });
     os << ");\n}\n";
   }
 
   // Emit default implementations for the external model.
-  if (interface.getExtraClassDeclaration())
-    return;
-
   for (auto &method : interface.getMethods()) {
     if (!method.getDefaultImplementation())
       continue;
     os << "template<typename ConcreteModel, typename " << valueTemplate
        << ">\n";
     emitCPPType(method.getReturnType(), os);
     os << "detail::" << interface.getName()
        << "InterfaceTraits::ExternalModel<ConcreteModel, " << valueTemplate
if (auto verify = interface.getVerify()) {
     os << "    static ::mlir::LogicalResult verifyTrait(::mlir::Operation *op) "
           "{\n      "
        << tblgen::tgfmt(verify->trim(), &verifyCtx) << "\n    }\n";
   }
   if (auto extraTraitDecls = interface.getExtraTraitClassDeclaration())
     os << tblgen::tgfmt(*extraTraitDecls, &traitMethodFmt) << "\n";
 
   os << "  };\n";
-
-  // Emit a utility wrapper trait class.
-  os << llvm::formatv("  template <typename {1}>\n"
-                      "  struct Trait : public {0}Trait<{1}> {{};\n",
-                      interfaceName, valueTemplate);
 }
 
 void InterfaceGenerator::emitInterfaceDecl(Interface interface) {
   llvm::SmallVector<StringRef, 2> namespaces;
   llvm::SplitString(interface.getCppNamespace(), namespaces, "::");
   for (StringRef ns : namespaces)
     os << "namespace " << ns << " {\n";
 
   StringRef interfaceName = interface.getName();
   auto interfaceTraitsName = (interfaceName + "InterfaceTraits").str();
 
   // Emit a forward declaration of the interface class so that it becomes usable
   // in the signature of its methods.
   os << "class " << interfaceName << ";\n";
 
   // Emit the traits struct containing the concept and model declarations.
   os << "namespace detail {\n"
      << "struct " << interfaceTraitsName << " {\n";
   emitConceptDecl(interface);
   emitModelDecl(interface);
-  os << "};\n} // end namespace detail\n";
+  os << "};";
+
+  // Emit the derived trait for the interface.
+  os << "template <typename " << valueTemplate << ">\n";
+  os << "struct " << interface.getName() << "Trait;\n";
+
+  os << "\n} // end namespace detail\n";
 
   // Emit the main interface class declaration.
   os << llvm::formatv("class {0} : public ::mlir::{3}<{1}, detail::{2}> {\n"
                       "public:\n"
                       "  using ::mlir::{3}<{1}, detail::{2}>::{3};\n",
                       interfaceName, interfaceName, interfaceTraitsName,
                       interfaceBaseType);
 
-  // Emit the derived trait for the interface.
-  emitTraitDecl(interface, interfaceName, interfaceTraitsName);
+  // Emit a utility wrapper trait class.
+  os << llvm::formatv("  template <typename {1}>\n"
+                      "  struct Trait : public detail::{0}Trait<{1}> {{};\n",
+                      interfaceName, valueTemplate);
 
   // Insert the method declarations.
   bool isOpInterface = isa<OpInterface>(interface);
   for (auto &method : interface.getMethods()) {
     emitCPPType(method.getReturnType(), os << "  ");
     emitMethodNameAndArgs(method, os, valueType, /*addThisArg=*/false,
                           /*addConst=*/!isOpInterface);
     os << ";\n";
   }
 
   // Emit any extra declarations.
   if (Optional<StringRef> extraDecls = interface.getExtraClassDeclaration())
     os << *extraDecls << "\n";
 
   os << "};\n";
 
+  os << "namespace detail {\n";
+  emitTraitDecl(interface, interfaceName, interfaceTraitsName);
+  os << "}// namespace detail\n";
+
   emitModelMethodsDef(interface);
 
   for (StringRef ns : llvm::reverse(namespaces))
     os << "} // namespace " << ns << "\n";
 }
 
 bool InterfaceGenerator::emitInterfaceDecls() {
   llvm::emitSourceFileHeader("Interface Declarations", os);