[PATCH] D74246: [mlir][Pass] Enable printing pass options as part of `-help`.

[Jacques Pienaar via Phabricator via llvm-commits]

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

Nice, looks much cleaner. Mostly small nits.



================
Comment at: mlir/include/mlir/Pass/PassOptions.h:188
+  /// map.
+  PassOptions(const PassOptions &) {}
+
----------------
Why not make it non-copyable?


================
Comment at: mlir/lib/Pass/PassRegistry.cpp:43
+  llvm::outs().indent(indent) << "--" << arg;
+  llvm::outs().indent(numSpaces) << " -   " << desc << '\n';
+}
----------------
So it has numSpaces + 1 empty space before - ? Could llvm::format make this simpler?


================
Comment at: mlir/lib/Pass/PassRegistry.cpp:200-203
+                       [](OptionBase *const *lhs, OptionBase *const *rhs) {
+                         return (*lhs)->getArgStr().compare(
+                             (*rhs)->getArgStr());
+                       });
----------------
Nit: Hoist out into cmp lambda? I think it would make it easier to read (and may actually be shorter as it reduces whitespace)


================
Comment at: mlir/lib/Pass/PassRegistry.cpp:552
+                  "A textual description of a pass pipeline to run",
+                  /*indent=*/4, globalWidth, /*isTopLevel=*/!opt.hasArgStr());
+
----------------
Any way to make these less magical? E.g., we have 4 here, 5 above, 6 below and 2 for indent (which is matches my editor config so I won't complain 😛 )


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74246