On 26 January 2016 at 03:24, Manuel López-Ibáñez <lopeziba...@gmail.com> wrote: > 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.
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
