[Mlir-commits] [mlir] 4639486 - [mlir][tblgen] Improving error messages

[wren romano]

Author: wren romano
Date: 2022-06-30T18:25:53-07:00
New Revision: 46394861a3305bd530b1d0cf8ae5b7a36a4a0b4d

URL: https://github.com/llvm/llvm-project/commit/46394861a3305bd530b1d0cf8ae5b7a36a4a0b4d
DIFF: https://github.com/llvm/llvm-project/commit/46394861a3305bd530b1d0cf8ae5b7a36a4a0b4d.diff

LOG: [mlir][tblgen] Improving error messages

This differential improves two error conditions, by detecting them earlier and by providing better messages to help users understand what went wrong.

Reviewed By: jpienaar

Differential Revision: https://reviews.llvm.org/D128555

Added: 
    mlir/test/mlir-tblgen/attr-or-type-builder-invalid.td

Modified: 
    mlir/lib/TableGen/AttrOrTypeDef.cpp
    mlir/lib/TableGen/Builder.cpp

Removed: 
    


################################################################################
diff  --git a/mlir/lib/TableGen/AttrOrTypeDef.cpp b/mlir/lib/TableGen/AttrOrTypeDef.cpp
index 17f329502c8f2..67dc24b2335cd 100644
--- a/mlir/lib/TableGen/AttrOrTypeDef.cpp
+++ b/mlir/lib/TableGen/AttrOrTypeDef.cpp
@@ -81,7 +81,13 @@ AttrOrTypeDef::AttrOrTypeDef(const llvm::Record *def) : def(def) {
                     "'assemblyFormat' or 'hasCustomAssemblyFormat' can only be "
                     "used when 'mnemonic' is set");
   }
-  // Assembly format requires accessors to be generated.
+  // Assembly format parser requires default builders to be generated.
+  if (hasDeclarativeFormat && skipDefaultBuilders()) {
+    PrintFatalError(
+        getLoc(),
+        "'assemblyFormat' requires 'skipDefaultBuilders' to be false");
+  }
+  // Assembly format printer requires accessors to be generated.
   if (hasDeclarativeFormat && !genAccessors()) {
     PrintFatalError(getLoc(),
                     "'assemblyFormat' requires 'genAccessors' to be true");

diff  --git a/mlir/lib/TableGen/Builder.cpp b/mlir/lib/TableGen/Builder.cpp
index d25d09d7569f2..e8dec691dfdc3 100644
--- a/mlir/lib/TableGen/Builder.cpp
+++ b/mlir/lib/TableGen/Builder.cpp
@@ -22,6 +22,13 @@ StringRef Builder::Parameter::getCppType() const {
   if (const auto *stringInit = dyn_cast<llvm::StringInit>(def))
     return stringInit->getValue();
   const llvm::Record *record = cast<llvm::DefInit>(def)->getDef();
+  // Inlining the first part of `Record::getValueAsString` to give better
+  // error messages.
+  const llvm::RecordVal *type = record->getValue("type");
+  if (!type || !type->getValue()) {
+    llvm::PrintFatalError("Builder DAG arguments must be either strings or "
+                          "defs which inherit from CArg");
+  }
   return record->getValueAsString("type");
 }
 

diff  --git a/mlir/test/mlir-tblgen/attr-or-type-builder-invalid.td b/mlir/test/mlir-tblgen/attr-or-type-builder-invalid.td
new file mode 100644
index 0000000000000..4db7162184550
--- /dev/null
+++ b/mlir/test/mlir-tblgen/attr-or-type-builder-invalid.td
@@ -0,0 +1,29 @@
+// RUN: not mlir-tblgen -gen-typedef-defs -I %S/../../include %s 2>&1 | FileCheck %s
+
+include "mlir/IR/AttrTypeBase.td"
+include "mlir/IR/OpBase.td"
+
+def Test_Dialect : Dialect {
+  let name = "TestDialect";
+  let cppNamespace = "::test";
+}
+
+class InvalidType<string name> : TypeDef<Test_Dialect, name> {
+  let mnemonic = ?;
+}
+
+// This definition should not generate an error due to the use in `InvalidTypeA`
+// CHECK-NOT: Record `TestParameter' does not have a field named `type'!
+def TestParameter : TypeParameter<"int", "int parameter">;
+
+// Test builder uses wrong record class.
+def InvalidTypeA : InvalidType<"InvalidTypeA"> {
+  let parameters = (ins "int":$v0);
+  let builders = [
+    // CHECK: Builder DAG arguments must be either strings or defs which inherit from CArg
+    TypeBuilder<(ins TestParameter:$arg0), [{
+      return $_get($_ctxt, arg0);
+    }]>
+  ];
+}
+