From a2dc674723c545933d96e425504ec204ba3b23bb Mon Sep 17 00:00:00 2001 From: Artem Chikin Date: Mon, 29 Jul 2024 17:16:54 -0700 Subject: [PATCH] [Dependency Scanning] Disable validation of Swift dependency modules' existing pre-built candidate binary module files in the scanner, on a non-caching build. As-is, this default interferes with the incremental build machinery which conservatively assumes that binary module dependencies must cause dependents to be re-built. --- include/swift/AST/SearchPathOptions.h | 7 ++++--- include/swift/Option/FrontendOptions.td | 2 ++ lib/Frontend/CompilerInvocation.cpp | 9 ++++++--- lib/Frontend/ModuleInterfaceLoader.cpp | 5 ++--- lib/Serialization/ScanningLoaders.cpp | 2 +- lib/Serialization/SerializedModuleLoader.cpp | 2 +- test/ScanDependencies/compiled_swift_modules.swift | 6 +++--- .../restrict-swiftmodule-to-revision.swift | 6 +++--- test/ScanDependencies/testable-dependencies.swift | 12 ++++++------ 9 files changed, 28 insertions(+), 23 deletions(-) diff --git a/include/swift/AST/SearchPathOptions.h b/include/swift/AST/SearchPathOptions.h index 62ae837cecb7c..0e2fb14451964 100644 --- a/include/swift/AST/SearchPathOptions.h +++ b/include/swift/AST/SearchPathOptions.h @@ -542,8 +542,9 @@ class SearchPathOptions { /// Specify the module loading behavior of the compilation. ModuleLoadingMode ModuleLoadMode = ModuleLoadingMode::PreferSerialized; - /// Legacy scanner search behavior. - bool NoScannerModuleValidation = false; + /// New scanner search behavior. Validate up-to-date existing Swift module + /// dependencies in the scanner itself. + bool ScannerModuleValidation = false; /// Return all module search paths that (non-recursively) contain a file whose /// name is in \p Filenames. @@ -593,7 +594,7 @@ class SearchPathOptions { hash_combine_range(RuntimeLibraryImportPaths.begin(), RuntimeLibraryImportPaths.end()), DisableModulesValidateSystemDependencies, - NoScannerModuleValidation, + ScannerModuleValidation, ModuleLoadMode); } diff --git a/include/swift/Option/FrontendOptions.td b/include/swift/Option/FrontendOptions.td index 734215f392992..6e6fedc4aa47d 100644 --- a/include/swift/Option/FrontendOptions.td +++ b/include/swift/Option/FrontendOptions.td @@ -1386,6 +1386,8 @@ def disable_sending_args_and_results_with_region_isolation : Flag<["-"], HelpText<"Disable sending args and results when region based isolation is enabled. Only enabled with asserts">, Flags<[HelpHidden]>; +def scanner_module_validation: Flag<["-"], "scanner-module-validation">, + HelpText<"Validate binary modules in the dependency scanner">; def no_scanner_module_validation: Flag<["-"], "no-scanner-module-validation">, HelpText<"Do not validate binary modules in scanner and delegate the validation to swift-frontend">; def module_load_mode: Separate<["-"], "module-load-mode">, diff --git a/lib/Frontend/CompilerInvocation.cpp b/lib/Frontend/CompilerInvocation.cpp index caea2b2606c1c..3cdd621963e85 100644 --- a/lib/Frontend/CompilerInvocation.cpp +++ b/lib/Frontend/CompilerInvocation.cpp @@ -2005,6 +2005,7 @@ static bool validateSwiftModuleFileArgumentAndAdd(const std::string &swiftModule static bool ParseSearchPathArgs(SearchPathOptions &Opts, ArgList &Args, DiagnosticEngine &Diags, + const CASOptions &CASOpts, StringRef workingDirectory) { using namespace options; namespace path = llvm::sys::path; @@ -2148,8 +2149,10 @@ static bool ParseSearchPathArgs(SearchPathOptions &Opts, ArgList &Args, Opts.ScannerPrefixMapper.push_back(Opt.str()); } - Opts.NoScannerModuleValidation |= - Args.hasArg(OPT_no_scanner_module_validation); + // rdar://132340493 disable scanner-side validation for non-caching builds + Opts.ScannerModuleValidation |= Args.hasFlag(OPT_scanner_module_validation, + OPT_no_scanner_module_validation, + CASOpts.EnableCaching); std::optional forceModuleLoadingMode; if (auto *A = Args.getLastArg(OPT_module_load_mode)) @@ -3556,7 +3559,7 @@ bool CompilerInvocation::parseArgs( ParseSymbolGraphArgs(SymbolGraphOpts, ParsedArgs, Diags, LangOpts); if (ParseSearchPathArgs(SearchPathOpts, ParsedArgs, Diags, - workingDirectory)) { + CASOpts, workingDirectory)) { return true; } diff --git a/lib/Frontend/ModuleInterfaceLoader.cpp b/lib/Frontend/ModuleInterfaceLoader.cpp index 9e6143fb42b41..c15909bcf811e 100644 --- a/lib/Frontend/ModuleInterfaceLoader.cpp +++ b/lib/Frontend/ModuleInterfaceLoader.cpp @@ -1403,7 +1403,7 @@ ModuleInterfaceCheckerImpl::getCompiledModuleCandidatesForInterface( auto validateModule = [&](StringRef modulePath) { // Legacy behavior do not validate module. - if (Ctx.SearchPathOpts.NoScannerModuleValidation) + if (!Ctx.SearchPathOpts.ScannerModuleValidation) return true; // If we picked the other module already, no need to validate this one since @@ -1922,8 +1922,7 @@ InterfaceSubContextDelegateImpl::InterfaceSubContextDelegateImpl( searchPathOpts.PluginSearchOpts; // Get module loading behavior options. - genericSubInvocation.getSearchPathOptions().NoScannerModuleValidation = - searchPathOpts.NoScannerModuleValidation; + genericSubInvocation.getSearchPathOptions().ScannerModuleValidation = searchPathOpts.ScannerModuleValidation; genericSubInvocation.getSearchPathOptions().ModuleLoadMode = searchPathOpts.ModuleLoadMode; diff --git a/lib/Serialization/ScanningLoaders.cpp b/lib/Serialization/ScanningLoaders.cpp index 0980246a27228..40e3747842c6d 100644 --- a/lib/Serialization/ScanningLoaders.cpp +++ b/lib/Serialization/ScanningLoaders.cpp @@ -160,7 +160,7 @@ SwiftModuleScanner::scanInterfaceFile(Twine moduleInterfacePath, auto compiledCandidates = getCompiledCandidates(Ctx, realModuleName.str(), InPath); if (!compiledCandidates.empty() && - !Ctx.SearchPathOpts.NoScannerModuleValidation) { + Ctx.SearchPathOpts.ScannerModuleValidation) { assert(compiledCandidates.size() == 1 && "Should only have 1 candidate module"); auto BinaryDep = scanModuleFile(compiledCandidates[0], isFramework, diff --git a/lib/Serialization/SerializedModuleLoader.cpp b/lib/Serialization/SerializedModuleLoader.cpp index b08fcc670841b..1376ad92b4161 100644 --- a/lib/Serialization/SerializedModuleLoader.cpp +++ b/lib/Serialization/SerializedModuleLoader.cpp @@ -455,7 +455,7 @@ SerializedModuleLoaderBase::scanModuleFile(Twine modulePath, bool isFramework, isRequiredOSSAModules(), Ctx.LangOpts.SDKName, Ctx.SearchPathOpts.DeserializedPathRecoverer, loadedModuleFile); - if (!Ctx.SearchPathOpts.NoScannerModuleValidation) { + if (Ctx.SearchPathOpts.ScannerModuleValidation) { // If failed to load, just ignore and return do not found. if (loadInfo.status != serialization::Status::Valid) { if (Ctx.LangOpts.EnableModuleLoadingRemarks) diff --git a/test/ScanDependencies/compiled_swift_modules.swift b/test/ScanDependencies/compiled_swift_modules.swift index 4ccda3f788fc1..df3b76a446041 100644 --- a/test/ScanDependencies/compiled_swift_modules.swift +++ b/test/ScanDependencies/compiled_swift_modules.swift @@ -10,7 +10,7 @@ import Foo // HAS_BINARY: "swiftPrebuiltExternal": "Foo" -// HAS_NO_COMPILED-NOT: "{{.*}}Foo.swiftmodule{{.*}}.swiftmodule" +// HAS_NO_COMPILED-NOT: "compiledModulePath":{{.*}}"{{.*}}Foo.swiftmodule{{.*}}.swiftmodule" // Step 1: build swift interface and swift module side by side // RUN: %target-swift-frontend -emit-module %t/Foo.swift -emit-module-path %t/Foo.swiftmodule/%target-swiftmodule-name -module-name Foo -emit-module-interface-path %t/Foo.swiftmodule/%target-swiftinterface-name @@ -20,7 +20,7 @@ import Foo // RUN: %validate-json %t/deps.json | %FileCheck %s -check-prefix=HAS_COMPILED /// Check scanner picked binary dependency if not requesting raw scan output. -// RUN: %target-swift-frontend -scan-dependencies %s -o %t/deps.json -I %t -emit-dependencies -emit-dependencies-path %t/deps.d +// RUN: %target-swift-frontend -scan-dependencies %s -scanner-module-validation -o %t/deps.json -I %t -emit-dependencies -emit-dependencies-path %t/deps.d // RUN: %validate-json %t/deps.json | %FileCheck %s -check-prefix=HAS_BINARY // Step 3: remove the adjacent module. @@ -38,7 +38,7 @@ import Foo // RUN: %validate-json %t/deps.json | %FileCheck %s -check-prefix=HAS_COMPILED /// Check scanner picked binary dependency if not requesting raw scan output. -// RUN: %target-swift-frontend -scan-dependencies %s -o %t/deps.json -I %t -emit-dependencies -emit-dependencies-path %t/deps.d -sdk %t -prebuilt-module-cache-path %t/ResourceDir/%target-sdk-name/prebuilt-modules +// RUN: %target-swift-frontend -scan-dependencies %s -scanner-module-validation -o %t/deps.json -I %t -emit-dependencies -emit-dependencies-path %t/deps.d -sdk %t -prebuilt-module-cache-path %t/ResourceDir/%target-sdk-name/prebuilt-modules // RUN: %validate-json %t/deps.json | %FileCheck %s -check-prefix=HAS_BINARY // Step 6: update the interface file from where the prebuilt module cache was built. diff --git a/test/ScanDependencies/restrict-swiftmodule-to-revision.swift b/test/ScanDependencies/restrict-swiftmodule-to-revision.swift index 3cc29829d5cb3..2221ca5d58d4c 100644 --- a/test/ScanDependencies/restrict-swiftmodule-to-revision.swift +++ b/test/ScanDependencies/restrict-swiftmodule-to-revision.swift @@ -17,21 +17,21 @@ // RUN: -emit-module-interface-path %t/revision/Foo.swiftinterface -enable-library-evolution %t/foo.swift // RUN: env SWIFT_DEBUG_FORCE_SWIFTMODULE_REVISION=1.0.0.0.1 \ -// RUN: %target-swift-frontend -scan-dependencies -module-name Test -O -module-load-mode prefer-binary \ +// RUN: %target-swift-frontend -scan-dependencies -scanner-module-validation -module-name Test -O -module-load-mode prefer-binary \ // RUN: -disable-implicit-string-processing-module-import -disable-implicit-concurrency-module-import -parse-stdlib \ // RUN: %t/main.swift -o %t/deps-1.json -swift-version 5 -I %t/match // RUN: %FileCheck %s --check-prefix=BINARY --input-file=%t/deps-1.json // RUN: env SWIFT_DEBUG_FORCE_SWIFTMODULE_REVISION=1.0.0.0.1 \ -// RUN: %target-swift-frontend -scan-dependencies -module-name Test -O -module-load-mode prefer-binary \ +// RUN: %target-swift-frontend -scan-dependencies -scanner-module-validation -module-name Test -O -module-load-mode prefer-binary \ // RUN: -disable-implicit-string-processing-module-import -disable-implicit-concurrency-module-import -parse-stdlib \ // RUN: %t/main.swift -o %t/deps-2.json -swift-version 5 -I %t/new // RUN: %FileCheck %s --check-prefix=TEXTUAL --input-file=%t/deps-2.json // RUN: env SWIFT_DEBUG_FORCE_SWIFTMODULE_REVISION=1.0.0.0.1 \ -// RUN: %target-swift-frontend -scan-dependencies -module-name Test -O -module-load-mode prefer-binary \ +// RUN: %target-swift-frontend -scan-dependencies -scanner-module-validation -module-name Test -O -module-load-mode prefer-binary \ // RUN: -disable-implicit-string-processing-module-import -disable-implicit-concurrency-module-import -parse-stdlib \ // RUN: %t/main.swift -o %t/deps-3.json -swift-version 5 -I %t/revision diff --git a/test/ScanDependencies/testable-dependencies.swift b/test/ScanDependencies/testable-dependencies.swift index 34724315488ce..b43f2c8ac9359 100644 --- a/test/ScanDependencies/testable-dependencies.swift +++ b/test/ScanDependencies/testable-dependencies.swift @@ -17,7 +17,7 @@ // RUN: %t/A.swift /// Import testable build, should use binary but no extra dependencies. -// RUN: %target-swift-frontend -scan-dependencies -module-load-mode prefer-serialized -module-name Test %t/main.swift \ +// RUN: %target-swift-frontend -scan-dependencies -scanner-module-validation -module-load-mode prefer-serialized -module-name Test %t/main.swift \ // RUN: -disable-implicit-string-processing-module-import -disable-implicit-concurrency-module-import -parse-stdlib \ // RUN: -o %t/deps1.json -I %t/testable -swift-version 5 -Rmodule-loading // RUN: %{python} %S/../CAS/Inputs/SwiftDepsExtractor.py %t/deps1.json Test directDependencies | %FileCheck %s --check-prefix TEST1 @@ -26,7 +26,7 @@ // EMPTY-NOT: B /// Import regular build, should use binary. -// RUN: %target-swift-frontend -scan-dependencies -module-load-mode prefer-serialized -module-name Test %t/main.swift \ +// RUN: %target-swift-frontend -scan-dependencies -scanner-module-validation -module-load-mode prefer-serialized -module-name Test %t/main.swift \ // RUN: -disable-implicit-string-processing-module-import -disable-implicit-concurrency-module-import -parse-stdlib \ // RUN: -o %t/deps2.json -I %t/regular -swift-version 5 -Rmodule-loading // RUN: %{python} %S/../CAS/Inputs/SwiftDepsExtractor.py %t/deps2.json Test directDependencies | %FileCheck %s --check-prefix TEST2 @@ -34,7 +34,7 @@ // RUN: %{python} %S/../CAS/Inputs/SwiftDepsExtractor.py %t/deps2.json swiftPrebuiltExternal:A directDependencies | %FileCheck %s --check-prefix EMPTY --allow-empty /// Testable import testable build, should use binary, even interface is preferred. -// RUN: %target-swift-frontend -scan-dependencies -module-load-mode prefer-interface -module-name Test %t/testable.swift \ +// RUN: %target-swift-frontend -scan-dependencies -scanner-module-validation -module-load-mode prefer-interface -module-name Test %t/testable.swift \ // RUN: -disable-implicit-string-processing-module-import -disable-implicit-concurrency-module-import -parse-stdlib -enable-testing \ // RUN: -o %t/deps3.json -I %t/testable -I %t/internal -swift-version 5 -Rmodule-loading // RUN: %{python} %S/../CAS/Inputs/SwiftDepsExtractor.py %t/deps3.json Test directDependencies | %FileCheck %s --check-prefix TEST3 @@ -43,7 +43,7 @@ // TEST3-A: "swift": "B" /// Testable import sees non-testable module first, keep searching. -// RUN: %target-swift-frontend -scan-dependencies -module-load-mode prefer-interface -module-name Test %t/testable.swift \ +// RUN: %target-swift-frontend -scan-dependencies -scanner-module-validation -module-load-mode prefer-interface -module-name Test %t/testable.swift \ // RUN: -disable-implicit-string-processing-module-import -disable-implicit-concurrency-module-import -parse-stdlib -enable-testing \ // RUN: -o %t/deps4.json -I %t/regular -I %t/testable -I %t/internal -swift-version 5 -Rmodule-loading 2>&1 | %FileCheck %s --check-prefix WARN // RUN: %{python} %S/../CAS/Inputs/SwiftDepsExtractor.py %t/deps4.json Test directDependencies | %FileCheck %s --check-prefix TEST4 @@ -52,7 +52,7 @@ // TEST4-A: "swift": "B" /// Testable import non-testable build enable testing, warning about the finding then error out. -// RUN: %target-swift-frontend -scan-dependencies -module-load-mode prefer-interface -module-name Test %t/testable.swift \ +// RUN: %target-swift-frontend -scan-dependencies -scanner-module-validation -module-load-mode prefer-interface -module-name Test %t/testable.swift \ // RUN: -disable-implicit-string-processing-module-import -disable-implicit-concurrency-module-import -parse-stdlib -enable-testing \ // RUN: -o %t/deps5.json -I %t/regular -swift-version 5 -Rmodule-loading 2>&1 | %FileCheck %s --check-prefix WARN --check-prefix ERROR // WARN: warning: ignore swiftmodule built without '-enable-testing' @@ -60,7 +60,7 @@ /// Regular import a testable module with no interface, don't load optional dependencies. // RUN: rm %t/testable/A.swiftinterface -// RUN: %target-swift-frontend -scan-dependencies -module-load-mode prefer-interface -module-name Test %t/main.swift \ +// RUN: %target-swift-frontend -scan-dependencies -scanner-module-validation -module-load-mode prefer-interface -module-name Test %t/main.swift \ // RUN: -disable-implicit-string-processing-module-import -disable-implicit-concurrency-module-import -parse-stdlib -enable-testing \ // RUN: -o %t/deps6.json -I %t/testable -swift-version 5 -Rmodule-loading // RUN: %{python} %S/../CAS/Inputs/SwiftDepsExtractor.py %t/deps6.json Test directDependencies | %FileCheck %s --check-prefix TEST6