[Frontend] Make the module trace emission more robust.

✔ More informative error messages in case of crashes.
✔ Handling and documenting different cases.
✔ Test cases for different cases.
✔ Make SDKDependencies.swift pass again.

lib/FrontendTool/FrontendTool.cpp

@@ -237,12 +237,165 @@ template <> struct ObjectTraits<LoadedModuleTraceFormat> {
 }
 }
 
+/// Compute the per-module information to be recorded in the trace file.
+//
+// The most interesting/tricky thing here is _which_ paths get recorded in
+// the trace file as dependencies. It depends on how the module was synthesized.
+// The key points are:
+//
+// 1. Paths to swiftmodules in the module cache or in the prebuilt cache are not
+//    recorded - Precondition: the corresponding path to the swiftinterface must
+//    already be present as a key in pathToModuleDecl.
+// 2. swiftmodules next to a swiftinterface are saved if they are up-to-date.
+//
+// FIXME: Use the VFS instead of handling paths directly. We are particularly
+// sloppy about handling relative paths in the dependency tracker.
+static void computeSwiftModuleTraceInfo(
+    const SmallPtrSetImpl<ModuleDecl *> &importedModules,
+    const llvm::DenseMap<StringRef, ModuleDecl *> &pathToModuleDecl,
+    const DependencyTracker &depTracker,
+    StringRef prebuiltCachePath,
+    std::vector<SwiftModuleTraceInfo> &traceInfo) {
+
+  SmallString<256> buffer;
+
+  std::string errMsg;
+  llvm::raw_string_ostream err(errMsg);
+
+  // FIXME: Use PrettyStackTrace instead.
+  auto errorUnexpectedPath =
+      [&pathToModuleDecl](llvm::raw_string_ostream &errStream) {
+    errStream << "The module <-> path mapping we have is:\n";
+    for (auto &m: pathToModuleDecl)
+      errStream << m.second->getName() << " <-> " << m.first << '\n';
+    llvm::report_fatal_error(errStream.str());
+  };
+
+  using namespace llvm::sys;
+
+  auto computeAdjacentInterfacePath = [](SmallVectorImpl<char> &modPath) {
+    auto swiftInterfaceExt =
+      file_types::getExtension(file_types::TY_SwiftModuleInterfaceFile);
+    path::replace_extension(modPath, swiftInterfaceExt);
+  };
+
+  for (auto &depPath : depTracker.getDependencies()) {
+
+    // Decide if this is a swiftmodule based on the extension of the raw
+    // dependency path, as the true file may have a different one.
+    // For example, this might happen when the canonicalized path points to
+    // a Content Addressed Storage (CAS) location.
+    auto moduleFileType =
+      file_types::lookupTypeForExtension(path::extension(depPath));
+    auto isSwiftmodule =
+      moduleFileType == file_types::TY_SwiftModuleFile;
+    auto isSwiftinterface =
+      moduleFileType == file_types::TY_SwiftModuleInterfaceFile;
+
+    if (!(isSwiftmodule || isSwiftinterface))
+      continue;
+
+    auto dep = pathToModuleDecl.find(depPath);
+    if (dep != pathToModuleDecl.end()) {
+      // Great, we recognize the path! Check if the file is still around.
+
+      ModuleDecl *depMod = dep->second;
+      if(depMod->isResilient() && !isSwiftinterface) {
+        SmallString<256> moduleAdjacentInterfacePath(depPath);
+        computeAdjacentInterfacePath(moduleAdjacentInterfacePath);
+        // FIXME: The behavior of fs::exists for relative paths is undocumented.
+        // Use something else instead?
+        if (!fs::exists(moduleAdjacentInterfacePath)) {
+          err << "The module " << depMod->getName().str() << " has library"
+              << " evolution enabled but we're recording a non-adjacent"
+              << " swiftmodule at\n" << depPath << "\nin the trace.";
+          llvm::report_fatal_error(err.str());
+        }
+        buffer.clear();
+      }
+
+      // FIXME: Better error handling
+      StringRef realDepPath
+        = fs::real_path(depPath, buffer, /*expand_tile*/true)
+        ? StringRef(depPath) // Couldn't find the canonical path, assume
+                             // this is good enough.
+        : buffer.str();
+
+      traceInfo.push_back({
+        /*Name=*/
+        depMod->getName(),
+        /*Path=*/
+        realDepPath,
+        // TODO: There is an edge case which is not handled here.
+        // When we build a framework using -import-underlying-module, or an
+        // app/test using -import-objc-header, we should look at the direct
+        // imports of the bridging modules, and mark those as our direct
+        // imports.
+        /*IsImportedDirectly=*/
+        importedModules.find(depMod) != importedModules.end(),
+        /*SupportsLibraryEvolution=*/
+        depMod->isResilient()
+      });
+      buffer.clear();
+
+      continue;
+    }
+
+    // If the depTracker had an interface, that means that we must've
+    // built a swiftmodule from that interface, so we should have that
+    // filename available.
+    if (isSwiftinterface) {
+      err << "Unexpected path for swiftinterface file:\n" << depPath << "\n";
+      errorUnexpectedPath(err);
+    }
+
+    // Skip cached modules in the prebuilt cache. We will add the corresponding
+    // swiftinterface from the SDK directly, but this isn't checked. :-/
+    //
+    // FIXME: This is incorrect if both paths are not relative w.r.t. to the
+    // same root.
+    if (StringRef(depPath).startswith(prebuiltCachePath))
+      continue;
+
+    // If we have a swiftmodule next to an interface, that interface path will
+    // be saved (not checked), so don't save the path to this swiftmodule.
+    SmallString<256> moduleAdjacentInterfacePath(depPath);
+    computeAdjacentInterfacePath(moduleAdjacentInterfacePath);
+    if (pathToModuleDecl.find(moduleAdjacentInterfacePath)
+        != pathToModuleDecl.end())
+      continue;
+
+    // FIXME: The behavior of fs::exists for relative paths is undocumented.
+    // Use something else instead?
+    if (fs::exists(moduleAdjacentInterfacePath)) {
+      err << "Found swiftinterface at\n" << moduleAdjacentInterfacePath
+          << "\nbut it was not recorded\n";
+      errorUnexpectedPath(err);
+    }
+    buffer.clear();
+
+    err << "Don't know how to handle the dependency at:\n" << depPath
+        << "\nfor module trace emission.\n";
+    errorUnexpectedPath(err);
+  }
+
+  // Almost a re-implementation of reversePathSortedFilenames :(.
+  std::sort(
+    traceInfo.begin(), traceInfo.end(),
+    [](const SwiftModuleTraceInfo &m1, const SwiftModuleTraceInfo &m2) -> bool {
+      return std::lexicographical_compare(
+        m1.Path.rbegin(), m1.Path.rend(),
+        m2.Path.rbegin(), m2.Path.rend());
+  });
+}
+
 static bool emitLoadedModuleTraceIfNeeded(ModuleDecl *mainModule,
                                           DependencyTracker *depTracker,
+                                          StringRef prebuiltCachePath,
                                           StringRef loadedModuleTracePath) {
   ASTContext &ctxt = mainModule->getASTContext();
   assert(!ctxt.hadError()
-         && "We may not be able to emit a proper trace if there was an error.");
+         && "We should've already exited earlier if there was an error.");
 
   if (loadedModuleTracePath.empty())
     return false;
@@ -269,67 +422,22 @@ static bool emitLoadedModuleTraceIfNeeded(ModuleDecl *mainModule,
   llvm::DenseMap<StringRef, ModuleDecl *> pathToModuleDecl;
   for (auto &module : ctxt.LoadedModules) {
     ModuleDecl *loadedDecl = module.second;
-    assert(loadedDecl && "Expected loaded module to be non-null.");
+    if (!loadedDecl)
+      llvm::report_fatal_error("Expected loaded modules to be non-null.");
     if (loadedDecl == mainModule)
       continue;
-    assert(!loadedDecl->getModuleFilename().empty()
-           && ("Don't know how to handle modules with empty names."
-               " One potential reason for getting an empty module name might"
-               " be that the module could not be deserialized correctly."));
+    if (loadedDecl->getModuleFilename().empty())
+      llvm::report_fatal_error(
+        "Don't know how to handle modules with empty names."
+        " One potential reason for getting an empty module name might"
+        " be that the module could not be deserialized correctly.");
     pathToModuleDecl.insert(
       std::make_pair(loadedDecl->getModuleFilename(), loadedDecl));
   }
 
   std::vector<SwiftModuleTraceInfo> swiftModules;
-  SmallString<256> buffer;
-  for (auto &depPath : depTracker->getDependencies()) {
-    StringRef realDepPath;
-    // FIXME: appropriate error handling
-    if (llvm::sys::fs::real_path(depPath, buffer,/*expand_tilde=*/true))
-      // Couldn't find the canonical path, so let's just assume the old one was
-      // canonical (enough).
-      realDepPath = depPath;
-    else
-      realDepPath = buffer.str();
-
-    // Decide if this is a swiftmodule based on the extension of the raw
-    // dependency path, as the true file may have a different one.
-    // For example, this might happen when the canonicalized path points to
-    // a Content Addressed Storage (CAS) location.
-    auto moduleFileType =
-      file_types::lookupTypeForExtension(llvm::sys::path::extension(depPath));
-    if (moduleFileType == file_types::TY_SwiftModuleFile
-        || moduleFileType == file_types::TY_SwiftModuleInterfaceFile) {
-      auto dep = pathToModuleDecl.find(depPath);
-      assert(dep != pathToModuleDecl.end()
-             && "Dependency must've been loaded.");
-      ModuleDecl *depMod = dep->second;
-      swiftModules.push_back({
-        /*Name=*/
-        depMod->getName(),
-        /*Path=*/
-        realDepPath,
-        // TODO: There is an edge case which is not handled here.
-        // When we build a framework using -import-underlying-module, or an
-        // app/test using -import-objc-header, we should look at the direct
-        // imports of the bridging modules, and mark those as our direct
-        // imports.
-        /*IsImportedDirectly=*/
-        importedModules.find(depMod) != importedModules.end(),
-        /*SupportsLibraryEvolution=*/
-        depMod->isResilient()
-      });
-    }
-  }
-
-  // Almost a re-implementation of reversePathSortedFilenames :(.
-  std::sort(
-    swiftModules.begin(), swiftModules.end(),
-    [](const SwiftModuleTraceInfo &m1, const SwiftModuleTraceInfo &m2) -> bool {
-      return std::lexicographical_compare(
-        m1.Path.rbegin(), m1.Path.rend(),
-        m2.Path.rbegin(), m2.Path.rend());
-  });
+  computeSwiftModuleTraceInfo(importedModules, pathToModuleDecl, *depTracker,
+                              prebuiltCachePath, swiftModules);
 
   LoadedModuleTraceFormat trace = {
       /*version=*/LoadedModuleTraceFormat::CurrentVersion,
@@ -339,8 +447,7 @@ static bool emitLoadedModuleTraceIfNeeded(ModuleDecl *mainModule,
   };
 
   // raw_fd_ostream is unbuffered, and we may have multiple processes writing,
-  // so first write the whole thing into memory and dump out that buffer to the
-  // file.
+  // so first write to memory and then dump the buffer to the trace file.
   std::string stringBuffer;
   {
     llvm::raw_string_ostream memoryBuffer(stringBuffer);
@@ -349,7 +456,6 @@ static bool emitLoadedModuleTraceIfNeeded(ModuleDecl *mainModule,
     json::jsonize(jsonOutput, trace, /*Required=*/true);
   }
   stringBuffer += "\n";
-
   out << stringBuffer;
 
   return true;
@@ -362,7 +468,8 @@ emitLoadedModuleTraceForAllPrimariesIfNeeded(ModuleDecl *mainModule,
   return opts.InputsAndOutputs.forEachInputProducingSupplementaryOutput(
       [&](const InputFile &input) -> bool {
         return emitLoadedModuleTraceIfNeeded(
-            mainModule, depTracker, input.loadedModuleTracePath());
+          mainModule, depTracker, opts.PrebuiltModuleCachePath,
+          input.loadedModuleTracePath());
       });
 }