[PATCH] D113127: [NFC][llvm-libtool-darwin] Refactor code

[James Henderson via Phabricator via llvm-commits]

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

Couple of small suggestions to improve the moved code whilst you're moving it. Otherwise, LGTM, though probably best wait for the later patches to be approved before landing this.



================
Comment at: llvm/tools/llvm-libtool-darwin/llvm-libtool-darwin.cpp:242-245
+    Expected<NewArchiveMember> NMOrErr =
+        NewArchiveMember::getFile(FileName, C.Deterministic);
+    if (!NMOrErr)
+      return createFileError(FileName, NMOrErr.takeError());
----------------
I'd probably avoid the "NM" acronym here, as I initially conflated that with the llvm-nm tool for some reason. I'd suggest just calling it `NewMemberOrErr` and then adding a `NewMember` variable immediately after the check which is just the dereferenced version, something like the inline edit. The second variable provides a shorter name, and also makes it clear that this has been checked already (since by the time you get to the end of this method, it's quite a long time since you did the check - someone reading the bit in isolation would have to go back and check it).


================
Comment at: llvm/tools/llvm-libtool-darwin/llvm-libtool-darwin.cpp:264-266
+    if (Error E = verifyAndAddMachOObject(std::move(*NMOrErr)))
+      return E;
     return Error::success();
----------------
You could simplify this to the inline edit.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113127