[PATCH] D100139: [ifs][elfabi] Merge llvm-ifs/elfabi tools

[Petr Hosek via Phabricator via cfe-commits]

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

LGTM



================
Comment at: clang/lib/Driver/ToolChains/InterfaceStubs.cpp:18
 namespace ifstool {
 void Merger::ConstructJob(Compilation &C, const JobAction &JA,
                           const InputInfo &Output, const InputInfoList &Inputs,
----------------
Ideally, this job would use the IFS library to construct `.ifs` or stub file directly without invoking an external tool. Can you please leave a `TODO` here along those lines?


================
Comment at: llvm/tools/llvm-ifs/llvm-ifs.cpp:47
+} // end anonymous namespace
 
+// Command line flags:
----------------
Ideally we would use `OptTable` for option parsing, can you please leave a `TODO` comment here along those lines?


================
Comment at: llvm/tools/llvm-ifs/llvm-ifs.cpp:244-245
+            MemoryBuffer::getFile(FilePath)) {
+      // Compare IFS output with existing IFS file.
+      // If IFS file unchanged, abort updating.
+      if ((*BufOrError)->getBuffer() == IFSStr)
----------------



================
Comment at: llvm/tools/llvm-ifs/llvm-ifs.cpp:414
+      IFSTarget HintTarget = parseTriple(OptTargetTripleHint);
+      if (Stub.Target.Arch.getValue() != HintTarget.Arch.getValue()) {
+        fatalError(make_error<StringError>(
----------------
No need for `{` and `}` here and below since each `if` has only one statement.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100139