Merge pull request swiftlang#27160 from slavapestov/shadowing-scoped-import-fix

AST: Fix source break with new shadowing rules

Author: slavapestov

include/swift/AST/ImportCache.h

@@ -140,52 +140,13 @@ class alignas(ModuleDecl::ImportedModule) ImportCache {
     return !getAllVisibleAccessPaths(mod, dc).empty();
   }
 
-  /// Determines if 'mod' is visible from 'dc' as a result of a scoped import.
-  /// Note that if 'mod' was not imported from 'dc' at all, this also returns
-  /// false.
-  bool isScopedImport(const ModuleDecl *mod, DeclBaseName name,
-                      const DeclContext *dc) {
-    auto accessPaths = getAllVisibleAccessPaths(mod, dc);
-    for (auto accessPath : accessPaths) {
-      if (accessPath.empty())
-        continue;
-      if (ModuleDecl::matchesAccessPath(accessPath, name))
-        return true;
-    }
-
-    return false;
-  }
-
   /// Returns all access paths in 'mod' that are visible from 'dc' if we
   /// subtract imports of 'other'.
   ArrayRef<ModuleDecl::AccessPathTy>
   getAllAccessPathsNotShadowedBy(const ModuleDecl *mod,
                                  const ModuleDecl *other,
                                  const DeclContext *dc);
 
-  /// Returns 'true' if a declaration named 'name' defined in 'other' shadows
-  /// defined in 'mod', because no access paths can find 'name' in 'mod' from
-  /// 'dc' if we ignore imports of 'other'.
-  bool isShadowedBy(const ModuleDecl *mod,
-                    const ModuleDecl *other,
-                    DeclBaseName name,
-                    const DeclContext *dc) {
-     auto accessPaths = getAllAccessPathsNotShadowedBy(mod, other, dc);
-     return llvm::none_of(accessPaths,
-                          [&](ModuleDecl::AccessPathTy accessPath) {
-                            return ModuleDecl::matchesAccessPath(accessPath, name);
-                          });
-  }
-
-  /// Qualified lookup into types uses a slightly different check that does not
-  /// take access paths into account.
-  bool isShadowedBy(const ModuleDecl *mod,
-                    const ModuleDecl *other,
-                    const DeclContext *dc) {
-    auto accessPaths = getAllAccessPathsNotShadowedBy(mod, other, dc);
-    return accessPaths.empty();
-  }
-
   /// This is a hack to cope with main file parsing and REPL parsing, where
   /// we can add ImportDecls after name binding.
   void clear() {

lib/AST/NameLookup.cpp

@@ -193,6 +193,27 @@ static void recordShadowedDeclsAfterSignatureMatch(
     auto firstDecl = decls[firstIdx];
     auto firstModule = firstDecl->getModuleContext();
     auto name = firstDecl->getBaseName();
+
+    auto isShadowed = [&](ArrayRef<ModuleDecl::AccessPathTy> paths) {
+      for (auto path : paths) {
+        if (ModuleDecl::matchesAccessPath(path, name))
+          return false;
+      }
+
+      return true;
+    };
+
+    auto isScopedImport = [&](ArrayRef<ModuleDecl::AccessPathTy> paths) {
+      for (auto path : paths) {
+        if (path.empty())
+          continue;
+        if (ModuleDecl::matchesAccessPath(path, name))
+          return true;
+      }
+
+      return false;
+    };
+
     for (unsigned secondIdx : range(firstIdx + 1, decls.size())) {
       // Determine whether one module takes precedence over another.
       auto secondDecl = decls[secondIdx];
@@ -206,22 +227,28 @@ static void recordShadowedDeclsAfterSignatureMatch(
       if (firstModule != secondModule &&
           firstDecl->getDeclContext()->isModuleScopeContext() &&
           secondDecl->getDeclContext()->isModuleScopeContext()) {
-        // First, scoped imports shadow unscoped imports.
-        bool firstScoped = imports.isScopedImport(firstModule, name, dc);
-        bool secondScoped = imports.isScopedImport(secondModule, name, dc);
-        if (!firstScoped && secondScoped) {
+        auto firstPaths = imports.getAllAccessPathsNotShadowedBy(
+          firstModule, secondModule, dc);
+        auto secondPaths = imports.getAllAccessPathsNotShadowedBy(
+          secondModule, firstModule, dc);
+
+        // Check if one module shadows the other.
+        if (isShadowed(firstPaths)) {
           shadowed.insert(firstDecl);
           break;
-        } else if (firstScoped && !secondScoped) {
+        } else if (isShadowed(secondPaths)) {
           shadowed.insert(secondDecl);
           continue;
         }
 
-        // Now check if one module shadows the other.
-        if (imports.isShadowedBy(firstModule, secondModule, name, dc)) {
+        // We might be in a situation where neither module shadows the
+        // other, but one declaration is visible via a scoped import.
+        bool firstScoped = isScopedImport(firstPaths);
+        bool secondScoped = isScopedImport(secondPaths);
+        if (!firstScoped && secondScoped) {
           shadowed.insert(firstDecl);
           break;
-        } else if (imports.isShadowedBy(secondModule, firstModule, name, dc)) {
+        } else if (firstScoped && !secondScoped) {
           shadowed.insert(secondDecl);
           continue;
         }
@@ -278,10 +305,16 @@ static void recordShadowedDeclsAfterSignatureMatch(
       if (firstModule != secondModule &&
           !firstDecl->getDeclContext()->isModuleScopeContext() &&
           !secondDecl->getDeclContext()->isModuleScopeContext()) {
-        if (imports.isShadowedBy(firstModule, secondModule, dc)) {
+        auto firstPaths = imports.getAllAccessPathsNotShadowedBy(
+          firstModule, secondModule, dc);
+        auto secondPaths = imports.getAllAccessPathsNotShadowedBy(
+          secondModule, firstModule, dc);
+
+        // Check if one module shadows the other.
+        if (isShadowed(firstPaths)) {
           shadowed.insert(firstDecl);
           break;
-        } else if (imports.isShadowedBy(secondModule, firstModule, dc)) {
+        } else if (isShadowed(secondPaths)) {
           shadowed.insert(secondDecl);
           continue;
         }

test/NameBinding/Inputs/sdf.swift

@@ -0,0 +1,5 @@
+@_exported import asdf
+
+public struct S { public init(x: Int) {} }
+public struct D { public init(x: Int) {} }
+public struct F { public init(x: Int) {} }

test/NameBinding/import-resolution-3.swift

@@ -0,0 +1,10 @@
+// RUN: %empty-directory(%t)
+// RUN: %target-swift-frontend -emit-module -o %t %S/Inputs/asdf.swift
+// RUN: %target-swift-frontend -emit-module -o %t %S/Inputs/sdf.swift -I %t
+// RUN: %target-swift-frontend -typecheck %s -I %t -verify
+
+import asdf
+import sdf
+import struct sdf.S
+
+var uS: S = sdf.S(x: 123)