On Fri, Sep 30, 2016 at 12:43 PM, Marek Polacek <pola...@redhat.com> wrote: > On Fri, Sep 23, 2016 at 10:31:33AM -0400, Jason Merrill wrote: >> On Fri, Sep 23, 2016 at 9:15 AM, Marek Polacek <pola...@redhat.com> wrote: >> > On Wed, Sep 21, 2016 at 03:52:09PM -0400, Jason Merrill wrote: >> >> On Mon, Sep 19, 2016 at 2:49 PM, Jason Merrill <ja...@redhat.com> wrote: >> >> > I suppose that an INTEGER_CST of character type is necessarily a >> >> > character constant, so adding a check for !char_type_p ought to do the >> >> > trick. >> >> >> >> Indeed it does. I'm checking this in: >> > >> > Nice, thanks. What about the original patch? We still need to warn >> > (or error for C++11) for pointer comparisons. >> >> If we still accept pointer comparisons in C++, that's another bug with >> treating \0 as a null pointer constant. This seems to be because >> ocp_convert of \0 to int produces an INTEGER_CST indistinguishable >> from literal 0. > > I was trying to fix this in ocp_convert, by using NOP_EXPRs, but that wasn't > successful. But since we're interested in ==/!=, I think this can be fixed > easily in cp_build_binary_op. Actually, all that seems to be needed is using > orig_op as the argument to null_ptr_cst_p, but that wouldn't give the correct > diagnostics, so I did this. By checking orig_op we can see if the operands > are > character literals or not, because orig_op is an operand before the default > conversions.
What is wrong about the diagnostic from just using orig_op? "ISO C++ forbids comparison between pointer and integer" seems fine to me, and will help the user to realize that they need to index off the pointer. I see that some of the calls to null_ptr_cst_p in cp_build_binary_op have already been changed to check orig_op*, but not all. Let's update the remaining calls, that should do the trick without adding a new error. Jason
