diff options
| -rw-r--r-- | clang/lib/Sema/AnalysisBasedWarnings.cpp | 40 | ||||
| -rw-r--r-- | clang/test/SemaObjC/arc-repeated-weak.mm | 71 |
2 files changed, 101 insertions, 10 deletions
diff --git a/clang/lib/Sema/AnalysisBasedWarnings.cpp b/clang/lib/Sema/AnalysisBasedWarnings.cpp index 6d09f2c116f..b039bc6f92b 100644 --- a/clang/lib/Sema/AnalysisBasedWarnings.cpp +++ b/clang/lib/Sema/AnalysisBasedWarnings.cpp @@ -27,6 +27,7 @@ #include "clang/AST/StmtObjC.h" #include "clang/AST/StmtCXX.h" #include "clang/AST/EvaluatedExprVisitor.h" +#include "clang/AST/ParentMap.h" #include "clang/AST/StmtVisitor.h" #include "clang/AST/RecursiveASTVisitor.h" #include "clang/Analysis/AnalysisContext.h" @@ -903,10 +904,30 @@ public: }; } +static bool isInLoop(const ParentMap &PM, const Stmt *S) { + assert(S); + + do { + switch (S->getStmtClass()) { + case Stmt::DoStmtClass: + case Stmt::ForStmtClass: + case Stmt::WhileStmtClass: + case Stmt::CXXForRangeStmtClass: + case Stmt::ObjCForCollectionStmtClass: + return true; + default: + break; + } + } while ((S = PM.getParent(S))); + + return false; +} + static void diagnoseRepeatedUseOfWeak(Sema &S, const sema::FunctionScopeInfo *CurFn, - const Decl *D) { + const Decl *D, + const ParentMap &PM) { typedef sema::FunctionScopeInfo::WeakObjectProfileTy WeakObjectProfileTy; typedef sema::FunctionScopeInfo::WeakObjectUseMap WeakObjectUseMap; typedef sema::FunctionScopeInfo::WeakUseVector WeakUseVector; @@ -918,8 +939,6 @@ static void diagnoseRepeatedUseOfWeak(Sema &S, 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(); @@ -932,6 +951,19 @@ static void diagnoseRepeatedUseOfWeak(Sema &S, if (UI == UE) continue; + // If there was only one read, followed by any number of writes, and the + // read is not within a loop, don't warn. + if (UI == Uses.begin()) { + WeakUseVector::const_iterator UI2 = UI; + for (++UI2; UI2 != UE; ++UI2) + if (UI2->isUnsafe()) + break; + + if (UI2 == UE) + if (!isInLoop(PM, UI->getUseExpr())) + continue; + } + UsesByStmt.push_back(StmtUsesPair(UI->getUseExpr(), I)); } @@ -1519,7 +1551,7 @@ AnalysisBasedWarnings::IssueWarnings(sema::AnalysisBasedWarnings::Policy P, if (S.getLangOpts().ObjCARCWeak && Diags.getDiagnosticLevel(diag::warn_arc_repeated_use_of_weak, D->getLocStart()) != DiagnosticsEngine::Ignored) - diagnoseRepeatedUseOfWeak(S, fscope, D); + diagnoseRepeatedUseOfWeak(S, fscope, D, AC.getParentMap()); // Collect statistics about the CFG if it was built. if (S.CollectStats && AC.isCFGBuilt()) { diff --git a/clang/test/SemaObjC/arc-repeated-weak.mm b/clang/test/SemaObjC/arc-repeated-weak.mm index ca13e20dcf2..a59f435693d 100644 --- a/clang/test/SemaObjC/arc-repeated-weak.mm +++ b/clang/test/SemaObjC/arc-repeated-weak.mm @@ -1,4 +1,4 @@ -// RUN: %clang_cc1 -fsyntax-only -fobjc-runtime-has-weak -fobjc-arc -fblocks -Wno-objc-root-class -Warc-repeated-use-of-weak -verify %s +// RUN: %clang_cc1 -fsyntax-only -fobjc-runtime-has-weak -fobjc-arc -fblocks -Wno-objc-root-class -std=c++11 -Warc-repeated-use-of-weak -verify %s @interface Test { @public @@ -181,6 +181,70 @@ void assignToStrongWithCasts(Test *a) { } } +void assignAfterRead(Test *a) { + // Special exception for a single read before any writes. + if (!a.weakProp) // no-warning + a.weakProp = get(); // no-warning +} + +void readOnceWriteMany(Test *a) { + if (!a.weakProp) { // no-warning + a.weakProp = get(); // no-warning + a.weakProp = get(); // no-warning + } +} + +void readOnceAfterWrite(Test *a) { + a.weakProp = get(); // expected-note{{also accessed here}} + if (!a.weakProp) { // expected-warning{{weak property 'weakProp' is accessed multiple times in this function}} + a.weakProp = get(); // expected-note{{also accessed here}} + } +} + +void readOnceWriteManyLoops(Test *a, Test *b, Test *c, Test *d, Test *e) { + while (condition()) { + if (!a.weakProp) { // expected-warning{{weak property 'weakProp' is accessed multiple times in this function}} + a.weakProp = get(); // expected-note{{also accessed here}} + a.weakProp = get(); // expected-note{{also accessed here}} + } + } + + do { + if (!b.weakProp) { // expected-warning{{weak property 'weakProp' is accessed multiple times in this function}} + b.weakProp = get(); // expected-note{{also accessed here}} + b.weakProp = get(); // expected-note{{also accessed here}} + } + } while (condition()); + + for (id x = get(); x; x = get()) { + if (!c.weakProp) { // expected-warning{{weak property 'weakProp' is accessed multiple times in this function}} + c.weakProp = get(); // expected-note{{also accessed here}} + c.weakProp = get(); // expected-note{{also accessed here}} + } + } + + for (id x in get()) { + if (!d.weakProp) { // expected-warning{{weak property 'weakProp' is accessed multiple times in this function}} + d.weakProp = get(); // expected-note{{also accessed here}} + d.weakProp = get(); // expected-note{{also accessed here}} + } + } + + int array[] = { 1, 2, 3 }; + for (int i : array) { + if (!e.weakProp) { // expected-warning{{weak property 'weakProp' is accessed multiple times in this function}} + e.weakProp = get(); // expected-note{{also accessed here}} + e.weakProp = get(); // expected-note{{also accessed here}} + } + } +} + +void readOnlyLoop(Test *a) { + while (condition()) { + use(a.weakProp); // expected-warning{{weak property 'weakProp' is accessed multiple times in this function}} + } +} + @interface Test (Methods) @end @@ -248,11 +312,6 @@ public: // Most of these would require flow-sensitive analysis to silence correctly. -void assignAfterRead(Test *a) { - if (!a.weakProp) // expected-warning{{weak property 'weakProp' 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}} |
