On Wed, Oct 30, 2019 at 9:23 PM David Blaikie <dblai...@gmail.com> wrote:
>
> 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?
I also think that's a bug. It looks like it's coming from
Sema::DiagnoseEqualityWithExtraParens(). It looks like it presents
both fixits, which strikes me as a bad thing to do when automatically
applying fixits.
~Aaron
>
> On Tue, Oct 29, 2019 at 10:13 AM Robert Ankeney <rrank...@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 <dblai...@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@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@lists.llvm.org
>>>> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-users
_______________________________________________
cfe-users mailing list
cfe-users@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-users
