ConstraintSystem: Use scoring to implement MemberImportVisibility.

Previously, the constraint solver would first attempt member lookup that
excluded members from transitively imported modules. If there were no viable
candidates, it would perform a second lookup that included the previously
excluded members, treating any candidates as unviable. This meant that if the
member reference did resolve to one of the unviable candidates the resulting
AST would be broken, which could cause unwanted knock-on diagnostics.

Now, members from transitively imported modules are always returned in the set
of viable candidates. However, scoring will always prioritize candidates from
directly imported modules over members from transitive imports. This solves the
ambiguities that `MemberImportVisibility` is designed to prevent. If the only
viable candidates are from transitively imported modules, though, then the
reference will be resolved successfully and diagnosed later in
`MiscDiagnostics.cpp`. The resulting AST will not contain any errors, which
ensures that necessary access levels can be computed correctly for the imports
suggested by `MemberImportVisibility` fix-its.

Resolves rdar://126637855.

Author: tshortli

include/swift/AST/DeclContext.h

@@ -680,6 +680,9 @@ class alignas(1 << DeclContextAlignInBits) DeclContext
   /// Looks up a precedence group with a given \p name.
   PrecedenceGroupLookupResult lookupPrecedenceGroup(Identifier name) const;
 
+  /// Returns true if the parent module of \p decl is imported by this context.
+  bool isDeclImported(const Decl *decl) const;
+
   /// Return the ASTContext for a specified DeclContext by
   /// walking up to the enclosing module and returning its ASTContext.
   LLVM_READONLY

include/swift/AST/SourceFile.h

@@ -507,11 +507,6 @@ class SourceFile final : public FileUnit {
     MissingImportForMemberDiagnostics[decl].push_back(loc);
   }
 
-  /// Returns true if there is a pending missing import diagnostic for \p decl.
-  bool hasDelayedMissingImportForMemberDiagnostic(const ValueDecl *decl) const {
-    return MissingImportForMemberDiagnostics.contains(decl);
-  }
-
   DelayedMissingImportForMemberDiags
   takeDelayedMissingImportForMemberDiagnostics() {
     DelayedMissingImportForMemberDiags diags;

include/swift/Sema/ConstraintSystem.h

@@ -960,6 +960,8 @@ enum ScoreKind: unsigned int {
   SK_Hole,
   /// A reference to an @unavailable declaration.
   SK_Unavailable,
+  /// A reference to a declaration from a module that has not been imported.
+  SK_MissingImport,
   /// A reference to an async function in a synchronous context.
   ///
   /// \note Any score kind after this is considered a conversion that doesn't
@@ -1124,6 +1126,9 @@ struct Score {
     case SK_Unavailable:
       return "use of an unavailable declaration";
 
+    case SK_MissingImport:
+      return "use of a declaration that has not been imported";
+
     case SK_AsyncInSyncMismatch:
       return "async-in-synchronous mismatch";
 

lib/AST/NameLookup.cpp

@@ -2325,8 +2325,7 @@ static bool missingImportForMemberDecl(const DeclContext *dc, ValueDecl *decl) {
   if (!ctx.LangOpts.hasFeature(Feature::MemberImportVisibility))
     return false;
 
-  auto declModule = decl->getDeclContext()->getParentModule();
-  return !ctx.getImportCache().isImportedBy(declModule, dc);
+  return !dc->isDeclImported(decl);
 }
 
 /// Determine whether the given declaration is an acceptable lookup
@@ -2504,6 +2503,11 @@ bool DeclContext::lookupQualified(Type type,
                          loc, options, decls);
 }
 
+bool DeclContext::isDeclImported(const Decl *decl) const {
+  auto declModule = decl->getDeclContext()->getParentModule();
+  return getASTContext().getImportCache().isImportedBy(declModule, this);
+}
+
 static void installPropertyWrapperMembersIfNeeded(NominalTypeDecl *target,
                                                   DeclNameRef member) {
   auto &Context = target->getASTContext();

lib/Sema/CSRanking.cpp

@@ -963,7 +963,7 @@ SolutionCompareResult ConstraintSystem::compareSolutions(
              ? SolutionCompareResult::Better
              : SolutionCompareResult::Worse;
   }
-  
+
   // Compute relative score.
   unsigned score1 = 0;
   unsigned score2 = 0;

lib/Sema/CSSimplify.cpp

@@ -10346,7 +10346,8 @@ performMemberLookup(ConstraintKind constraintKind, DeclNameRef memberName,
   // include them in the unviable candidates list.
   if (result.ViableCandidates.empty() && result.UnviableCandidates.empty() &&
       includeInaccessibleMembers) {
-    NameLookupOptions lookupOptions = defaultMemberLookupOptions;
+    NameLookupOptions lookupOptions =
+        defaultConstraintSolverMemberLookupOptions;
 
     // Local function that looks up additional candidates using the given lookup
     // options, recording the results as unviable candidates.

lib/Sema/ConstraintSystem.cpp

@@ -292,8 +292,8 @@ LookupResult &ConstraintSystem::lookupMember(Type base, DeclNameRef name,
   if (result) return *result;
 
   // Lookup the member.
-  result = TypeChecker::lookupMember(DC, base, name, loc,
-                                     defaultMemberLookupOptions);
+  result = TypeChecker::lookupMember(
+      DC, base, name, loc, defaultConstraintSolverMemberLookupOptions);
 
   // If we are in an @_unsafeInheritExecutor context, swap out
   // declarations for their _unsafeInheritExecutor_ counterparts if they
@@ -3966,6 +3966,12 @@ void ConstraintSystem::resolveOverload(ConstraintLocator *locator,
     if (isDeclUnavailable(decl, locator))
       increaseScore(SK_Unavailable, locator);
 
+    // If the declaration is from a module that hasn't been imported, note that.
+    if (getASTContext().LangOpts.hasFeature(Feature::MemberImportVisibility)) {
+      if (!useDC->isDeclImported(decl))
+        increaseScore(SK_MissingImport, locator);
+    }
+
     // If this overload is disfavored, note that.
     if (decl->getAttrs().hasAttribute<DisfavoredOverloadAttr>())
       increaseScore(SK_DisfavoredOverload, locator);

lib/Sema/MiscDiagnostics.cpp

@@ -6163,6 +6163,40 @@ diagnoseDictionaryLiteralDuplicateKeyEntries(const Expr *E,
   const_cast<Expr *>(E)->walk(Walker);
 }
 
+// Verifies that references to members are valid under the restrictions of the
+// MemberImportVisibility feature.
+static void diagnoseMissingMemberImports(const Expr *E, const DeclContext *DC) {
+  class DiagnoseWalker : public BaseDiagnosticWalker {
+    const DeclContext *dc;
+
+  public:
+    DiagnoseWalker(const DeclContext *dc) : dc(dc) {}
+
+    PreWalkResult<Expr *> walkToExprPre(Expr *E) override {
+      if (auto declRef = E->getReferencedDecl())
+        checkDecl(declRef.getDecl(), E->getLoc());
+
+      return Action::Continue(E);
+    }
+
+    void checkDecl(const ValueDecl *decl, SourceLoc loc) {
+      // Only diagnose uses of members.
+      if (!decl->getDeclContext()->isTypeContext())
+        return;
+
+      if (!dc->isDeclImported(decl))
+        maybeDiagnoseMissingImportForMember(decl, dc, loc);
+    }
+  };
+
+  auto &ctx = DC->getASTContext();
+  if (!ctx.LangOpts.hasFeature(Feature::MemberImportVisibility))
+    return;
+
+  DiagnoseWalker walker(DC);
+  const_cast<Expr *>(E)->walk(walker);
+}
+
 //===----------------------------------------------------------------------===//
 // High-level entry points.
 //===----------------------------------------------------------------------===//
@@ -6189,6 +6223,7 @@ void swift::performSyntacticExprDiagnostics(const Expr *E,
   diagnoseConstantArgumentRequirement(E, DC);
   diagUnqualifiedAccessToMethodNamedSelf(E, DC);
   diagnoseDictionaryLiteralDuplicateKeyEntries(E, DC);
+  diagnoseMissingMemberImports(E, DC);
 }
 
 void swift::performStmtDiagnostics(const Stmt *S, DeclContext *DC) {

lib/Sema/ResilienceDiagnostics.cpp

@@ -279,8 +279,7 @@ static bool diagnoseValueDeclRefExportability(SourceLoc loc, const ValueDecl *D,
   // Some diagnostics emitted with the `MemberImportVisibility` feature enabled
   // subsume these diagnostics.
   if (originKind == DisallowedOriginKind::MissingImport &&
-      ctx.LangOpts.hasFeature(Feature::MemberImportVisibility) && SF &&
-      SF->hasDelayedMissingImportForMemberDiagnostic(D))
+      ctx.LangOpts.hasFeature(Feature::MemberImportVisibility) && SF)
     return false;
 
   if (auto accessor = dyn_cast<AccessorDecl>(D)) {

lib/Sema/TypeChecker.h

@@ -174,6 +174,14 @@ inline NameLookupOptions operator|(NameLookupFlags flag1,
 /// Default options for member name lookup.
 const NameLookupOptions defaultMemberLookupOptions;
 
+/// Default options for member name lookup in the constraint solver.
+/// Overloads which come from modules that have not been imported should be
+/// deprioritized by ranking and diagnosed by MiscDiagnostics, so we allow
+/// them to be found in constraint solver lookups to improve diagnostics
+/// overall.
+const NameLookupOptions defaultConstraintSolverMemberLookupOptions(
+    NameLookupFlags::IgnoreMissingImports);
+
 /// Default options for member type lookup.
 const NameLookupOptions defaultMemberTypeLookupOptions;