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

[Aaron Ballman via cfe-commits]

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
>> >>> >
>> >
>> >