Add a new helper, atomicallyWritingToFile, and use it

This replaces the use of a Clang utility function that was
inexplicably a non-static member function of CompilerInstance. It
would be nice to sink this all the way to LLVM and share the
implementation across both projects, but the Clang implementation does
a handful of things we don't need, and it's hard to justify including
them in an LLVM-level interface. (I stared at
llvm/Support/FileSystem.h for a long time before giving up.)

Anyway, Serialization and FrontendTool both get their atomic writes
now without depending on Clang, and without duplicating the
scaffolding around the Clang API. We should probably adopt this for
all our output files.

No functionality change.

include/swift/Basic/FileSystem.h

@@ -2,7 +2,7 @@
 //
 // This source file is part of the Swift.org open source project
 //
-// Copyright (c) 2014 - 2017 Apple Inc. and the Swift project authors
+// Copyright (c) 2014 - 2018 Apple Inc. and the Swift project authors
 // Licensed under Apache License v2.0 with Runtime Library Exception
 //
 // See https://swift.org/LICENSE.txt for license information
@@ -13,10 +13,16 @@
 #ifndef SWIFT_BASIC_FILESYSTEM_H
 #define SWIFT_BASIC_FILESYSTEM_H
 
-#include "llvm/ADT/Twine.h"
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/StringRef.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include <system_error>
 
+namespace llvm {
+  class raw_pwrite_stream;
+  class Twine;
+}
+
 namespace clang {
   namespace vfs {
     class FileSystem;
@@ -24,6 +30,24 @@ namespace clang {
 }
 
 namespace swift {
+  /// Invokes \p action with a raw_ostream that refers to a temporary file,
+  /// which is then renamed into place as \p outputPath when the action
+  /// completes.
+  ///
+  /// If a temporary file cannot be created for whatever reason, \p action will
+  /// be invoked with a stream directly opened at \p outputPath. Otherwise, if
+  /// there is already a file at \p outputPath, it will not be overwritten if
+  /// the new contents are identical.
+  ///
+  /// If the process is interrupted with a signal, any temporary file will be
+  /// removed.
+  ///
+  /// As a special case, an output path of "-" is treated as referring to
+  /// stdout.
+  std::error_code atomicallyWritingToFile(
+      llvm::StringRef outputPath, bool binaryMode,
+      llvm::function_ref<void(llvm::raw_pwrite_stream &)> action);
+
   /// Moves a file from \p source to \p destination, unless there is already
   /// a file at \p destination that contains the same data as \p source.
   ///
@@ -38,6 +62,7 @@ namespace swift {
                    const llvm::Twine &Name, int64_t FileSize = -1,
                    bool RequiresNullTerminator = true, bool IsVolatile = false);
   } // end namespace vfs
+
 } // end namespace swift
 
 #endif // SWIFT_BASIC_FILESYSTEM_H

lib/Basic/FileSystem.cpp

@@ -2,7 +2,7 @@
 //
 // This source file is part of the Swift.org open source project
 //
-// Copyright (c) 2014 - 2017 Apple Inc. and the Swift project authors
+// Copyright (c) 2014 - 2018 Apple Inc. and the Swift project authors
 // Licensed under Apache License v2.0 with Runtime Library Exception
 //
 // See https://swift.org/LICENSE.txt for license information
@@ -11,9 +11,15 @@
 //===----------------------------------------------------------------------===//
 
 #include "swift/Basic/FileSystem.h"
-#include "llvm/Support/FileSystem.h"
+
-#include "llvm/Support/Process.h"
+#include "swift/Basic/LLVM.h"
 #include "clang/Basic/FileManager.h"
+#include "llvm/ADT/Twine.h"
+#include "llvm/Support/Errc.h"
+#include "llvm/Support/FileSystem.h"
+#include "llvm/Support/Path.h"
+#include "llvm/Support/Process.h"
+#include "llvm/Support/Signals.h"
 
 using namespace swift;
 
@@ -30,6 +36,137 @@ namespace {
   };
 } // end anonymous namespace
 
+/// Does some simple checking to see if a temporary file can be written next to
+/// \p outputPath and then renamed into place.
+///
+/// Helper for swift::atomicallyWritingToFile.
+///
+/// If the result is an error, the write won't succeed at all, and the calling
+/// operation should bail out early.
+static llvm::ErrorOr<bool>
+canUseTemporaryForWrite(const StringRef outputPath) {
+  namespace fs = llvm::sys::fs;
+
+  if (outputPath == "-") {
+    // Special case: "-" represents stdout, and LLVM's output stream APIs are
+    // aware of this. It doesn't make sense to use a temporary in this case.
+    return false;
+  }
+
+  fs::file_status status;
+  (void)fs::status(outputPath, status);
+  if (!fs::exists(status)) {
+    // Assume we'll be able to write to both a temporary file and to the final
+    // destination if the final destination doesn't exist yet.
+    return true;
+  }
+
+  // Fail early if we can't write to the final destination.
+  if (!fs::can_write(outputPath))
+    return llvm::make_error_code(llvm::errc::operation_not_permitted);
+
+  // Only use a temporary if the output is a regular file. This handles
+  // things like '-o /dev/null'
+  return fs::is_regular_file(status);
+}
+
+/// Attempts to open a temporary file next to \p outputPath, with the intent
+/// that once the file has been written it will be renamed into place.
+///
+/// Helper for swift::atomicallyWritingToFile.
+///
+/// \param[out] openedStream On success, a stream opened for writing to the
+/// temporary file that was just created.
+/// \param outputPath The path to the final output file, which is used to decide
+/// where to put the temporary.
+/// \param openFlags Controls how the output stream will be opened.
+///
+/// \returns The path to the temporary file that was opened, or \c None if the
+/// file couldn't be created.
+static Optional<std::string>
+tryToOpenTemporaryFile(Optional<llvm::raw_fd_ostream> &openedStream,
+                       const StringRef outputPath,
+                       const llvm::sys::fs::OpenFlags openFlags) {
+  namespace fs = llvm::sys::fs;
+
+  // Create a temporary file path.
+  // Insert a placeholder for a random suffix before the extension (if any).
+  // Then because some tools glob for build artifacts (such as clang's own
+  // GlobalModuleIndex.cpp), also append .tmp.
+  SmallString<128> tempPath;
+  const StringRef outputExtension = llvm::sys::path::extension(outputPath);
+  tempPath = outputPath.drop_back(outputExtension.size());
+  tempPath += "-%%%%%%%%";
+  tempPath += outputExtension;
+  tempPath += ".tmp";
+
+  int fd;
+  const unsigned perms = fs::all_read | fs::all_write;
+  std::error_code EC = fs::createUniqueFile(tempPath, fd, tempPath, perms,
+                                            openFlags);
+
+  if (EC) {
+    // Ignore the specific error; the caller has to fall back to not using a
+    // temporary anyway.
+    return None;
+  }
+
+  openedStream.emplace(fd, /*shouldClose=*/true);
+  // Make sure the temporary file gets removed if we crash.
+  llvm::sys::RemoveFileOnSignal(tempPath);
+  return tempPath.str().str();
+}
+
+std::error_code swift::atomicallyWritingToFile(
+    const StringRef outputPath, const bool binaryMode,
+    const llvm::function_ref<void(llvm::raw_pwrite_stream &)> action) {
+  namespace fs = llvm::sys::fs;
+
+  // FIXME: This is mostly a simplified version of
+  // clang::CompilerInstance::createOutputFile. It would be great to share the
+  // implementation.
+  assert(!outputPath.empty());
+
+  llvm::ErrorOr<bool> canUseTemporary = canUseTemporaryForWrite(outputPath);
+  if (std::error_code error = canUseTemporary.getError())
+    return error;
+
+  Optional<std::string> temporaryPath;
+  {
+    const fs::OpenFlags openFlags = (binaryMode ? fs::F_None : fs::F_Text);
+
+    Optional<llvm::raw_fd_ostream> OS;
+    if (canUseTemporary.get()) {
+      temporaryPath = tryToOpenTemporaryFile(OS, outputPath, openFlags);
+
+      if (!temporaryPath) {
+        assert(!OS.hasValue());
+        // If we failed to create the temporary, fall back 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.hasValue()) {
+      std::error_code error;
+      OS.emplace(outputPath, error, openFlags);
+      if (error)
+        return error;
+    }
+
+    action(OS.getValue());
+    // In addition to scoping the use of 'OS', ending the scope here also
+    // ensures that it's been flushed (by destroying it).
+  }
+
+  if (!temporaryPath.hasValue()) {
+    // If we didn't use a temporary, we're done!
+    return std::error_code();
+  }
+
+  return swift::moveFileIfDifferent(temporaryPath.getValue(), outputPath);
+}
+
 std::error_code swift::moveFileIfDifferent(const llvm::Twine &source,
                                            const llvm::Twine &destination) {
   namespace fs = llvm::sys::fs;

lib/FrontendTool/FrontendTool.cpp

@@ -2,7 +2,7 @@
 //
 // This source file is part of the Swift.org open source project
 //
-// Copyright (c) 2014 - 2017 Apple Inc. and the Swift project authors
+// Copyright (c) 2014 - 2018 Apple Inc. and the Swift project authors
 // Licensed under Apache License v2.0 with Runtime Library Exception
 //
 // See https://swift.org/LICENSE.txt for license information
@@ -62,11 +62,7 @@
 #include "swift/Syntax/SyntaxNodes.h"
 #include "swift/TBDGen/TBDGen.h"
 
-// FIXME: We're just using CompilerInstance::createOutputFile.
-// This API should be sunk down to LLVM.
-#include "clang/Frontend/CompilerInstance.h"
 #include "clang/AST/ASTContext.h"
-#include "clang/APINotes/Types.h"
 
 #include "llvm/ADT/Statistic.h"
 #include "llvm/IR/LLVMContext.h"
@@ -313,76 +309,60 @@ static bool writeSIL(SILModule &SM, const PrimarySpecificPaths &PSPs,
                   PSPs.OutputFilename, opts.EmitSortedSIL);
 }
 
-/// Invokes \p action with a raw_ostream that refers to a temporary file, which
+/// A wrapper around swift::atomicallyWritingToFile that handles diagnosing any
-/// is then renamed into place as \p outputPath when the action completes.
+/// filesystem errors and ignores empty output paths.
 ///
-/// If a temporary file cannot be created for whatever reason, \p action will
+/// \returns true if there were any errors, either from the filesystem
-/// be invoked with a stream directly opened at \p outputPath. Otherwise, if
+/// operations or from \p action returning true.
-/// there is already a file at \p outputPath, it will not be overwritten if
-/// the new contents are identical.
-///
-/// If the process is interrupted with a signal, any temporary file will be
-/// removed.
-///
-/// As a special case, an output path of "-" is treated as referring to stdout.
-///
-/// If an error occurs, it is reported via \p diags.
 static bool atomicallyWritingToTextFile(
     StringRef outputPath, DiagnosticEngine &diags,
     llvm::function_ref<bool(llvm::raw_pwrite_stream &)> action) {
-  if (outputPath.empty())
+  assert(!outputPath.empty());
-    return false;
 
-  std::string tmpFilePath;
+  bool actionFailed = false;
-  bool hadError;
+  std::error_code EC =
-  {
+      swift::atomicallyWritingToFile(outputPath, /*binary*/false,
-    clang::CompilerInstance Clang;
+                                     [&](llvm::raw_pwrite_stream &out) {
-
+    actionFailed = action(out);
-    std::error_code EC;
+  });
-    std::unique_ptr<llvm::raw_pwrite_stream> out =
+  if (EC) {
-      Clang.createOutputFile(outputPath, EC,
+    diags.diagnose(SourceLoc(), diag::error_opening_output,
-                             /*Binary=*/false,
+                   outputPath, EC.message());
-                             /*RemoveFileOnSignal=*/true,
+    return true;
-                             /*BaseInput=*/"",
-                             llvm::sys::path::extension(outputPath),
-                             /*UseTemporary=*/true,
-                             /*CreateMissingDirectories=*/false,
-                             /*ResultPathName=*/nullptr,
-                             &tmpFilePath);
-
-    if (!out) {
-      diags.diagnose(SourceLoc(), diag::error_opening_output,
-                     tmpFilePath, EC.message());
-      return true;
-    }
-
-    hadError = action(*out);
   }
-
+  return actionFailed;
-  if (!tmpFilePath.empty()) {
-    if (auto EC = swift::moveFileIfDifferent(tmpFilePath, outputPath)) {
-      diags.diagnose(SourceLoc(), diag::error_opening_output,
-                     outputPath, EC.message());
-      return true;
-    }
-  }
-
-  return hadError;
 }
 
+/// Prints the Objective-C "generated header" interface for \p M to \p
+/// outputPath.
+///
+/// ...unless \p outputPath is empty, in which case it does nothing.
+///
+/// \returns true if there were any errors
+///
+/// \see swift::printAsObjC
 static bool printAsObjCIfNeeded(StringRef outputPath, ModuleDecl *M,
                                 StringRef bridgingHeader, bool moduleIsPublic) {
+  if (outputPath.empty())
+    return false;
   return atomicallyWritingToTextFile(outputPath, M->getDiags(),
-                                 [&](raw_ostream &out) -> bool {
+                                     [&](raw_ostream &out) -> bool {
     auto requiredAccess = moduleIsPublic ? AccessLevel::Public
                                          : AccessLevel::Internal;
     return printAsObjC(out, M, bridgingHeader, requiredAccess);
   });
 }
 
+/// Prints the stable textual interface for \p M to \p outputPath.
+///
+/// ...unless \p outputPath is empty, in which case it does nothing.
+///
+/// \returns true if there were any errors
 static bool printModuleInterfaceIfNeeded(StringRef outputPath, ModuleDecl *M) {
+  if (outputPath.empty())
+    return false;
   return atomicallyWritingToTextFile(outputPath, M->getDiags(),
-                                 [&](raw_ostream &out) -> bool {
+                                     [&](raw_ostream &out) -> bool {
     auto printOptions = PrintOptions::printTextualInterfaceFile();
     SmallVector<Decl *, 16> topLevelDecls;
     M->getTopLevelDecls(topLevelDecls);

lib/Serialization/Serialization.cpp

@@ -2,7 +2,7 @@
 //
 // This source file is part of the Swift.org open source project
 //
-// Copyright (c) 2014 - 2017 Apple Inc. and the Swift project authors
+// Copyright (c) 2014 - 2018 Apple Inc. and the Swift project authors
 // Licensed under Apache License v2.0 with Runtime Library Exception
 //
 // See https://swift.org/LICENSE.txt for license information
@@ -38,9 +38,7 @@
 #include "swift/Serialization/SerializationOptions.h"
 #include "swift/Strings.h"
 
-// FIXME: We're just using CompilerInstance::createOutputFile.
+#include "clang/Basic/Module.h"
-// This API should be sunk down to LLVM.
-#include "clang/Frontend/CompilerInstance.h"
 
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringExtras.h"
@@ -5085,44 +5083,15 @@ void Serializer::writeDocToStream(raw_ostream &os, ModuleOrSourceFile DC,
 static inline bool
 withOutputFile(ASTContext &ctx, StringRef outputPath,
                llvm::function_ref<void(raw_ostream &)> action){
-  namespace path = llvm::sys::path;
+  std::error_code EC = swift::atomicallyWritingToFile(outputPath,
-  clang::CompilerInstance Clang;
+                                                      /*binary*/true,
+                                                      action);
+  if (!EC)
+    return false;
 
-  std::string tmpFilePath;
+  ctx.Diags.diagnose(SourceLoc(), diag::error_opening_output,
-  {
+                     outputPath, EC.message());
-    std::error_code EC;
+  return true;
-    std::unique_ptr<llvm::raw_pwrite_stream> out =
-      Clang.createOutputFile(outputPath, EC,
-                             /*Binary=*/true,
-                             /*RemoveFileOnSignal=*/true,
-                             /*BaseInput=*/"",
-                             path::extension(outputPath),
-                             /*UseTemporary=*/true,
-                             /*CreateMissingDirectories=*/false,
-                             /*ResultPathName=*/nullptr,
-                             &tmpFilePath);
-
-    if (!out) {
-      StringRef problematicPath =
-          tmpFilePath.empty() ? outputPath : StringRef(tmpFilePath);
-      ctx.Diags.diagnose(SourceLoc(), diag::error_opening_output,
-                         problematicPath, EC.message());
-      return true;
-    }
-
-    action(*out);
-  }
-
-  if (!tmpFilePath.empty()) {
-    std::error_code EC = swift::moveFileIfDifferent(tmpFilePath, outputPath);
-    if (EC) {
-      ctx.Diags.diagnose(SourceLoc(), diag::error_opening_output,
-                         outputPath, EC.message());
-      return true;
-    }
-  }
-
-  return false;
 }
 
 void swift::serialize(ModuleOrSourceFile DC,