[PATCH] D102736: Fix tmp files being left on Windows builds.
Alexandre Ganea via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Fri May 21 05:50:15 PDT 2021
aganea added inline comments.
================
Comment at: clang/include/clang/Frontend/CompilerInstance.h:170
std::string TempFilename;
+ Optional<llvm::sys::fs::TempFile> TempFile;
----------------
Can we always use `TempFile`? And remove `TempFilename` in that case?
================
Comment at: clang/lib/Frontend/CompilerInstance.cpp:728
+
+ std::string ErrorMsg;
+ if (OF.TempFile) {
----------------
I think the idea overall in LLVM is to defer the error rendering as late as possible. You could probably only keep an `Error` variable here, and render it below in the diagnostic with `toString(E)`.
================
Comment at: clang/lib/Frontend/CompilerInstance.cpp:733
+ } else {
+ if (std::error_code EC =
+ llvm::sys::fs::rename(OF.TempFilename, NewOutFile))
----------------
Here you can probably use `ECError` or `errorCodeToError()` with what is said slightly above.
================
Comment at: clang/lib/Frontend/CompilerInstance.cpp:840
int fd;
- std::error_code EC = llvm::sys::fs::createUniqueFile(
- TempPath, fd, TempPath,
- Binary ? llvm::sys::fs::OF_None : llvm::sys::fs::OF_Text);
+#if _WIN32
+ // On Windows, use llvm::sys::fs::TempFile to write the output file so
----------------
Usually it is better to avoid platform-specific logic. Could we use `TempFile` all the time on all platforms?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D102736/new/
https://reviews.llvm.org/D102736
More information about the cfe-commits
mailing list