[PATCH] D100139: [ifs][elfabi] Merge llvm-ifs/elfabi tools
Petr Hosek via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Sun Jun 13 16:09:51 PDT 2021
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
More information about the cfe-commits
mailing list