Merge pull request #75850 from tshortli/member-import-visibility-overrides

AST: Look through missing imports in OverriddenDeclsRequest

Author: tshortli

lib/Sema/TypeCheckDeclOverride.cpp

@@ -814,8 +814,11 @@ namespace {
     /// The type of the declaration, cached here once it has been computed.
     Type cachedDeclType;
 
+    /// Whether to ignore missing imports when looking for overridden methods.
+    bool ignoreMissingImports;
+
   public:
-    OverrideMatcher(ValueDecl *decl);
+    OverrideMatcher(ValueDecl *decl, bool ignoreMissingImports);
 
     /// Returns true when it's possible to perform any override matching.
     explicit operator bool() const {
@@ -860,8 +863,9 @@ namespace {
   };
 }
 
-OverrideMatcher::OverrideMatcher(ValueDecl *decl)
-    : ctx(decl->getASTContext()), decl(decl) {
+OverrideMatcher::OverrideMatcher(ValueDecl *decl, bool ignoreMissingImports)
+    : ctx(decl->getASTContext()), decl(decl),
+      ignoreMissingImports(ignoreMissingImports) {
   // The final step for this constructor is to set up the superclass type,
   // without which we will not perform an matching. Early exits therefore imply
   // that there is no way we can match this declaration.
@@ -916,8 +920,12 @@ SmallVector<OverrideMatch, 2> OverrideMatcher::match(
     for (auto *ctx : superContexts) {
       ctx->synthesizeSemanticMembersIfNeeded(membersName);
     }
-    dc->lookupQualified(superContexts, DeclNameRef(membersName),
-                        decl->getLoc(), NL_QualifiedDefault, members);
+    auto lookupOptions = NL_QualifiedDefault;
+    if (ignoreMissingImports)
+      lookupOptions |= NL_IgnoreMissingImports;
+
+    dc->lookupQualified(superContexts, DeclNameRef(membersName), decl->getLoc(),
+                        lookupOptions, members);
   }
 
   // Check each member we found.
@@ -1373,50 +1381,12 @@ TinyPtrVector<ValueDecl *> OverrideMatcher::checkPotentialOverrides(
   return overridden;
 }
 
-/// Determine which method or subscript this method or subscript overrides
-/// (if any).
-///
-/// \returns true if an error occurred.
-bool swift::checkOverrides(ValueDecl *decl) {
-  // If there is a @_nonoverride attribute, this does not override anything.
-  if (decl->getAttrs().hasAttribute<NonOverrideAttr>())
-    return false;
-
-  // If we already computed overridden declarations and either succeeded
-  // or invalidated the attribute, there's nothing more to do.
-  if (decl->overriddenDeclsComputed()) {
-    // If we computed an overridden declaration successfully, we're done.
-    if (decl->getOverriddenDecl())
-      return false;
-
-    // If we set the override attribute to "invalid", we already diagnosed
-    // something here.
-    if (decl->getAttrs().hasAttribute<OverrideAttr>(/*AllowInvalid=*/true) &&
-        !decl->getAttrs().hasAttribute<OverrideAttr>())
-      return true;
-
-    // Otherwise, we have more checking to do.
-  }
-
-  // Members of constrained extensions are not considered to be overrides.
-  if (auto *ext = dyn_cast<ExtensionDecl>(decl->getDeclContext()))
-    if (ext->isConstrainedExtension())
-      return false;
-
-  // Accessor methods get overrides through their storage declaration, and
-  // all checking can be performed via that mechanism.
-  if (isa<AccessorDecl>(decl)) {
-    (void)decl->getOverriddenDecls();
-    return false;
-  }
-
-  // Don't bother checking any further for invalid decls since they won't match
-  // anything.
-  if (decl->isInvalid())
-    return true;
-
+static bool
+checkPotentialOverrides(ValueDecl *decl,
+                        TinyPtrVector<ValueDecl *> &potentialOverrides,
+                        bool ignoreMissingImports) {
   // Set up matching, but bail out if there's nothing to match.
-  OverrideMatcher matcher(decl);
+  OverrideMatcher matcher(decl, ignoreMissingImports);
   if (!matcher) return false;
 
   // Look for members with the same name and matching types as this
@@ -1470,10 +1440,80 @@ bool swift::checkOverrides(ValueDecl *decl) {
 
   // FIXME: Check for missing 'override' keyword here?
 
+  for (auto match : matcher.checkPotentialOverrides(matches, attempt)) {
+    potentialOverrides.push_back(match);
+  }
+
+  return true;
+}
+
+/// Determine which method or subscript this method or subscript overrides
+/// (if any).
+///
+/// \returns true if an error occurred.
+bool swift::checkOverrides(ValueDecl *decl) {
+  // If there is a @_nonoverride attribute, this does not override anything.
+  if (decl->getAttrs().hasAttribute<NonOverrideAttr>())
+    return false;
+
+  // If we already computed overridden declarations and either succeeded
+  // or invalidated the attribute, there's nothing more to do.
+  if (decl->overriddenDeclsComputed()) {
+    // If we computed an overridden declaration successfully, we're done.
+    if (decl->getOverriddenDecl())
+      return false;
+
+    // If we set the override attribute to "invalid", we already diagnosed
+    // something here.
+    if (decl->getAttrs().hasAttribute<OverrideAttr>(/*AllowInvalid=*/true) &&
+        !decl->getAttrs().hasAttribute<OverrideAttr>())
+      return true;
+
+    // Otherwise, we have more checking to do.
+  }
+
+  // Members of constrained extensions are not considered to be overrides.
+  if (auto *ext = dyn_cast<ExtensionDecl>(decl->getDeclContext()))
+    if (ext->isConstrainedExtension())
+      return false;
+
+  // Accessor methods get overrides through their storage declaration, and
+  // all checking can be performed via that mechanism.
+  if (isa<AccessorDecl>(decl)) {
+    (void)decl->getOverriddenDecls();
+    return false;
+  }
+
+  // Don't bother checking any further for invalid decls since they won't match
+  // anything.
+  if (decl->isInvalid())
+    return true;
+
+  TinyPtrVector<ValueDecl *> overridden;
+  if (!checkPotentialOverrides(decl, overridden,
+                               /*ignoreMissingImports=*/false))
+    return false;
+
+  auto &ctx = decl->getASTContext();
+  if (overridden.empty() &&
+      ctx.LangOpts.hasFeature(Feature::MemberImportVisibility)) {
+    // If we didn't find anything, try broadening the search by ignoring missing
+    // imports.
+    if (!checkPotentialOverrides(decl, overridden,
+                                 /*ignoreMissingImports=*/true))
+      return false;
+
+    if (!overridden.empty()) {
+      auto first = overridden.front();
+      if (maybeDiagnoseMissingImportForMember(first, decl->getDeclContext(),
+                                              decl->getLoc()))
+        return true;
+    }
+  }
+
   // We performed override checking, so record the overrides.
   // FIXME: It's weird to be pushing state here, but how do we say that
   // this check subsumes the normal 'override' check?
-  auto overridden = matcher.checkPotentialOverrides(matches, attempt);
   if (overridden.empty())
     invalidateOverrideAttribute(decl);
   decl->setOverriddenDecls(overridden);
@@ -2263,8 +2303,8 @@ computeOverriddenAssociatedTypes(AssociatedTypeDecl *assocType) {
   return overriddenAssocTypes;
 }
 
-llvm::TinyPtrVector<ValueDecl *>
-OverriddenDeclsRequest::evaluate(Evaluator &evaluator, ValueDecl *decl) const {
+static llvm::TinyPtrVector<ValueDecl *>
+computeOverriddenDecls(ValueDecl *decl, bool ignoreMissingImports) {
   // Value to return in error cases
   auto noResults = llvm::TinyPtrVector<ValueDecl *>();
 
@@ -2359,7 +2399,7 @@ OverriddenDeclsRequest::evaluate(Evaluator &evaluator, ValueDecl *decl) const {
       return noResults;
 
     // Check the correctness of the overrides.
-    OverrideMatcher matcher(accessor);
+    OverrideMatcher matcher(accessor, ignoreMissingImports);
     return matcher.checkPotentialOverrides(
                                        matches,
                                        OverrideCheckingAttempt::PerfectMatch);
@@ -2373,7 +2413,7 @@ OverriddenDeclsRequest::evaluate(Evaluator &evaluator, ValueDecl *decl) const {
     return noResults;
 
   // Try to match potential overridden declarations.
-  OverrideMatcher matcher(decl);
+  OverrideMatcher matcher(decl, ignoreMissingImports);
   if (!matcher) {
     return noResults;
   }
@@ -2398,6 +2438,27 @@ OverriddenDeclsRequest::evaluate(Evaluator &evaluator, ValueDecl *decl) const {
                                          OverrideCheckingAttempt::PerfectMatch);
 }
 
+llvm::TinyPtrVector<ValueDecl *>
+OverriddenDeclsRequest::evaluate(Evaluator &evaluator, ValueDecl *decl) const {
+  auto &ctx = decl->getASTContext();
+  auto overridden = computeOverriddenDecls(decl, false);
+
+  // If we didn't find anything, try broadening the search by ignoring missing
+  // imports.
+  if (overridden.empty() &&
+      ctx.LangOpts.hasFeature(Feature::MemberImportVisibility)) {
+    overridden = computeOverriddenDecls(decl, true);
+    if (!overridden.empty()) {
+      auto first = overridden.front();
+      if (maybeDiagnoseMissingImportForMember(first, decl->getDeclContext(),
+                                              decl->getLoc()))
+        return {};
+    }
+  }
+
+  return overridden;
+}
+
 bool IsABICompatibleOverrideRequest::evaluate(Evaluator &evaluator,
                                               ValueDecl *decl) const {
   auto base = decl->getOverriddenDecl();

test/NameLookup/members_transitive.swift

@@ -7,7 +7,7 @@
 // RUN: %target-swift-frontend -typecheck %s -I %t -verify -swift-version 5 -package-name TestPackage -enable-experimental-feature MemberImportVisibility -verify-additional-prefix member-visibility-
 
 import members_C
-// expected-member-visibility-note 11{{add import of module 'members_B'}}{{1-1=internal import members_B\n}}
+// expected-member-visibility-note 12{{add import of module 'members_B'}}{{1-1=internal import members_B\n}}
 
 
 func testExtensionMembers(x: X, y: Y<Z>) {
@@ -70,7 +70,7 @@ func testTopLevelTypes() {
 
 class DerivedFromClassInC: DerivedClassInC {
   override func methodInA() {}
-  override func methodInB() {} // expected-member-visibility-error{{method does not override any method from its superclass}}
+  override func methodInB() {} // expected-member-visibility-error{{instance method 'methodInB()' is not available due to missing import of defining module 'members_B'}}
   override func methodInC() {}
 }