Merge pull request #64323 from tshortli/avoid-adding-implicit-import-during-override-checking

Sema: Only add implicit imports to module interface during resilience typechecking

Author: tshortli

lib/AST/Module.cpp

@@ -3115,21 +3115,6 @@ RestrictedImportKind SourceFile::getRestrictedImportKind(const ModuleDecl *modul
   if (imports.isImportedBy(module, getParentModule()))
     return RestrictedImportKind::None;
 
-  if (importKind == RestrictedImportKind::MissingImport &&
-      (module->getLibraryLevel() == LibraryLevel::API ||
-       getParentModule()->getLibraryLevel() != LibraryLevel::API)) {
-    // Hack to fix swiftinterfaces in case of missing imports.
-    // We can get rid of this logic when we don't leak the use of non-locally
-    // imported things in API.
-    ImportPath::Element pathElement = {module->getName(), SourceLoc()};
-    auto pathArray = getASTContext().AllocateCopy(
-                       llvm::makeArrayRef(pathElement));
-    auto missingImport = ImportedModule(
-                           ImportPath::Access(pathArray),
-                           const_cast<ModuleDecl *>(module));
-    addMissingImportedModule(missingImport);
-  }
-
   return importKind;
 }
 

lib/Sema/ResilienceDiagnostics.cpp

@@ -28,6 +28,27 @@
 
 using namespace swift;
 
+static bool addMissingImport(SourceLoc loc, const Decl *D,
+                             const ExportContext &where) {
+  ASTContext &ctx = where.getDeclContext()->getASTContext();
+  ModuleDecl *M = D->getModuleContext();
+  auto *SF = where.getDeclContext()->getParentSourceFile();
+
+  // Only add imports of API level modules if this is an API level module.
+  if (M->getLibraryLevel() != LibraryLevel::API &&
+      SF->getParentModule()->getLibraryLevel() == LibraryLevel::API)
+    return false;
+
+  // Hack to fix swiftinterfaces in case of missing imports. We can get rid of
+  // this logic when we don't leak the use of non-locally imported things in
+  // API.
+  auto missingImport = ImportedModule(ImportPath::Access(),
+                                      const_cast<ModuleDecl *>(M));
+  SF->addMissingImportedModule(missingImport);
+  ctx.Diags.diagnose(loc, diag::missing_import_inserted, M->getName());
+  return true;
+}
+
 bool TypeChecker::diagnoseInlinableDeclRefAccess(SourceLoc loc,
                                                  const ValueDecl *D,
                                                  const ExportContext &where) {
@@ -184,8 +205,7 @@ static bool diagnoseTypeAliasDeclRefExportability(SourceLoc loc,
 
   if (originKind == DisallowedOriginKind::MissingImport &&
       !ctx.LangOpts.isSwiftVersionAtLeast(6))
-    ctx.Diags.diagnose(loc, diag::missing_import_inserted,
-                       definingModule->getName());
+    addMissingImport(loc, D, where);
 
   return true;
 }
@@ -202,7 +222,7 @@ static bool diagnoseValueDeclRefExportability(SourceLoc loc, const ValueDecl *D,
   if (originKind == DisallowedOriginKind::None)
     return false;
 
-  ASTContext &ctx = definingModule->getASTContext();
+  ASTContext &ctx = where.getDeclContext()->getASTContext();
 
   auto fragileKind = where.getFragileFunctionKind();
   auto reason = where.getExportabilityReason();
@@ -236,8 +256,7 @@ static bool diagnoseValueDeclRefExportability(SourceLoc loc, const ValueDecl *D,
 
     if (originKind == DisallowedOriginKind::MissingImport &&
         downgradeToWarning == DowngradeToWarning::Yes)
-      ctx.Diags.diagnose(loc, diag::missing_import_inserted,
-                         definingModule->getName());
+      addMissingImport(loc, D, where);
   }
 
   return true;
@@ -296,9 +315,9 @@ TypeChecker::diagnoseConformanceExportability(SourceLoc loc,
                                originKind == DisallowedOriginKind::MissingImport,
                                6);
 
-    if (originKind == DisallowedOriginKind::MissingImport &&
-        !ctx.LangOpts.isSwiftVersionAtLeast(6))
-      ctx.Diags.diagnose(loc, diag::missing_import_inserted,
-                         M->getName());
+  if (originKind == DisallowedOriginKind::MissingImport &&
+      !ctx.LangOpts.isSwiftVersionAtLeast(6))
+    addMissingImport(loc, ext, where);
+
   return true;
 }

test/ModuleInterface/implicit-import.swift

@@ -0,0 +1,64 @@
+// RUN: %empty-directory(%t)
+// RUN: split-file %s %t
+
+// RUN: %target-swift-emit-module-interface(%t/DefinesProtocol.swiftinterface) -I %t %t/DefinesProtocol.swift -module-name DefinesProtocol
+// RUN: %target-swift-typecheck-module-from-interface(%t/DefinesProtocol.swiftinterface) -I %t -module-name DefinesProtocol
+
+// RUN: %target-swift-emit-module-interface(%t/DefinesTypealias.swiftinterface) -I %t %t/DefinesTypealias.swift -module-name DefinesTypealias
+// RUN: %target-swift-typecheck-module-from-interface(%t/DefinesTypealias.swiftinterface) -I %t -module-name DefinesTypealias
+
+// RUN: %target-swift-emit-module-interface(%t/DefinesStruct.swiftinterface) -I %t %t/DefinesStruct.swift -module-name DefinesStruct
+// RUN: %target-swift-typecheck-module-from-interface(%t/DefinesStruct.swiftinterface) -I %t -module-name DefinesStruct
+
+// RUN: %target-swift-emit-module-interface(%t/DefinesExtension.swiftinterface) -I %t %t/DefinesExtension.swift -module-name DefinesExtension
+// RUN: %target-swift-typecheck-module-from-interface(%t/DefinesExtension.swiftinterface) -I %t -module-name DefinesExtension
+
+// RUN: %target-swift-emit-module-interface(%t/Client.swiftinterface) -verify -I %t %t/File2.swift %t/File1.swift -module-name Client
+// RUN: %target-swift-typecheck-module-from-interface(%t/Client.swiftinterface) -I %t -module-name Client
+// RUN: %FileCheck %t/File2.swift < %t/Client.swiftinterface
+
+//--- DefinesProtocol.swift
+
+public protocol P {}
+
+//--- DefinesTypealias.swift
+
+import DefinesProtocol
+
+public typealias Q = P
+
+//--- DefinesStruct.swift
+
+public struct C {}
+
+//--- DefinesExtension.swift
+
+import DefinesStruct
+
+extension C {
+  public func foo() {}
+}
+
+//--- File1.swift
+
+import DefinesExtension
+
+//--- File2.swift
+
+// CHECK:       import DefinesExtension
+// CHECK-NEXT:  import DefinesProtocol
+// CHECK-NEXT:  import DefinesStruct
+// CHECK-NEXT:  import DefinesTypealias
+
+import DefinesTypealias
+import DefinesStruct
+
+public func takesQ<T>(_ b: T) where T: Q { }
+// expected-warning @-1 {{'Q' aliases 'DefinesProtocol.P' and cannot be used here because 'DefinesProtocol' was not imported by this file; this is an error in Swift 6}}
+// expected-note @-2 {{The missing import of module 'DefinesProtocol' will be added implicitly}}
+
+@inlinable public func takesC(_ c: C) {
+  c.foo()
+  // expected-warning @-1 {{instance method 'foo()' cannot be used in an '@inlinable' function because 'DefinesExtension' was not imported by this file; this is an error in Swift 6}}
+  // expected-note @-2 {{The missing import of module 'DefinesExtension' will be added implicitly}}
+}