AST: Fix source break with new shadowing rules

My recent refactoring of top-level lookup replaced the old
shadowing done as part of top-level lookup with two separate
rules:

- If all paths from the current source file to a module 'B'
  go through a module 'A', then declarations in 'A' shadow
  declarations in 'B'.

- If a declaration in module 'A' was found via a scoped
  import and a declaration with the same name in module 'B'
  was found via an unscoped import, prefer the declaration
  from module 'A'.

However this caused a source break when you have a scenario
like the following:

- A source file imports 'A', 'B', and 'B.Foo'.

- 'A' re-exports 'B'.

- Both 'A' and 'B' define a type named 'Foo'.

The problem is that the scoped import 'B.Foo' can actually
find both 'A.Foo' and 'B.Foo', since 'B' re-exports 'A'.

Furthermore, since the source file explicitly imports 'A',
'B' does not shadow 'A' in the import graph.

As a result neither shadowing rule would eliminate the
ambiguity.

The new rule combines the scoped import check and the
shadowing check by considering all access paths to 'A'
that are not shadowed by 'B'. Using this rule, 'A.Foo'
is only seen via the access path 'A', whereas 'B.Foo'
is seen under both 'B' and 'B.Foo'. Since 'B.Foo' is seen
via a scoped import and 'A.Foo' is only seen via an
unscoped import, we can conclude that 'B.Foo' shadows
'A.Foo'.

Fixes <rdar://problem/55205050>.

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)