[PATCH] D79004: [mlir][assemblyFormat] Fix bug when using AttrSizedOperandSegments trait with only non-buildable operand types

[Jean-Michel Gorius via Phabricator via llvm-commits]

Kayjukh added inline comments.


================
Comment at: mlir/tools/mlir-tblgen/OpFormatGen.cpp:744
   if (!buildableTypes.empty()) {
     body << "  Builder &builder = parser.getBuilder();\n";
 
----------------
This could be removed and the next lines adapted as noted in the comment.


================
Comment at: mlir/tools/mlir-tblgen/OpFormatGen.cpp:747
     FmtContext typeBuilderCtx;
     typeBuilderCtx.withBuilder("builder");
     for (auto &it : buildableTypes)
----------------
We should follow what is done in `OperationFormat::genParser` and use `parser.getBuilder()` instead of a potentially fragile local variable declaration.


================
Comment at: mlir/tools/mlir-tblgen/OpFormatGen.cpp:870
+    if (buildableTypes.empty())
+      body << "  Builder &builder = parser.getBuilder();\n";
     body << "  result.addAttribute(\"operand_segment_sizes\", "
----------------
This seems to be a somewhat fragile fix, especially regarding the mirrored declaration of `Builder &builder` in `OperationFormat::genParserTypeResolution`. To reduce the coupling between the functions in this file, I would rather suggest to replace the usages of `builder` with `parser.getBuilder()`.


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

https://reviews.llvm.org/D79004