[Mlir-commits] [mlir] [MLIR][ODS] Error on optional attribute used outside optional group in assemblyFormat (PR #188726)

[llvmlistbot at llvm.org]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-mlir

Author: Mehdi Amini (joker-eph)

<details>
<summary>Changes</summary>

Using an optional attribute directly in assemblyFormat (e.g., `$attr attr-dict`) without wrapping it in an optional group causes the generated printer to call `printAttribute` with a null `Attribute` when the attribute is absent. This leads to a crash in the alias initializer when it calls `getAlias` on a null attribute.

Add a validation check in `OpFormatParser::verifyAttributes` that detects this pattern and emits a diagnostic error with a helpful note pointing users to the correct `($attr^)?` syntax.

Fixes #<!-- -->58064

Assisted-by: Claude Code

---
Full diff: https://github.com/llvm/llvm-project/pull/188726.diff


3 Files Affected:

- (modified) mlir/test/mlir-tblgen/op-format-invalid.td (+6) 
- (modified) mlir/test/mlir-tblgen/op-format-spec.td (+1-1) 
- (modified) mlir/tools/mlir-tblgen/OpFormatGen.cpp (+19) 


``````````diff
diff --git a/mlir/test/mlir-tblgen/op-format-invalid.td b/mlir/test/mlir-tblgen/op-format-invalid.td
index 0a022ad43a749..59446bb9d7019 100644
--- a/mlir/test/mlir-tblgen/op-format-invalid.td
+++ b/mlir/test/mlir-tblgen/op-format-invalid.td
@@ -355,6 +355,12 @@ def OIListStartingToken : TestFormat_Op<[{
 // Optional Groups
 //===----------------------------------------------------------------------===//
 
+// CHECK: error: optional attribute 'attr' cannot be used outside of an optional group
+// CHECK: note: to conditionally print the attribute, use '($attr^)?'
+def OptionalAttrOutsideGroup : TestFormat_Op<[{
+  $attr attr-dict
+}]>, Arguments<(ins OptionalAttr<I64Attr>:$attr)>;
+
 // CHECK: error: optional groups can only be used as top-level elements
 def OptionalInvalidA : TestFormat_Op<[{
   type(($attr^)?) attr-dict
diff --git a/mlir/test/mlir-tblgen/op-format-spec.td b/mlir/test/mlir-tblgen/op-format-spec.td
index 1ac231116454b..0ad6d0959809d 100644
--- a/mlir/test/mlir-tblgen/op-format-spec.td
+++ b/mlir/test/mlir-tblgen/op-format-spec.td
@@ -175,7 +175,7 @@ def StringValid : TestFormat_Op<[{ custom<Foo>("foo") attr-dict }]>;
 
 // CHECK-NOT: error:
 def VariableValidA : TestFormat_Op<[{
-  $attr `:` attr-dict
+  ($attr^)? `:` attr-dict
 }]>, Arguments<(ins OptionalAttr<I1Attr>:$attr)>;
 def VariableValidB : TestFormat_Op<[{
   (`foo` $attr^)? `:` attr-dict
diff --git a/mlir/tools/mlir-tblgen/OpFormatGen.cpp b/mlir/tools/mlir-tblgen/OpFormatGen.cpp
index ff51fb403ffc8..c79f0f377644e 100644
--- a/mlir/tools/mlir-tblgen/OpFormatGen.cpp
+++ b/mlir/tools/mlir-tblgen/OpFormatGen.cpp
@@ -2890,6 +2890,25 @@ OpFormatParser::verifyAttributes(SMLoc loc,
     }
   }
 
+  // Check that optional attributes are not used directly (i.e. outside of an
+  // optional group or oilist). Printing an absent optional attribute passes a
+  // null Attribute to the printer, which leads to crashes in alias
+  // initialisation. OIList elements require optional attributes by design, so
+  // attributes nested inside them are not checked here.
+  for (FormatElement *element : elements) {
+    if (auto *attrVar = dyn_cast<AttributeVariable>(element)) {
+      const NamedAttribute *var = attrVar->getVar();
+      if (var->attr.isOptional()) {
+        return emitErrorAndNote(
+            loc,
+            "optional attribute '" + var->name +
+                "' cannot be used outside of an optional group",
+            "to conditionally print the attribute, use '($" + var->name +
+                "^)?'");
+      }
+    }
+  }
+
   return success();
 }
 

``````````

</details>


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