[PATCH] D84209: [llvm-libtool-darwin] Add support for -D and -U option

[James Henderson via Phabricator via llvm-commits]

jhenderson added inline comments.


================
Comment at: llvm/tools/llvm-libtool-darwin/llvm-libtool-darwin.cpp:59
+struct Config {
+  bool Deterministic; // Updated by 'D' and 'U' modifiers.
+};
----------------
How about `Deterministic = true` (or false, whichever is the preferred default)?


================
Comment at: llvm/tools/llvm-libtool-darwin/llvm-libtool-darwin.cpp:114
 static Error addChildMember(std::vector<NewArchiveMember> &Members,
-                            const object::Archive::Child &M) {
+                            const object::Archive::Child &M, Config C) {
   Expected<NewArchiveMember> NMOrErr =
----------------
`Config` is likely to grow, so it probably is best to pass it as a `const &`, here and everywhere else.


================
Comment at: llvm/tools/llvm-libtool-darwin/llvm-libtool-darwin.cpp:193-194
+                             "cannot specify both -D and -U flags");
+  else
+    C.Deterministic = NonDeterministicOption ? false : true;
+
----------------
Consider this instead, combined with the default initialiser:
```
else if (NonDeterminisitcOption)
  C.Deterministic = false;
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84209