[Mlir-commits] [mlir] [MLIR] Validate directly parsed optional operand types (PR #212493)

[lorenzo chelini]

https://github.com/chelini created https://github.com/llvm/llvm-project/pull/212493

Commit 6c51938067fa ([MLIR] Guard optional operand resolution in generated op parsers) guarded resolution of every optional operand to avoid indexing an empty inferred type range. The guard also skipped resolveOperands when an optional operand was absent but an independently optional type directive was present. This accepted malformed assembly and silently discarded the orphan type when printing the operation.

Keep the guard for inferred type resolutions, which may access an empty optional type vector, but always resolve directly parsed type ranges so resolveOperands validates operand/type cardinality. Add coverage for both valid forms and a type-only negative reproducer.

>From e855807c9e3967ff77148101748047bc21d57e18 Mon Sep 17 00:00:00 2001
From: lorenzo chelini <lchelini at nvidia.com>
Date: Thu, 23 Jul 2026 12:10:29 +0200
Subject: [PATCH] [MLIR] Validate directly parsed optional operand types

Commit 6c51938067fa ([MLIR] Guard optional operand resolution in generated op parsers) guarded resolution of every optional operand to avoid indexing an empty inferred type range. The guard also skipped resolveOperands when an optional operand was absent but an independently optional type directive was present. This accepted malformed assembly and silently discarded the orphan type when printing the operation.

Keep the guard for inferred type resolutions, which may access an empty optional type vector, but always resolve directly parsed type ranges so resolveOperands validates operand/type cardinality. Add coverage for both valid forms and a type-only negative reproducer.
---
 mlir/test/IR/invalid-ops.mlir               |  5 +++++
 mlir/test/lib/Dialect/Test/TestOpsSyntax.td | 10 ++++++++++
 mlir/test/mlir-tblgen/op-format.mlir        |  6 ++++++
 mlir/tools/mlir-tblgen/OpFormatGen.cpp      | 17 ++++++++++++-----
 4 files changed, 33 insertions(+), 5 deletions(-)

diff --git a/mlir/test/IR/invalid-ops.mlir b/mlir/test/IR/invalid-ops.mlir
index 2f5dd28b51911..8f91d4e1d4c70 100644
--- a/mlir/test/IR/invalid-ops.mlir
+++ b/mlir/test/IR/invalid-ops.mlir
@@ -126,6 +126,11 @@ test.variadic_args_types_split "hello_world" : i32
 
 // -----
 
+// expected-error at +1 {{number of operands and types do not match: got 0 operands and 1 types}}
+test.format_optional_operand_type : i64
+
+// -----
+
 // Test multiple verifier errors in the same split to ensure all are reported.
 
 func.func @verify_fail_1() {
diff --git a/mlir/test/lib/Dialect/Test/TestOpsSyntax.td b/mlir/test/lib/Dialect/Test/TestOpsSyntax.td
index 096d4d255b729..1d9ff9bdef3a2 100644
--- a/mlir/test/lib/Dialect/Test/TestOpsSyntax.td
+++ b/mlir/test/lib/Dialect/Test/TestOpsSyntax.td
@@ -706,6 +706,16 @@ def FormatTypesMatchOptionalOp : TEST_Op<"format_types_match_optional", [
   }];
 }
 
+def FormatOptionalOperandTypeOp
+    : TEST_Op<"format_optional_operand_type"> {
+  let arguments = (ins Optional<AnyType>:$optional);
+  let assemblyFormat = [{
+    (`(` $optional^ `)`)?
+    (`:` type($optional)^)?
+    attr-dict
+  }];
+}
+
 //===----------------------------------------------------------------------===//
 // InferTypeOpInterface type inference in assembly format
 //===----------------------------------------------------------------------===//
diff --git a/mlir/test/mlir-tblgen/op-format.mlir b/mlir/test/mlir-tblgen/op-format.mlir
index 7ff9091d5500d..5f44e5cffe675 100644
--- a/mlir/test/mlir-tblgen/op-format.mlir
+++ b/mlir/test/mlir-tblgen/op-format.mlir
@@ -494,6 +494,12 @@ test.format_infer_variadic_type_from_non_variadic %i64, %i64 : i64
 // CHECK: test.format_types_match_optional
 test.format_types_match_optional
 
+// CHECK: test.format_optional_operand_type
+test.format_optional_operand_type
+
+// CHECK: test.format_optional_operand_type(%[[I64]]) : i64
+test.format_optional_operand_type(%i64) : i64
+
 //===----------------------------------------------------------------------===//
 // InferTypeOpInterface type inference
 //===----------------------------------------------------------------------===//
diff --git a/mlir/tools/mlir-tblgen/OpFormatGen.cpp b/mlir/tools/mlir-tblgen/OpFormatGen.cpp
index 4d5c31ae2ecae..2ae276a62e939 100644
--- a/mlir/tools/mlir-tblgen/OpFormatGen.cpp
+++ b/mlir/tools/mlir-tblgen/OpFormatGen.cpp
@@ -318,6 +318,10 @@ struct OperationFormat {
     std::optional<StringRef> getVarTransformer() const {
       return variableTransformer;
     }
+    /// Returns true if the type is parsed directly from the assembly format.
+    bool isDirectlyParsed() const {
+      return !builderIdx && !getVariable() && !getAttribute();
+    }
     void setResolver(ConstArgument arg, std::optional<StringRef> transformer) {
       resolver = arg;
       variableTransformer = transformer;
@@ -1890,19 +1894,22 @@ void OperationFormat::genParserOperandTypeResolution(
   // separately.
   for (unsigned i = 0, e = op.getNumOperands(); i != e; ++i) {
     NamedTypeConstraint &operand = op.getOperand(i);
-    // Optional operands may not be present; guard resolution to avoid
-    // out-of-bounds access on the (potentially empty) types vector.
-    if (operand.isOptional())
+    TypeResolution &operandType = operandTypes[i];
+    // Inferred type resolution may access another optional variable's empty
+    // type vector. Directly parsed type ranges are safe and must always be
+    // resolved so that operand/type cardinality is validated.
+    bool guardOptionalOperand =
+        operand.isOptional() && !operandType.isDirectlyParsed();
+    if (guardOptionalOperand)
       body << "  if (!" << operand.name << "Operands.empty()) {\n";
     body << "  if (parser.resolveOperands(" << operand.name << "Operands, ";
 
     // Resolve the type of this operand.
-    TypeResolution &operandType = operandTypes[i];
     emitTypeResolver(operandType, operand.name);
 
     body << ", " << operand.name
          << "OperandsLoc, result.operands))\n    return ::mlir::failure();\n";
-    if (operand.isOptional())
+    if (guardOptionalOperand)
       body << "  }\n";
   }
 }