[PATCH] D137837: [Support] Move Target/CPU Printing out of CommandLine

[Francesco Petrogalli via Phabricator via llvm-commits]

fpetrogalli added inline comments.


================
Comment at: llvm/lib/Support/CommandLine.cpp:2523-2526
+  std::vector<VersionPrinterTy> ExtraPrinters;
+
 public:
   void print() {
----------------
There might be tools that rely on having the `ExtraPrinter` mechanisms available from `struct CommandLineCommonOptions`.  Would it be possible to just pass the extra printer here as a an argument of `print()`, and leave the `ExtraPrinters` mechanism inside  ` CommandLineCommonOptions`?




================
Comment at: llvm/lib/Support/CommandLine.cpp:2542-2549
     OS << '\n';
+
+    // Iterate over any registered extra printers and call them to add further
+    // information.
+    if (!ExtraPrinters.empty()) {
+      outs() << '\n';
+      for (const auto &I : ExtraPrinters)
----------------
I think you have modified the output here. I believe that now the tools will print 2 empty lines right after `" with assertions"`, while before it was printing only one empty line. Maybe it is not important, but I suspect that there might be some tools that parse the output printed by this function. Better to make sure we can guarantee the same output. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137837