[clang] 20797b1 - Revert "Fix tmp files being left on Windows builds." for now;

Amy Huang via cfe-commits cfe-commits at lists.llvm.org
Tue Jun 1 19:52:29 PDT 2021


Author: Amy Huang
Date: 2021-06-01T19:51:47-07:00
New Revision: 20797b129f844d4b12ffb2b12cf33baa2d42985c

URL: https://github.com/llvm/llvm-project/commit/20797b129f844d4b12ffb2b12cf33baa2d42985c
DIFF: https://github.com/llvm/llvm-project/commit/20797b129f844d4b12ffb2b12cf33baa2d42985c.diff

LOG: Revert "Fix tmp files being left on Windows builds." for now;
causing some asan test failures.

This reverts commit 7daa18215905c831e130c7542f17619e9d936dfc.

Added: 
    

Modified: 
    clang/include/clang/Frontend/CompilerInstance.h
    clang/lib/Frontend/CompilerInstance.cpp
    llvm/lib/Support/Path.cpp
    llvm/lib/Support/Windows/Path.inc

Removed: 
    


################################################################################
diff  --git a/clang/include/clang/Frontend/CompilerInstance.h b/clang/include/clang/Frontend/CompilerInstance.h
index 9d6dd8fa1006f..5e879d672d801 100644
--- a/clang/include/clang/Frontend/CompilerInstance.h
+++ b/clang/include/clang/Frontend/CompilerInstance.h
@@ -22,7 +22,6 @@
 #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>
@@ -166,10 +165,11 @@ class CompilerInstance : public ModuleLoader {
   /// failed.
   struct OutputFile {
     std::string Filename;
-    Optional<llvm::sys::fs::TempFile> File;
+    std::string TempFilename;
 
-    OutputFile(std::string filename, Optional<llvm::sys::fs::TempFile> file)
-        : Filename(std::move(filename)), File(std::move(file)) {}
+    OutputFile(std::string filename, std::string tempFilename)
+        : Filename(std::move(filename)), TempFilename(std::move(tempFilename)) {
+    }
   };
 
   /// The list of active output files.

diff  --git a/clang/lib/Frontend/CompilerInstance.cpp b/clang/lib/Frontend/CompilerInstance.cpp
index b1b983d620653..5df40c742d469 100644
--- a/clang/lib/Frontend/CompilerInstance.cpp
+++ b/clang/lib/Frontend/CompilerInstance.cpp
@@ -703,37 +703,31 @@ void CompilerInstance::createSema(TranslationUnitKind TUKind,
 // Output Files
 
 void CompilerInstance::clearOutputFiles(bool EraseFiles) {
-  // Ignore errors that occur when trying to discard the temp file.
   for (OutputFile &OF : OutputFiles) {
     if (EraseFiles) {
-      if (OF.File)
-        consumeError(OF.File->discard());
+      if (!OF.TempFilename.empty()) {
+        llvm::sys::fs::remove(OF.TempFilename);
+        continue;
+      }
       if (!OF.Filename.empty())
         llvm::sys::fs::remove(OF.Filename);
       continue;
     }
 
-    if (!OF.File)
+    if (OF.TempFilename.empty())
       continue;
 
-    if (OF.File->TmpName.empty()) {
-      consumeError(OF.File->discard());
-      continue;
-    }
-
     // If '-working-directory' was passed, the output filename should be
     // relative to that.
     SmallString<128> NewOutFile(OF.Filename);
     FileMgr->FixupRelativePath(NewOutFile);
-
-    llvm::Error E = OF.File->keep(NewOutFile);
-    if (!E)
+    std::error_code EC = llvm::sys::fs::rename(OF.TempFilename, NewOutFile);
+    if (!EC)
       continue;
-
     getDiagnostics().Report(diag::err_unable_to_rename_temp)
-        << OF.File->TmpName << OF.Filename << std::move(E);
+        << OF.TempFilename << OF.Filename << EC.message();
 
-    llvm::sys::fs::remove(OF.File->TmpName);
+    llvm::sys::fs::remove(OF.TempFilename);
   }
   OutputFiles.clear();
   if (DeleteBuiltModules) {
@@ -815,7 +809,7 @@ CompilerInstance::createOutputFileImpl(StringRef OutputPath, bool Binary,
     }
   }
 
-  Optional<llvm::sys::fs::TempFile> Temp;
+  std::string TempFile;
   if (UseTemporary) {
     // Create a temporary file.
     // Insert -%%%%%%%% before the extension (if any), and because some tools
@@ -827,36 +821,25 @@ CompilerInstance::createOutputFileImpl(StringRef OutputPath, bool Binary,
     TempPath += "-%%%%%%%%";
     TempPath += OutputExtension;
     TempPath += ".tmp";
-    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;
+    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);
+      }
+    }
 
-      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 (!EC) {
+      OS.reset(new llvm::raw_fd_ostream(fd, /*shouldClose=*/true));
+      OSFile = TempFile = 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
@@ -880,7 +863,7 @@ CompilerInstance::createOutputFileImpl(StringRef OutputPath, bool Binary,
   // 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(Temp));
+                           std::move(TempFile));
 
   if (!Binary || OS->supportsSeeking())
     return std::move(OS);

diff  --git a/llvm/lib/Support/Path.cpp b/llvm/lib/Support/Path.cpp
index d549b0011b1f4..005c27c3a42af 100644
--- a/llvm/lib/Support/Path.cpp
+++ b/llvm/lib/Support/Path.cpp
@@ -1229,7 +1229,7 @@ Error TempFile::keep(const Twine &Name) {
   auto H = reinterpret_cast<HANDLE>(_get_osfhandle(FD));
   std::error_code RenameEC = setDeleteDisposition(H, false);
   if (!RenameEC) {
-    RenameEC = rename_handle(H, Name);
+    RenameEC = rename_fd(FD, Name);
     // If rename failed because it's cross-device, copy instead
     if (RenameEC ==
       std::error_code(ERROR_NOT_SAME_DEVICE, std::system_category())) {

diff  --git a/llvm/lib/Support/Windows/Path.inc b/llvm/lib/Support/Windows/Path.inc
index daa69bf04b44e..19b9f9ef7cbce 100644
--- a/llvm/lib/Support/Windows/Path.inc
+++ b/llvm/lib/Support/Windows/Path.inc
@@ -560,6 +560,11 @@ static std::error_code rename_handle(HANDLE FromHandle, const Twine &To) {
   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;


        


More information about the cfe-commits mailing list