From: Fariborz Jahanian Date: Fri, 1 Nov 2013 00:26:48 +0000 (+0000) Subject: ObjectiveC migrator. When inferring readwrite property, X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=257ddb2b4721e71749174ddaf03c6692df9dd346;p=clang ObjectiveC migrator. When inferring readwrite property, do not remove the setter if its availability differs from availability of the getter (which is now turned into a property). Otherwise, synthesized setter will inherit availability of the property (which is incorrect). // rdar://15300059 git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@193837 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/lib/ARCMigrate/ObjCMT.cpp b/lib/ARCMigrate/ObjCMT.cpp index f6cc19a737..f2f355c30f 100644 --- a/lib/ARCMigrate/ObjCMT.cpp +++ b/lib/ARCMigrate/ObjCMT.cpp @@ -278,7 +278,7 @@ static void rewriteToObjCProperty(const ObjCMethodDecl *Getter, const ObjCMethodDecl *Setter, const NSAPI &NS, edit::Commit &commit, unsigned LengthOfPrefix, - bool Atomic) { + bool Atomic, bool AvailabilityArgsMatch) { ASTContext &Context = NS.getASTContext(); bool LParenAdded = false; std::string PropertyString = "@property "; @@ -390,7 +390,7 @@ static void rewriteToObjCProperty(const ObjCMethodDecl *Getter, commit.replace(CharSourceRange::getCharRange(Getter->getLocStart(), EndGetterSelectorLoc), PropertyString); - if (Setter) { + if (Setter && AvailabilityArgsMatch) { SourceLocation EndLoc = Setter->getDeclaratorEndLoc(); // Get location past ';' EndLoc = EndLoc.getLocWithOffset(1); @@ -910,23 +910,49 @@ static bool TypeIsInnerPointer(QualType T) { return true; } -static bool AttributesMatch(const Decl *Decl1, const Decl *Decl2) { - if (Decl1->hasAttrs() != Decl2->hasAttrs()) - return false; - - if (!Decl1->hasAttrs()) +/// \brief Check whether the two versions match. +static bool versionsMatch(const VersionTuple &X, const VersionTuple &Y) { + return (X == Y); +} + +/// AvailabilityAttrsMatch - This routine checks that if comparing two +/// availability attributes, all their components match. It returns +/// true, if not dealing with availability or when all components of +/// availability attributes match. This routine is only called when +/// the attributes are of the same kind. +static bool AvailabilityAttrsMatch(Attr *At1, Attr *At2) { + const AvailabilityAttr *AA1 = dyn_cast(At1); + if (!AA1) return true; + const AvailabilityAttr *AA2 = dyn_cast(At2); - const AttrVec &Attrs1 = Decl1->getAttrs(); - const AttrVec &Attrs2 = Decl2->getAttrs(); + VersionTuple Introduced1 = AA1->getIntroduced(); + VersionTuple Deprecated1 = AA1->getDeprecated(); + VersionTuple Obsoleted1 = AA1->getObsoleted(); + bool IsUnavailable1 = AA1->getUnavailable(); + VersionTuple Introduced2 = AA2->getIntroduced(); + VersionTuple Deprecated2 = AA2->getDeprecated(); + VersionTuple Obsoleted2 = AA2->getObsoleted(); + bool IsUnavailable2 = AA2->getUnavailable(); + return (versionsMatch(Introduced1, Introduced2) && + versionsMatch(Deprecated1, Deprecated2) && + versionsMatch(Obsoleted1, Obsoleted2) && + IsUnavailable1 == IsUnavailable2); + +} + +static bool MatchTwoAttributeLists(const AttrVec &Attrs1, const AttrVec &Attrs2, + bool &AvailabilityArgsMatch) { // This list is very small, so this need not be optimized. for (unsigned i = 0, e = Attrs1.size(); i != e; i++) { bool match = false; for (unsigned j = 0, f = Attrs2.size(); j != f; j++) { - // Matching attribute kind only. We are not getting into - // details of the attributes. For all practical purposes + // Matching attribute kind only. Except for Availabilty attributes, + // we are not getting into details of the attributes. For all practical purposes // this is sufficient. if (Attrs1[i]->getKind() == Attrs2[j]->getKind()) { + if (AvailabilityArgsMatch) + AvailabilityArgsMatch = AvailabilityAttrsMatch(Attrs1[i], Attrs2[j]); match = true; break; } @@ -937,6 +963,28 @@ static bool AttributesMatch(const Decl *Decl1, const Decl *Decl2) { return true; } +/// AttributesMatch - This routine checks list of attributes for two +/// decls. It returns false, if there is a mismatch in kind of +/// attributes seen in the decls. It returns true if the two decls +/// have list of same kind of attributes. Furthermore, when there +/// are availability attributes in the two decls, it sets the +/// AvailabilityArgsMatch to false if availability attributes have +/// different versions, etc. +static bool AttributesMatch(const Decl *Decl1, const Decl *Decl2, + bool &AvailabilityArgsMatch) { + if (!Decl1->hasAttrs() || !Decl2->hasAttrs()) { + AvailabilityArgsMatch = (Decl1->hasAttrs() == Decl2->hasAttrs()); + return true; + } + AvailabilityArgsMatch = true; + const AttrVec &Attrs1 = Decl1->getAttrs(); + const AttrVec &Attrs2 = Decl2->getAttrs(); + bool match = MatchTwoAttributeLists(Attrs1, Attrs2, AvailabilityArgsMatch); + if (match && (Attrs2.size() > Attrs1.size())) + return MatchTwoAttributeLists(Attrs2, Attrs1, AvailabilityArgsMatch); + return match; +} + static bool IsValidIdentifier(ASTContext &Ctx, const char *Name) { if (!isIdentifierHead(Name[0])) @@ -1001,8 +1049,9 @@ bool ObjCMigrateASTConsumer::migrateProperty(ASTContext &Ctx, if (SetterMethod) { if ((ASTMigrateActions & FrontendOptions::ObjCMT_ReadwriteProperty) == 0) return false; + bool AvailabilityArgsMatch; if (SetterMethod->isDeprecated() || - !AttributesMatch(Method, SetterMethod)) + !AttributesMatch(Method, SetterMethod, AvailabilityArgsMatch)) return false; // Is this a valid setter, matching the target getter? @@ -1017,7 +1066,8 @@ bool ObjCMigrateASTConsumer::migrateProperty(ASTContext &Ctx, rewriteToObjCProperty(Method, SetterMethod, *NSAPIObj, commit, LengthOfPrefix, (ASTMigrateActions & - FrontendOptions::ObjCMT_AtomicProperty) != 0); + FrontendOptions::ObjCMT_AtomicProperty) != 0, + AvailabilityArgsMatch); Editor->commit(commit); return true; } @@ -1028,7 +1078,8 @@ bool ObjCMigrateASTConsumer::migrateProperty(ASTContext &Ctx, rewriteToObjCProperty(Method, 0 /*SetterMethod*/, *NSAPIObj, commit, LengthOfPrefix, (ASTMigrateActions & - FrontendOptions::ObjCMT_AtomicProperty) != 0); + FrontendOptions::ObjCMT_AtomicProperty) != 0, + /*AvailabilityArgsMatch*/false); Editor->commit(commit); return true; } diff --git a/test/ARCMT/objcmt-atomic-property.m.result b/test/ARCMT/objcmt-atomic-property.m.result index 668c8218d5..761a208a16 100644 --- a/test/ARCMT/objcmt-atomic-property.m.result +++ b/test/ARCMT/objcmt-atomic-property.m.result @@ -25,12 +25,12 @@ typedef char BOOL; @property (retain) NSString *StrongProp; -- (NSString *) UnavailProp __attribute__((unavailable)); +@property (retain) NSString *UnavailProp __attribute__((unavailable)); - (void) setUnavailProp : (NSString *)Val; @property (retain) NSString *UnavailProp1 __attribute__((unavailable)); -- (NSString *) UnavailProp2; +@property (retain) NSString *UnavailProp2; - (void) setUnavailProp2 : (NSString *)Val __attribute__((unavailable)); @property (copy) NSDictionary *undoAction; @@ -161,19 +161,19 @@ DEPRECATED @interface NSURL // Do not infer a property. -- (NSURL *)appStoreReceiptURL NS_AVAILABLE; +@property (retain) NSURL *appStoreReceiptURL NS_AVAILABLE; - (void) setAppStoreReceiptURL : (NSURL *)object; @property (retain) NSURL *appStoreReceiptURLX NS_AVAILABLE; // Do not infer a property. -- (NSURL *)appStoreReceiptURLY ; +@property (retain) NSURL *appStoreReceiptURLY ; - (void) setAppStoreReceiptURLY : (NSURL *)object NS_AVAILABLE; @property (readonly) id OkToInfer NS_AVAILABLE; // Do not infer a property. -- (NSURL *)appStoreReceiptURLZ ; +@property (retain) NSURL *appStoreReceiptURLZ ; - (void) setAppStoreReceiptURLZ : (NSURL *)object NS_AVAILABLE; // Do not infer a property. diff --git a/test/ARCMT/objcmt-property-availability.m b/test/ARCMT/objcmt-property-availability.m new file mode 100644 index 0000000000..d499221a74 --- /dev/null +++ b/test/ARCMT/objcmt-property-availability.m @@ -0,0 +1,46 @@ +// RUN: rm -rf %t +// RUN: %clang_cc1 -objcmt-migrate-readwrite-property -objcmt-migrate-readonly-property -mt-migrate-directory %t %s -x objective-c -fobjc-runtime-has-weak -fobjc-arc -triple x86_64-apple-darwin11 +// RUN: c-arcmt-test -mt-migrate-directory %t | arcmt-test -verify-transformed-files %s.result +// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -fsyntax-only -x objective-c -fobjc-runtime-has-weak -fobjc-arc %s.result +// rdar://15300059 + + +#define __NSi_7_0 introduced=7.0 +#define __NSi_6_0 introduced=6.0 + +#define CF_AVAILABLE(_mac, _ios) __attribute__((availability(ios,__NSi_##_ios))) +#define CF_AVAILABLE_MAC(_mac) __attribute__((availability(macosx,__NSi_##_mac))) +#define CF_AVAILABLE_IOS(_ios) __attribute__((availability(macosx,unavailable))) + +#define NS_AVAILABLE(_mac, _ios) CF_AVAILABLE(_mac, _ios) +#define NS_AVAILABLE_MAC(_mac) CF_AVAILABLE_MAC(_mac) +#define NS_AVAILABLE_IOS(_ios) CF_AVAILABLE_IOS(_ios) + +#define UNAVAILABLE __attribute__((unavailable("not available in automatic reference counting mode"))) + +@interface MKMapItem +- (MKMapItem *)source NS_AVAILABLE(10_9, 6_0); +- (void)setSource:(MKMapItem *)source NS_AVAILABLE(10_9, 7_0); + +- (void)setDest:(MKMapItem *)source NS_AVAILABLE(10_9, 6_0); +- (MKMapItem *)dest NS_AVAILABLE(10_9, 6_0); + +- (MKMapItem *)final; +- (void)setFinal:(MKMapItem *)source; + +- (MKMapItem *)total NS_AVAILABLE(10_9, 6_0); +- (void)setTotal:(MKMapItem *)source; + +- (MKMapItem *)comp NS_AVAILABLE(10_9, 6_0); +- (void)setComp:(MKMapItem *)source UNAVAILABLE; + +- (MKMapItem *)tally UNAVAILABLE NS_AVAILABLE(10_9, 6_0); +- (void)setTally:(MKMapItem *)source UNAVAILABLE NS_AVAILABLE(10_9, 6_0); + +- (MKMapItem *)itally NS_AVAILABLE(10_9, 6_0); +- (void)setItally:(MKMapItem *)source UNAVAILABLE NS_AVAILABLE(10_9, 6_0); + +- (MKMapItem *)normal UNAVAILABLE; +- (void)setNormal:(MKMapItem *)source UNAVAILABLE NS_AVAILABLE(10_9, 6_0); +@end + diff --git a/test/ARCMT/objcmt-property-availability.m.result b/test/ARCMT/objcmt-property-availability.m.result new file mode 100644 index 0000000000..681f9a99bf --- /dev/null +++ b/test/ARCMT/objcmt-property-availability.m.result @@ -0,0 +1,43 @@ +// RUN: rm -rf %t +// RUN: %clang_cc1 -objcmt-migrate-readwrite-property -objcmt-migrate-readonly-property -mt-migrate-directory %t %s -x objective-c -fobjc-runtime-has-weak -fobjc-arc -triple x86_64-apple-darwin11 +// RUN: c-arcmt-test -mt-migrate-directory %t | arcmt-test -verify-transformed-files %s.result +// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -fsyntax-only -x objective-c -fobjc-runtime-has-weak -fobjc-arc %s.result +// rdar://15300059 + + +#define __NSi_7_0 introduced=7.0 +#define __NSi_6_0 introduced=6.0 + +#define CF_AVAILABLE(_mac, _ios) __attribute__((availability(ios,__NSi_##_ios))) +#define CF_AVAILABLE_MAC(_mac) __attribute__((availability(macosx,__NSi_##_mac))) +#define CF_AVAILABLE_IOS(_ios) __attribute__((availability(macosx,unavailable))) + +#define NS_AVAILABLE(_mac, _ios) CF_AVAILABLE(_mac, _ios) +#define NS_AVAILABLE_MAC(_mac) CF_AVAILABLE_MAC(_mac) +#define NS_AVAILABLE_IOS(_ios) CF_AVAILABLE_IOS(_ios) + +#define UNAVAILABLE __attribute__((unavailable("not available in automatic reference counting mode"))) + +@interface MKMapItem +@property (nonatomic, retain) MKMapItem *source NS_AVAILABLE(10_9, 6_0); +- (void)setSource:(MKMapItem *)source NS_AVAILABLE(10_9, 7_0); + +@property (nonatomic, retain) MKMapItem *dest NS_AVAILABLE(10_9, 6_0); + +@property (nonatomic, retain) MKMapItem *final; + +@property (nonatomic, retain) MKMapItem *total NS_AVAILABLE(10_9, 6_0); +- (void)setTotal:(MKMapItem *)source; + +- (MKMapItem *)comp NS_AVAILABLE(10_9, 6_0); +- (void)setComp:(MKMapItem *)source UNAVAILABLE; + +@property (nonatomic, retain) MKMapItem *tally UNAVAILABLE NS_AVAILABLE(10_9, 6_0); + +- (MKMapItem *)itally NS_AVAILABLE(10_9, 6_0); +- (void)setItally:(MKMapItem *)source UNAVAILABLE NS_AVAILABLE(10_9, 6_0); + +- (MKMapItem *)normal UNAVAILABLE; +- (void)setNormal:(MKMapItem *)source UNAVAILABLE NS_AVAILABLE(10_9, 6_0); +@end + diff --git a/test/ARCMT/objcmt-property.m.result b/test/ARCMT/objcmt-property.m.result index bdb135b42a..2d19ddb680 100644 --- a/test/ARCMT/objcmt-property.m.result +++ b/test/ARCMT/objcmt-property.m.result @@ -25,12 +25,12 @@ typedef char BOOL; @property (nonatomic, retain) NSString *StrongProp; -- (NSString *) UnavailProp __attribute__((unavailable)); +@property (nonatomic, retain) NSString *UnavailProp __attribute__((unavailable)); - (void) setUnavailProp : (NSString *)Val; @property (nonatomic, retain) NSString *UnavailProp1 __attribute__((unavailable)); -- (NSString *) UnavailProp2; +@property (nonatomic, retain) NSString *UnavailProp2; - (void) setUnavailProp2 : (NSString *)Val __attribute__((unavailable)); @property (nonatomic, copy) NSDictionary *undoAction; @@ -161,19 +161,19 @@ DEPRECATED @interface NSURL // Do not infer a property. -- (NSURL *)appStoreReceiptURL NS_AVAILABLE; +@property (nonatomic, retain) NSURL *appStoreReceiptURL NS_AVAILABLE; - (void) setAppStoreReceiptURL : (NSURL *)object; @property (nonatomic, retain) NSURL *appStoreReceiptURLX NS_AVAILABLE; // Do not infer a property. -- (NSURL *)appStoreReceiptURLY ; +@property (nonatomic, retain) NSURL *appStoreReceiptURLY ; - (void) setAppStoreReceiptURLY : (NSURL *)object NS_AVAILABLE; @property (nonatomic, readonly) id OkToInfer NS_AVAILABLE; // Do not infer a property. -- (NSURL *)appStoreReceiptURLZ ; +@property (nonatomic, retain) NSURL *appStoreReceiptURLZ ; - (void) setAppStoreReceiptURLZ : (NSURL *)object NS_AVAILABLE; // Do not infer a property.