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