Preserve availability on ObjC subscript getters and setters (#17105)

This resolves SR-7398.

Author: ChristopherRogers

lib/ClangImporter/ImportDecl.cpp

@@ -6530,18 +6530,24 @@ SwiftDeclConverter::importSubscript(Decl *decl,
 
   // Determine the selector of the counterpart.
   FuncDecl *getter = nullptr, *setter = nullptr;
+  const clang::ObjCMethodDecl *getterObjCMethod = nullptr,
+                              *setterObjCMethod = nullptr;
   clang::Selector counterpartSelector;
   if (objcMethod->getSelector() == Impl.objectAtIndexedSubscript) {
     getter = cast<FuncDecl>(decl);
+    getterObjCMethod = objcMethod;
     counterpartSelector = Impl.setObjectAtIndexedSubscript;
   } else if (objcMethod->getSelector() == Impl.setObjectAtIndexedSubscript) {
     setter = cast<FuncDecl>(decl);
+    setterObjCMethod = objcMethod;
     counterpartSelector = Impl.objectAtIndexedSubscript;
   } else if (objcMethod->getSelector() == Impl.objectForKeyedSubscript) {
     getter = cast<FuncDecl>(decl);
+    getterObjCMethod = objcMethod;
     counterpartSelector = Impl.setObjectForKeyedSubscript;
   } else if (objcMethod->getSelector() == Impl.setObjectForKeyedSubscript) {
     setter = cast<FuncDecl>(decl);
+    setterObjCMethod = objcMethod;
     counterpartSelector = Impl.objectForKeyedSubscript;
   } else {
     llvm_unreachable("Unknown getter/setter selector");
@@ -6552,24 +6558,29 @@ SwiftDeclConverter::importSubscript(Decl *decl,
                           clang::ObjCMethodDecl::Optional);
 
   if (auto *counterpart = findCounterpart(counterpartSelector)) {
+    const clang::ObjCMethodDecl *counterpartMethod = nullptr;
+
     // If the counterpart to the method we're attempting to import has the
     // swift_private attribute, don't import as a subscript.
     if (auto importedFrom = counterpart->getClangDecl()) {
       if (importedFrom->hasAttr<clang::SwiftPrivateAttr>())
         return nullptr;
 
-      auto counterpartMethod = dyn_cast<clang::ObjCMethodDecl>(importedFrom);
+      counterpartMethod = cast<clang::ObjCMethodDecl>(importedFrom);
       if (optionalMethods)
         optionalMethods = (counterpartMethod->getImplementationControl() ==
                            clang::ObjCMethodDecl::Optional);
     }
 
     assert(!counterpart || !counterpart->isStatic());
 
-    if (getter)
+    if (getter) {
       setter = counterpart;
-    else
+      setterObjCMethod = counterpartMethod;
+    } else {
       getter = counterpart;
+      getterObjCMethod = counterpartMethod;
+    }
   }
 
   // Swift doesn't have write-only subscripting.
@@ -6657,6 +6668,7 @@ SwiftDeclConverter::importSubscript(Decl *decl,
       // Otherwise, just forget we had a setter.
       // FIXME: This feels very, very wrong.
       setter = nullptr;
+      setterObjCMethod = nullptr;
       setterIndex = nullptr;
     }
 
@@ -6717,9 +6729,14 @@ SwiftDeclConverter::importSubscript(Decl *decl,
         buildSubscriptSetterDecl(Impl, subscript, setter, elementTy,
                                  dc, setterIndex);
 
-  /// Record the subscript as an alternative declaration.
+  // Record the subscript as an alternative declaration.
   Impl.addAlternateDecl(associateWithSetter ? setter : getter, subscript);
 
+  // Import attributes for the accessors if there is a pair.
+  Impl.importAttributes(getterObjCMethod, getterThunk);
+  if (setterObjCMethod)
+    Impl.importAttributes(setterObjCMethod, setterThunk);
+
   subscript->setGenericEnvironment(dc->getGenericEnvironmentOfContext());
 
   subscript->setIsSetterMutating(false);
@@ -7374,6 +7391,11 @@ void ClangImporter::Implementation::importAttributes(
     Decl *MappedDecl,
     const clang::ObjCContainerDecl *NewContext)
 {
+  // Subscripts are special-cased since there isn't a 1:1 mapping
+  // from its accessor(s) to the subscript declaration.
+  if (isa<SubscriptDecl>(MappedDecl))
+    return;
+
   ASTContext &C = SwiftContext;
 
   if (auto maybeDefinition = getDefinitionForClangTypeDecl(ClangDecl))

test/ClangImporter/Inputs/custom-modules/AvailabilityExtras.h

@@ -97,6 +97,10 @@ typedef NS_ENUM(NSInteger, NSEnumAddedCasesIn2017) {
 @interface AccessorDeprecations: NSObject
 @property int fullyDeprecated __attribute__((deprecated));
 
+@property NSInteger fullyDeprecatedOnAccessors;
+- (NSInteger)fullyDeprecatedOnAccessors __attribute__((deprecated));
+- (void)setFullyDeprecatedOnAccessors:(NSInteger)fullyDeprecatedOnAccessors __attribute__((deprecated));
+
 @property int getterDeprecated;
 - (int)getterDeprecated __attribute__((deprecated));
 @property (class) int getterDeprecatedClass;
@@ -112,6 +116,10 @@ typedef NS_ENUM(NSInteger, NSEnumAddedCasesIn2017) {
 @interface UnavailableAccessors: NSObject
 @property NSInteger fullyUnavailable __attribute__((unavailable));
 
+@property NSInteger fullyUnavailableOnAccessors;
+- (NSInteger)fullyUnavailableOnAccessors __attribute__((unavailable));
+- (void)setFullyUnavailableOnAccessors:(NSInteger)fullyUnavailableOnAccessors __attribute__((unavailable));
+
 @property NSInteger getterUnavailable;
 - (NSInteger)getterUnavailable __attribute__((unavailable));
 @property (class) NSInteger getterUnavailableClass;
@@ -122,3 +130,42 @@ typedef NS_ENUM(NSInteger, NSEnumAddedCasesIn2017) {
 @property (class) NSInteger setterUnavailableClass;
 + (void)setSetterUnavailableClass:(NSInteger)setterUnavailable __attribute__((unavailable));
 @end
+
+
+@interface UnavailableSubscript: NSObject
+- (nonnull NSString *)objectAtIndexedSubscript:(NSInteger)i __attribute__((unavailable("bad subscript getter")));
+- (void)setObject:(nonnull NSString *)obj atIndexedSubscript:(NSInteger)i __attribute__((unavailable("bad subscript setter")));
+@end
+
+@interface UnavailableGetterSubscript: NSObject
+- (nonnull NSString *)objectAtIndexedSubscript:(NSInteger)i __attribute__((unavailable("bad subscript getter")));
+- (void)setObject:(nonnull NSString *)obj atIndexedSubscript:(NSInteger)i;
+@end
+
+@interface UnavailableSetterSubscript: NSObject
+- (nonnull NSString *)objectAtIndexedSubscript:(NSInteger)i;
+- (void)setObject:(nonnull NSString *)obj atIndexedSubscript:(NSInteger)i __attribute__((unavailable("bad subscript setter")));
+@end
+
+@interface UnavailableReadOnlySubscript: NSObject
+- (nonnull NSString *)objectAtIndexedSubscript:(NSInteger)i __attribute__((unavailable));
+@end
+
+@interface DeprecatedSubscript: NSObject
+- (nonnull NSString *)objectAtIndexedSubscript:(NSInteger)i __attribute__((deprecated("bad subscript getter")));
+- (void)setObject:(nonnull NSString *)obj atIndexedSubscript:(NSInteger)i __attribute__((deprecated("bad subscript setter")));
+@end
+
+@interface DeprecatedGetterSubscript: NSObject
+- (nonnull NSString *)objectAtIndexedSubscript:(NSInteger)i __attribute__((deprecated("bad subscript getter")));
+- (void)setObject:(nonnull NSString *)obj atIndexedSubscript:(NSInteger)i;
+@end
+
+@interface DeprecatedSetterSubscript: NSObject
+- (nonnull NSString *)objectAtIndexedSubscript:(NSInteger)i;
+- (void)setObject:(nonnull NSString *)obj atIndexedSubscript:(NSInteger)i __attribute__((deprecated("bad subscript setter")));
+@end
+
+@interface DeprecatedReadOnlySubscript: NSObject
+- (nonnull NSString *)objectAtIndexedSubscript:(NSInteger)i __attribute__((deprecated));
+@end

test/ClangImporter/availability.swift

@@ -25,11 +25,19 @@ func test_unavailable_func(_ x : NSObject) {
   NSDeallocateObject(x) // expected-error {{'NSDeallocateObject' is unavailable}}
 }
 
-func test_unavailable_accessors(_ obj: UnavailableAccessors) {
+func test_unavailable_accessors(_ obj: UnavailableAccessors,
+    _ sub: UnavailableSubscript,
+    _ subGetter: UnavailableGetterSubscript,
+    _ subSetter: UnavailableSetterSubscript,
+    _ subReadOnly: UnavailableReadOnlySubscript) {
   _ = obj.fullyUnavailable // expected-error {{'fullyUnavailable' is unavailable}}
   obj.fullyUnavailable = 0 // expected-error {{'fullyUnavailable' is unavailable}}
   obj.fullyUnavailable += 1 // expected-error {{'fullyUnavailable' is unavailable}}
 
+  _ = obj.fullyUnavailableOnAccessors // expected-error {{getter for 'fullyUnavailableOnAccessors' is unavailable}}
+  obj.fullyUnavailableOnAccessors = 0 // expected-error {{setter for 'fullyUnavailableOnAccessors' is unavailable}}
+  obj.fullyUnavailableOnAccessors += 1 // expected-error {{getter for 'fullyUnavailableOnAccessors' is unavailable}} expected-error {{setter for 'fullyUnavailableOnAccessors' is unavailable}}
+
   _ = obj.getterUnavailable // expected-error {{getter for 'getterUnavailable' is unavailable}}
   obj.getterUnavailable = 0
   obj.getterUnavailable += 1 // expected-error {{getter for 'getterUnavailable' is unavailable}}
@@ -45,15 +53,37 @@ func test_unavailable_accessors(_ obj: UnavailableAccessors) {
   _ = UnavailableAccessors.setterUnavailableClass
   UnavailableAccessors.setterUnavailableClass = 0 // expected-error {{setter for 'setterUnavailableClass' is unavailable}}
   UnavailableAccessors.setterUnavailableClass += 1 // expected-error {{setter for 'setterUnavailableClass' is unavailable}}
+
+  _ = sub[0] // expected-error {{getter for 'subscript' is unavailable: bad subscript getter}}
+  sub[0] = "" // expected-error {{setter for 'subscript' is unavailable: bad subscript setter}}
+  sub[0] += "" // expected-error {{getter for 'subscript' is unavailable: bad subscript getter}} expected-error {{setter for 'subscript' is unavailable: bad subscript setter}}
+
+  _ = subGetter[0] // expected-error {{getter for 'subscript' is unavailable: bad subscript getter}}
+  subGetter[0] = ""
+  subGetter[0] += "" // expected-error {{getter for 'subscript' is unavailable: bad subscript getter}}
+
+  _ = subSetter[0]
+  subSetter[0] = "" // expected-error {{setter for 'subscript' is unavailable: bad subscript setter}}
+  subSetter[0] += "" // expected-error {{setter for 'subscript' is unavailable: bad subscript setter}}
+
+  _ = subReadOnly[0] // expected-error {{getter for 'subscript' is unavailable}}
 }
 
-func test_deprecated(_ s:UnsafeMutablePointer<CChar>, _ obj: AccessorDeprecations) {
+func test_deprecated(_ s:UnsafeMutablePointer<CChar>, _ obj: AccessorDeprecations,
+    _ sub: DeprecatedSubscript,
+    _ subGetter: DeprecatedGetterSubscript,
+    _ subSetter: DeprecatedSetterSubscript,
+    _ subReadOnly: DeprecatedReadOnlySubscript) {
   _ = tmpnam(s) // expected-warning {{'tmpnam' is deprecated: Due to security concerns inherent in the design of tmpnam(3), it is highly recommended that you use mkstemp(3) instead.}}
 
   _ = obj.fullyDeprecated // expected-warning {{'fullyDeprecated' is deprecated}}
   obj.fullyDeprecated = 0 // expected-warning {{'fullyDeprecated' is deprecated}}
   obj.fullyDeprecated += 1 // expected-warning {{'fullyDeprecated' is deprecated}}
 
+  _ = obj.fullyDeprecatedOnAccessors // expected-warning {{getter for 'fullyDeprecatedOnAccessors' is deprecated}}
+  obj.fullyDeprecatedOnAccessors = 0 // expected-warning {{setter for 'fullyDeprecatedOnAccessors' is deprecated}}
+  obj.fullyDeprecatedOnAccessors += 1 // expected-warning {{getter for 'fullyDeprecatedOnAccessors' is deprecated}} expected-warning {{setter for 'fullyDeprecatedOnAccessors' is deprecated}}
+
   _ = obj.getterDeprecated // expected-warning {{getter for 'getterDeprecated' is deprecated}}
   obj.getterDeprecated = 0
   obj.getterDeprecated += 1 // expected-warning {{getter for 'getterDeprecated' is deprecated}}
@@ -69,6 +99,20 @@ func test_deprecated(_ s:UnsafeMutablePointer<CChar>, _ obj: AccessorDeprecation
   _ = AccessorDeprecations.setterDeprecatedClass
   AccessorDeprecations.setterDeprecatedClass = 0 // expected-warning {{setter for 'setterDeprecatedClass' is deprecated}}
   AccessorDeprecations.setterDeprecatedClass += 1 // expected-warning {{setter for 'setterDeprecatedClass' is deprecated}}
+
+  _ = sub[0] // expected-warning {{getter for 'subscript' is deprecated: bad subscript getter}}
+  sub[0] = "" // expected-warning {{setter for 'subscript' is deprecated: bad subscript setter}}
+  sub[0] += "" // expected-warning {{getter for 'subscript' is deprecated: bad subscript getter}} expected-warning {{setter for 'subscript' is deprecated: bad subscript setter}}
+
+  _ = subGetter[0] // expected-warning {{getter for 'subscript' is deprecated: bad subscript getter}}
+  subGetter[0] = ""
+  subGetter[0] += "" // expected-warning {{getter for 'subscript' is deprecated: bad subscript getter}}
+
+  _ = subSetter[0]
+  subSetter[0] = "" // expected-warning {{setter for 'subscript' is deprecated: bad subscript setter}}
+  subSetter[0] += "" // expected-warning {{setter for 'subscript' is deprecated: bad subscript setter}}
+
+  _ = subReadOnly[0] // expected-warning {{getter for 'subscript' is deprecated}}
 }
 
 func test_NSInvocation(_ x: NSInvocation,         // expected-error {{'NSInvocation' is unavailable}}