From 15913ba75c4706c368156d365c43bba7e84a4cfe Mon Sep 17 00:00:00 2001 From: Jordan Rose Date: Tue, 2 Apr 2019 09:29:01 -0700 Subject: [PATCH] Merge pull request #23726 from jrose-apple/substandard-by-any-name Check access control for the generic requirements of subscripts and typealiases (cherry picked from commit 4dcea1cad18035109ec602da27e5285d7c1b2f2f) --- lib/Sema/TypeCheckAccess.cpp | 50 ++++++++++++++++++--------- test/IDE/print_ast_tc_decls.swift | 4 +-- test/Sema/accessibility.swift | 10 ++++++ test/attr/attr_usableFromInline.swift | 10 +++++- 4 files changed, 55 insertions(+), 19 deletions(-) diff --git a/lib/Sema/TypeCheckAccess.cpp b/lib/Sema/TypeCheckAccess.cpp index 59f13d7a3856c..388ac8b42876d 100644 --- a/lib/Sema/TypeCheckAccess.cpp +++ b/lib/Sema/TypeCheckAccess.cpp @@ -162,29 +162,30 @@ void AccessControlCheckerBase::checkTypeAccessImpl( llvm::function_ref 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 typeAccessScope = - TypeAccessScopeChecker::getAccessScope(type, useDC, - checkUsableFromInline); - auto downgradeToWarning = DowngradeToWarning::No; + AccessScope problematicAccessScope = AccessScope::getPublic(); + if (type) { + Optional 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 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 @@ -366,9 +369,16 @@ void AccessControlCheckerBase::checkGenericParamAccess( if (minAccessScope.isPublic()) return; + // FIXME: Promote these to an error in the next -swift-version break. + if (isa(owner) || isa(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, @@ -536,6 +546,8 @@ class AccessControlChecker : public AccessControlCheckerBase, } void visitTypeAliasDecl(TypeAliasDecl *TAD) { + checkGenericParamAccess(TAD->getGenericParams(), TAD); + checkTypeAccess(TAD->getUnderlyingTypeLoc(), TAD, /*mayBeInferred*/false, [&](AccessScope typeAccessScope, const TypeRepr *complainRepr, @@ -803,6 +815,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; @@ -1120,6 +1134,8 @@ class UsableFromInlineChecker : public AccessControlCheckerBase, } void visitTypeAliasDecl(TypeAliasDecl *TAD) { + checkGenericParamAccess(TAD->getGenericParams(), TAD); + checkTypeAccess(TAD->getUnderlyingTypeLoc(), TAD, /*mayBeInferred*/false, [&](AccessScope typeAccessScope, const TypeRepr *complainRepr, @@ -1295,6 +1311,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, diff --git a/test/IDE/print_ast_tc_decls.swift b/test/IDE/print_ast_tc_decls.swift index 6fc4589bd5a19..9dff5585c5bf6 100644 --- a/test/IDE/print_ast_tc_decls.swift +++ b/test/IDE/print_ast_tc_decls.swift @@ -1375,5 +1375,5 @@ public typealias MyPairI = MyPair // PASS_PRINT_AST: public typealias MyPairI = MyPair public typealias MyPairAlias = MyPair // PASS_PRINT_AST: public typealias MyPairAlias = MyPair -public typealias MyPairAlias2 = MyPair where U: BarProtocol -// PASS_PRINT_AST: public typealias MyPairAlias2 = MyPair where T : FooProtocol, U : BarProtocol +typealias MyPairAlias2 = MyPair where U: BarProtocol +// PASS_PRINT_AST: typealias MyPairAlias2 = MyPair where T : FooProtocol, U : BarProtocol diff --git a/test/Sema/accessibility.swift b/test/Sema/accessibility.swift index 150502587f3c7..6bddacd91f2e5 100644 --- a/test/Sema/accessibility.swift +++ b/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) -> Int { return 0 } // expected-warning {{subscript should not be declared public because its generic parameter uses a private type}} {{none}} + public subscript(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 // expected-warning {{type alias should not be declared public because its generic parameter uses a private type}} +public typealias TestGenericAliasWhereClause = T where T: PrivateProto // expected-warning {{type alias should not be declared public because its generic requirement uses a private type}} diff --git a/test/attr/attr_usableFromInline.swift b/test/attr/attr_usableFromInline.swift index d23e9287c47ce..9673353e10c54 100644 --- a/test/attr/attr_usableFromInline.swift +++ b/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) -> Int { return 0 } // expected-warning {{type referenced from a generic parameter of a '@usableFromInline' subscript should be '@usableFromInline' or public}} + @usableFromInline subscript(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 // expected-warning {{type referenced from a generic parameter of a '@usableFromInline' type alias should be '@usableFromInline' or public}} +@usableFromInline typealias TestGenericAliasWhereClause = T where T: InternalProtocol // expected-warning {{type referenced from a generic requirement of a '@usableFromInline' type alias should be '@usableFromInline' or public}}