[Type checking] A declaration in an overlay can shadow an imported declaration.

If a declaration provided in a Swift overlay has the same signature as
a declaration imported from the Clang module it overlays, or some
private framework that re-exports its interface through that Clang
module, the declaration provided in the Swift overlay shadows the one
imported from Clang.

To make this work for unqualified lookup, allow the shadowing rules to
kick in there as well. Make this a very narrow fix, because the
underlying problems that prevented us from doing this shadowing still
persist.

Fixes rdar://problem/35691030 and its several dupes.

Author: DougGregor

lib/AST/NameLookup.cpp

@@ -15,7 +15,7 @@
 //===----------------------------------------------------------------------===//
 
 #include "NameLookupImpl.h"
-#include "swift/Basic/Statistic.h"
+#include "swift/ClangImporter/ClangImporter.h"
 #include "swift/AST/NameLookup.h"
 #include "swift/AST/ASTContext.h"
 #include "swift/AST/ASTScope.h"
@@ -26,6 +26,7 @@
 #include "swift/AST/Initializer.h"
 #include "swift/AST/ReferencedNameTracker.h"
 #include "swift/Basic/SourceManager.h"
+#include "swift/Basic/Statistic.h"
 #include "swift/Basic/STLExtras.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/TinyPtrVector.h"
@@ -281,20 +282,35 @@ bool swift::removeShadowedDecls(SmallVectorImpl<ValueDecl*> &decls,
         // module.
         // FIXME: This is a hack. We should query a (lazily-built, cached)
         // module graph to determine shadowing.
-        if ((firstModule == curModule) == (secondModule == curModule))
-          continue;
+        if ((firstModule == curModule) != (secondModule == curModule)) {
+          // If the first module is the current module, the second declaration
+          // is shadowed by the first.
+          if (firstModule == curModule) {
+            shadowed.insert(secondDecl);
+            continue;
+          }
 
-        // If the first module is the current module, the second declaration
-        // is shadowed by the first.
-        if (firstModule == curModule) {
-          shadowed.insert(secondDecl);
-          continue;
+          // Otherwise, the first declaration is shadowed by the second. There is
+          // no point in continuing to compare the first declaration to others.
+          shadowed.insert(firstDecl);
+          break;
         }
 
-        // Otherwise, the first declaration is shadowed by the second. There is
-        // no point in continuing to compare the first declaration to others.
-        shadowed.insert(firstDecl);
-        break;
+        // Prefer declarations in an overlay to similar declarations in
+        // the Clang module it customizes.
+        if (firstDecl->hasClangNode() != secondDecl->hasClangNode()) {
+          if (isInOverlayModuleForImportedModule(firstDecl->getDeclContext(),
+                                                 secondDecl->getDeclContext())){
+            shadowed.insert(secondDecl);
+            continue;
+          }
+
+          if (isInOverlayModuleForImportedModule(secondDecl->getDeclContext(),
+                                                 firstDecl->getDeclContext())) {
+            shadowed.insert(firstDecl);
+            break;
+          }
+        }
       }
     }
   }

lib/Sema/TypeCheckNameLookup.cpp

@@ -75,19 +75,37 @@ namespace {
         Options |= NameLookupFlags::IgnoreAccessControl;
     }
 
+    /// Determine whether we should filter out the results by removing
+    /// overridden and shadowed declarations.
+    /// FIXME: We should *always* do this, but there are weird assumptions
+    /// about the results of unqualified name lookup, e.g., that a local
+    /// variable not having a type indicates that it hasn't been seen yet.
+    bool shouldFilterResults() const {
+      // Member lookups always filter results.
+      if (IsMemberLookup) return true;
+
+      bool allAreInOtherModules = true;
+      auto currentModule = DC->getParentModule();
+      for (const auto &found : Result) {
+        // We found a member, so we need to filter.
+        if (found.getBaseDecl() != nullptr)
+          return true;
+
+        // We found something in our own module.
+        if (found.getValueDecl()->getDeclContext()->getParentModule() ==
+              currentModule)
+          allAreInOtherModules = false;
+      }
+
+      // FIXME: Only perform shadowing if we found things from other modules.
+      // This prevents us from introducing additional type-checking work
+      // during name lookup.
+      return allAreInOtherModules;
+    }
+
     ~LookupResultBuilder() {
-      // If any of the results have a base, we need to remove
-      // overridden and shadowed declarations.
-      // FIXME: We should *always* remove overridden and shadowed declarations,
-      // but there are weird assumptions about the results of unqualified
-      // name lookup, e.g., that a local variable not having a type indicates
-      // that it hasn't been seen yet.
-      if (!IsMemberLookup &&
-          std::find_if(Result.begin(), Result.end(),
-                       [](const LookupResultEntry &found) {
-                         return found.getBaseDecl() != nullptr;
-                       }) == Result.end())
-        return;
+      // Check whether we should do this filtering aat all.
+      if (!shouldFilterResults()) return;
 
       // Remove any overridden declarations from the found-declarations set.
       removeOverriddenDecls(FoundDecls);

test/ClangImporter/Inputs/privateframeworks/bridging-somekit.h

@@ -0,0 +1 @@
+#import <SomeKit/SKWidget.h>

test/ClangImporter/Inputs/privateframeworks/bridging-somekitcore.h

@@ -0,0 +1 @@
+#import <SomeKitCore/SKWidget.h>

test/ClangImporter/Inputs/privateframeworks/overlay/SomeKit.swift

@@ -47,3 +47,10 @@ public func inlineWidgetOperations(_ widget: SKWidget) {
   someKitGlobalFunc()
   hao.anObject = widget
 }
+
+@available(swift, deprecated: 4.2)
+public func someKitOtherGlobalFunc() { }
+
+extension SKWidget {
+  public var name: String { return "blah" }
+}

test/ClangImporter/Inputs/privateframeworks/withoutprivate/SomeKit.framework/Headers/SKWidget.h

@@ -24,3 +24,8 @@ typedef enum __attribute__((ns_error_domain(SKWidgetErrorDomain))) __attribute__
 @end
 
 extern void someKitGlobalFunc(void);
+
+static inline void someKitOtherGlobalFunc(void) { }
+
+extern NSString * _Nonnull someKitGetWidgetName(SKWidget * _Nonnull)
+  __attribute__((swift_name("getter:SKWidget.name(self:)")));

test/ClangImporter/Inputs/privateframeworks/withoutprivate/SomeKit.framework/Headers/SomeKit.apinotes

@@ -0,0 +1,10 @@
+---
+Name: SomeKit
+Functions:
+- Name: someKitOtherGlobalFunc
+  SwiftPrivate: true
+SwiftVersions:
+- Version: 4
+  Functions:
+  - Name: someKitOtherGlobalFunc
+    SwiftPrivate: false

test/ClangImporter/Inputs/privateframeworks/withprivate/SomeKit.framework/Headers/SomeKit.apinotes

@@ -0,0 +1,7 @@
+---
+Name: SomeKit
+SwiftVersions:
+- Version: 4
+  Functions:
+  - Name: someKitOtherGlobalFunc
+    SwiftPrivate: false

test/ClangImporter/Inputs/privateframeworks/withprivate/SomeKitCore.framework/Headers/SKWidget.h

@@ -25,3 +25,8 @@ typedef enum __attribute__((ns_error_domain(SKWidgetErrorDomain))) __attribute__
 @end
 
 extern void someKitGlobalFunc(void);
+
+static inline void someKitOtherGlobalFunc(void) { }
+
+extern NSString * _Nonnull someKitGetWidgetName(SKWidget * _Nonnull)
+  __attribute__((swift_name("getter:SKWidget.name(self:)")));

test/ClangImporter/Inputs/privateframeworks/withprivate/SomeKitCore.framework/Headers/SomeKitCore.apinotes

@@ -0,0 +1,10 @@
+---
+Name: SomeKitCore
+Functions:
+- Name: someKitOtherGlobalFunc
+  SwiftPrivate: true
+SwiftVersions:
+- Version: 4
+  Functions:
+  - Name: someKitOtherGlobalFunc
+    SwiftPrivate: false

test/ClangImporter/private_frameworks.swift

@@ -9,19 +9,19 @@
 // RUN: %target-swift-frontend(mock-sdk: %clang-importer-sdk-nosource -I %t) -emit-module -F %S/Inputs/privateframeworks/withprivate -o %t %S/Inputs/privateframeworks/overlay/SomeKit.swift
 
 // Use the overlay with private frameworks.
-// RUN: %target-swift-frontend(mock-sdk: %clang-importer-sdk-nosource -I %t) -emit-sil -o /dev/null -F %S/Inputs/privateframeworks/withprivate %s -verify
+// RUN: %target-swift-frontend(mock-sdk: %clang-importer-sdk-nosource -I %t) -emit-sil -o /dev/null -F %S/Inputs/privateframeworks/withprivate -swift-version 4 %s -import-objc-header %S/Inputs/privateframeworks/bridging-somekitcore.h -verify
 
 // Use the overlay without private frameworks.
-// RUN: %target-swift-frontend(mock-sdk: %clang-importer-sdk-nosource -I %t) -emit-sil -o /dev/null -F %S/Inputs/privateframeworks/withoutprivate -I %t %s
+// RUN: %target-swift-frontend(mock-sdk: %clang-importer-sdk-nosource -I %t) -emit-sil -o /dev/null -F %S/Inputs/privateframeworks/withoutprivate -I %t -swift-version 4 -import-objc-header %S/Inputs/privateframeworks/bridging-somekit.h %s
 
 // Build the overlay with public frameworks.
 // RUN: %target-swift-frontend(mock-sdk: %clang-importer-sdk-nosource -I %t) -emit-module -F %S/Inputs/privateframeworks/withoutprivate -o %t %S/Inputs/privateframeworks/overlay/SomeKit.swift
 
 // Use the overlay with private frameworks.
-// RUN: %target-swift-frontend(mock-sdk: %clang-importer-sdk-nosource -I %t) -emit-sil -o /dev/null -F %S/Inputs/privateframeworks/withprivate %s -verify
+// RUN: %target-swift-frontend(mock-sdk: %clang-importer-sdk-nosource -I %t) -emit-sil -o /dev/null -F %S/Inputs/privateframeworks/withprivate -swift-version 4 %s -import-objc-header %S/Inputs/privateframeworks/bridging-somekitcore.h -verify
 
 // Use the overlay without private frameworks.
-// RUN: %target-swift-frontend(mock-sdk: %clang-importer-sdk-nosource -I %t) -emit-sil -o /dev/null -F %S/Inputs/privateframeworks/withoutprivate -I %t %s
+// RUN: %target-swift-frontend(mock-sdk: %clang-importer-sdk-nosource -I %t) -emit-sil -o /dev/null -F %S/Inputs/privateframeworks/withoutprivate -I %t -swift-version 4 %s -import-objc-header %S/Inputs/privateframeworks/bridging-somekit.h 
 
 // REQUIRES: objc_interop
 
@@ -36,10 +36,17 @@ func testWidget(widget: SKWidget) {
 
   widget.doSomethingElse(widget)
   inlineWidgetOperations(widget)
+
+  let _ = widget.name
 }
 
 func testError(widget: SKWidget) {
   let c: SKWidget.Error.Code = SKWidget.Error(.boom).getCode(from: widget)
   if c.isBoom { }
 }
 
+func testGlobals() {
+  someKitGlobalFunc()
+  SomeKit.someKitOtherGlobalFunc()
+  someKitOtherGlobalFunc()
+}