On Thu, Sep 15, 2016 at 8:16 AM, Marek Polacek <pola...@redhat.com> wrote: > On Wed, Sep 14, 2016 at 12:07:49AM -0400, Jason Merrill wrote: >> On Sat, Sep 10, 2016 at 10:58 AM, Marek Polacek <pola...@redhat.com> wrote: >> > Spurred by the recent <http://www.viva64.com/en/b/0425/> findings, I >> > decided to >> > implement a warning that warns when a pointer is compared with a zero >> > character >> > literal (constant), because this isn't likely to be intended. So e.g. >> > >> > ptr == L'\0' >> > >> > is probably wrong and should've been written as >> > >> > ptr[0] == L'\0' >> > >> > Jonathan pointed out that this would actually be invalid C++11 since >> > pointer >> > conversions are only allowed for integer literals, not char literals. >> >> Ah, indeed. And if we fix that, we get an error rather than a >> warning. Maybe let's handle this by wrapping character literals in a >> redundant NOP_EXPR? > > So I've tried. Wrapping character literals in a NOP_EXPR would make us > error on that comparison (good), but it breaks -Wchar-subscripts, which > could be solved by adding STRIP_NOPS, but unfortunately it breaks even > -Wmemset-transposed-args: '\0' would become (char) '\0', which is not a > literal zero anymore. And if I do sth like > + if (TREE_CODE (arg2) == NOP_EXPR > + && TREE_TYPE (arg2) == TREE_TYPE (TREE_OPERAND (arg2, 0))) > + arg2 = TREE_OPERAND (arg2, 0); > then (int) 0 would became a literal zero and we'd warn when not appropriate. > > We should also error for e.g. void *p = '\0'; but the problem with the > NOP_EXPR cast is that convert_for_init strips nops, so we wouldn't reject > this code.
Good point. > So it seems we may need some CHARACTER_CST or somesuch? 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. > Note that we should also take boolean-literals into account. We already check for INTEGER_TYPE, so boolean literals should already be excluded. Jason
