Merge pull request #23726 from jrose-apple/substandard-by-any-name

Check access control for the generic requirements of subscripts and typealiases

Author: jrose-apple

lib/Sema/TypeCheckAccess.cpp

@@ -162,29 +162,30 @@ void AccessControlCheckerBase::checkTypeAccessImpl(
     llvm::function_ref<CheckTypeAccessCallback> diagnose) {
   if (TC.Context.isAccessControlDisabled())
     return;
-  if (!type)
-    return;
   // Don't spend time checking local declarations; this is always valid by the
   // time we get to this point.
   if (!contextAccessScope.isPublic() &&
       contextAccessScope.getDeclContext()->isLocalContext())
     return;
 
-  Optional<AccessScope> typeAccessScope =
-      TypeAccessScopeChecker::getAccessScope(type, useDC,
-                                             checkUsableFromInline);
-  auto downgradeToWarning = DowngradeToWarning::No;
+  AccessScope problematicAccessScope = AccessScope::getPublic();
+  if (type) {
+    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.
-  if (!typeAccessScope.hasValue())
-    return;
+    // 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.
+    if (!typeAccessScope.hasValue())
+      return;
+    problematicAccessScope = *typeAccessScope;
+  }
 
-  AccessScope problematicAccessScope = *typeAccessScope;
+  auto downgradeToWarning = DowngradeToWarning::No;
 
-  if (contextAccessScope.hasEqualDeclContextWith(*typeAccessScope) ||
-      contextAccessScope.isChildOf(*typeAccessScope)) {
+  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.
@@ -194,7 +195,9 @@ void AccessControlCheckerBase::checkTypeAccessImpl(
     Optional<AccessScope> typeReprAccessScope =
         TypeReprAccessScopeChecker::getAccessScope(typeRepr, useDC,
                                                    checkUsableFromInline);
-    assert(typeReprAccessScope && "valid Type but not valid TypeRepr?");
+    if (!typeReprAccessScope.hasValue())
+      return;
+
     if (contextAccessScope.hasEqualDeclContextWith(*typeReprAccessScope) ||
         contextAccessScope.isChildOf(*typeReprAccessScope)) {
       // Only if both the Type and the TypeRepr follow the access rules can
@@ -365,9 +368,16 @@ void AccessControlCheckerBase::checkGenericParamAccess(
   if (minAccessScope.isPublic())
     return;
 
+  // FIXME: Promote these to an error in the next -swift-version break.
+  if (isa<SubscriptDecl>(owner) || isa<TypeAliasDecl>(owner))
+    downgradeToWarning = DowngradeToWarning::Yes;
+
   if (checkUsableFromInline) {
-    auto diagID = diag::generic_param_usable_from_inline;
     if (!TC.Context.isSwiftVersionAtLeast(5))
+      downgradeToWarning = DowngradeToWarning::Yes;
+
+    auto diagID = diag::generic_param_usable_from_inline;
+    if (downgradeToWarning == DowngradeToWarning::Yes)
       diagID = diag::generic_param_usable_from_inline_warn;
     auto diag = TC.diagnose(owner,
                             diagID,
@@ -535,6 +545,8 @@ class AccessControlChecker : public AccessControlCheckerBase,
   }
 
   void visitTypeAliasDecl(TypeAliasDecl *TAD) {
+    checkGenericParamAccess(TAD->getGenericParams(), TAD);
+
     checkTypeAccess(TAD->getUnderlyingTypeLoc(), TAD, /*mayBeInferred*/false,
                     [&](AccessScope typeAccessScope,
                         const TypeRepr *complainRepr,
@@ -802,6 +814,8 @@ class AccessControlChecker : public AccessControlCheckerBase,
   }
 
   void visitSubscriptDecl(SubscriptDecl *SD) {
+    checkGenericParamAccess(SD->getGenericParams(), SD);
+
     auto minAccessScope = AccessScope::getPublic();
     const TypeRepr *complainRepr = nullptr;
     auto downgradeToWarning = DowngradeToWarning::No;
@@ -1119,6 +1133,8 @@ class UsableFromInlineChecker : public AccessControlCheckerBase,
   }
 
   void visitTypeAliasDecl(TypeAliasDecl *TAD) {
+    checkGenericParamAccess(TAD->getGenericParams(), TAD);
+
     checkTypeAccess(TAD->getUnderlyingTypeLoc(), TAD, /*mayBeInferred*/false,
                     [&](AccessScope typeAccessScope,
                         const TypeRepr *complainRepr,
@@ -1294,6 +1310,8 @@ class UsableFromInlineChecker : public AccessControlCheckerBase,
   }
 
   void visitSubscriptDecl(SubscriptDecl *SD) {
+    checkGenericParamAccess(SD->getGenericParams(), SD);
+
     for (auto &P : *SD->getIndices()) {
       checkTypeAccess(P->getTypeLoc(), SD, /*mayBeInferred*/false,
                       [&](AccessScope typeAccessScope,

test/IDE/print_ast_tc_decls.swift

@@ -1375,5 +1375,5 @@ public typealias MyPairI<B> = MyPair<Int, B>
 // PASS_PRINT_AST: public typealias MyPairI<B> = MyPair<Int, B>
 public typealias MyPairAlias<T, U> = MyPair<T, U>
 // PASS_PRINT_AST: public typealias MyPairAlias<T, U> = MyPair<T, U>
-public typealias MyPairAlias2<T: FooProtocol, U> = MyPair<T, U> where U: BarProtocol
-// PASS_PRINT_AST: public typealias MyPairAlias2<T, U> = MyPair<T, U> where T : FooProtocol, U : BarProtocol
+typealias MyPairAlias2<T: FooProtocol, U> = MyPair<T, U> where U: BarProtocol
+// PASS_PRINT_AST: typealias MyPairAlias2<T, U> = MyPair<T, U> where T : FooProtocol, U : BarProtocol

test/Sema/accessibility.swift

@@ -869,3 +869,13 @@ public extension ObjCSub {
     set {}
   }
 }
+
+public struct TestGenericSubscripts {
+  // We'd like these to be errors in a future version of Swift, but they weren't
+  // in Swift 5.0.
+  public subscript<T: PrivateProto>(_: T) -> Int { return 0 } // expected-warning {{subscript should not be declared public because its generic parameter uses a private type}} {{none}}
+  public subscript<T>(where _: T) -> Int where T: PrivateProto { return 0 } // expected-warning {{subscript should not be declared public because its generic requirement uses a private type}} {{none}}
+}
+
+public typealias TestGenericAlias<T: PrivateProto> = T // expected-warning {{type alias should not be declared public because its generic parameter uses a private type}}
+public typealias TestGenericAliasWhereClause<T> = T where T: PrivateProto // expected-warning {{type alias should not be declared public because its generic requirement uses a private type}}

test/attr/attr_usableFromInline.swift

@@ -87,7 +87,7 @@ internal struct InternalStruct {}
 // expected-error@-1 {{type referenced from the underlying type of a '@usableFromInline' type alias must be '@usableFromInline' or public}}
 
 protocol InternalProtocol {
-  // expected-note@-1 4{{type declared here}}
+  // expected-note@-1 * {{type declared here}}
   associatedtype T
 }
 
@@ -147,3 +147,11 @@ enum BadEnum {
 @usableFromInline
 class BadClass : InternalClass {}
 // expected-error@-1 {{type referenced from the superclass of a '@usableFromInline' class must be '@usableFromInline' or public}}
+
+public struct TestGenericSubscripts {
+  @usableFromInline subscript<T: InternalProtocol>(_: T) -> Int { return 0 } // expected-warning {{type referenced from a generic parameter of a '@usableFromInline' subscript should be '@usableFromInline' or public}}
+  @usableFromInline subscript<T>(where _: T) -> Int where T: InternalProtocol { return 0 } // expected-warning {{type referenced from a generic requirement of a '@usableFromInline' subscript should be '@usableFromInline' or public}}
+}
+
+@usableFromInline typealias TestGenericAlias<T: InternalProtocol> = T // expected-warning {{type referenced from a generic parameter of a '@usableFromInline' type alias should be '@usableFromInline' or public}}
+@usableFromInline typealias TestGenericAliasWhereClause<T> = T where T: InternalProtocol // expected-warning {{type referenced from a generic requirement of a '@usableFromInline' type alias should be '@usableFromInline' or public}}