[cfe-commits] Clang change to detect and warn about unused private members

[Nico Weber]

On Wed, Jan 11, 2012 at 1:49 PM, Nico Weber <thakis at chromium.org> wrote:
> Hi Daniel,
>
> On Wed, Jan 11, 2012 at 1:11 AM, Daniel Jasper <djasper at google.com> wrote:
>> Hi,
>>
>> the attached change is designed to detect and warn about private unused
>> members of C++ classes. It checks whether a class is fully defined in the
>> current translation unit, i.e. all methods are either defined or pure
>> virtual and all friends are defined. Otherwise, the private member could be
>> used by a function defined in another translation unit.
>
> that's a cool warning! Here are a few cases where it flags false
> positives (it finds tons of true positives too, but those are boring
> :-) ).
>
> It flags |stackArray| in ICU's cmemory.h. This is declared for storage
> and accessed through pointer aliasing, probably not much that can be
> done about this:
> http://codesearch.google.com/codesearch#OAMlx_jo-ck/src/third_party/icu/source/common/cmemory.h&exact_package=chromium&q=file:cmemory.h&l=285

(Sorry, I highlighted the wrong line. It complains about line 460, but
as zygoloid and chandler commented off-list, that class has undefined
behavior anyway.)

>
> In flags StaticResource::instance_ in v8's utils.h:
> http://codesearch.google.com/codesearch#OAMlx_jo-ck/src/v8/src/utils.h&exact_package=chromium&q=file:v8/src/utils.h&l=300
> This is supposed to be accessed through the class below, Access, which
> is a friend of utils.h. Do you check if any friends use private
> variables?
>
> In chromium itself, it flags ShadowingAtExitManager member variables.
> This is basically a RAII class which only exists to call a protected
> superclass constructor, and which only exists if UNIT_TESTS is
> defined: http://codesearch.google.com/codesearch#OAMlx_jo-ck/src/base/at_exit.h&l=71
> So maybe the "constructor without arguments" heuristic could be
> tweaked to exclude constructors that call superclass constructors with
> arguments?
> (Here's another RAII class whose constructor takes 0 arguments:
> http://codesearch.google.com/codesearch#OAMlx_jo-ck/src/base/logging_unittest.cc&exact_package=chromium&q=logstatesaver&l=26
> )
>
> (This is not an exhaustive list, just the things I found after a quick look.)
>
> Nico