This is the mail archive of the gcc@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: Help! Regarding Bug 17896


On 26 January 2016 at 03:24, Manuel LÃpez-IbÃÃez <lopezibanez@gmail.com> wrote:
> On 25 January 2016 at 20:17, Mikhail Maltsev <maltsevm@gmail.com> wrote:
>> As I understand, the bug report suggests that we say "suggest || instead of |
>> when joining booleans" instead. We now have the API to show fix-it hints, so it
>> would be nice to output something like
>>
>> test.c:17:21: warning: suggest || instead of | when joining booleans
>>    foo ((a == b) | b == c);
>>                  ^
>>                  ||
>>
>> Grep 'set_fixit_hint' in the source code to see an example of it's usage.
>
> I think you should focus on one single very simple thing for a first
> patch, either adding fix-it hints to the existing warning or adding
> the alternative text. I would in fact suggest to add fix-it hints
> without touching anything else, since this is probably less
> controversial [*] and more straightforward. How?
>
> 1) Grep 'set_fixit_hint' in gcc/* gcc/cp/* gcc/c/* gcc/c-family/* and
> see how it is used.
> 2) Grep 'suggest parentheses around comparison in operand'
> 3) Run gcc under gdb and stop at the moment the warning above is given.
> 4) Figure out what you need to do to make fix-it hints work in this
> case so we give (spaces are probably messed up by gmail):
>
>  test.c:17:17: note: suggest parentheses around comparison in operand of '|'
>    foo ((a == b) | b == c);
>                  ^
>                    (     )
>
> The rest is the standard for submitting patches:
> https://gcc.gnu.org/wiki/GettingStarted#Basics:_Contributing_to_GCC_in_10_easy_steps
>
> This patch is probably a few lines, so you may not even need a
> copyright assignment. The goal of starting simple is to learn the
> process of submitting a patch without being distracted by the
> discussion about the patch itself. Once you feel confident with the
> process, you can try changing the text of the warning (but then I
> would suggest you find out first what the C/C++ maintainers want
> before implementing it). Note that another easyhack
> (https://gcc.gnu.org/PR38612) is also a matter of changing the warning
> text and the C++ maintainer already gave his blessing to the new text
> in comment  #5.
>
> Cheers,
>
> Manuel.
>
> [*] I don't agree with the suggestion in the bug report. I think we
> should either follow Clang here or we should say something like "&
> will evaluate the boolean expression to its right, did you mean '&&'?"
> The C/C++ maintainers may even have different opinions.


I agree with Manuel (and his comments on
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=17896) that we should
follow clang here and show fix-it hint "& will evaluate the boolean
expression to its right, did you mean '&&'?". That will be more clear
message than "suggest && instead of & when joining booleans".

So I am asking community for their suggestion.


-- 
Thanks and Regards,
Prasad Ghangal


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]