This is the mail archive of the
gcc@gcc.gnu.org
mailing list for the GCC project.
Re: Help! Regarding Bug 17896
- From: Prasad Ghangal <prasad dot ghangal at gmail dot com>
- To: gcc at gcc dot gnu dot org
- Cc: Mikhail Maltsev <maltsevm at gmail dot com>, Manuel LÃpez-IbÃÃez <lopezibanez at gmail dot com>, prathamesh dot kulkarni at linaro dot org
- Date: Thu, 28 Jan 2016 13:40:31 +0530
- Subject: Re: Help! Regarding Bug 17896
- Authentication-results: sourceware.org; auth=none
- References: <CAE+uiWZ46B6OffCFU38rgL7DExd=BCQ9VPo4KQwnLLooJT1Grw at mail dot gmail dot com> <56A682DA dot 7030804 at gmail dot com> <CAESRpQAF03a=7MKgsbafadgp7e2gfgVjwt_dZDC8LJJv11onOw at mail dot gmail dot com>
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