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

[Jacques Pienaar]

jpienaar wrote:

> I mean for failures in inferring result types

I think there is a misunderstanding here: I'm saying passing invalid arguments to builder is at best UB. This inference method or not I see as secondary. Its creating an expectation we can't support - infer functions aren't robust to invalid arguments today, they were written to allow some by users but very haphazardly rather than some formalized way. So this can't be supported/would just lead to a lot of folks being upset that there is only some vague concept of partial support while resulting in weirder crashes.

This is an builders for internal representation. The fact that we don't check types & attributes on construction is semi due to when verification for them were added and also to allow for mutable attributes (which we don't need for operations as we have OperationState, so create need only be called post the builds and when its correct). And we may have chosen an inconsistent default there (should have added getUnchecked rather than getChecked to match operations). MLIR does assume valid internal state and/or freedom to crash if not so. This is a change on the developer and framework expectations IMHO.

> This means I have to rewrite custom implementations of both the generated op builders and any op builders which happen to depend on those.

Isn't it the case here that for those we also generate build method that takes the explicit type? So all the generated build methods should be fine AFAIK (as we generate with and without), and one can wrap those builds inside a `createOrThrowException` helper. But yes if one build calls another manually written build (or create) and the type inference happens internally to a called one then one can't wrap that in general. In some cases one can while doing redundant work, e.g., one can still infer result type and use that only to throw exception or not, but that's only for limited cases. I don't have stats here though: how many such cases exist?

Verifiers are not frontend utils, the error messages are also currently more for compiler writers rather than general audience.  Builders even more so are IR construction rather than user level. For regular end-users, I'd wrap it as we did for the Python builders. I'd add all the convenience wrappers there too TBH (make discoverability easier). The generation of those are rather direct. A goal was (well is, but that project is more thought than code) to have the invariants more declarative so that one could generate different variants, but at the moment one would get error messages for specific audience.

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