rnk added a comment. Nice, this seems to be working out.
================ Comment at: clang/include/clang/Frontend/CompilerInstance.h:170 std::string TempFilename; + Optional<llvm::sys::fs::TempFile> TempFile; ---------------- aganea wrote: > Can we always use `TempFile`? And remove `TempFilename` in that case? Having a field and type both named `TempFile` is setting off my portability alarms. I think some old versions of GCC don't like this. Maybe the field can be called `Temporary`? `TemporaryFile`? I dono. ================ Comment at: clang/lib/Frontend/CompilerInstance.cpp:708-709 + if (OF.TempFile) { + if (llvm::Error E = OF.TempFile->discard()) + consumeError(std::move(E)); } ---------------- Can this be `consumeError(OF.TempFile->discard())`? I find `consumeError` hard to read, I wish it were `ignoreErrors` or something like that. Short of that, please add a comment at the top of this function that we ignore any errors about failing to remove files. ================ Comment at: clang/lib/Frontend/CompilerInstance.cpp:720-721 + if (OF.TempFile->TmpName.empty()) { + if (llvm::Error E = OF.TempFile->discard()) + consumeError(std::move(E)); continue; ---------------- Ditto re consumeError simplification ================ Comment at: clang/lib/Frontend/CompilerInstance.cpp:730 + + std::string ErrorMsg; + llvm::Error E = OF.TempFile->keep(NewOutFile); ---------------- `ErrorMsg` seems unused now ================ Comment at: clang/lib/Frontend/CompilerInstance.cpp:736 getDiagnostics().Report(diag::err_unable_to_rename_temp) - << OF.TempFilename << OF.Filename << EC.message(); + << OF.TempFile->TmpName << OF.Filename << toString(std::move(E)); ---------------- Can you skip the toString and just do `<< std::move(E)`? I see an operator<< overload for it: https://github.com/llvm/llvm-project/blob/main/clang/include/clang/Basic/Diagnostic.h#L1518 ================ Comment at: llvm/lib/Support/Path.cpp:1264 FD = -1; - return errorCodeToError(RenameEC); ---------------- Please revert unrelated whitespace changes Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D102736/new/ https://reviews.llvm.org/D102736 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits