Make ops with StructAttr's actually verify `isa<TheStruct>`.

[silvas]

Summary

Previously, they would only only verify isa<DictionaryAttr> on such attrs
which resulted in crashes down the line from code assuming that the
verifier was doing the more thorough check introduced in this patch.
The key change here is for StructAttr to use
CPred<"$_self.isa<" # name # ">()"> instead of isa<DictionaryAttr>.

To test this, introduce struct attrs to the test dialect. Previously,
StructAttr was only being tested by unittests/, which didn't verify how
StructAttr interacted with ODS.

[rriddle]

mlir/test/lib/Dialect/Test/TestDialect.cpp

483

Why is the namespace necessary here? I wouldn't expect it to be.

[silvas]

update

mlir/test/lib/Dialect/Test/TestDialect.cpp

483

Indeed, removed. I think some prior version of the patch required it.

mlir/include/mlir/IR/OpBase.td

class BitEnumAttr<string name, string description,
 
   // The delimiter used to separate bit enum cases in strings.
   string separator = "|";
 }
 
 //===----------------------------------------------------------------------===//
 // Composite attribute kinds
 
-class DictionaryAttrBase : Attr<CPred<"$_self.isa<DictionaryAttr>()">,
-                          "dictionary of named attribute values"> {
+class DictionaryAttrBase<Pred condition, string description> :
+    Attr<condition, description> {
   let storageType = [{ DictionaryAttr }];
   let returnType = [{ DictionaryAttr }];
   let valueType = NoneType;
   let convertFromStorage = "$_self";
 }
 
-def DictionaryAttr : DictionaryAttrBase;
+def DictionaryAttr : DictionaryAttrBase<CPred<"$_self.isa<DictionaryAttr>()">,
+                                        "dictionary of named attribute values">;
 
 class ElementsAttrBase<Pred condition, string description> :
     Attr<condition, description> {
   let storageType = [{ ElementsAttr }];
   let returnType = [{ ElementsAttr }];
   let convertFromStorage = "$_self";
 }
 
class StructFieldAttr<string thisName, Attr thisType> {
   // Attribute type wrapped by the struct attr.
   Attr type = thisType;
 }
 
 // Structured attribute that wraps a DictionaryAttr and provides both a
 // validation method and set of accessors for a fixed set of fields. This is
 // useful when representing data that would normally be in a structure.
 class StructAttr<string name, Dialect dialect,
-                 list<StructFieldAttr> attributes> : DictionaryAttrBase {
+                 list<StructFieldAttr> attributes> :
+    DictionaryAttrBase<CPred<"$_self.isa<" # name # ">()">,
+        "DictionaryAttr with field(s): " #
+        StrJoin<!foreach(a, attributes, "'" # a.name # "'"), ", ">.result #
+        " (each field having its own constraints)"> {
   // Name for this StructAttr.
   string className = name;
 
   // Return type should match the name of the structure.
   let returnType = name;
 
   // Storage type should match the name of the structure.
   let storageType = name;

mlir/test/IR/attribute.mlir

func @wrong_shape_fail() {
   // expected-error @+1 {{failed to satisfy constraint: 32-bit signless int elements attribute of shape [2]}}
   "test.ranked_int_elements_attr"() {
     matrix_i64_attr = dense<6> : tensor<4x8xi64>,
     vector_i32_attr = dense<5> : tensor<i32>
   } : () -> ()
   return
 }
 
+//===----------------------------------------------------------------------===//
+// Test StructAttr
+//===----------------------------------------------------------------------===//
+
+// -----
+
+func @missing_fields() {
+  // expected-error @+1 {{failed to satisfy constraint: DictionaryAttr with field(s): 'some_field', 'some_other_field' (each field having its own constraints)}}
+  "test.struct_attr"() {the_struct_attr = {}} : () -> ()
+  return
+}
+
+// -----
+
+func @erroneous_fields() {
+  // expected-error @+1 {{failed to satisfy constraint: DictionaryAttr with field(s): 'some_field', 'some_other_field' (each field having its own constraints)}}
+  "test.struct_attr"() {the_struct_attr = {some_field = 1 : i8, some_other_field = 1}} : () -> ()
+  return
+}
+

mlir/test/lib/Dialect/Test/CMakeLists.txt

 set(LLVM_OPTIONAL_SOURCES
   TestDialect.cpp
   TestPatterns.cpp
 )
 
 set(LLVM_TARGET_DEFINITIONS TestOps.td)
 mlir_tablegen(TestOps.h.inc -gen-op-decls)
 mlir_tablegen(TestOps.cpp.inc -gen-op-defs)
 mlir_tablegen(TestOpsDialect.h.inc -gen-dialect-decls)
 mlir_tablegen(TestOpEnums.h.inc -gen-enum-decls)
 mlir_tablegen(TestOpEnums.cpp.inc -gen-enum-defs)
+mlir_tablegen(TestOpStructs.h.inc -gen-struct-attr-decls)
+mlir_tablegen(TestOpStructs.cpp.inc -gen-struct-attr-defs)
 mlir_tablegen(TestPatterns.inc -gen-rewriters)
 add_public_tablegen_target(MLIRTestOpsIncGen)
 
 add_llvm_library(MLIRTestDialect
   TestDialect.cpp
   TestPatterns.cpp
 
   DEPENDS

mlir/test/lib/Dialect/Test/TestDialect.h

 #include "mlir/Interfaces/DerivedAttributeOpInterface.h"
 #include "mlir/Interfaces/InferTypeOpInterface.h"
 #include "mlir/Interfaces/SideEffects.h"
 
 #include "TestOpEnums.h.inc"
 
 namespace mlir {
 
+#include "TestOpStructs.h.inc"
 #include "TestOpsDialect.h.inc"
 
 #define GET_OP_CLASSES
 #include "TestOps.h.inc"
 
 } // end namespace mlir
 
 #endif // MLIR_TESTDIALECT_H

mlir/test/lib/Dialect/Test/TestDialect.cpp

 //===----------------------------------------------------------------------===//
 // Dialect Registration
 //===----------------------------------------------------------------------===//
 
 // Static initialization for Test dialect registration.
 static mlir::DialectRegistration<mlir::TestDialect> testDialect;
 
 #include "TestOpEnums.cpp.inc"
+#include "TestOpStructs.cpp.inc"
 
 #define GET_OP_CLASSES
 #include "TestOps.cpp.inc"

mlir/test/lib/Dialect/Test/TestOps.td

 def SomeI64Enum: I64EnumAttr<
   "SomeI64Enum", "", [I64Case5, I64Case10]>;
 
 def I64EnumAttrOp : TEST_Op<"i64_enum_attr"> {
   let arguments = (ins SomeI64Enum:$attr);
   let results = (outs I32:$val);
 }
 
+def SomeStructAttr : StructAttr<"SomeStructAttr", Test_Dialect, [
+  StructFieldAttr<"some_field", I64Attr>,
+  StructFieldAttr<"some_other_field", I64Attr>
+]> {}
+
+def StructAttrOp : TEST_Op<"struct_attr"> {
+  let arguments = (ins SomeStructAttr:$the_struct_attr);
+  let results = (outs);
+}
 
 def IntAttrOp : TEST_Op<"int_attrs"> {
   let arguments = (ins
     AnyI32Attr:$any_i32_attr,
     IndexAttr:$index_attr,
     UI32Attr:$ui32_attr,
     SI32Attr:$si32_attr
   );