[ScanDependency] Do not use public/private swiftinterface in the same package

When scanning finds a dependency in the same package, do not load
public/private swiftinterface since they do not have the package level
decl to compile the current module. Always prefer package module (if
enabled), or use binary module, unless it is building a public/private
swiftinterface file in which case the interface file is preferred.

This also does some clean up to sync up the code path between implicit
and explicit module finding path.

rdar://122356964

Author: cachemeifyoucan

include/swift/Basic/LangOptions.h

@@ -440,6 +440,9 @@ namespace swift {
     /// The name of the package this module belongs to.
     std::string PackageName;
 
+    /// Allow importing a non-package interface from the same package.
+    bool AllowNonPackageInterfaceImportFromSamePackage = false;
+
     /// Diagnose implicit 'override'.
     bool WarnImplicitOverrides = false;
 

include/swift/Frontend/FrontendInputsAndOutputs.h

@@ -172,6 +172,7 @@ class FrontendInputsAndOutputs {
   bool shouldTreatAsLLVM() const;
   bool shouldTreatAsSIL() const;
   bool shouldTreatAsModuleInterface() const;
+  bool shouldTreatAsNonPackageModuleInterface() const;
   bool shouldTreatAsObjCHeader() const;
 
   bool areAllNonPrimariesSIB() const;

include/swift/Serialization/SerializedModuleLoader.h

@@ -68,6 +68,11 @@ struct SerializedModuleBaseName {
   std::optional<std::string> findInterfacePath(llvm::vfs::FileSystem &fs,
                                                ASTContext &ctx) const;
 
+  /// Returns the package-name of the interface file.
+  std::optional<std::string>
+  getPackageNameFromInterface(StringRef interfacePath,
+                              llvm::vfs::FileSystem &fs) const;
+
   /// Returns the .package.swiftinterface path if its package-name also applies to
   /// the the importing module. Returns an empty optional otherwise.
   std::optional<std::string>

lib/Frontend/CompilerInvocation.cpp

@@ -974,8 +974,15 @@ static bool ParseLangArgs(LangOptions &Opts, ArgList &Args,
     auto pkgName = A->getValue();
     if (StringRef(pkgName).empty())
       Diags.diagnose(SourceLoc(), diag::error_empty_package_name);
-    else
+    else {
       Opts.PackageName = pkgName;
+      // Unless the input type is public or private swift interface, do not
+      // allow non package interface imports for dependencies in the same
+      // package.
+      Opts.AllowNonPackageInterfaceImportFromSamePackage =
+          FrontendOpts.InputsAndOutputs
+              .shouldTreatAsNonPackageModuleInterface();
+    }
   }
 
   if (const Arg *A = Args.getLastArg(OPT_require_explicit_availability_EQ)) {

lib/Frontend/FrontendInputsAndOutputs.cpp

@@ -203,6 +203,16 @@ bool FrontendInputsAndOutputs::shouldTreatAsModuleInterface() const {
   return InputType == file_types::TY_SwiftModuleInterfaceFile;
 }
 
+bool FrontendInputsAndOutputs::shouldTreatAsNonPackageModuleInterface() const {
+  if (!hasSingleInput())
+    return false;
+
+  file_types::ID InputType =
+      file_types::lookupTypeFromFilename(getFilenameOfFirstInput());
+  return InputType == file_types::TY_SwiftModuleInterfaceFile ||
+         InputType == file_types::TY_PrivateSwiftModuleInterfaceFile;
+}
+
 bool FrontendInputsAndOutputs::shouldTreatAsSIL() const {
   if (hasSingleInput()) {
     // If we have exactly one input filename, and its extension is "sil",

lib/Frontend/ModuleInterfaceLoader.cpp

@@ -1244,8 +1244,8 @@ std::error_code ModuleInterfaceLoader::findModuleFilesInDirectory(
 
   // First check to see if the .swiftinterface exists at all. Bail if not.
   auto &fs = *Ctx.SourceMgr.getFileSystem();
-  std::string InPath = BaseName.findInterfacePath(fs, Ctx).value_or("");
-  if (InPath.empty()) {
+  auto InPath = BaseName.findInterfacePath(fs, Ctx);
+  if (!InPath) {
     if (fs.exists(ModPath)) {
       LLVM_DEBUG(llvm::dbgs()
                  << "No .swiftinterface file found adjacent to module file "
@@ -1256,22 +1256,22 @@ std::error_code ModuleInterfaceLoader::findModuleFilesInDirectory(
   }
 
   if (ModuleInterfaceSourcePath)
-    ModuleInterfaceSourcePath->assign(InPath.begin(), InPath.end());
+    ModuleInterfaceSourcePath->assign(InPath->begin(), InPath->end());
 
   // If we've been told to skip building interfaces, we are done here and do
   // not need to have the module actually built. For example, if we are
   // currently answering a `canImport` query, it is enough to have found
   // the interface.
   if (skipBuildingInterface) {
     if (ModuleInterfacePath)
-      ModuleInterfacePath->assign(InPath.begin(), InPath.end());
+      ModuleInterfacePath->assign(InPath->begin(), InPath->end());
     return std::error_code();
   }
 
   // Create an instance of the Impl to do the heavy lifting.
   auto ModuleName = ModuleID.Item.str();
   ModuleInterfaceLoaderImpl Impl(
-      Ctx, ModPath, InPath, ModuleName, InterfaceChecker.CacheDir,
+      Ctx, ModPath, *InPath, ModuleName, InterfaceChecker.CacheDir,
       InterfaceChecker.PrebuiltCacheDir, InterfaceChecker.BackupInterfaceDir,
       ModuleID.Loc, InterfaceChecker.Opts,
       InterfaceChecker.RequiresOSSAModules,
@@ -1290,7 +1290,7 @@ std::error_code ModuleInterfaceLoader::findModuleFilesInDirectory(
   if (ModuleBuffer) {
     *ModuleBuffer = std::move(*ModuleBufferOrErr);
     if (ModuleInterfacePath)
-      ModuleInterfacePath->assign(InPath.begin(), InPath.end());
+      ModuleInterfacePath->assign(InPath->begin(), InPath->end());
   }
 
   // Open .swiftsourceinfo file if it's present.

lib/Serialization/ScanningLoaders.cpp

@@ -50,9 +50,9 @@ std::error_code SwiftModuleScanner::findModuleFilesInDirectory(
   auto &fs = *Ctx.SourceMgr.getFileSystem();
 
   auto ModPath = BaseName.getName(file_types::TY_SwiftModuleFile);
-  auto InPath = BaseName.getName(file_types::TY_SwiftModuleInterfaceFile);
+  auto InPath = BaseName.findInterfacePath(fs, Ctx);
 
-  if (LoadMode == ModuleLoadingMode::OnlySerialized || !fs.exists(InPath)) {
+  if (LoadMode == ModuleLoadingMode::OnlySerialized || !InPath) {
     if (fs.exists(ModPath)) {
       // The module file will be loaded directly.
       auto dependencies = scanModuleFile(ModPath, IsFramework);
@@ -61,26 +61,12 @@ std::error_code SwiftModuleScanner::findModuleFilesInDirectory(
         return std::error_code();
       }
       return dependencies.getError();
-    } else {
-      return std::make_error_code(std::errc::no_such_file_or_directory);
     }
+    return std::make_error_code(std::errc::no_such_file_or_directory);
   }
-  assert(fs.exists(InPath));
+  assert(InPath);
 
-  // Use package.swiftinterface if it exists and its package-name applies to
-  // the importer module.
-  auto PkgInPath = BaseName.getPackageInterfacePathIfInSamePackage(fs, Ctx).value_or("");
-  if (!PkgInPath.empty() && fs.exists(PkgInPath)) {
-    InPath = PkgInPath;
-  } else {
-    // If not in package, use the private interface file if exits.
-    auto PrivateInPath =
-    BaseName.getName(file_types::TY_PrivateSwiftModuleInterfaceFile);
-    if (fs.exists(PrivateInPath)) {
-      InPath = PrivateInPath;
-    }
-  }
-  auto dependencies = scanInterfaceFile(InPath, IsFramework);
+  auto dependencies = scanInterfaceFile(*InPath, IsFramework);
   if (dependencies) {
     this->dependencies = std::move(dependencies.get());
     return std::error_code();

lib/Serialization/SerializedModuleLoader.cpp

@@ -604,35 +604,43 @@ std::optional<std::string>
 SerializedModuleBaseName::getPackageInterfacePathIfInSamePackage(
     llvm::vfs::FileSystem &fs, ASTContext &ctx) const {
   if (!ctx.LangOpts.EnablePackageInterfaceLoad)
-    return {};
+    return std::nullopt;
 
   std::string packagePath{
-    getName(file_types::TY_PackageSwiftModuleInterfaceFile)};
+      getName(file_types::TY_PackageSwiftModuleInterfaceFile)};
 
   if (fs.exists(packagePath)) {
     // Read the interface file and extract its package-name argument value
-    std::string result;
-    if (auto packageFile = llvm::MemoryBuffer::getFile(packagePath)) {
-      llvm::BumpPtrAllocator alloc;
-      llvm::StringSaver argSaver(alloc);
-      SmallVector<const char*, 8> args;
-      (void)extractCompilerFlagsFromInterface(packagePath,
-                                              (*packageFile)->getBuffer(), argSaver, args);
-      for (unsigned I = 0, N = args.size(); I + 1 < N; I++) {
-        StringRef current(args[I]), next(args[I + 1]);
-        if (current == "-package-name") {
-          // Instead of `break` here, continue to get the last value in case of dupes,
-          // to be consistent with the default parsing logic.
-          result = next;
-        }
+    if (auto packageName = getPackageNameFromInterface(packagePath, fs)) {
+      // Return the .package.swiftinterface path if the package name applies to
+      // the importer module.
+      if (*packageName == ctx.LangOpts.PackageName)
+        return packagePath;
+    }
+  }
+  return std::nullopt;
+}
+
+std::optional<std::string>
+SerializedModuleBaseName::getPackageNameFromInterface(
+    StringRef interfacePath, llvm::vfs::FileSystem &fs) const {
+  std::optional<std::string> result;
+  if (auto interfaceFile = fs.getBufferForFile(interfacePath)) {
+    llvm::BumpPtrAllocator alloc;
+    llvm::StringSaver argSaver(alloc);
+    SmallVector<const char *, 8> args;
+    (void)extractCompilerFlagsFromInterface(
+        interfacePath, (*interfaceFile)->getBuffer(), argSaver, args);
+    for (unsigned I = 0, N = args.size(); I + 1 < N; I++) {
+      StringRef current(args[I]), next(args[I + 1]);
+      if (current == "-package-name") {
+        // Instead of `break` here, continue to get the last value in case of
+        // dupes, to be consistent with the default parsing logic.
+        result = next;
       }
     }
-    // Return the .package.swiftinterface path if the package name applies to
-    // the importer module.
-    if (!result.empty() && result == ctx.LangOpts.PackageName)
-      return packagePath;
   }
-  return {};
+  return result;
 }
 
 std::optional<std::string>
@@ -642,16 +650,18 @@ SerializedModuleBaseName::findInterfacePath(llvm::vfs::FileSystem &fs,
   // Ensure the public swiftinterface already exists, otherwise bail early
   // as it's considered the module doesn't exist.
   if (!fs.exists(interfacePath))
-    return {};
-
-  // Check if a package interface exists and if the package name applies to
-  // the importer module.
-  auto pkgPath = getPackageInterfacePathIfInSamePackage(fs, ctx).value_or("");
-  if (!pkgPath.empty() && fs.exists(pkgPath))
-    return pkgPath;
+    return std::nullopt;
+
+  // If in the same package, try return the package interface path if not
+  // preferring the interface file.
+  if (!ctx.LangOpts.AllowNonPackageInterfaceImportFromSamePackage) {
+    if (auto packageName = getPackageNameFromInterface(interfacePath, fs)) {
+      if (*packageName == ctx.LangOpts.PackageName)
+        return getPackageInterfacePathIfInSamePackage(fs, ctx);
+    }
+  }
 
-  // If above fails, use the existing logic as fallback.
-  // If present, use the private interface instead of the public one.
+  // Otherwise, use the private interface instead of the public one.
   std::string privatePath{
       getName(file_types::TY_PrivateSwiftModuleInterfaceFile)};
   if (fs.exists(privatePath))
@@ -704,35 +714,33 @@ bool SerializedModuleLoaderBase::findModule(
     std::optional<SerializedModuleBaseName> firstAbsoluteBaseName;
 
     for (const auto &targetSpecificBaseName : targetSpecificBaseNames) {
-      SerializedModuleBaseName
-      absoluteBaseName{currPath, targetSpecificBaseName};
+      SerializedModuleBaseName absoluteBaseName{currPath,
+                                                targetSpecificBaseName};
 
       if (!firstAbsoluteBaseName.has_value())
         firstAbsoluteBaseName.emplace(absoluteBaseName);
 
       auto result = findModuleFilesInDirectory(
           moduleID, absoluteBaseName, moduleInterfacePath,
           moduleInterfaceSourcePath, moduleBuffer, moduleDocBuffer,
-          moduleSourceInfoBuffer, skipBuildingInterface,
-          IsFramework, isTestableDependencyLookup);
-      if (!result) {
+          moduleSourceInfoBuffer, skipBuildingInterface, IsFramework,
+          isTestableDependencyLookup);
+      if (!result)
         return SearchResult::Found;
-      } else if (result == std::errc::not_supported) {
+      if (result == std::errc::not_supported)
         return SearchResult::Error;
-      } else if (result != std::errc::no_such_file_or_directory) {
+      if (result != std::errc::no_such_file_or_directory)
         return SearchResult::NotFound;
-      }
     }
 
     // We can only get here if all targetFileNamePairs failed with
     // 'std::errc::no_such_file_or_directory'.
-    if (firstAbsoluteBaseName
-        && maybeDiagnoseTargetMismatch(moduleID.Loc, moduleName,
-                                       *firstAbsoluteBaseName)) {
+    if (firstAbsoluteBaseName &&
+        maybeDiagnoseTargetMismatch(moduleID.Loc, moduleName,
+                                    *firstAbsoluteBaseName))
       return SearchResult::Error;
-    } else {
-      return SearchResult::NotFound;
-    }
+
+    return SearchResult::NotFound;
   };
 
   SmallVector<std::string, 4> InterestingFilenames = {
@@ -788,13 +796,11 @@ bool SerializedModuleLoaderBase::findModule(
           moduleID, absoluteBaseName, moduleInterfacePath,
           moduleInterfaceSourcePath, moduleBuffer, moduleDocBuffer,
           moduleSourceInfoBuffer, skipBuildingInterface, isFramework);
-      if (!result) {
+      if (!result)
         return true;
-      } else if (result == std::errc::not_supported) {
+      if (result == std::errc::not_supported)
         return false;
-      } else {
-        continue;
-      }
+      continue;
     }
     case ModuleSearchPathKind::Framework:
     case ModuleSearchPathKind::DarwinImplicitFramework: {

test/ScanDependencies/package_interface.swift

@@ -4,6 +4,7 @@
 // RUN: %target-swift-frontend -emit-module %t/Bar.swift -I %t \
 // RUN:   -module-name Bar -package-name barpkg \
 // RUN:   -enable-library-evolution -swift-version 5 \
+// RUN:   -o %t/Bar.swiftmodule \
 // RUN:   -emit-module-interface-path %t/Bar.swiftinterface \
 // RUN:   -emit-private-module-interface-path %t/Bar.private.swiftinterface \
 // RUN:   -emit-package-module-interface-path %t/Bar.package.swiftinterface
@@ -21,10 +22,16 @@
 // RUN:   %t/Client.swift -module-name Client -swift-version 5
 // RUN: %FileCheck %s --input-file=%t/deps3.json --check-prefix CHECK --check-prefix CHECK-PRIVATE
 
+/// If -experimental-package-interface-load is not used but in the same package, it should find the binary module
+// RUN: %target-swift-frontend -scan-dependencies -I %t \
+// RUN:   %t/Client.swift -module-name Client -package-name barpkg -swift-version 5 | \
+// RUN:   %FileCheck %s --check-prefix CHECK-BINARY
+
 // CHECK: "swift": "Bar"
 // CHECK: "modulePath": "{{.*}}{{/|\\}}Bar-{{.*}}.swiftmodule"
 // CHECK-PACKAGE: "moduleInterfacePath": "{{.*}}{{/|\\}}Bar.package.swiftinterface"
 // CHECK-PRIVATE: "moduleInterfacePath": "{{.*}}{{/|\\}}Bar.private.swiftinterface"
+// CHECK-BINARY: "swiftPrebuiltExternal": "Bar"
 
 //--- Bar.swift
 public enum PubEnum {

test/Serialization/load_package_module.swift

@@ -31,7 +31,7 @@
 
 // RUN: not %target-swift-frontend -typecheck %t/ClientLoadInterface.swift -package-name libPkg -I %t 2> %t/resultY.output
 // RUN: %FileCheck %s -check-prefix CHECK-Y < %t/resultY.output
-// CHECK-Y: error: module 'LibInterface' is in package 'libPkg' but was built from a non-package interface; modules of the same package can only be loaded if built from source or package interface: {{.*}}LibInterface.private.swiftinterface
+// CHECK-Y: error: no such module 'LibInterface'
 
 
 //--- Lib.swift