On Thu, Jul 26, 2018 at 12:52 PM, Richard Smith <rich...@metafoo.co.uk> wrote:
> Other than the (fixed) ffmpeg false positive, I see this firing in three > places. > > One of them is in the libc tests for memset and bzero, where the 0 > argument is intentional. > One of them is here: https://github.com/apache/xerces-c/blob/trunk/ > src/xercesc/util/XMLUTF16Transcoder.cpp#L114 (note that this code is > correct). > And one of them was a real bug where the second and third arguments of > memset were transposed. > > That's an extremely low false positive rate, much lower than what we'd > expect for an enabled-by-default warning. I find this extremely surprising; > I would have expected the ratio of true-- to false-positives to be much > much higher. But ultimately data wins out over expectations. > > How does our experience compare to other people's experiences? Are our > findings abnormal? (They may well be; our corpus is very C++-heavy.) If > this is indeed typical, we should reconsider whether to have these warnings > enabled by default, as they do not meet our bar for false positives. > I am confused by the "low/high" and "meet/do not meet" in this comment. Is this warning currently enabled by default? Is it currently enabled by -Wall? Are you saying that "2-in-3-firings false positive" is sufficiently high that this warning should be disabled by default? Are you saying that "2-in-a-gazillion-lines false positive" is sufficiently low that this warning should be included in -Wall? Are you saying something else? My experience at Green Hills many years ago was that this warning caught a ton of real bugs; but I don't remember if it was "on by default" or in our equivalent of "-Wall" or what. I think it should be enabled by -Wall (or -Wmost), and have no opinion as to whether it should also be on-by-default. –Arthur
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits