[PATCH] D83520: [llvm-libtool-darwin] Allow flattening archives

[Shoaib Meenai via Phabricator via llvm-commits]

smeenai added inline comments.


================
Comment at: llvm/tools/llvm-libtool-darwin/llvm-libtool-darwin.cpp:97-102
+    Error Err = Error::success();
+    for (const object::Archive::Child &Child : Lib.children(Err))
+      if (Error E = addChildMember(Members, Child))
+        return createFileError(FileName, std::move(E));
+    if (Err)
+      return createFileError(FileName, std::move(Err));
----------------
smeenai wrote:
> Thinking through this a little more, `Err` is passed to the iterator to indicate iteration issues, so you should be checking it inside the loop in each iteration (probably before you call `addChildMember`). You still need to do an explicit check of `Err` afterward, so that in the case where the loop didn't iterate at all, the Error is still considered checked (it's guaranteed to be a success in that case, but you still need to do an explicit boolean check to mark it as checked). Perhaps the following would suffice for that? I'm not sure what pattern other parts of the codebase use for this.
> 
> ```
> static_cast<bool>(Err);
> ```
Ah, never mind. The iteration will stop in case of an error, so having the error check after the loop is perfectly fine. (See llvm/include/llvm/ADT/fallible_iterator.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83520