Skip to content

Commit

Permalink
Refactor: Simplify boolean conditional return statements in lib/ARCMi…
Browse files Browse the repository at this point in the history
…grate

Patch by Richard Thomson! (+a couple of modifications to address comments)

Differential revision: http://reviews.llvm.org/D10009

llvm-svn: 252261
  • Loading branch information
alexfh committed Nov 6, 2015
1 parent 3394642 commit ad98885
Show file tree
Hide file tree
Showing 5 changed files with 18 additions and 47 deletions.
32 changes: 10 additions & 22 deletions clang/lib/ARCMigrate/ObjCMT.cpp
Expand Up @@ -214,25 +214,15 @@ namespace {
// FIXME. This duplicates one in RewriteObjCFoundationAPI.cpp
bool subscriptOperatorNeedsParens(const Expr *FullExpr) {
const Expr* Expr = FullExpr->IgnoreImpCasts();
if (isa<ArraySubscriptExpr>(Expr) ||
isa<CallExpr>(Expr) ||
isa<DeclRefExpr>(Expr) ||
isa<CXXNamedCastExpr>(Expr) ||
isa<CXXConstructExpr>(Expr) ||
isa<CXXThisExpr>(Expr) ||
isa<CXXTypeidExpr>(Expr) ||
isa<CXXUnresolvedConstructExpr>(Expr) ||
isa<ObjCMessageExpr>(Expr) ||
isa<ObjCPropertyRefExpr>(Expr) ||
isa<ObjCProtocolExpr>(Expr) ||
isa<MemberExpr>(Expr) ||
isa<ObjCIvarRefExpr>(Expr) ||
isa<ParenExpr>(FullExpr) ||
isa<ParenListExpr>(Expr) ||
isa<SizeOfPackExpr>(Expr))
return false;

return true;
return !(isa<ArraySubscriptExpr>(Expr) || isa<CallExpr>(Expr) ||
isa<DeclRefExpr>(Expr) || isa<CXXNamedCastExpr>(Expr) ||
isa<CXXConstructExpr>(Expr) || isa<CXXThisExpr>(Expr) ||
isa<CXXTypeidExpr>(Expr) ||
isa<CXXUnresolvedConstructExpr>(Expr) ||
isa<ObjCMessageExpr>(Expr) || isa<ObjCPropertyRefExpr>(Expr) ||
isa<ObjCProtocolExpr>(Expr) || isa<MemberExpr>(Expr) ||
isa<ObjCIvarRefExpr>(Expr) || isa<ParenExpr>(FullExpr) ||
isa<ParenListExpr>(Expr) || isa<SizeOfPackExpr>(Expr));
}

/// \brief - Rewrite message expression for Objective-C setter and getters into
Expand Down Expand Up @@ -665,9 +655,7 @@ ClassImplementsAllMethodsAndProperties(ASTContext &Ctx,
return false;
}
}
if (HasAtleastOneRequiredProperty || HasAtleastOneRequiredMethod)
return true;
return false;
return HasAtleastOneRequiredProperty || HasAtleastOneRequiredMethod;
}

static bool rewriteToObjCInterfaceDecl(const ObjCInterfaceDecl *IDecl,
Expand Down
4 changes: 1 addition & 3 deletions clang/lib/ARCMigrate/TransEmptyStatementsAndDealloc.cpp
Expand Up @@ -104,9 +104,7 @@ class EmptyChecker : public StmtVisitor<EmptyChecker, bool> {
return false;
if (!S->getThen() || !Visit(S->getThen()))
return false;
if (S->getElse() && !Visit(S->getElse()))
return false;
return true;
return !S->getElse() || Visit(S->getElse());
}
bool VisitWhileStmt(WhileStmt *S) {
if (S->getConditionVariable())
Expand Down
4 changes: 1 addition & 3 deletions clang/lib/ARCMigrate/TransGCAttrs.cpp
Expand Up @@ -152,9 +152,7 @@ class GCAttrsCollector : public RecursiveASTVisitor<GCAttrsCollector> {
return ID->getImplementation() != nullptr;
if (ObjCCategoryDecl *CD = dyn_cast<ObjCCategoryDecl>(ContD))
return CD->getImplementation() != nullptr;
if (isa<ObjCImplDecl>(ContD))
return true;
return false;
return isa<ObjCImplDecl>(ContD);
}
return false;
}
Expand Down
20 changes: 5 additions & 15 deletions clang/lib/ARCMigrate/TransRetainReleaseDealloc.cpp
Expand Up @@ -150,11 +150,8 @@ class RetainReleaseDeallocRemover :
return true;
}

if (!hasSideEffects(rec, Pass.Ctx)) {
if (tryRemoving(RecContainer))
return true;
}
Pass.TA.replace(RecContainer->getSourceRange(), RecRange);
if (hasSideEffects(rec, Pass.Ctx) || !tryRemoving(RecContainer))
Pass.TA.replace(RecContainer->getSourceRange(), RecRange);

return true;
}
Expand All @@ -174,11 +171,8 @@ class RetainReleaseDeallocRemover :
/// return var;
///
bool isCommonUnusedAutorelease(ObjCMessageExpr *E) {
if (isPlusOneAssignBeforeOrAfterAutorelease(E))
return true;
if (isReturnedAfterAutorelease(E))
return true;
return false;
return isPlusOneAssignBeforeOrAfterAutorelease(E) ||
isReturnedAfterAutorelease(E);
}

bool isReturnedAfterAutorelease(ObjCMessageExpr *E) {
Expand Down Expand Up @@ -225,11 +219,7 @@ class RetainReleaseDeallocRemover :
// Check for "RefD = [+1 retained object];".

if (BinaryOperator *Bop = dyn_cast<BinaryOperator>(S)) {
if (RefD != getReferencedDecl(Bop->getLHS()))
return false;
if (isPlusOneAssign(Bop))
return true;
return false;
return (RefD == getReferencedDecl(Bop->getLHS())) && isPlusOneAssign(Bop);
}

if (DeclStmt *DS = dyn_cast<DeclStmt>(S)) {
Expand Down
5 changes: 1 addition & 4 deletions clang/lib/ARCMigrate/Transforms.cpp
Expand Up @@ -113,10 +113,7 @@ bool trans::isPlusOne(const Expr *E) {
while (implCE && implCE->getCastKind() == CK_BitCast)
implCE = dyn_cast<ImplicitCastExpr>(implCE->getSubExpr());

if (implCE && implCE->getCastKind() == CK_ARCConsumeObject)
return true;

return false;
return implCE && implCE->getCastKind() == CK_ARCConsumeObject;
}

/// \brief 'Loc' is the end of a statement range. This returns the location
Expand Down

0 comments on commit ad98885

Please sign in to comment.