[PATCH] D73406: [mlir] Add support for generating the parser/printer from the declarative operation format.

[Lei Zhang via Phabricator via llvm-commits]

antiagainst accepted this revision.
antiagainst added a comment.
This revision is now accepted and ready to land.

Nice! Just some nits.



================
Comment at: mlir/tools/mlir-tblgen/OpFormatGen.cpp:241
+/// {1}: The name of the attribute.
+const char *attrParserCode = R"(
+  {0} {1}Attr;
----------------
Nit: const char * const  ?


================
Comment at: mlir/tools/mlir-tblgen/OpFormatGen.cpp:306
+  if (isa<OperandsDirective>(arg))
+    return "fullOperand";
+  if (isa<ResultsDirective>(arg))
----------------
allOperands and allResults? fullOperand/fullResult is a bit confusing for me.


================
Comment at: mlir/tools/mlir-tblgen/OpFormatGen.cpp:332
+              .Case("]", "RSquare")
+       << "())\n    return failure();\n";
+}
----------------
I feel this line can be uniqued with line 318 and put at the end.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73406