-Warc-repeated-use-of-weak: check ivars and variables as well.
Like properties, loading from a weak ivar twice in the same function can
give you inconsistent results if the object is deallocated between the
two loads. It is safer to assign to a strong local variable and use that.
Second half of <rdar://problem/12280249>.
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@164855 91177308-0d34-0410-b5e6-96231b3b80d8
diff --git a/lib/Sema/AnalysisBasedWarnings.cpp b/lib/Sema/AnalysisBasedWarnings.cpp
index 86e9dc2..bc25c0a 100644
--- a/lib/Sema/AnalysisBasedWarnings.cpp
+++ b/lib/Sema/AnalysisBasedWarnings.cpp
@@ -957,19 +957,42 @@
const WeakObjectProfileTy &Key = I->second->first;
const WeakUseVector &Uses = I->second->second;
- // For complicated expressions like self.foo.bar, it's hard to keep track
- // of whether 'self.foo' is the same between two cases. We can only be
- // 100% sure of a repeated use if the "base" part of the key is a variable,
- // rather than, say, another property.
+ // For complicated expressions like 'a.b.c' and 'x.b.c', WeakObjectProfileTy
+ // may not contain enough information to determine that these are different
+ // properties. We can only be 100% sure of a repeated use in certain cases,
+ // and we adjust the diagnostic kind accordingly so that the less certain
+ // case can be turned off if it is too noisy.
unsigned DiagKind;
if (Key.isExactProfile())
DiagKind = diag::warn_arc_repeated_use_of_weak;
else
DiagKind = diag::warn_arc_possible_repeated_use_of_weak;
+ // Classify the weak object being accessed for better warning text.
+ // This enum should stay in sync with the cases in
+ // warn_arc_repeated_use_of_weak and warn_arc_possible_repeated_use_of_weak.
+ enum {
+ Variable,
+ Property,
+ ImplicitProperty,
+ Ivar
+ } ObjectKind;
+
+ const NamedDecl *D = Key.getProperty();
+ if (isa<VarDecl>(D))
+ ObjectKind = Variable;
+ else if (isa<ObjCPropertyDecl>(D))
+ ObjectKind = Property;
+ else if (isa<ObjCMethodDecl>(D))
+ ObjectKind = ImplicitProperty;
+ else if (isa<ObjCIvarDecl>(D))
+ ObjectKind = Ivar;
+ else
+ llvm_unreachable("Unexpected weak object kind!");
+
// Show the first time the object was read.
S.Diag(FirstRead->getLocStart(), DiagKind)
- << FunctionKind
+ << ObjectKind << D << FunctionKind
<< FirstRead->getSourceRange();
// Print all the other accesses as notes.
diff --git a/lib/Sema/Sema.cpp b/lib/Sema/Sema.cpp
index 84cdb2b..8555562 100644
--- a/lib/Sema/Sema.cpp
+++ b/lib/Sema/Sema.cpp
@@ -82,65 +82,77 @@
return M->getSelfDecl() == Param;
}
+FunctionScopeInfo::WeakObjectProfileTy::BaseInfoTy
+FunctionScopeInfo::WeakObjectProfileTy::getBaseInfo(const Expr *E) {
+ E = E->IgnoreParenCasts();
+
+ const NamedDecl *D = 0;
+ bool IsExact = false;
+
+ switch (E->getStmtClass()) {
+ case Stmt::DeclRefExprClass:
+ D = cast<DeclRefExpr>(E)->getDecl();
+ IsExact = isa<VarDecl>(D);
+ break;
+ case Stmt::MemberExprClass: {
+ const MemberExpr *ME = cast<MemberExpr>(E);
+ D = ME->getMemberDecl();
+ IsExact = isa<CXXThisExpr>(ME->getBase()->IgnoreParenImpCasts());
+ break;
+ }
+ case Stmt::ObjCIvarRefExprClass: {
+ const ObjCIvarRefExpr *IE = cast<ObjCIvarRefExpr>(E);
+ D = IE->getDecl();
+ IsExact = isSelfExpr(IE->getBase());
+ break;
+ }
+ case Stmt::PseudoObjectExprClass: {
+ const PseudoObjectExpr *POE = cast<PseudoObjectExpr>(E);
+ const ObjCPropertyRefExpr *BaseProp =
+ dyn_cast<ObjCPropertyRefExpr>(POE->getSyntacticForm());
+ if (BaseProp) {
+ D = getBestPropertyDecl(BaseProp);
+
+ const Expr *DoubleBase = BaseProp->getBase();
+ if (const OpaqueValueExpr *OVE = dyn_cast<OpaqueValueExpr>(DoubleBase))
+ DoubleBase = OVE->getSourceExpr();
+
+ IsExact = isSelfExpr(DoubleBase);
+ }
+ break;
+ }
+ default:
+ break;
+ }
+
+ return BaseInfoTy(D, IsExact);
+}
+
+
FunctionScopeInfo::WeakObjectProfileTy::WeakObjectProfileTy(
const ObjCPropertyRefExpr *PropE)
- : Base(0, false), Property(getBestPropertyDecl(PropE)) {
+ : Base(0, true), Property(getBestPropertyDecl(PropE)) {
if (PropE->isObjectReceiver()) {
const OpaqueValueExpr *OVE = cast<OpaqueValueExpr>(PropE->getBase());
- const Expr *E = OVE->getSourceExpr()->IgnoreParenCasts();
-
- switch (E->getStmtClass()) {
- case Stmt::DeclRefExprClass:
- Base.setPointer(cast<DeclRefExpr>(E)->getDecl());
- Base.setInt(isa<VarDecl>(Base.getPointer()));
- break;
- case Stmt::MemberExprClass: {
- const MemberExpr *ME = cast<MemberExpr>(E);
- Base.setPointer(ME->getMemberDecl());
- Base.setInt(isa<CXXThisExpr>(ME->getBase()->IgnoreParenImpCasts()));
- break;
- }
- case Stmt::ObjCIvarRefExprClass: {
- const ObjCIvarRefExpr *IE = cast<ObjCIvarRefExpr>(E);
- Base.setPointer(IE->getDecl());
- if (isSelfExpr(IE->getBase()))
- Base.setInt(true);
- break;
- }
- case Stmt::PseudoObjectExprClass: {
- const PseudoObjectExpr *POE = cast<PseudoObjectExpr>(E);
- const ObjCPropertyRefExpr *BaseProp =
- dyn_cast<ObjCPropertyRefExpr>(POE->getSyntacticForm());
- if (BaseProp) {
- Base.setPointer(getBestPropertyDecl(BaseProp));
-
- const Expr *DoubleBase = BaseProp->getBase();
- if (const OpaqueValueExpr *OVE = dyn_cast<OpaqueValueExpr>(DoubleBase))
- DoubleBase = OVE->getSourceExpr();
-
- if (isSelfExpr(DoubleBase))
- Base.setInt(true);
- }
- break;
- }
- default:
- break;
- }
+ const Expr *E = OVE->getSourceExpr();
+ Base = getBaseInfo(E);
} else if (PropE->isClassReceiver()) {
Base.setPointer(PropE->getClassReceiver());
- Base.setInt(true);
} else {
assert(PropE->isSuperReceiver());
- Base.setInt(true);
}
}
-void FunctionScopeInfo::recordUseOfWeak(const ObjCPropertyRefExpr *RefExpr) {
- assert(RefExpr);
- WeakUseVector &Uses =
- WeakObjectUses[FunctionScopeInfo::WeakObjectProfileTy(RefExpr)];
- Uses.push_back(WeakUseTy(RefExpr, RefExpr->isMessagingGetter()));
+FunctionScopeInfo::WeakObjectProfileTy::WeakObjectProfileTy(
+ const DeclRefExpr *DRE)
+ : Base(0, true), Property(DRE->getDecl()) {
+ assert(isa<VarDecl>(Property));
+}
+
+FunctionScopeInfo::WeakObjectProfileTy::WeakObjectProfileTy(
+ const ObjCIvarRefExpr *IvarE)
+ : Base(getBaseInfo(IvarE->getBase())), Property(IvarE->getDecl()) {
}
void FunctionScopeInfo::markSafeWeakUse(const Expr *E) {
@@ -164,22 +176,27 @@
return;
}
- if (const ObjCPropertyRefExpr *RefExpr = dyn_cast<ObjCPropertyRefExpr>(E)) {
- // Has this property been seen as a weak property?
- FunctionScopeInfo::WeakObjectUseMap::iterator Uses =
- WeakObjectUses.find(FunctionScopeInfo::WeakObjectProfileTy(RefExpr));
- if (Uses == WeakObjectUses.end())
- return;
-
- // Has there been a read from the property using this Expr?
- FunctionScopeInfo::WeakUseVector::reverse_iterator ThisUse =
- std::find(Uses->second.rbegin(), Uses->second.rend(), WeakUseTy(E, true));
- if (ThisUse == Uses->second.rend())
- return;
-
- ThisUse->markSafe();
+ // Has this weak object been seen before?
+ FunctionScopeInfo::WeakObjectUseMap::iterator Uses;
+ if (const ObjCPropertyRefExpr *RefExpr = dyn_cast<ObjCPropertyRefExpr>(E))
+ Uses = WeakObjectUses.find(FunctionScopeInfo::WeakObjectProfileTy(RefExpr));
+ else if (const ObjCIvarRefExpr *IvarE = dyn_cast<ObjCIvarRefExpr>(E))
+ Uses = WeakObjectUses.find(FunctionScopeInfo::WeakObjectProfileTy(IvarE));
+ else if (const DeclRefExpr *DRE = dyn_cast<DeclRefExpr>(E))
+ Uses = WeakObjectUses.find(FunctionScopeInfo::WeakObjectProfileTy(DRE));
+ else
return;
- }
+
+ if (Uses == WeakObjectUses.end())
+ return;
+
+ // Has there been a read from the object using this Expr?
+ FunctionScopeInfo::WeakUseVector::reverse_iterator ThisUse =
+ std::find(Uses->second.rbegin(), Uses->second.rend(), WeakUseTy(E, true));
+ if (ThisUse == Uses->second.rend())
+ return;
+
+ ThisUse->markSafe();
}
BlockScopeInfo::~BlockScopeInfo() { }
diff --git a/lib/Sema/SemaChecking.cpp b/lib/Sema/SemaChecking.cpp
index 7951a71..ebb6cd1 100644
--- a/lib/Sema/SemaChecking.cpp
+++ b/lib/Sema/SemaChecking.cpp
@@ -5639,9 +5639,19 @@
if (LHSType.isNull())
LHSType = LHS->getType();
+
+ Qualifiers::ObjCLifetime LT = LHSType.getObjCLifetime();
+
+ if (LT == Qualifiers::OCL_Weak) {
+ DiagnosticsEngine::Level Level =
+ Diags.getDiagnosticLevel(diag::warn_arc_repeated_use_of_weak, Loc);
+ if (Level != DiagnosticsEngine::Ignored)
+ getCurFunction()->markSafeWeakUse(LHS);
+ }
+
if (checkUnsafeAssigns(Loc, LHSType, RHS))
return;
- Qualifiers::ObjCLifetime LT = LHSType.getObjCLifetime();
+
// FIXME. Check for other life times.
if (LT != Qualifiers::OCL_None)
return;
diff --git a/lib/Sema/SemaExpr.cpp b/lib/Sema/SemaExpr.cpp
index 597e4d6..0570dd3 100644
--- a/lib/Sema/SemaExpr.cpp
+++ b/lib/Sema/SemaExpr.cpp
@@ -1425,6 +1425,15 @@
MarkDeclRefReferenced(E);
+ if (getLangOpts().ObjCARCWeak && isa<VarDecl>(D) &&
+ Ty.getObjCLifetime() == Qualifiers::OCL_Weak) {
+ DiagnosticsEngine::Level Level =
+ Diags.getDiagnosticLevel(diag::warn_arc_repeated_use_of_weak,
+ E->getLocStart());
+ if (Level != DiagnosticsEngine::Ignored)
+ getCurFunction()->recordUseOfWeak(E);
+ }
+
// Just in case we're building an illegal pointer-to-member.
FieldDecl *FD = dyn_cast<FieldDecl>(D);
if (FD && FD->isBitField())
@@ -1986,9 +1995,22 @@
ObjCMethodFamily MF = CurMethod->getMethodFamily();
if (MF != OMF_init && MF != OMF_dealloc && MF != OMF_finalize)
Diag(Loc, diag::warn_direct_ivar_access) << IV->getDeclName();
- return Owned(new (Context)
- ObjCIvarRefExpr(IV, IV->getType(), Loc,
- SelfExpr.take(), true, true));
+
+ ObjCIvarRefExpr *Result = new (Context) ObjCIvarRefExpr(IV, IV->getType(),
+ Loc,
+ SelfExpr.take(),
+ true, true);
+
+ if (getLangOpts().ObjCAutoRefCount) {
+ if (IV->getType().getObjCLifetime() == Qualifiers::OCL_Weak) {
+ DiagnosticsEngine::Level Level =
+ Diags.getDiagnosticLevel(diag::warn_arc_repeated_use_of_weak, Loc);
+ if (Level != DiagnosticsEngine::Ignored)
+ getCurFunction()->recordUseOfWeak(Result);
+ }
+ }
+
+ return Owned(Result);
}
} else if (CurMethod->isInstanceMethod()) {
// We should warn if a local variable hides an ivar.
diff --git a/lib/Sema/SemaExprMember.cpp b/lib/Sema/SemaExprMember.cpp
index ec787d6..ff58069 100644
--- a/lib/Sema/SemaExprMember.cpp
+++ b/lib/Sema/SemaExprMember.cpp
@@ -13,6 +13,7 @@
#include "clang/Sema/SemaInternal.h"
#include "clang/Sema/Lookup.h"
#include "clang/Sema/Scope.h"
+#include "clang/Sema/ScopeInfo.h"
#include "clang/AST/DeclCXX.h"
#include "clang/AST/DeclObjC.h"
#include "clang/AST/DeclTemplate.h"
@@ -1272,9 +1273,23 @@
if (warn)
Diag(MemberLoc, diag::warn_direct_ivar_access) << IV->getDeclName();
}
- return Owned(new (Context) ObjCIvarRefExpr(IV, IV->getType(),
- MemberLoc, BaseExpr.take(),
- IsArrow));
+
+ ObjCIvarRefExpr *Result = new (Context) ObjCIvarRefExpr(IV, IV->getType(),
+ MemberLoc,
+ BaseExpr.take(),
+ IsArrow);
+
+ if (getLangOpts().ObjCAutoRefCount) {
+ if (IV->getType().getObjCLifetime() == Qualifiers::OCL_Weak) {
+ DiagnosticsEngine::Level Level =
+ Diags.getDiagnosticLevel(diag::warn_arc_repeated_use_of_weak,
+ MemberLoc);
+ if (Level != DiagnosticsEngine::Ignored)
+ getCurFunction()->recordUseOfWeak(Result);
+ }
+ }
+
+ return Owned(Result);
}
// Objective-C property access.
diff --git a/lib/Sema/SemaPseudoObject.cpp b/lib/Sema/SemaPseudoObject.cpp
index b07a59b..501fa11 100644
--- a/lib/Sema/SemaPseudoObject.cpp
+++ b/lib/Sema/SemaPseudoObject.cpp
@@ -845,7 +845,8 @@
S.Diags.getDiagnosticLevel(diag::warn_arc_repeated_use_of_weak,
SyntacticForm->getLocStart());
if (Level != DiagnosticsEngine::Ignored)
- S.getCurFunction()->recordUseOfWeak(SyntacticRefExpr);
+ S.getCurFunction()->recordUseOfWeak(SyntacticRefExpr,
+ SyntacticRefExpr->isMessagingGetter());
}
return PseudoOpBuilder::complete(SyntacticForm);