On Tue, Apr 17, 2012 at 3:32 PM, Richard Trieu <span dir="ltr"><<a href="mailto:rtrieu@google.com" target="_blank">rtrieu@google.com</a>></span> wrote:<br><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="gmail_quote"><div><div class="h5">On Thu, Apr 5, 2012 at 10:25 AM, Douglas Gregor <span dir="ltr"><<a href="mailto:dgregor@apple.com" target="_blank">dgregor@apple.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">

<div><div><br>
On Mar 22, 2012, at 4:20 PM, Richard Trieu <<a href="mailto:rtrieu@google.com" target="_blank">rtrieu@google.com</a>> wrote:<br>
<br>
> New patch for the for-loop warning.<br>
><br>
> Changes:<br>
> Both visitors are now EvaluatedExprVisitor<br>
> The DeclExtractor, used on the conditional, works on a whitelist of Expr's instead of a blacklist.<br>
> Fixed edge cases in the DeclMatcher with respect to LValueToRValue conversions.  Some casts aren't directly on DeclRefExprs.<br>
> Added variable names to the warning.  Up to 4 variable names will be displayed.<br>
</div></div>> <loop-analysis3.patch><br>
<br>
<br>
+  void VisitCastExpr(CastExpr *E) {<br>
+    if (E->getCastKind() == CK_LValueToRValue)<br>
+      EvalDecl = true;<br>
+    Visit(E->getSubExpr());<br>
+  }<br>
<br>
The use of EvalDecl here and here:<br>
<br>
+  void VisitDeclRefExpr(DeclRefExpr *E) {<br>
+    if (EvalDecl) return;<br>
+<br>
+    if (VarDecl *VD = dyn_cast<VarDecl>(E->getDecl()))<br>
+      if (Decls.count(VD)) {<br>
<div>+        FoundDecl = true;<br>
+        return;<br>
+      }<br>
</div>+  }<br>
<br>
feels a bit hacky. Why not look at the subexpression of an lvalue-to-rvalue cast, looking through any implicitly-generated nodes, and skip visitation if it is a DeclRefExpr?<br></blockquote><div><br></div></div></div><div>
I remember why now.  Besides cast expressions and paren expressions, conditional operators can get between an lvalue-to-rvalue cast and a DeclRefExpr.  </div><div class="im">
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
Otherwise, this patch seems fine. I'm curious to see how it works out in practice.<br>
<br>
        - Doug<br></blockquote><div> </div></div></div>Couple of updates:<div>Got rid EvalDecl and changed how lvalue-to-rvalue casts are handled.</div><div>Moved caret location to first decl in conditional expression.</div>
<div>
If too many things need to be underlined, underline the whole condition expression.</div><div>Skip checking if there is a dereference in the conditional.</div>
</blockquote></div><br><div>Committed at r155835</div>