On Aug 17, 2012, at 15:43 , David Blaikie <[email protected]> wrote:
> On Fri, Aug 17, 2012 at 3:33 PM, Nico Weber <[email protected]> wrote: >> Should this really be on by default? On chrome, this triggers a single >> time (linux-only): >> >> ../../third_party/tcmalloc/chromium/src/stack_trace_table.cc:138:16: >> warning: expression which evaluates to zero treated as a null pointer >> constant of type 'void *' [-Wnon-literal-null-conversion] >> out[idx++] = static_cast<uintptr_t>(0); >> ^~~~~~~~~~~~~~~~~~~~~~~~~ >> >> out is declared as `void** out = new void*[out_len];`. The warning >> isn't wrong, but it looks rather pedantic to me. Should this be only >> in -Wall (or maybe even in -pedantic)? > > Might be a fair candidate for -Wall, though it did find some > reasonable stuff in google. 18 cases overall with some fairly > interesting ones (see b/6954211 for the ones that've been committed so > far, or cl/32692314 for some of the remaining ones. > > The worst offenders are integer constants with value 0 that aren't at > all intended to be pointers. (most easily occurred in function calls > where the caller thought the argument was of one type but it's > actually of a pointer type) > > I have some more once this warning opens up to cover comparisons, > conditional operands, and return statements - there's a lot of > confusing "cstr == '\0'" code where the user probably meant to deref > the lhs but didn't. IMHO, this should remain on by default. The Chromium example clearly shows an impedance mismatch between the array and the value being stored. I would say it's not unlikely that at one point the array was a uintptr_t*, but was changed, and this part of the code wasn't updated to match because it didn't warn. But I can see the argument that "because this isn't harmful, we shouldn't warn unless asked to". _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
