[Mlir-commits] [mlir] [MLIR] Add two-phase region parsing API to OpAsmParser (PR #190545)

[Henrich Lauko]

xlauko wrote:

> I can imagine some syntax here, however I'm not clear on the need of the two-phases here. If this was all driven by the syntax like you're showing here then the generated code could construct directly the vector of region argument without this "2 phases" API. So I'm not totally clear on this.
 
The reason in here was that, for function, you might not want to materialize region, if function is just declaration. 
It came to me as cleaner design than destroying region later, when second parser finds out there is no funciton body.

> Seems to me if we step back a bit, the fundamental being what you're doing here is providing the ability to convey "some state" between two custom parsers. You're hardcoding some API on the OpAsmParser so that this state can only be used for region argument parsing (actually, other than the API name, I would say "it can be used to carry `Argument`s between custom parsers"), but we could imagine generalizing it.

Would you suggest to generalizing this? The nice thing I found about this was that at least you can guard yourself against wrong use of state, in general case I guess one might have varius desired invariants.

Anyway, I don't mind reworking this for more generic purpose RFC that covers also ODS extensions for both functions and control flow contructs. Though I would be happy for pointers what would you find good/potential directions?
Have there been any discussion about this before?



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