Skip to content

Preserve availability on ObjC subscript getters and setters #17105

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jun 27, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 26 additions & 4 deletions lib/ClangImporter/ImportDecl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6517,18 +6517,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");
Expand All @@ -6539,24 +6545,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.
Expand Down Expand Up @@ -6644,6 +6655,7 @@ SwiftDeclConverter::importSubscript(Decl *decl,
// Otherwise, just forget we had a setter.
// FIXME: This feels very, very wrong.
setter = nullptr;
setterObjCMethod = nullptr;
setterIndex = nullptr;
}

Expand Down Expand Up @@ -6704,9 +6716,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);
Expand Down Expand Up @@ -7347,6 +7364,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))
Expand Down
47 changes: 47 additions & 0 deletions test/ClangImporter/Inputs/custom-modules/AvailabilityExtras.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -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
48 changes: 46 additions & 2 deletions test/ClangImporter/availability.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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}}
Expand All @@ -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}}
Expand All @@ -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}}
Expand Down