diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td index 32c03f7364dc..156cf54d706a 100644 --- a/clang/include/clang/Basic/DiagnosticGroups.td +++ b/clang/include/clang/Basic/DiagnosticGroups.td @@ -282,6 +282,9 @@ def AutomaticReferenceCounting : DiagGroup<"arc", [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">; diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index 6c92a6d23fd0..fa492164ab51 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -709,6 +709,18 @@ def warn_receiver_is_weak : Warning < "weak %select{receiver|property|implicit property}0 may be " "unpredictably null in ARC mode">, InGroup>, 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, 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, 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">; diff --git a/clang/include/clang/Sema/ScopeInfo.h b/clang/include/clang/Sema/ScopeInfo.h index b4752f5dbb12..e4a663c05976 100644 --- a/clang/include/clang/Sema/ScopeInfo.h +++ b/clang/include/clang/Sema/ScopeInfo.h @@ -21,6 +21,7 @@ namespace clang { +class Decl; class BlockDecl; class CXXMethodDecl; class IdentifierInfo; @@ -29,6 +30,7 @@ class ReturnStmt; class Scope; class SwitchStmt; class VarDecl; +class ObjCPropertyRefExpr; namespace sema { @@ -113,6 +115,125 @@ public: /// prior to being emitted. SmallVector 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 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; + 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 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 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 + 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 @@ public: }; +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 { + 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 Pair; + return DenseMapInfo::getHashValue(Pair(Val.Base, Val.Property)); + } + + static bool isEqual(const ProfileTy &LHS, const ProfileTy &RHS) { + return LHS == RHS; + } + }; +} // end namespace llvm + #endif diff --git a/clang/lib/Sema/AnalysisBasedWarnings.cpp b/clang/lib/Sema/AnalysisBasedWarnings.cpp index a3aee9afe083..86e9dc2d6483 100644 --- a/clang/lib/Sema/AnalysisBasedWarnings.cpp +++ b/clang/lib/Sema/AnalysisBasedWarnings.cpp @@ -870,6 +870,121 @@ static void DiagnoseSwitchLabelsFallthrough(Sema &S, AnalysisDeclContext &AC, } +namespace { + typedef std::pair + StmtUsesPair; +} + +template<> +class BeforeThanCompare { + 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 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(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(CurFn)) + FunctionKind = Block; + else if (isa(CurFn)) + FunctionKind = Lambda; + else if (isa(D)) + FunctionKind = Method; + else + FunctionKind = Function; + + // Iterate through the sorted problems and emit warnings for each. + for (SmallVectorImpl::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) { @@ -1364,6 +1479,11 @@ AnalysisBasedWarnings::IssueWarnings(sema::AnalysisBasedWarnings::Policy P, 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; diff --git a/clang/lib/Sema/Sema.cpp b/clang/lib/Sema/Sema.cpp index 08ccfa425987..84cdb2b6f04a 100644 --- a/clang/lib/Sema/Sema.cpp +++ b/clang/lib/Sema/Sema.cpp @@ -54,6 +54,132 @@ void FunctionScopeInfo::Clear() { 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(E); + if (!DRE) + return false; + + const ImplicitParamDecl *Param = dyn_cast(DRE->getDecl()); + if (!Param) + return false; + + const ObjCMethodDecl *M = dyn_cast(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(PropE->getBase()); + const Expr *E = OVE->getSourceExpr()->IgnoreParenCasts(); + + switch (E->getStmtClass()) { + case Stmt::DeclRefExprClass: + Base.setPointer(cast(E)->getDecl()); + Base.setInt(isa(Base.getPointer())); + break; + case Stmt::MemberExprClass: { + const MemberExpr *ME = cast(E); + Base.setPointer(ME->getMemberDecl()); + Base.setInt(isa(ME->getBase()->IgnoreParenImpCasts())); + break; + } + case Stmt::ObjCIvarRefExprClass: { + const ObjCIvarRefExpr *IE = cast(E); + Base.setPointer(IE->getDecl()); + if (isSelfExpr(IE->getBase())) + Base.setInt(true); + break; + } + case Stmt::PseudoObjectExprClass: { + const PseudoObjectExpr *POE = cast(E); + const ObjCPropertyRefExpr *BaseProp = + dyn_cast(POE->getSyntacticForm()); + if (BaseProp) { + Base.setPointer(getBestPropertyDecl(BaseProp)); + + const Expr *DoubleBase = BaseProp->getBase(); + if (const OpaqueValueExpr *OVE = dyn_cast(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(E)) { + markSafeWeakUse(POE->getSyntacticForm()); + return; + } + + if (const ConditionalOperator *Cond = dyn_cast(E)) { + markSafeWeakUse(Cond->getTrueExpr()); + markSafeWeakUse(Cond->getFalseExpr()); + return; + } + + if (const BinaryConditionalOperator *Cond = + dyn_cast(E)) { + markSafeWeakUse(Cond->getCommon()); + markSafeWeakUse(Cond->getFalseExpr()); + return; + } + + if (const ObjCPropertyRefExpr *RefExpr = dyn_cast(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() { } diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp index 01aaf8be323b..592956b9dbc2 100644 --- a/clang/lib/Sema/SemaDecl.cpp +++ b/clang/lib/Sema/SemaDecl.cpp @@ -6588,6 +6588,21 @@ void Sema::AddInitializerToDecl(Decl *RealDecl, Expr *Init, if (VDecl->hasAttr()) 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); diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp index 630281a42426..597e4d6f77c5 100644 --- a/clang/lib/Sema/SemaExpr.cpp +++ b/clang/lib/Sema/SemaExpr.cpp @@ -7726,6 +7726,19 @@ QualType Sema::CheckAssignmentOperands(Expr *LHSExpr, ExprResult &RHS, if (!DRE || DRE->getDecl()->hasAttr()) 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()); } diff --git a/clang/lib/Sema/SemaPseudoObject.cpp b/clang/lib/Sema/SemaPseudoObject.cpp index 31496b32a07c..b07a59b81c2e 100644 --- a/clang/lib/Sema/SemaPseudoObject.cpp +++ b/clang/lib/Sema/SemaPseudoObject.cpp @@ -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 @@ namespace { 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 @@ namespace { 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 @@ static ObjCMethodDecl *LookupMethodInReceiverType(Sema &S, Selector sel, 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 @@ ObjCPropertyOpBuilder::buildIncDecOperation(Scope *Sc, SourceLocation opcLoc, 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. // diff --git a/clang/test/SemaObjC/arc-repeated-weak.mm b/clang/test/SemaObjC/arc-repeated-weak.mm new file mode 100644 index 000000000000..728ffcb951d7 --- /dev/null +++ b/clang/test/SemaObjC/arc-repeated-weak.mm @@ -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 + +@interface Test { +@public + Test *ivar; +} +@property(weak) Test *weakProp; +@property(strong) Test *strongProp; + +- (__weak id)implicitProp; + ++ (__weak id)weakProp; +@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}} + }); +} + + +@interface Test (Methods) +@end + +@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 +} +@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 +}