-Warc-repeated-use-of-weak: allow single reads in loops from local variables.

Previously, the warning would erroneously fire on this:

for (Test *a in someArray)
  use(a.weakProp);

...because it looks like the same property is being accessed over and over.
However, clearly this is not the case. We now ignore loops like this for
local variables, but continue to warn if the base object is a parameter,
global variable, or instance variable, on the assumption that these are
not repeatedly usually assigned to within loops.

Additionally, do-while loops where the condition is 'false' are not really
loops at all; usually they're just used for semicolon-swallowing macros or
using "break" like "goto".

<rdar://problem/12578785&12578849>

llvm-svn: 166942

1 files changed, 30 insertions, 5 deletions

diff --git a/clang/lib/Sema/AnalysisBasedWarnings.cpp b/clang/lib/Sema/AnalysisBasedWarnings.cpp
index b039bc6f92b..a20817f9650 100644
--- a/clang/lib/Sema/AnalysisBasedWarnings.cpp
+++ b/clang/lib/Sema/AnalysisBasedWarnings.cpp
@@ -904,17 +904,24 @@ public:
 };
 }
 
-static bool isInLoop(const ParentMap &PM, const Stmt *S) {
+static bool isInLoop(const ASTContext &Ctx, 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;
+    case Stmt::DoStmtClass: {
+      const Expr *Cond = cast<DoStmt>(S)->getCond();
+      llvm::APSInt Val;
+      if (!Cond->EvaluateAsInt(Val, Ctx))
+        return true;
+      return Val.getBoolValue();
+    }
     default:
       break;
     }
@@ -932,6 +939,8 @@ static void diagnoseRepeatedUseOfWeak(Sema &S,
   typedef sema::FunctionScopeInfo::WeakObjectUseMap WeakObjectUseMap;
   typedef sema::FunctionScopeInfo::WeakUseVector WeakUseVector;
 
+  ASTContext &Ctx = S.getASTContext();
+
   const WeakObjectUseMap &WeakMap = CurFn->getWeakObjectUses();
 
   // Extract all weak objects that are referenced more than once.
@@ -952,16 +961,32 @@ static void diagnoseRepeatedUseOfWeak(Sema &S,
       continue;
 
     // If there was only one read, followed by any number of writes, and the
-    // read is not within a loop, don't warn.
+    // read is not within a loop, don't warn. Additionally, don't warn in a
+    // loop if the base object is a local variable -- local variables are often
+    // changed in loops.
     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()))
+      if (UI2 == UE) {
+        if (!isInLoop(Ctx, PM, UI->getUseExpr()))
           continue;
+
+        const WeakObjectProfileTy &Profile = I->first;
+        if (!Profile.isExactProfile())
+          continue;
+
+        const NamedDecl *Base = Profile.getBase();
+        if (!Base)
+          Base = Profile.getProperty();
+        assert(Base && "A profile always has a base or property.");
+
+        if (const VarDecl *BaseVar = dyn_cast<VarDecl>(Base))
+          if (BaseVar->hasLocalStorage() && !isa<ParmVarDecl>(Base))
+            continue;
+      }
     }
 
     UsesByStmt.push_back(StmtUsesPair(UI->getUseExpr(), I));