Merge pull request #83362 from tshortli/reject-inaccessible-dynamic-member-lookup-subscripts

Sema: Diagnose `@dynamicMemberLookup` subscripts that are not accessible enough

Author: tshortli

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",

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;

test/SILOptimizer/diagnose_unreachable.swift

@@ -507,7 +507,7 @@ struct OuterStruct {
 }
 
 @dynamicMemberLookup
-public enum DynamicLookupEnum {
+enum DynamicLookupEnum {
     subscript<T>(dynamicMember keyPath: KeyPath<OuterStruct, T>) -> T {
         fatalError()
     }

test/SILOptimizer/keypath_opt_crash.swift

@@ -10,7 +10,7 @@ import Foundation
 public struct S {
   private let x: NSXPCConnection
 
-  subscript<T>(dynamicMember property: ReferenceWritableKeyPath<NSXPCConnection, T>) -> T {
+  public subscript<T>(dynamicMember property: ReferenceWritableKeyPath<NSXPCConnection, T>) -> T {
     get {
       x[keyPath: property]
     }

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
 //===----------------------------------------------------------------------===//

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
+  }
+}

test/embedded/keypath-crash.swift

@@ -12,7 +12,7 @@ public struct Binding<Value> {
     self.wrappedValue = get()
   }
 
-  subscript<Subject>(dynamicMember keyPath: WritableKeyPath<Value, Subject>) -> Binding<Subject> {
+  public subscript<Subject>(dynamicMember keyPath: WritableKeyPath<Value, Subject>) -> Binding<Subject> {
     get { fatalError() }
   }
 }

test/embedded/keypaths2.swift

@@ -12,14 +12,15 @@ public struct Box<T>: ~Copyable {
         self.value = UnsafeMutablePointer<T>.allocate(capacity: 1)
     }
 
-    subscript<U>(dynamicMember member: WritableKeyPath<T, U>) -> U {
+    public subscript<U>(dynamicMember member: WritableKeyPath<T, U>) -> U {
         @_transparent
         get { return self.value.pointer(to: member)!.pointee }
 
         @_transparent
         set { self.value.pointer(to: member)!.pointee = newValue }
     }
 
+    @usableFromInline
     var value: UnsafeMutablePointer<T>
 }