From bc30953c15326986aaeec895d7d67048df261ab9 Mon Sep 17 00:00:00 2001 From: Artem Chikin Date: Thu, 14 Aug 2025 14:01:11 -0700 Subject: [PATCH 1/2] [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. --- .../ModuleDependencyCacheSerialization.cpp | 15 +++++++-- .../ScanDependencies/serialized_imports.swift | 32 +++++++++++++++---- 2 files changed, 39 insertions(+), 8 deletions(-) diff --git a/lib/DependencyScan/ModuleDependencyCacheSerialization.cpp b/lib/DependencyScan/ModuleDependencyCacheSerialization.cpp index 4ab832fb962c9..c3ed6f2fb8248 100644 --- a/lib/DependencyScan/ModuleDependencyCacheSerialization.cpp +++ b/lib/DependencyScan/ModuleDependencyCacheSerialization.cpp @@ -49,8 +49,8 @@ class ModuleDependenciesCacheDeserializer { std::vector> ArraysOfMacroDependenciesIDs; std::vector ImportStatements; std::vector> ArraysOfImportStatementIDs; - std::vector> ArraysOfSearchPathIDs; std::vector> ArraysOfOptionalImportStatementIDs; + std::vector> ArraysOfSearchPathIDs; llvm::BitstreamCursor Cursor; SmallVector 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 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(); registerRecordAbbr(); registerRecordAbbr(); + registerRecordAbbr(); registerRecordAbbr(); registerRecordAbbr(); registerRecordAbbr(); diff --git a/test/ScanDependencies/serialized_imports.swift b/test/ScanDependencies/serialized_imports.swift index 1b62894eb6067..ae87a32eb11f2 100644 --- a/test/ScanDependencies/serialized_imports.swift +++ b/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 From b14cbbb0397642f9351b5f30a3d45e5e664042bf Mon Sep 17 00:00:00 2001 From: Artem Chikin Date: Fri, 15 Aug 2025 14:49:43 -0700 Subject: [PATCH 2/2] [Dependency Scanning] Add serialization of optional imports to the JSON output And to the corresponding graph data structure the output gets generated from --- .../swift/DependencyScan/DependencyScanImpl.h | 3 + .../SerializedModuleDependencyCacheFormat.h | 2 +- lib/DependencyScan/DependencyScanJSON.cpp | 12 +++- lib/DependencyScan/ScanDependencies.cpp | 67 ++++++++++--------- .../ScanDependencies/serialized_imports.swift | 10 ++- 5 files changed, 58 insertions(+), 36 deletions(-) diff --git a/include/swift/DependencyScan/DependencyScanImpl.h b/include/swift/DependencyScan/DependencyScanImpl.h index 433f3962d2134..13408e2c98bcd 100644 --- a/include/swift/DependencyScan/DependencyScanImpl.h +++ b/include/swift/DependencyScan/DependencyScanImpl.h @@ -65,6 +65,9 @@ struct swiftscan_dependency_info_s { /// The list of source import infos. swiftscan_import_info_set_t *imports; + /// The list of source optional import infos. + swiftscan_import_info_set_t *optional_imports; + /// Specific details of a particular kind of module. swiftscan_module_details_t details; }; diff --git a/include/swift/DependencyScan/SerializedModuleDependencyCacheFormat.h b/include/swift/DependencyScan/SerializedModuleDependencyCacheFormat.h index c61cb26cd815a..e1a324ef1b5a8 100644 --- a/include/swift/DependencyScan/SerializedModuleDependencyCacheFormat.h +++ b/include/swift/DependencyScan/SerializedModuleDependencyCacheFormat.h @@ -41,7 +41,7 @@ using llvm::BCVBR; const unsigned char MODULE_DEPENDENCY_CACHE_FORMAT_SIGNATURE[] = {'I', 'M', 'D','C'}; const unsigned MODULE_DEPENDENCY_CACHE_FORMAT_VERSION_MAJOR = 10; /// Increment this on every change. -const unsigned MODULE_DEPENDENCY_CACHE_FORMAT_VERSION_MINOR = 3; +const unsigned MODULE_DEPENDENCY_CACHE_FORMAT_VERSION_MINOR = 4; /// Various identifiers in this format will rely on having their strings mapped /// using this ID. diff --git a/lib/DependencyScan/DependencyScanJSON.cpp b/lib/DependencyScan/DependencyScanJSON.cpp index 5f435b80873f6..fd609e1d19798 100644 --- a/lib/DependencyScan/DependencyScanJSON.cpp +++ b/lib/DependencyScan/DependencyScanJSON.cpp @@ -228,9 +228,13 @@ void writeLinkLibraries(llvm::raw_ostream &out, void writeImportInfos(llvm::raw_ostream &out, const swiftscan_import_info_set_t *imports, - unsigned indentLevel, bool trailingComma) { + bool optional, unsigned indentLevel, + bool trailingComma) { out.indent(indentLevel * 2); - out << "\"imports\": "; + if (optional) + out << "\"optionalImports\": "; + else + out << "\"imports\": "; out << "[\n"; for (size_t i = 0; i < imports->count; ++i) { @@ -441,7 +445,9 @@ void writeJSON(llvm::raw_ostream &out, /*trailingComma=*/true); writeLinkLibraries(out, moduleInfo.link_libraries, 3, /*trailingComma=*/true); - writeImportInfos(out, moduleInfo.imports, + writeImportInfos(out, moduleInfo.imports, /*optional*/ false, + 3, /*trailingComma=*/true); + writeImportInfos(out, moduleInfo.optional_imports, /*optional*/ true, 3, /*trailingComma=*/true); } // Swift and Clang-specific details. diff --git a/lib/DependencyScan/ScanDependencies.cpp b/lib/DependencyScan/ScanDependencies.cpp index 07f22e8c2a6dc..e5e3d29add55c 100644 --- a/lib/DependencyScan/ScanDependencies.cpp +++ b/lib/DependencyScan/ScanDependencies.cpp @@ -922,37 +922,44 @@ static swiftscan_dependency_graph_t generateFullDependencyGraph( } moduleInfo->link_libraries = linkLibrarySet; - // Create source import infos set for this module - auto imports = moduleDependencyInfo.getModuleImports(); - swiftscan_import_info_set_t *importInfoSet = - new swiftscan_import_info_set_t; - importInfoSet->count = imports.size(); - importInfoSet->imports = new swiftscan_import_info_t[importInfoSet->count]; - for (size_t i = 0; i < imports.size(); ++i) { - const auto &ii = imports[i]; - swiftscan_import_info_s *iInfo = new swiftscan_import_info_s; - iInfo->import_identifier = create_clone(ii.importIdentifier.c_str()); - iInfo->access_level = - static_cast(ii.accessLevel); - - const auto &sourceLocations = ii.importLocations; - swiftscan_source_location_set_t *sourceLocSet = - new swiftscan_source_location_set_t; - sourceLocSet->count = sourceLocations.size(); - sourceLocSet->source_locations = - new swiftscan_source_location_t[sourceLocSet->count]; - for (size_t j = 0; j < sourceLocations.size(); ++j) { - const auto &sl = sourceLocations[j]; - swiftscan_source_location_s *slInfo = new swiftscan_source_location_s; - slInfo->buffer_identifier = create_clone(sl.bufferIdentifier.c_str()); - slInfo->line_number = sl.lineNumber; - slInfo->column_number = sl.columnNumber; - sourceLocSet->source_locations[j] = slInfo; + auto createImportSetInfo = [&](ArrayRef imports) + -> swiftscan_import_info_set_t * { + swiftscan_import_info_set_t *importInfoSet = + new swiftscan_import_info_set_t; + importInfoSet->count = imports.size(); + importInfoSet->imports = + new swiftscan_import_info_t[importInfoSet->count]; + for (size_t i = 0; i < imports.size(); ++i) { + const auto &ii = imports[i]; + swiftscan_import_info_s *iInfo = new swiftscan_import_info_s; + iInfo->import_identifier = create_clone(ii.importIdentifier.c_str()); + iInfo->access_level = + static_cast(ii.accessLevel); + + const auto &sourceLocations = ii.importLocations; + swiftscan_source_location_set_t *sourceLocSet = + new swiftscan_source_location_set_t; + sourceLocSet->count = sourceLocations.size(); + sourceLocSet->source_locations = + new swiftscan_source_location_t[sourceLocSet->count]; + for (size_t j = 0; j < sourceLocations.size(); ++j) { + const auto &sl = sourceLocations[j]; + swiftscan_source_location_s *slInfo = new swiftscan_source_location_s; + slInfo->buffer_identifier = create_clone(sl.bufferIdentifier.c_str()); + slInfo->line_number = sl.lineNumber; + slInfo->column_number = sl.columnNumber; + sourceLocSet->source_locations[j] = slInfo; + } + iInfo->source_locations = sourceLocSet; + importInfoSet->imports[i] = iInfo; } - iInfo->source_locations = sourceLocSet; - importInfoSet->imports[i] = iInfo; - } - moduleInfo->imports = importInfoSet; + return importInfoSet; + }; + // Create source import infos set for this module + moduleInfo->imports = + createImportSetInfo(moduleDependencyInfo.getModuleImports()); + moduleInfo->optional_imports = + createImportSetInfo(moduleDependencyInfo.getOptionalModuleImports()); } swiftscan_dependency_graph_t result = new swiftscan_dependency_graph_s; diff --git a/test/ScanDependencies/serialized_imports.swift b/test/ScanDependencies/serialized_imports.swift index ae87a32eb11f2..41b3945a715f3 100644 --- a/test/ScanDependencies/serialized_imports.swift +++ b/test/ScanDependencies/serialized_imports.swift @@ -40,10 +40,16 @@ // CHECK-IMPORTS-NEXT: "importLocations": [ // CHECK-IMPORTS-NEXT: { // CHECK-IMPORTS-NEXT: "bufferIdentifier": "{{.*}}serialized_imports.swift", -// CHECK-IMPORTS-NEXT: "linuNumber": 49, +// CHECK-IMPORTS-NEXT: "linuNumber": 54, // CHECK-IMPORTS-NEXT: "columnNumber": 8 // CHECK-IMPORTS-NEXT: } -// CHECK-IMPORTS-NEXT: ] + +// CHECK-IMPORTS: "optionalImports": [ +// CHECK-IMPORTS-NEXT: { +// CHECK-IMPORTS-NEXT: "identifier": "E_Private", +// CHECK-IMPORTS-NEXT: "accessLevel": "public" // CHECK-IMPORTS-NEXT: } +// CHECK-IMPORTS-NEXT: ], import E +import E.Private