---

yaml
---
r: 349505
b: refs/heads/master-next
c: 86e95c2
h: refs/heads/master
i:
  349503: 50ed6e1

Author: swift-ci

[refs]

@@ -1,6 +1,6 @@
 ---
 refs/heads/master: 3574c513bbc5578dd9346b4ea9ab5995c5927bb5
-refs/heads/master-next: e3185144862ce63bac34f2b0b2bd8d9a0e579cff
+refs/heads/master-next: 86e95c23aa0d37f24ec138b7853146c1cead2e40
 refs/tags/osx-passed: b6b74147ef8a386f532cf9357a1bde006e552c54
 refs/tags/swift-2.2-SNAPSHOT-2015-12-01-a: 6bb18e013c2284f2b45f5f84f2df2887dc0f7dea
 refs/tags/swift-2.2-SNAPSHOT-2015-12-01-b: 66d897bfcf64a82cb9a87f5e663d889189d06d07

branches/master-next/include/swift/AST/IRGenOptions.h

@@ -26,6 +26,7 @@
 // FIXME: This include is just for llvm::SanitizerCoverageOptions. We should
 // split the header upstream so we don't include so much.
 #include "llvm/Transforms/Instrumentation.h"
+#include "llvm/Support/raw_ostream.h"
 #include "llvm/Support/VersionTuple.h"
 #include <string>
 #include <vector>
@@ -253,13 +254,18 @@ class IRGenOptions {
         SanitizeCoverage(llvm::SanitizerCoverageOptions()),
         TypeInfoFilter(TypeInfoDumpFilter::All) {}
 
-  // Get a hash of all options which influence the llvm compilation but are not
-  // reflected in the llvm module itself.
-  unsigned getLLVMCodeGenOptionsHash() {
-    unsigned Hash = (unsigned)OptMode;
-    Hash = (Hash << 1) | DisableLLVMOptzns;
-    Hash = (Hash << 1) | DisableSwiftSpecificLLVMOptzns;
-    return Hash;
+  /// Appends to \p os an arbitrary string representing all options which
+  /// influence the llvm compilation but are not reflected in the llvm module
+  /// itself.
+  void writeLLVMCodeGenOptionsTo(llvm::raw_ostream &os) {
+    // We put a letter between each value simply to keep them from running into
+    // one another. There might be a vague correspondence between meaning and
+    // letter, but don't sweat it.
+    os << 'O' << (unsigned)OptMode
+       << 'd' << DisableLLVMOptzns
+       << 'D' << DisableSwiftSpecificLLVMOptzns
+       << 'p' << GenerateProfile
+       << 's' << Sanitizers.toRaw();
   }
 
   /// Should LLVM IR value names be emitted and preserved?

branches/master-next/include/swift/Basic/FileSystem.h

@@ -56,6 +56,26 @@ namespace swift {
   std::error_code moveFileIfDifferent(const llvm::Twine &source,
                                       const llvm::Twine &destination);
 
+  enum class FileDifference : uint8_t {
+    /// The source and destination paths refer to the exact same file.
+    IdenticalFile,
+    /// The source and destination paths refer to separate files with identical
+    /// contents.
+    SameContents,
+    /// The source and destination paths refer to separate files with different
+    /// contents.
+    DifferentContents
+  };
+
+  /// Compares the files at \p source and \p destination to determine if they
+  /// are the exact same files, different files with the same contents, or
+  /// different files with different contents. If \p allowDestinationErrors is
+  /// set, file system errors relating to the \p destination file return a
+  /// \c DifferentFile result, rather than an error.
+  llvm::ErrorOr<FileDifference>
+  areFilesDifferent(const llvm::Twine &source, const llvm::Twine &destination,
+                    bool allowDestinationErrors);
+
   namespace vfs {
     llvm::ErrorOr<std::unique_ptr<llvm::MemoryBuffer>>
     getFileOrSTDIN(llvm::vfs::FileSystem &FS,

branches/master-next/lib/Basic/FileSystem.cpp

@@ -163,13 +163,14 @@ std::error_code swift::atomicallyWritingToFile(
   return swift::moveFileIfDifferent(temporaryPath.getValue(), outputPath);
 }
 
-std::error_code swift::moveFileIfDifferent(const llvm::Twine &source,
-                                           const llvm::Twine &destination) {
+llvm::ErrorOr<FileDifference>
+swift::areFilesDifferent(const llvm::Twine &source,
+                         const llvm::Twine &destination,
+                         bool allowDestinationErrors) {
   namespace fs = llvm::sys::fs;
 
-  // First check for a self-move.
   if (fs::equivalent(source, destination))
-    return std::error_code();
+    return FileDifference::IdenticalFile;
 
   OpenFileRAII sourceFile;
   fs::file_status sourceStatus;
@@ -182,45 +183,80 @@ std::error_code swift::moveFileIfDifferent(const llvm::Twine &source,
     return error;
   }
 
+  /// Converts an error from the destination file into either an error or
+  /// DifferentContents return, depending on `allowDestinationErrors`.
+  auto convertDestinationError = [=](std::error_code error) ->
+      llvm::ErrorOr<FileDifference> {
+    if (allowDestinationErrors)
+      return FileDifference::DifferentContents;
+    return error;
+  };
+
   OpenFileRAII destFile;
   fs::file_status destStatus;
-  bool couldReadDest = !fs::openFileForRead(destination, destFile.fd);
-  if (couldReadDest)
-    couldReadDest = !fs::status(destFile.fd, destStatus);
-
-  // If we could read the destination file, and it matches the source file in
-  // size, they may be the same. Do an actual comparison of the contents.
-  if (couldReadDest && sourceStatus.getSize() == destStatus.getSize()) {
-    uint64_t size = sourceStatus.getSize();
-    bool same = false;
-    if (size == 0) {
-      same = true;
-    } else {
-      std::error_code sourceRegionErr;
-      fs::mapped_file_region sourceRegion(sourceFile.fd,
-                                          fs::mapped_file_region::readonly,
-                                          size, 0, sourceRegionErr);
-      if (sourceRegionErr)
-        return sourceRegionErr;
-
-      std::error_code destRegionErr;
-      fs::mapped_file_region destRegion(destFile.fd,
-                                        fs::mapped_file_region::readonly,
-                                        size, 0, destRegionErr);
-
-      if (!destRegionErr) {
-        same = (0 == memcmp(sourceRegion.const_data(), destRegion.const_data(),
-                            size));
-      }
-    }
-
-    // If the file contents are the same, we are done. Just delete the source.
-    if (same)
-      return fs::remove(source);
+  if (std::error_code error = fs::openFileForRead(destination, destFile.fd)) {
+    // If we can't open the destination file, fail in the specified fashion.
+    return convertDestinationError(error);
   }
+  if (std::error_code error = fs::status(destFile.fd, destStatus)) {
+    // If we can't open the destination file, fail in the specified fashion.
+    return convertDestinationError(error);
+  }
+
+  uint64_t size = sourceStatus.getSize();
+  if (size != destStatus.getSize())
+    // If the files are different sizes, they must be different.
+    return FileDifference::DifferentContents;
+  if (size == 0)
+    // If both files are zero size, they must be the same.
+    return FileDifference::SameContents;
+
+  // The two files match in size, so we have to compare the bytes to determine
+  // if they're the same.
+  std::error_code sourceRegionErr;
+  fs::mapped_file_region sourceRegion(sourceFile.fd,
+                                      fs::mapped_file_region::readonly,
+                                      size, 0, sourceRegionErr);
+  if (sourceRegionErr)
+    return sourceRegionErr;
+
+  std::error_code destRegionErr;
+  fs::mapped_file_region destRegion(destFile.fd,
+                                    fs::mapped_file_region::readonly,
+                                    size, 0, destRegionErr);
+
+  if (destRegionErr)
+    return convertDestinationError(destRegionErr);
+
+  if (0 == memcmp(sourceRegion.const_data(), destRegion.const_data(), size))
+    return FileDifference::SameContents;
+
+  return FileDifference::DifferentContents;
+}
+
+std::error_code swift::moveFileIfDifferent(const llvm::Twine &source,
+                                           const llvm::Twine &destination) {
+  namespace fs = llvm::sys::fs;
+
+  auto result = areFilesDifferent(source, destination,
+                                  /*allowDestinationErrors=*/true);
 
-  // If we get here, we weren't able to prove that the files are the same.
-  return fs::rename(source, destination);
+  if (!result)
+    return result.getError();
+
+  switch (*result) {
+  case FileDifference::IdenticalFile:
+    // Do nothing for a self-move.
+    return std::error_code();
+
+  case FileDifference::SameContents:
+    // Files are identical; remove the source file.
+    return fs::remove(source);
+
+  case FileDifference::DifferentContents:
+    // Files are different; overwrite the destination file.
+    return fs::rename(source, destination);
+  }
 }
 
 llvm::ErrorOr<std::unique_ptr<llvm::MemoryBuffer>>

branches/master-next/lib/IRGen/IRGen.cpp

@@ -351,7 +351,7 @@ static void getHashOfModule(MD5::MD5Result &Result, IRGenOptions &Opts,
 
   // Add all options which influence the llvm compilation but are not yet
   // reflected in the llvm module itself.
-  HashStream << Opts.getLLVMCodeGenOptionsHash();
+  Opts.writeLLVMCodeGenOptionsTo(HashStream);
 
   HashStream.final(Result);
 }
@@ -431,6 +431,22 @@ static void countStatsPostIRGen(UnifiedStatsReporter &Stats,
   }
 }
 
+template<typename ...ArgTypes>
+void
+diagnoseSync(DiagnosticEngine *Diags, llvm::sys::Mutex *DiagMutex,
+             SourceLoc Loc, Diag<ArgTypes...> ID,
+             typename swift::detail::PassArgument<ArgTypes>::type... Args) {
+  if (!Diags)
+    return;
+  if (DiagMutex)
+    DiagMutex->lock();
+
+  Diags->diagnose(Loc, ID, std::move(Args)...);
+
+  if (DiagMutex)
+    DiagMutex->unlock();
+}
+
 /// Run the LLVM passes. In multi-threaded compilation this will be done for
 /// multiple LLVM modules in parallel.
 bool swift::performLLVM(IRGenOptions &Opts, DiagnosticEngine *Diags,
@@ -441,6 +457,18 @@ bool swift::performLLVM(IRGenOptions &Opts, DiagnosticEngine *Diags,
                         const version::Version &effectiveLanguageVersion,
                         StringRef OutputFilename,
                         UnifiedStatsReporter *Stats) {
+#ifndef NDEBUG
+  // To check that we only skip generating code when it would have no effect, in
+  // assertion builds we still generate the code, but write it into a temporary
+  // file that we compare to the original file.
+
+  /// The OutputFilename originally passed to us, if we are generating code for
+  /// an assertion. Empty if not.
+  StringRef OriginalOutputFilename = "";
+  /// Scratch buffer for temporary file's name.
+  SmallString<64> AssertScratch;
+#endif
+
   if (Opts.UseIncrementalLLVMCodeGen && HashGlobal) {
     // Check if we can skip the llvm part of the compilation if we have an
     // existing object file which was generated from the same llvm IR.
@@ -462,7 +490,24 @@ bool swift::performLLVM(IRGenOptions &Opts, DiagnosticEngine *Diags,
         !Opts.PrintInlineTree &&
         !needsRecompile(OutputFilename, HashData, HashGlobal, DiagMutex)) {
       // The llvm IR did not change. We don't need to re-create the object file.
+#ifdef NDEBUG
       return false;
+#else
+      // ...but we're in an asserts build, so we want to check that assumption.
+      auto AssertSuffix = llvm::sys::path::filename(OutputFilename);
+
+      auto EC = llvm::sys::fs::createTemporaryFile("assert", AssertSuffix,
+                                                   AssertScratch);
+      if (EC) {
+        diagnoseSync(Diags, DiagMutex,
+                     SourceLoc(), diag::error_opening_output,
+                     AssertScratch, EC.message());
+        return true;
+      }
+
+      OriginalOutputFilename = OutputFilename;
+      OutputFilename = AssertScratch;
+#endif
     }
 
     // Store the hash in the global variable so that it is written into the
@@ -480,14 +525,9 @@ bool swift::performLLVM(IRGenOptions &Opts, DiagnosticEngine *Diags,
     RawOS.emplace(OutputFilename, EC, OSFlags);
 
     if (RawOS->has_error() || EC) {
-      if (Diags) {
-        if (DiagMutex)
-          DiagMutex->lock();
-        Diags->diagnose(SourceLoc(), diag::error_opening_output,
-                        OutputFilename, EC.message());
-        if (DiagMutex)
-          DiagMutex->unlock();
-      }
+      diagnoseSync(Diags, DiagMutex,
+                   SourceLoc(), diag::error_opening_output,
+                   OutputFilename, EC.message());
       RawOS->clear_error();
       return true;
     }
@@ -529,13 +569,8 @@ bool swift::performLLVM(IRGenOptions &Opts, DiagnosticEngine *Diags,
     bool fail = TargetMachine->addPassesToEmitFile(EmitPasses, *RawOS, nullptr,
                                                    FileType, !Opts.Verify);
     if (fail) {
-      if (Diags) {
-        if (DiagMutex)
-          DiagMutex->lock();
-        Diags->diagnose(SourceLoc(), diag::error_codegen_init_fail);
-        if (DiagMutex)
-          DiagMutex->unlock();
-      }
+      diagnoseSync(Diags, DiagMutex,
+                   SourceLoc(), diag::error_codegen_init_fail);
       return true;
     }
     break;
@@ -559,6 +594,44 @@ bool swift::performLLVM(IRGenOptions &Opts, DiagnosticEngine *Diags,
     if (DiagMutex)
       DiagMutex->unlock();
   }
+
+#ifndef NDEBUG
+  if (!OriginalOutputFilename.empty()) {
+    // We're done changing the file; make sure it's saved before we compare.
+    RawOS->close();
+
+    auto result =
+        swift::areFilesDifferent(OutputFilename, OriginalOutputFilename,
+                                 /*allowDestinationErrors=*/false);
+
+    if (!result)
+      // File system error.
+      llvm::report_fatal_error(
+          Twine("Error comparing files: ") + result.getError().message()
+      );
+
+    switch (*result) {
+    case FileDifference::DifferentContents:
+      llvm::report_fatal_error(
+          "Swift skipped an LLVM compile that would have changed output; pass "
+          "-Xfrontend -disable-incremental-llvm-codegen to work around this bug"
+      );
+      // Note for future debuggers: If you see this error, either you changed
+      // LLVM and need to clean your build folder to rebuild everything with it,
+      // or IRGenOptions::writeLLVMCodeGenOptionsTo() doesn't account for a flag
+      // that changed LLVM's output between this compile and the previous one.
+
+    case FileDifference::SameContents:
+      // Removing the file is best-effort.
+      (void)llvm::sys::fs::remove(OutputFilename);
+      break;
+
+    case FileDifference::IdenticalFile:
+      llvm_unreachable("one of these should be a temporary file");
+    }
+  }
+#endif
+
   return false;
 }
 

branches/master-next/test/IRGen/module_hash.swift

@@ -31,6 +31,25 @@
 // CHECK: test.o: MD5=[[TEST3_MD5:[0-9a-f]+]]
 // CHECK: test.o: prev MD5=[[TEST2_MD5]] recompiling
 
+// RUN: echo "empty file" >>%t/log
+// RUN: %target-swift-frontend -c -primary-file %S/../Inputs/empty.swift -module-name=empty -o %t/empty.o -Xllvm -debug-only=irgen 2>>%t/log
+// RUN: echo "empty file with profiling" >>%t/log
+// RUN: %target-swift-frontend -c -primary-file %S/../Inputs/empty.swift -module-name=empty -o %t/empty.o -Xllvm -debug-only=irgen -profile-generate 2>>%t/log
+
+// CHECK-LABEL: empty file
+// CHECK: empty.o: MD5=[[EMPTY_MD5:[0-9a-f]+]]
+// CHECK-LABEL: empty file with profiling
+// CHECK: empty.o: prev MD5=[[EMPTY_MD5]] recompiling
+
+// RUN: echo "empty file" >>%t/log
+// RUN: %target-swift-frontend -c -primary-file %S/../Inputs/empty.swift -module-name=empty -o %t/empty.o -Xllvm -debug-only=irgen 2>>%t/log
+// RUN: echo "empty file with fuzzer" >>%t/log
+// RUN: %target-swift-frontend -c -primary-file %S/../Inputs/empty.swift -module-name=empty -o %t/empty.o -Xllvm -debug-only=irgen -sanitize=fuzzer 2>>%t/log
+
+// CHECK-LABEL: empty file
+// CHECK: empty.o: MD5=[[EMPTY_MD5_2:[0-9a-f]+]]
+// CHECK-LABEL: empty file with fuzzer
+// CHECK: empty.o: prev MD5=[[EMPTY_MD5_2]] recompiling
 
 // Test with multiple output files