[cfe-commits] [patch] warn on use of non-0/NULL/nullptr expressions in conditional operator operands and comparisons

[David Blaikie]

Apparently our checks for weird null pointer constants (false, '\0',
3-3, non-type template parameters, non-literal const ints, etc) didn't
actually catch cases where these null pointers were used in
conditional operands (x ? ptr : false) or comparisons (cstr == '\0').

This patch fixes that (& I've just updated two test cases to verify
the two cases - conditional and comparison. I could add more to cover
the various null pointer types too, but that doesn't seem to be very
valuable) but has one rather questionable/wrong/discussion-worthy
part, based on the way I've solved this. The solution was simply to
move these checks from where they are to where they probably should
be: SemaChecking.cpp

The problem with SemaChecking's general approach is that it basically
tries to find the maximally distinct types that don't involve any
explicit constructs along the way (eg: find the target type through
which a particular expression is ultimately used then compare that to
the intrinsic type of the expression, ignoring all the implicit stuff
in between - if the difference is interesting, warn). This includes
SubstNonTypeTemplateParmExprs - it walks right through those. As long
as it does that, this warning as I've written it won't catch this
case:

template<int X> void func() { int *i = X; }
...
func<0>();

But, honestly, I don't think I've found any instances of this apart
from in LLVM's test suite. For the purposes of not regressing the test
suite I tried a dirty hack (you'll see this in the attached patch)
where "IgnoreParenImpCasts" doesn't ignore
SubstNonTypeTemplateParmExprs - this maintains compatibility and
doesn't break any tests... but I really expect it should (break some
other tests) I just don't know what those tests are. This function is
called from a variety of places, and I expect some of them might care
(but perhaps they actually want the behavior change I've made - who
knows)

So I just thought I'd put this out there - chances are the
simplest/low-risk thing to do is to revert my change to
IgnoreParenImpCasts and modify the one test case that covers dependent
null pointer expressions such as that (like I said, doesn't really
catch any bugs that I've seen)
-------------- next part --------------
A non-text attachment was scrubbed...
Name: null_comparisons.diff
Type: application/octet-stream
Size: 4884 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120815/51afaf63/attachment.obj>