From 9302cbe568dce5d935f87791e67d071d93ac15d8 Mon Sep 17 00:00:00 2001 From: Allan Shortlidge Date: Sun, 27 Jul 2025 22:56:08 -0700 Subject: [PATCH] Sema: Diagnose `@dynamicMemberLookup` subscripts that are not accessible enough. Since this is a source breaking change, downgrade the diagnostic to a warning until the next language version unless library evolution is enabled, in which case this would have resulted in a broken `.swiftinterface` and it therefore makes sense to diagnose eagerly. Resolves rdar://156919010. --- include/swift/AST/DiagnosticsSema.def | 4 + lib/Sema/TypeCheckAttr.cpp | 30 +++++++ test/SILOptimizer/diagnose_unreachable.swift | 2 +- test/SILOptimizer/keypath_opt_crash.swift | 2 +- test/attr/attr_dynamic_member_lookup.swift | 83 ++++++++++++++++++- .../attr_dynamic_member_lookup_swift7.swift | 74 +++++++++++++++++ test/embedded/keypath-crash.swift | 2 +- test/embedded/keypaths2.swift | 3 +- 8 files changed, 195 insertions(+), 5 deletions(-) create mode 100644 test/attr/attr_dynamic_member_lookup_swift7.swift diff --git a/include/swift/AST/DiagnosticsSema.def b/include/swift/AST/DiagnosticsSema.def index 59dc7f29a680e..c193b8727e171 100644 --- a/include/swift/AST/DiagnosticsSema.def +++ b/include/swift/AST/DiagnosticsSema.def @@ -1730,6 +1730,10 @@ ERROR(invalid_dynamic_member_lookup_type,none, NOTE(invalid_dynamic_member_subscript, none, "add an explicit argument label to this subscript to satisfy " "the '@dynamicMemberLookup' requirement", ()) +ERROR(dynamic_member_lookup_candidate_inaccessible,none, + "'@dynamicMemberLookup' requires %0 to be as accessible as its " + "enclosing type", + (ValueDecl *)) ERROR(string_index_not_integer,none, "String must not be indexed with %0, it has variable size elements", diff --git a/lib/Sema/TypeCheckAttr.cpp b/lib/Sema/TypeCheckAttr.cpp index 05531e2b657e9..0b5434b2d6550 100644 --- a/lib/Sema/TypeCheckAttr.cpp +++ b/lib/Sema/TypeCheckAttr.cpp @@ -2141,6 +2141,36 @@ visitDynamicMemberLookupAttr(DynamicMemberLookupAttr *attr) { if (candidates.empty()) { emitInvalidTypeDiagnostic(oneCandidate->getLoc()); + return; + } + + // Diagnose if there are no candidates that are sufficiently accessible. + auto requiredAccessScope = decl->getFormalAccessScope(/*useDC=*/nullptr); + auto accessDC = requiredAccessScope.getDeclContext(); + auto inaccessibleCandidate = candidates.front().getValueDecl(); + candidates.filter([&](LookupResultEntry entry, bool isOuter) -> bool { + return entry.getValueDecl()->isAccessibleFrom(accessDC); + }); + + if (candidates.empty()) { + auto futureVersion = version::Version::getFutureMajorLanguageVersion(); + bool shouldError = ctx.isSwiftVersionAtLeast(futureVersion); + + // Diagnose as an error in resilient modules regardless of language + // version since this will break the swiftinterface. + shouldError |= decl->getModuleContext()->isResilient(); + + auto diag = diagnose(inaccessibleCandidate->getLoc(), + diag::dynamic_member_lookup_candidate_inaccessible, + inaccessibleCandidate); + fixItAccess(diag, inaccessibleCandidate, + requiredAccessScope.requiredAccessForDiagnostics()); + diag.warnUntilSwiftVersionIf(!shouldError, futureVersion); + + if (shouldError) { + attr->setInvalid(); + return; + } } return; diff --git a/test/SILOptimizer/diagnose_unreachable.swift b/test/SILOptimizer/diagnose_unreachable.swift index ee8cbd9fe24c9..36e81087202c1 100644 --- a/test/SILOptimizer/diagnose_unreachable.swift +++ b/test/SILOptimizer/diagnose_unreachable.swift @@ -507,7 +507,7 @@ struct OuterStruct { } @dynamicMemberLookup -public enum DynamicLookupEnum { +enum DynamicLookupEnum { subscript(dynamicMember keyPath: KeyPath) -> T { fatalError() } diff --git a/test/SILOptimizer/keypath_opt_crash.swift b/test/SILOptimizer/keypath_opt_crash.swift index 6117c41ab7489..a65f56cfc5bea 100644 --- a/test/SILOptimizer/keypath_opt_crash.swift +++ b/test/SILOptimizer/keypath_opt_crash.swift @@ -10,7 +10,7 @@ import Foundation public struct S { private let x: NSXPCConnection - subscript(dynamicMember property: ReferenceWritableKeyPath) -> T { + public subscript(dynamicMember property: ReferenceWritableKeyPath) -> T { get { x[keyPath: property] } diff --git a/test/attr/attr_dynamic_member_lookup.swift b/test/attr/attr_dynamic_member_lookup.swift index c921c4aee0414..780d51123bdb9 100644 --- a/test/attr/attr_dynamic_member_lookup.swift +++ b/test/attr/attr_dynamic_member_lookup.swift @@ -1,4 +1,5 @@ -// RUN: %target-typecheck-verify-swift -enable-experimental-feature KeyPathWithMethodMembers +// RUN: %target-typecheck-verify-swift -enable-experimental-feature KeyPathWithMethodMembers -verify-additional-prefix non-resilient- +// RUN: %target-typecheck-verify-swift -enable-experimental-feature KeyPathWithMethodMembers -enable-library-evolution -verify-additional-prefix resilient- // REQUIRES: swift_feature_KeyPathWithMethodMembers var global = 42 @@ -198,6 +199,86 @@ class InvalidDerived : InvalidBase { subscript(dynamicMember: String) -> Int { g // expected-error @+1 {{value of type 'InvalidDerived' has no member 'dynamicallyLookedUp'}} _ = InvalidDerived().dynamicallyLookedUp +//===----------------------------------------------------------------------===// +// Access level +//===----------------------------------------------------------------------===// + +@dynamicMemberLookup +public struct Accessible1 { + public subscript(dynamicMember member: String) -> Int { + return 42 + } +} + +@dynamicMemberLookup +private struct Accessible2 { + fileprivate subscript(dynamicMember member: String) -> Int { + return 42 + } +} + +@dynamicMemberLookup +open class Accessible3 { + public subscript(dynamicMember member: String) -> Int { + return 42 + } +} + +@dynamicMemberLookup +open class Accessible4 { + open subscript(dynamicMember member: String) -> Int { + return 42 + } +} + +@dynamicMemberLookup +public struct Accessible5 { + subscript(dynamicMember member: String) -> Int { + return 42 + } + + public subscript(dynamicMember member: StaticString) -> Int { + return 42 + } +} + +@dynamicMemberLookup +public struct Inaccessible1 { + // expected-non-resilient-warning @+2 {{'@dynamicMemberLookup' requires 'subscript(dynamicMember:)' to be as accessible as its enclosing type; this will be an error in a future Swift language mode}}{{3-3=public }} + // expected-resilient-error @+1 {{'@dynamicMemberLookup' requires 'subscript(dynamicMember:)' to be as accessible as its enclosing type}}{{3-3=public }} + subscript(dynamicMember member: String) -> Int { + return 42 + } +} + +@dynamicMemberLookup +public struct Inaccessible2 { + // expected-non-resilient-warning @+3 {{'@dynamicMemberLookup' requires 'subscript(dynamicMember:)' to be as accessible as its enclosing type; this will be an error in a future Swift language mode}}{{21-29=public}} + // expected-resilient-error @+2 {{'@dynamicMemberLookup' requires 'subscript(dynamicMember:)' to be as accessible as its enclosing type}}{{21-29=public}} + // expected-error @+1 {{'@usableFromInline' attribute can only be applied to internal or package declarations, but subscript 'subscript(dynamicMember:)' is public}} + @usableFromInline internal subscript(dynamicMember member: String) -> Int { + return 42 + } +} + +@dynamicMemberLookup +internal struct Inaccessible3 { + // expected-non-resilient-warning @+2 {{'@dynamicMemberLookup' requires 'subscript(dynamicMember:)' to be as accessible as its enclosing type; this will be an error in a future Swift language mode}}{{3-10=internal}} + // expected-resilient-error @+1 {{'@dynamicMemberLookup' requires 'subscript(dynamicMember:)' to be as accessible as its enclosing type}}{{3-10=internal}} + private subscript(dynamicMember member: String) -> Int { + return 42 + } +} + +@dynamicMemberLookup +private struct Inaccessible4 { + // expected-non-resilient-warning @+2 {{'@dynamicMemberLookup' requires 'subscript(dynamicMember:)' to be as accessible as its enclosing type; this will be an error in a future Swift language mode}}{{3-10=fileprivate}} + // expected-resilient-error @+1 {{'@dynamicMemberLookup' requires 'subscript(dynamicMember:)' to be as accessible as its enclosing type}}{{3-10=fileprivate}} + private subscript(dynamicMember member: String) -> Int { + return 42 + } +} + //===----------------------------------------------------------------------===// // Existentials //===----------------------------------------------------------------------===// diff --git a/test/attr/attr_dynamic_member_lookup_swift7.swift b/test/attr/attr_dynamic_member_lookup_swift7.swift new file mode 100644 index 0000000000000..8361d6d0c4aa6 --- /dev/null +++ b/test/attr/attr_dynamic_member_lookup_swift7.swift @@ -0,0 +1,74 @@ +// RUN: %target-typecheck-verify-swift -swift-version 7 +// REQUIRES: swift7 + +@dynamicMemberLookup +public struct Accessible1 { + public subscript(dynamicMember member: String) -> Int { + return 42 + } +} + +@dynamicMemberLookup +private struct Accessible2 { + fileprivate subscript(dynamicMember member: String) -> Int { + return 42 + } +} + +@dynamicMemberLookup +open class Accessible3 { + public subscript(dynamicMember member: String) -> Int { + return 42 + } +} + +@dynamicMemberLookup +open class Accessible4 { + open subscript(dynamicMember member: String) -> Int { + return 42 + } +} + +@dynamicMemberLookup +public struct Accessible5 { + subscript(dynamicMember member: String) -> Int { + return 42 + } + + public subscript(dynamicMember member: StaticString) -> Int { + return 42 + } +} + +@dynamicMemberLookup +public struct Inaccessible1 { + // expected-error @+1 {{'@dynamicMemberLookup' requires 'subscript(dynamicMember:)' to be as accessible as its enclosing type}}{{3-3=public }} + subscript(dynamicMember member: String) -> Int { + return 42 + } +} + +@dynamicMemberLookup +public struct Inaccessible2 { + // expected-error @+2 {{'@dynamicMemberLookup' requires 'subscript(dynamicMember:)' to be as accessible as its enclosing type}}{{21-29=public}} + // expected-error @+1 {{'@usableFromInline' attribute can only be applied to internal or package declarations, but subscript 'subscript(dynamicMember:)' is public}} + @usableFromInline internal subscript(dynamicMember member: String) -> Int { + return 42 + } +} + +@dynamicMemberLookup +internal struct Inaccessible3 { + // expected-error @+1 {{'@dynamicMemberLookup' requires 'subscript(dynamicMember:)' to be as accessible as its enclosing type}}{{3-10=internal}} + private subscript(dynamicMember member: String) -> Int { + return 42 + } +} + +@dynamicMemberLookup +private struct Inaccessible4 { + // expected-error @+1 {{'@dynamicMemberLookup' requires 'subscript(dynamicMember:)' to be as accessible as its enclosing type}}{{3-10=fileprivate}} + private subscript(dynamicMember member: String) -> Int { + return 42 + } +} diff --git a/test/embedded/keypath-crash.swift b/test/embedded/keypath-crash.swift index ae62fb9f2709b..6487a9f537d05 100644 --- a/test/embedded/keypath-crash.swift +++ b/test/embedded/keypath-crash.swift @@ -12,7 +12,7 @@ public struct Binding { self.wrappedValue = get() } - subscript(dynamicMember keyPath: WritableKeyPath) -> Binding { + public subscript(dynamicMember keyPath: WritableKeyPath) -> Binding { get { fatalError() } } } diff --git a/test/embedded/keypaths2.swift b/test/embedded/keypaths2.swift index 691962f344409..1d743f9509a40 100644 --- a/test/embedded/keypaths2.swift +++ b/test/embedded/keypaths2.swift @@ -12,7 +12,7 @@ public struct Box: ~Copyable { self.value = UnsafeMutablePointer.allocate(capacity: 1) } - subscript(dynamicMember member: WritableKeyPath) -> U { + public subscript(dynamicMember member: WritableKeyPath) -> U { @_transparent get { return self.value.pointer(to: member)!.pointee } @@ -20,6 +20,7 @@ public struct Box: ~Copyable { set { self.value.pointer(to: member)!.pointee = newValue } } + @usableFromInline var value: UnsafeMutablePointer }