[cfe-commits] r157347 - in /cfe/trunk: lib/ARCMigrate/TransProperties.cpp lib/ARCMigrate/TransRetainReleaseDealloc.cpp lib/ARCMigrate/Transforms.cpp lib/ARCMigrate/Transforms.h test/ARCMT/Common.h test/ARCMT/autoreleases.m test/ARCMT/autoreleases.m.result test/ARCMT/checking.m test/ARCMT/dispatch.m test/ARCMT/dispatch.m.result
Argyrios Kyrtzidis
akyrtzi at gmail.com
Wed May 23 14:50:04 PDT 2012
Author: akirtzidis
Date: Wed May 23 16:50:04 2012
New Revision: 157347
URL: http://llvm.org/viewvc/llvm-project?rev=157347&view=rev
Log:
[arcmt] Remove an unused -autorelease, without failing with error, for this
idiom that is used commonly in setters:
[backingValue autorelease];
backingValue = [newValue retain]; // in general a +1 assign
rdar://9914061
Modified:
cfe/trunk/lib/ARCMigrate/TransProperties.cpp
cfe/trunk/lib/ARCMigrate/TransRetainReleaseDealloc.cpp
cfe/trunk/lib/ARCMigrate/Transforms.cpp
cfe/trunk/lib/ARCMigrate/Transforms.h
cfe/trunk/test/ARCMT/Common.h
cfe/trunk/test/ARCMT/autoreleases.m
cfe/trunk/test/ARCMT/autoreleases.m.result
cfe/trunk/test/ARCMT/checking.m
cfe/trunk/test/ARCMT/dispatch.m
cfe/trunk/test/ARCMT/dispatch.m.result
Modified: cfe/trunk/lib/ARCMigrate/TransProperties.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/ARCMigrate/TransProperties.cpp?rev=157347&r1=157346&r2=157347&view=diff
==============================================================================
--- cfe/trunk/lib/ARCMigrate/TransProperties.cpp (original)
+++ cfe/trunk/lib/ARCMigrate/TransProperties.cpp Wed May 23 16:50:04 2012
@@ -309,17 +309,8 @@
if (RE->getDecl() != Ivar)
return true;
- if (ObjCMessageExpr *
- ME = dyn_cast<ObjCMessageExpr>(E->getRHS()->IgnoreParenCasts()))
- if (ME->getMethodFamily() == OMF_retain)
+ if (isPlusOneAssign(E))
return false;
-
- ImplicitCastExpr *implCE = dyn_cast<ImplicitCastExpr>(E->getRHS());
- while (implCE && implCE->getCastKind() == CK_BitCast)
- implCE = dyn_cast<ImplicitCastExpr>(implCE->getSubExpr());
-
- if (implCE && implCE->getCastKind() == CK_ARCConsumeObject)
- return false;
}
return true;
Modified: cfe/trunk/lib/ARCMigrate/TransRetainReleaseDealloc.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/ARCMigrate/TransRetainReleaseDealloc.cpp?rev=157347&r1=157346&r2=157347&view=diff
==============================================================================
--- cfe/trunk/lib/ARCMigrate/TransRetainReleaseDealloc.cpp (original)
+++ cfe/trunk/lib/ARCMigrate/TransRetainReleaseDealloc.cpp Wed May 23 16:50:04 2012
@@ -64,14 +64,16 @@
return true;
case OMF_autorelease:
if (isRemovable(E)) {
- // An unused autorelease is badness. If we remove it the receiver
- // will likely die immediately while previously it was kept alive
- // by the autorelease pool. This is bad practice in general, leave it
- // and emit an error to force the user to restructure his code.
- Pass.TA.reportError("it is not safe to remove an unused 'autorelease' "
- "message; its receiver may be destroyed immediately",
- E->getLocStart(), E->getSourceRange());
- return true;
+ if (!isCommonUnusedAutorelease(E)) {
+ // An unused autorelease is badness. If we remove it the receiver
+ // will likely die immediately while previously it was kept alive
+ // by the autorelease pool. This is bad practice in general, leave it
+ // and emit an error to force the user to restructure his code.
+ Pass.TA.reportError("it is not safe to remove an unused 'autorelease' "
+ "message; its receiver may be destroyed immediately",
+ E->getLocStart(), E->getSourceRange());
+ return true;
+ }
}
// Pass through.
case OMF_retain:
@@ -156,6 +158,80 @@
}
private:
+ /// \brief Checks for idioms where an unused -autorelease is common.
+ ///
+ /// Currently only returns true for this idiom which is common in property
+ /// setters:
+ ///
+ /// [backingValue autorelease];
+ /// backingValue = [newValue retain]; // in general a +1 assign
+ ///
+ bool isCommonUnusedAutorelease(ObjCMessageExpr *E) {
+ Expr *Rec = E->getInstanceReceiver();
+ if (!Rec)
+ return false;
+
+ Decl *RefD = getReferencedDecl(Rec);
+ if (!RefD)
+ return false;
+
+ Stmt *OuterS = E, *InnerS;
+ do {
+ InnerS = OuterS;
+ OuterS = StmtMap->getParent(InnerS);
+ }
+ while (OuterS && (isa<ParenExpr>(OuterS) ||
+ isa<CastExpr>(OuterS) ||
+ isa<ExprWithCleanups>(OuterS)));
+
+ if (!OuterS)
+ return false;
+
+ // Find next statement after the -autorelease.
+
+ Stmt::child_iterator currChildS = OuterS->child_begin();
+ Stmt::child_iterator childE = OuterS->child_end();
+ for (; currChildS != childE; ++currChildS) {
+ if (*currChildS == InnerS)
+ break;
+ }
+ if (currChildS == childE)
+ return false;
+ ++currChildS;
+ if (currChildS == childE)
+ return false;
+
+ Stmt *nextStmt = *currChildS;
+ if (!nextStmt)
+ return false;
+ nextStmt = nextStmt->IgnoreImplicit();
+
+ // Check for "RefD = [+1 retained object];".
+
+ if (BinaryOperator *Bop = dyn_cast<BinaryOperator>(nextStmt)) {
+ if (RefD != getReferencedDecl(Bop->getLHS()))
+ return false;
+ if (isPlusOneAssign(Bop))
+ return true;
+ }
+ return false;
+ }
+
+ Decl *getReferencedDecl(Expr *E) {
+ if (!E)
+ return 0;
+
+ E = E->IgnoreParenCasts();
+ if (DeclRefExpr *DRE = dyn_cast<DeclRefExpr>(E))
+ return DRE->getDecl();
+ if (MemberExpr *ME = dyn_cast<MemberExpr>(E))
+ return ME->getMemberDecl();
+ if (ObjCIvarRefExpr *IRE = dyn_cast<ObjCIvarRefExpr>(E))
+ return IRE->getDecl();
+
+ return 0;
+ }
+
/// \brief Check if the retain/release is due to a GCD/XPC macro that are
/// defined as:
///
Modified: cfe/trunk/lib/ARCMigrate/Transforms.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/ARCMigrate/Transforms.cpp?rev=157347&r1=157346&r2=157347&view=diff
==============================================================================
--- cfe/trunk/lib/ARCMigrate/Transforms.cpp (original)
+++ cfe/trunk/lib/ARCMigrate/Transforms.cpp Wed May 23 16:50:04 2012
@@ -14,6 +14,7 @@
#include "clang/AST/StmtVisitor.h"
#include "clang/Lex/Lexer.h"
#include "clang/Basic/SourceManager.h"
+#include "clang/Analysis/DomainSpecific/CocoaConventions.h"
#include "llvm/ADT/StringSwitch.h"
#include "llvm/ADT/DenseSet.h"
#include <map>
@@ -56,6 +57,47 @@
return true;
}
+bool trans::isPlusOneAssign(const BinaryOperator *E) {
+ if (E->getOpcode() != BO_Assign)
+ return false;
+
+ if (const ObjCMessageExpr *
+ ME = dyn_cast<ObjCMessageExpr>(E->getRHS()->IgnoreParenCasts()))
+ if (ME->getMethodFamily() == OMF_retain)
+ return true;
+
+ if (const CallExpr *
+ callE = dyn_cast<CallExpr>(E->getRHS()->IgnoreParenCasts())) {
+ if (const FunctionDecl *FD = callE->getDirectCallee()) {
+ if (FD->getAttr<CFReturnsRetainedAttr>())
+ return true;
+
+ if (FD->isGlobal() &&
+ FD->getIdentifier() &&
+ FD->getParent()->isTranslationUnit() &&
+ FD->getLinkage() == ExternalLinkage &&
+ ento::cocoa::isRefType(callE->getType(), "CF",
+ FD->getIdentifier()->getName())) {
+ StringRef fname = FD->getIdentifier()->getName();
+ if (fname.endswith("Retain") ||
+ fname.find("Create") != StringRef::npos ||
+ fname.find("Copy") != StringRef::npos) {
+ return true;
+ }
+ }
+ }
+ }
+
+ const ImplicitCastExpr *implCE = dyn_cast<ImplicitCastExpr>(E->getRHS());
+ while (implCE && implCE->getCastKind() == CK_BitCast)
+ implCE = dyn_cast<ImplicitCastExpr>(implCE->getSubExpr());
+
+ if (implCE && implCE->getCastKind() == CK_ARCConsumeObject)
+ return true;
+
+ return false;
+}
+
/// \brief 'Loc' is the end of a statement range. This returns the location
/// immediately after the semicolon following the statement.
/// If no semicolon is found or the location is inside a macro, the returned
Modified: cfe/trunk/lib/ARCMigrate/Transforms.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/ARCMigrate/Transforms.h?rev=157347&r1=157346&r2=157347&view=diff
==============================================================================
--- cfe/trunk/lib/ARCMigrate/Transforms.h (original)
+++ cfe/trunk/lib/ARCMigrate/Transforms.h Wed May 23 16:50:04 2012
@@ -154,6 +154,8 @@
bool canApplyWeak(ASTContext &Ctx, QualType type,
bool AllowOnUnknownClass = false);
+bool isPlusOneAssign(const BinaryOperator *E);
+
/// \brief 'Loc' is the end of a statement range. This returns the location
/// immediately after the semicolon following the statement.
/// If no semicolon is found or the location is inside a macro, the returned
Modified: cfe/trunk/test/ARCMT/Common.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/ARCMT/Common.h?rev=157347&r1=157346&r2=157347&view=diff
==============================================================================
--- cfe/trunk/test/ARCMT/Common.h (original)
+++ cfe/trunk/test/ARCMT/Common.h Wed May 23 16:50:04 2012
@@ -68,3 +68,14 @@
extern __attribute__((ns_returns_retained)) id objc_retainedObject(objc_objectptr_t __attribute__((cf_consumed)) pointer);
extern __attribute__((ns_returns_not_retained)) id objc_unretainedObject(objc_objectptr_t pointer);
extern objc_objectptr_t objc_unretainedPointer(id object);
+
+#define dispatch_retain(object) ({ dispatch_object_t _o = (object); _dispatch_object_validate(_o); (void)[_o retain]; })
+#define dispatch_release(object) ({ dispatch_object_t _o = (object); _dispatch_object_validate(_o); [_o release]; })
+#define xpc_retain(object) ({ xpc_object_t _o = (object); _xpc_object_validate(_o); [_o retain]; })
+#define xpc_release(object) ({ xpc_object_t _o = (object); _xpc_object_validate(_o); [_o release]; })
+
+typedef id dispatch_object_t;
+typedef id xpc_object_t;
+
+void _dispatch_object_validate(dispatch_object_t object);
+void _xpc_object_validate(xpc_object_t object);
Modified: cfe/trunk/test/ARCMT/autoreleases.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/ARCMT/autoreleases.m?rev=157347&r1=157346&r2=157347&view=diff
==============================================================================
--- cfe/trunk/test/ARCMT/autoreleases.m (original)
+++ cfe/trunk/test/ARCMT/autoreleases.m Wed May 23 16:50:04 2012
@@ -3,29 +3,21 @@
// RUN: diff %t %s.result
// DISABLE: mingw32
-typedef unsigned char BOOL;
+#include "Common.h"
- at interface NSObject {
- id isa;
-}
-+new;
-+alloc;
--init;
--autorelease;
- at end
-
- at interface NSAutoreleasePool : NSObject
-- drain;
- at end
-
@interface A : NSObject {
@package
id object;
}
@end
- at interface B : NSObject
+ at interface B : NSObject {
+ id _prop;
+ xpc_object_t _xpc_prop;
+}
- (BOOL)containsSelf:(A*)a;
+ at property (retain) id prop;
+ at property (retain) xpc_object_t xpc_prop;
@end
@implementation A
@@ -35,6 +27,26 @@
- (BOOL)containsSelf:(A*)a {
return a->object == self;
}
+
+-(id) prop {
+ return _prop;
+}
+-(void) setProp:(id) newVal {
+ [_prop autorelease];
+ _prop = [newVal retain];
+}
+-(void) setProp2:(CFTypeRef) newVal {
+ [_prop autorelease];
+ _prop = (id)CFRetain(newVal);
+}
+
+-(id) xpc_prop {
+ return _xpc_prop;
+}
+-(void) setXpc_prop:(xpc_object_t) newVal {
+ [_xpc_prop autorelease];
+ _xpc_prop = xpc_retain(newVal);
+}
@end
void NSLog(id, ...);
@@ -47,3 +59,8 @@
[pool drain];
return 0;
}
+
+void test(A *prevVal, A *newVal) {
+ [prevVal autorelease];
+ prevVal = [newVal retain];
+}
Modified: cfe/trunk/test/ARCMT/autoreleases.m.result
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/ARCMT/autoreleases.m.result?rev=157347&r1=157346&r2=157347&view=diff
==============================================================================
--- cfe/trunk/test/ARCMT/autoreleases.m.result (original)
+++ cfe/trunk/test/ARCMT/autoreleases.m.result Wed May 23 16:50:04 2012
@@ -3,29 +3,21 @@
// RUN: diff %t %s.result
// DISABLE: mingw32
-typedef unsigned char BOOL;
+#include "Common.h"
- at interface NSObject {
- id isa;
-}
-+new;
-+alloc;
--init;
--autorelease;
- at end
-
- at interface NSAutoreleasePool : NSObject
-- drain;
- at end
-
@interface A : NSObject {
@package
id object;
}
@end
- at interface B : NSObject
+ at interface B : NSObject {
+ id _prop;
+ xpc_object_t _xpc_prop;
+}
- (BOOL)containsSelf:(A*)a;
+ at property (strong) id prop;
+ at property (strong) xpc_object_t xpc_prop;
@end
@implementation A
@@ -35,6 +27,23 @@
- (BOOL)containsSelf:(A*)a {
return a->object == self;
}
+
+-(id) prop {
+ return _prop;
+}
+-(void) setProp:(id) newVal {
+ _prop = newVal;
+}
+-(void) setProp2:(CFTypeRef) newVal {
+ _prop = (__bridge_transfer id)CFRetain(newVal);
+}
+
+-(id) xpc_prop {
+ return _xpc_prop;
+}
+-(void) setXpc_prop:(xpc_object_t) newVal {
+ _xpc_prop = newVal;
+}
@end
void NSLog(id, ...);
@@ -47,3 +56,7 @@
}
return 0;
}
+
+void test(A *prevVal, A *newVal) {
+ prevVal = newVal;
+}
Modified: cfe/trunk/test/ARCMT/checking.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/ARCMT/checking.m?rev=157347&r1=157346&r2=157347&view=diff
==============================================================================
--- cfe/trunk/test/ARCMT/checking.m (original)
+++ cfe/trunk/test/ARCMT/checking.m Wed May 23 16:50:04 2012
@@ -91,6 +91,9 @@
[a release];
[a autorelease]; // expected-error {{it is not safe to remove an unused 'autorelease' message; its receiver may be destroyed immediately}} \
// expected-error {{ARC forbids explicit message send}}
+ [a autorelease]; // expected-error {{it is not safe to remove an unused 'autorelease' message; its receiver may be destroyed immediately}} \
+ // expected-error {{ARC forbids explicit message send}}
+ a = 0;
CFStringRef cfstr;
NSString *str = (NSString *)cfstr; // expected-error {{cast of C pointer type 'CFStringRef' (aka 'const struct __CFString *') to Objective-C pointer type 'NSString *' requires a bridged cast}} \
Modified: cfe/trunk/test/ARCMT/dispatch.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/ARCMT/dispatch.m?rev=157347&r1=157346&r2=157347&view=diff
==============================================================================
--- cfe/trunk/test/ARCMT/dispatch.m (original)
+++ cfe/trunk/test/ARCMT/dispatch.m Wed May 23 16:50:04 2012
@@ -4,17 +4,6 @@
#include "Common.h"
-#define dispatch_retain(object) ({ dispatch_object_t _o = (object); _dispatch_object_validate(_o); (void)[_o retain]; })
-#define dispatch_release(object) ({ dispatch_object_t _o = (object); _dispatch_object_validate(_o); [_o release]; })
-#define xpc_retain(object) ({ xpc_object_t _o = (object); _xpc_object_validate(_o); [_o retain]; })
-#define xpc_release(object) ({ xpc_object_t _o = (object); _xpc_object_validate(_o); [_o release]; })
-
-typedef id dispatch_object_t;
-typedef id xpc_object_t;
-
-void _dispatch_object_validate(dispatch_object_t object);
-void _xpc_object_validate(xpc_object_t object);
-
dispatch_object_t getme(void);
void func(dispatch_object_t o) {
Modified: cfe/trunk/test/ARCMT/dispatch.m.result
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/ARCMT/dispatch.m.result?rev=157347&r1=157346&r2=157347&view=diff
==============================================================================
--- cfe/trunk/test/ARCMT/dispatch.m.result (original)
+++ cfe/trunk/test/ARCMT/dispatch.m.result Wed May 23 16:50:04 2012
@@ -4,17 +4,6 @@
#include "Common.h"
-#define dispatch_retain(object) ({ dispatch_object_t _o = (object); _dispatch_object_validate(_o); (void)[_o retain]; })
-#define dispatch_release(object) ({ dispatch_object_t _o = (object); _dispatch_object_validate(_o); [_o release]; })
-#define xpc_retain(object) ({ xpc_object_t _o = (object); _xpc_object_validate(_o); [_o retain]; })
-#define xpc_release(object) ({ xpc_object_t _o = (object); _xpc_object_validate(_o); [_o release]; })
-
-typedef id dispatch_object_t;
-typedef id xpc_object_t;
-
-void _dispatch_object_validate(dispatch_object_t object);
-void _xpc_object_validate(xpc_object_t object);
-
dispatch_object_t getme(void);
void func(dispatch_object_t o) {
More information about the cfe-commits
mailing list