akhuang updated this revision to Diff 347090.
akhuang marked an inline comment as done.
akhuang added a comment.
Use TempFile on both linux and windows
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D102736/new/
https://reviews.llvm.org/D102736
Files:
clang/include/clang/Frontend/CompilerInstance.h
clang/lib/Frontend/CompilerInstance.cpp
llvm/lib/Support/Path.cpp
llvm/lib/Support/Windows/Path.inc
Index: llvm/lib/Support/Windows/Path.inc
===================================================================
--- llvm/lib/Support/Windows/Path.inc
+++ llvm/lib/Support/Windows/Path.inc
@@ -560,11 +560,6 @@
return errc::permission_denied;
}
-static std::error_code rename_fd(int FromFD, const Twine &To) {
- HANDLE FromHandle = reinterpret_cast<HANDLE>(_get_osfhandle(FromFD));
- return rename_handle(FromHandle, To);
-}
-
std::error_code rename(const Twine &From, const Twine &To) {
// Convert to utf-16.
SmallVector<wchar_t, 128> WideFrom;
Index: llvm/lib/Support/Path.cpp
===================================================================
--- llvm/lib/Support/Path.cpp
+++ llvm/lib/Support/Path.cpp
@@ -1229,7 +1229,7 @@
auto H = reinterpret_cast<HANDLE>(_get_osfhandle(FD));
std::error_code RenameEC = setDeleteDisposition(H, false);
if (!RenameEC) {
- RenameEC = rename_fd(FD, Name);
+ RenameEC = rename_handle(H, Name);
// If rename failed because it's cross-device, copy instead
if (RenameEC ==
std::error_code(ERROR_NOT_SAME_DEVICE, std::system_category())) {
@@ -1261,7 +1261,6 @@
return errorCodeToError(EC);
}
FD = -1;
-
return errorCodeToError(RenameEC);
}
Index: clang/lib/Frontend/CompilerInstance.cpp
===================================================================
--- clang/lib/Frontend/CompilerInstance.cpp
+++ clang/lib/Frontend/CompilerInstance.cpp
@@ -704,29 +704,38 @@
void CompilerInstance::clearOutputFiles(bool EraseFiles) {
for (OutputFile &OF : OutputFiles) {
if (EraseFiles) {
- if (!OF.TempFilename.empty()) {
- llvm::sys::fs::remove(OF.TempFilename);
- continue;
+ if (OF.TempFile) {
+ if (llvm::Error E = OF.TempFile->discard())
+ consumeError(std::move(E));
}
if (!OF.Filename.empty())
llvm::sys::fs::remove(OF.Filename);
continue;
}
- if (OF.TempFilename.empty())
+ if (!OF.TempFile)
+ continue;
+
+ if (OF.TempFile->TmpName.empty()) {
+ if (llvm::Error E = OF.TempFile->discard())
+ consumeError(std::move(E));
continue;
+ }
// If '-working-directory' was passed, the output filename should be
// relative to that.
SmallString<128> NewOutFile(OF.Filename);
FileMgr->FixupRelativePath(NewOutFile);
- std::error_code EC = llvm::sys::fs::rename(OF.TempFilename, NewOutFile);
- if (!EC)
+
+ std::string ErrorMsg;
+ llvm::Error E = OF.TempFile->keep(NewOutFile);
+ if (!E)
continue;
+
getDiagnostics().Report(diag::err_unable_to_rename_temp)
- << OF.TempFilename << OF.Filename << EC.message();
+ << OF.TempFile->TmpName << OF.Filename << toString(std::move(E));
- llvm::sys::fs::remove(OF.TempFilename);
+ llvm::sys::fs::remove(OF.TempFile->TmpName);
}
OutputFiles.clear();
if (DeleteBuiltModules) {
@@ -808,7 +817,7 @@
}
}
- std::string TempFile;
+ Optional<llvm::sys::fs::TempFile> Temp;
if (UseTemporary) {
// Create a temporary file.
// Insert -%%%%%%%% before the extension (if any), and because some tools
@@ -820,25 +829,36 @@
TempPath += "-%%%%%%%%";
TempPath += OutputExtension;
TempPath += ".tmp";
- 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 (CreateMissingDirectories &&
- EC == llvm::errc::no_such_file_or_directory) {
- StringRef Parent = llvm::sys::path::parent_path(OutputPath);
- EC = llvm::sys::fs::create_directories(Parent);
- if (!EC) {
- EC = llvm::sys::fs::createUniqueFile(TempPath, fd, TempPath,
- Binary ? llvm::sys::fs::OF_None
- : llvm::sys::fs::OF_Text);
- }
- }
+ Expected<llvm::sys::fs::TempFile> ExpectedFile =
+ llvm::sys::fs::TempFile::create(TempPath);
+
+ llvm::Error E = handleErrors(
+ ExpectedFile.takeError(), [&](const llvm::ECError &E) -> llvm::Error {
+ std::error_code EC = E.convertToErrorCode();
+ if (CreateMissingDirectories &&
+ EC == llvm::errc::no_such_file_or_directory) {
+ StringRef Parent = llvm::sys::path::parent_path(OutputPath);
+ EC = llvm::sys::fs::create_directories(Parent);
+ if (!EC) {
+ ExpectedFile = llvm::sys::fs::TempFile::create(TempPath);
+ if (!ExpectedFile)
+ return llvm::errorCodeToError(
+ llvm::errc::no_such_file_or_directory);
+ }
+ }
+ return llvm::errorCodeToError(EC);
+ });
+
+ if (E) {
+ consumeError(std::move(E));
+ } else {
+ Temp = std::move(ExpectedFile.get());
+ TempPath = Temp->TmpName;
- if (!EC) {
- OS.reset(new llvm::raw_fd_ostream(fd, /*shouldClose=*/true));
- OSFile = TempFile = std::string(TempPath.str());
+ OS.reset(new llvm::raw_fd_ostream(Temp->FD, /*shouldClose=*/false,
+ Binary ? llvm::sys::fs::OF_None
+ : llvm::sys::fs::OF_Text));
+ OSFile = std::string(TempPath.str());
}
// If we failed to create the temporary, fallback to writing to the file
// directly. This handles the corner case where we cannot write to the
@@ -862,7 +882,7 @@
// Add the output file -- but don't try to remove "-", since this means we are
// using stdin.
OutputFiles.emplace_back(((OutputPath != "-") ? OutputPath : "").str(),
- std::move(TempFile));
+ std::move(Temp));
if (!Binary || OS->supportsSeeking())
return std::move(OS);
Index: clang/include/clang/Frontend/CompilerInstance.h
===================================================================
--- clang/include/clang/Frontend/CompilerInstance.h
+++ clang/include/clang/Frontend/CompilerInstance.h
@@ -22,6 +22,7 @@
#include "llvm/ADT/IntrusiveRefCntPtr.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/Support/BuryPointer.h"
+#include "llvm/Support/FileSystem.h"
#include <cassert>
#include <list>
#include <memory>
@@ -165,11 +166,10 @@
/// failed.
struct OutputFile {
std::string Filename;
- std::string TempFilename;
+ Optional<llvm::sys::fs::TempFile> TempFile;
- OutputFile(std::string filename, std::string tempFilename)
- : Filename(std::move(filename)), TempFilename(std::move(tempFilename)) {
- }
+ OutputFile(std::string filename, Optional<llvm::sys::fs::TempFile> tempFile)
+ : Filename(std::move(filename)), TempFile(std::move(tempFile)) {}
};
/// The list of active output files.
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
