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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits