[PATCH] D33880: COFF: Introduce LD shim around LINK

[Rui Ueyama via Phabricator via llvm-commits]

ruiu added inline comments.


================
Comment at: MinGW/CMakeLists.txt:9
+
+add_lld_library(lldMinGW
+  Driver.cpp
----------------
martell wrote:
> I also renamed this to MinGW, not sure if that is desirable for a lib name even though it is consistent.
Yeah, I think MinGW is not exactly right, but I can't come up with a better name. It is probably not that bad, as it is a bit inaccurate but clear on what it refers to.


================
Comment at: MinGW/Driver.cpp:53
+// Create table mapping all options defined in Options.td
+static const opt::OptTable::Info infoTable[] = {
+#define OPTION(X1, X2, ID, KIND, GROUP, ALIAS, X7, X8, X9, X10, X11, X12)      \
----------------
infoTable -> InfoTable

(Yet another style issue, and honestly I'm a bit tired of pointing them out, so please please fix them before sending your patch to review. If you do not pay attention to the documented and obvious coding style rules, I would probably suspend doing "real" code review and instead pointing out only style issues until your code follows the coding style. I understand that we cannot and do not want to reduce the error rate to zero, and I really appreciate your work on lld/mingw, but pointing out that variables are in CamelCase in the LLVM coding style on virtually every patch update is a waste of both your and my time.)


================
Comment at: MinGW/Driver.cpp:77-78
+  llvm_shutdown();
+  outs().flush();
+  errs().flush();
+  exit(1);
----------------
I don't think you need to call flush() because exit() will indirectly call them.


Repository:
  rL LLVM

https://reviews.llvm.org/D33880