From 7a68d364f449efdf4f7437a9f51d294971ae3977 Mon Sep 17 00:00:00 2001 From: Steven Wu Date: Wed, 3 Apr 2024 08:50:34 -0700 Subject: [PATCH 1/2] [Caching] Embed bridging header in binary module correctly when caching When caching is enabled with include-tree, the bridging header PCH is created from the include tree directly. Setup the rewriter correctly when embedding the bridging header into swift binary module. rdar://125719747 --- .../swift/AST/DiagnosticsClangImporter.def | 2 + include/swift/ClangImporter/ClangImporter.h | 6 +- lib/ClangImporter/ClangImporter.cpp | 70 +++++++++++++++++-- lib/ClangImporter/ImporterImpl.h | 1 + lib/Serialization/Serialization.cpp | 18 +++-- test/CAS/bridging-header.swift | 57 ++++++++++++++- 6 files changed, 137 insertions(+), 17 deletions(-) diff --git a/include/swift/AST/DiagnosticsClangImporter.def b/include/swift/AST/DiagnosticsClangImporter.def index 600709b3f2805..288f23b9d4c5d 100644 --- a/include/swift/AST/DiagnosticsClangImporter.def +++ b/include/swift/AST/DiagnosticsClangImporter.def @@ -42,6 +42,8 @@ WARNING(could_not_rewrite_bridging_header,none, ERROR(bridging_header_pch_error,Fatal, "failed to emit precompiled header '%0' for bridging header '%1'", (StringRef, StringRef)) +ERROR(err_rewrite_bridging_header,none, + "failed to serialize bridging header: '%0'", (StringRef)) ERROR(emit_pcm_error,Fatal, "failed to emit precompiled module '%0' for module map '%1'", diff --git a/include/swift/ClangImporter/ClangImporter.h b/include/swift/ClangImporter/ClangImporter.h index e5fc22588da83..01d6415d525d1 100644 --- a/include/swift/ClangImporter/ClangImporter.h +++ b/include/swift/ClangImporter/ClangImporter.h @@ -402,8 +402,10 @@ class ClangImporter final : public ClangModuleLoader { getWrapperForModule(const clang::Module *mod, bool returnOverlayIfPossible = false) const override; - std::string getBridgingHeaderContents(StringRef headerPath, off_t &fileSize, - time_t &fileModTime); + std::string + getBridgingHeaderContents(StringRef headerPath, off_t &fileSize, + time_t &fileModTime, + StringRef pchIncludeTree); /// Makes a temporary replica of the ClangImporter's CompilerInstance, reads /// an Objective-C header file into the replica and emits a PCH file of its diff --git a/lib/ClangImporter/ClangImporter.cpp b/lib/ClangImporter/ClangImporter.cpp index 361afa715c24a..b6e6734afd0de 100644 --- a/lib/ClangImporter/ClangImporter.cpp +++ b/lib/ClangImporter/ClangImporter.cpp @@ -55,13 +55,16 @@ #include "clang/Basic/DiagnosticOptions.h" #include "clang/Basic/FileEntry.h" #include "clang/Basic/IdentifierTable.h" +#include "clang/Basic/LangStandard.h" #include "clang/Basic/Module.h" #include "clang/Basic/TargetInfo.h" #include "clang/Basic/Version.h" #include "clang/CAS/CASOptions.h" +#include "clang/CAS/IncludeTree.h" #include "clang/CodeGen/ObjectFilePCHContainerOperations.h" #include "clang/Frontend/CompilerInvocation.h" #include "clang/Frontend/FrontendActions.h" +#include "clang/Frontend/IncludeTreePPActions.h" #include "clang/Frontend/TextDiagnosticPrinter.h" #include "clang/Frontend/Utils.h" #include "clang/Index/IndexingAction.h" @@ -80,7 +83,11 @@ #include "llvm/ADT/STLExtras.h" #include "llvm/ADT/SmallVector.h" #include "llvm/ADT/StringExtras.h" +#include "llvm/CAS/CASReference.h" +#include "llvm/CAS/ObjectStore.h" #include "llvm/Support/CrashRecoveryContext.h" +#include "llvm/Support/Error.h" +#include "llvm/Support/ErrorHandling.h" #include "llvm/Support/FileCollector.h" #include "llvm/Support/FileSystem.h" #include "llvm/Support/Memory.h" @@ -1776,16 +1783,46 @@ bool ClangImporter::importBridgingHeader(StringRef header, ModuleDecl *adapter, std::move(sourceBuffer), implicitImport); } -std::string ClangImporter::getBridgingHeaderContents(StringRef headerPath, - off_t &fileSize, - time_t &fileModTime) { +static llvm::Expected +setupIncludeTreeInput(clang::CompilerInvocation &invocation, + StringRef headerPath, StringRef pchIncludeTree) { + auto DB = invocation.getCASOpts().getOrCreateDatabases(); + if (!DB) + return DB.takeError(); + auto CAS = DB->first; + auto ID = CAS->parseID(pchIncludeTree); + if (!ID) + return ID.takeError(); + auto includeTreeRef = CAS->getReference(*ID); + if (!includeTreeRef) + return llvm::cas::ObjectStore::createUnknownObjectError(*ID); + + invocation.getFrontendOpts().Inputs.push_back(clang::FrontendInputFile( + *includeTreeRef, headerPath, clang::Language::ObjC)); + + return *includeTreeRef; +} + +std::string ClangImporter::getBridgingHeaderContents( + StringRef headerPath, off_t &fileSize, time_t &fileModTime, + StringRef pchIncludeTree) { auto invocation = std::make_shared(*Impl.Invocation); invocation->getFrontendOpts().DisableFree = false; invocation->getFrontendOpts().Inputs.clear(); - invocation->getFrontendOpts().Inputs.push_back( - clang::FrontendInputFile(headerPath, clang::Language::ObjC)); + + std::optional includeTreeRef; + if (pchIncludeTree.empty()) + invocation->getFrontendOpts().Inputs.push_back( + clang::FrontendInputFile(headerPath, clang::Language::ObjC)); + else if (auto err = + setupIncludeTreeInput(*invocation, headerPath, pchIncludeTree) + .moveInto(includeTreeRef)) { + Impl.diagnose({}, diag::err_rewrite_bridging_header, + toString(std::move(err))); + return ""; + } invocation->getPreprocessorOpts().resetNonModularOptions(); @@ -1806,18 +1843,36 @@ std::string ClangImporter::getBridgingHeaderContents(StringRef headerPath, // write to an in-memory buffer. class RewriteIncludesAction : public clang::PreprocessorFrontendAction { raw_ostream &OS; + std::optional includeTreeRef; void ExecuteAction() override { clang::CompilerInstance &compiler = getCompilerInstance(); + // If the input is include tree, setup the IncludeTreePPAction. + if (includeTreeRef) { + auto IncludeTreeRoot = clang::cas::IncludeTreeRoot::get( + compiler.getOrCreateObjectStore(), *includeTreeRef); + if (!IncludeTreeRoot) + llvm::report_fatal_error(IncludeTreeRoot.takeError()); + auto PPCachedAct = + clang::createPPActionsFromIncludeTree(*IncludeTreeRoot); + if (!PPCachedAct) + llvm::report_fatal_error(PPCachedAct.takeError()); + compiler.getPreprocessor().setPPCachedActions( + std::move(*PPCachedAct)); + } + clang::RewriteIncludesInInput(compiler.getPreprocessor(), &OS, compiler.getPreprocessorOutputOpts()); } + public: - explicit RewriteIncludesAction(raw_ostream &os) : OS(os) {} + explicit RewriteIncludesAction( + raw_ostream &os, std::optional includeTree) + : OS(os), includeTreeRef(includeTree) {} }; llvm::raw_string_ostream os(result); - RewriteIncludesAction action(os); + RewriteIncludesAction action(os, includeTreeRef); rewriteInstance.ExecuteAction(action); }); @@ -2553,6 +2608,7 @@ ClangImporter::Implementation::Implementation( !ctx.ClangImporterOpts.BridgingHeader.empty()), DisableOverlayModules(ctx.ClangImporterOpts.DisableOverlayModules), EnableClangSPI(ctx.ClangImporterOpts.EnableClangSPI), + UseClangIncludeTree(ctx.ClangImporterOpts.UseClangIncludeTree), importSymbolicCXXDecls( ctx.LangOpts.hasFeature(Feature::ImportSymbolicCXXDecls)), IsReadingBridgingPCH(false), diff --git a/lib/ClangImporter/ImporterImpl.h b/lib/ClangImporter/ImporterImpl.h index cd0245697a184..dc9ec7f93ed8b 100644 --- a/lib/ClangImporter/ImporterImpl.h +++ b/lib/ClangImporter/ImporterImpl.h @@ -467,6 +467,7 @@ class LLVM_LIBRARY_VISIBILITY ClangImporter::Implementation const bool BridgingHeaderExplicitlyRequested; const bool DisableOverlayModules; const bool EnableClangSPI; + const bool UseClangIncludeTree; bool importSymbolicCXXDecls; bool IsReadingBridgingPCH; diff --git a/lib/Serialization/Serialization.cpp b/lib/Serialization/Serialization.cpp index 9fe0323aee6a5..d171c4d7f13e0 100644 --- a/lib/Serialization/Serialization.cpp +++ b/lib/Serialization/Serialization.cpp @@ -1321,21 +1321,25 @@ void Serializer::writeInputBlock() { time_t importedHeaderModTime = 0; std::string contents; auto importedHeaderPath = Options.ImportedHeader; + std::string pchIncludeTree; // 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 (Options.ExplicitModuleBuild && llvm::sys::path::extension(importedHeaderPath) - .ends_with(file_types::getExtension(file_types::TY_PCH))) - importedHeaderPath = clangImporter->getClangInstance() - .getASTReader() - ->getModuleManager() - .lookupByFileName(importedHeaderPath) - ->OriginalSourceFileName; + .ends_with(file_types::getExtension(file_types::TY_PCH))) { + auto *pch = clangImporter->getClangInstance() + .getASTReader() + ->getModuleManager() + .lookupByFileName(importedHeaderPath); + pchIncludeTree = pch->IncludeTreeID; + importedHeaderPath = pch->OriginalSourceFileName; + } if (!importedHeaderPath.empty()) { contents = clangImporter->getBridgingHeaderContents( - importedHeaderPath, importedHeaderSize, importedHeaderModTime); + importedHeaderPath, importedHeaderSize, importedHeaderModTime, + pchIncludeTree); } assert(publicImportSet.count(bridgingHeaderImport)); ImportedHeader.emit(ScratchRecord, diff --git a/test/CAS/bridging-header.swift b/test/CAS/bridging-header.swift index cae66faa194ae..52c6cd722d933 100644 --- a/test/CAS/bridging-header.swift +++ b/test/CAS/bridging-header.swift @@ -1,6 +1,11 @@ // RUN: %empty-directory(%t) // RUN: split-file %s %t +// RUN: %target-swift-frontend -emit-module -o %t/temp.swiftmodule -module-name Test -swift-version 5 \ +// RUN: -disable-implicit-string-processing-module-import -disable-implicit-concurrency-module-import \ +// RUN: -Xcc -fmodule-map-file=%t/a.modulemap -Xcc -fmodule-map-file=%t/b.modulemap -import-objc-header %t/Bridging.h \ +// RUN: %t/test.swift + // RUN: %target-swift-frontend -scan-dependencies -module-name Test -module-cache-path %t/clang-module-cache -O \ // RUN: -disable-implicit-string-processing-module-import -disable-implicit-concurrency-module-import \ // RUN: %t/test.swift -o %t/deps.json -swift-version 5 -cache-compile-job -cas-path %t/cas \ @@ -27,9 +32,57 @@ // CHECK-NEXT: "-Xcc", // CHECK-NEXT: "llvmcas://{{.*}}" +/// Try build then import from a non-caching compilation. + +// RUN: %{python} %S/Inputs/BuildCommandExtractor.py %t/deps.json clang:SwiftShims > %t/shim.cmd +// RUN: %swift_frontend_plain @%t/shim.cmd +// RUN: %{python} %S/Inputs/BuildCommandExtractor.py %t/deps.json clang:B > %t/B.cmd +// RUN: %swift_frontend_plain @%t/B.cmd +// RUN: %{python} %S/Inputs/BuildCommandExtractor.py %t/deps.json clang:A > %t/A.cmd +// RUN: %swift_frontend_plain @%t/A.cmd + +// RUN: %{python} %S/Inputs/GenerateExplicitModuleMap.py %t/deps.json > %t/map.json +// RUN: llvm-cas --cas %t/cas --make-blob --data %t/map.json > %t/map.casid + +// RUN: %{python} %S/Inputs/BuildCommandExtractor.py %t/deps.json bridgingHeader | tail -n +2 > %t/header.cmd +// RUN: %target-swift-frontend @%t/header.cmd -disable-implicit-swift-modules %t/Bridging.h -O -o %t/bridging.pch +// RUN: %cache-tool -cas-path %t/cas -cache-tool-action print-output-keys -- \ +// RUN: %target-swift-frontend @%t/header.cmd -disable-implicit-swift-modules %t/Bridging.h -O -o %t/bridging.pch > %t/keys.json +// RUN: %{python} %S/Inputs/ExtractOutputKey.py %t/keys.json %t/Bridging.h > %t/key + +// RUN: %{python} %S/Inputs/BuildCommandExtractor.py %t/deps.json Test > %t/MyApp.cmd +// RUN: echo "\"-disable-implicit-string-processing-module-import\"" >> %t/MyApp.cmd +// RUN: echo "\"-disable-implicit-concurrency-module-import\"" >> %t/MyApp.cmd +// RUN: echo "\"-disable-implicit-swift-modules\"" >> %t/MyApp.cmd +// RUN: echo "\"-import-objc-header\"" >> %t/MyApp.cmd +// RUN: echo "\"%t/bridging.pch\"" >> %t/MyApp.cmd +// RUN: echo "\"-bridging-header-pch-key\"" >> %t/MyApp.cmd +// RUN: echo "\"@%t/key\"" >> %t/MyApp.cmd +// RUN: echo "\"-explicit-swift-module-map-file\"" >> %t/MyApp.cmd +// RUN: echo "\"@%t/map.casid\"" >> %t/MyApp.cmd + +// RUN: %target-swift-frontend -cache-compile-job -module-name Test -O -cas-path %t/cas @%t/MyApp.cmd %t/test.swift \ +// RUN: -emit-module -o %t/Test.swiftmodule + +// RUN: %target-swift-frontend -typecheck -module-name User -swift-version 5 \ +// RUN: -disable-implicit-string-processing-module-import -disable-implicit-concurrency-module-import \ +// RUN: -Xcc -fmodule-map-file=%t/a.modulemap -Xcc -fmodule-map-file=%t/b.modulemap \ +// RUN: -I %t %t/user.swift + //--- test.swift import B -public func test() {} +public func test() { + b() +} +public class TestB: B {} + +//--- user.swift +import Test + +func user() { + var b: TestB + test() +} //--- Bridging.h #include "Foo.h" @@ -42,6 +95,8 @@ public func test() {} //--- b.h void b(void); +@interface B +@end //--- a.modulemap module A { From e654e371de66b078a2f820475be99d512c08fef3 Mon Sep 17 00:00:00 2001 From: Steven Wu Date: Fri, 5 Apr 2024 17:30:23 -0700 Subject: [PATCH 2/2] [BridgingHeader] Implicit import bridging header from CAS module The binary module built from a CAS build will have the embeded bridging header info with 0 modTime. Allow a regular build to import such a module with the same behavior as if the module is built from a regular build. rdar://126221616 --- lib/ClangImporter/ClangImporter.cpp | 7 +++++- test/CAS/bridging-header.swift | 37 +++++++++++++++++++++++++++-- 2 files changed, 41 insertions(+), 3 deletions(-) diff --git a/lib/ClangImporter/ClangImporter.cpp b/lib/ClangImporter/ClangImporter.cpp index b6e6734afd0de..a65c78ba54de3 100644 --- a/lib/ClangImporter/ClangImporter.cpp +++ b/lib/ClangImporter/ClangImporter.cpp @@ -1727,8 +1727,13 @@ bool ClangImporter::importHeader(StringRef header, ModuleDecl *adapter, StringRef cachedContents, SourceLoc diagLoc) { clang::FileManager &fileManager = Impl.Instance->getFileManager(); auto headerFile = fileManager.getFile(header, /*OpenFile=*/true); + // Prefer importing the header directly if the header content matches by + // checking size and mod time. This allows correct import if some no-modular + // headers are already imported into clang importer. If mod time is zero, then + // the module should be built from CAS and there is no mod time to verify. if (headerFile && (*headerFile)->getSize() == expectedSize && - (*headerFile)->getModificationTime() == expectedModTime) { + (expectedModTime == 0 || + (*headerFile)->getModificationTime() == expectedModTime)) { return importBridgingHeader(header, adapter, diagLoc, false, true); } diff --git a/test/CAS/bridging-header.swift b/test/CAS/bridging-header.swift index 52c6cd722d933..bc0691e9af0f9 100644 --- a/test/CAS/bridging-header.swift +++ b/test/CAS/bridging-header.swift @@ -1,3 +1,4 @@ +// REQUIRES: objc_interop // RUN: %empty-directory(%t) // RUN: split-file %s %t @@ -64,13 +65,28 @@ // RUN: %target-swift-frontend -cache-compile-job -module-name Test -O -cas-path %t/cas @%t/MyApp.cmd %t/test.swift \ // RUN: -emit-module -o %t/Test.swiftmodule +/// Importing binary module with bridging header built from CAS from a regluar build. +/// This should succeed even it is also importing a bridging header that shares same header dependencies (with proper header guard). // RUN: %target-swift-frontend -typecheck -module-name User -swift-version 5 \ // RUN: -disable-implicit-string-processing-module-import -disable-implicit-concurrency-module-import \ // RUN: -Xcc -fmodule-map-file=%t/a.modulemap -Xcc -fmodule-map-file=%t/b.modulemap \ -// RUN: -I %t %t/user.swift +// RUN: -I %t %t/user.swift -import-objc-header %t/Bridging2.h + +/// Importing binary module with bridging header built from CAS from a cached build. This should work without additional bridging header deps. +// RUN: %target-swift-frontend -scan-dependencies -module-name User -module-cache-path %t/clang-module-cache -O \ +// RUN: -disable-implicit-string-processing-module-import -disable-implicit-concurrency-module-import \ +// RUN: %t/user.swift -o %t/deps2.json -swift-version 5 -cache-compile-job -cas-path %t/cas \ +// RUN: -Xcc -fmodule-map-file=%t/a.modulemap -Xcc -fmodule-map-file=%t/b.modulemap -I %t + +// RUN: %{python} %S/Inputs/GenerateExplicitModuleMap.py %t/deps2.json > %t/map2.json +// RUN: llvm-cas --cas %t/cas --make-blob --data %t/map2.json > %t/map2.casid +// RUN: %{python} %S/Inputs/BuildCommandExtractor.py %t/deps2.json User > %t/User.cmd +// RUN: %target-swift-frontend -cache-compile-job -module-name User -O -cas-path %t/cas \ +// RUN: -disable-implicit-string-processing-module-import -disable-implicit-concurrency-module-import -disable-implicit-swift-modules \ +// RUN: -explicit-swift-module-map-file @%t/map2.casid @%t/User.cmd %t/user.swift \ +// RUN: -emit-module -o %t/User.swiftmodule //--- test.swift -import B public func test() { b() } @@ -84,14 +100,31 @@ func user() { test() } +extension A { + public func testA() {} +} + + //--- Bridging.h #include "Foo.h" +#include "Foo2.h" + +//--- Bridging2.h +#include "Foo.h" +#include "Foo2.h" //--- Foo.h #import "a.h" +//--- Foo2.h +#pragma once +int Foo = 0; + //--- a.h #include "b.h" +struct A { + int a; +}; //--- b.h void b(void);