On 25 January 2016 at 20:17, Mikhail Maltsev <malts...@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.
