[cfe-commits] r164854 - in /cfe/trunk: include/clang/Basic/DiagnosticGroups.td include/clang/Basic/DiagnosticSemaKinds.td include/clang/Sema/ScopeInfo.h lib/Sema/AnalysisBasedWarnings.cpp lib/Sema/Sema.cpp lib/Sema/SemaDecl.cpp lib/Sema/SemaExpr.cpp lib/Sema/SemaPseudoObject.cpp test/SemaObjC/arc-repeated-weak.mm
Jordan Rose
jordan_rose at apple.com
Fri Sep 28 15:21:30 PDT 2012
Author: jrose
Date: Fri Sep 28 17:21:30 2012
New Revision: 164854
URL: http://llvm.org/viewvc/llvm-project?rev=164854&view=rev
Log:
Add a warning (off by default) for repeated use of the same weak property.
The motivating example:
if (self.weakProp)
use(self.weakProp);
As with any non-atomic test-then-use, it is possible a weak property to be
non-nil at the 'if', but be deallocated by the time it is used. The correct
way to write this example is as follows:
id tmp = self.weakProp;
if (tmp)
use(tmp);
The warning is controlled by -Warc-repeated-use-of-receiver, and uses the
property name and base to determine if the same property on the same object
is being accessed multiple times. In cases where the base is more
complicated than just a single Decl (e.g. 'foo.bar.weakProp'), it picks a
Decl for some degree of uniquing and reports the problem under a subflag,
-Warc-maybe-repeated-use-of-receiver. This gives a way to tune the
aggressiveness of the warning for a particular project.
The warning is not on by default because it is not flow-sensitive and thus
may have a higher-than-acceptable rate of false positives, though it is
less noisy than -Wreceiver-is-weak. On the other hand, it will not warn
about some cases that may be legitimate issues that -Wreceiver-is-weak
will catch, and it does not attempt to reason about methods returning weak
values.
Even though this is not a real "analysis-based" check I've put the bug
emission code in AnalysisBasedWarnings for two reasons: (1) to run on
every kind of code body (function, method, block, or lambda), and (2) to
suggest that it may be enhanced by flow-sensitive analysis in the future.
The second (smaller) half of this work is to extend it to weak locals
and weak ivars. This should use most of the same infrastructure.
Part of <rdar://problem/12280249>
Added:
cfe/trunk/test/SemaObjC/arc-repeated-weak.mm
Modified:
cfe/trunk/include/clang/Basic/DiagnosticGroups.td
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/include/clang/Sema/ScopeInfo.h
cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp
cfe/trunk/lib/Sema/Sema.cpp
cfe/trunk/lib/Sema/SemaDecl.cpp
cfe/trunk/lib/Sema/SemaExpr.cpp
cfe/trunk/lib/Sema/SemaPseudoObject.cpp
Modified: cfe/trunk/include/clang/Basic/DiagnosticGroups.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticGroups.td?rev=164854&r1=164853&r2=164854&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/DiagnosticGroups.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticGroups.td Fri Sep 28 17:21:30 2012
@@ -282,6 +282,9 @@
[ARCUnsafeRetainedAssign,
ARCRetainCycles,
ARCNonPodMemAccess]>;
+def ARCRepeatedUseOfWeakMaybe : DiagGroup<"arc-maybe-repeated-use-of-weak">;
+def ARCRepeatedUseOfWeak : DiagGroup<"arc-repeated-use-of-weak",
+ [ARCRepeatedUseOfWeakMaybe]>;
def Selector : DiagGroup<"selector">;
def Protocol : DiagGroup<"protocol">;
def SuperSubClassMismatch : DiagGroup<"super-class-method-mismatch">;
Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=164854&r1=164853&r2=164854&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Fri Sep 28 17:21:30 2012
@@ -709,6 +709,18 @@
"weak %select{receiver|property|implicit property}0 may be "
"unpredictably null in ARC mode">,
InGroup<DiagGroup<"receiver-is-weak">>, DefaultIgnore;
+def warn_arc_repeated_use_of_weak : Warning <
+ "weak property is accessed multiple times in this "
+ "%select{function|method|block|lambda}0 but may be unpredictably set to nil; "
+ "assign to a strong variable to keep the object alive">,
+ InGroup<ARCRepeatedUseOfWeak>, DefaultIgnore;
+def warn_arc_possible_repeated_use_of_weak : Warning <
+ "weak property may be accessed multiple times in this "
+ "%select{function|method|block|lambda}0 but may be unpredictably set to nil; "
+ "assign to a strong variable to keep the object alive">,
+ InGroup<ARCRepeatedUseOfWeakMaybe>, DefaultIgnore;
+def note_arc_weak_also_accessed_here : Note<
+ "also accessed here">;
def err_incomplete_synthesized_property : Error<
"cannot synthesize property %0 with incomplete type %1">;
Modified: cfe/trunk/include/clang/Sema/ScopeInfo.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/ScopeInfo.h?rev=164854&r1=164853&r2=164854&view=diff
==============================================================================
--- cfe/trunk/include/clang/Sema/ScopeInfo.h (original)
+++ cfe/trunk/include/clang/Sema/ScopeInfo.h Fri Sep 28 17:21:30 2012
@@ -21,6 +21,7 @@
namespace clang {
+class Decl;
class BlockDecl;
class CXXMethodDecl;
class IdentifierInfo;
@@ -29,6 +30,7 @@
class Scope;
class SwitchStmt;
class VarDecl;
+class ObjCPropertyRefExpr;
namespace sema {
@@ -113,6 +115,125 @@
/// prior to being emitted.
SmallVector<PossiblyUnreachableDiag, 4> PossiblyUnreachableDiags;
+public:
+ /// Represents a simple identification of a weak object.
+ ///
+ /// Part of the implementation of -Wrepeated-use-of-weak.
+ ///
+ /// This is used to determine if two weak accesses refer to the same object.
+ /// Here are some examples of how various accesses are "profiled":
+ ///
+ /// Access Expression | "Base" Decl | "Property" Decl
+ /// :---------------: | :-----------------: | :------------------------------:
+ /// self.property | self (VarDecl) | property (ObjCPropertyDecl)
+ /// self.implicitProp | self (VarDecl) | -implicitProp (ObjCMethodDecl)
+ /// self->ivar.prop | ivar (ObjCIvarDecl) | prop (ObjCPropertyDecl)
+ /// cxxObj.obj.prop | obj (FieldDecl) | prop (ObjCPropertyDecl)
+ /// [self foo].prop | 0 (unknown) | prop (ObjCPropertyDecl)
+ /// self.prop1.prop2 | prop1 (ObjCPropertyDecl) | prop2 (ObjCPropertyDecl)
+ /// MyClass.prop | MyClass (ObjCInterfaceDecl) | -prop (ObjCMethodDecl)
+ ///
+ /// Objects are identified with only two Decls to make it reasonably fast to
+ /// compare them.
+ class WeakObjectProfileTy {
+ /// The base object decl, as described in the class documentation.
+ ///
+ /// The extra flag is "true" if the Base and Property are enough to uniquely
+ /// identify the object in memory.
+ ///
+ /// \sa isExactProfile()
+ typedef llvm::PointerIntPair<const NamedDecl *, 1, bool> BaseInfoTy;
+ BaseInfoTy Base;
+
+ /// The "property" decl, as described in the class documentation.
+ ///
+ /// Note that this may not actually be an ObjCPropertyDecl, e.g. in the
+ /// case of "implicit" properties (regular methods accessed via dot syntax).
+ const NamedDecl *Property;
+
+ // For use in DenseMap.
+ friend struct llvm::DenseMapInfo<WeakObjectProfileTy>;
+ inline WeakObjectProfileTy();
+ static inline WeakObjectProfileTy getSentinel();
+
+ public:
+ WeakObjectProfileTy(const ObjCPropertyRefExpr *RE);
+
+ const NamedDecl *getProperty() const { return Property; }
+
+ /// Returns true if the object base specifies a known object in memory,
+ /// rather than, say, an instance variable or property of another object.
+ ///
+ /// Note that this ignores the effects of aliasing; that is, \c foo.bar is
+ /// considered an exact profile if \c foo is a local variable, even if
+ /// another variable \c foo2 refers to the same object as \c foo.
+ ///
+ /// For increased precision, accesses with base variables that are
+ /// properties or ivars of 'self' (e.g. self.prop1.prop2) are considered to
+ /// be exact, though this is not true for arbitrary variables
+ /// (foo.prop1.prop2).
+ bool isExactProfile() const {
+ return Base.getInt();
+ }
+
+ bool operator==(const WeakObjectProfileTy &Other) const {
+ return Base == Other.Base && Property == Other.Property;
+ }
+ };
+
+ /// Represents a single use of a weak object.
+ ///
+ /// Stores both the expression and whether the access is potentially unsafe
+ /// (i.e. it could potentially be warned about).
+ ///
+ /// Part of the implementation of -Wrepeated-use-of-weak.
+ class WeakUseTy {
+ llvm::PointerIntPair<const Expr *, 1, bool> Rep;
+ public:
+ WeakUseTy(const Expr *Use, bool IsRead) : Rep(Use, IsRead) {}
+
+ const Expr *getUseExpr() const { return Rep.getPointer(); }
+ bool isUnsafe() const { return Rep.getInt(); }
+ void markSafe() { Rep.setInt(false); }
+
+ bool operator==(const WeakUseTy &Other) const {
+ return Rep == Other.Rep;
+ }
+ };
+
+ /// Used to collect uses of a particular weak object in a function body.
+ ///
+ /// Part of the implementation of -Wrepeated-use-of-weak.
+ typedef SmallVector<WeakUseTy, 4> WeakUseVector;
+
+ /// Used to collect all uses of weak objects in a function body.
+ ///
+ /// Part of the implementation of -Wrepeated-use-of-weak.
+ typedef llvm::SmallDenseMap<WeakObjectProfileTy, WeakUseVector, 8>
+ WeakObjectUseMap;
+
+private:
+ /// Used to collect all uses of weak objects in this function body.
+ ///
+ /// Part of the implementation of -Wrepeated-use-of-weak.
+ WeakObjectUseMap WeakObjectUses;
+
+public:
+ /// Record that a weak property was accessed.
+ ///
+ /// Part of the implementation of -Wrepeated-use-of-weak.
+ void recordUseOfWeak(const ObjCPropertyRefExpr *PropE);
+
+ /// Record that a given expression is a "safe" access of a weak object (e.g.
+ /// assigning it to a strong variable.)
+ ///
+ /// Part of the implementation of -Wrepeated-use-of-weak.
+ void markSafeWeakUse(const Expr *E);
+
+ const WeakObjectUseMap &getWeakObjectUses() const {
+ return WeakObjectUses;
+ }
+
void setHasBranchIntoScope() {
HasBranchIntoScope = true;
}
@@ -387,7 +508,39 @@
};
+FunctionScopeInfo::WeakObjectProfileTy::WeakObjectProfileTy()
+ : Base(0, false), Property(0) {}
+
+FunctionScopeInfo::WeakObjectProfileTy
+FunctionScopeInfo::WeakObjectProfileTy::getSentinel() {
+ FunctionScopeInfo::WeakObjectProfileTy Result;
+ Result.Base.setInt(true);
+ return Result;
+}
+
}
}
+namespace llvm {
+ template <>
+ struct DenseMapInfo<clang::sema::FunctionScopeInfo::WeakObjectProfileTy> {
+ typedef clang::sema::FunctionScopeInfo::WeakObjectProfileTy ProfileTy;
+ static inline ProfileTy getEmptyKey() {
+ return ProfileTy();
+ }
+ static inline ProfileTy getTombstoneKey() {
+ return ProfileTy::getSentinel();
+ }
+
+ static unsigned getHashValue(const ProfileTy &Val) {
+ typedef std::pair<ProfileTy::BaseInfoTy, const clang::NamedDecl *> Pair;
+ return DenseMapInfo<Pair>::getHashValue(Pair(Val.Base, Val.Property));
+ }
+
+ static bool isEqual(const ProfileTy &LHS, const ProfileTy &RHS) {
+ return LHS == RHS;
+ }
+ };
+} // end namespace llvm
+
#endif
Modified: cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp?rev=164854&r1=164853&r2=164854&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp (original)
+++ cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp Fri Sep 28 17:21:30 2012
@@ -871,6 +871,121 @@
}
namespace {
+ typedef std::pair<const Stmt *,
+ sema::FunctionScopeInfo::WeakObjectUseMap::const_iterator>
+ StmtUsesPair;
+}
+
+template<>
+class BeforeThanCompare<StmtUsesPair> {
+ const SourceManager &SM;
+
+public:
+ explicit BeforeThanCompare(const SourceManager &SM) : SM(SM) { }
+
+ bool operator()(const StmtUsesPair &LHS, const StmtUsesPair &RHS) {
+ return SM.isBeforeInTranslationUnit(LHS.first->getLocStart(),
+ RHS.first->getLocStart());
+ }
+};
+
+
+static void diagnoseRepeatedUseOfWeak(Sema &S,
+ const sema::FunctionScopeInfo *CurFn,
+ const Decl *D) {
+ typedef sema::FunctionScopeInfo::WeakObjectProfileTy WeakObjectProfileTy;
+ typedef sema::FunctionScopeInfo::WeakObjectUseMap WeakObjectUseMap;
+ typedef sema::FunctionScopeInfo::WeakUseVector WeakUseVector;
+
+ const WeakObjectUseMap &WeakMap = CurFn->getWeakObjectUses();
+
+ // Extract all weak objects that are referenced more than once.
+ SmallVector<StmtUsesPair, 8> UsesByStmt;
+ for (WeakObjectUseMap::const_iterator I = WeakMap.begin(), E = WeakMap.end();
+ I != E; ++I) {
+ const WeakUseVector &Uses = I->second;
+ if (Uses.size() <= 1)
+ continue;
+
+ // Find the first read of the weak object.
+ WeakUseVector::const_iterator UI = Uses.begin(), UE = Uses.end();
+ for ( ; UI != UE; ++UI) {
+ if (UI->isUnsafe())
+ break;
+ }
+
+ // If there were only writes to this object, don't warn.
+ if (UI == UE)
+ continue;
+
+ UsesByStmt.push_back(StmtUsesPair(UI->getUseExpr(), I));
+ }
+
+ if (UsesByStmt.empty())
+ return;
+
+ // Sort by first use so that we emit the warnings in a deterministic order.
+ std::sort(UsesByStmt.begin(), UsesByStmt.end(),
+ BeforeThanCompare<StmtUsesPair>(S.getSourceManager()));
+
+ // Classify the current code body 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.
+ // FIXME: Should we use a common classification enum and the same set of
+ // possibilities all throughout Sema?
+ enum {
+ Function,
+ Method,
+ Block,
+ Lambda
+ } FunctionKind;
+
+ if (isa<sema::BlockScopeInfo>(CurFn))
+ FunctionKind = Block;
+ else if (isa<sema::LambdaScopeInfo>(CurFn))
+ FunctionKind = Lambda;
+ else if (isa<ObjCMethodDecl>(D))
+ FunctionKind = Method;
+ else
+ FunctionKind = Function;
+
+ // Iterate through the sorted problems and emit warnings for each.
+ for (SmallVectorImpl<StmtUsesPair>::const_iterator I = UsesByStmt.begin(),
+ E = UsesByStmt.end();
+ I != E; ++I) {
+ const Stmt *FirstRead = I->first;
+ 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.
+ unsigned DiagKind;
+ if (Key.isExactProfile())
+ DiagKind = diag::warn_arc_repeated_use_of_weak;
+ else
+ DiagKind = diag::warn_arc_possible_repeated_use_of_weak;
+
+ // Show the first time the object was read.
+ S.Diag(FirstRead->getLocStart(), DiagKind)
+ << FunctionKind
+ << FirstRead->getSourceRange();
+
+ // Print all the other accesses as notes.
+ for (WeakUseVector::const_iterator UI = Uses.begin(), UE = Uses.end();
+ UI != UE; ++UI) {
+ if (UI->getUseExpr() == FirstRead)
+ continue;
+ S.Diag(UI->getUseExpr()->getLocStart(),
+ diag::note_arc_weak_also_accessed_here)
+ << UI->getUseExpr()->getSourceRange();
+ }
+ }
+}
+
+
+namespace {
struct SLocSort {
bool operator()(const UninitUse &a, const UninitUse &b) {
// Prefer a more confident report over a less confident one.
@@ -1364,6 +1479,11 @@
DiagnoseSwitchLabelsFallthrough(S, AC, !FallThroughDiagFull);
}
+ if (S.getLangOpts().ObjCARCWeak &&
+ Diags.getDiagnosticLevel(diag::warn_arc_repeated_use_of_weak,
+ D->getLocStart()) != DiagnosticsEngine::Ignored)
+ diagnoseRepeatedUseOfWeak(S, fscope, D);
+
// Collect statistics about the CFG if it was built.
if (S.CollectStats && AC.isCFGBuilt()) {
++NumFunctionsAnalyzed;
Modified: cfe/trunk/lib/Sema/Sema.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/Sema.cpp?rev=164854&r1=164853&r2=164854&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/Sema.cpp (original)
+++ cfe/trunk/lib/Sema/Sema.cpp Fri Sep 28 17:21:30 2012
@@ -54,6 +54,132 @@
Returns.clear();
ErrorTrap.reset();
PossiblyUnreachableDiags.clear();
+ WeakObjectUses.clear();
+}
+
+static const NamedDecl *getBestPropertyDecl(const ObjCPropertyRefExpr *PropE) {
+ if (PropE->isExplicitProperty())
+ return PropE->getExplicitProperty();
+
+ return PropE->getImplicitPropertyGetter();
+}
+
+static bool isSelfExpr(const Expr *E) {
+ E = E->IgnoreParenImpCasts();
+
+ const DeclRefExpr *DRE = dyn_cast<DeclRefExpr>(E);
+ if (!DRE)
+ return false;
+
+ const ImplicitParamDecl *Param = dyn_cast<ImplicitParamDecl>(DRE->getDecl());
+ if (!Param)
+ return false;
+
+ const ObjCMethodDecl *M = dyn_cast<ObjCMethodDecl>(Param->getDeclContext());
+ if (!M)
+ return false;
+
+ return M->getSelfDecl() == Param;
+}
+
+FunctionScopeInfo::WeakObjectProfileTy::WeakObjectProfileTy(
+ const ObjCPropertyRefExpr *PropE)
+ : Base(0, false), 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;
+ }
+ } 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()));
+}
+
+void FunctionScopeInfo::markSafeWeakUse(const Expr *E) {
+ E = E->IgnoreParenImpCasts();
+
+ if (const PseudoObjectExpr *POE = dyn_cast<PseudoObjectExpr>(E)) {
+ markSafeWeakUse(POE->getSyntacticForm());
+ return;
+ }
+
+ if (const ConditionalOperator *Cond = dyn_cast<ConditionalOperator>(E)) {
+ markSafeWeakUse(Cond->getTrueExpr());
+ markSafeWeakUse(Cond->getFalseExpr());
+ return;
+ }
+
+ if (const BinaryConditionalOperator *Cond =
+ dyn_cast<BinaryConditionalOperator>(E)) {
+ markSafeWeakUse(Cond->getCommon());
+ markSafeWeakUse(Cond->getFalseExpr());
+ 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();
+ return;
+ }
}
BlockScopeInfo::~BlockScopeInfo() { }
Modified: cfe/trunk/lib/Sema/SemaDecl.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=164854&r1=164853&r2=164854&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaDecl.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDecl.cpp Fri Sep 28 17:21:30 2012
@@ -6588,6 +6588,21 @@
if (VDecl->hasAttr<BlocksAttr>())
checkRetainCycles(VDecl, Init);
+
+ // It is safe to assign a weak reference into a strong variable.
+ // Although this code can still have problems:
+ // id x = self.weakProp;
+ // id y = self.weakProp;
+ // we do not warn to warn spuriously when 'x' and 'y' are on separate
+ // paths through the function. This should be revisited if
+ // -Wrepeated-use-of-weak is made flow-sensitive.
+ if (VDecl->getType().getObjCLifetime() == Qualifiers::OCL_Strong) {
+ DiagnosticsEngine::Level Level =
+ Diags.getDiagnosticLevel(diag::warn_arc_repeated_use_of_weak,
+ Init->getLocStart());
+ if (Level != DiagnosticsEngine::Ignored)
+ getCurFunction()->markSafeWeakUse(Init);
+ }
}
Init = MaybeCreateExprWithCleanups(Init);
Modified: cfe/trunk/lib/Sema/SemaExpr.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=164854&r1=164853&r2=164854&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaExpr.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExpr.cpp Fri Sep 28 17:21:30 2012
@@ -7726,6 +7726,19 @@
if (!DRE || DRE->getDecl()->hasAttr<BlocksAttr>())
checkRetainCycles(LHSExpr, RHS.get());
+ // It is safe to assign a weak reference into a strong variable.
+ // Although this code can still have problems:
+ // id x = self.weakProp;
+ // id y = self.weakProp;
+ // we do not warn to warn spuriously when 'x' and 'y' are on separate
+ // paths through the function. This should be revisited if
+ // -Wrepeated-use-of-weak is made flow-sensitive.
+ DiagnosticsEngine::Level Level =
+ Diags.getDiagnosticLevel(diag::warn_arc_repeated_use_of_weak,
+ RHS.get()->getLocStart());
+ if (Level != DiagnosticsEngine::Ignored)
+ getCurFunction()->markSafeWeakUse(RHS.get());
+
} else if (getLangOpts().ObjCAutoRefCount) {
checkUnsafeExprAssigns(Loc, LHSExpr, RHS.get());
}
Modified: cfe/trunk/lib/Sema/SemaPseudoObject.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaPseudoObject.cpp?rev=164854&r1=164853&r2=164854&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaPseudoObject.cpp (original)
+++ cfe/trunk/lib/Sema/SemaPseudoObject.cpp Fri Sep 28 17:21:30 2012
@@ -31,6 +31,7 @@
//===----------------------------------------------------------------------===//
#include "clang/Sema/SemaInternal.h"
+#include "clang/Sema/ScopeInfo.h"
#include "clang/Sema/Initialization.h"
#include "clang/AST/ExprObjC.h"
#include "clang/Lex/Preprocessor.h"
@@ -186,7 +187,7 @@
UnaryOperatorKind opcode,
Expr *op);
- ExprResult complete(Expr *syntacticForm);
+ virtual ExprResult complete(Expr *syntacticForm);
OpaqueValueExpr *capture(Expr *op);
OpaqueValueExpr *captureValueAsResult(Expr *op);
@@ -238,6 +239,9 @@
Expr *rebuildAndCaptureObject(Expr *syntacticBase);
ExprResult buildGet();
ExprResult buildSet(Expr *op, SourceLocation, bool);
+ ExprResult complete(Expr *SyntacticForm);
+
+ bool isWeakProperty() const;
};
/// A PseudoOpBuilder for Objective-C array/dictionary indexing.
@@ -471,6 +475,23 @@
return S.LookupMethodInObjectType(sel, IT, false);
}
+bool ObjCPropertyOpBuilder::isWeakProperty() const {
+ QualType T;
+ if (RefExpr->isExplicitProperty()) {
+ const ObjCPropertyDecl *Prop = RefExpr->getExplicitProperty();
+ if (Prop->getPropertyAttributes() & ObjCPropertyDecl::OBJC_PR_weak)
+ return true;
+
+ T = Prop->getType();
+ } else if (Getter) {
+ T = Getter->getResultType();
+ } else {
+ return false;
+ }
+
+ return T.getObjCLifetime() == Qualifiers::OCL_Weak;
+}
+
bool ObjCPropertyOpBuilder::findGetter() {
if (Getter) return true;
@@ -818,6 +839,18 @@
return PseudoOpBuilder::buildIncDecOperation(Sc, opcLoc, opcode, op);
}
+ExprResult ObjCPropertyOpBuilder::complete(Expr *SyntacticForm) {
+ if (S.getLangOpts().ObjCAutoRefCount && isWeakProperty()) {
+ DiagnosticsEngine::Level Level =
+ S.Diags.getDiagnosticLevel(diag::warn_arc_repeated_use_of_weak,
+ SyntacticForm->getLocStart());
+ if (Level != DiagnosticsEngine::Ignored)
+ S.getCurFunction()->recordUseOfWeak(SyntacticRefExpr);
+ }
+
+ return PseudoOpBuilder::complete(SyntacticForm);
+}
+
// ObjCSubscript build stuff.
//
Added: cfe/trunk/test/SemaObjC/arc-repeated-weak.mm
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjC/arc-repeated-weak.mm?rev=164854&view=auto
==============================================================================
--- cfe/trunk/test/SemaObjC/arc-repeated-weak.mm (added)
+++ cfe/trunk/test/SemaObjC/arc-repeated-weak.mm Fri Sep 28 17:21:30 2012
@@ -0,0 +1,203 @@
+// RUN: %clang_cc1 -fsyntax-only -fobjc-runtime-has-weak -fobjc-arc -fblocks -Wno-objc-root-class -Warc-repeated-use-of-weak -verify %s
+
+ at interface Test {
+ at public
+ Test *ivar;
+}
+ at property(weak) Test *weakProp;
+ at property(strong) Test *strongProp;
+
+- (__weak id)implicitProp;
+
++ (__weak id)weakProp;
+ at end
+
+extern void use(id);
+extern id get();
+extern bool condition();
+#define nil ((id)0)
+
+void sanity(Test *a) {
+ use(a.weakProp); // expected-warning{{weak property is accessed multiple times in this function but may be unpredictably set to nil; assign to a strong variable to keep the object alive}}
+ use(a.weakProp); // expected-note{{also accessed here}}
+
+ use(a.strongProp);
+ use(a.strongProp); // no-warning
+
+ use(a.weakProp); // expected-note{{also accessed here}}
+}
+
+void singleUse(Test *a) {
+ use(a.weakProp); // no-warning
+ use(a.strongProp); // no-warning
+}
+
+void assignsOnly(Test *a) {
+ a.weakProp = get(); // no-warning
+
+ id next = get();
+ if (next)
+ a.weakProp = next; // no-warning
+}
+
+void assignThenRead(Test *a) {
+ a.weakProp = get(); // expected-note{{also accessed here}}
+ use(a.weakProp); // expected-warning{{weak property is accessed multiple times}}
+}
+
+void twoVariables(Test *a, Test *b) {
+ use(a.weakProp); // no-warning
+ use(b.weakProp); // no-warning
+}
+
+void doubleLevelAccess(Test *a) {
+ use(a.strongProp.weakProp); // expected-warning{{weak property may be accessed multiple times in this function but may be unpredictably set to nil; assign to a strong variable to keep the object alive}}
+ use(a.strongProp.weakProp); // expected-note{{also accessed here}}
+}
+
+void doubleLevelAccessIvar(Test *a) {
+ use(a.strongProp.weakProp); // expected-warning{{weak property may be accessed multiple times}}
+ use(a.strongProp.weakProp); // expected-note{{also accessed here}}
+}
+
+void implicitProperties(Test *a) {
+ use(a.implicitProp); // expected-warning{{weak property is accessed multiple times}}
+ use(a.implicitProp); // expected-note{{also accessed here}}
+}
+
+void classProperties() {
+ use(Test.weakProp); // expected-warning{{weak property is accessed multiple times}}
+ use(Test.weakProp); // expected-note{{also accessed here}}
+}
+
+void classPropertiesAreDifferent(Test *a) {
+ use(Test.weakProp); // no-warning
+ use(a.weakProp); // no-warning
+ use(a.strongProp.weakProp); // no-warning
+}
+
+
+void assignToStrongWrongInit(Test *a) {
+ id val = a.weakProp; // expected-note{{also accessed here}}
+ use(a.weakProp); // expected-warning{{weak property is accessed multiple times}}
+}
+
+void assignToStrongWrong(Test *a) {
+ id val;
+ val = a.weakProp; // expected-note{{also accessed here}}
+ use(a.weakProp); // expected-warning{{weak property is accessed multiple times}}
+}
+
+void assignToStrongOK(Test *a) {
+ if (condition()) {
+ id val = a.weakProp; // no-warning
+ (void)val;
+ } else {
+ id val;
+ val = a.weakProp; // no-warning
+ (void)val;
+ }
+}
+
+void assignToStrongConditional(Test *a) {
+ id val = (condition() ? a.weakProp : a.weakProp); // no-warning
+ id val2 = a.implicitProp ?: a.implicitProp; // no-warning
+}
+
+void testBlock(Test *a) {
+ use(a.weakProp); // no-warning
+
+ use(^{
+ use(a.weakProp); // expected-warning{{weak property is accessed multiple times in this block}}
+ use(a.weakProp); // expected-note{{also accessed here}}
+ });
+}
+
+
+ at interface Test (Methods)
+ at end
+
+ at implementation Test (Methods)
+- (void)sanity {
+ use(self.weakProp); // expected-warning{{weak property is accessed multiple times in this method but may be unpredictably set to nil; assign to a strong variable to keep the object alive}}
+ use(self.weakProp); // expected-note{{also accessed here}}
+}
+
+- (void)doubleLevelAccessForSelf {
+ use(self.strongProp.weakProp); // expected-warning{{weak property is accessed multiple times}}
+ use(self.strongProp.weakProp); // expected-note{{also accessed here}}
+
+ use(self->ivar.weakProp); // expected-warning{{weak property is accessed multiple times}}
+ use(self->ivar.weakProp); // expected-note{{also accessed here}}
+}
+
+- (void)distinctFromOther:(Test *)other {
+ use(self.strongProp.weakProp); // no-warning
+ use(other.strongProp.weakProp); // no-warning
+
+ use(self->ivar.weakProp); // no-warning
+ use(other->ivar.weakProp); // no-warning
+}
+ at end
+
+
+class Wrapper {
+ Test *a;
+
+public:
+ void fields() {
+ use(a.weakProp); // expected-warning{{weak property is accessed multiple times in this function but may be unpredictably set to nil; assign to a strong variable to keep the object alive}}
+ use(a.weakProp); // expected-note{{also accessed here}}
+ }
+
+ void distinctFromOther(Test *b, const Wrapper &w) {
+ use(a.weakProp); // no-warning
+ use(b.weakProp); // no-warning
+ use(w.a.weakProp); // no-warning
+ }
+
+ static void doubleLevelAccessField(const Wrapper &x, const Wrapper &y) {
+ use(x.a.weakProp); // expected-warning{{weak property may be accessed multiple times}}
+ use(y.a.weakProp); // expected-note{{also accessed here}}
+ }
+};
+
+
+// -----------------------
+// False positives
+// -----------------------
+
+// Most of these would require flow-sensitive analysis to silence correctly.
+
+void assignAfterRead(Test *a) {
+ if (!a.weakProp) // expected-warning{{weak property is accessed multiple times}}
+ a.weakProp = get(); // expected-note{{also accessed here}}
+}
+
+void assignNil(Test *a) {
+ if (condition())
+ a.weakProp = nil; // expected-note{{also accessed here}}
+
+ use(a.weakProp); // expected-warning{{weak property is accessed multiple times}}
+}
+
+void branch(Test *a) {
+ if (condition())
+ use(a.weakProp); // expected-warning{{weak property is accessed multiple times}}
+ else
+ use(a.weakProp); // expected-note{{also accessed here}}
+}
+
+void doubleLevelAccess(Test *a, Test *b) {
+ use(a.strongProp.weakProp); // expected-warning{{weak property may be accessed multiple times}}
+ use(b.strongProp.weakProp); // expected-note{{also accessed here}}
+
+ use(a.weakProp.weakProp); // no-warning
+}
+
+void doubleLevelAccessIvar(Test *a, Test *b) {
+ use(a->ivar.weakProp); // expected-warning{{weak property may be accessed multiple times}}
+ use(b->ivar.weakProp); // expected-note{{also accessed here}}
+
+ use(a.strongProp.weakProp); // no-warning
+}
More information about the cfe-commits
mailing list