[PATCH] D108850: [LLD] Remove global state in lldCommon

[Fangrui Song via Phabricator via llvm-commits]

MaskRay accepted this revision as: MaskRay.
MaskRay added a comment.

LGTM.



================
Comment at: lld/tools/lld/lld.cpp:150
+      return mingw::link;
+    else if (f == Gnu)
+      return elf::link;
----------------
https://llvm.org/docs/CodingStandards.html#use-early-exits-and-continue-to-simplify-code


================
Comment at: lld/tools/lld/lld.cpp:163
+  };
+  // Run the driver. If an error occurs, false will be returned.
+  bool r = link()(args, stdoutOS, stderrOS, exitEarly, inTestOutputDisabled);
----------------
`If an error occurs, link() return false.`

After `r` becomes `int`, `return` may be ambitious on whether it applies to the function or to the `r` variable.


================
Comment at: lld/tools/lld/lld.cpp:164
+  // Run the driver. If an error occurs, false will be returned.
+  bool r = link()(args, stdoutOS, stderrOS, exitEarly, inTestOutputDisabled);
+
----------------
You can make `r` an int and avoid repeated ` ? 1 : 0` below.


================
Comment at: lld/tools/lld/lld.cpp:166
+
+  // Call exit() if we can to avoid calling destructors.
+  if (exitEarly)
----------------
Call `_Exit` to skip destructors if exitEarly is true.

---

The original comment was

  // Exit immediately if we don't need to return to the caller.
  // This saves time because the overhead of calling destructors
  // for all globally-allocated objects is not negligible.

Is it worth copying some words to the new comment?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108850