[Mlir-commits] [mlir] [mlir][AsmParser] Fix parseOptionalAttribute consuming loc() as an attribute (PR #180668)

[Matthias Springer]

matthias-springer wrote:

To summarize, the problem here is that the MLIR operation syntax is ambiguous. Your new test op highlights that nicely.

```
def TestOptionalLocGroupOp : TEST_Op<"optional_attr_op"> {
  let arguments = (ins OptionalAttr<AnyAttr>:$opt_attr);
  let assemblyFormat = "($opt_attr^)? attr-dict";
}
```

Example:
```mlir
test.optional_attr_op loc(...)
//                    ^^^^^^^^
//      Are you parsing $opt_attr or the trailing location
//      of the operation?
```

I think we should change the MLIR operation syntax, so that the notation is no longer ambiguous. We can change (1) the syntax when using an attribute in the assembly format or (2) the syntax when specifying a trailing location. I would opt for (1) because it's a somewhat niche feature and not widely used.

I propose this syntax:
```
test.optional_attr_op #loc(...)
//                    ^^^^^^^^^
//   #loc indicates a user-defined attribute.

test.optional_attr_op loc(...)
//                    ^^^^^^^^
//   loc indicates a trailing location.
```

@joker-eph What do you think?


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