[cfe-commits] r166942 - in /cfe/trunk: include/clang/Sema/ScopeInfo.h lib/Sema/AnalysisBasedWarnings.cpp test/SemaObjC/arc-repeated-weak.mm

[Jordan Rose]

Author: jrose
Date: Mon Oct 29 12:46:47 2012
New Revision: 166942

URL: http://llvm.org/viewvc/llvm-project?rev=166942&view=rev
Log:
-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>

Modified:
    cfe/trunk/include/clang/Sema/ScopeInfo.h
    cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp
    cfe/trunk/test/SemaObjC/arc-repeated-weak.mm

Modified: cfe/trunk/include/clang/Sema/ScopeInfo.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/ScopeInfo.h?rev=166942&r1=166941&r2=166942&view=diff
==============================================================================
--- cfe/trunk/include/clang/Sema/ScopeInfo.h (original)
+++ cfe/trunk/include/clang/Sema/ScopeInfo.h Mon Oct 29 12:46:47 2012
@@ -169,6 +169,7 @@
     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,

Modified: cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp?rev=166942&r1=166941&r2=166942&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp (original)
+++ cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp Mon Oct 29 12:46:47 2012
@@ -904,17 +904,24 @@
 };
 }
 
-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 @@
   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 @@
       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));

Modified: cfe/trunk/test/SemaObjC/arc-repeated-weak.mm
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjC/arc-repeated-weak.mm?rev=166942&r1=166941&r2=166942&view=diff
==============================================================================
--- cfe/trunk/test/SemaObjC/arc-repeated-weak.mm (original)
+++ cfe/trunk/test/SemaObjC/arc-repeated-weak.mm Mon Oct 29 12:46:47 2012
@@ -257,6 +257,38 @@
   }
 }
 
+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 @@
 
   use(a.strongProp.weakProp); // no-warning
 }
+