[Dependency Scanning] Fix optional import statement serialization logic

Previously, serialization of optional import statement node arrays erroneously used the node layout of a non-optional import node array while maintaining a separate index for these arrays which got serialized into each module's info node. This meant it got serialized out in a sequence of non-optional import arrays, and as a result, the consumer deserialized them as such, which broke ordering of which arrays of import nodes belong which module, causing mismatches between a module's actual import set and the import which got deserialized for it.

This change fixes the optional import serialization routine to use the purpose-made separate layout which gets read out according to the separate optional import array index.

Author: artemcm

lib/DependencyScan/ModuleDependencyCacheSerialization.cpp

@@ -49,8 +49,8 @@ class ModuleDependenciesCacheDeserializer {
   std::vector<std::vector<uint64_t>> ArraysOfMacroDependenciesIDs;
   std::vector<ScannerImportStatementInfo> ImportStatements;
   std::vector<std::vector<uint64_t>> ArraysOfImportStatementIDs;
-  std::vector<std::vector<uint64_t>> ArraysOfSearchPathIDs;
   std::vector<std::vector<uint64_t>> ArraysOfOptionalImportStatementIDs;
+  std::vector<std::vector<uint64_t>> ArraysOfSearchPathIDs;
 
   llvm::BitstreamCursor Cursor;
   SmallVector<uint64_t, 64> Scratch;
@@ -1170,6 +1170,7 @@ class ModuleDependenciesCacheSerializer {
   unsigned writeImportStatementInfos(const ModuleDependencyInfo &dependencyInfo,
                                      bool optional);
   void writeImportStatementInfosArray(unsigned startIndex, unsigned count);
+  void writeOptionalImportStatementInfosArray(unsigned startIndex, unsigned count);
 
   void writeModuleInfo(ModuleDependencyID moduleID,
                        const ModuleDependencyInfo &dependencyInfo);
@@ -1478,7 +1479,7 @@ void ModuleDependenciesCacheSerializer::writeImportStatementInfos(
       }
       auto optionalEntries = optionalImportInfoArrayMap.at(moduleID);
       if (optionalEntries.second != 0) {
-        writeImportStatementInfosArray(optionalEntries.first, optionalEntries.second);
+        writeOptionalImportStatementInfosArray(optionalEntries.first, optionalEntries.second);
         OptionalImportInfosArrayIDsMap.insert({moduleID, lastOptionalImportInfoArrayIndex++});
       }
     }
@@ -1530,6 +1531,15 @@ void ModuleDependenciesCacheSerializer::writeImportStatementInfosArray(
       Out, ScratchRecord, AbbrCodes[ImportStatementArrayLayout::Code], vec);
 }
 
+void ModuleDependenciesCacheSerializer::writeOptionalImportStatementInfosArray(
+    unsigned startIndex, unsigned count) {
+  using namespace graph_block;
+  std::vector<unsigned> vec(count);
+  std::iota(vec.begin(), vec.end(), startIndex);
+  OptionalImportStatementArrayLayout::emitRecord(
+      Out, ScratchRecord, AbbrCodes[OptionalImportStatementArrayLayout::Code], vec);
+}
+
 void ModuleDependenciesCacheSerializer::writeModuleInfo(
     ModuleDependencyID moduleID, const ModuleDependencyInfo &dependencyInfo) {
   using namespace graph_block;
@@ -1965,6 +1975,7 @@ void ModuleDependenciesCacheSerializer::writeInterModuleDependenciesCache(
   registerRecordAbbr<SearchPathArrayLayout>();
   registerRecordAbbr<ImportStatementLayout>();
   registerRecordAbbr<ImportStatementArrayLayout>();
+  registerRecordAbbr<OptionalImportStatementArrayLayout>();
   registerRecordAbbr<ModuleInfoLayout>();
   registerRecordAbbr<SwiftSourceModuleDetailsLayout>();
   registerRecordAbbr<SwiftInterfaceModuleDetailsLayout>();

test/ScanDependencies/serialized_imports.swift

@@ -3,12 +3,14 @@
 // RUN: %empty-directory(%t/module-cache)
 
 // Run the scanner once, emitting the serialized scanner cache
-// RUN: %target-swift-frontend -scan-dependencies -module-load-mode prefer-interface -Rdependency-scan-cache -serialize-dependency-scan-cache -load-dependency-scan-cache -dependency-scan-cache-path %t/cache.moddepcache -module-cache-path %t/module-cache %s -o %t/deps.json -I %S/Inputs/CHeaders -I %S/Inputs/Swift -disable-implicit-concurrency-module-import -disable-implicit-string-processing-module-import -enable-cross-import-overlays 2>&1 | %FileCheck %s -check-prefix CHECK-REMARK-SAVE
+// RUN: %target-swift-frontend -scan-dependencies -module-load-mode prefer-interface -Rdependency-scan-cache -serialize-dependency-scan-cache -load-dependency-scan-cache -dependency-scan-cache-path %t/cache.moddepcache -module-cache-path %t/module-cache %s -o %t/deps.json -I %S/Inputs/CHeaders -I %S/Inputs/Swift -disable-implicit-concurrency-module-import -disable-implicit-string-processing-module-import -enable-cross-import-overlays -module-name deps 2>&1 | %FileCheck %s -check-prefix CHECK-REMARK-SAVE
 // RUN: llvm-bcanalyzer --dump %t/cache.moddepcache > %t/cache.moddepcache.initial.dump.txt
+// RUN: %validate-json %t/deps.json | %FileCheck %s -check-prefix CHECK-IMPORTS
 
 // Run the scanner again, but now re-using previously-serialized cache
-// RUN: %target-swift-frontend -scan-dependencies -module-load-mode prefer-interface -Rdependency-scan-cache -serialize-dependency-scan-cache -load-dependency-scan-cache -dependency-scan-cache-path %t/cache.moddepcache -module-cache-path %t/module-cache %s -o %t/deps.json -I %S/Inputs/CHeaders -I %S/Inputs/Swift -disable-implicit-concurrency-module-import -disable-implicit-string-processing-module-import -enable-cross-import-overlays 2>&1 | %FileCheck %s -check-prefix CHECK-REMARK-LOAD
+// RUN: %target-swift-frontend -scan-dependencies -module-load-mode prefer-interface -Rdependency-scan-cache -serialize-dependency-scan-cache -load-dependency-scan-cache -dependency-scan-cache-path %t/cache.moddepcache -module-cache-path %t/module-cache %s -o %t/deps_incremental.json -I %S/Inputs/CHeaders -I %S/Inputs/Swift -disable-implicit-concurrency-module-import -disable-implicit-string-processing-module-import -enable-cross-import-overlays -module-name deps 2>&1 | %FileCheck %s -check-prefix CHECK-REMARK-LOAD
 // RUN: llvm-bcanalyzer --dump %t/cache.moddepcache > %t/cache.moddepcache.dump.txt
+// RUN: %validate-json %t/deps_incremental.json | %FileCheck %s -check-prefix CHECK-IMPORTS
 
 // Ensure that the initial scan, and the secondary scan which just re-used the initial scan's results report
 // the same number of import statement nodes, ensuring that serialization-deserialization did not affect
@@ -22,8 +24,26 @@
 // CHECK-REMARK-SAVE: remark: Incremental module scan: Serializing module scanning dependency cache to:
 // CHECK-REMARK-LOAD: remark: Incremental module scan: Re-using serialized module scanning dependency cache from:
 
-import E
-
-
-
+// CHECK-IMPORTS:      "modulePath": "deps.swiftmodule",
+// CHECK-IMPORTS:      "imports": [
+// CHECK-IMPORTS-NEXT:        {
+// CHECK-IMPORTS-NEXT:          "identifier": "Swift",
+// CHECK-IMPORTS-NEXT:          "accessLevel": "public"
+// CHECK-IMPORTS-NEXT:        },
+// CHECK-IMPORTS-NEXT:        {
+// CHECK-IMPORTS-NEXT:          "identifier": "SwiftOnoneSupport",
+// CHECK-IMPORTS-NEXT:          "accessLevel": "public"
+// CHECK-IMPORTS-NEXT:        },
+// CHECK-IMPORTS-NEXT:        {
+// CHECK-IMPORTS-NEXT:          "identifier": "E",
+// CHECK-IMPORTS-NEXT:          "accessLevel": "public",
+// CHECK-IMPORTS-NEXT:          "importLocations": [
+// CHECK-IMPORTS-NEXT:            {
+// CHECK-IMPORTS-NEXT:              "bufferIdentifier": "{{.*}}serialized_imports.swift",
+// CHECK-IMPORTS-NEXT:              "linuNumber": 49,
+// CHECK-IMPORTS-NEXT:              "columnNumber": 8
+// CHECK-IMPORTS-NEXT:            }
+// CHECK-IMPORTS-NEXT:          ]
+// CHECK-IMPORTS-NEXT:        }
 
+import E