Merge pull request #71857 from cachemeifyoucan/eng/PR-122356964

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

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