-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>

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@166942 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/include/clang/Sema/ScopeInfo.h b/include/clang/Sema/ScopeInfo.h
index c49dee7b41e0d347391a1a5f14497dbb81daaa53..feda9c96b8574ed4fb3484631e7cf4e8c2cfc02d 100644 (file)
--- a/include/clang/Sema/ScopeInfo.h
+++ b/include/clang/Sema/ScopeInfo.h
@@ -169,6 +169,7 @@ public:
     WeakObjectProfileTy(const DeclRefExpr *RE);
     WeakObjectProfileTy(const ObjCIvarRefExpr *RE);
 
+    const NamedDecl *getBase() const { return Base.getPointer(); }
     const NamedDecl *getProperty() const { return Property; }
 
     /// Returns true if the object base specifies a known object in memory,

diff --git a/lib/Sema/AnalysisBasedWarnings.cpp b/lib/Sema/AnalysisBasedWarnings.cpp
index b039bc6f92bf98ce73fd4e5223cb50034c6afb39..a20817f965008eb66d5c3b396c4070e2e7bc98c9 100644 (file)
--- a/lib/Sema/AnalysisBasedWarnings.cpp
+++ b/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));

diff --git a/test/SemaObjC/arc-repeated-weak.mm b/test/SemaObjC/arc-repeated-weak.mm
index 1539a9bcae07579cf3bef9d43e04294a7489d6a5..e652bee82d535f0b6c926a3a0cf3220d26e7541c 100644 (file)
--- a/test/SemaObjC/arc-repeated-weak.mm
+++ b/test/SemaObjC/arc-repeated-weak.mm
@@ -257,6 +257,38 @@ void readOnlyLoop(Test *a) {
   }
 }
 
+void readInIterationLoop() {
+  for (Test *a in get())
+    use(a.weakProp); // no-warning
+}
+
+void readDoubleLevelAccessInLoop() {
+  for (Test *a in get()) {
+    use(a.strongProp.weakProp); // no-warning
+  }
+}
+
+void readParameterInLoop(Test *a) {
+  for (id unused in get()) {
+    use(a.weakProp); // expected-warning{{weak property 'weakProp' is accessed multiple times in this function}}
+    (void)unused;
+  }
+}
+
+void readGlobalInLoop() {
+  static __weak id a;
+  for (id unused in get()) {
+    use(a); // expected-warning{{weak variable 'a' is accessed multiple times in this function}}
+    (void)unused;
+  }
+}
+
+void doWhileLoop(Test *a) {
+  do {
+    use(a.weakProp); // no-warning
+  } while(0);
+}
+
 
 @interface Test (Methods)
 @end
@@ -351,3 +383,4 @@ void doubleLevelAccessIvar(Test *a, Test *b) {
 
   use(a.strongProp.weakProp); // no-warning
 }
+