<div style="font-family: arial, helvetica, sans-serif"><font size="2"><br><br><div class="gmail_quote">On Mon, Jun 25, 2012 at 9:42 PM, Nico Weber <span dir="ltr"><<a href="mailto:thakis@chromium.org" target="_blank">thakis@chromium.org</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi Ted,<br>
<div class="im"><br>
On Mon, Jun 25, 2012 at 12:40 PM, Ted Kremenek <<a href="mailto:kremenek@apple.com">kremenek@apple.com</a>> wrote:<br>
> Hi Nico,<br>
><br>
> How expensive is this check?  This code makes me nervous:<br>
<br>
</div>I didn't measure, but note that this is only executed if the lhs and<br>
rhs are both MemberExprs, and they both refer to the same Decl. This<br>
isn't true for most assignments.<br></blockquote><div><br></div><div>Would it be cheaper to just check whether the MemberExpr directly sets one member (e.g. var_ = var_)? I think cases like t->s_->a_ = t->s_->a_; are very rare.</div>
<div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Is it enough to do measure the impact on a full build of chromium, or<br>
would you like to see more data?<br></blockquote><div><br></div><div>The question is whether code with this kind of bug will be commonly committed to a codebase. My guess would be that this would lead to bugs that are painstakingly debugged before committing. Thus, the warning might fire much more often during development. But I don't have data.</div>
<div><br></div><div>Daniel</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<span class="HOEnZb"><font color="#888888"><br>
Nico<br>
</font></span><div class="HOEnZb"><div class="h5"><br>
><br>
> +  llvm::FoldingSetNodeID lhsID, rhsID;<br>
> +  ML->getBase()->Profile(lhsID, Sema.Context, true);<br>
> +  MR->getBase()->Profile(rhsID, Sema.Context, true);<br>
> +  if (lhsID == rhsID)<br>
> +    Sema.Diag(Loc, diag::warn_identity_memvar_assign);<br>
><br>
> If this is done on every assignment, might this be a bit expensive for marginal gain?<br>
><br>
> On Jun 25, 2012, at 12:26 PM, Nico Weber <<a href="mailto:thakis@chromium.org">thakis@chromium.org</a>> wrote:<br>
><br>
>> Hi,<br>
>><br>
>> the attached patch adds a warning for self-assignments of member<br>
>> variables. This is PR13104. I do this every now and then locally when<br>
>> writing setter functions: I type "void set_var(int var) { var_ =" and<br>
>> then try to hit ctrl-p twice to complete the lhs to "var" in vim, but<br>
>> miss and hit it only once, so that I end up with "var_ = var_;"<br>
>><br>
>> This finds 0 bugs and 0 false positives in chromium, so I'm not sure<br>
>> how useful this is. Opinions?<br>
>><br>
>> Nico<br>
>> <clang-memvar-assign.patch>_______________________________________________<br>
>> cfe-commits mailing list<br>
>> <a href="mailto:cfe-commits@cs.uiuc.edu">cfe-commits@cs.uiuc.edu</a><br>
>> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
><br>
<br>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@cs.uiuc.edu">cfe-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
</div></div></blockquote></div><br></font></div>