Merge pull request #84259 from tshortli/member-import-visibility-migrate-crash

Sema: Fix a crash in migrate mode for `MemberImportVisibility`

Author: tshortli

lib/Sema/PreCheckTarget.cpp

@@ -568,6 +568,7 @@ Expr *TypeChecker::resolveDeclRefExpr(UnresolvedDeclRefExpr *UDRE,
   //       chance to diagnose name shadowing which requires explicit
   //       name/module qualifier to access top-level name.
   lookupOptions |= NameLookupFlags::IncludeOuterResults;
+  lookupOptions |= NameLookupFlags::IgnoreMissingImports;
 
   LookupResult Lookup;
 
@@ -662,19 +663,6 @@ Expr *TypeChecker::resolveDeclRefExpr(UnresolvedDeclRefExpr *UDRE,
       return errorResult();
     }
 
-    // Try ignoring missing imports.
-    relookupOptions |= NameLookupFlags::IgnoreMissingImports;
-    auto nonImportedResults =
-        TypeChecker::lookupUnqualified(DC, LookupName, Loc, relookupOptions);
-    if (nonImportedResults) {
-      const ValueDecl *first = nonImportedResults.front().getValueDecl();
-      maybeDiagnoseMissingImportForMember(first, DC, Loc);
-
-      // Don't try to recover here; we'll get more access-related diagnostics
-      // downstream if the type of the inaccessible decl is also inaccessible.
-      return errorResult();
-    }
-
     // TODO: Name will be a compound name if it was written explicitly as
     // one, but we should also try to propagate labels into this.
     DeclNameLoc nameLoc = UDRE->getNameLoc();
@@ -791,14 +779,21 @@ Expr *TypeChecker::resolveDeclRefExpr(UnresolvedDeclRefExpr *UDRE,
   // FIXME: Need to refactor the way we build an AST node from a lookup result!
 
   auto buildTypeExpr = [&](TypeDecl *D) -> Expr * {
+    auto *LookupDC = Lookup[0].getDeclContext();
+
+    // If the type decl is a member that wasn't imported, diagnose it now when
+    // MemberImportVisibility is enabled.
+    if (!UDRE->isImplicit() && LookupDC)
+      maybeDiagnoseMissingImportForMember(D, LookupDC,
+                                          UDRE->getNameLoc().getStartLoc());
+
     // FIXME: This is odd.
     if (isa<ModuleDecl>(D)) {
       return new (Context) DeclRefExpr(
           D, UDRE->getNameLoc(),
           /*Implicit=*/false, AccessSemantics::Ordinary, D->getInterfaceType());
     }
 
-    auto *LookupDC = Lookup[0].getDeclContext();
     bool makeTypeValue = false;
 
     if (isa<GenericTypeParamDecl>(D) &&
@@ -890,6 +885,13 @@ Expr *TypeChecker::resolveDeclRefExpr(UnresolvedDeclRefExpr *UDRE,
           DC, UDRE->getNameLoc().getBaseNameLoc(), ResultValues);
     }
 
+    // If there is a single result and it's a member that wasn't imported,
+    // diagnose it now when MemberImportVisibility is enabled.
+    if (ResultValues.size() == 1) {
+      maybeDiagnoseMissingImportForMember(ResultValues.front(), DC,
+                                          UDRE->getNameLoc().getStartLoc());
+    }
+
     return buildRefExpr(ResultValues, DC, UDRE->getNameLoc(),
                         UDRE->isImplicit(), UDRE->getFunctionRefInfo());
   }

lib/Sema/TypeCheckNameLookup.cpp

@@ -1013,11 +1013,26 @@ bool swift::maybeDiagnoseMissingImportForMember(const ValueDecl *decl,
                                                 const DeclContext *dc,
                                                 SourceLoc loc,
                                                 DiagnosticBehavior limit) {
+  // Only diagnose references in source files.
+  auto sf = dc->getParentSourceFile();
+  if (!sf)
+    return false;
+
+  auto &ctx = dc->getASTContext();
+  if (!ctx.LangOpts.hasFeature(Feature::MemberImportVisibility,
+                          /*allowMigration=*/true))
+    return false;
+
+  // Only diagnose members.
+  if (!decl->getDeclContext()->isTypeContext())
+    return false;
+
+  // Only diagnose declarations that haven't been imported.
   if (dc->isDeclImported(decl))
     return false;
 
   auto definingModule = decl->getModuleContextForNameLookup();
-  if (dc->getASTContext().LangOpts.EnableCXXInterop) {
+  if (ctx.LangOpts.EnableCXXInterop) {
     // With Cxx interop enabled, there are some declarations that always belong
     // to the Clang header import module which should always be implicitly
     // visible. However, that module is not implicitly imported in source files
@@ -1026,12 +1041,6 @@ bool swift::maybeDiagnoseMissingImportForMember(const ValueDecl *decl,
       return false;
   }
 
-  auto sf = dc->getParentSourceFile();
-  if (!sf)
-    return false;
-
-  auto &ctx = dc->getASTContext();
-
   // In lazy typechecking mode just emit the diagnostic immediately without a
   // fix-it since there won't be an opportunity to emit delayed diagnostics.
   if (ctx.TypeCheckerOpts.EnableLazyTypecheck) {

test/NameLookup/member_import_visibility_migrate.swift

@@ -14,7 +14,7 @@
 // RUN: %target-swift-frontend -emit-module -o %t %t/InternalUsesOnlyDefaultedImportSPIOnly.swift -I %t
 // RUN: %target-swift-frontend -emit-module -o %t %t/PublicUsesOnlySPIOnly.swift -I %t
 
-// RUN: %target-swift-frontend -typecheck -verify -swift-version 5 \
+// RUN: %target-swift-frontend -emit-silgen -verify -swift-version 5 \
 // RUN:   -primary-file %t/main.swift \
 // RUN:   %t/imports.swift \
 // RUN:   -I %t -package-name Package \
@@ -81,6 +81,31 @@ extension Int {
   internal func usesTypealiasInMixedUses_Internal(x: TypealiasInMixedUses) {} // expected-note {{type alias 'TypealiasInMixedUses' from 'MixedUses' used here}}
 }
 
+struct GenericType<T> { }
+
+extension Int {
+  var referencesMemberInInternalUsesOnly: Int { memberInInternalUsesOnly } // expected-note {{property 'memberInInternalUsesOnly' from 'InternalUsesOnly' used here}}
+
+  func testTypes<T: ProtocolInInternalUsesOnly>(_ t: T) -> TypealiasInInternalUsesOnly? {
+    // expected-note@-1 {{protocol 'ProtocolInInternalUsesOnly' from 'InternalUsesOnly' used here}}
+    // expected-note@-2 {{type alias 'TypealiasInInternalUsesOnly' from 'InternalUsesOnly' used here}}
+
+    let _: TypealiasInInternalUsesOnly = 0 // expected-note {{type alias 'TypealiasInInternalUsesOnly' from 'InternalUsesOnly' used here}}
+    _ = TypealiasInInternalUsesOnly.self // expected-note {{type alias 'TypealiasInInternalUsesOnly' from 'InternalUsesOnly' used here}}
+    _ = (TypealiasInInternalUsesOnly).self // expected-note {{type alias 'TypealiasInInternalUsesOnly' from 'InternalUsesOnly' used here}}
+    _ = (Int, TypealiasInInternalUsesOnly).self // expected-note {{type alias 'TypealiasInInternalUsesOnly' from 'InternalUsesOnly' used here}}
+    _ = GenericType<TypealiasInInternalUsesOnly>.self // expected-note {{type alias 'TypealiasInInternalUsesOnly' from 'InternalUsesOnly' used here}}
+    return t as? TypealiasInInternalUsesOnly // expected-note {{type alias 'TypealiasInInternalUsesOnly' from 'InternalUsesOnly' used here}}
+  }
+
+  func testLeadingDotSyntax() {
+    func takesP<T: ProtocolInInternalUsesOnly>(_: T) { } // expected-note {{protocol 'ProtocolInInternalUsesOnly' from 'InternalUsesOnly' used here}}
+    takesP(.staticDefaultMemberInInternalUsesOnly) // expected-note {{static property 'staticDefaultMemberInInternalUsesOnly' from 'InternalUsesOnly' used here}}
+  }
+}
+
+extension Int.TypealiasInInternalUsesOnly { } // expected-note {{type alias 'TypealiasInInternalUsesOnly' from 'InternalUsesOnly' used here}}
+
 //--- imports.swift
 
 internal import InternalUsesOnly
@@ -97,10 +122,17 @@ internal import ImportsOtherModules
 //--- InternalUsesOnly.swift
 
 extension Int {
+  public protocol ProtocolInInternalUsesOnly { }
   public typealias TypealiasInInternalUsesOnly = Self
   public var memberInInternalUsesOnly: Int { return self }
 }
 
+extension Int: Int.ProtocolInInternalUsesOnly { }
+
+extension Int.ProtocolInInternalUsesOnly where Self == Int {
+  public static var staticDefaultMemberInInternalUsesOnly: Int { 0 }
+}
+
 //--- InternalUsesOnlyDefaultedImport.swift
 
 extension Int {