[Mlir-commits] [mlir] [MLIR] Fix a problem with parseOptionalAttribute (PR #188125)

[llvmlistbot at llvm.org]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-mlir-core

Author: Andy Kaylor (andykaylor)

<details>
<summary>Changes</summary>

The recent changes to introduce a non-virtual template overload of `parseOptionalAttribute` triggered a regression in CIR where we had a default-valued attribute using a dialect-specific enum followed by a required attribute that also used a dialect-specific enum. The `parseOptionalAttribute` change caused our test for the required attribute being missing to fail with the wrong message. Debugging indicated that `parseOptionalAttribute` was consuming an unrelated token in the operation (in this case `@`), causing an incorrect diagnosis of the problem.

This change updates the new `parseOptionalAttribute` implementation to specifically look for attributes in the `#dialect.mnemnoc<>` form before falling back on the default handling.

This fixes a regression in `clang/test/CIR/IR/invalid-linkage.cir`

Assisted by: Cursor / claude-4.6-opus-high

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


2 Files Affected:

- (modified) mlir/include/mlir/IR/OpImplementation.h (+16-1) 
- (modified) mlir/lib/AsmParser/AsmParserImpl.h (+7) 


``````````diff
diff --git a/mlir/include/mlir/IR/OpImplementation.h b/mlir/include/mlir/IR/OpImplementation.h
index 96c8f3d79c5e0..b6b9e0e0ef239 100644
--- a/mlir/include/mlir/IR/OpImplementation.h
+++ b/mlir/include/mlir/IR/OpImplementation.h
@@ -1156,17 +1156,32 @@ class AsmParser {
   virtual OptionalParseResult parseOptionalAttribute(SymbolRefAttr &result,
                                                      Type type = {}) = 0;
 
+  /// Parse an optional attribute that must begin with a '#' hash identifier
+  /// (i.e. `#dialect.mnemonic<...>`). Returns std::nullopt if the next token
+  /// is not a hash identifier, without consuming any tokens.
+  virtual OptionalParseResult parseOptionalHashAttribute(Attribute &result,
+                                                         Type type = {}) = 0;
+
   /// Parse an optional attribute of a specific typed result. This overload
   /// handles concrete attribute types (e.g. FloatAttr) that are not covered by
   /// a dedicated virtual overload. It parses any attribute and then validates
   /// that the result is of the expected type, emitting an error if not.
+  ///
+  /// For attribute types that define a custom `parse` method (i.e. AttrDef
+  /// types with assemblyFormat), parsing is restricted to `#`-prefixed
+  /// attribute syntax to avoid greedily consuming tokens (like `@symbol`)
+  /// that are intended for later format elements.
   template <
       typename AttrType,
       typename = std::enable_if_t<!llvm::is_one_of<
           AttrType, Attribute, ArrayAttr, StringAttr, SymbolRefAttr>::value>>
   OptionalParseResult parseOptionalAttribute(AttrType &result, Type type = {}) {
     Attribute attr;
-    OptionalParseResult parseResult = parseOptionalAttribute(attr, type);
+    OptionalParseResult parseResult;
+    if constexpr (detect_has_parse_method<AttrType>::value)
+      parseResult = parseOptionalHashAttribute(attr, type);
+    else
+      parseResult = parseOptionalAttribute(attr, type);
     if (!parseResult.has_value() || failed(*parseResult))
       return parseResult;
     result = dyn_cast<AttrType>(attr);
diff --git a/mlir/lib/AsmParser/AsmParserImpl.h b/mlir/lib/AsmParser/AsmParserImpl.h
index 3c60287a09a0f..e26a12da9aada 100644
--- a/mlir/lib/AsmParser/AsmParserImpl.h
+++ b/mlir/lib/AsmParser/AsmParserImpl.h
@@ -458,6 +458,13 @@ class AsmParserImpl : public BaseT {
                                              Type type) override {
     return parser.parseOptionalAttribute(result, type);
   }
+  OptionalParseResult parseOptionalHashAttribute(Attribute &result,
+                                                 Type type) override {
+    if (parser.getToken().isNot(Token::hash_identifier))
+      return std::nullopt;
+    result = parser.parseAttribute(type);
+    return success(static_cast<bool>(result));
+  }
   OptionalParseResult parseOptionalAttribute(ArrayAttr &result,
                                              Type type) override {
     return parser.parseOptionalAttribute(result, type);

``````````

</details>


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