<div dir="ltr">Two separate issues here<br><br>1) the fixit hint, as one of a set of alternatives, isn't likely to be removed/changed - the (albeit quirky) convention of using extra () to indicate an intentional assignment in a condition has been around for a while. So if you use the extra parens without writing an assignment, the compiler's going to suggest you resolve this "conflict" with the style - either you didn't intend the extra (), or you intended to use assignment. Those are the two alternative suggestions being made.<br><br>2) automatically applying one fixit hint of several ambiguous ones seems like a bug to me - Aaron - do you know anything about this? Is this accurate/intended/etc?</div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Tue, Oct 29, 2019 at 10:13 AM Robert Ankeney <<a href="mailto:rrankene@gmail.com" target="_blank">rrankene@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr">This was just a example of what I ran into when I used run-clang-tidy.py across a compilation database with a -export-fixes=fix.yaml and then ra<div> clang-apply-replacements. Mainly I object to the suggestion+fixit to switch to an assignment. Most coding standards would disallow assignments</div><div>in if conditionals. If anything, I would think a suggestion of "if (true == isValid)" would be more appropriate.</div><div><br></div><div>Thanks for the feedback!</div><div>Robert</div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, Oct 28, 2019 at 2:17 PM David Blaikie <<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr">clang-tidy in the command line you gave didn't seem to modify the file for me, did it modify the file for you? <br><br>Are you objecting to the suggestion, or that it was automatically applied? I would think it'd be a bug to apply any fixit/hint if there are multiple possible suggestions.<br><br>But the existence of the suggestion (without the application of it) to the user seems right to me. The use of extra () to suppress the assignment-in-conditional warning (-Wparentheses) has been around for quite a while, so it's possible that the user intended assignment rather than comparison when they added the extra parentheses.<br><br>- Dave</div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Sun, Oct 27, 2019 at 11:32 AM Robert Ankeney via cfe-users <<a href="mailto:cfe-users@lists.llvm.org" target="_blank">cfe-users@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr">For the following code (wrong.cpp):<div><br></div><div><font face="monospace">bool check(bool isValid)<br>{<br>    bool retVal = false;<br><br>    if (( isValid == true ))<br>    {<br>        retVal = true;<br>    }<br><br>    return retVal;<br>}</font><br></div><div><br></div><div>when I run:</div><div>    clang-tidy -checks=modernize-use-default-member-init wrong.cpp</div><div><br></div><div>I get:</div><div>4 warnings and 1 error generated.<br>Error while processing /llvm/match/ctBad/wrong.cpp.<br>/llvm/match/ctBad/wrong.cpp:5:19: error: equality comparison with extraneous parentheses [clang-diagnostic-parentheses-equality]<br><font face="monospace">    if (( isValid == true ))<br>        ~         ^~      ~<br>                  =</font><br>/llvm/match/ctBad/wrong.cpp:5:19: note: remove extraneous parentheses around the comparison to silence this warning<br>/llvm/match/ctBad/wrong.cpp:5:19: note: use '=' to turn this equality comparison into an assignment<br></div><div><br></div><div>Note it turns the if into:</div><div><font face="monospace">    if ( isValid = true )</font></div><div><br></div><div>Seems like a very bad idea. Removing the redundant parentheses seems fine, but changing the comparison to an assignment does not. Is this a bug?</div><div><br></div><div>Thanks,</div><div>Robert</div><div><br></div></div>
_______________________________________________<br>
cfe-users mailing list<br>
<a href="mailto:cfe-users@lists.llvm.org" target="_blank">cfe-users@lists.llvm.org</a><br>
<a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-users" rel="noreferrer" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-users</a><br>
</blockquote></div>
</blockquote></div>
</blockquote></div>