[Mlir-commits] [mlir] [tblgen] Use `emitError` for inferResultTypes failures (PR #165488)

[Jacques Pienaar]

jpienaar wrote:

> The context here is using MLIR through FFI, in particular I heavily use MLIR through Python bindings. When MLIR asserts, it crashes the python interpreter with no stack trace, so instead for op creation failures we take the diagnostics and raise them as a Python exception.

That should already be happening https://github.com/llvm/llvm-project/blame/9d1b6eec60490c09ab8367667fb70637695179c3/mlir/lib/Bindings/Python/IRCore.cpp#L1529 . If you have example where that isn't we should fix it.

> I think that's exactly what the generated builders for inferred result types do.

Yes I wrote that (41a73dd) :)

> For instance the generic Operation::create doesn't assert. 

If you have a NamedAttrList with repeated keys, it will assert fail.

> the contract as I understand it is to create the op and then call verify which uses emitError to report failures.

In general yes. The equivalent here would be for type inference to succeed but return an ErrorType that then fails verification of the trait. The problem with type though it is pervasive and the created op's output can be used as input when creating the next (if null, segfault when dereferenced while creating the next) and then you have potentially far propagation of invalid state, which complicates debugging.

> In the MLIR C API, since inferring result types may fail, the contract is to [return a null Operation](https://github.com/llvm/llvm-project/blob/main/mlir/lib/CAPI/IR/IR.cpp#L615-L623) rather than assert.

Yes, this is returning a Operation* effectively rather than FooOp view. You'll see no one checks if a C++ Op::create returned null or functions that take it as input checks. So it would result in silent corruption/error at distance vs today as everyone expects it doesn't get to that.

> My goal here is to be able to use the generated builders rather than expect to have to explicitly write all builders for inferred result types

You don't need to though, I'm saying you can do something like

```c++
template <class T>
FailureOr<T> createFailable(...) { ... }
```

and then do exactly the same as is done in Python, and this just forwards to the underlying generated builders, so nothing extra to write.


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