r185091 - Small improvements to createOutputFile.

[Rafael Espindola]

Author: rafael
Date: Thu Jun 27 13:26:26 2013
New Revision: 185091

URL: http://llvm.org/viewvc/llvm-project?rev=185091&view=rev
Log:
Small improvements to createOutputFile.

* Use a single stat to find out if the file exists and if it is a regular file.
* Use early returns when possible.
* Add comments explaining why we have each check.

Modified:
    cfe/trunk/include/clang/Frontend/CompilerInstance.h
    cfe/trunk/lib/Frontend/CompilerInstance.cpp
    cfe/trunk/test/Frontend/output-failures.c

Modified: cfe/trunk/include/clang/Frontend/CompilerInstance.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Frontend/CompilerInstance.h?rev=185091&r1=185090&r2=185091&view=diff
==============================================================================
--- cfe/trunk/include/clang/Frontend/CompilerInstance.h (original)
+++ cfe/trunk/include/clang/Frontend/CompilerInstance.h Thu Jun 27 13:26:26 2013
@@ -591,10 +591,10 @@ public:
   /// \return - Null on error.
   llvm::raw_fd_ostream *
   createOutputFile(StringRef OutputPath,
-                   bool Binary = true, bool RemoveFileOnSignal = true,
-                   StringRef BaseInput = "",
-                   StringRef Extension = "",
-                   bool UseTemporary = false,
+                   bool Binary, bool RemoveFileOnSignal,
+                   StringRef BaseInput,
+                   StringRef Extension,
+                   bool UseTemporary,
                    bool CreateMissingDirectories = false);
 
   /// Create a new output file, optionally deriving the output path name.
@@ -624,13 +624,13 @@ public:
   /// will be stored here on success.
   static llvm::raw_fd_ostream *
   createOutputFile(StringRef OutputPath, std::string &Error,
-                   bool Binary = true, bool RemoveFileOnSignal = true,
-                   StringRef BaseInput = "",
-                   StringRef Extension = "",
-                   bool UseTemporary = false,
-                   bool CreateMissingDirectories = false,
-                   std::string *ResultPathName = 0,
-                   std::string *TempPathName = 0);
+                   bool Binary, bool RemoveFileOnSignal,
+                   StringRef BaseInput,
+                   StringRef Extension,
+                   bool UseTemporary,
+                   bool CreateMissingDirectories,
+                   std::string *ResultPathName,
+                   std::string *TempPathName);
 
   /// }
   /// @name Initialization Utility Methods

Modified: cfe/trunk/lib/Frontend/CompilerInstance.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/CompilerInstance.cpp?rev=185091&r1=185090&r2=185091&view=diff
==============================================================================
--- cfe/trunk/lib/Frontend/CompilerInstance.cpp (original)
+++ cfe/trunk/lib/Frontend/CompilerInstance.cpp Thu Jun 27 13:26:26 2013
@@ -519,34 +519,50 @@ CompilerInstance::createOutputFile(Strin
   OwningPtr<llvm::raw_fd_ostream> OS;
   std::string OSFile;
 
-  if (UseTemporary && OutFile != "-") {
-    // Only create the temporary if the parent directory exists (or create
-    // missing directories is true) and we can actually write to OutPath,
-    // otherwise we want to fail early.
+  if (UseTemporary) {
+    if (OutFile == "-")
+      UseTemporary = false;
+    else {
+      llvm::sys::fs::file_status Status;
+      llvm::sys::fs::status(OutputPath, Status);
+      if (llvm::sys::fs::exists(Status)) {
+        // Fail early if we can't write to the final destination.
+        if (!llvm::sys::fs::can_write(OutputPath))
+          return 0;
+
+        // Don't use a temporary if the output is a special file. This handles
+        // things like '-o /dev/null'
+        if (!llvm::sys::fs::is_regular_file(Status))
+          UseTemporary = false;
+      }
+    }
+  }
+
+  if (UseTemporary) {
     SmallString<256> AbsPath(OutputPath);
     llvm::sys::fs::make_absolute(AbsPath);
-    StringRef OutPath = AbsPath;
-    bool ParentExists = false;
-    if (llvm::sys::fs::exists(llvm::sys::path::parent_path(AbsPath.str()),
-                              ParentExists))
-      ParentExists = false;
-    bool Exists;
-    if ((CreateMissingDirectories || ParentExists) &&
-        ((llvm::sys::fs::exists(AbsPath.str(), Exists) || !Exists) ||
-         (llvm::sys::fs::is_regular_file(OutPath) &&
-          llvm::sys::fs::can_write(AbsPath.c_str())))) {
-      // Create a temporary file.
-      SmallString<128> TempPath;
-      TempPath = OutFile;
-      TempPath += "-%%%%%%%%";
-      int fd;
-      if (llvm::sys::fs::unique_file(TempPath.str(), fd, TempPath,
-                                     /*makeAbsolute=*/false, 0664)
-          == llvm::errc::success) {
-        OS.reset(new llvm::raw_fd_ostream(fd, /*shouldClose=*/true));
-        OSFile = TempFile = TempPath.str();
-      }
+
+    // If the parent directory doesn't exist and we can't create it, fail.
+    bool ParentExists =
+        llvm::sys::fs::exists(llvm::sys::path::parent_path(AbsPath.str()));
+    if (!CreateMissingDirectories && !ParentExists) {
+      Error = "Parent directory doesn't exist";
+      return 0;
+    }
+
+    // Create a temporary file.
+    SmallString<128> TempPath;
+    TempPath = OutFile;
+    TempPath += "-%%%%%%%%";
+    int fd;
+    if (!llvm::sys::fs::unique_file(TempPath.str(), fd, TempPath,
+                                   /*makeAbsolute=*/false, 0664)) {
+      OS.reset(new llvm::raw_fd_ostream(fd, /*shouldClose=*/true));
+      OSFile = TempFile = 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
+    // directory, but can write to the file.
   }
 
   if (!OS) {

Modified: cfe/trunk/test/Frontend/output-failures.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Frontend/output-failures.c?rev=185091&r1=185090&r2=185091&view=diff
==============================================================================
--- cfe/trunk/test/Frontend/output-failures.c (original)
+++ cfe/trunk/test/Frontend/output-failures.c Thu Jun 27 13:26:26 2013
@@ -1,4 +1,4 @@
 // RUN: not %clang_cc1 -emit-llvm -o %S/doesnotexist/somename %s 2> %t
 // RUN: FileCheck -check-prefix=OUTPUTFAIL -input-file=%t %s
 
-// OUTPUTFAIL: Error opening output file '{{.*}}doesnotexist{{.*}}'
+// OUTPUTFAIL:  unable to open output file '{{.*}}doesnotexist{{.*}}': 'Parent directory doesn't exist'