[cfe-users] clang-tidy bug?

[David Blaikie via cfe-users]

Two separate issues here

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.

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?

On Tue, Oct 29, 2019 at 10:13 AM Robert Ankeney <rrankene at gmail.com> wrote:

> 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
>  clang-apply-replacements. Mainly I object to the suggestion+fixit to
> switch to an assignment. Most coding standards would disallow assignments
> in if conditionals. If anything, I would think a suggestion of "if (true
> == isValid)" would be more appropriate.
>
> Thanks for the feedback!
> Robert
>
> On Mon, Oct 28, 2019 at 2:17 PM David Blaikie <dblaikie at gmail.com> wrote:
>
>> clang-tidy in the command line you gave didn't seem to modify the file
>> for me, did it modify the file for you?
>>
>> 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.
>>
>> 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.
>>
>> - Dave
>>
>> On Sun, Oct 27, 2019 at 11:32 AM Robert Ankeney via cfe-users <
>> cfe-users at lists.llvm.org> wrote:
>>
>>> For the following code (wrong.cpp):
>>>
>>> bool check(bool isValid)
>>> {
>>>     bool retVal = false;
>>>
>>>     if (( isValid == true ))
>>>     {
>>>         retVal = true;
>>>     }
>>>
>>>     return retVal;
>>> }
>>>
>>> when I run:
>>>     clang-tidy -checks=modernize-use-default-member-init wrong.cpp
>>>
>>> I get:
>>> 4 warnings and 1 error generated.
>>> Error while processing /llvm/match/ctBad/wrong.cpp.
>>> /llvm/match/ctBad/wrong.cpp:5:19: error: equality comparison with
>>> extraneous parentheses [clang-diagnostic-parentheses-equality]
>>>     if (( isValid == true ))
>>>         ~         ^~      ~
>>>                   =
>>> /llvm/match/ctBad/wrong.cpp:5:19: note: remove extraneous parentheses
>>> around the comparison to silence this warning
>>> /llvm/match/ctBad/wrong.cpp:5:19: note: use '=' to turn this equality
>>> comparison into an assignment
>>>
>>> Note it turns the if into:
>>>     if ( isValid = true )
>>>
>>> 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?
>>>
>>> Thanks,
>>> Robert
>>>
>>> _______________________________________________
>>> cfe-users mailing list
>>> cfe-users at lists.llvm.org
>>> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-users
>>>
>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-users/attachments/20191030/f4dfe73c/attachment.html>