Merge pull request #83054 from tshortli/unavailable-setters-require-read-only-key-paths

ConstraintSystem: Make key paths for properties with unavailable setters read-only

Author: tshortli

include/swift/AST/AvailabilityConstraint.h

@@ -174,6 +174,12 @@ enum class AvailabilityConstraintFlag : uint8_t {
   /// Include constraints for all domains, regardless of whether they are active
   /// or relevant to type checking.
   IncludeAllDomains = 1 << 1,
+
+  /// By default, non-type declarations that are universally unavailable are
+  /// always diagnosed, regardless of whether the context of the reference
+  /// is also universally unavailable. If this flag is set, though, those
+  /// references are allowed.
+  AllowUniversallyUnavailableInCompatibleContexts = 1 << 2,
 };
 using AvailabilityConstraintFlags = OptionSet<AvailabilityConstraintFlag>;
 

lib/AST/AvailabilityConstraint.cpp

@@ -118,11 +118,16 @@ DeclAvailabilityConstraints::getPrimaryConstraint() const {
 }
 
 static bool canIgnoreConstraintInUnavailableContexts(
-    const Decl *decl, const AvailabilityConstraint &constraint) {
+    const Decl *decl, const AvailabilityConstraint &constraint,
+    const AvailabilityConstraintFlags flags) {
   auto domain = constraint.getDomain();
 
   switch (constraint.getReason()) {
   case AvailabilityConstraint::Reason::UnconditionallyUnavailable:
+    if (flags.contains(AvailabilityConstraintFlag::
+                           AllowUniversallyUnavailableInCompatibleContexts))
+      return true;
+
     // Always reject uses of universally unavailable declarations, regardless
     // of context, since there are no possible compilation configurations in
     // which they are available. However, make an exception for types and
@@ -162,11 +167,12 @@ static bool canIgnoreConstraintInUnavailableContexts(
 static bool
 shouldIgnoreConstraintInContext(const Decl *decl,
                                 const AvailabilityConstraint &constraint,
-                                const AvailabilityContext &context) {
+                                const AvailabilityContext &context,
+                                const AvailabilityConstraintFlags flags) {
   if (!context.isUnavailable())
     return false;
 
-  if (!canIgnoreConstraintInUnavailableContexts(decl, constraint))
+  if (!canIgnoreConstraintInUnavailableContexts(decl, constraint, flags))
     return false;
 
   return context.containsUnavailableDomain(constraint.getDomain());
@@ -256,7 +262,7 @@ static void getAvailabilityConstraintsForDecl(
   // declaration is unconditionally unavailable in a domain for which
   // the context is already unavailable.
   llvm::erase_if(constraints, [&](const AvailabilityConstraint &constraint) {
-    return shouldIgnoreConstraintInContext(decl, constraint, context);
+    return shouldIgnoreConstraintInContext(decl, constraint, context, flags);
   });
 }
 

lib/Sema/ConstraintSystem.cpp

@@ -4997,11 +4997,7 @@ bool ConstraintSystem::isReadOnlyKeyPathComponent(
   // If the setter is unavailable, then the keypath ought to be read-only
   // in this context.
   if (auto setter = storage->getOpaqueAccessor(AccessorKind::Set)) {
-    // FIXME: [availability] Fully unavailable setters should cause the key path
-    // to be readonly too.
-    auto constraint =
-        getUnsatisfiedAvailabilityConstraint(setter, DC, referenceLoc);
-    if (constraint && constraint->isPotentiallyAvailable())
+    if (getUnsatisfiedAvailabilityConstraint(setter, DC, referenceLoc))
       return true;
   }
 

lib/Sema/TypeCheckAvailability.cpp

@@ -1771,8 +1771,17 @@ std::optional<AvailabilityConstraint>
 swift::getUnsatisfiedAvailabilityConstraint(const Decl *decl,
                                             const DeclContext *referenceDC,
                                             SourceLoc referenceLoc) {
+  AvailabilityConstraintFlags flags;
+
+  // In implicit code, allow references to universally unavailable declarations
+  // as long as the context is also universally unavailable.
+  if (referenceLoc.isInvalid())
+    flags |= AvailabilityConstraintFlag::
+        AllowUniversallyUnavailableInCompatibleContexts;
+
   return getAvailabilityConstraintsForDecl(
-             decl, AvailabilityContext::forLocation(referenceLoc, referenceDC))
+             decl, AvailabilityContext::forLocation(referenceLoc, referenceDC),
+             flags)
       .getPrimaryConstraint();
 }
 

lib/Sema/TypeCheckStorage.cpp

@@ -1063,7 +1063,7 @@ getPropertyWrapperLValueness(VarDecl *var) {
 ///     - Wrapped: \c self._member.wrappedValue
 ///     - Composition: \c self._member.wrappedValue.wrappedValue….wrappedValue
 ///     - Projected: \c self._member.projectedValue
-///     - Enclosed instance: \c Wrapper[_enclosedInstance: self, …]
+///     - Enclosed instance: \c Wrapper[_enclosingInstance: self, …]
 static Expr *buildStorageReference(AccessorDecl *accessor,
                                    AbstractStorageDecl *storage,
                                    TargetImpl target,

test/Availability/availability_accessors.swift

@@ -63,14 +63,14 @@ struct BaseStruct<T> {
   var unavailableSetter: T {
     get { fatalError() }
     @available(*, unavailable)
-    set { fatalError() } // expected-note 38 {{setter for 'unavailableSetter' has been explicitly marked unavailable here}}
+    set { fatalError() } // expected-note 33 {{setter for 'unavailableSetter' has been explicitly marked unavailable here}}
   }
 
   var unavailableGetterAndSetter: T {
     @available(*, unavailable)
     get { fatalError() } // expected-note 67 {{getter for 'unavailableGetterAndSetter' has been explicitly marked unavailable here}}
     @available(*, unavailable)
-    set { fatalError() } // expected-note 38 {{setter for 'unavailableGetterAndSetter' has been explicitly marked unavailable here}}
+    set { fatalError() } // expected-note 33 {{setter for 'unavailableGetterAndSetter' has been explicitly marked unavailable here}}
   }
 }
 
@@ -318,17 +318,17 @@ func testKeyPathAssignments_Struct(_ someValue: StructValue) {
   a[keyPath: \.[takesInOut(&x.unavailableGetter.a.b)]] = 0 // expected-error {{getter for 'unavailableGetter' is unavailable}}
   a[keyPath: \.[takesInOut(&x.unavailableGetter[0].b)]] = 0 // expected-error {{getter for 'unavailableGetter' is unavailable}}
 
-  x[keyPath: \.unavailableSetter] = someValue // expected-error {{setter for 'unavailableSetter' is unavailable}}
-  x[keyPath: \.unavailableSetter.a] = someValue.a // expected-error {{setter for 'unavailableSetter' is unavailable}}
-  x[keyPath: \.unavailableSetter[0]] = someValue.a // expected-error {{setter for 'unavailableSetter' is unavailable}}
-  x[keyPath: \.unavailableSetter[0].b] = 1 // expected-error {{setter for 'unavailableSetter' is unavailable}}
+  x[keyPath: \.unavailableSetter] = someValue // expected-error {{cannot assign through subscript: key path is read-only}}
+  x[keyPath: \.unavailableSetter.a] = someValue.a // expected-error {{cannot assign through subscript: key path is read-only}}
+  x[keyPath: \.unavailableSetter[0]] = someValue.a // expected-error {{cannot assign through subscript: key path is read-only}}
+  x[keyPath: \.unavailableSetter[0].b] = 1 // expected-error {{cannot assign through subscript: key path is read-only}}
   a[keyPath: \.[takesInOut(&x.unavailableSetter.a.b)]] = 0 // expected-error {{setter for 'unavailableSetter' is unavailable}}
   a[keyPath: \.[takesInOut(&x.unavailableSetter[0].b)]] = 0 // expected-error {{setter for 'unavailableSetter' is unavailable}}
 
-  x[keyPath: \.unavailableGetterAndSetter] = someValue // expected-error {{setter for 'unavailableGetterAndSetter' is unavailable}}
-  x[keyPath: \.unavailableGetterAndSetter.a] = someValue.a // expected-error {{setter for 'unavailableGetterAndSetter' is unavailable}}
-  x[keyPath: \.unavailableGetterAndSetter[0]] = someValue.a // expected-error {{setter for 'unavailableGetterAndSetter' is unavailable}}
-  x[keyPath: \.unavailableGetterAndSetter[0].b] = 1 // expected-error {{setter for 'unavailableGetterAndSetter' is unavailable}}
+  x[keyPath: \.unavailableGetterAndSetter] = someValue // expected-error {{cannot assign through subscript: key path is read-only}}
+  x[keyPath: \.unavailableGetterAndSetter.a] = someValue.a // expected-error {{cannot assign through subscript: key path is read-only}}
+  x[keyPath: \.unavailableGetterAndSetter[0]] = someValue.a // expected-error {{cannot assign through subscript: key path is read-only}}
+  x[keyPath: \.unavailableGetterAndSetter[0].b] = 1 // expected-error {{cannot assign through subscript: key path is read-only}}
   a[keyPath: \.[takesInOut(&x.unavailableGetterAndSetter.a.b)]] = 0 // expected-error {{getter for 'unavailableGetterAndSetter' is unavailable}} expected-error {{setter for 'unavailableGetterAndSetter' is unavailable}}
   a[keyPath: \.[takesInOut(&x.unavailableGetterAndSetter[0].b)]] = 0 // expected-error {{getter for 'unavailableGetterAndSetter' is unavailable}} expected-error {{setter for 'unavailableGetterAndSetter' is unavailable}}
 }
@@ -352,7 +352,7 @@ func testKeyPathAssignments_Class(_ someValue: ClassValue) {
   a[keyPath: \.[takesInOut(&x.unavailableGetter.a.b)]] = 0 // expected-error {{getter for 'unavailableGetter' is unavailable}}
   a[keyPath: \.[takesInOut(&x.unavailableGetter[0].b)]] = 0 // expected-error {{getter for 'unavailableGetter' is unavailable}}
 
-  x[keyPath: \.unavailableSetter] = someValue // expected-error {{setter for 'unavailableSetter' is unavailable}}
+  x[keyPath: \.unavailableSetter] = someValue // expected-error {{cannot assign through subscript: key path is read-only}}
   // FIXME: spurious unavailable setter error
   x[keyPath: \.unavailableSetter.a] = someValue.a // expected-error {{setter for 'unavailableSetter' is unavailable}}
   // FIXME: spurious unavailable setter error
@@ -362,7 +362,7 @@ func testKeyPathAssignments_Class(_ someValue: ClassValue) {
   a[keyPath: \.[takesInOut(&x.unavailableSetter.a.b)]] = 0
   a[keyPath: \.[takesInOut(&x.unavailableSetter[0].b)]] = 0
 
-  x[keyPath: \.unavailableGetterAndSetter] = someValue // expected-error {{setter for 'unavailableGetterAndSetter' is unavailable}}
+  x[keyPath: \.unavailableGetterAndSetter] = someValue // expected-error {{cannot assign through subscript: key path is read-only}}
   x[keyPath: \.unavailableGetterAndSetter.a] = someValue.a // expected-error {{setter for 'unavailableGetterAndSetter' is unavailable}}
   x[keyPath: \.unavailableGetterAndSetter[0]] = someValue.a // expected-error {{setter for 'unavailableGetterAndSetter' is unavailable}}
   // FIXME: spurious unavailable setter error

test/Availability/property_wrapper_availability.swift

@@ -186,3 +186,36 @@ func unavailableOnMacOSFunc(
   @WrappedValueUnavailableOnMacOS var unavailableWrappedValueLocal = S()
   @WrappedValueAvailable51 var wrappedValueAavailable51 = S()
 }
+
+@propertyWrapper
+struct Observable<Value> {
+  private var stored: Value
+
+  init(wrappedValue: Value) {
+    self.stored = wrappedValue
+  }
+
+  var wrappedValue: Value {
+    get { fatalError() }
+    set { fatalError() }
+  }
+
+  static subscript<EnclosingSelf>(
+      _enclosingInstance observed: EnclosingSelf,
+      wrapped wrappedKeyPath: ReferenceWritableKeyPath<EnclosingSelf, Value>,
+      storage storageKeyPath: ReferenceWritableKeyPath<EnclosingSelf, Self>
+    ) -> Value {
+    get { fatalError() }
+    set { fatalError() }
+  }
+}
+
+@available(macOS, unavailable)
+class UnavailableOnMacOSObserved {
+  @Observable var observedProperty = 17
+}
+
+@available(*, unavailable)
+class UniversallyUnavailableObserved {
+  @Observable var observedProperty = 17
+}

test/expr/unary/keypath/keypath-availability.swift

@@ -1,36 +1,128 @@
-// RUN: %target-swift-frontend -target %target-cpu-apple-macosx10.9 -typecheck -verify %s
+// RUN: %target-swift-frontend -typecheck -verify %s
 // REQUIRES: OS=macosx
 
 struct Butt {
-    var setter_conditionally_available: Int {
-        get { fatalError() }
+  var setter_conditionally_available: Int {
+    get { fatalError() }
 
-        @available(macOS 10.10, *)
-        set { fatalError() }
-    }
+    @available(macOS 99, *)
+    set { fatalError() }
+  }
+
+  var setter_unavailable_on_macos: Int {
+    get { fatalError() }
+
+    @available(macOS, unavailable)
+    set { fatalError() }
+  }
+
+  var setter_universally_unavailable: Int {
+    get { fatalError() }
+
+    @available(*, unavailable)
+    set { fatalError() }
+  }
+}
+
+@dynamicMemberLookup
+struct Lens<T> {
+  var obj: T
+
+  init(_ obj: T) {
+    self.obj = obj
+  }
+
+  subscript<U>(dynamicMember member: KeyPath<T, U>) -> Lens<U> {
+    get { return Lens<U>(obj[keyPath: member]) }
+  }
+
+  subscript<U>(dynamicMember member: WritableKeyPath<T, U>) -> Lens<U> {
+    get { return Lens<U>(obj[keyPath: member]) }
+    set { obj[keyPath: member] = newValue.obj }
+  }
 }
 
 func assertExactType<T>(of _: inout T, is _: T.Type) {}
 
 @available(macOS 10.9, *)
-public func unavailableSetterContext() {
-    var kp = \Butt.setter_conditionally_available
-    assertExactType(of: &kp, is: KeyPath<Butt, Int>.self)
+public func availableOnMacOS_10_9() {
+  var kp = \Butt.setter_conditionally_available
+  var lens = Lens(Butt())
+  assertExactType(of: &kp, is: KeyPath<Butt, Int>.self)
+  _ = lens.setter_conditionally_available
+  lens.setter_conditionally_available = Lens(1) // expected-error {{cannot assign to property: 'lens' is immutable}}
+}
+
+@available(macOS 99, *)
+public func availableOnMacOS_99() {
+  var kp = \Butt.setter_conditionally_available
+  var lens = Lens(Butt())
+  assertExactType(of: &kp, is: WritableKeyPath<Butt, Int>.self)
+  _ = lens.setter_conditionally_available
+  lens.setter_conditionally_available = Lens(1)
 }
-@available(macOS 10.10, *)
-public func availableSetterContext() {
+
+public func alwaysAvailableOnMacOS() {
+  var lens = Lens(Butt())
+
+  if #available(macOS 99, *) {
     var kp = \Butt.setter_conditionally_available
     assertExactType(of: &kp, is: WritableKeyPath<Butt, Int>.self)
+    _ = lens.setter_conditionally_available
+    // FIXME: [availability] setter_conditionally_available should be writable in this branch
+    lens.setter_conditionally_available = Lens(1) // expected-error {{cannot assign to property: 'lens' is immutable}}
+  } else {
+    var kp = \Butt.setter_conditionally_available
+    assertExactType(of: &kp, is: KeyPath<Butt, Int>.self)
+    _ = lens.setter_conditionally_available
+    lens.setter_conditionally_available = Lens(1) // expected-error {{cannot assign to property: 'lens' is immutable}}
+  }
+
+  var kp2 = \Butt.setter_unavailable_on_macos
+  assertExactType(of: &kp2, is: KeyPath<Butt, Int>.self)
+  _ = lens.setter_unavailable_on_macos
+  lens.setter_unavailable_on_macos = Lens(1) // expected-error {{cannot assign to property: 'lens' is immutable}}
+
+  var kp3 = \Butt.setter_universally_unavailable
+  assertExactType(of: &kp3, is: KeyPath<Butt, Int>.self)
+  _ = lens.setter_universally_unavailable
+  lens.setter_universally_unavailable = Lens(1) // expected-error {{cannot assign to property: 'lens' is immutable}}
 }
-@available(macOS 10.9, *)
-public func conditionalAvailableSetterContext() {
-    if #available(macOS 10.10, *) {
-        var kp = \Butt.setter_conditionally_available
-        assertExactType(of: &kp, is: WritableKeyPath<Butt, Int>.self)
-    } else {
-        var kp = \Butt.setter_conditionally_available
-        assertExactType(of: &kp, is: KeyPath<Butt, Int>.self)
-    }
+
+@available(macOS, unavailable)
+public func unvailableOnMacOS() {
+  var lens = Lens(Butt())
+  var kp = \Butt.setter_conditionally_available
+  assertExactType(of: &kp, is: WritableKeyPath<Butt, Int>.self)
+  _ = lens.setter_conditionally_available
+  lens.setter_conditionally_available = Lens(1)
+
+  var kp2 = \Butt.setter_unavailable_on_macos
+  assertExactType(of: &kp2, is: WritableKeyPath<Butt, Int>.self)
+  _ = lens.setter_unavailable_on_macos
+  lens.setter_unavailable_on_macos = Lens(1)
+
+  var kp3 = \Butt.setter_universally_unavailable
+  assertExactType(of: &kp3, is: KeyPath<Butt, Int>.self)
+  _ = lens.setter_universally_unavailable
+  lens.setter_universally_unavailable = Lens(1) // expected-error {{cannot assign to property: 'lens' is immutable}}
 }
 
-// FIXME: Check additional unavailability conditions
+@available(*, unavailable)
+public func universallyUnavailable() {
+  var lens = Lens(Butt())
+  var kp = \Butt.setter_conditionally_available
+  assertExactType(of: &kp, is: WritableKeyPath<Butt, Int>.self)
+  _ = lens.setter_conditionally_available
+  lens.setter_conditionally_available = Lens(1)
+
+  var kp2 = \Butt.setter_unavailable_on_macos
+  assertExactType(of: &kp2, is: WritableKeyPath<Butt, Int>.self)
+  _ = lens.setter_unavailable_on_macos
+  lens.setter_unavailable_on_macos = Lens(1)
+
+  var kp3 = \Butt.setter_universally_unavailable
+  assertExactType(of: &kp3, is: KeyPath<Butt, Int>.self)
+  _ = lens.setter_universally_unavailable
+  lens.setter_universally_unavailable = Lens(1)
+}