ModuleInterface: Setup logic to load swiftinterfaces by default

xymus · xymus · commit d66369626ade · 2024-02-14T10:49:50.000-08:00
The Swift compiler can load either the binary swiftmodule file or the
textual swiftinterface file when importing a module. It currently picks
the swiftmodule over the swiftinterface, unless there’s an exception. We
should flip the default for distributed modules, prefer the
swiftinterface over the swiftmodule unless there’s an exception.

rdar://122955640
diff --git a/include/swift/AST/DiagnosticsFrontend.def b/include/swift/AST/DiagnosticsFrontend.def
@@ -444,11 +444,19 @@ NOTE(compiled_module_ignored_reason,none,
      "|it belongs to a framework in the SDK"
      "|loading from module interfaces is preferred"
      "|it's a compiler host module"
-     "|the module name is blocklisted}1",
+     "|the module name is blocklisted,"
+     "|the default is to load from module interfaces}1",
      (StringRef, unsigned))
 NOTE(out_of_date_module_here,none,
      "%select{compiled|cached|forwarding|prebuilt}0 module is out of date: '%1'",
      (unsigned, StringRef))
+NOTE(module_interface_ignored_reason,none,
+     "not defaulting to module interface because %select{%error"
+     "|it is a local module"
+     "|it was blocklisted"
+     "|the reader is a debugger}1"
+     ", preferring binary module at '%0'",
+     (StringRef, unsigned))
 NOTE(module_interface_dependency_out_of_date,none,
      "dependency is out of date: '%0'",
      (StringRef))
diff --git a/lib/Frontend/ModuleInterfaceLoader.cpp b/lib/Frontend/ModuleInterfaceLoader.cpp
@@ -240,12 +240,21 @@ struct ModuleRebuildInfo {
     InterfacePreferred,
     CompilerHostModule,
     Blocklisted,
+    DistributedInterfaceByDefault,
+  };
+  // Keep aligned with diag::module_interface_ignored_reason.
+  enum class ReasonModuleInterfaceIgnored {
+    NotIgnored,
+    LocalModule,
+    Blocklisted,
+    Debugger,
   };
   struct CandidateModule {
     std::string path;
     llvm::Optional<serialization::Status> serializationStatus;
     ModuleKind kind;
     ReasonIgnored reasonIgnored;
+    ReasonModuleInterfaceIgnored reasonModuleInterfaceIgnored;
     SmallVector<std::string, 10> outOfDateDependencies;
     SmallVector<std::string, 10> missingDependencies;
   };
@@ -259,6 +268,7 @@ struct ModuleRebuildInfo {
                                 llvm::None,
                                 ModuleKind::Normal,
                                 ReasonIgnored::NotIgnored,
+                                ReasonModuleInterfaceIgnored::NotIgnored,
                                 {},
                                 {}});
     return candidateModules.back();
@@ -290,13 +300,21 @@ struct ModuleRebuildInfo {
         .missingDependencies.push_back(depPath.str());
   }
 
-  /// Sets the reason that the module at \c path was ignored. If this is
+  /// Sets the reason that the module at \c modulePath was ignored. If this is
   /// anything besides \c NotIgnored a note will be added stating why the module
   /// was ignored.
   void addIgnoredModule(StringRef modulePath, ReasonIgnored reasonIgnored) {
     getOrInsertCandidateModule(modulePath).reasonIgnored = reasonIgnored;
   }
 
+  /// Record why no swiftinterfaces were preferred over the binary swiftmodule
+  /// at \c modulePath.
+  void addIgnoredModuleInterface(StringRef modulePath,
+                                 ReasonModuleInterfaceIgnored reasonIgnored) {
+    getOrInsertCandidateModule(modulePath).reasonModuleInterfaceIgnored =
+                                                                 reasonIgnored;
+  }
+
   /// Determines if we saw the given module path and registered is as out of
   /// date.
   bool sawOutOfDateModule(StringRef modulePath) {
@@ -365,7 +383,7 @@ struct ModuleRebuildInfo {
     // We may have found multiple failing modules, that failed for different
     // reasons. Emit a note for each of them.
     for (auto &mod : candidateModules) {
-      // If a the compiled module was ignored, diagnose the reason.
+      // If the compiled module was ignored, diagnose the reason.
       if (mod.reasonIgnored != ReasonIgnored::NotIgnored) {
         diags.diagnose(loc, diag::compiled_module_ignored_reason, mod.path,
                        (unsigned)mod.reasonIgnored);
@@ -397,6 +415,19 @@ struct ModuleRebuildInfo {
       }
     }
   }
+
+  /// Emits a diagnostic for the reason why binary swiftmodules were preferred
+  /// over textual swiftinterfaces.
+  void diagnoseIgnoredModuleInterfaces(ASTContext &ctx, SourceLoc loc) {
+    for (auto &mod : candidateModules) {
+      auto interfaceIgnore = mod.reasonModuleInterfaceIgnored;
+      if (interfaceIgnore == ReasonModuleInterfaceIgnored::NotIgnored)
+        continue;
+
+      ctx.Diags.diagnose(loc, diag::module_interface_ignored_reason,
+                         mod.path, (unsigned)interfaceIgnore);
+    }
+  }
 };
 
 /// Constructs the full path of the dependency \p dep by prepending the SDK
@@ -759,6 +790,12 @@ class ModuleInterfaceLoaderImpl {
     return pathStartsWith(hostPath, path);
   }
 
+  bool isInSDK(StringRef path) {
+    StringRef sdkPath = ctx.SearchPathOpts.getSDKPath();
+    if (sdkPath.empty()) return false;
+    return pathStartsWith(sdkPath, path);
+  }
+
   bool isInSystemFrameworks(StringRef path, bool publicFramework) {
     StringRef sdkPath = ctx.SearchPathOpts.getSDKPath();
     if (sdkPath.empty()) return false;
@@ -773,26 +810,64 @@ class ModuleInterfaceLoaderImpl {
 
   std::pair<std::string, std::string> getCompiledModuleCandidates() {
     using ReasonIgnored = ModuleRebuildInfo::ReasonIgnored;
+    using ReasonModuleInterfaceIgnored =
+                               ModuleRebuildInfo::ReasonModuleInterfaceIgnored;
     std::pair<std::string, std::string> result;
-    // Should we attempt to load a swiftmodule adjacent to the swiftinterface?
-    bool shouldLoadAdjacentModule = !ctx.IgnoreAdjacentModules;
 
-    if (modulePath.contains(".sdk")) {
-      if (ctx.blockListConfig.hasBlockListAction(moduleName,
-          BlockListKeyKind::ModuleName, BlockListAction::ShouldUseTextualModule)) {
-        shouldLoadAdjacentModule = false;
-        rebuildInfo.addIgnoredModule(modulePath, ReasonIgnored::Blocklisted);
+    bool ignoreByDefault = ctx.blockListConfig.hasBlockListAction(
+                                       "Swift_UseSwiftinterfaceByDefault",
+                                       BlockListKeyKind::ModuleName,
+                                       BlockListAction::ShouldUseBinaryModule);
+    bool shouldLoadAdjacentModule;
+    if (ignoreByDefault) {
+      ReasonModuleInterfaceIgnored ignore =
+        ReasonModuleInterfaceIgnored::NotIgnored;
+
+      if (!isInSDK(modulePath) &&
+          !isInResourceHostDir(modulePath)) {
+        ignore = ReasonModuleInterfaceIgnored::LocalModule;
+      } else if (ctx.blockListConfig.hasBlockListAction(moduleName,
+                                     BlockListKeyKind::ModuleName,
+                                     BlockListAction::ShouldUseBinaryModule)) {
+        ignore = ReasonModuleInterfaceIgnored::Blocklisted;
+      } else if (ctx.LangOpts.DebuggerSupport) {
+        ignore = ReasonModuleInterfaceIgnored::Debugger;
       }
-    }
 
-    // Don't use the adjacent swiftmodule for frameworks from the public
-    // Frameworks folder of the SDK.
-    if (isInSystemFrameworks(modulePath, /*publicFramework*/true)) {
-      shouldLoadAdjacentModule = false;
-      rebuildInfo.addIgnoredModule(modulePath, ReasonIgnored::PublicFramework);
-    } else if (isInResourceHostDir(modulePath)) {
-      shouldLoadAdjacentModule = false;
-      rebuildInfo.addIgnoredModule(modulePath, ReasonIgnored::CompilerHostModule);
+      shouldLoadAdjacentModule =
+        ignore != ReasonModuleInterfaceIgnored::NotIgnored;
+      if (shouldLoadAdjacentModule) {
+        // Prefer the swiftmodule.
+        rebuildInfo.addIgnoredModuleInterface(modulePath, ignore);
+      } else {
+        // Prefer the swiftinterface.
+        rebuildInfo.addIgnoredModule(modulePath,
+                                 ReasonIgnored::DistributedInterfaceByDefault);
+      }
+    } else {
+      // Should we attempt to load a swiftmodule adjacent to the swiftinterface?
+      shouldLoadAdjacentModule = !ctx.IgnoreAdjacentModules;
+
+      if (modulePath.contains(".sdk")) {
+        if (ctx.blockListConfig.hasBlockListAction(moduleName,
+                                    BlockListKeyKind::ModuleName,
+                                    BlockListAction::ShouldUseTextualModule)) {
+          shouldLoadAdjacentModule = false;
+          rebuildInfo.addIgnoredModule(modulePath, ReasonIgnored::Blocklisted);
+        }
+      }
+
+      // Don't use the adjacent swiftmodule for frameworks from the public
+      // Frameworks folder of the SDK.
+      if (isInSystemFrameworks(modulePath, /*publicFramework*/true)) {
+        shouldLoadAdjacentModule = false;
+        rebuildInfo.addIgnoredModule(modulePath,
+                                     ReasonIgnored::PublicFramework);
+      } else if (isInResourceHostDir(modulePath)) {
+        shouldLoadAdjacentModule = false;
+        rebuildInfo.addIgnoredModule(modulePath,
+                                     ReasonIgnored::CompilerHostModule);
+      }
     }
 
     switch (loadMode) {
@@ -1075,8 +1150,10 @@ class ModuleInterfaceLoaderImpl {
     // If we errored with anything other than 'no such file or directory',
     // fail this load and let the other module loader diagnose it.
     if (!moduleOrErr &&
-        moduleOrErr.getError() != std::errc::no_such_file_or_directory)
+        moduleOrErr.getError() != std::errc::no_such_file_or_directory) {
+      rebuildInfo.diagnoseIgnoredModuleInterfaces(ctx, diagnosticLoc);
       return moduleOrErr.getError();
+    }
 
     // We discovered a module! Return that module's buffer so we can load it.
     if (moduleOrErr) {
@@ -1099,6 +1176,7 @@ class ModuleInterfaceLoaderImpl {
         }
       }
 
+
       return std::move(module.moduleBuffer);
     }
     // If building from interface is disabled, return error.
@@ -1114,6 +1192,7 @@ class ModuleInterfaceLoaderImpl {
     // Note that we use `diags` so that we emit this remark even when we're
     // emitting other messages to `emptyDiags` (see below); these act as status
     // messages to explain what's taking so long.
+    // XYHERE for print even on positive? or alternative
     auto remarkRebuildAll = [&]() {
       rebuildInfo.diagnose(ctx, diags, prebuiltCacheDir, diagnosticLoc,
                            diag::rebuilding_module_from_interface, moduleName,
diff --git a/test/ModuleInterface/prefer-swiftinterface-by-default.swift b/test/ModuleInterface/prefer-swiftinterface-by-default.swift
@@ -0,0 +1,95 @@
+/// The blocklist can enable loading distributed swiftinterfaces by default.
+
+// RUN: %empty-directory(%t)
+// RUN: split-file %s %t --leading-lines
+// REQUIRES: VENDOR=apple
+
+/// Setup the SDK and local modules.
+//// SDK module in S/L/F.
+// RUN: %target-swift-frontend -emit-module -module-name SDKModule %t/Empty.swift \
+// RUN:   -enable-library-evolution -swift-version 5 -parse-stdlib \
+// RUN:   -emit-module-path %t/sdk/System/Library/Frameworks/SDKModule.framework/Modules/SDKModule.swiftmodule/%target-swiftmodule-name \
+// RUN:   -emit-module-interface-path %t/sdk/System/Library/Frameworks/SDKModule.framework/Modules/SDKModule.swiftmodule/%target-swiftinterface-name
+// RUN: %target-swift-typecheck-module-from-interface(%t/sdk/System/Library/Frameworks/SDKModule.framework/Modules/SDKModule.swiftmodule/%target-swiftinterface-name) -module-name SDKModule
+
+//// SDK module not in S/L/F.
+// RUN: %target-swift-frontend -emit-module -module-name SDKModuleAtUnusualPath %t/Empty.swift \
+// RUN:   -enable-library-evolution -swift-version 5 -parse-stdlib \
+// RUN:   -emit-module-path %t/sdk/System/Library/OtherFrameworks/SDKModuleAtUnusualPath.framework/Modules/SDKModuleAtUnusualPath.swiftmodule/%target-swiftmodule-name \
+// RUN:   -emit-module-interface-path %t/sdk/System/Library/OtherFrameworks/SDKModuleAtUnusualPath.framework/Modules/SDKModuleAtUnusualPath.swiftmodule/%target-swiftinterface-name
+// RUN: %target-swift-typecheck-module-from-interface(%t/sdk/System/Library/OtherFrameworks/SDKModuleAtUnusualPath.framework/Modules/SDKModuleAtUnusualPath.swiftmodule/%target-swiftinterface-name) -module-name SDKModuleAtUnusualPath
+
+//// Local module.
+// RUN: %target-swift-frontend -emit-module -module-name LocalModule %t/Empty.swift \
+// RUN:   -enable-library-evolution -swift-version 5 -parse-stdlib \
+// RUN:   -emit-module-path %t/not_sdk/LocalModule.swiftmodule \
+// RUN:   -emit-module-interface-path %t/not_sdk/LocalModule.swiftinterface
+// RUN: %target-swift-typecheck-module-from-interface(%t/not_sdk/LocalModule.swiftinterface) -module-name LocalModule
+
+//// Host resource-dir module.
+// RUN: %target-swift-frontend -emit-module -module-name HostResourceDirModule %t/Empty.swift \
+// RUN:   -enable-library-evolution -swift-version 5 -parse-stdlib \
+// RUN:   -emit-module-path %t/res/host/HostResourceDirModule.swiftmodule \
+// RUN:   -emit-module-interface-path %t/res/host/HostResourceDirModule.swiftinterface
+// RUN: %target-swift-typecheck-module-from-interface(%t/res/host/HostResourceDirModule.swiftinterface) -module-name HostResourceDirModule
+
+//// Blocklisted module.
+// RUN: %target-swift-frontend -emit-module -module-name BlocklistedModule %t/Empty.swift \
+// RUN:   -enable-library-evolution -swift-version 5 -parse-stdlib \
+// RUN:   -emit-module-path %t/sdk/System/Library/Frameworks/BlocklistedModule.framework/Modules/BlocklistedModule.swiftmodule/%target-swiftmodule-name \
+// RUN:   -emit-module-interface-path %t/sdk/System/Library/Frameworks/BlocklistedModule.framework/Modules/BlocklistedModule.swiftmodule/%target-swiftinterface-name
+// RUN: %target-swift-typecheck-module-from-interface(%t/sdk/System/Library/Frameworks/BlocklistedModule.framework/Modules/BlocklistedModule.swiftmodule/%target-swiftinterface-name) -module-name BlocklistedModule
+
+/// Build a client preferring swiftinterfaces.
+// RUN: %target-swift-frontend -typecheck -module-name Main %t/Client_SwiftinterfacesByDefault.swift \
+// RUN:   -parse-stdlib -module-cache-path %t/cache \
+// RUN:   -blocklist-file %t/blocklistEnabled.yml \
+// RUN:   -sdk %t/sdk -I %t/not_sdk -resource-dir %t/res -I %t/res/host/ \
+// RUN:   -F %t/sdk/System/Library/OtherFrameworks/ \
+// RUN:   -Rmodule-interface-rebuild -verify
+
+/// Build a client preferring swiftmodules.
+// RUN: %empty-directory(%t/cache)
+// RUN: %target-swift-frontend -typecheck -module-name Main %t/Client_SwiftmodulesByDefault.swift \
+// RUN:   -parse-stdlib -module-cache-path %t/cache \
+// RUN:   -blocklist-file %t/blocklistDisabled.yml \
+// RUN:   -sdk %t/sdk -I %t/not_sdk -resource-dir %t/res -I %t/res/host/ \
+// RUN:   -F %t/sdk/System/Library/OtherFrameworks/ \
+// RUN:   -Rmodule-interface-rebuild -verify
+
+//--- Empty.swift
+
+//--- Client_SwiftinterfacesByDefault.swift
+/// New behavior
+import SDKModule // expected-remark {{rebuilding module 'SDKModule' from interface}}
+// expected-note @-1 {{was ignored because the default is to load from module interfaces}}
+import SDKModuleAtUnusualPath // expected-remark {{rebuilding module 'SDKModuleAtUnusualPath' from interface}}
+// expected-note @-1 {{was ignored because the default is to load from module interfaces}}
+import HostResourceDirModule // expected-remark {{rebuilding module 'HostResourceDirModule' from interface}}
+// expected-note @-1 {{was ignored because the default is to load from module interfaces}}
+import LocalModule // expected-note {{not defaulting to module interface because it is a local module, preferring binary module at}}
+import BlocklistedModule // expected-note {{not defaulting to module interface because it was blocklisted, preferring binary module at}}
+
+//--- Client_SwiftmodulesByDefault.swift
+/// Old behavior
+import SDKModule // expected-remark {{rebuilding module 'SDKModule' from interface}}
+// expected-note @-1 {{was ignored because it belongs to a framework in the SDK}}
+import SDKModuleAtUnusualPath
+import HostResourceDirModule // expected-remark {{rebuilding module 'HostResourceDirModule' from interface}}
+// expected-note @-1 {{was ignored because it's a compiler host module}}
+import LocalModule
+import BlocklistedModule // expected-remark {{rebuilding module 'BlocklistedModule' from interface}}
+// expected-note @-1 {{was ignored because it belongs to a framework in the SDK}}
+
+//--- blocklistDisabled.yml
+---
+ShouldUseBinaryModule:
+  ModuleName:
+    - BlocklistedModule
+
+//--- blocklistEnabled.yml
+---
+ShouldUseBinaryModule:
+  ModuleName:
+    - Swift_UseSwiftinterfaceByDefault
+    - BlocklistedModule
