[PATCH] D80138: [mlir] ensureRegionTerminator: take OpBuilder

[Alex Zinenko via Phabricator via llvm-commits]

ftynse marked an inline comment as done.
ftynse added inline comments.


================
Comment at: mlir/include/mlir/IR/OpDefinition.h:1123
+    /// OpBuilder is available at the call site, e.g., in the parser.
     static void ensureTerminator(Region &region, Builder &builder,
                                  Location loc) {
----------------
nicolasvasilache wrote:
> would it be better to make it explicit by having a small wrapper class and force users to cast?
> Making the non-safe use case hard here and everywhere else would have likely saved many hours of engineering,  
There are two kinds of users for this: `::build` and `::parse`. The former has OpBuilder readily available and the latter does not (and neither it should). Unless somebody explicitly upcasts the OpBuilder inside `::build` for some reason, this should be fine.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D80138/new/

https://reviews.llvm.org/D80138