Improving -Wunused-member-function

Nico Weber thakis at chromium.org
Wed Mar 26 14:41:09 PDT 2014


Hi Nuno,

this looks like a cool warning. I tried it on chromium; it looks like the
implementation could use some polishing.

* We have a DISALLOW_COPY_AND_ASSIGN macro that expands to a constructor
name, to be places in a "private:" section to make a class not
instantiatable (
https://code.google.com/p/chromium/codesearch#chromium/src/base/macros.h&l=45).
The warning complains about these constructors. Maybe it shouldn't
warn
on declared-but-not-defined constructors / destructors / assignment
operators?

* It complains about private functions that override pure virtual methods –
but removing them leads to "can't instantiate class with pure virtual
methods" errors.

Nico


On Sun, Mar 23, 2014 at 3:14 PM, Nuno Lopes <nunoplopes at sapo.pt> wrote:

> 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).
>
> The second part of the plan is to mark all methods described previously
> (well, the used ones) with internal linkage. In my naive view it seems
> fine, but is this legal per the standard?  I think it can be an important
> optimization, since then LLVM can more freely inline and remove the symbols
> altogether.
>
> 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.
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140326/656df5f0/attachment.html>


More information about the cfe-commits mailing list