[PATCH] D12759: [clang-tidy] Add misc-sizeof-container check to find sizeof() uses on stlcontainers.

Aaron Ballman via cfe-commits cfe-commits at lists.llvm.org
Wed Sep 16 14:34:54 PDT 2015


On Wed, Sep 16, 2015 at 5:33 PM, Alexander Kornienko <alexfh at google.com> wrote:
> It was about a hundred in a huge codebase. It's definitely manageable, but
> the experiment has shown that this kind of a mistake is not likely to
> happen.

Fair enough, let's axe it until we see evidence it may catch real bugs.

~Aaron

>
> On 16 Sep 2015 23:25, "Aaron Ballman" <aaron.ballman at gmail.com> wrote:
>>
>> On Wed, Sep 16, 2015 at 5:21 PM, Alexander Kornienko <alexfh at google.com>
>> wrote:
>> > All found results were intended usages of sizeof on containers. 100%
>> > false
>> > positive rate that is.
>>
>> Yes, but is that 4 results in 10MM LoC, or 4000 results in 40k LoC?
>> ;-) I guess I just don't have a good feel for how large the codebase
>> is, and how many times it resulted in matching sane code. If it's
>> really low (like 4 out of 10MM LoC), I think the checker may possibly
>> be useful (just not in that code base). If it's anything remotely
>> high, then I would agree, let's ditch it and not look back.
>>
>> ~Aaron
>>
>> >
>> > On 16 Sep 2015 21:23, "Aaron Ballman" <aaron.ballman at gmail.com> wrote:
>> >>
>> >> On Wed, Sep 16, 2015 at 2:21 PM, Alexander Kornienko
>> >> <alexfh at google.com>
>> >> wrote:
>> >> > An update: I didn't find a single bug with this check in a large
>> >> > codebase.
>> >> > Turns out that it's rather useless. I'm inclined to kill it.
>> >>
>> >> How bad is the false positive rate?
>> >>
>> >> ~Aaron
>> >>
>> >> >
>> >> >
>> >> > On Sun, Sep 13, 2015 at 11:22 AM, Alexander Kornienko
>> >> > <alexfh at google.com>
>> >> > wrote:
>> >> >>
>> >> >> I've also found a bunch of similar cases in our codebase, and I'm
>> >> >> trying
>> >> >> to figure out whether the pattern can be narrowed down to just
>> >> >> dangerous
>> >> >> cases. If we don't find a way to do so, we'll probably have to
>> >> >> resort
>> >> >> to "//
>> >> >> NOLINT" to shut clang-tidy up.
>> >> >>
>> >> >> On 13 Sep 2015 10:52, "Kim Gräsman" <kim.grasman at gmail.com> wrote:
>> >> >>>
>> >> >>> Late to the party, but I wanted to ask: is there a way to indicate
>> >> >>> to
>> >> >>> the checker that we really *did* mean sizeof()?
>> >> >>>
>> >> >>> I think I've stumbled over code in our code base that uses
>> >> >>> sizeof(container) to report memory usage statistics and it seems
>> >> >>> valid, so it'd be nice if this checker could be silenced on a
>> >> >>> case-by-case basis.
>> >> >>>
>> >> >>> Thanks,
>> >> >>> - Kim
>> >> >>>
>> >> >>> On Sat, Sep 12, 2015 at 12:09 AM, Alexander Kornienko via
>> >> >>> cfe-commits
>> >> >>> <cfe-commits at lists.llvm.org> wrote:
>> >> >>> > Indeed. But this has been fixed before I could get to it.
>> >> >>> >
>> >> >>> >
>> >> >>> > On Thu, Sep 10, 2015 at 10:47 PM, Aaron Ballman via cfe-commits
>> >> >>> > <cfe-commits at lists.llvm.org> wrote:
>> >> >>> >>
>> >> >>> >> aaron.ballman added a comment.
>> >> >>> >>
>> >> >>> >> This appears to have broken one of the bots:
>> >> >>> >>
>> >> >>> >> http://bb.pgr.jp/builders/ninja-x64-msvc-RA-centos6/builds/15065
>> >> >>> >>
>> >> >>> >>
>> >> >>> >> http://reviews.llvm.org/D12759
>> >> >>> >>
>> >> >>> >>
>> >> >>> >>
>> >> >>> >> _______________________________________________
>> >> >>> >> cfe-commits mailing list
>> >> >>> >> cfe-commits at lists.llvm.org
>> >> >>> >> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>> >> >>> >
>> >> >>> >
>> >> >>> > _______________________________________________
>> >> >>> > cfe-commits mailing list
>> >> >>> > cfe-commits at lists.llvm.org
>> >> >>> > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>> >> >>> >
>> >> >
>> >> >


More information about the cfe-commits mailing list