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

[Alexander Kornienko via cfe-commits]

On Thu, Sep 10, 2015 at 5:22 PM, Aaron Ballman <aaron.ballman at gmail.com>
wrote:

> > ================
> > Comment at: clang-tidy/misc/SizeofContainerCheck.cpp:49
> > @@ +48,3 @@
> > +  SourceLocation SizeOfLoc = SizeOf->getLocStart();
> > +  auto Diag = diag(SizeOfLoc, "sizeof() doesn't return the size of the "
> > +                              "container. Did you mean .size()?");
> > ----------------
> > 1. Done.
> > 2. It's actually emitting the diagnostic. And I thought that the comment
> on line 52 below explains what happens in enough detail (`// Don't generate
> fixes for macros.`). If something is still unclear, can you explain what
> exactly?
>
> It's not intuitive that creating the local variable actually emits the
> diagnostic, so it seems like you would be able to hoist the early
> return up above the local declaration when in fact that would remove
> the diagnostic entirely. The comment about generating fixes suggests
> an additional diagnostic, at least to me.
>

This side effect of the diag() method is one of the core things in the
clang-tidy API for checks. The same pattern is used with other Diag/diag
methods in clang that produce various DiagnosticBuilders, and so far I
didn't see problems with it being misleading. So it shouldn't need a
comment at each usage, imho. May be a comment at the method definition
needs to cover this aspect as well, if it doesn't.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150910/fbc90537/attachment.html>