From 81bfa1a9cef9ee367994991ba1b0fb8e198397b1 Mon Sep 17 00:00:00 2001 From: Artem Chikin Date: Tue, 10 Oct 2023 15:35:30 -0700 Subject: [PATCH 1/2] [Serialization] Serialize source '.h' path when an explicit '.pch' bridging header is provided In explicit module builds, bridging header is passed directly as a '.pch' input. Loading clients may not be able to directly import this PCH because it was built against mis-matched dependencies with a different context hash. So instead they should directly injest the '.h' depndency and build it against their own set of dependencies. --- lib/Serialization/Serialization.cpp | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/lib/Serialization/Serialization.cpp b/lib/Serialization/Serialization.cpp index ae6ce59f8648f..1281b7b7a7d0b 100644 --- a/lib/Serialization/Serialization.cpp +++ b/lib/Serialization/Serialization.cpp @@ -52,6 +52,8 @@ #include "swift/Serialization/SerializationOptions.h" #include "swift/Strings.h" #include "clang/AST/DeclTemplate.h" +#include "clang/Frontend/CompilerInstance.h" +#include "clang/Serialization/ASTReader.h" #include "llvm/ADT/SmallSet.h" #include "llvm/ADT/SmallString.h" #include "llvm/ADT/StringExtras.h" @@ -1296,15 +1298,27 @@ void Serializer::writeInputBlock() { off_t importedHeaderSize = 0; time_t importedHeaderModTime = 0; std::string contents; - if (!Options.ImportedHeader.empty()) { + auto importedHeaderPath = Options.ImportedHeader; + // We do not want to serialize the explicitly-specified .pch path if one was + // provided. Instead we write out the path to the original header source so + // that clients can consume it. + if (llvm::sys::path::extension(importedHeaderPath) + .endswith(file_types::getExtension(file_types::TY_PCH))) + importedHeaderPath = clangImporter->getClangInstance() + .getASTReader() + ->getModuleManager() + .lookupByFileName(importedHeaderPath) + ->OriginalSourceFileName; + + if (!importedHeaderPath.empty()) { contents = clangImporter->getBridgingHeaderContents( - Options.ImportedHeader, importedHeaderSize, importedHeaderModTime); + importedHeaderPath, importedHeaderSize, importedHeaderModTime); } assert(publicImportSet.count(bridgingHeaderImport)); ImportedHeader.emit(ScratchRecord, publicImportSet.count(bridgingHeaderImport), importedHeaderSize, importedHeaderModTime, - Options.ImportedHeader); + importedHeaderPath); if (!contents.empty()) { contents.push_back('\0'); ImportedHeaderContents.emit(ScratchRecord, contents); From 478bab233b72dc34fe34461a1232e819006f8c5b Mon Sep 17 00:00:00 2001 From: Artem Chikin Date: Tue, 10 Oct 2023 15:53:47 -0700 Subject: [PATCH 2/2] [Dependency Scanning] Do now write out bridging header dependencies of binary modules unless in CAS mode We only record these dependencies in CAS mode, because we require explicit PCH tasks to be produced for imported header of binary module dependencies. In the meantime, in non-CAS mode loading clients will consume the `.h` files encoded in the `.swiftmodules` directly. Followup changes to SwiftDriver will enable explicit PCH compilation of such dependenceis, but for the time being restore prior behavior for non-CAS explicit module builds. Resolves rdar://116006619 --- .../Serialization/SerializationOptions.h | 1 + lib/Frontend/Frontend.cpp | 2 ++ lib/Serialization/Serialization.cpp | 3 ++- lib/Serialization/SerializedModuleLoader.cpp | 19 +++++++++++++------ .../header_deps_of_binary.swift | 18 +++++++++++++----- 5 files changed, 31 insertions(+), 12 deletions(-) diff --git a/include/swift/Serialization/SerializationOptions.h b/include/swift/Serialization/SerializationOptions.h index b1822fa26b264..40ac1e4a4b377 100644 --- a/include/swift/Serialization/SerializationOptions.h +++ b/include/swift/Serialization/SerializationOptions.h @@ -158,6 +158,7 @@ namespace swift { bool EmbeddedSwiftModule = false; bool IsOSSA = false; bool SerializeExternalDeclsOnly = false; + bool ExplicitModuleBuild = false; }; } // end namespace swift diff --git a/lib/Frontend/Frontend.cpp b/lib/Frontend/Frontend.cpp index 6e6d33ca9543e..ffc371ffd54f5 100644 --- a/lib/Frontend/Frontend.cpp +++ b/lib/Frontend/Frontend.cpp @@ -233,6 +233,8 @@ SerializationOptions CompilerInvocation::computeSerializationOptions( serializationOpts.SerializeExternalDeclsOnly = opts.SerializeExternalDeclsOnly; + serializationOpts.ExplicitModuleBuild = FrontendOpts.DisableImplicitModules; + return serializationOpts; } diff --git a/lib/Serialization/Serialization.cpp b/lib/Serialization/Serialization.cpp index 1281b7b7a7d0b..fb5d120d092d4 100644 --- a/lib/Serialization/Serialization.cpp +++ b/lib/Serialization/Serialization.cpp @@ -1302,7 +1302,8 @@ void Serializer::writeInputBlock() { // We do not want to serialize the explicitly-specified .pch path if one was // provided. Instead we write out the path to the original header source so // that clients can consume it. - if (llvm::sys::path::extension(importedHeaderPath) + if (Options.ExplicitModuleBuild && + llvm::sys::path::extension(importedHeaderPath) .endswith(file_types::getExtension(file_types::TY_PCH))) importedHeaderPath = clangImporter->getClangInstance() .getASTReader() diff --git a/lib/Serialization/SerializedModuleLoader.cpp b/lib/Serialization/SerializedModuleLoader.cpp index d9bc44ae88067..6cd330a115138 100644 --- a/lib/Serialization/SerializedModuleLoader.cpp +++ b/lib/Serialization/SerializedModuleLoader.cpp @@ -477,12 +477,19 @@ SerializedModuleLoaderBase::scanModuleFile(Twine modulePath, bool isFramework) { auto importedHeaderSet = binaryModuleImports.get().headerImports; std::vector importedHeaders; - importedHeaders.reserve(importedHeaderSet.size()); - llvm::transform(importedHeaderSet.keys(), - std::back_inserter(importedHeaders), - [](llvm::StringRef N) { - return N.str(); - }); + // FIXME: We only record these dependencies in CAS mode, because + // we require explicit PCH tasks to be produced for imported header + // of binary module dependencies. In the meantime, in non-CAS mode + // loading clients will consume the `.h` files encoded in the `.swiftmodules` + // directly. + if (Ctx.ClangImporterOpts.CASOpts) { + importedHeaders.reserve(importedHeaderSet.size()); + llvm::transform(importedHeaderSet.keys(), + std::back_inserter(importedHeaders), + [](llvm::StringRef N) { + return N.str(); + }); + } auto &importedOptionalModuleSet = binaryModuleOptionalImports.get().moduleImports; std::vector importedOptionalModuleNames; diff --git a/test/ScanDependencies/header_deps_of_binary.swift b/test/ScanDependencies/header_deps_of_binary.swift index 1221380782b18..94862cbc0f616 100644 --- a/test/ScanDependencies/header_deps_of_binary.swift +++ b/test/ScanDependencies/header_deps_of_binary.swift @@ -2,7 +2,9 @@ // RUN: %empty-directory(%t) // RUN: %empty-directory(%t/clang-module-cache) // RUN: %empty-directory(%t/PCH) +// RUN: %empty-directory(%t/HEADER) // RUN: %empty-directory(%t/SwiftModules) +// RUN: %empty-directory(%t/CAS) // - Set up Foo Swift dependency // RUN: echo "extension Profiler {" >> %t/foo.swift @@ -10,10 +12,10 @@ // RUN: echo "}" >> %t/foo.swift // - Set up Foo bridging header -// RUN: echo "struct Profiler { void* ptr; };" >> %t/foo.h +// RUN: echo "struct Profiler { void* ptr; };" >> %t/HEADER/foo.h // - Compile bridging header -// RUN: %target-swift-frontend -enable-objc-interop -emit-pch %t/foo.h -o %t/PCH/foo.pch -disable-implicit-swift-modules +// RUN: %target-swift-frontend -enable-objc-interop -emit-pch %t/HEADER/foo.h -o %t/PCH/foo.pch -disable-implicit-swift-modules // - Set up explicit dependencies for Foo // RUN: %target-swift-emit-pcm -module-name SwiftShims %swift-lib-dir/swift/shims/module.modulemap -o %t/inputs/SwiftShims.pcm @@ -50,21 +52,27 @@ // RUN: %target-swift-frontend -emit-module -emit-module-path %t/SwiftModules/Foo.swiftmodule %t/foo.swift -module-name Foo -import-objc-header %t/PCH/foo.pch -disable-implicit-concurrency-module-import -disable-implicit-string-processing-module-import -disable-implicit-swift-modules -explicit-swift-module-map-file %t/foo_inputs_map.json // - Scan main module -// RUN: %target-swift-frontend -scan-dependencies %s -I %t/SwiftModules -I %S/Inputs/Swift -o %t/deps.json +// RUN: %target-swift-frontend -scan-dependencies %s -I %t/SwiftModules -I %S/Inputs/Swift -o %t/deps.json -cache-compile-job -cas-path %t/cas // RUN: %validate-json %t/deps.json | %FileCheck %s +// - Scan main module without a CAS and ensure no headerDependencies are emitted +// RUN: %target-swift-frontend -scan-dependencies %s -I %t/SwiftModules -I %S/Inputs/Swift -o %t/deps_nocas.json +// RUN: %validate-json %t/deps_nocas.json | %FileCheck %s --check-prefix=CHECK-NO-HEADERS + // CHECK: "swift": "FooClient" // CHECK: "swift": "FooClient" // CHECK: "swiftPrebuiltExternal": "Foo" // CHECK: "commandLine": [ // CHECK: "-include-pch", // CHECK-NEXT: "-Xcc", -// CHECK-NEXT: "{{.*}}{{/|\\}}PCH{{/|\\}}foo.pch" +// CHECK-NEXT: "{{.*}}{{/|\\}}HEADER{{/|\\}}foo.h" // CHECK: "swiftPrebuiltExternal": "Foo" // CHECK: "headerDependencies": [ -// CHECK: "{{.*}}{{/|\\}}PCH{{/|\\}}foo.pch" +// CHECK: "{{.*}}{{/|\\}}HEADER{{/|\\}}foo.h" // CHECK: ], +// CHECK-NO-HEADERS-NOT: "headerDependencies" + import FooClient