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

Rui Ueyama via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sun Aug 6 18:49:44 PDT 2017


ruiu added inline comments.


================
Comment at: MinGW/Driver.cpp:73
+static std::vector<StringRef> SearchPaths;
+static bool files = false;
+
----------------
remove this `files` global variable.


================
Comment at: MinGW/Driver.cpp:78-80
+  outs().flush();
+  errs().flush();
+  _exit(1);
----------------
Just call `exit()` instead of `_exit()`.  `_exit` is a performance hack that don't want to replicate to a thin wrapper.


================
Comment at: MinGW/Driver.cpp:83
+
+static cl::TokenizerCallback getQuotingStyle(opt::InputArgList &Args) {
+  if (auto *Arg = Args.getLastArg(OPT_rsp_quoting)) {
----------------
Do you really need this?


================
Comment at: MinGW/Driver.cpp:141
+void createFiles(opt::InputArgList &Args) {
+  for (auto *Arg : Args)
+    switch (Arg->getOption().getID()) {
----------------
Add {}


================
Comment at: MinGW/Driver.cpp:150
+    }
+  if (!files)
+    error("no input files");
----------------
Just check if there's at least one OPT_l or OPT_INPUT.


================
Comment at: MinGW/Driver.cpp:154
+
+static StringRef getString(opt::InputArgList &Args, unsigned Key,
+                           StringRef Default = "") {
----------------
Use `Args.getLastArgValue` instead.


================
Comment at: MinGW/Driver.cpp:168
+
+static void forward(opt::InputArgList &Args, unsigned Key, StringRef outArg) {
+  StringRef S = getString(Args, Key);
----------------
Please spend a bit more time on reviewing your patch yourself before sending it to review so that everything follows the LLVM style. You want to read your patch from top to bottom several times (that's what I do when I send a patch.)

outArg -> OutArg.


Repository:
  rL LLVM

https://reviews.llvm.org/D33880





More information about the llvm-commits mailing list