Improving -Wunused-member-function

[Richard Smith]

On Fri, Apr 18, 2014 at 5:40 AM, Nuno Lopes <nunoplopes at sapo.pt> wrote:

> Hi,
>
> Please find in attach a second version of the patch, which reuses the
> cache of defined records properly.
> With this patch, -Wunused-member-function now flags unused private methods
> whenever the class is fully defined.
> This patch does not attempt to fix false positives that were being
> triggered before in classes in anonymous namespaces.  I'll fix those
> afterwards.
>
> OK to commit?
>

No, it's not OK to delete the early pruning of UnusedFileScopedDecls.
Without that, we'll load in all unused declarations within a module for
every compilation that includes the module.

The approach taken by warning and the existing unused private field warning
interact badly with modules in general -- they force us to deserialize most
private members within every imported module, just in case the class became
completely-defined within this compilation, because their approach is "for
each maybe-unused thing, check if it's unused, then diagnose". Instead of
the current approach, we could go for "for each class completed in this
compilation, check for unused private members".


> Thanks,
> Nuno
>
> ----- Original Message ----- From: Nuno Lopes
> Sent: Sunday, March 23, 2014 11:14 PM
> Subject: Improving -Wunused-member-function
>
>
>  Hi,
>>
>> I would like to improve -Wunused-member-function to detect unused private
>> methods, similarly to how -Wunused-private-fields works.
>> I think clang should be able to flag a method if 1) it is unused, 2) all
>> the
>> methods of the class are defined in the TU, and 3) any of the following
>> conditions holds:
>> - The method is private.
>> - The method is protected and the class is final.
>> - The method is public and the class is in an anonymous namespace.
>>
>> I have a simple implementation in attach that can handle the first case
>> (private methods) only.
>> I'm not very happy with it, though. In particular I would like to move the
>> logic somewhere else, so that we can reuse it from Codegen. And right now
>> I'm not caching things properly.  Any suggestions to where this code
>> belongs?  Should it go directly to Decl? (but that would imply adding a
>> few
>> fields for cache purposes).
>>
>> Any comments and suggestions are welcomed!
>>
>> Thanks,
>> Nuno
>>
>> P.S.: I run the attached patch over the LLVM codebase and I already fixed
>> a
>> bunch of cases it detected (but left many still). So big code bases will
>> certainly benefit from this analysis. Moreover, removing unused decls
>> triggered more -Wunused-private-fields  warnings.
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140418/de4dda93/attachment.html>