jdoerfert added a comment. Can we test this, or is it tested with a follow up commit at least? If so, which one (add to commit message as well, e.g., tests contained in D...). Some notes, exclusively on the unused do not strip flag. Everything else looks good reading it.
================ Comment at: clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp:188 + return createFileError(TempFile, EC); + + SmallVector<StringRef, 8> StripArgs; ---------------- ``` if (!StripSections) return static_cast<std::string>(TempFile); ``` ================ Comment at: clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp:258 + if (ToBeDeleted.empty()) + return None; + ---------------- if (!StripSections) return None; ================ Comment at: clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp:435 + Arg = **NewFileOrErr; + } + } ---------------- Does this work with the "do not strip option"? I somehow doubt it and would expect an error. ================ Comment at: clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp:445 // TODO: Wrap device image in a host binary and pass it to the linker. WithColor::warning(errs(), argv[0]) << "Offload linking not yet supported.\n"; ---------------- I know this is not new but I don't understand the warning here. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116545/new/ https://reviews.llvm.org/D116545 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
