Don't open symbols into a module when evaluating canImport statements

The module loaders now have API to check whether a given module can be
imported without importing the referenced module.  This provides a
significant speed boost to condition resolution and no longer
introduces symbols from the referenced module into the current context
without the user explicitly requesting it.

The definition of ‘canImport’ does not necessarily mean that a full
import without error is possible, merely that the path to the import is
visible to the compiler and the module is loadable in some form or
another.

Note that this means this check is insufficient to guarantee that you
are on one platform or another.  For those kinds of checks, use
‘os(OSNAME)’.

Author: CodaFi

include/swift/AST/ASTContext.h

@@ -580,6 +580,13 @@ class ASTContext {
   /// Does nothing in non-asserts (NDEBUG) builds.
   void verifyAllLoadedModules() const;
 
+  /// \brief Check whether the module with a given name can be imported without
+  /// importing it.
+  ///
+  /// Note that even if this check succeeds, errors may still occur if the
+  /// module is loaded in full.
+  bool canImportModule(std::pair<Identifier, SourceLoc> ModulePath);
+
   /// \returns a module with a given name that was already loaded.  If the
   /// module was not loaded, returns nullptr.
   ModuleDecl *getLoadedModule(

include/swift/AST/ModuleLoader.h

@@ -74,6 +74,13 @@ class ModuleLoader {
 public:
   virtual ~ModuleLoader() = default;
 
+  /// \brief Check whether the module with a given name can be imported without
+  /// importing it.
+  ///
+  /// Note that even if this check succeeds, errors may still occur if the
+  /// module is loaded in full.
+  virtual bool canImportModule(std::pair<Identifier, SourceLoc> named) = 0;
+
   /// \brief Import a module with the given module path.
   ///
   /// \param importLoc The location of the 'import' keyword.

include/swift/ClangImporter/ClangImporter.h

@@ -94,6 +94,13 @@ class ClangImporter final : public ClangModuleLoader {
 
   ~ClangImporter();
 
+  /// \brief Check whether the module with a given name can be imported without
+  /// importing it.
+  ///
+  /// Note that even if this check succeeds, errors may still occur if the
+  /// module is loaded in full.
+  virtual bool canImportModule(std::pair<Identifier, SourceLoc> named) override;
+
   /// \brief Import a module with the given module path.
   ///
   /// Clang modules will be imported using the Objective-C ARC dialect,

include/swift/Sema/SourceLoader.h

@@ -48,6 +48,13 @@ class SourceLoader : public ModuleLoader {
   SourceLoader &operator=(const SourceLoader &) = delete;
   SourceLoader &operator=(SourceLoader &&) = delete;
 
+  /// \brief Check whether the module with a given name can be imported without
+  /// importing it.
+  ///
+  /// Note that even if this check succeeds, errors may still occur if the
+  /// module is loaded in full.
+  virtual bool canImportModule(std::pair<Identifier, SourceLoc> named) override;
+
   /// \brief Import a module with the given module path.
   ///
   /// \param importLoc The location of the 'import' keyword.

include/swift/Serialization/SerializedModuleLoader.h

@@ -48,6 +48,13 @@ class SerializedModuleLoader : public ModuleLoader {
   SerializedModuleLoader &operator=(const SerializedModuleLoader &) = delete;
   SerializedModuleLoader &operator=(SerializedModuleLoader &&) = delete;
 
+  /// \brief Check whether the module with a given name can be imported without
+  /// importing it.
+  ///
+  /// Note that even if this check succeeds, errors may still occur if the
+  /// module is loaded in full.
+  virtual bool canImportModule(std::pair<Identifier, SourceLoc> named) override;
+
   /// \brief Import a module with the given module path.
   ///
   /// \param importLoc The location of the 'import' keyword.

lib/AST/ASTContext.cpp

@@ -1279,6 +1279,20 @@ GenericEnvironment *ASTContext::getOrCreateCanonicalGenericEnvironment(
   return env;
 }
 
+bool ASTContext::canImportModule(std::pair<Identifier, SourceLoc> ModulePath) {
+  // If this module has already been successfully imported, it is importable.
+  if (getLoadedModule(ModulePath) != nullptr)
+    return true;
+
+  // Otherwise, ask the module loaders.
+  for (auto &importer : Impl.ModuleLoaders) {
+    if (importer->canImportModule(ModulePath)) {
+      return true;
+    }
+  }
+  return false;
+}
+
 Module *
 ASTContext::getModule(ArrayRef<std::pair<Identifier, SourceLoc>> ModulePath) {
   assert(!ModulePath.empty());

lib/ClangImporter/ClangImporter.cpp

@@ -1022,6 +1022,23 @@ bool ClangImporter::isModuleImported(const clang::Module *M) {
   return M->NameVisibility == clang::Module::NameVisibilityKind::AllVisible;
 }
 
+bool ClangImporter::canImportModule(std::pair<Identifier, SourceLoc> moduleID) {
+  // Look up the top-level module to see if it exists.
+  // FIXME: This only works with top-level modules.
+  auto &clangHeaderSearch = Impl.getClangPreprocessor().getHeaderSearchInfo();
+  clang::Module *clangModule =
+    clangHeaderSearch.lookupModule(moduleID.first.str());
+  if (!clangModule) {
+    return false;
+  }
+
+  clang::Module::Requirement r;
+  clang::Module::UnresolvedHeaderDirective mh;
+  clang::Module *m;
+  auto &ctx = Impl.getClangASTContext();
+  return clangModule->isAvailable(ctx.getLangOpts(), getTargetInfo(), r, mh, m);
+}
+
 Module *ClangImporter::loadModule(
     SourceLoc importLoc,
     ArrayRef<std::pair<Identifier, SourceLoc>> path) {

lib/Parse/ParseStmt.cpp

@@ -1790,8 +1790,7 @@ Parser::classifyConditionalCompilationExpr(Expr *condition,
             return None;
           }
 
-          auto *M = Context.getModule({ { argumentIdent, UDRE->getLoc() } });
-          return (M && !M->failedToLoad());
+          return Context.canImportModule({ argumentIdent, UDRE->getLoc() });
         }
 
         if (!fullCheck) {

lib/Sema/SourceLoader.cpp

@@ -70,6 +70,22 @@ class SkipNonTransparentFunctions : public DelayedParsingCallbacks {
 
 } // unnamed namespace
 
+bool SourceLoader::canImportModule(std::pair<Identifier, SourceLoc> ID) {
+  // Search the memory buffers to see if we can find this file on disk.
+  FileOrError inputFileOrError = findModule(Ctx, ID.first.str(),
+                                            ID.second);
+  if (!inputFileOrError) {
+    auto err = inputFileOrError.getError();
+    if (err != std::errc::no_such_file_or_directory) {
+      Ctx.Diags.diagnose(ID.second, diag::sema_opening_import,
+                         ID.first, err.message());
+    }
+
+    return false;
+  }
+  return true;
+}
+
 Module *SourceLoader::loadModule(SourceLoc importLoc,
                              ArrayRef<std::pair<Identifier, SourceLoc>> path) {
   // FIXME: Swift submodules?

lib/Serialization/SerializedModuleLoader.cpp

@@ -19,6 +19,7 @@
 #include "swift/Basic/SourceManager.h"
 #include "swift/Basic/Version.h"
 #include "llvm/ADT/SmallString.h"
+#include "llvm/Support/FileSystem.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/Debug.h"
@@ -39,15 +40,25 @@ SerializedModuleLoader::~SerializedModuleLoader() = default;
 static std::error_code
 openModuleFiles(StringRef DirName, StringRef ModuleFilename,
                 StringRef ModuleDocFilename,
-                std::unique_ptr<llvm::MemoryBuffer> &ModuleBuffer,
-                std::unique_ptr<llvm::MemoryBuffer> &ModuleDocBuffer,
+                std::unique_ptr<llvm::MemoryBuffer> *ModuleBuffer,
+                std::unique_ptr<llvm::MemoryBuffer> *ModuleDocBuffer,
                 llvm::SmallVectorImpl<char> &Scratch) {
+  assert(((ModuleBuffer && ModuleDocBuffer)
+            || (!ModuleBuffer && !ModuleDocBuffer))
+         && "Module and Module Doc buffer must both be initialized or NULL");
   // Try to open the module file first.  If we fail, don't even look for the
   // module documentation file.
   Scratch.clear();
   llvm::sys::path::append(Scratch, DirName, ModuleFilename);
+  // If there are no buffers to load into, simply check for the existence of
+  // the module file.
+  if (!(ModuleBuffer || ModuleDocBuffer)) {
+    return llvm::sys::fs::access(StringRef(Scratch.data(), Scratch.size()),
+                                 llvm::sys::fs::AccessMode::Exist);
+  }
+
   llvm::ErrorOr<std::unique_ptr<llvm::MemoryBuffer>> ModuleOrErr =
-    llvm::MemoryBuffer::getFile(StringRef(Scratch.data(), Scratch.size()));
+  llvm::MemoryBuffer::getFile(StringRef(Scratch.data(), Scratch.size()));
   if (!ModuleOrErr)
     return ModuleOrErr.getError();
 
@@ -56,21 +67,23 @@ openModuleFiles(StringRef DirName, StringRef ModuleFilename,
   Scratch.clear();
   llvm::sys::path::append(Scratch, DirName, ModuleDocFilename);
   llvm::ErrorOr<std::unique_ptr<llvm::MemoryBuffer>> ModuleDocOrErr =
-    llvm::MemoryBuffer::getFile(StringRef(Scratch.data(), Scratch.size()));
+  llvm::MemoryBuffer::getFile(StringRef(Scratch.data(), Scratch.size()));
   if (!ModuleDocOrErr &&
       ModuleDocOrErr.getError() != std::errc::no_such_file_or_directory) {
     return ModuleDocOrErr.getError();
   }
-  ModuleBuffer = std::move(ModuleOrErr.get());
+
+  *ModuleBuffer = std::move(ModuleOrErr.get());
   if (ModuleDocOrErr)
-    ModuleDocBuffer = std::move(ModuleDocOrErr.get());
+    *ModuleDocBuffer = std::move(ModuleDocOrErr.get());
+
   return std::error_code();
 }
 
 static bool
 findModule(ASTContext &ctx, AccessPathElem moduleID,
-           std::unique_ptr<llvm::MemoryBuffer> &moduleBuffer,
-           std::unique_ptr<llvm::MemoryBuffer> &moduleDocBuffer,
+           std::unique_ptr<llvm::MemoryBuffer> *moduleBuffer,
+           std::unique_ptr<llvm::MemoryBuffer> *moduleDocBuffer,
            bool &isFramework) {
   llvm::SmallString<64> moduleFilename(moduleID.first.str());
   moduleFilename += '.';
@@ -95,7 +108,6 @@ findModule(ASTContext &ctx, AccessPathElem moduleID,
 
   llvm::SmallString<128> scratch;
   llvm::SmallString<128> currPath;
-
   isFramework = false;
   for (auto path : ctx.SearchPathOpts.ImportSearchPaths) {
     auto err = openModuleFiles(path,
@@ -353,6 +365,21 @@ FileUnit *SerializedModuleLoader::loadAST(
   return nullptr;
 }
 
+bool
+SerializedModuleLoader::canImportModule(std::pair<Identifier, SourceLoc> mID) {
+  // First see if we find it in the registered memory buffers.
+  if (!MemoryBuffers.empty()) {
+    auto bufIter = MemoryBuffers.find(mID.first.str());
+    if (bufIter != MemoryBuffers.end()) {
+      return true;
+    }
+  }
+
+  // Otherwise look on disk.
+  bool isFramework = false;
+  return findModule(Ctx, mID, nullptr, nullptr, isFramework);
+}
+
 Module *SerializedModuleLoader::loadModule(SourceLoc importLoc,
                                            Module::AccessPathTy path) {
   // FIXME: Swift submodules?
@@ -378,7 +405,7 @@ Module *SerializedModuleLoader::loadModule(SourceLoc importLoc,
 
   // Otherwise look on disk.
   if (!moduleInputBuffer) {
-    if (!findModule(Ctx, moduleID, moduleInputBuffer, moduleDocInputBuffer,
+    if (!findModule(Ctx, moduleID, &moduleInputBuffer, &moduleDocInputBuffer,
                     isFramework)) {
       return nullptr;
     }

test/ClangImporter/Inputs/missing-requirement/module.modulemap

@@ -0,0 +1,3 @@
+module MissingRequirement {
+  requires missing
+}

test/ClangImporter/MixedSource/can_import_objc_idempotent.swift

@@ -0,0 +1,40 @@
+// RUN: rm -rf %t && mkdir -p %t
+// RUN: cp -R %S/Inputs/mixed-target %t
+
+// FIXME: BEGIN -enable-source-import hackaround
+// RUN: %target-swift-frontend(mock-sdk: %clang-importer-sdk) -emit-module -o %t %clang-importer-sdk-path/swift-modules/CoreGraphics.swift
+// RUN: %target-swift-frontend(mock-sdk: %clang-importer-sdk) -emit-module -o %t %clang-importer-sdk-path/swift-modules/Foundation.swift
+// FIXME: END -enable-source-import hackaround
+
+// RUN: %target-swift-frontend(mock-sdk: %clang-importer-sdk-nosource -I %t) -I %S/../Inputs/custom-modules -import-objc-header %t/mixed-target/header.h -emit-module-path %t/MixedWithHeader.swiftmodule %S/Inputs/mixed-with-header.swift %S/../../Inputs/empty.swift -module-name MixedWithHeader -disable-objc-attr-requires-foundation-module
+// RUN: %target-swift-frontend(mock-sdk: %clang-importer-sdk-nosource -I %t) -I %S/../Inputs/custom-modules -typecheck %s -verify
+
+// RUN: rm -rf %t/mixed-target/
+// RUN: %target-swift-frontend(mock-sdk: %clang-importer-sdk-nosource -I %t) -I %S/../Inputs/custom-modules -typecheck %s -verify
+
+// REQUIRES: objc_interop
+
+// Test that 'canImport(Foo)' directives do not open symbols from 'Foo' into the
+// current module.  Only an 'import Foo' statement should do this.
+
+#if canImport(AppKit)
+  class AppKitView : NSView {} // expected-error {{use of undeclared type 'NSView'}}
+#endif
+
+#if canImport(UIKit)
+  class UIKitView : UIView {} // expected-error {{use of undeclared type 'UIView'}}
+#endif
+
+#if canImport(CoreGraphics)
+  let square = CGRect(x: 100, y: 100, width: 100, height: 100) // expected-error {{use of unresolved identifier 'CGRect'}}
+  // expected-error@-1 {{'square' used within its own type}}
+  // expected-error@-2 {{could not infer type for 'square'}}
+  let (r, s) = square.divided(atDistance: 50, from: .minXEdge)
+#endif
+
+#if canImport(MixedWithHeader)
+let object = NSObject() // expected-error {{use of unresolved identifier 'NSObject'}}
+  // expected-error@-1 {{'object' used within its own type}}
+  // expected-error@-2 {{could not infer type for 'object'}}
+let someAPI = Derived() // expected-error {{use of unresolved identifier 'Derived'}}
+#endif

test/ClangImporter/can_import_missing_requirement.swift

@@ -0,0 +1,9 @@
+// RUN: %target-swift-frontend(mock-sdk: %clang-importer-sdk) -typecheck -I %S/Inputs/missing-requirement %s -verify
+
+// REQUIRES: objc_interop
+
+class Unique {}
+
+#if canImport(MissingRequirement)
+  class Unique {} // No diagnostic
+#endif

test/Parse/ConditionalCompilation/can_import_idempotent.swift

@@ -0,0 +1,10 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+//
+// RUN: echo "public var dummyVar = Int()" | %target-swift-frontend -module-name DummyModule -emit-module -o %t -
+// RUN: %target-swift-frontend -typecheck -verify -I %t %s
+
+#if canImport(DummyModule)
+print(DummyModule.dummyVar) // expected-error {{use of unresolved identifier 'DummyModule'}}
+print(dummyVar) // expected-error {{use of unresolved identifier 'dummyVar'}}
+#endif