<div dir="ltr">Thanks! See comments on code review.</div><br><div class="gmail_quote"><div dir="ltr">On Tue, May 31, 2016 at 12:41 AM Piotr Padlewski <<a href="mailto:piotr.padlewski@gmail.com">piotr.padlewski@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><a href="http://reviews.llvm.org/D20801" target="_blank">http://reviews.llvm.org/D20801</a> please see review.</div><div dir="ltr"><br><div><br></div><div>Piotr</div></div><div class="gmail_extra"><br><div class="gmail_quote">2016-05-30 23:30 GMT+02:00 Piotr Padlewski <span dir="ltr"><<a href="mailto:piotr.padlewski@gmail.com" target="_blank">piotr.padlewski@gmail.com</a>></span>:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">I don't see any objection, so I am taking care of this refactor right now.</div><div><div><div class="gmail_extra"><br><div class="gmail_quote">2016-05-19 18:55 GMT+02:00 Manuel Klimek <span dir="ltr"><<a href="mailto:klimek@google.com" target="_blank">klimek@google.com</a>></span>:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">So, I'm all for (2) unless we find people who object, and making people write has(ignoringParenImpCasts())<div>That's already what we force people to do for various has* versions.</div></div><div><div><br><div class="gmail_quote"><div dir="ltr">On Thu, May 19, 2016 at 6:31 PM Piotr Padlewski <<a href="mailto:piotr.padlewski@gmail.com" target="_blank">piotr.padlewski@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">Sure, I assumed that the feature to "has" be equvalent to <span style="font-size:12.8px"> has(ignoringParenImpCasts(x)) was important enough, to have new matcher, so you would not have to write </span><span style="font-size:12.8px">has(ignoringParenImpCasts(x)).</span></div><div class="gmail_extra"><br><div class="gmail_quote">2016-05-19 17:10 GMT+02:00 Manuel Klimek <span dir="ltr"><<a href="mailto:klimek@google.com" target="_blank">klimek@google.com</a>></span>:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><span><div dir="ltr">On Thu, May 19, 2016 at 3:48 PM Piotr Padlewski <<a href="mailto:piotr.padlewski@gmail.com" target="_blank">piotr.padlewski@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><p>The question is: in how many places the ignoring implicit casts is needed for has? I am not sure about statistics, but if it would turned out that in more than half places, the developer that wrote check didn't need that, then I would probably go with option 2.</p><p>For option 2 I would suggest to add another matcher that would be equivalent to "has" that we have right now, but I am not sure how to call it. "hasIndirect" would suggest that it only accepts indirect childs.</p></div></blockquote><div><br></div></span><div>Why would we need a new matcher? We already have has(ignoringParenImpCasts(x)) which would be equivalent to today's has(x), don't we? </div><div><div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><p>Anyway, I like option 2 more, because it seems more resonable. </p></div><div dir="ltr"><p><br></p><p>Piotr</p></div><div dir="ltr"><p><br></p>
<div class="gmail_quote">17.05.2016 12:22 PM "Manuel Klimek" <<a href="mailto:klimek@google.com" target="_blank">klimek@google.com</a>> napisał(a):<br type="attribution"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">The problem is that in C++ you often have implicit conversions that are completely irrelevant.<div>has() ignoring implicit conversions is more closely resembling what's written in the code.</div><div><br></div><div>Both options:</div><div>1. adding a new function hasDirect</div><div>2. changing has() to not go through implicit conversions, and refactoring all uses of has() to has(ignoringParenImpCasts())</div><div>... seem fine to me, with different trade-offs.</div><div><br></div><div>Thoughts?</div><div>/Manuel</div></div><br><div class="gmail_quote"><div dir="ltr">On Sun, May 15, 2016 at 3:06 PM Piotr Padlewski <<a href="mailto:piotr.padlewski@gmail.com" target="_blank">piotr.padlewski@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div>Thanks for response Alexey.</div>Is there any reason why it do so? This is very unintuitive and it also makes it harder and uglier to use matchers - instead of saying something(has(something2())) I have to say something2(hasParent(something())).</div><div dir="ltr"><div><br></div><div>Piotr</div></div><div class="gmail_extra"><br><div class="gmail_quote">2016-05-15 13:38 GMT+02:00 Alexey Sidorin <span dir="ltr"><<a href="mailto:alexey.v.sidorin@ya.ru" target="_blank">alexey.v.sidorin@ya.ru</a>></span>:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div text="#000000" bgcolor="#FFFFFF">
<div>Hello Piotr,<br>
<br>
has() matcher ignores implicit casts and parens. That's not a bug
(however, it will be good to point it in doxygen).<br>
<br>
<br>
13.05.2016 15:27, Piotr Padlewski via cfe-dev пишет:<br>
</div>
<blockquote type="cite"><div><div>
<div dir="ltr">I am have a problem with has matcher. It doesn't
work for cases like
<div>implicitCastExpr(has(implicitCastExpr()))</div>
<div>or </div>
<div>
<pre style="font-size:medium;white-space:pre-wrap;width:50em;color:rgb(0,0,0)">cxxMemberCallExpr(has(materializeTemporaryExpr())))</pre>
<pre style="font-size:medium;white-space:pre-wrap;width:50em;color:rgb(0,0,0)">or</pre>
<pre style="width:50em"><pre style="color:rgb(0,0,0);font-size:medium;white-space:pre-wrap;width:50em">returnStmt(has(implicitCastExpr()))</pre><pre style="color:rgb(0,0,0);font-size:medium;white-space:pre-wrap;width:50em">Here is a bug in bugzilla</pre><pre style="width:50em"><font color="#000000" size="3"><span style="white-space:pre-wrap"><a href="https://llvm.org/bugs/show_bug.cgi?id=27713" target="_blank">https://llvm.org/bugs/show_bug.cgi?id=27713</a>
</span></font></pre><pre style="width:50em"><font color="#000000" size="3"><span style="white-space:pre-wrap">
</span></font></pre><pre style="width:50em"><font color="#000000" size="3"><span style="white-space:pre-wrap">Am I doing something wrong or it is a bug? This thing blocks me on 2 clang-tidy checks that I am working on.</span></font></pre><pre style="width:50em"><font color="#000000" size="3"><span style="white-space:pre-wrap">
</span></font></pre><pre style="width:50em"><font color="#000000" size="3"><span style="white-space:pre-wrap">Thanks</span></font></pre><pre style="width:50em"><font color="#000000" size="3"><span style="white-space:pre-wrap">Piotr</span></font></pre><pre style="color:rgb(0,0,0);font-size:medium;white-space:pre-wrap;width:50em"></pre></pre>
</div>
</div>
<br>
<fieldset></fieldset>
<br>
</div></div><pre>_______________________________________________
cfe-dev mailing list
<a href="mailto:cfe-dev@lists.llvm.org" target="_blank">cfe-dev@lists.llvm.org</a>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev</a>
</pre>
</blockquote>
<br>
</div>
</blockquote></div><br></div>
</blockquote></div>
</blockquote></div>
</div></blockquote></div></div></div></div>
</blockquote></div><br></div>
</blockquote></div>
</div></div></blockquote></div><br></div>
</div></div></blockquote></div><br></div>
</blockquote></div>