[Mlir-commits] [mlir] [MLIR][ODS] Require the value after oilist keyword (PR #229334)

[Arseniy Obolenskiy]

https://github.com/aobolensk created https://github.com/llvm/llvm-project/pull/229334

The generated parser treated optional attribute inside an oilist clause as optional even after its keyword was parsed

As a result, a keyword without a value was accepted, but the attribute was simply dropped

Prerequisite for https://github.com/llvm/llvm-project/pull/228977

>From 098d000df45a393d236ec3186fba5894ab75bb54 Mon Sep 17 00:00:00 2001
From: Arseniy Obolenskiy <arseniy.obolenskiy at amd.com>
Date: Tue, 6 Oct 2026 09:56:29 +0200
Subject: [PATCH] [MLIR][ODS] Require the value after oilist keyword

The generated parser treated optional attribute inside an oilist clause as optional even after its keyword was parsed

As a result, a keyword without a value was accepted, but the attribute was simply dropped
---
 mlir/test/Dialect/OpenMP/invalid.mlir       | 10 +++++++++-
 mlir/test/IR/traits.mlir                    | 18 ++++++++++++++++++
 mlir/test/lib/Dialect/Test/TestOpsSyntax.td |  9 +++++++++
 mlir/tools/mlir-tblgen/OpFormatGen.cpp      |  2 +-
 4 files changed, 37 insertions(+), 2 deletions(-)

diff --git a/mlir/test/Dialect/OpenMP/invalid.mlir b/mlir/test/Dialect/OpenMP/invalid.mlir
index c3fe4f60e4166..ad74eeadfb829 100644
--- a/mlir/test/Dialect/OpenMP/invalid.mlir
+++ b/mlir/test/Dialect/OpenMP/invalid.mlir
@@ -3002,8 +3002,16 @@ func.func @omp_target_update_invalid_motion_type(%map1 : memref<?xi32>) {
 
 // -----
 
-func.func @omp_target_map_must_specify_both_var_ptr_ptr_args(%arg : memref<?xi32>) {
+func.func @omp_map_var_ptr_ptr_missing_type(%arg : memref<?xi32>) {
+  // expected-error @below {{expected attribute value}}
   %map1 = omp.map.info var_ptr(%arg : memref<?xi32>, tensor<?xi32>) map_clauses(to) capture(ByRef) var_ptr_ptr(%arg : memref<?xi32>, ) name("") -> memref<?xi32>
+  return
+}
+
+// -----
+
+func.func @omp_target_map_must_specify_both_var_ptr_ptr_args(%arg : memref<?xi32>) {
+  %map1 = "omp.map.info"(%arg, %arg) <{map_capture_type = #omp.variable_capture_kind<ByRef>, map_type = #omp.clause_map_flags<to>, operandSegmentSizes = array<i32: 1, 1, 0, 0>, var_ptr_type = tensor<?xi32>}> : (memref<?xi32>, memref<?xi32>) -> memref<?xi32>
 
   // expected-error @below {{if varPtrPtr or varPtrPtrType is specified, then both must be present}}
   omp.target_update map_entries(%map1 : memref<?xi32>)
diff --git a/mlir/test/IR/traits.mlir b/mlir/test/IR/traits.mlir
index 5e5c2afe88cbd..648dce42ea597 100644
--- a/mlir/test/IR/traits.mlir
+++ b/mlir/test/IR/traits.mlir
@@ -600,6 +600,17 @@ func.func @succeededOilistTrivialProperties() {
 
 // -----
 
+// CHECK-LABEL: @succeededOilistOptionalAttr
+func.func @succeededOilistOptionalAttr() {
+  // CHECK: test.oilist_optional_attr keyword nested <nested = <1, !test.smpla, [5, 6]>>
+  test.oilist_optional_attr nested <nested = <1, !test.smpla, [5, 6]>> keyword
+  // CHECK: test.oilist_optional_attr nested <nested = <1, !test.smpla, [5, 6]>>
+  test.oilist_optional_attr nested #test.cmpnd_nested<nested = <1, !test.smpla, [5, 6]>>
+  return
+}
+
+// -----
+
 // CHECK-LABEL: @succeededOilistSimple
 func.func @succeededOilistSimple(%arg0 : i32, %arg1 : i32, %arg2 : i32) {
   // CHECK: test.oilist_with_simple_args keyword %{{.*}} : i32
@@ -690,6 +701,13 @@ func.func @failedOilistWithTrailingSeparator() {
 
 // -----
 
+func.func @failedOilistOptionalAttrMissingValue() {
+  // expected-error at +1 {{expected '<'}}
+  test.oilist_optional_attr nested
+}
+
+// -----
+
 func.func @failedHasDominanceScopeOutsideDominanceFreeScope() -> () {
   "test.ssacfg_region"() ({
     test.graph_region {
diff --git a/mlir/test/lib/Dialect/Test/TestOpsSyntax.td b/mlir/test/lib/Dialect/Test/TestOpsSyntax.td
index 8b0ded9ad27d9..f2d6ba3c5855e 100644
--- a/mlir/test/lib/Dialect/Test/TestOpsSyntax.td
+++ b/mlir/test/lib/Dialect/Test/TestOpsSyntax.td
@@ -147,6 +147,15 @@ def OIListWithSeparator : TEST_Op<"oilist_with_separator"> {
   }];
 }
 
+def OIListOptionalAttr : TEST_Op<"oilist_optional_attr"> {
+  let arguments = (ins UnitAttr:$keyword,
+                       OptionalAttr<CompoundAttrNested>:$nested);
+  let assemblyFormat = [{
+    oilist( `keyword` $keyword
+          | `nested` $nested) attr-dict
+  }];
+}
+
 def OIListAllowedLiteral : TEST_Op<"oilist_allowed_literal"> {
   let assemblyFormat = [{
     oilist( `foo` | `bar` ) `buzz` attr-dict
diff --git a/mlir/tools/mlir-tblgen/OpFormatGen.cpp b/mlir/tools/mlir-tblgen/OpFormatGen.cpp
index 33a7ac7c42244..a4798440cf28d 100644
--- a/mlir/tools/mlir-tblgen/OpFormatGen.cpp
+++ b/mlir/tools/mlir-tblgen/OpFormatGen.cpp
@@ -2032,7 +2032,7 @@ void OperationFormat::genElementParser(FormatElement *element, MethodBody &body,
         }
       } else {
         for (FormatElement *el : pelement)
-          genElementParser(el, body, attrTypeCtx);
+          genElementParser(el, body, attrTypeCtx, GenContext::Optional);
       }
       body << "    } else ";
     }