[cfe-commits] [Patch] Extend -Wloop-analysis to while and do-while loops

[Douglas Gregor]

On Jun 7, 2012, at 6:19 PM, Richard Trieu wrote:

> This patch extends the loops checks that originally only had checked for-loops to also check while and do-while loops.  Changes are:
> 
> - Add a bit to VarDecl to keep track if a variable has been passed by reference (but not const reference) or had its address taken.

This particular bit should be separated from the rest of the patch, because it's generally useful on its own and will need a bit more scrutiny/validation/testing. In particular, this

Index: include/clang/AST/Expr.h
===================================================================
--- include/clang/AST/Expr.h	(revision 158039)
+++ include/clang/AST/Expr.h	(working copy)
@@ -1545,7 +1545,12 @@
            (input->isInstantiationDependent() ||
             type->isInstantiationDependentType()),
            input->containsUnexpandedParameterPack()),
-      Opc(opc), Loc(l), Val(input) {}
+      Opc(opc), Loc(l), Val(input) {
+    if (opc == UO_AddrOf)
+      if (DeclRefExpr *DRE = dyn_cast<DeclRefExpr>(input->IgnoreParens()))
+        if (VarDecl *VD = dyn_cast<VarDecl>(DRE->getDecl()))
+          VD->setReference(true);
+  }
 
is not something we should do in AST. Sema should be in charge of marking the 'address taken' bit, and should do so for any of the constructs---implicit or explicit---that take the address of a variable. And if we're going to add this AddressTaken bit, we need to find a good way to validate that it is correct, because it could be useful elsewhere (e.g., in IR generation).


> - If the bit above is set on a variable, do not emit the warning.  Also, don't emit the warning for parameter variables in while loops.
> - Don't warn on variable declared inside a conditional.
> - Don't warn when there is a function call to a noreturn function inside the loop.

@@ -1196,7 +1149,6 @@
   class DeclMatcher : public EvaluatedExprVisitor<DeclMatcher> {
     llvm::SmallPtrSet<VarDecl*, 8> &Decls;
     bool FoundDecl;
-    //bool EvalDecl;
 
 public:
   typedef EvaluatedExprVisitor<DeclMatcher> Inherited;

Orphan declaration here?

The rest (= the parts that don't introduce, set, or use the new 'HasReference' bit) looks okay.

	- Doug