[Sema] Apply import restriction in TypeAccessScopeChecker's AccessScope

xymus · xymus · commit 1a1df7e9ec45 · 2023-09-13T15:40:33.000-07:00
This cleans up the logic and allows using `private` imported
declarations in `fileprivate` declarations.
diff --git a/lib/Sema/TypeAccessScopeChecker.h b/lib/Sema/TypeAccessScopeChecker.h
@@ -31,7 +31,6 @@ class TypeAccessScopeChecker {
   bool TreatUsableFromInlineAsPublic;
 
   llvm::Optional<AccessScope> Scope = AccessScope::getPublic();
-  ImportAccessLevel ImportRestriction = llvm::None;
 
   TypeAccessScopeChecker(const DeclContext *useDC,
                          bool treatUsableFromInlineAsPublic)
@@ -44,41 +43,22 @@ class TypeAccessScopeChecker {
 
     auto AS = VD->getFormalAccessScope(File, TreatUsableFromInlineAsPublic);
     Scope = Scope->intersectWith(AS);
-
-    auto targetModule = VD->getDeclContext()->getParentModule();
-    if (targetModule != File->getParentModule()) {
-      auto localImportRestriction = File->getImportAccessLevel(targetModule);
-
-      if (localImportRestriction.has_value() &&
-          (!ImportRestriction.has_value() ||
-           localImportRestriction.value().accessLevel <
-             ImportRestriction.value().accessLevel)) {
-        ImportRestriction = localImportRestriction;
-      }
-    }
-
     return Scope.has_value();
   }
 
 public:
 
-  struct Result {
-    llvm::Optional<AccessScope> Scope;
-    ImportAccessLevel Import;
-  };
-
-  static Result
+  static llvm::Optional<AccessScope>
   getAccessScope(TypeRepr *TR, const DeclContext *useDC,
                  bool treatUsableFromInlineAsPublic = false) {
     TypeAccessScopeChecker checker(useDC, treatUsableFromInlineAsPublic);
     TR->walk(TypeReprIdentFinder([&](const IdentTypeRepr *typeRepr) {
       return checker.visitDecl(typeRepr->getBoundDecl());
     }));
-    return {checker.Scope,
-            checker.ImportRestriction};
+    return checker.Scope;
   }
 
-  static Result
+  static llvm::Optional<AccessScope>
   getAccessScope(Type T, const DeclContext *useDC,
                  bool treatUsableFromInlineAsPublic = false) {
     TypeAccessScopeChecker checker(useDC, treatUsableFromInlineAsPublic);
@@ -88,8 +68,7 @@ class TypeAccessScopeChecker {
       return TypeWalker::Action::Stop;
     }));
 
-    return {checker.Scope,
-            checker.ImportRestriction};
+    return checker.Scope;
   }
 };
 
diff --git a/lib/Sema/TypeCheckAccess.cpp b/lib/Sema/TypeCheckAccess.cpp
@@ -196,62 +196,46 @@ void AccessControlCheckerBase::checkTypeAccessImpl(
   ImportAccessLevel problematicImport = llvm::None;
 
   if (type) {
-    auto scopeAndImport =
+    llvm::Optional<AccessScope> typeAccessScope =
         TypeAccessScopeChecker::getAccessScope(type, useDC,
                                                checkUsableFromInline);
 
     // Note: This means that the type itself is invalid for this particular
     // context, because it references declarations from two incompatible scopes.
     // In this case we should have diagnosed the bad reference already.
-    llvm::Optional<AccessScope> typeAccessScope = scopeAndImport.Scope;
     if (!typeAccessScope.has_value())
       return;
 
-    problematicImport = scopeAndImport.Import;
     problematicAccessScope = *typeAccessScope;
   }
 
   auto downgradeToWarning = DowngradeToWarning::No;
 
-  AccessLevel importAccessLevel = AccessLevel::Public;
-  if (problematicImport.has_value())
-    importAccessLevel = problematicImport->accessLevel;
-
   // Check if type can be referenced in this context.
   // - hasEqualDeclContextWith checks for matching scopes (public to public,
-  // internal to the same module, private to same scope, etc.)
+  //   internal to the same module, private to same scope, etc.)
   // - isChildOf checks for use of public in internal, etc.
-  // - Comparison to importAccessLevel ensures the type access scope isn't
-  //   restricted by an access-level on an import.
-  if ((contextAccessScope.hasEqualDeclContextWith(problematicAccessScope) ||
-      contextAccessScope.isChildOf(problematicAccessScope)) &&
-      contextAccessScope.accessLevelForDiagnostics() <= importAccessLevel) {
+  if (contextAccessScope.hasEqualDeclContextWith(problematicAccessScope) ||
+      contextAccessScope.isChildOf(problematicAccessScope)) {
 
     // /Also/ check the TypeRepr, if present. This can be important when we're
     // unable to preserve typealias sugar that's present in the TypeRepr.
     if (!typeRepr)
       return;
 
-    auto typeReprAccessScopeAndImport =
+    auto typeReprAccessScope =
         TypeAccessScopeChecker::getAccessScope(typeRepr, useDC,
                                                checkUsableFromInline);
-    auto typeReprAccessScope = typeReprAccessScopeAndImport.Scope;
     if (!typeReprAccessScope.has_value())
       return;
 
-    if (typeReprAccessScopeAndImport.Import.has_value())
-       importAccessLevel = typeReprAccessScopeAndImport.Import->accessLevel;
-
-    if ((contextAccessScope.hasEqualDeclContextWith(*typeReprAccessScope) ||
-         contextAccessScope.isChildOf(*typeReprAccessScope)) &&
-        contextAccessScope.accessLevelForDiagnostics() <= importAccessLevel) {
+    if (contextAccessScope.hasEqualDeclContextWith(*typeReprAccessScope) ||
+        contextAccessScope.isChildOf(*typeReprAccessScope)) {
       // Only if both the Type and the TypeRepr follow the access rules can
       // we exit; otherwise we have to emit a diagnostic.
       return;
     }
     problematicAccessScope = *typeReprAccessScope;
-    if (typeReprAccessScopeAndImport.Import.has_value())
-      problematicImport = typeReprAccessScopeAndImport.Import;
   } else {
     // The type violates the rules of access control (with or without taking the
     // TypeRepr into account).
@@ -268,8 +252,7 @@ void AccessControlCheckerBase::checkTypeAccessImpl(
       // Downgrade the error to a warning in this case for source compatibility.
       llvm::Optional<AccessScope> typeReprAccessScope =
           TypeAccessScopeChecker::getAccessScope(typeRepr, useDC,
-                                                 checkUsableFromInline)
-              .Scope;
+                                                 checkUsableFromInline);
       assert(typeReprAccessScope && "valid Type but not valid TypeRepr?");
       if (contextAccessScope.hasEqualDeclContextWith(*typeReprAccessScope) ||
           contextAccessScope.isChildOf(*typeReprAccessScope)) {
diff --git a/lib/Sema/TypeCheckProtocol.cpp b/lib/Sema/TypeCheckProtocol.cpp
@@ -3430,8 +3430,7 @@ void ConformanceChecker::recordTypeWitness(AssociatedTypeDecl *assocType,
         // non-resilient modules.
         llvm::Optional<AccessScope> underlyingTypeScope =
             TypeAccessScopeChecker::getAccessScope(type, DC,
-                                                   /*usableFromInline*/ false)
-                .Scope;
+                                                   /*usableFromInline*/ false);
         assert(underlyingTypeScope.has_value() &&
                "the type is already invalid and we shouldn't have gotten here");
 
diff --git a/test/Sema/access-level-import-diag-priority.swift b/test/Sema/access-level-import-diag-priority.swift
@@ -1,7 +1,7 @@
 /// Report the most restricted types when many types are problematic.
 
 // RUN: %empty-directory(%t)
-// RUN: split-file %s %t
+// RUN: split-file --leading-lines %s %t
 
 /// Build the libraries.
 // RUN: %target-swift-frontend -emit-module %t/PublicLib.swift -o %t
@@ -37,11 +37,11 @@ public struct PrivateImportType {}
 public import PublicLib
 package import PackageLib // expected-note {{struct 'PackageImportType' imported as 'package' from 'PackageLib' here}}
 internal import InternalLib // expected-note {{struct 'InternalImportType' imported as 'internal' from 'InternalLib' here}}
-fileprivate import FileprivateLib // expected-note 2 {{struct 'FileprivateImportType' imported as 'fileprivate' from 'FileprivateLib' here}}
-private import PrivateLib // expected-note 2 {{struct 'PrivateImportType' imported as 'private' from 'PrivateLib' here}}
+fileprivate import FileprivateLib // expected-note 3 {{struct 'FileprivateImportType' imported as 'fileprivate' from 'FileprivateLib' here}}
+private import PrivateLib // expected-note 1 {{struct 'PrivateImportType' imported as 'private' from 'PrivateLib' here}}
 
 /// Simple ordering
-public func publicFuncUsesPrivate(_ a: PublicImportType, b: PackageImportType, c: InternalImportType, d: FileprivateImportType, e: PrivateImportType) { // expected-error {{function cannot be declared public because its parameter uses a private type}}
+public func publicFuncUsesPrivate(_ a: PublicImportType, b: PackageImportType, c: InternalImportType, d: FileprivateImportType, e: PrivateImportType) { // expected-error {{function cannot be declared public because its parameter uses a fileprivate type}}
     var _: PrivateImportType
 }
 public func publicFuncUsesFileprivate(_ a: PublicImportType, b: PackageImportType, c: InternalImportType, d: FileprivateImportType) { // expected-error {{function cannot be declared public because its parameter uses a fileprivate type}}
diff --git a/test/Sema/access-level-import-exportability.swift b/test/Sema/access-level-import-exportability.swift
@@ -1,5 +1,5 @@
 // RUN: %empty-directory(%t)
-// RUN: split-file %s %t
+// RUN: split-file --leading-lines %s %t
 
 /// Build the libraries.
 // RUN: %target-swift-frontend -emit-module %t/PublicLib.swift -o %t \
@@ -129,7 +129,7 @@ public func PublicFuncUsesInternal(_: InternalImportType) { // expected-error {{
     var _: InternalImportType
 }
 
-public class PublicSubclassFilepriovate : FileprivateImportClass {} // expected-error {{class cannot be declared public because its superclass is fileprivate}}
+public class PublicSubclassFileprivate : FileprivateImportClass {} // expected-error {{class cannot be declared public because its superclass is fileprivate}}
 
 /// More complete test.
 //--- CompletenessClient.swift
@@ -207,7 +207,7 @@ fileprivate func FileprivateFuncUsesInternal(_: InternalImportType) {
 fileprivate func FileprivateFuncUsesFileprivate(_: FileprivateImportType) {
     var _: FileprivateImportType
 }
-fileprivate func FileprivateFuncUsesPrivate(_: PrivateImportType) { // expected-error {{function cannot be declared fileprivate because its parameter uses a private type}}
+fileprivate func FileprivateFuncUsesPrivate(_: PrivateImportType) {
     var _: PrivateImportType
 }
 
@@ -291,7 +291,7 @@ fileprivate func FileprivateFuncReturnUsesInternal() -> InternalImportType {
 fileprivate func FileprivateFuncReturnUsesFileprivate() -> FileprivateImportType {
     fatalError()
 }
-fileprivate func FileprivateFuncReturnUsesPrivate() -> PrivateImportType { // expected-error {{function cannot be declared fileprivate because its result uses a private type}}
+fileprivate func FileprivateFuncReturnUsesPrivate() -> PrivateImportType {
     fatalError()
 }
 
@@ -415,7 +415,7 @@ fileprivate struct FileprivateSubscriptUsesFileprivate {
     }
 }
 fileprivate struct FileprivateSubscriptUsesPrivate {
-    fileprivate subscript(index: PrivateImportType) -> PrivateImportType { // expected-error {{subscript cannot be declared fileprivate because its index uses a private type}}
+    fileprivate subscript(index: PrivateImportType) -> PrivateImportType {
         fatalError()
     }
 }
@@ -462,7 +462,7 @@ public protocol PublicProtoRefinesPrivate: PrivateImportProto {} // expected-err
 public class PublicSubclassPublic : PublicImportClass {}
 public class PublicSubclassPackage : PackageImportClass {} // expected-error {{class cannot be declared public because its superclass is package}}
 public class PublicSubclassInternal : InternalImportClass {} // expected-error {{class cannot be declared public because its superclass is internal}}
-public class PublicSubclassFilepriovate : FileprivateImportClass {} // expected-error {{class cannot be declared public because its superclass is fileprivate}}
+public class PublicSubclassFileprivate : FileprivateImportClass {} // expected-error {{class cannot be declared public because its superclass is fileprivate}}
 public class PublicSubclassPrivate : PrivateImportClass {} // expected-error {{class cannot be declared public because its superclass is private}}
 
 
@@ -481,7 +481,7 @@ package protocol PackageProtoRefinesPrivate: PrivateImportProto {} // expected-e
 package class PackageSubclassPublic : PublicImportClass {}
 package class PackageSubclassPackage : PackageImportClass {}
 package class PackageSubclassInternal : InternalImportClass {} // expected-error {{class cannot be declared package because its superclass is internal}}
-package class PackageSubclassFilepriovate : FileprivateImportClass {} // expected-error {{class cannot be declared package because its superclass is fileprivate}}
+package class PackageSubclassFileprivate : FileprivateImportClass {} // expected-error {{class cannot be declared package because its superclass is fileprivate}}
 package class PackageSubclassPrivate : PrivateImportClass {} // expected-error {{class cannot be declared package because its superclass is private}}
 
 
@@ -500,27 +500,27 @@ internal protocol InternalProtoRefinesPrivate: PrivateImportProto {} // expected
 internal class InternalSubclassPublic : PublicImportClass {}
 internal class InternalSubclassPackage : PackageImportClass {}
 internal class InternalSubclassInternal : InternalImportClass {}
-internal class InternalSubclassFilepriovate : FileprivateImportClass {} // expected-error {{class cannot be declared internal because its superclass is fileprivate}}
+internal class InternalSubclassFileprivate : FileprivateImportClass {} // expected-error {{class cannot be declared internal because its superclass is fileprivate}}
 internal class InternalSubclassPrivate : PrivateImportClass {} // expected-error {{class cannot be declared internal because its superclass is private}}
 
 
 fileprivate protocol FileprivateProtoUsesPublic: PublicImportProto where T == PublicImportType {}
 fileprivate protocol FileprivateProtoWherePackage: PublicImportProto where T == PackageImportType {}
 fileprivate protocol FileprivateProtoWhereInternal: PublicImportProto where T == InternalImportType {}
 fileprivate protocol FileprivateProtoWhereFileprivate: PublicImportProto where T == FileprivateImportType {}
-fileprivate protocol FileprivateProtoWherePrivate: PublicImportProto where T == PrivateImportType {} // expected-error {{fileprivate protocol's 'where' clause cannot use a private struct}}
+fileprivate protocol FileprivateProtoWherePrivate: PublicImportProto where T == PrivateImportType {}
 
 fileprivate protocol FileprivateProtoRefinesPublic: PublicImportProto {}
 fileprivate protocol FileprivateProtoRefinesPackage: PackageImportProto {}
 fileprivate protocol FileprivateProtoRefinesInternal: InternalImportProto {}
 fileprivate protocol FileprivateProtoRefinesFileprivate: FileprivateImportProto {}
-fileprivate protocol FileprivateProtoRefinesPrivate: PrivateImportProto {} // expected-error {{fileprivate protocol cannot refine a private protocol}}
+fileprivate protocol FileprivateProtoRefinesPrivate: PrivateImportProto {}
 
 fileprivate class FileprivateSubclassPublic : PublicImportClass {}
 fileprivate class FileprivateSubclassPackage : PackageImportClass {}
 fileprivate class FileprivateSubclassInternal : InternalImportClass {}
-fileprivate class FileprivateSubclassFilepriovate : FileprivateImportClass {}
-fileprivate class FileprivateSubclassPrivate : PrivateImportClass {} // expected-error {{class cannot be declared fileprivate because its superclass is private}}
+fileprivate class FileprivateSubclassFileprivate : FileprivateImportClass {}
+fileprivate class FileprivateSubclassPrivate : PrivateImportClass {}
 
 
 private protocol PrivateProtoUsesPublic: PublicImportProto where T == PublicImportType {}
@@ -538,7 +538,7 @@ private protocol PrivateProtoRefinesPrivate: PrivateImportProto {}
 private class PrivateSubclassPublic : PublicImportClass {}
 private class PrivateSubclassPackage : PackageImportClass {}
 private class PrivateSubclassInternal : InternalImportClass {}
-private class PrivateSubclassFilepriovate : FileprivateImportClass {}
+private class PrivateSubclassFileprivate : FileprivateImportClass {}
 private class PrivateSubclassPrivate : PrivateImportClass {}
 
 public struct PublicTypeAliasUses {
@@ -570,7 +570,7 @@ fileprivate struct FileprivateTypeAliasUses {
     fileprivate typealias TAPackage = PackageImportProto
     fileprivate typealias TAInternal = InternalImportProto
     fileprivate typealias TAFileprivate = FileprivateImportProto
-    fileprivate typealias TAPrivate = PrivateImportProto // expected-error {{type alias cannot be declared fileprivate because its underlying type uses a private type}}
+    fileprivate typealias TAPrivate = PrivateImportProto
 }
 
 private struct PrivateTypeAliasUses {
@@ -629,13 +629,13 @@ fileprivate protocol FileprivateProtocol {
     associatedtype ATDefaultInternal = InternalImportProto
     associatedtype ATDefaultFileprivate = FileprivateImportProto
     // Accocciated type have a minimum formal access level at internal.
-    associatedtype ATDefaultPrivate = PrivateImportProto // expected-error {{associated type in an internal protocol uses a private type in its default definition}}
+    associatedtype ATDefaultPrivate = PrivateImportProto
 
     associatedtype ATRequirePublic: PublicImportProto
     associatedtype ATRequirePackage: PackageImportProto
     associatedtype ATRequireInternal: InternalImportProto
     associatedtype ATRequireFileprivate: FileprivateImportProto
-    associatedtype ATRequirePrivate: PrivateImportProto // expected-error {{associated type in an internal protocol uses a private type in its requirement}}
+    associatedtype ATRequirePrivate: PrivateImportProto
 }
 
 private protocol PrivateProtocol {
@@ -732,7 +732,7 @@ fileprivate struct FileprivateVars {
     fileprivate var b: PackageImportType
     fileprivate var c: InternalImportType
     fileprivate var d: FileprivateImportType
-    fileprivate var e: PrivateImportType // expected-error {{property cannot be declared fileprivate because its type uses a private type}}
+    fileprivate var e: PrivateImportType
 
     @PublicLibWrapper
     fileprivate var f: PublicImportType
@@ -743,13 +743,13 @@ fileprivate struct FileprivateVars {
     @FileprivateLibWrapper
     fileprivate var i: PublicImportType
     @PrivateLibWrapper
-    fileprivate var j: PublicImportType // expected-error {{property cannot be declared fileprivate because its property wrapper type uses a private type}}
+    fileprivate var j: PublicImportType
 
     fileprivate var k = PublicImportType()
     fileprivate var l = PackageImportType()
     fileprivate var m = InternalImportType()
     fileprivate var n = FileprivateImportType()
-    fileprivate var o = PrivateImportType() // expected-error {{property cannot be declared fileprivate because its type 'PrivateImportType' uses a private type}}
+    fileprivate var o = PrivateImportType()
 }
 
 private struct PrivateVars {
@@ -799,7 +799,7 @@ fileprivate struct FileprivateGenericUsesPublic<A: PublicImportProto> {}
 fileprivate struct FileprivateGenericUsesPackage<A: PackageImportProto> {}
 fileprivate struct FileprivateGenericUsesInternal<A: InternalImportProto> {}
 fileprivate struct FileprivateGenericUsesFileprivate<A: FileprivateImportProto> {}
-fileprivate struct FileprivateGenericUsesPrivate<A: PrivateImportProto> {} // expected-error {{generic struct cannot be declared fileprivate because its generic parameter uses a private type}}
+fileprivate struct FileprivateGenericUsesPrivate<A: PrivateImportProto> {}
 
 private struct PrivateGenericUsesPublic<A: PublicImportProto> {}
 private struct PrivateGenericUsesPackage<A: PackageImportProto> {}
