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