This is the mail archive of the
mailing list for the GCC project.
Re: Help! Regarding Bug 17896
- From: Manuel LÃpez-IbÃÃez <lopezibanez at gmail dot com>
- To: Mikhail Maltsev <maltsevm at gmail dot com>
- Cc: Prasad Ghangal <prasad dot ghangal at gmail dot com>, gcc Mailing List <gcc at gcc dot gnu dot org>
- Date: Mon, 25 Jan 2016 21:54:28 +0000
- 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>
On 25 January 2016 at 20:17, Mikhail Maltsev <email@example.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:
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.
[*] 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.